PR 6: refactor: per-area small refactors (CTS, Scheduler, Students, CMS)#179
Open
rlorenzo wants to merge 6 commits into
Open
PR 6: refactor: per-area small refactors (CTS, Scheduler, Students, CMS)#179rlorenzo wants to merge 6 commits into
rlorenzo wants to merge 6 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds IScheduleEntity and ApplyScheduleFilters; refactors Codecs to map-driven EncodeWithMap/DecodeWithMap and adds UU tests; consolidates StudentGroupService projection/photo/Ross IAM logic with relational tests; converts CourseStudents.vue logged-in rows to data-driven rendering. ChangesSchedule Filtering Abstraction
Codec Parameterization and Testing
StudentGroupService Consolidation
StudentGroupService Relational Tests
CourseStudents Table Data-Driven Rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
25b942b to
d0b2e7f
Compare
rlorenzo
force-pushed
the
refactor/dead-code-and-shared-chrome
branch
from
ebe32ec to
75abeb2
Compare
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
d0b2e7f to
0ea0117
Compare
rlorenzo
force-pushed
the
refactor/dead-code-and-shared-chrome
branch
5 times, most recently
from
d26c5d8 to
8cbfb69
Compare
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
0ea0117 to
0fa6909
Compare
rlorenzo
force-pushed
the
refactor/dead-code-and-shared-chrome
branch
2 times, most recently
from
0d62a4e to
a0c55c4
Compare
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
0fa6909 to
5cbd5f4
Compare
rlorenzo
force-pushed
the
refactor/dead-code-and-shared-chrome
branch
from
a0c55c4 to
d8a3735
Compare
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
5cbd5f4 to
8eb247a
Compare
rlorenzo
force-pushed
the
refactor/dead-code-and-shared-chrome
branch
from
d8a3735 to
ddd64e7
Compare
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
8eb247a to
a461dcd
Compare
rlorenzo
force-pushed
the
refactor/dead-code-and-shared-chrome
branch
from
ddd64e7 to
66fe537
Compare
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
a461dcd to
6d0722d
Compare
3 tasks
rlorenzo
force-pushed
the
refactor/dead-code-and-shared-chrome
branch
from
66fe537 to
d09da71
Compare
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
6d0722d to
9c30e16
Compare
Bundle ReportChanges will decrease total bundle size by 894 bytes (-0.04%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: viper-frontend-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #179 +/- ##
==========================================
+ Coverage 43.02% 44.21% +1.18%
==========================================
Files 881 894 +13
Lines 51436 51604 +168
Branches 4812 4810 -2
==========================================
+ Hits 22131 22815 +684
+ Misses 28779 28225 -554
- Partials 526 564 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
9c30e16 to
743f4e5
Compare
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
743f4e5 to
b8152a2
Compare
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/Areas/Students/Services/StudentGroupService.cs (1)
407-433: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd
.AsNoTracking()to read-only query.Same issue. Insert
.AsNoTracking()before.OrderBy().⚡ Suggested fix
sg != null ? sg.StudentgrpV3grp : null); // Apply group filtering if specified if (!string.IsNullOrEmpty(groupType) && !string.IsNullOrEmpty(groupId)) { query = groupType.ToLower() switch { "eighths" => query.Where(s => s.EighthsGroup == groupId), "twentieths" => query.Where(s => s.TwentiethsGroup == groupId), "teams" => query.Where(s => s.TeamNumber == groupId), "v3specialty" => query.Where(s => s.V3SpecialtyGroup == groupId), _ => query }; } -var students = await query.OrderBy(s => s.LastName).ThenBy(s => s.FirstName).ToListAsync(); +var students = await query.AsNoTracking().OrderBy(s => s.LastName).ThenBy(s => s.FirstName).ToListAsync();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/Areas/Students/Services/StudentGroupService.cs` around lines 407 - 433, The LINQ query building a sequence of StudentBaseRecord is read-only and should use AsNoTracking to avoid EF change tracking overhead; update the code that constructs "query" (the projection creating new StudentBaseRecord in StudentGroupService) to call .AsNoTracking() on the IQueryable before applying .OrderBy(...).ThenBy(...)/ToListAsync(), preserving any groupType filtering logic (the switch) so call .AsNoTracking() on the final query instance just prior to ordering and materialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/CMS/CodecsTests.cs`:
- Line 4: The namespace declared as Test.CMS doesn't match the project's
expected root namespace; update the namespace declaration to Viper.test.CMS
(replace the Test.CMS namespace token) so the class/test types in this file use
the Viper.test.CMS namespace and satisfy the ReSharper/CI checks.
In `@web/Areas/Students/Services/StudentGroupService.cs`:
- Around line 765-772: The query against _sisContext.StudentDesignations in
StudentGroupService should be executed as a read-only query; insert
.AsNoTracking() immediately after _sisContext.StudentDesignations (before the
existing .Where(...) calls) so the EF Core change tracker is not used for this
read-only projection (retain the rest of the chain:
.Where(...).Select(...).Where(...).Distinct().ToListAsync()).
- Line 738: Remove the null-forgiving operator from student.MailId in
StudentGroupService.cs and handle nullable MailId properly: either filter the
students iteration to only include those with MailId != null before mapping, or
assign MailId = student.MailId (no !) and ensure the consumer (e.g., the
StudentPhoto model or downstream code) accepts a nullable MailId; locate the
mapping where MailId = student.MailId! and replace it with a safe nullable-aware
approach consistent with how MailId is handled elsewhere (see other assignments
around lines with StudentPhoto.MailId).
- Around line 113-126: The LINQ query building StudentBaseRecord instances is
read-only but currently tracks entities; update the query used in
StudentGroupService (the variable named query that projects to
StudentBaseRecord) to call .AsNoTracking() before the OrderBy/ThenBy and
ToListAsync so EF Core doesn't enable change tracking for this read-only
projection; locate the projection creating new StudentBaseRecord and insert
.AsNoTracking() on that IQueryable prior to .OrderBy(s => s.LastName).ThenBy(s
=> s.FirstName).ToListAsync().
- Around line 341-354: The query projection creating StudentBaseRecord from
queryBase should be treated as read-only—modify the LINQ chain so that the
IQueryable returned by the Select (the variable named query) uses AsNoTracking()
before applying OrderBy/ThenBy; locate the Select(...) producing
StudentBaseRecord in StudentGroupService (the variable query) and insert
.AsNoTracking() on that query before calling OrderBy(s => s.LastName).ThenBy(s
=> s.FirstName).ToListAsync().
---
Outside diff comments:
In `@web/Areas/Students/Services/StudentGroupService.cs`:
- Around line 407-433: The LINQ query building a sequence of StudentBaseRecord
is read-only and should use AsNoTracking to avoid EF change tracking overhead;
update the code that constructs "query" (the projection creating new
StudentBaseRecord in StudentGroupService) to call .AsNoTracking() on the
IQueryable before applying .OrderBy(...).ThenBy(...)/ToListAsync(), preserving
any groupType filtering logic (the switch) so call .AsNoTracking() on the final
query instance just prior to ordering and materialization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d79bf74a-c6b5-4d4d-84dc-7f186950c9ab
📒 Files selected for processing (10)
VueApp/src/CTS/pages/CourseStudents.vuetest/CMS/CodecsTests.csweb/Areas/CMS/Data/Codecs.csweb/Areas/ClinicalScheduler/Services/InstructorScheduleService.csweb/Areas/ClinicalScheduler/Services/ScheduleQueryExtensions.csweb/Areas/ClinicalScheduler/Services/StudentScheduleService.csweb/Areas/Students/Services/StudentGroupService.csweb/Models/CTS/IScheduleEntity.csweb/Models/CTS/InstructorSchedule.csweb/Models/CTS/StudentSchedule.cs
Contributor
Author
|
@CodeRabbit review |
Contributor
Action performedReview triggered.
|
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
b25beef to
83489ac
Compare
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
VueApp/src/CTS/pages/CourseStudents.vue (1)
48-84:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftReplace native table row/cell markup with Quasar table components in this changed block.
Line 48 introduces a
v-foron native<tr>/<td>, which violates the Vue guideline requiring Quasar components instead of HTML elements. Refactor this table rendering toq-table(or Quasar table primitives) so this section is compliant.As per coding guidelines, "
**/*.vue: Use Quasar components (q-btn,q-dialog,q-banner, etc.) instead of HTML elements."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@VueApp/src/CTS/pages/CourseStudents.vue` around lines 48 - 84, The native <tr>/<td> loop rendering loggedInStudents must be replaced with Quasar table primitives: use <q-table> with :rows="loggedInStudents" and a body slot (or use QTr/QTd components) instead of v-for on <tr>; map columns for name/time and render the action cells using the existing q-btn/q-icon inside QTd, keeping the :key as row.id and preserving the AssessmentCompetency route construction and aria-labels (refer to loggedInStudents, AssessmentCompetency route string, and the q-btn/q-icon instances) so the table uses Quasar components throughout.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/Students/StudentGroupServiceQueryTests.cs`:
- Around line 200-236: Add a negative Ross fixture to the existing test
GetStudentsByCourseAsync_IncludeRoss_ReturnsRossStudentEnrolledInCourse: create
another SeedStudent + StudentDesignation pair (use a distinct pkey/pidm like
"P10"/"PIDM10" and matching IamId) and add a Roster entry for that pidm with
RosterCrn "12345" but set RosterEnrollStatus to a non-"RE" value (e.g. "WD" or
"DR"). Save to _sisContext and _coursesContext like the others, then keep the
existing call to _service.GetStudentsByCourseAsync(Term, "12345",
includeRossStudents: true) and add an assertion that this non-RE Ross student is
not present in result (e.g. assert no s with LastName matching the new seed or
that result.Count remains 2). Ensure you reference the existing SeedStudent,
StudentDesignation, Roster, _sisContext/_coursesContext and
GetStudentsByCourseAsync in the change.
In `@web/Areas/ClinicalScheduler/Services/ScheduleQueryExtensions.cs`:
- Around line 22-25: The current ScheduleQueryExtensions implementation uses
Week.WeekGradYears.Any(...) inside the Where, causing a correlated subquery;
instead preload the matching WeekIds from WeekGradYears (filter by GradYear and
Select WeekId/Distinct) and change the predicate to use
weekIds.Contains(s.WeekId). Add an overload on the
ApplyScheduleFilters/extension in ScheduleQueryExtensions that accepts an
IEnumerable<int> weekIds (or accept a nullable preloaded list) and, when
provided, apply query = query.Where(s => weekIds.Contains(s.WeekId)); keep the
original signature behavior when weekIds is null so callers that preload WeekIds
(from WeekGradYears where GradYear == classYear) can pass them in and avoid the
per-row Any().
In `@web/Areas/Students/Services/StudentGroupService.cs`:
- Around line 741-755: The GroupAssignment is being built from untrimmed
student.EighthsGroup and student.TwentiethsGroup which causes padded values to
appear even though the DTO fields are trimmed; fix by trimming those inputs
before calling FormatGroupAssignment (use student.EighthsGroup?.Trim() and
student.TwentiethsGroup?.Trim() when computing groupAssignment) so that
FormatGroupAssignment receives normalized values; ensure the existing
EighthsGroup and TwentiethsGroup assignments on the StudentPhoto DTO remain
trimmed as they are.
- Around line 447-459: The Ross-only lookup currently re-queries Rosters into
courseRosterPidms without the original RosterEnrollStatus == "RE" filter, which
can reintroduce withdrawn/waitlisted students; change the second query to reuse
the already-filtered enrolledPidms (the variable created for enrolled students)
instead of courseRosterPidms so the EF query in StudentGroupService (the
rossStudentsInCourse AAUD lookup) only considers actively enrolled PIDMs and
avoids re-adding non-enrolled Ross students.
---
Outside diff comments:
In `@VueApp/src/CTS/pages/CourseStudents.vue`:
- Around line 48-84: The native <tr>/<td> loop rendering loggedInStudents must
be replaced with Quasar table primitives: use <q-table> with
:rows="loggedInStudents" and a body slot (or use QTr/QTd components) instead of
v-for on <tr>; map columns for name/time and render the action cells using the
existing q-btn/q-icon inside QTd, keeping the :key as row.id and preserving the
AssessmentCompetency route construction and aria-labels (refer to
loggedInStudents, AssessmentCompetency route string, and the q-btn/q-icon
instances) so the table uses Quasar components throughout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 52013b18-6c8b-49ac-997d-893b713acd14
📒 Files selected for processing (11)
VueApp/src/CTS/pages/CourseStudents.vuetest/CMS/CodecsTests.cstest/Students/StudentGroupServiceQueryTests.csweb/Areas/CMS/Data/Codecs.csweb/Areas/ClinicalScheduler/Services/InstructorScheduleService.csweb/Areas/ClinicalScheduler/Services/ScheduleQueryExtensions.csweb/Areas/ClinicalScheduler/Services/StudentScheduleService.csweb/Areas/Students/Services/StudentGroupService.csweb/Models/CTS/IScheduleEntity.csweb/Models/CTS/InstructorSchedule.csweb/Models/CTS/StudentSchedule.cs
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
829c1dd to
0366b1b
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/Areas/Students/Services/StudentGroupService.cs (1)
459-463:⚠️ Potential issue | 🟠 MajorFix unparameterized Ross IAM membership
Contains(useEF.Parameter)
rossStudentsInCourse.Contains(i.IdsIamId)is still a plain list membership check inside the EF query; parameterize it like the other Ross filters to keep SQL translation consistent.🔧 Suggested fix
- where i.IdsIamId != null && rossStudentsInCourse.Contains(i.IdsIamId) + where i.IdsIamId != null && EF.Parameter(rossStudentsInCourse).Contains(i.IdsIamId!)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/Areas/Students/Services/StudentGroupService.cs` around lines 459 - 463, The LINQ query uses rossStudentsInCourse.Contains(i.IdsIamId) unparameterized; create an EF parameter for the collection and use that in the query. Before building the query, create a parameter (e.g., var rossStudentsParam = EF.Parameter(rossStudentsInCourse) or the project-specific EF.Parameter helper) and replace rossStudentsInCourse.Contains(i.IdsIamId) with rossStudentsParam.Contains(i.IdsIamId) inside the query that references _aaudContext.Ids / People / Students (symbols: rossStudentsInCourse, rossStudentsParam, i.IdsIamId, _aaudContext).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/Students/StudentGroupServiceQueryTests.cs`:
- Around line 108-151: SeedStudent currently always sets IdsMailid non-null so
the MailId ?? string.Empty branch in BuildStudentPhotoListAsync is untested;
modify the test fixtures by adding one student seeded via SeedStudent (or a new
overload/flag) with IdsMailid set to null (i.e., create an enrolled fixture
where IdsMailid is null) and then assert the returned photo list entry has
MailId == string.Empty; update assertions around BuildStudentPhotoListAsync to
include this null-mail regression case and ensure the new fixture is included in
the test ranges noted (also apply the same change for the other tests
referenced).
In `@web/Areas/Students/Services/StudentGroupService.cs`:
- Around line 718-754: BuildStudentPhotoListAsync currently manually projects
StudentBaseRecord -> StudentPhoto with a foreach; create a Mapperly mapping
(e.g., IStudentMapper or StudentMappers) that maps StudentBaseRecord to
StudentPhoto and refactor BuildStudentPhotoListAsync to use LINQ
students.Select(s => mapper.Map(s, photoUrls, defaultPhotoUrl)).ToList() instead
of the foreach; to enable the mapper to perform the same per-row logic, relocate
the helper methods ResolvePhotoUrl, FormatStudentDisplayName, and
FormatGroupAssignment so they are accessible to the mapper (make them
internal/static or move into the mapper partial), and ensure the mapper
signature accepts the photoUrls dictionary and defaultPhotoUrl so
HasPhoto/PhotoUrl, TeamNumber/V3SpecialtyGroup logic and trimming are applied
identically.
---
Outside diff comments:
In `@web/Areas/Students/Services/StudentGroupService.cs`:
- Around line 459-463: The LINQ query uses
rossStudentsInCourse.Contains(i.IdsIamId) unparameterized; create an EF
parameter for the collection and use that in the query. Before building the
query, create a parameter (e.g., var rossStudentsParam =
EF.Parameter(rossStudentsInCourse) or the project-specific EF.Parameter helper)
and replace rossStudentsInCourse.Contains(i.IdsIamId) with
rossStudentsParam.Contains(i.IdsIamId) inside the query that references
_aaudContext.Ids / People / Students (symbols: rossStudentsInCourse,
rossStudentsParam, i.IdsIamId, _aaudContext).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 536594cd-37e2-452f-8443-60ecb46085ec
📒 Files selected for processing (4)
test/CMS/CodecsTests.cstest/Students/StudentGroupServiceQueryTests.csweb/Areas/CMS/Data/Codecs.csweb/Areas/Students/Services/StudentGroupService.cs
Contributor
Author
|
@bsedwards Some code deduplication and photo gallery bugs |
Photo-gallery queries order/filter on the source before projecting so EF Core can translate them, large Contains lists use EF.Parameter (OPENJSON), and the catch blocks rethrow with context instead of masking failures. Adds SQLite-backed regression tests.
The course photo gallery threw "Cannot use multiple context instances within a single query execution" whenever Ross students were requested and any were active for the term: the Ross-in-course lookup joined the Courses and AAUD DbContexts in a single query, which EF cannot translate. Before the gallery fix this surfaced as a silently empty gallery (the exception was swallowed); after it, as a 500. Resolve the Ross lookup against the RE-filtered enrolled PIDM list already loaded from the Courses context, so only the AAUD context is queried and withdrawn or waitlisted Ross students stay excluded. Add a SQLite-backed regression test for the include-Ross course path, which reproduces the failure on the old query and asserts a withdrawn Ross student stays out.
rlorenzo
force-pushed
the
refactor/per-area-small
branch
from
485f63f to
0bf9919
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Per-area dedup driven by jscpd findings on the original
code-anaylsisbranch: 4 self-contained refactors across CTS, Scheduler, Students, and CMS (11 files). The Students refactor also uncovered and fixed two photo-gallery bugs (see below), so this PR carries behavior fixes and new regression tests in addition to the dedup.(This was formerly "Part 6 of 6" of a stack; PRs #174 through #178 are all merged to
main, and this branch is rebased off the latestmain, so it now stands alone.)Commits
refactor(cts): collapse hardcoded CourseStudents rows into a v-for: replace 3 near-identical<tr>blocks with a singlev-for.refactor(scheduler): share schedule filter LINQ via IScheduleEntity: introduceIScheduleEntity+ScheduleQueryExtensionsso the by-clinician / by-rotation / by-week scheduling queries share their filter LINQ.refactor(students): consolidate StudentGroupService photo + Ross helpers: extractBuildStudentPhotoListAsync,FormatStudentDisplayName,FormatGroupAssignment,ResolvePhotoUrl, andGetActiveRossIamIdsAsyncso the by-class-level / by-group / by-course paths no longer hand-roll the same logic. This commit also includes:StudentBaseRecordconstructor and then appliedOrderBy/Where, which EF Core cannot translate; the resulting runtimeInvalidOperationExceptionwas swallowed by a catch and surfaced as an empty (but 200 OK) result. Fixed by moving ordering and filtering ahead of the projection and rethrowing with context instead of silently returning empty.GroupAssignment. The combined group label was built from untrimmed values while the individualEighthsGroup/TwentiethsGroupfields were trimmed; padded AAUD values no longer leak into the displayed "1A1 / 1AA" label.EF.Parameter()on largeContainslists. The Ross IAM-id and enrolled-PIDM lookups now wrap their collections inEF.Parameter(...)so SQL Server translates them viaOPENJSON(per the >10-item guideline) rather than inlining hundreds of literals.test/Students/StudentGroupServiceQueryTests.cs): SQLite-backed tests that exercise all three query paths end to end, so the projection-translation failure cannot silently regress again.refactor(cms): reduce Codecs to UU-only and cover with tests: collapse the UU encode/decode methods onto shared helpers (DecodeWithMap,EncodeWithMap), delete the unused XX variants and their maps (ColdFusion only ever uses the default UU encoding, so XX was dead in both the legacy app and here), and addCodecsTests.UUEncodehas no caller yet but is kept for the CMS migration, where new files must store their AES key UU-encoded so legacy CF can decrypt them during the parallel-run period.fix(students): avoid cross-context join in course Ross gallery: second photo-gallery bug, in the course path. Requesting Ross students for a course joined the Courses and AAUD DbContexts in a single query ("Cannot use multiple context instances within a single query execution"); before the projection fix above this surfaced as a silently empty gallery, after it as a 500. The Ross lookup now resolves against the RE-filtered enrolled-PIDM list already loaded from the Courses context, which also keeps withdrawn/waitlisted Ross students out of course galleries (previously the Ross-only lookup ignored enrollment status). Adds a regression test for the include-Ross course path, including a withdrawn Ross student that must stay excluded.refactor(students): add AsNoTracking and EF.Parameter to Ross queries: review-feedback query hygiene;.AsNoTracking()on the read-only AAUD lookups andEF.Parameter(...)on the remaining list-basedContainsfilters in the Ross paths.Test plan
includeRossStudents=trueacross multiple courses;StudentGroupServiceQueryTestscover all three paths plus the include-Ross course path in CI)CodecsTests, run in CI); XX path removed