Skip to content

Commit b33fa93

Browse files
ChristopherChudzickiclaudeCopilot
authored
dashboard refactor: useHomeDashboardData (#3342)
* dashboard refactor phase 3: extract home dashboard view-model helpers Move home-only data orchestration out of the home dashboard component and into a typed composer (private function in HomeEnrollmentsDisplay.tsx) that calls named pure helpers from model/dashboardViewModel.ts. New pure helpers in dashboardViewModel.ts (with unit tests): bucketAndSortHomeEnrollments, enrollmentCourseIsInPrograms, groupModuleCoursesByProgramId, getModuleCourseIdsFromPrograms, getNonContractProgramEnrollments, getTopLevelProgramEnrollments, isProgramAsCourse, isNonContractEnrollment. ProgramEnrollmentDisplay's two inline reduces (enrollmentsByCourseId, programEnrollmentsById) now use the simplified helpers added in Phase 1. Drop the dead courses parameter from groupCourseRunEnrollmentsByCourseId — its filter never tripped because both callers only look up by ids the helper would have included. 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. Add an Open question noting Phase 3 inlined useHomeDashboardData as a private function rather than its own file; Phase 4/5 settle the hook location and test strategy before they begin. Rename ProgramContent.test.tsx's mock testid enrollment-display → program-enrollment-display for clarity now that two dashboards exist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add home dashboard B2B course filter test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * dashboard refactor phase 3 v3: extract useHomeDashboardData hook + assembleHomeCardList Set the consistent composer/testing pattern for the dashboard refactor (Phases 4-5 follow it): - Add pure assembleHomeCardList(buckets+programEnrollments) -> { cards, initiallyVisibleCount } owning home card ordering + the MIN_VISIBLE promotion rule (was stranded in EnrollmentExpandCollapse). Isolated unit tests in dashboardViewModel.test.ts. - Move useHomeDashboardData out of HomeEnrollmentsDisplay.tsx into a separate exported hooks/useHomeDashboardData.ts; emits the durable contract { cards, initiallyVisibleCount, ... }. - Add hooks/useHomeDashboardData.test.tsx renderHook suite reusing the shared setupEnrollments scenario (no duplicated mock setup). - EnrollmentExpandCollapse reduced to zero-logic slice + toggle. - HomeEnrollmentsDisplay.test.tsx left exactly untouched as the behavior-preservation oracle. - Update dashboardRefactorPlan.md: resolve the hook location/testing open question, document Phase 3 v3, re-scope Phase 6 to a checkpoint, add overall testing-strategy section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * address phase 3 v3 review feedback - B#1: resolveSlotForLanguage calls local pickDisplayedEnrollmentForLegacyDashboard instead of round-tripping through helpers.selectBestEnrollment; drop the now-unused ../helpers import. Removes the gratuitous helpers<->viewModel cycle leg. Zero behavior change (resolveSlotForLanguage/Program/Contract suites green). - A: add assembleHomeCardList case (nonExpired>0, no expired -> initiallyVisibleCount === cards.length), closing the case matrix. - B#2: fix DashboardCourseSlot JSDoc typo. - B#3: scope the type-only-import comment to that line (not a file-wide no-cycle claim). - Rewrite useHomeDashboardData JSDoc: timeless/descriptive, drop the refactor-historical 'remains unchanged' framing. - Plan: record the uniform durable-contract assertion rule (composer suites assert the durable contract, never through the adapter; displayed-*/run-selection pinned only in the resolveSlotForLanguage unit suite) so Phase 4/5 inherit it. Oracle (HomeEnrollmentsDisplay.test.tsx) still untouched. Full DashboardPage suite 411/411. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * plan: language-picker decided (hook owns it, Phase 4); fold reviewer-C refinements - Resolved decision: language picker = option (b) — useDashboardLanguagePicker extracted in Phase 4, composed by useProgramDashboardData; component presentational. Rationale recorded (hook needs the key to build slots regardless, so the YAGNI-defer argument doesn't hold). - Phase 4 pre-decisions block: (1) slot/adapter prove-or-delete, default lean (A) build it for real on Program; (2) test-utils infra-only + composable buildProgramScenario; (3) language picker (b). - Phase 5: hard exit gate — no OrgProgramDisplay/OrgProgramCollectionDisplay inline resolveSlotForLanguage / b2b_contract_id filtering. - Phase 7: checkbox to migrate DashboardResource/DashboardType into dashboardViewModel.ts before deleting DashboardCard.tsx. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * address gpt-5.4 review: doc accuracy + hook contract coverage - A: scope the 'oracle untouched' wording in dashboardRefactorPlan.md. HomeEnrollmentsDisplay.test.tsx is NOT modified by the v3 extraction (empty diff for 8ec7d30^..HEAD), but it is new on this branch (created cf3c0db, extended d812730 in Phase 3 v1). Plan now says 'not modified by the extraction commit' and flags the branch-vs-main narrative for reviewers, instead of a blanket 'untouched'. - B: add a renderHook test asserting the durable contract fields courseProgramsById / moduleCoursesByProgramId (program-as-course lookups the renderer needs) — previously only covered indirectly via the component suite. - C: no change. ProgramContent.test.tsx is modified on the branch only by a mechanical test-id rename in Phase 3 v1 (cf3c0db), forced by the EnrollmentDisplay->ProgramEnrollmentDisplay rename; its low-value nature is pre-existing and out of refactor scope. Oracle still not modified by v3. Full DashboardPage suite 412/412. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: correct adapter/slot wording in dashboard refactor plan CoursewareCard consumes a per-row variant union, never a slot — fix the two 'consumes the slot directly' phrasings (lines 214, 731) that contradicted the Phase 7 variant-union design and stability rule. Also fix two stale spots that drifted from the Resolved decision: the layer table's composer-location cell and the net-file-count reading note (Phase 3 v3 added a hooks/ directory of separate exported composers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: correct layer-2 assertion rule for displayedEnrollment/displayedRun The old rule banned displayedEnrollment/displayedRun from composer renderHook suites and pinned them only in the pure resolveSlotForLanguage suite — but those fields are returned, render-driving contract, and a returned field that is 'wrong' to assert is a smell. The pure suite proves the derivation; it cannot catch wiring (hook feeding the resolver wrong-scoped enrollments or a stale language key). Split by which bug each layer catches: composer suite asserts a representative displayedEnrollment/displayedRun (wiring); pure suite owns the exhaustive language-resolution matrix (derivation). Drop the stale 'only durable' framing in the lead-in accordingly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7560ba8 commit b33fa93

9 files changed

Lines changed: 1237 additions & 371 deletions

frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,46 @@ describe("HomeEnrollmentsDisplay", () => {
5757
})
5858
})
5959

