From cf3c0db28b5e4459a52a2a93954092e66a29b2ac Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 13 May 2026 17:59:40 -0400 Subject: [PATCH 1/2] dashboard refactor phase 3: extract home dashboard view-model helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../HomeEnrollmentsDisplay.test.tsx | 40 ++ .../HomeEnrollmentsDisplay.tsx | 329 ++++++---------- .../ProgramEnrollmentDisplay.tsx | 23 +- .../dashboardRefactorPlan.md | 93 +++-- .../model/dashboardViewModel.test.ts | 350 +++++++++++++++++- .../model/dashboardViewModel.ts | 183 ++++++++- .../DashboardPage/ProgramContent.test.tsx | 15 +- 7 files changed, 728 insertions(+), 305 deletions(-) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx index d25ee8edaf..1b54e91825 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx @@ -57,6 +57,46 @@ describe("HomeEnrollmentsDisplay", () => { }) }) + test("Renders one card per enrollment when a single course has multiple enrollments", async () => { + const mitxOnlineUser = mitxonline.factories.user.user() + setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) + + const sharedCourseId = faker.number.int() + const enrollmentA = mitxonline.factories.enrollment.courseEnrollment({ + run: { + title: "Same Course — Run A", + course: { id: sharedCourseId }, + }, + }) + const enrollmentB = mitxonline.factories.enrollment.courseEnrollment({ + run: { + title: "Same Course — Run B", + course: { id: sharedCourseId }, + }, + }) + + setMockResponse.get(mitxonline.urls.enrollment.enrollmentsListV3(), [ + enrollmentA, + enrollmentB, + ]) + setMockResponse.get( + mitxonline.urls.programEnrollments.enrollmentsListV3(), + [], + ) + + renderWithProviders() + + await screen.findByRole("heading", { name: "My Learning" }) + const cards = await screen.findAllByTestId("enrollment-card-desktop") + expect(cards).toHaveLength(2) + expect( + cards.find((c) => c.textContent?.includes("Same Course — Run A")), + ).toBeDefined() + expect( + cards.find((c) => c.textContent?.includes("Same Course — Run B")), + ).toBeDefined() + }) + test("Renders the proper amount of unenroll and email settings buttons in the context menus", async () => { const { enrollments } = setupApis() renderWithProviders() diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.tsx index 34031ee335..37bc6f76d7 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.tsx @@ -1,6 +1,5 @@ import React from "react" -import { DASHBOARD_MY_LEARNING_ID } from "@/common/urls" -import { enrollmentQueries } from "api/mitxonline-hooks/enrollment" +import { keepPreviousData, useQuery } from "@tanstack/react-query" import { Collapse, Link, @@ -12,32 +11,35 @@ import { theme, } from "ol-components" import { Alert } from "@mitodl/smoot-design" -import { keepPreviousData, useQuery } from "@tanstack/react-query" import { - EnrollmentStatus, - getEnrollmentStatus, - getKey, - ResourceType, -} from "./helpers" -import { - DashboardCard, - DashboardResource, - DashboardType, -} from "./DashboardCard" -import { coursesQueries } from "api/mitxonline-hooks/courses" -import { programsQueries } from "api/mitxonline-hooks/programs" -import { - ContractPage, CourseRunEnrollmentV3, CourseWithCourseRunsSerializerV2, - DisplayModeEnum, V2ProgramDetail, - V2ProgramRequirement, V3UserProgramEnrollment, } from "@mitodl/mitxonline-api-axios/v2" +import { coursesQueries } from "api/mitxonline-hooks/courses" +import { enrollmentQueries } from "api/mitxonline-hooks/enrollment" import { mitxUserQueries } from "api/mitxonline-hooks/user" +import { programsQueries } from "api/mitxonline-hooks/programs" +import { DASHBOARD_MY_LEARNING_ID } from "@/common/urls" +import { getKey, ResourceType } from "./helpers" +import { + DashboardCard, + DashboardResource, + DashboardType, +} from "./DashboardCard" import { ProgramAsCourseCard } from "./ProgramAsCourseCard" -import { getIdsFromReqTree } from "@/common/mitxonline" +import { + bucketAndSortHomeEnrollments, + enrollmentCourseIsInPrograms, + getModuleCourseIdsFromPrograms, + getNonContractProgramEnrollments, + getTopLevelProgramEnrollments, + groupCourseRunEnrollmentsByCourseId, + groupModuleCoursesByProgramId, + isNonContractEnrollment, + isProgramAsCourse, +} from "./model/dashboardViewModel" const Wrapper = styled.div(({ theme }) => ({ marginTop: "32px", @@ -102,62 +104,7 @@ const ShowAllContainer = styled.div(({ theme }) => ({ }, })) -const alphabeticalSort = (a: CourseRunEnrollmentV3, b: CourseRunEnrollmentV3) => - a.run.course.title.localeCompare(b.run.course.title) - -const startsSooner = (a: CourseRunEnrollmentV3, b: CourseRunEnrollmentV3) => { - if (!a.run.start_date && !b.run.start_date) return 0 - if (!a.run.start_date) return 1 - if (!b.run.start_date) return -1 - const x = new Date(a.run.start_date) - const y = new Date(b.run.start_date) - return x.getTime() - y.getTime() -} - -const sortEnrollments = (enrollments: CourseRunEnrollmentV3[]) => { - const expired: CourseRunEnrollmentV3[] = [] - const completed: CourseRunEnrollmentV3[] = [] - const started: CourseRunEnrollmentV3[] = [] - const notStarted: CourseRunEnrollmentV3[] = [] - enrollments.forEach((enrollment) => { - if (!enrollment?.b2b_contract_id) { - const enrollmentStatus = getEnrollmentStatus(enrollment) - if (enrollmentStatus === EnrollmentStatus.Completed) { - completed.push(enrollment) - } else if ( - enrollment.run.end_date && - new Date(enrollment.run.end_date) < new Date() - ) { - expired.push(enrollment) - } else if ( - enrollment.run.start_date && - new Date(enrollment.run.start_date) < new Date() - ) { - started.push(enrollment) - } else { - notStarted.push(enrollment) - } - } - }) - - return { - completed: completed.sort(alphabeticalSort), - expired: expired.sort(alphabeticalSort), - started: started.sort(alphabeticalSort), - notStarted: notStarted.sort(startsSooner), - } -} - -const dedupeEnrollments = ( - enrollments: CourseRunEnrollmentV3[], - enrolledPrograms: V2ProgramDetail[], -) => { - return enrollments.filter((enrollment) => { - return !enrolledPrograms.some((program) => - program.courses?.includes(enrollment.run.course.id), - ) - }) -} +const SUPPORT_EMAIL = process.env.NEXT_PUBLIC_MITOL_SUPPORT_EMAIL || "" const getResourceKey = (resource: DashboardResource): string => { if (resource.type === DashboardType.ProgramEnrollment) { @@ -185,17 +132,7 @@ const isProgramAsCourseEnrollment = ( resource: DashboardResource, ): resource is ProgramEnrollmentResource => { if (resource.type !== DashboardType.ProgramEnrollment) return false - return resource.data.program.display_mode === DisplayModeEnum.Course -} - -type ProgramAsCourseProgramData = { - id: number - readable_id: string - title?: string | null - start_date?: string | null - end_date?: string | null - courses?: number[] - req_tree?: V2ProgramRequirement[] + return isProgramAsCourse(resource.data.program) } interface EnrollmentExpandCollapseProps { @@ -203,7 +140,7 @@ interface EnrollmentExpandCollapseProps { maybeShown: DashboardResource[] isLoading?: boolean enrollmentsByCourseId: Record - courseProgramsById: Map + courseProgramsById: Map moduleCoursesByProgramId: Record onUpgradeError?: (error: string) => void } @@ -298,59 +235,39 @@ const EnrollmentExpandCollapse: React.FC = ({ ) } -const getTopLevelProgramEnrollments = ( - programEnrollments: V3UserProgramEnrollment[], - programs: V2ProgramDetail[], -) => { - const childIds = new Set( - programs.flatMap( - (program) => getIdsFromReqTree(program.req_tree).programIds, - ), - ) - return programEnrollments.filter( - (enrollment) => !childIds.has(enrollment.program.id), - ) -} -const getNonContractProgramEnrollments = ( - programEnrollments: V3UserProgramEnrollment[], - contracts: ContractPage[], -) => { - const contractPrograms = new Set( - contracts.flatMap((contract) => contract.programs), - ) - return programEnrollments.filter( - (enrollment) => !contractPrograms.has(enrollment.program.id), - ) +type HomeDashboardData = { + started: CourseRunEnrollmentV3[] + notStarted: CourseRunEnrollmentV3[] + completed: CourseRunEnrollmentV3[] + expired: CourseRunEnrollmentV3[] + programEnrollments: V3UserProgramEnrollment[] + enrollmentsByCourseId: Record + courseProgramsById: Map + moduleCoursesByProgramId: Record + isLoading: boolean } /** - * Renders the "My Learning" section for non-B2B enrollments. - * - * Cards are ordered and grouped as follows: - * 1. Started courses (past start date, not expired, not completed) - * 2. Not-yet-started courses - * 3. Completed courses (any passing grade) - * 4. Program enrollments (excluding those covered by a B2B contract) - * 5. Expired courses (past end date, not completed) — hidden behind "Show all" + * Composes the queries and pure-model helpers that drive the home dashboard. + * Returns bucket data (started / notStarted / completed / expired) plus the + * auxiliary lookups the renderer needs for ProgramAsCourseCard. * - * Exception: if groups 1–4 are all empty, up to MIN_VISIBLE expired courses - * are shown directly so the section is never blank for an enrolled user. - * The section is hidden entirely only when there are no enrollments at all. + * Private to this file: the home dashboard is its only consumer, and the + * model-layer helpers it composes are individually unit-tested. */ -const HomeEnrollmentsDisplay: React.FC = () => { - const [upgradeError, setUpgradeError] = React.useState(null) +const useHomeDashboardData = (): HomeDashboardData => { const { data: enrolledCourses, isLoading: courseEnrollmentsLoading } = useQuery(enrollmentQueries.courseRunEnrollmentsList()) - - const contracts = useQuery({ - ...mitxUserQueries.me(), // this query has the contracts and should already be loaded + const { data: contracts, isLoading: contractsLoading } = useQuery({ + ...mitxUserQueries.me(), select: (user) => user.b2b_organizations.flatMap((org) => org.contracts), }) const { data: programEnrollments, isLoading: programEnrollmentsLoading } = useQuery(enrollmentQueries.programEnrollmentsList()) + const nonContractProgramEnrollments = - contracts.data && programEnrollments - ? getNonContractProgramEnrollments(programEnrollments, contracts.data) + contracts && programEnrollments + ? getNonContractProgramEnrollments(programEnrollments, contracts) : [] const enrolledProgramIds = nonContractProgramEnrollments.map( (enrollment) => enrollment.program.id, @@ -363,83 +280,81 @@ const HomeEnrollmentsDisplay: React.FC = () => { page_size: enrolledProgramIds.length, }), enabled: enrolledProgramIds.length > 0, - // If the query key changes, show the old data while loading - // example: Deleting a program enrollment placeholderData: keepPreviousData, }) - const filteredProgramEnrollments = enrolledPrograms - ? getTopLevelProgramEnrollments( - nonContractProgramEnrollments, - enrolledPrograms.results, - ) - : [] + const enrolledProgramsResults = enrolledPrograms?.results ?? [] + const coursePrograms = enrolledProgramsResults.filter(isProgramAsCourse) + const courseProgramModuleIds = getModuleCourseIdsFromPrograms(coursePrograms) - const homeCoursePrograms = enrolledPrograms?.results.filter( - (program) => program.display_mode === DisplayModeEnum.Course, - ) + const { data: courseProgramModuleCourses, isLoading: moduleCoursesLoading } = + useQuery({ + ...coursesQueries.coursesList({ + id: courseProgramModuleIds, + page_size: courseProgramModuleIds.length || undefined, + }), + enabled: courseProgramModuleIds.length > 0, + placeholderData: keepPreviousData, + }) + + const isCovered = enrollmentCourseIsInPrograms(enrolledProgramsResults) + const bucketableEnrollments = (enrolledCourses ?? []) + .filter(isNonContractEnrollment) + .filter((enrollment) => !isCovered(enrollment)) + const buckets = bucketAndSortHomeEnrollments(bucketableEnrollments) - const homeCourseProgramModuleIds = [ - ...new Set( - homeCoursePrograms?.flatMap( - (courseProgram) => getIdsFromReqTree(courseProgram.req_tree).courseIds, - ), + return { + ...buckets, + programEnrollments: enrolledPrograms + ? getTopLevelProgramEnrollments( + nonContractProgramEnrollments, + enrolledProgramsResults, + ) + : [], + enrollmentsByCourseId: groupCourseRunEnrollmentsByCourseId( + enrolledCourses ?? [], ), - ] + courseProgramsById: new Map(coursePrograms.map((p) => [p.id, p])), + moduleCoursesByProgramId: groupModuleCoursesByProgramId( + coursePrograms, + courseProgramModuleCourses?.results ?? [], + ), + isLoading: + courseEnrollmentsLoading || + programEnrollmentsLoading || + contractsLoading || + enrolledProgramsLoading || + moduleCoursesLoading, + } +} +/** + * Renders the "My Learning" section for non-B2B enrollments. + * + * Cards are ordered and grouped as follows: + * 1. Started courses (past start date, not expired, not completed) + * 2. Not-yet-started courses + * 3. Completed courses (any passing grade) + * 4. Program enrollments (excluding those covered by a B2B contract) + * 5. Expired courses (past end date, not completed) — hidden behind "Show all" + * + * Exception: if groups 1–4 are all empty, up to MIN_VISIBLE expired courses + * are shown directly so the section is never blank for an enrolled user. + * The section is hidden entirely only when there are no enrollments at all. + */ +const HomeEnrollmentsDisplay: React.FC = () => { + const [upgradeError, setUpgradeError] = React.useState(null) const { - data: homeCourseProgramModuleCourses, - isLoading: homeCourseProgramModuleCoursesLoading, - } = useQuery({ - ...coursesQueries.coursesList({ - id: homeCourseProgramModuleIds, - page_size: homeCourseProgramModuleIds.length || undefined, - }), - enabled: homeCourseProgramModuleIds.length > 0, - placeholderData: keepPreviousData, - }) - - const homeCourseProgramsById = new Map( - (homeCoursePrograms ?? []).map((courseProgram) => [ - courseProgram.id, - courseProgram, - ]), - ) - - const homeModuleCoursesByProgramId = (() => { - const allCourses = homeCourseProgramModuleCourses?.results ?? [] - - return (homeCoursePrograms ?? []).reduce< - Record - >((acc, courseProgram) => { - const courseIds = new Set(courseProgram.courses ?? []) - acc[courseProgram.id] = allCourses.filter((course) => - courseIds.has(course.id), - ) - return acc - }, {}) - })() - - const homeEnrollmentsByCourseId = (enrolledCourses || []).reduce( - (acc, enrollment) => { - const courseId = enrollment.run.course.id - if (!acc[courseId]) { - acc[courseId] = [] - } - acc[courseId].push(enrollment) - return acc - }, - {} as Record, - ) - - const supportEmail = process.env.NEXT_PUBLIC_MITOL_SUPPORT_EMAIL || "" - - const filteredEnrollments = React.useMemo(() => { - if (!enrolledCourses) return [] - return dedupeEnrollments(enrolledCourses, enrolledPrograms?.results ?? []) - }, [enrolledCourses, enrolledPrograms?.results]) - const { completed, expired, started, notStarted } = - sortEnrollments(filteredEnrollments) + started, + notStarted, + completed, + expired, + programEnrollments, + enrollmentsByCourseId, + courseProgramsById, + moduleCoursesByProgramId, + isLoading, + } = useHomeDashboardData() const normallyShown: DashboardResource[] = [ ...started.map((data) => ({ @@ -454,7 +369,7 @@ const HomeEnrollmentsDisplay: React.FC = () => { data, type: DashboardType.CourseRunEnrollment, })), - ...filteredProgramEnrollments.map((data) => ({ + ...programEnrollments.map((data) => ({ data, type: DashboardType.ProgramEnrollment, })), @@ -477,7 +392,7 @@ const HomeEnrollmentsDisplay: React.FC = () => { onClose={() => setUpgradeError(null)} > {upgradeError}{" "} - + Contact Support {" "} for assistance. @@ -486,16 +401,10 @@ const HomeEnrollmentsDisplay: React.FC = () => { diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.tsx index 475b3f7eff..ef373f15fb 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.tsx @@ -14,12 +14,13 @@ import { getRequirementsProgress, getKey, ResourceType } from "./helpers" import { DashboardCard, DashboardType } from "./DashboardCard" import { getDistinctDashboardLanguageOptions, + groupCourseRunEnrollmentsByCourseId, + groupProgramEnrollmentsByProgramId, resolveSlotForLanguage, } from "./model/dashboardViewModel" import { coursesQueries } from "api/mitxonline-hooks/courses" import { programsQueries } from "api/mitxonline-hooks/programs" import { - CourseRunEnrollmentV3, CourseWithCourseRunsSerializerV2, DisplayModeEnum, V2ProgramDetail, @@ -219,24 +220,12 @@ const ProgramEnrollmentDisplay: React.FC = ({ requiredProgramsLoading || requiredProgramCoursesLoading - const enrollmentsByCourseId = (rawEnrollments || []).reduce( - (acc, enrollment) => { - const courseId = enrollment.run.course.id - if (!acc[courseId]) { - acc[courseId] = [] - } - acc[courseId].push(enrollment) - return acc - }, - {} as Record, + const enrollmentsByCourseId = groupCourseRunEnrollmentsByCourseId( + rawEnrollments ?? [], ) - const programEnrollmentsById = (programEnrollments ?? []).reduce( - (acc, enrollment) => { - acc[enrollment.program.id] = enrollment - return acc - }, - {} as Record, + const programEnrollmentsById = groupProgramEnrollmentsByProgramId( + programEnrollments ?? [], ) const allProgramCourses = React.useMemo( diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md index d7366bfe29..e83efc0e23 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md @@ -197,10 +197,9 @@ Add focused files under the existing dashboard area. The end state consolidates ```text frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ - hooks/ - useHomeDashboardData.ts - useProgramDashboardData.ts - useContractDashboardData.ts + hooks/ # may not materialize — see Open question after Phase 3 + useProgramDashboardData.ts # location TBD: separate file vs. inlined in component + useContractDashboardData.ts # location TBD: separate file vs. inlined in component model/ dashboardViewModel.ts dashboardAdapters.ts @@ -208,9 +207,9 @@ frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ Responsibilities: -- `hooks/useHomeDashboardData.ts`: owns home queries, home enrollment sorting, home program-as-course data assembly, and loading aggregation. -- `hooks/useProgramDashboardData.ts`: owns program detail query, course/enrollment/program enrollment joins, requirement-section construction, language options, slot assembly, and program-as-course slot context. -- `hooks/useContractDashboardData.ts`: owns contract-scoped courses/programs/collections, contract enrollment filtering, program filtering, collection shaping, language options, slot assembly, and loading aggregation. +- `useHomeDashboardData` (composer): owns home queries, home enrollment sorting, home program-as-course data assembly, and loading aggregation. **As shipped in Phase 3, inlined as a private function inside `HomeEnrollmentsDisplay.tsx` rather than a standalone file** — see Open question after Phase 3. +- `useProgramDashboardData` (composer): owns program detail query, course/enrollment/program enrollment joins, requirement-section construction, language options, slot assembly, and program-as-course slot context. Location decided per Open question. +- `useContractDashboardData` (composer): owns contract-scoped courses/programs/collections, contract enrollment filtering, program filtering, collection shaping, language options, slot assembly, and loading aggregation. Location decided per Open question. - `model/dashboardViewModel.ts`: the **single pure-model home**. Owns the canonical types (`DashboardCourseSlot`, section shapes, home buckets), grouping helpers, slot constructors, requirement-section builders, language-option computation, the composite slot/language resolver introduced in Phase 2, the renamed display-policy helper, and Cases 2 + 3 of `getResolvedRunForSelectedLanguage`. By Phase 7's end, every pure helper that survives the cleanup lives here. - `model/dashboardAdapters.ts`: temporary adapters from the new view model to current `DashboardCard` / `ProgramAsCourseCard` props. Deletion candidate at Phase 7 — its lifespan ends when `CoursewareCard` consumes the slot directly. @@ -218,7 +217,7 @@ Responsibilities: | File | Layer | Knows React? | Knows queries? | Long-term? | | ------------------------------- | -------------------------- | ------------ | -------------- | --------------------------------------- | -| `hooks/useXxxDashboardData.ts` | Composer (data + actions) | Yes | Yes | Yes | +| `useXxxDashboardData` composer | Composer (data + actions) | Yes | Yes | Yes (location: file or inlined; TBD) | | `model/dashboardViewModel.ts` | Pure model — single home | No | No | Yes — load-bearing | | `model/dashboardAdapters.ts` | Pure model (legacy bridge) | No | No | No — deleted in Phase 7 | | `helpers.ts` (existing) | Pure model | No | No | No — absorbed into viewModel by Phase 7 | @@ -293,7 +292,8 @@ This phase also folds in a small, isolated bug fix: enrolled-but-not-enrollable ```bash yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/languageOptions.test.ts -yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx yarn test frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx ``` @@ -316,11 +316,11 @@ Expected: all tests pass; callsite line-counts in the two render components meas **Purpose:** Make the home dashboard easier to reason about without changing its enrollment-flat behavior. -**Files:** +**Files (as shipped):** -- Create: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/hooks/useHomeDashboardData.ts` -- Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx` -- Test: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx` +- Create: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.tsx` (extracted from `EnrollmentDisplay.tsx`; carries the inlined `useHomeDashboardData` composer — see Open question) +- Rename: `EnrollmentDisplay.tsx` → `ProgramEnrollmentDisplay.tsx` (wrapper deleted; consumers in `HomeContent.tsx` / `ProgramContent.tsx` now import the dashboards directly) +- Test: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx` (new) and `ProgramEnrollmentDisplay.test.tsx` (renamed) - [ ] Move home-only queries out of `AllEnrollmentsDisplay`: - course-run enrollments, @@ -337,7 +337,7 @@ Expected: all tests pass; callsite line-counts in the two render components meas - [ ] Keep one card per enrollment on home. - [ ] Keep existing show/hide/expand behavior. - [ ] Keep existing `onUpgradeError` behavior. -- [ ] Update `AllEnrollmentsDisplay` to consume the hook and render as before. +- [ ] Update the home dashboard component to consume the hook and render as before. - [ ] Add or update tests proving: - multiple enrollments for the same course still render as multiple home cards, - B2B contract enrollments are excluded from home buckets as before, @@ -346,30 +346,42 @@ Expected: all tests pass; callsite line-counts in the two render components meas - [ ] Run: ```bash -yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx ``` Expected: all tests pass. -**Phase exit check: thin composer.** Per the Working agreement's success criterion, this phase delivers cleanup only if `useHomeDashboardData.ts` is a _composer_, not a re-home of inline orchestration. Concrete checks at exit: +**Phase exit check: thin composer.** Per the Working agreement's success criterion, this phase delivers cleanup only if `useHomeDashboardData` is a _composer_, not a re-home of inline orchestration. Concrete checks at exit: - Hook body line count: target ~80 lines or less, mostly fetch + compose + return. If significantly larger, the hook is implementing logic that belongs in `model/dashboardViewModel.ts`. -- Inline transforms in the hook body: target zero. Anything that loops, filters, sorts, joins, groups, or branches on data shape is a named helper imported from the model layer with its own unit test. +- The hook composes named helpers; it doesn't re-home orchestration. Glue like `enrollments.filter(existingPredicate)` or `enrollments.sort(existingComparator)` is fine — wrapping it in a named helper would be noise. What's not fine: predicates or comparators _defined_ inline, multi-step pipelines that together encode "what the home dashboard means," or grouping/joining inline. Those are named helpers in `model/dashboardViewModel.ts` with their own unit tests. Litmus: if a reader has to read the inline code to learn a domain rule, the rule belongs in a named helper. - Helper test coverage: every transform helper has an isolated unit test, separate from the hook's integration test. -- Consuming callsite shrinkage: `EnrollmentDisplay.tsx`'s home path measurably drops in size because the orchestration moved. +- Consuming callsite shrinkage: the home path drops in size because the orchestration moved into `HomeEnrollmentsDisplay.tsx`'s composer (and the leftover `ProgramEnrollmentDisplay.tsx` no longer carries home concerns). If any of these fails, the phase has relocated complexity rather than reduced it. Stop and split before declaring done. +## Open question (resolve before Phase 4 starts): hook location and testing + +Phase 3's `useHomeDashboardData` ended up inlined as a private function inside `HomeEnrollmentsDisplay.tsx` rather than living in `hooks/`. The reasoning: the hook is glue (composition of named helpers from the model layer), and a standalone hook file with no isolated tests reads as "why no tests?" Inlining keeps the readability split between data and render without orphaning a file. + +Before Phase 4 begins, settle for `useProgramDashboardData` and `useContractDashboardData`: + +- **Location**: separate `hooks/...` file vs inlined in the component file. A separate file should pair with isolated tests, not be orphaned. +- **Test strategy**: integration via the component test (status quo for Phase 3) vs dedicated `renderHook` tests. Integration is slow to bisect when a hook regresses; `renderHook` duplicates mock setup. Contract orchestration is the largest tangle and may justify isolated tests on complexity grounds alone, even if program does not. +- **Language picker**: candidate for its own `useDashboardLanguagePicker(options)` (selected key + reset-on-options-change effect) reusable across program and contract. Where it lives follows the same location decision. + +What the model layer must hold regardless: `buildRequirementSections`, slot constructors, contract-scoped filters (`programHasContractRuns`, `programsInCollections`, `sortedPrograms`), collection shaping. These are domain logic and need named helpers with unit tests no matter where the hook lives. The "thin composer" exit checks apply to the hook function whether it's a file or inline. + ## Phase 4: Extract `useProgramDashboardData` **Purpose:** Move program-dashboard query orchestration, requirement-tree shaping, language policy, and slot construction out of `ProgramEnrollmentDisplay`. **Files:** -- Create: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/hooks/useProgramDashboardData.ts` -- Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx` +- Add: `useProgramDashboardData` composer (location TBD — see Open question; default per Phase 3 precedent is inlined in `ProgramEnrollmentDisplay.tsx`) +- Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.tsx` - Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramAsCourseCard.tsx` only if needed for adapter boundaries -- Test: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx` +- Test: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx` - Test: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramAsCourseCard.test.tsx` - [ ] Move program detail query and course queries into the hook. @@ -397,18 +409,18 @@ If any of these fails, the phase has relocated complexity rather than reduced it - [ ] Run: ```bash -yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramAsCourseCard.test.tsx ``` Expected: all tests pass. -**Phase exit check: thin composer.** Per the Working agreement's success criterion, this phase delivers cleanup only if `useProgramDashboardData.ts` is a _composer_, not a re-home of inline orchestration. Concrete checks at exit: +**Phase exit check: thin composer.** Per the Working agreement's success criterion, this phase delivers cleanup only if `useProgramDashboardData` is a _composer_, not a re-home of inline orchestration. Concrete checks at exit: - Hook body line count: target ~80 lines or less, mostly fetch + compose + return. Requirement-section construction, enrollment grouping, language-option computation, and slot construction are named helpers in `model/dashboardViewModel.ts` (or `dashboardLanguagePolicy.ts`), not inline code. -- Inline transforms in the hook body: target zero. Anything that loops, filters, sorts, joins, groups, or branches on data shape is a named helper. +- The hook composes named helpers; it doesn't re-home orchestration. Glue like `enrollments.filter(existingPredicate)` or `enrollments.sort(existingComparator)` is fine — wrapping it in a named helper would be noise. What's not fine: predicates or comparators _defined_ inline, multi-step pipelines that together encode "what the program dashboard means," or grouping/joining inline. Those are named helpers in `model/dashboardViewModel.ts` (or `dashboardLanguagePolicy.ts`). - Helper test coverage: `buildRequirementSections`, slot constructors, and grouping helpers each have isolated unit tests. -- Consuming callsite shrinkage: `EnrollmentDisplay.tsx`'s program path measurably drops in size. +- Consuming callsite shrinkage: `ProgramEnrollmentDisplay.tsx`'s program path measurably drops in size. If any of these fails, the phase has relocated complexity rather than reduced it. Stop and split before declaring done. @@ -455,7 +467,7 @@ Expected: all tests pass. **Phase exit check: thin composer.** Per the Working agreement's success criterion, this phase delivers cleanup only if `useContractDashboardData.ts` is a _composer_, not a re-home of inline orchestration. Concrete checks at exit: - Hook body line count: target ~80 lines or less, mostly fetch + compose + return. Contract-scoped filtering (`programHasContractRuns`, `programsInCollections`, `sortedPrograms`), collection shaping, and slot construction are named helpers in `model/dashboardViewModel.ts`, not inline code. -- Inline transforms in the hook body: target zero. Anything that loops, filters, sorts, joins, groups, or branches on data shape is a named helper. +- The hook composes named helpers; it doesn't re-home orchestration. Glue like `enrollments.filter(existingPredicate)` or `enrollments.sort(existingComparator)` is fine — wrapping it in a named helper would be noise. What's not fine: predicates or comparators _defined_ inline, multi-step pipelines that together encode "what the contract dashboard means," or grouping/joining inline. Those are named helpers in `model/dashboardViewModel.ts`. - Helper test coverage: each contract-scoping helper has an isolated unit test that does not require mounting the full dashboard. - Consuming callsite shrinkage: `ContractContent.tsx` measurably drops in size — this is the most-watched signal because contract orchestration is currently the largest single tangle. - B2B-specific concern: shared helpers between `useProgramDashboardData` and `useContractDashboardData` are explicitly identified. If the same logic is being inlined twice instead of factored once, fix it before declaring done. @@ -464,16 +476,17 @@ If any of these fails, the phase has relocated complexity rather than reduced it ## Phase 6: Shrink render components -**Purpose:** Make `EnrollmentDisplay.tsx` and `ContractContent.tsx` mostly presentational after their data composers exist. +**Purpose:** Make `HomeEnrollmentsDisplay.tsx`, `ProgramEnrollmentDisplay.tsx`, and `ContractContent.tsx` mostly presentational after their data composers exist. **Files:** -- Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx` +- Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.tsx` +- Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.tsx` - Modify: `frontends/main/src/app-pages/DashboardPage/ContractContent.tsx` - Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramAsCourseCard.tsx` if module-slot inputs are now available - Test: existing dashboard tests touched above -- [ ] Remove now-duplicated inline query and grouping code from `EnrollmentDisplay.tsx`. +- [ ] Remove now-duplicated inline query and grouping code from `HomeEnrollmentsDisplay.tsx` and `ProgramEnrollmentDisplay.tsx`. - [ ] Remove now-duplicated inline query and grouping code from `ContractContent.tsx`. - [ ] Keep UI state that is truly UI-only in components: - selected language key, @@ -486,7 +499,8 @@ If any of these fails, the phase has relocated complexity rather than reduced it - [ ] Run: ```bash -yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx yarn test frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramAsCourseCard.test.tsx ``` @@ -511,7 +525,7 @@ This is a structural unification, not a redesign. Each variant must reproduce to - `contractCourse` — only if contract-specific behavior cannot be expressed via existing variant inputs (`contractId`, `useVerifiedEnrollment`); prefer to fold this into `courseEnrollment` / `unenrolledCourse` rather than add a variant. - [ ] For each variant, write a test that asserts byte-for-byte rendering parity with the corresponding case in today's `DashboardCard.test.tsx` / `ModuleCard.test.tsx`. Port — don't rewrite — these tests against `CoursewareCard`. - [ ] Migrate callsites in this order, with the prior step verified before moving on: - 1. Home dashboard (`AllEnrollmentsDisplay`) — lowest risk; one card per enrollment, V3 data. + 1. Home dashboard (`HomeEnrollmentsDisplay`) — lowest risk; one card per enrollment, V3 data. 2. Program-as-course rows (`ProgramAsCourseCard`) — replaces `ModuleCard` usage; localized to one component. 3. Program dashboard (`ProgramEnrollmentDisplay`) — slot-driven by Phase 4's hook. 4. Contract dashboard (`ContractContent.tsx`) — highest risk; B2B paths. @@ -564,7 +578,8 @@ Do not answer these in this refactor: Run targeted tests after each phase. Before merging the final phase in a PR series, run: ```bash -yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx yarn test frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramAsCourseCard.test.tsx yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/languageOptions.test.ts @@ -624,7 +639,11 @@ Test files (`*.test.tsx`, `*.test.ts`) are omitted for brevity. Each non-test so - DashboardCard.tsx deleted (Phase 7) - ModuleCard.tsx deleted (Phase 7) + CoursewareCard.tsx new (Phase 7) — unified router -~ EnrollmentDisplay.tsx shrinks substantially (Phase 3, 4, 6) ++ HomeEnrollmentsDisplay.tsx new (Phase 3) — extracted from EnrollmentDisplay.tsx; + carries inlined useHomeDashboardData composer +~ ProgramEnrollmentDisplay.tsx renamed from EnrollmentDisplay.tsx (Phase 3); + shrinks substantially (Phase 4, 6); will carry + useProgramDashboardData composer (Phase 4) ~ ProgramAsCourseCard.tsx uses CoursewareCard moduleRow (Phase 7) ~ OrganizationCards.tsx decouples DashboardCardRoot (Phase 7) ~ DashboardDialogs.tsx may shift if dialog wiring moves (Phase 7) @@ -645,15 +664,13 @@ Test files (`*.test.tsx`, `*.test.ts`) are omitted for brevity. Each non-test so absorbed contents of helpers.ts + languageOptions.ts (Phase 7) + dashboardAdapters.ts temp adapter — may be deleted in Phase 7 -+ hooks/ new directory (Phase 3–5) -+ useHomeDashboardData.ts Phase 3 -+ useProgramDashboardData.ts Phase 4 -+ useContractDashboardData.ts Phase 5 + # No hooks/ directory: useHomeDashboardData was inlined in HomeEnrollmentsDisplay.tsx (Phase 3). + # Location of useProgramDashboardData / useContractDashboardData (Phase 4–5) TBD per Open question. ``` **Reading notes:** - The `language/` directory is bracketed by Phase 2's exit decision — it's only created if the existing primitives become internal. Otherwise the composite is added to `languageOptions.ts` and no new file appears. - `model/dashboardAdapters.ts` shows up as new in Phase 1 but is a deletion candidate in Phase 7 — its lifespan depends on whether `CoursewareCard` consumes the slot directly without translation. -- Net file-count effect: roughly +6 to +9 added, –4 deleted; the surviving render-heavy components (`EnrollmentDisplay.tsx`, `ContractContent.tsx`) shrink substantially. +- Net file-count effect: roughly +5 to +8 added, –4 deleted (no `hooks/` directory after Phase 3 chose to inline composers); the surviving render-heavy components (`HomeEnrollmentsDisplay.tsx`, `ProgramEnrollmentDisplay.tsx`, `ContractContent.tsx`) shrink substantially. - The biggest LoC win is the deletion of `DashboardCard.tsx` (~1099 lines) and `ModuleCard.tsx` (~933 lines). New files are smaller and single-purpose. diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.test.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.test.ts index 561d244e1a..5f6cdbab89 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.test.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.test.ts @@ -1,8 +1,17 @@ +import { DisplayModeEnum } from "@mitodl/mitxonline-api-axios/v2" import { factories } from "api/mitxonline-test-utils" import { + bucketAndSortHomeEnrollments, + enrollmentCourseIsInPrograms, getDistinctDashboardLanguageOptions, + getModuleCourseIdsFromPrograms, + getNonContractProgramEnrollments, + getTopLevelProgramEnrollments, groupCourseRunEnrollmentsByCourseId, + groupModuleCoursesByProgramId, groupProgramEnrollmentsByProgramId, + isNonContractEnrollment, + isProgramAsCourse, pickDisplayedEnrollmentForLegacyDashboard, resolveSlotForLanguage, } from "./dashboardViewModel" @@ -63,30 +72,25 @@ describe("dashboardViewModel", () => { }) describe("groupCourseRunEnrollmentsByCourseId", () => { - test("groups enrollments by the course that owns their run", () => { - const run1 = factories.courses.courseRun({ id: 11 }) - const run2 = factories.courses.courseRun({ id: 22 }) - const course1 = factories.courses.course({ id: 1, courseruns: [run1] }) - const course2 = factories.courses.course({ id: 2, courseruns: [run2] }) - - const enrollment1 = factories.enrollment.courseEnrollment({ - run: { id: 11 }, + test("groups enrollments by their run's course id", () => { + const enrollmentA1 = factories.enrollment.courseEnrollment({ + run: { course: { id: 1 } }, }) - const enrollment2 = factories.enrollment.courseEnrollment({ - run: { id: 22 }, + const enrollmentA2 = factories.enrollment.courseEnrollment({ + run: { course: { id: 1 } }, }) - const unknownRunEnrollment = factories.enrollment.courseEnrollment({ - run: { id: 999 }, + const enrollmentB = factories.enrollment.courseEnrollment({ + run: { course: { id: 2 } }, }) - const grouped = groupCourseRunEnrollmentsByCourseId( - [course1, course2], - [enrollment1, enrollment2, unknownRunEnrollment], - ) + const grouped = groupCourseRunEnrollmentsByCourseId([ + enrollmentA1, + enrollmentA2, + enrollmentB, + ]) - expect(grouped[1]).toEqual([enrollment1]) - expect(grouped[2]).toEqual([enrollment2]) - expect(grouped[999]).toBeUndefined() + expect(grouped[1]).toEqual([enrollmentA1, enrollmentA2]) + expect(grouped[2]).toEqual([enrollmentB]) }) }) @@ -109,6 +113,314 @@ describe("dashboardViewModel", () => { }) }) + describe("isProgramAsCourse", () => { + test("matches programs whose display_mode is Course", () => { + const courseProgram = factories.programs.program({ + display_mode: DisplayModeEnum.Course, + }) + const regularProgram = factories.programs.program({ display_mode: null }) + expect(isProgramAsCourse(courseProgram)).toBe(true) + expect(isProgramAsCourse(regularProgram)).toBe(false) + }) + }) + + describe("isNonContractEnrollment", () => { + test("matches enrollments with no b2b_contract_id", () => { + const nonContract = factories.enrollment.courseEnrollment({ + b2b_contract_id: null, + }) + const contract = factories.enrollment.courseEnrollment({ + b2b_contract_id: 42, + }) + expect(isNonContractEnrollment(nonContract)).toBe(true) + expect(isNonContractEnrollment(contract)).toBe(false) + }) + }) + + describe("enrollmentCourseIsInPrograms", () => { + test("returns predicate matching enrollments whose course id appears in any program", () => { + const programA = factories.programs.program({ courses: [10, 11] }) + const programB = factories.programs.program({ courses: [20] }) + const inA = factories.enrollment.courseEnrollment({ + run: { course: { id: 10 } }, + }) + const inB = factories.enrollment.courseEnrollment({ + run: { course: { id: 20 } }, + }) + const outside = factories.enrollment.courseEnrollment({ + run: { course: { id: 99 } }, + }) + + const isCovered = enrollmentCourseIsInPrograms([programA, programB]) + expect([inA, inB, outside].filter(isCovered)).toEqual([inA, inB]) + }) + }) + + describe("getNonContractProgramEnrollments", () => { + test("excludes program enrollments whose program is in any contract", () => { + const inContract = factories.enrollment.programEnrollmentV3({ + program: factories.programs.program({ id: 1 }), + }) + const standalone = factories.enrollment.programEnrollmentV3({ + program: factories.programs.program({ id: 2 }), + }) + const contracts = [factories.contracts.contract({ programs: [1] })] + + expect( + getNonContractProgramEnrollments([inContract, standalone], contracts), + ).toEqual([standalone]) + }) + }) + + describe("getTopLevelProgramEnrollments", () => { + test("excludes program enrollments whose program is a child in another program's req_tree", () => { + const childProgram = factories.programs.program({ id: 50 }) + const parentProgram = factories.programs.program({ + id: 51, + req_tree: [ + { + id: 1, + data: { + node_type: "operator", + operator: "all_of", + operator_value: null, + elective_flag: false, + title: null, + course: null, + required_program: null, + }, + children: [ + { + id: 2, + data: { + node_type: "program", + required_program: childProgram.id, + operator: null, + operator_value: null, + elective_flag: false, + title: null, + course: null, + }, + children: [], + }, + ], + }, + ], + }) + const childEnrollment = factories.enrollment.programEnrollmentV3({ + program: childProgram, + }) + const parentEnrollment = factories.enrollment.programEnrollmentV3({ + program: parentProgram, + }) + + expect( + getTopLevelProgramEnrollments( + [childEnrollment, parentEnrollment], + [parentProgram], + ), + ).toEqual([parentEnrollment]) + }) + }) + + describe("getModuleCourseIdsFromPrograms", () => { + test("returns the distinct set of course ids referenced anywhere in the programs' req_trees", () => { + const program = factories.programs.program({ + req_tree: [ + { + id: 1, + data: { + node_type: "operator", + operator: "all_of", + operator_value: null, + elective_flag: false, + title: null, + course: null, + required_program: null, + }, + children: [ + { + id: 2, + data: { + node_type: "course", + course: 100, + operator: null, + operator_value: null, + elective_flag: false, + title: null, + required_program: null, + }, + children: [], + }, + { + id: 3, + data: { + node_type: "course", + course: 200, + operator: null, + operator_value: null, + elective_flag: false, + title: null, + required_program: null, + }, + children: [], + }, + { + id: 4, + data: { + node_type: "course", + course: 100, + operator: null, + operator_value: null, + elective_flag: false, + title: null, + required_program: null, + }, + children: [], + }, + ], + }, + ], + }) + + expect(getModuleCourseIdsFromPrograms([program]).sort()).toEqual([ + 100, 200, + ]) + }) + + test("dedupes course ids across multiple programs", () => { + const makeCourseLeaf = (id: number, courseId: number) => ({ + id, + data: { + node_type: "course" as const, + course: courseId, + operator: null, + operator_value: null, + elective_flag: false, + title: null, + required_program: null, + }, + children: [], + }) + const makeProgram = (programId: number, courseIds: number[]) => + factories.programs.program({ + id: programId, + req_tree: [ + { + id: programId * 10, + data: { + node_type: "operator", + operator: "all_of", + operator_value: null, + elective_flag: false, + title: null, + course: null, + required_program: null, + }, + children: courseIds.map((c) => makeCourseLeaf(c + programId, c)), + }, + ], + }) + + const programA = makeProgram(1, [100, 200]) + const programB = makeProgram(2, [200, 300]) + + expect( + getModuleCourseIdsFromPrograms([programA, programB]).sort(), + ).toEqual([100, 200, 300]) + }) + }) + + describe("groupModuleCoursesByProgramId", () => { + test("indexes courses by the program whose courses[] lists them", () => { + const programA = factories.programs.program({ id: 1, courses: [10, 11] }) + const programB = factories.programs.program({ id: 2, courses: [20] }) + const course10 = factories.courses.course({ id: 10 }) + const course11 = factories.courses.course({ id: 11 }) + const course20 = factories.courses.course({ id: 20 }) + const courseUnreferenced = factories.courses.course({ id: 99 }) + + const grouped = groupModuleCoursesByProgramId( + [programA, programB], + [course10, course11, course20, courseUnreferenced], + ) + + expect(grouped[1]).toEqual([course10, course11]) + expect(grouped[2]).toEqual([course20]) + }) + + test("maps program with empty courses[] to an empty array", () => { + const program = factories.programs.program({ id: 1, courses: [] }) + const course = factories.courses.course({ id: 10 }) + expect(groupModuleCoursesByProgramId([program], [course])).toEqual({ + 1: [], + }) + }) + }) + + describe("bucketAndSortHomeEnrollments", () => { + const past = new Date(Date.now() - 1000 * 60 * 60 * 24).toISOString() + const future = new Date(Date.now() + 1000 * 60 * 60 * 24).toISOString() + + test("buckets enrollments by status and sorts each bucket", () => { + const completedEnrollment = factories.enrollment.courseEnrollment({ + grades: [factories.enrollment.grade({ grade: 0.9, passed: true })], + run: { course: { title: "Zeta" } }, + }) + const expiredEnrollment = factories.enrollment.courseEnrollment({ + run: { start_date: past, end_date: past, course: { title: "Alpha" } }, + }) + const startedAlpha = factories.enrollment.courseEnrollment({ + run: { start_date: past, end_date: future, course: { title: "Alpha" } }, + }) + const startedBeta = factories.enrollment.courseEnrollment({ + run: { start_date: past, end_date: future, course: { title: "Beta" } }, + }) + const notStartedSooner = factories.enrollment.courseEnrollment({ + run: { + start_date: future, + end_date: null, + course: { title: "Late" }, + }, + }) + const notStartedLater = factories.enrollment.courseEnrollment({ + run: { + start_date: new Date( + Date.now() + 1000 * 60 * 60 * 24 * 7, + ).toISOString(), + end_date: null, + course: { title: "Earlier-by-title-but-later-by-date" }, + }, + }) + + const buckets = bucketAndSortHomeEnrollments([ + notStartedLater, + startedBeta, + startedAlpha, + notStartedSooner, + expiredEnrollment, + completedEnrollment, + ]) + + expect(buckets.completed).toEqual([completedEnrollment]) + expect(buckets.expired).toEqual([expiredEnrollment]) + expect(buckets.started).toEqual([startedAlpha, startedBeta]) + expect(buckets.notStarted).toEqual([notStartedSooner, notStartedLater]) + }) + + test("classifies a completed enrollment whose end_date has passed as completed, not expired", () => { + const completedAndExpired = factories.enrollment.courseEnrollment({ + grades: [factories.enrollment.grade({ grade: 0.9, passed: true })], + run: { start_date: past, end_date: past }, + }) + + const buckets = bucketAndSortHomeEnrollments([completedAndExpired]) + + expect(buckets.completed).toEqual([completedAndExpired]) + expect(buckets.expired).toEqual([]) + }) + }) + describe("getDistinctDashboardLanguageOptions", () => { test("includes enrollment language not present in enrollable language options", () => { const englishRun = factories.courses.courseRun({ diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts index 942baadbc5..02722357e8 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts @@ -8,13 +8,22 @@ */ import type { SimpleSelectOption } from "ol-components" import type { + BaseProgramDisplayMode, + ContractPage, CourseRunEnrollmentV3, CourseRunLanguageOption, CourseRunV2, CourseWithCourseRunsSerializerV2, + V2ProgramDetail, V3UserProgramEnrollment, } from "@mitodl/mitxonline-api-axios/v2" -import { selectBestEnrollment } from "../helpers" +import { DisplayModeEnum } from "@mitodl/mitxonline-api-axios/v2" +import { getIdsFromReqTree } from "@/common/mitxonline" +import { + EnrollmentStatus, + getEnrollmentStatus, + selectBestEnrollment, +} from "../helpers" import { getNativeLanguageName, // below five are used only for resolveSlotForLanguage @@ -91,23 +100,11 @@ const pickDisplayedEnrollmentForLegacyDashboard = ( } const groupCourseRunEnrollmentsByCourseId = ( - courses: CourseWithCourseRunsSerializerV2[], enrollments: CourseRunEnrollmentV3[], ): Record => { - const runIdToCourseId = new Map() - courses.forEach((course) => { - course.courseruns.forEach((run) => { - runIdToCourseId.set(run.id, course.id) - }) - }) - return enrollments.reduce>( (acc, enrollment) => { - const courseId = runIdToCourseId.get(enrollment.run.id) - if (!courseId) { - return acc - } - + const courseId = enrollment.run.course.id acc[courseId] = [...(acc[courseId] ?? []), enrollment] return acc }, @@ -127,6 +124,156 @@ const groupProgramEnrollmentsByProgramId = ( ) } +const isProgramAsCourse = (program: { + display_mode?: BaseProgramDisplayMode | null +}) => program.display_mode === DisplayModeEnum.Course + +const isNonContractEnrollment = (enrollment: CourseRunEnrollmentV3) => + !enrollment.b2b_contract_id + +/** + * Closure-factory predicate: returns a predicate that tests whether an + * enrollment's course id appears in any of the given programs' `courses[]`. + * The Set is built once per factory call and reused across the predicate's + * invocations, so the natural usage is `enrollments.filter(predicate)`. + */ +const enrollmentCourseIsInPrograms = (programs: V2ProgramDetail[]) => { + const coveredCourseIds = new Set(programs.flatMap((p) => p.courses)) + return (enrollment: CourseRunEnrollmentV3) => + coveredCourseIds.has(enrollment.run.course.id) +} + +const getNonContractProgramEnrollments = ( + programEnrollments: V3UserProgramEnrollment[], + contracts: ContractPage[], +) => { + const contractPrograms = new Set( + contracts.flatMap((contract) => contract.programs), + ) + return programEnrollments.filter( + (enrollment) => !contractPrograms.has(enrollment.program.id), + ) +} + +const getTopLevelProgramEnrollments = ( + programEnrollments: V3UserProgramEnrollment[], + programs: V2ProgramDetail[], +) => { + const childIds = new Set( + programs.flatMap( + (program) => getIdsFromReqTree(program.req_tree).programIds, + ), + ) + return programEnrollments.filter( + (enrollment) => !childIds.has(enrollment.program.id), + ) +} + +/** + * Distinct course ids referenced anywhere in the given programs' req_trees. + * Used to drive the home dashboard's program-as-course module course query. + */ +const getModuleCourseIdsFromPrograms = ( + programs: V2ProgramDetail[], +): number[] => { + return [ + ...new Set( + programs.flatMap( + (program) => getIdsFromReqTree(program.req_tree).courseIds, + ), + ), + ] +} + +/** + * For each program, the subset of `courses` whose ids appear in + * `program.courses[]`. Used by both home and program dashboards to wire + * program-as-course module data into the cards that render it. + */ +const groupModuleCoursesByProgramId = ( + programs: V2ProgramDetail[], + courses: CourseWithCourseRunsSerializerV2[], +): Record => { + return programs.reduce>( + (acc, program) => { + const programCourseIds = new Set(program.courses) + acc[program.id] = courses.filter((course) => + programCourseIds.has(course.id), + ) + return acc + }, + {}, + ) +} + +const byTitle = (a: CourseRunEnrollmentV3, b: CourseRunEnrollmentV3) => + a.run.course.title.localeCompare(b.run.course.title) + +const byStartsSooner = (a: CourseRunEnrollmentV3, b: CourseRunEnrollmentV3) => { + if (!a.run.start_date && !b.run.start_date) return 0 + if (!a.run.start_date) return 1 + if (!b.run.start_date) return -1 + return ( + new Date(a.run.start_date).getTime() - new Date(b.run.start_date).getTime() + ) +} + +type HomeEnrollmentBuckets = { + started: CourseRunEnrollmentV3[] + notStarted: CourseRunEnrollmentV3[] + completed: CourseRunEnrollmentV3[] + expired: CourseRunEnrollmentV3[] +} + +/** + * Bucket enrollments into the four home-dashboard sections and sort each + * bucket. Assumes input is already filtered to the enrollments the home + * dashboard renders (e.g. non-contract). + * + * Bucket policy: + * - completed: any passing grade + * - expired: end_date in the past + * - started: start_date in the past, not expired, not completed + * - notStarted: everything else + * + * Sort policy: alphabetical by course title in every bucket except + * notStarted, which is sorted by start_date ascending. + */ +const bucketAndSortHomeEnrollments = ( + enrollments: CourseRunEnrollmentV3[], +): HomeEnrollmentBuckets => { + const now = new Date() + const buckets: HomeEnrollmentBuckets = { + started: [], + notStarted: [], + completed: [], + expired: [], + } + enrollments.forEach((enrollment) => { + if (getEnrollmentStatus(enrollment) === EnrollmentStatus.Completed) { + buckets.completed.push(enrollment) + } else if ( + enrollment.run.end_date && + new Date(enrollment.run.end_date) < now + ) { + buckets.expired.push(enrollment) + } else if ( + enrollment.run.start_date && + new Date(enrollment.run.start_date) < now + ) { + buckets.started.push(enrollment) + } else { + buckets.notStarted.push(enrollment) + } + }) + return { + started: buckets.started.sort(byTitle), + notStarted: buckets.notStarted.sort(byStartsSooner), + completed: buckets.completed.sort(byTitle), + expired: buckets.expired.sort(byTitle), + } +} + /** * The mitxonline API emits language codes in POSIX form (`de_de`, * `pt_br`, `zh_hans`); convert to BCP 47 (`de-de`, `pt-br`, `zh-hans`) @@ -286,4 +433,12 @@ export { groupProgramEnrollmentsByProgramId, resolveSlotForLanguage, getDistinctDashboardLanguageOptions, + isProgramAsCourse, + isNonContractEnrollment, + enrollmentCourseIsInPrograms, + getNonContractProgramEnrollments, + getTopLevelProgramEnrollments, + getModuleCourseIdsFromPrograms, + groupModuleCoursesByProgramId, + bucketAndSortHomeEnrollments, } diff --git a/frontends/main/src/app-pages/DashboardPage/ProgramContent.test.tsx b/frontends/main/src/app-pages/DashboardPage/ProgramContent.test.tsx index 0e80dbb16b..057bcb4fec 100644 --- a/frontends/main/src/app-pages/DashboardPage/ProgramContent.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/ProgramContent.test.tsx @@ -2,10 +2,9 @@ import React from "react" import { renderWithProviders, screen } from "@/test-utils" import ProgramContent from "./ProgramContent" -// Mock the ProgramEnrollmentDisplay component jest.mock("./CoursewareDisplay/ProgramEnrollmentDisplay", () => ({ ProgramEnrollmentDisplay: jest.fn(({ programId }) => ( -
+
Mocked ProgramEnrollmentDisplay with programId: {programId}
)), @@ -16,9 +15,11 @@ describe("ProgramContent", () => { const programId = 123 renderWithProviders() - const enrollmentDisplay = screen.getByTestId("enrollment-display") - expect(enrollmentDisplay).toBeInTheDocument() - expect(enrollmentDisplay).toHaveAttribute("data-program-id", "123") + const programEnrollmentDisplay = screen.getByTestId( + "program-enrollment-display", + ) + expect(programEnrollmentDisplay).toBeInTheDocument() + expect(programEnrollmentDisplay).toHaveAttribute("data-program-id", "123") }) test("passes correct programId to ProgramEnrollmentDisplay", () => { @@ -34,13 +35,13 @@ describe("ProgramContent", () => { test("handles different programId values", () => { const { view } = renderWithProviders() - expect(screen.getByTestId("enrollment-display")).toHaveAttribute( + expect(screen.getByTestId("program-enrollment-display")).toHaveAttribute( "data-program-id", "1", ) view.rerender() - expect(screen.getByTestId("enrollment-display")).toHaveAttribute( + expect(screen.getByTestId("program-enrollment-display")).toHaveAttribute( "data-program-id", "999", ) From d8127306d4ee611d60eb923213102135a1c4b7b4 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 13 May 2026 19:03:50 -0400 Subject: [PATCH 2/2] Add home dashboard B2B course filter test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../HomeEnrollmentsDisplay.test.tsx | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx index 1b54e91825..95ae0826aa 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx @@ -819,6 +819,47 @@ describe("HomeEnrollmentsDisplay", () => { expect(screen.queryByText(b2bSimpleProgram.title)).not.toBeInTheDocument() }) + test("Filters B2B course enrollments from display", async () => { + const mitxOnlineUser = mitxonline.factories.user.user() + setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) + + mockedUseFeatureFlagEnabled.mockReturnValue(true) + + const b2bEnrollment = mitxonline.factories.enrollment.courseEnrollment({ + b2b_contract_id: 42, + run: { + title: "B2B Hidden Course", + course: { title: "B2B Hidden Course" }, + }, + }) + const nonB2BEnrollment = mitxonline.factories.enrollment.courseEnrollment({ + b2b_contract_id: null, + run: { + title: "Personal Visible Course", + course: { title: "Personal Visible Course" }, + }, + }) + + setMockResponse.get(mitxonline.urls.enrollment.enrollmentsListV3(), [ + b2bEnrollment, + nonB2BEnrollment, + ]) + setMockResponse.get( + mitxonline.urls.programEnrollments.enrollmentsListV3(), + [], + ) + + renderWithProviders() + + await screen.findByRole("heading", { name: "My Learning" }) + + const personalCourseCards = await screen.findAllByText( + nonB2BEnrollment.run.title, + ) + expect(personalCourseCards.length).toBeGreaterThan(0) + expect(screen.queryByText(b2bEnrollment.run.title)).not.toBeInTheDocument() + }) + test("Hides child program enrollments that are part of an enrolled parent program", async () => { const mitxOnlineUser = mitxonline.factories.user.user() setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser)