Dashboard Refactor, Step 3 (My Learning)#3340
Closed
ChristopherChudzicki wants to merge 6 commits into
Closed
Conversation
Move home-only query orchestration out of AllEnrollmentsDisplay into a new composer hook. The component shrinks 162 → ~75 lines and consumes typed bucket data plus auxiliary lookups for ProgramAsCourseCard. Pure helpers added to model/dashboardViewModel.ts and unit-tested: bucketAndSortHomeEnrollments, enrollmentCourseIsInPrograms, groupModuleCoursesByProgramId, getModuleCourseIdsFromPrograms, getNonContractProgramEnrollments, getTopLevelProgramEnrollments, isProgramAsCourse, isNonContractEnrollment. Simplify groupCourseRunEnrollmentsByCourseId by dropping the dead courses parameter — its filter never tripped because both production callers only look up by ids the helper would have included anyway. ProgramEnrollmentDisplay's two inline reduces (enrollmentsByCourseId and programEnrollmentsById) now use the simplified helpers, which were added in Phase 1 with no production callers until now. Soften the plan's "zero inline transforms" exit check across phases 3-5 to "no inline-defined domain logic" — composing a named predicate or comparator with .filter() is glue, not orchestration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EnrollmentDisplay.tsx had grown to ~836 lines because it housed both the home dashboard and the program dashboard. Pull the home half out: HomeEnrollmentsDashboard.tsx now owns its component, EnrollmentExpandCollapse, the home-only styled components, and SUPPORT_EMAIL. EnrollmentDisplay.tsx is now 836 → 548 lines and dispatches to either HomeEnrollmentsDashboard or ProgramEnrollmentDisplay. Phase 4 and Phase 5 will extract the program and contract dashboards into sibling files, leaving EnrollmentDisplay.tsx as a thin router (or removable entirely once each page can import its dashboard directly). DashboardCardStyled is duplicated across the two files for now — small styled wrapper, scheduled to be replaced by CoursewareCard in Phase 7. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 16 home tests now live alongside the component they cover and render <HomeEnrollmentsDashboard /> directly instead of going through the EnrollmentDisplay dispatcher. EnrollmentDisplay.test.tsx keeps only the program tests (will move to ProgramEnrollmentDashboard.test.tsx in Phase 4). Trims dead imports from EnrollmentDisplay.test.tsx (act, setupEnrollments) and from the new file (waitFor, mockAxiosInstance, invariant). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The hook is glue (composition of named helpers from the model layer) with no domain logic of its own. A standalone hook file with no isolated tests reads as "why no tests?" Inlining as a private function in the component file keeps the data-vs-render split readable without orphaning a file. Pure helpers in model/dashboardViewModel.ts continue to carry the testable logic. Add an open question to the plan doc: hook location and test strategy for Phase 4/5 should be settled before those phases begin. Program and contract dashboards may justify isolated hook tests on complexity grounds even if home does not, and the language picker is a candidate for its own reusable hook. Deletes the now-empty hooks/ directory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HomeContent now imports HomeEnrollmentsDashboard; ProgramContent imports ProgramEnrollmentDisplay. The EnrollmentDisplay dispatch wrapper (programId ? Program : Home) added no value once consumers already knew which dashboard they wanted. EnrollmentDisplay.tsx renamed to ProgramEnrollmentDisplay.tsx (the remaining content). Its test file follows; describe collapses from EnrollmentDisplay > ProgramId Filtering > tests to ProgramEnrollmentDisplay. ProgramAsCourseCard's ProgramCertificateButton import path updates; DashboardDialogs.test.tsx switches its home-flow render to HomeEnrollmentsDashboard directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Widen `isProgramAsCourse` to a structural type so it accepts both V2ProgramDetail and V3SimpleProgram; replace the inline display_mode check in HomeEnrollmentsDashboard with the shared helper. - Drop the redundant ProgramAsCourseProgramData shadow type and switch the prop to Map<number, V2ProgramDetail> (the hook's actual return). - Restore loading-window parity: programEnrollments returns [] while enrolledPrograms is mid-flight, matching pre-refactor behavior. - Add the defining home-invariant test: multiple enrollments for the same course render as multiple home cards. - Sweep stale EnrollmentDisplay.tsx references in the plan doc and note the inlined-hook decision in the file-structure diagram. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is Step 3 of a multi-step Dashboard refactor. It splits the previous single EnrollmentDisplay component (which switched between home and program dashboards based on a programId prop) into two separate top-level components (HomeEnrollmentsDashboard and ProgramEnrollmentDisplay), and moves the home-dashboard data-orchestration logic into a private useHomeDashboardData composer hook backed by named pure helpers in model/dashboardViewModel.ts.
Changes:
- Extract
HomeEnrollmentsDashboard(with privateuseHomeDashboardDatacomposer) fromEnrollmentDisplay; rename what remains toProgramEnrollmentDisplay; update consumers (HomeContent,ProgramContent,ProgramAsCourseCard,DashboardDialogs.test). - Move home-dashboard pure logic (bucketing/sorting enrollments, contract/parent-program filtering, program-as-course module wiring, simplified
groupCourseRunEnrollmentsByCourseIdthat now usesenrollment.run.course.idinstead of a runs→course map) into named, individually unit-tested helpers indashboardViewModel.ts. - Split the giant
EnrollmentDisplay.test.tsxinto focusedHomeEnrollmentsDashboard.test.tsxandProgramEnrollmentDisplay.test.tsx, and update the refactor plan doc to reflect Phase 3 as shipped.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| frontends/main/src/app-pages/DashboardPage/HomeContent.tsx | Switch from EnrollmentDisplay to HomeEnrollmentsDashboard. |
| frontends/main/src/app-pages/DashboardPage/ProgramContent.tsx | Switch from EnrollmentDisplay to ProgramEnrollmentDisplay. |
| frontends/main/src/app-pages/DashboardPage/ProgramContent.test.tsx | Update mock target/testid to new component name. |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDashboard.tsx | New: home dashboard component + private useHomeDashboardData composer. |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDashboard.test.tsx | New: tests for home dashboard (ported from old EnrollmentDisplay tests). |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.tsx | Strip out home-dashboard code and helpers; keep only program-dashboard rendering; consume new view-model helpers. |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx | New: program-dashboard tests (ported from old EnrollmentDisplay tests). |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx | Deleted; superseded by the two new test files. |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramAsCourseCard.tsx | Update import path of ProgramCertificateButton to the renamed file. |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx | Update test imports/usages to HomeEnrollmentsDashboard. |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts | Add named pure helpers (bucket/sort, contract/top-level filtering, module-course indexing, program-as-course predicate); simplify groupCourseRunEnrollmentsByCourseId signature. |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.test.ts | Add unit tests for all new helpers; update existing test for new grouping signature. |
| frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md | Update plan to reflect Phase 3 file layout and inlined-composer decision. |
Contributor
Author
|
Closing, going to split apart EnrollmentsDisplay first. |
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.
What are the relevant tickets?
Description (What does it do?)
Screenshots (if appropriate):
How can this be tested?
Additional Context