60+
test("Renders one card per enrollment when a single course has multiple enrollments", async () => {
61+
const mitxOnlineUser = mitxonline.factories.user.user()
62+
setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser)
63+
64+
const sharedCourseId = faker.number.int()
65+
const enrollmentA = mitxonline.factories.enrollment.courseEnrollment({
66+
run: {
67+
title: "Same Course — Run A",
68+
course: { id: sharedCourseId },
69+
},
70+
})
71+
const enrollmentB = mitxonline.factories.enrollment.courseEnrollment({
72+
run: {
73+
title: "Same Course — Run B",
74+
course: { id: sharedCourseId },
75+
},
76+
})
77+
78+
setMockResponse.get(mitxonline.urls.enrollment.enrollmentsListV3(), [
79+
enrollmentA,
80+
enrollmentB,
81+
])
82+
setMockResponse.get(
83+
mitxonline.urls.programEnrollments.enrollmentsListV3(),
84+
[],
85+
)
86+
87+
renderWithProviders(<HomeEnrollmentsDisplay />)
88+
89+
await screen.findByRole("heading", { name: "My Learning" })
90+
const cards = await screen.findAllByTestId("enrollment-card-desktop")
91+
expect(cards).toHaveLength(2)
92+
expect(
93+
cards.find((c) => c.textContent?.includes("Same Course — Run A")),
94+
).toBeDefined()
95+
expect(
96+
cards.find((c) => c.textContent?.includes("Same Course — Run B")),
97+
).toBeDefined()
98+
})
99+
60100
test("Renders the proper amount of unenroll and email settings buttons in the context menus", async () => {
61101
const { enrollments } = setupApis()
62102
renderWithProviders(<HomeEnrollmentsDisplay />)
@@ -779,6 +819,47 @@ describe("HomeEnrollmentsDisplay", () => {
779819
expect(screen.queryByText(b2bSimpleProgram.title)).not.toBeInTheDocument()
780820
})
781821

822+
test("Filters B2B course enrollments from display", async () => {
823+
const mitxOnlineUser = mitxonline.factories.user.user()
824+
setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser)
825+
826+
mockedUseFeatureFlagEnabled.mockReturnValue(true)
827+
828+
const b2bEnrollment = mitxonline.factories.enrollment.courseEnrollment({
829+
b2b_contract_id: 42,
830+
run: {
831+
title: "B2B Hidden Course",
832+
course: { title: "B2B Hidden Course" },
833+
},
834+
})
835+
const nonB2BEnrollment = mitxonline.factories.enrollment.courseEnrollment({
836+
b2b_contract_id: null,
837+
run: {
838+
title: "Personal Visible Course",
839+
course: { title: "Personal Visible Course" },
840+
},
841+
})
842+
843+
setMockResponse.get(mitxonline.urls.enrollment.enrollmentsListV3(), [
844+
b2bEnrollment,
845+
nonB2BEnrollment,
846+
])
847+
setMockResponse.get(
848+
mitxonline.urls.programEnrollments.enrollmentsListV3(),
849+
[],
850+
)
851+
852+
renderWithProviders(<HomeEnrollmentsDisplay />)
853+
854+
await screen.findByRole("heading", { name: "My Learning" })
855+
856+
const personalCourseCards = await screen.findAllByText(
857+
nonB2BEnrollment.run.title,
858+
)
859+
expect(personalCourseCards.length).toBeGreaterThan(0)
860+
expect(screen.queryByText(b2bEnrollment.run.title)).not.toBeInTheDocument()
861+
})
862+
782863
test("Hides child program enrollments that are part of an enrolled parent program", async () => {
783864
const mitxOnlineUser = mitxonline.factories.user.user()
784865
setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser)

0 commit comments

Comments
 (0)