Skip to content

PR 6: refactor: per-area small refactors (CTS, Scheduler, Students, CMS)#179

Open
rlorenzo wants to merge 4 commits intorefactor/dead-code-and-shared-chromefrom
refactor/per-area-small
Open

PR 6: refactor: per-area small refactors (CTS, Scheduler, Students, CMS)#179
rlorenzo wants to merge 4 commits intorefactor/dead-code-and-shared-chromefrom
refactor/per-area-small

Conversation

@rlorenzo
Copy link
Copy Markdown
Contributor

@rlorenzo rlorenzo commented May 5, 2026

Summary

Part 6 of 6. Stacks on top of PR #177 (parallel to PR #178). The smallest PR in the stack — 9 files, 4 self-contained refactors.

Each commit is a small per-area dedup driven by jscpd findings on the original code-anaylsis branch.

Commits

  • refactor(cts): collapse hardcoded CourseStudents rows into a v-for — replace 5 near-identical <tr> blocks with a single v-for.
  • refactor(scheduler): share schedule filter LINQ via IScheduleEntity — introduce IScheduleEntity + ScheduleQueryExtensions so the by-clinician / by-rotation / by-week scheduling queries share their filter LINQ.
  • refactor(students): consolidate StudentGroupService photo + Ross helpers — extract BuildStudentPhotoListAsync, FormatStudentDisplayName, FormatGroupAssignment, ResolvePhotoUrl, and GetActiveRossIamIdsAsync so the by-class-level / by-group / by-course paths no longer hand-roll the same logic.
  • refactor(cms): collapse Codecs UU/XX encoders onto shared map helpers — collapse 4 near-identical UU/XX encode/decode methods into 2 generic helpers (DecodeWithMap, EncodeWithMap) keyed by the encoding map.

Conflict resolution notes

Two cherry-picks needed conflict resolution because PR 3's analyzer cleanup ran ahead of these refactors in the original chronology, and the analyzer fixes touched the pre-refactor code shape:

  • StudentGroupService.cs: took the refactor's structure, then re-applied PR 3's analyzer fixes to the new helper (SqlException short qualifier, !string.IsNullOrEmpty(id) over id != null, sealed record StudentBaseRecord).
  • Codecs.cs: took the refactor's consolidated structure with Stream qualifiers shortened (the file's implicit-usings setup makes using System.IO; unnecessary).

Net effect: each file ends up at the same end-state as on the original code-anaylsis branch.

PR stack

Test plan

  • CI green
  • CTS course students table renders correctly (5+ students)
  • Clinical Scheduler: by-clinician / by-rotation / by-week views still filter the same set
  • Students photo gallery: by-class-level / by-group / by-course flows match prior behavior; Ross students excluded/included as before
  • CMS Codecs: round-trip a UU and XX encode/decode against a known fixture

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • review-ready

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f32145ad-8764-4439-9ee0-f0048c500982

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/per-area-small

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rlorenzo rlorenzo force-pushed the refactor/per-area-small branch from 25b942b to d0b2e7f Compare May 5, 2026 07:39
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from ebe32ec to 75abeb2 Compare May 5, 2026 07:39
@rlorenzo rlorenzo force-pushed the refactor/per-area-small branch from d0b2e7f to 0ea0117 Compare May 5, 2026 14:47
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch 5 times, most recently from d26c5d8 to 8cbfb69 Compare May 7, 2026 15:57
@rlorenzo rlorenzo force-pushed the refactor/per-area-small branch from 0ea0117 to 0fa6909 Compare May 7, 2026 15:57
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch 2 times, most recently from 0d62a4e to a0c55c4 Compare May 9, 2026 05:22
@rlorenzo rlorenzo force-pushed the refactor/per-area-small branch from 0fa6909 to 5cbd5f4 Compare May 9, 2026 05:35
rlorenzo added 4 commits May 9, 2026 10:26
The three repeated prototype rows now iterate over a single mock array,
which is how the real implementation will pull the API response.
Add an IScheduleEntity interface implemented by InstructorSchedule and
StudentSchedule, then route both services through a generic
ApplyScheduleFilters extension instead of duplicating the rotation /
service / week / date filter clauses.
Introduce the StudentBaseRecord projection shape, BuildStudentPhotoListAsync,
and GetActiveRossIamIdsAsync so the by-class-level / by-group / by-course
methods no longer hand-roll the same photo lookup, group-assignment
formatting, and Ross-IamIds pre-query.
The four public methods now delegate to private EncodeWithMap /
DecodeWithMap routines parameterised by the encoding map, eliminating
the byte-for-byte duplication between the UU and XX variants.
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from a0c55c4 to d8a3735 Compare May 9, 2026 17:27
@rlorenzo rlorenzo force-pushed the refactor/per-area-small branch from 5cbd5f4 to 8eb247a Compare May 9, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant