From 52548c0a30d5a3e0cdb979bcc63e0a6acc510498 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Mon, 11 May 2026 11:09:43 -0400 Subject: [PATCH 1/4] dashboard refactor phase 1 (#3320) * add refactor plan document * add inital adapters and view models, * add some suggested documentation changes --- .../dashboardRefactorPlan.md | 649 ++++++++++++++++++ .../CoursewareDisplay/helpers.ts | 21 +- .../model/dashboardAdapters.test.ts | 191 ++++++ .../model/dashboardAdapters.ts | 44 ++ .../model/dashboardViewModel.test.ts | 109 +++ .../model/dashboardViewModel.ts | 123 ++++ 6 files changed, 1118 insertions(+), 19 deletions(-) create mode 100644 frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md create mode 100644 frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.test.ts create mode 100644 frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.ts create mode 100644 frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.test.ts create mode 100644 frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md new file mode 100644 index 0000000000..2f6544413d --- /dev/null +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/dashboardRefactorPlan.md @@ -0,0 +1,649 @@ +# Dashboard Cleanup Implementation Plan + +> **For agentic workers:** This plan is **human-in-the-loop**. Do not autonomously execute multiple phases. Each phase ends at a review checkpoint with the human reviewer (see [Working agreement](#working-agreement)). The checkbox lists in each phase are **starting hypotheses**, not exhaustive specs — verify by reading current code before mechanically moving things, and surface anything you find that the plan didn't anticipate. + +**Goal:** Make the dashboard easier to reason about by separating data orchestration from rendering. Program and contract dashboards become slot-driven with `enrollments[]` as the source of truth, removing today's premature N→1 collapse. Visible behavior is preserved until multi-run UX is decided. + +**Architecture:** Use dashboard-specific data composers to move query orchestration, requirement-tree shaping, enrollment grouping, and language/run resolution out of render-heavy components. Keep `My Learning` enrollment-driven, make program/contract dashboards slot-driven, and adapt the new view model back into the existing cards before attempting card replacement. + +**Tech Stack:** Next.js App Router, React, TypeScript, React Query, generated MITxOnline API hooks, Jest + React Testing Library, `ol-components`, `@mitodl/smoot-design`. + +--- + +## Working agreement + +This plan is not a script. Every phase is a judgment opportunity, not a checklist to be auto-executed. + +**Cadence.** One phase at a time, with an explicit review checkpoint at each phase boundary. Agents should not advance to the next phase without human acknowledgement that the previous phase achieved its purpose. + +**Success criterion: complexity is reduced, not relocated.** The user-visible improvement of this refactor is "the dashboard is easier to reason about." That improvement is _not_ delivered by adding new directories or by colocating queries into hooks. It is delivered by **each artifact having a single, name-able responsibility that fits in your head**. A hook that composes 6 named helpers fits in your head; a hook that inlines 6 transforms does not. Moving a 700-line orchestration into a 700-line hook is a failure of this phase, even if every checkbox is ticked. Every extracted unit must be smaller, more focused, and more testable than what it replaced — or the extraction has not delivered cleanup. + +**Phase-boundary review questions.** At the end of every phase, the agent and reviewer answer together: + +1. **Did this phase make the dashboard easier to reason about?** Concretely — what is now smaller, more isolated, or better tested? If the answer is "not really," investigate why before continuing. +2. **What did we discover that the plan didn't anticipate?** Adjacent dead code, duplicated logic, missing tests, surprising couplings, naming inconsistencies. Decide per-finding: fold into the current phase, schedule for a later phase, file as a follow-up note, or explicitly drop. +3. **Is the next phase still scoped correctly?** Reality may differ from the plan's assumptions. Re-scope before proceeding rather than forcing the original list. +4. **Is anything in this phase actually a behavior change in disguise?** If yes, stop and split it into its own PR with product acknowledgement (per the goal: visible behavior is preserved unless explicitly called out). + +**How to read the phase task lists.** + +- Each checkbox is a **hypothesis** ("we believe this helper belongs in `dashboardLanguagePolicy.ts`"). Verify by reading callsites and tests before moving code. +- Lists are **starting points**, not exhaustive. If the phase needs less, do less. If it needs more, surface it at the checkpoint instead of silently expanding scope. +- Do not treat completion of every checkbox as "phase done." Phase done = the review questions above are answered well. + +**When to stop the phase.** + +- An assumption in the plan turns out to be wrong (e.g., a function is used somewhere not anticipated). +- A behavior change appears unavoidable. +- Test coverage in the area being touched is thinner than expected. +- The current phase is clearly delivering less cleanup value than expected. + +In all cases: stop, write up what you found, and pair with the human reviewer before continuing. + +--- + +## High-level summary + +This work is worthwhile now because the current dashboard implementation has structural risk independent of final product decisions about multi-run users. `EnrollmentDisplay.tsx` and `ContractContent.tsx` both mix data fetching, API shape translation, language selection, enrollment selection, requirement-tree shaping, and rendering. `DashboardCard.tsx` and `ModuleCard.tsx` overlap heavily, but deleting or unifying them first would preserve the wrong data contract. The code also repeatedly collapses multiple enrollments into one selected enrollment, which hides information that any future multi-run UI will need. + +Before introducing more new UX to the Dashboard, this plan makes the internal dashboard model honest: program and contract course slots should carry all matching enrollments, plus a clearly named temporary display choice. Current behavior should remain mostly stable while product decides whether alternate runs are shown via dropdown, dialog, inline list, expanded mode, or some other affordance. + +## Codebase review assessment + +Based on the current code, the plan is technically correct with the constraints below. + +### Confirmed by code + +- `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx` is overloaded: + - imports React Query hooks directly, + - builds home enrollment sections, + - builds program requirement sections, + - owns dashboard-wide program language state, + - calls `getSelectedLanguageOption`, `getCourseRunForSelectedLanguage`, `getEnrollmentForSelectedLanguage`, `getResolvedRunForSelectedLanguage`, + - calls `selectBestEnrollment` when no language-specific enrollment is selected, + - renders `DashboardCard` and `ProgramAsCourseCard`. +- `frontends/main/src/app-pages/DashboardPage/ContractContent.tsx` repeats much of that orchestration with contract-specific behavior: + - loads enrollments, programs, program collections, and courses, + - computes language options from contract courses, + - filters programs by contract membership and contract-scoped course runs, + - renders `OrgProgramDisplay` and `OrgProgramCollectionDisplay`, + - calls `selectBestContractEnrollmentForLanguage`, `getCourseRunForSelectedLanguage`, and `getResolvedRunForSelectedLanguage` at multiple card callsites. +- `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts` has `selectBestEnrollment(course, enrollments)`, which reduces all matching enrollments for a course to one enrollment using certificate, grade, then first-match priority. +- `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/languageOptions.ts` has several single-enrollment/run resolution helpers: + - `getEnrollmentForSelectedLanguage`, + - `selectBestContractEnrollmentForLanguage`, + - `getResolvedRunForSelectedLanguage`. +- `getResolvedRunForSelectedLanguage` currently adapts a V3 enrollment run into a V2 `CourseRunV2`-shaped object for downstream card consumers. That is a compatibility layer, not a desirable long-term enrolled-card model. +- `ProgramAsCourseCard.tsx` repeatedly calls `selectBestEnrollment` for module progress and module card rendering, then renders `ModuleCard`. +- `DashboardCard.tsx` still owns substantial business behavior: + - course/program/course-run resource branching, + - enrollment click behavior, + - B2B contract inference, + - title, certificate, upgrade banner, CTA, status indicator, start-date countdown, and context menu assembly. +- `ModuleCard.tsx` is a near-parallel card implementation with differences in button labels, certificate text, layout, and enrollment handler props. +- `OrganizationCards.tsx` imports only `DashboardCardRoot` from `DashboardCard.tsx`, so card deletion would need to decouple this styled dependency. + +### Corrections to earlier plans + +- Do not make `[0]` of a sorted enrollment list the durable card contract. It is acceptable as a temporary adapter detail, but the canonical slot model must name the derived display value explicitly. +- Do not change default-run selection policy to `status bucket > start_date` until product accepts the behavior. The current `selectBestEnrollment` policy may be imperfect, but replacing it is a product-visible change for multi-run users. +- Do not ship button-label convergence, CTA run annotations, course-level certificate aggregation changes, or "Completed badge + Continue CTA" behavior as part of this structural cleanup. +- Do not commit to deleting `DashboardCard.tsx` and `ModuleCard.tsx` on a fixed schedule. Delete them only after the new data model is in place, behavior parity is verified, and B2B/program-collection rendering has migrated safely. +- Do not force `My Learning` into the slot model. Home currently renders one card per enrollment, which already exposes multiple enrollments. Program and contract dashboards are the places that need slot + enrollments modeling. + +## Goals + +- Reduce dashboard complexity without waiting for final multi-run UX. +- Move data fetching, joining, grouping, and language/run resolution out of render-heavy components. +- Preserve visible behavior exactly during the first phases, except for explicitly called-out bug fixes or explicitly product-approved changes. +- Make program and contract dashboard course slots carry all relevant enrollments. +- Make the selected/displayed enrollment a derived presentation choice, not the canonical data shape. +- Keep current dashboard distinctions: + - home is enrollment-driven, + - program dashboard is requirement-slot-driven, + - contract dashboard is contract/program/collection-slot-driven. +- Centralize language/run policy so future multi-run UI is additive rather than another rewrite. + +## Non-goals + +- Choosing the final multi-run UX. +- Adding a multi-run dropdown, dialog, inline list, or expanded-run UI. +- Changing dashboard visuals or button copy. +- Changing default-run behavior unless required for a bug fix and explicitly called out. +- Backend API changes. +- Removing all V2/V3 compatibility immediately. +- Full B2C/B2B card convergence in the first implementation phase. +- Do not force full structural B2C/B2B card unification in the first implementation phase before the new data model is stable. Visual and copy convergence are in scope where they are already approved or clearly corrective. + +## Target model + +### Home dashboard model + +`My Learning` should remain enrollment-flat. + +```ts +type HomeDashboardData = { + isLoading: boolean + upgradeError: string | null + enrollments: { + started: CourseRunEnrollmentV3[] + notStarted: CourseRunEnrollmentV3[] + completed: CourseRunEnrollmentV3[] + expired: CourseRunEnrollmentV3[] + } + programEnrollments: V3UserProgramEnrollment[] + programAsCourseData: ProgramAsCourseViewModel[] +} +``` + +: + +> [!NOTE] +> The exact shape can change during implementation, but home should not collapse multiple enrollments into one course slot. + +### Program and contract course slot model + +Program and contract dashboards should use a slot model. + +```ts +type DashboardCourseSlot = { + course: CourseWithCourseRunsSerializerV2 + enrollments: CourseRunEnrollmentV3[] + selectedLanguageKey: string + availableLanguages: SimpleSelectOption[] + displayedEnrollment: CourseRunEnrollmentV3 | null + displayedRun: CourseRunV2 | null + contractId?: number + isContractPageResource?: boolean + ancestorContext?: { + programEnrollment?: V3UserProgramEnrollment + parentProgramReadableIds?: string[] + useVerifiedEnrollment?: boolean + } +} +``` + +Rules: + +- `enrollments` is the source of truth. +- `displayedEnrollment` and `displayedRun` are derived compatibility/display values. +- The initial derivation should preserve existing behavior. If this seems impossible or unreasonable/undesirable, raise attention. +- `displayedRun` is V2-shaped only because legacy `DashboardCard` requires `selectedCourseRun`. It is produced by the existing V3→V2 synthesis (Case 1 of `getResolvedRunForSelectedLanguage`). +- When legacy cards are replaced (Phase 8), enrolled cards consume V3 enrollment data from `enrollments[]` directly and Case 1 of the synthesis can be deleted. +- Cases 2 and 3 of `getResolvedRunForSelectedLanguage` remain. Case 2 is a trivial passthrough (the selected language has a real `CourseRunV2` in `course.courseruns`). Case 3 (pre-enrollment, no real `CourseRunV2` for the selected language) is a load-bearing workaround for an API gap: mitxonline does not surface per-language run metadata pre-enrollment. Case 3 is removable only if/when that API gap closes. + +### Open question: are display fields permanent on the slot? + +`displayedEnrollment` and `displayedRun` are not strictly necessary on the slot — the legacy adapter could compute them inline from `enrollments` + `selectedLanguageKey`. They are on the slot for convenience: the hook is computing them anyway (Phase 2's composite resolver), caching the result keeps the adapter as pure shape-conversion, and slot construction becomes unit-testable in isolation ("given course X, enrollments [A, B], lang Y → slot.displayedEnrollment = A"). +Whether these fields survive past Phase 7 hinges on multi-run UX direction: + +- **Parent-owned selection** (parent picks which enrollment to display, card receives `displayedEnrollment` as a controlled prop) → fields stay on the slot, expressing the parent's chosen display. +- **Card-owned selection** (card has internal state for enrollment selection) → fields are deleted with the legacy adapter; slot becomes purer (`course`, `enrollments`, `selectedLanguageKey`, `availableLanguages`, contract/ancestor). + Recommendation: parent-owned (consistency with `selectedLanguageKey`, no prop-to-state sync, URL-deep-linkable). But this is not blocking Phases 1–6; decide when multi-run UX lands. + +## Proposed file structure + +Add focused files under the existing dashboard area. The end state consolidates pure-model code into a single file (`model/dashboardViewModel.ts`); the existing `helpers.ts` and `languageOptions.ts` are progressively absorbed and deleted by Phase 7. + +```text +frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ + hooks/ + useHomeDashboardData.ts + useProgramDashboardData.ts + useContractDashboardData.ts + model/ + dashboardViewModel.ts + dashboardAdapters.ts +``` + +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. +- `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. + +**Layer roles (illustrative, not prescriptive — boundaries may shift at phase exits):** + +| File | Layer | Knows React? | Knows queries? | Long-term? | +| ------------------------------- | -------------------------- | ------------ | -------------- | --------------------------------------- | +| `hooks/useXxxDashboardData.ts` | Composer (data + actions) | Yes | Yes | Yes | +| `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 | +| `languageOptions.ts` (existing) | Pure model | No | No | No — absorbed into viewModel by Phase 7 | + +The `EnrollmentStatus` enum currently in `helpers.ts` is consumed by `EnrollmentStatusIndicator.tsx` and `ProgressBadge.tsx`. After consolidation it is exported from `dashboardViewModel.ts`; no separate "shared types" file is needed. + +Do not move card rendering into these files. Hooks and model helpers should return render-ready data, but cards remain responsible for presentation. + +## Phase 1: Extract pure model and adapter helpers (complete) + +**Purpose:** Introduce the slot vocabulary and behavior-preserving adapters without changing rendered output. + +**Files:** + +- Create: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts` +- Create: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.ts` +- Modify: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts` +- Test: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.test.tsx` +- Test: new tests for `dashboardViewModel.ts` and `dashboardAdapters.ts` + +- [ ] Add a `DashboardCourseSlot` type carrying `course`, `enrollments`, `selectedLanguageKey`, `displayedEnrollment`, `displayedRun`, and optional contract/ancestor context. +- [ ] Add grouping helpers for course-run enrollments by course id and program enrollments by program id. +- [ ] Add a clearly named display-policy helper, such as `pickDisplayedEnrollmentForLegacyDashboard`, that initially preserves the existing `selectBestEnrollment` behavior. +- [ ] Add adapter helpers that convert a `DashboardCourseSlot` into the existing legacy props: + - `resource`, + - `selectedCourseRun`, + - `buttonHref`, + - `contractId`, + - `programEnrollment`. +- [ ] Add tests proving that the adapter produces the same resource choice for: + - no enrollment, + - one enrollment, + - multiple enrollments where one has a certificate, + - multiple enrollments where one has the highest grade, + - selected-language enrollment, + - contract-scoped selected-language enrollment. +- [ ] Do not delete `selectBestEnrollment` in this phase. +- [ ] Do not change card labels, CTA behavior, certificate link behavior, or default displayed enrollment policy. +- [ ] Run: + +```bash +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.test.tsx +``` + +Expected: all tests pass. + +## Phase 2: Collapse language/run resolution to a single composite call + +**Purpose:** Replace the 4-call orchestration that every callsite repeats with a single composite slot-resolution function. The win is measured at callsites, not in file structure: today both `EnrollmentDisplay.tsx` and `ContractContent.tsx` call `getEnrollmentForSelectedLanguage`, `getSelectedLanguageOption`, `getCourseRunForSelectedLanguage`, and `getResolvedRunForSelectedLanguage` together every time they render a card. After Phase 2, callsites make one call and receive `{ displayedEnrollment, displayedRun, selectedLanguageOption }`. + +This phase also folds in a small, isolated bug fix: enrolled-but-not-enrollable languages currently disappear from the language picker (see [Language-union bug fix](#language-union-bug-fix) below). + +**Hypothesised approach** (verify before executing): + +- [ ] Add a composite function — working name `resolveSlotForLanguage(course, enrollments, selectedLanguageKey, { contractId })` — that returns `{ displayedEnrollment, displayedRun, selectedLanguageOption }`. Internally it calls today's primitives in the same order they are called at every callsite, so behavior is preserved by construction. **Place it in `model/dashboardViewModel.ts`** (Phase 1's pure-model home), not in `languageOptions.ts`. +- [ ] Add a composite function for the language picker — working name `getDashboardLanguageOptions(course, enrollments)` — that returns the union of enrollable V2 `course.language_options` and languages from the user's V3 `enrollment.run.language`. Also placed in `dashboardViewModel.ts`. +- [ ] Update callsites in `EnrollmentDisplay.tsx` and `ContractContent.tsx` to use the composites. The 4-call dance disappears at the callsite; render code stops knowing the orchestration exists. +- [ ] Begin migrating helpers from `languageOptions.ts` into `dashboardViewModel.ts` opportunistically (anything the composites depend on internally can move). Full deletion of `languageOptions.ts` is committed to Phase 7; this phase need only consolidate what it touches. +- [ ] Do not introduce a multi-run selector. +- [ ] Do not change the displayed-enrollment / displayed-run policy except where the language-union fix requires it. + +**Tests to add:** + +- enrolled language no longer enrollable still appears in the picker (the bug fix above); +- selected-language enrollment is preferred over next/best run; +- different-language enrollment is not displayed when a language with no matching enrollment is selected; +- contract scoping does not select an enrollment from another contract; +- fallback run synthesis still works for unenrolled selected-language cases (Case 3 of `getResolvedRunForSelectedLanguage`). + +**Run:** + +```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/ContractContent.test.tsx +``` + +Expected: all tests pass; callsite line-counts in the two render components measurably drop. + +### Language-union bug fix + +**Symptom:** When a user is enrolled in a course in language X and the course's V2 `language_options` later stops listing X as enrollable, the dashboard language picker drops X — so the user cannot select their own enrolled language and cannot see their enrollment. + +**Cause:** Today the picker is computed from `course.language_options` only. That field reflects what's currently enrollable, not what the user is already in. + +**Fix:** Compute picker options as the union of: + +- enrollable V2 `course.language_options`, and +- the language of every existing V3 enrollment for that course slot (`enrollment.run.language`). + +**Why this is in scope despite "preserve visible behavior exactly":** the existing behavior is a defect — the user loses access to their own enrollment. The fix is small, isolated, has a dedicated test, and is the only intentional behavior change in the structural cleanup phases. + +## Phase 3: Extract `useHomeDashboardData` + +**Purpose:** Make the home dashboard easier to reason about without changing its enrollment-flat behavior. + +**Files:** + +- 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` + +- [ ] Move home-only queries out of `AllEnrollmentsDisplay`: + - course-run enrollments, + - program enrollments, + - program details needed for deduping, + - contracts needed for non-contract program filtering, + - program-as-course module data if currently assembled inline. +- [ ] Move `sortEnrollments` and `dedupeEnrollments` behind the hook or pure model helpers. +- [ ] Return home data in the existing buckets: + - `started`, + - `notStarted`, + - `completed`, + - `expired`. +- [ ] 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. +- [ ] 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, + - program-course enrollments are deduped as before, + - empty sections remain hidden as before. +- [ ] Run: + +```bash +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.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: + +- 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. +- 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. + +If any of these fails, the phase has relocated complexity rather than reduced it. Stop and split before declaring done. + +## 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` +- 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/ProgramAsCourseCard.test.tsx` + +- [ ] Move program detail query and course queries into the hook. +- [ ] Move `requirementSections` construction into the hook. +- [ ] Move enrollment grouping into the hook. +- [ ] Move program enrollment lookup into the hook. +- [ ] Move available-language computation into the hook via `dashboardLanguagePolicy.ts`. +- [ ] Build course requirement items as `DashboardCourseSlot` objects. +- [ ] Preserve current selected-language state ownership in the component unless the hook needs it as an input. +- [ ] Return render-ready sections containing: + - section title, + - section node, + - section completion counts, + - items for course slots, + - items for program-as-course slots, + - items for nested program enrollments. +- [ ] Use `dashboardAdapters.ts` to keep rendering through `DashboardCard` in this phase. +- [ ] Keep `ProgramAsCourseCard` rendering through `ModuleCard` in this phase unless a minimal adapter is required. +- [ ] Add or update tests proving: + - requirement sections preserve ordering from `req_tree`, + - section completion counts remain correct, + - selected language changes displayed run/enrollment exactly as before, + - no matching selected-language enrollment renders the course/unenrolled state rather than a wrong-language enrollment, + - program-as-course sections still render module rows. +- [ ] Run: + +```bash +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.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: + +- 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. +- 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. + +If any of these fails, the phase has relocated complexity rather than reduced it. Stop and split before declaring done. + +## Phase 5: Extract `useContractDashboardData` + +**Purpose:** Move contract-scoped orchestration out of `ContractContent.tsx` while preserving B2B behavior. + +**Files:** + +- Create: `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/hooks/useContractDashboardData.ts` +- Modify: `frontends/main/src/app-pages/DashboardPage/ContractContent.tsx` +- Test: `frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx` + +- [ ] Move these queries into the hook: + - course-run enrollments, + - program enrollments, + - programs list scoped by `org_id` and `contract_id`, + - program collections, + - contract courses list. +- [ ] Move `programHasContractRuns` into the hook. +- [ ] Move `programsInCollections` and `sortedPrograms` computation into the hook. +- [ ] Move program collection filtering into the hook. +- [ ] Move `OrgProgramDisplay` course-slot data shaping into the hook or a pure helper consumed by the hook. +- [ ] Move `OrgProgramCollectionDisplay` first-course-per-program data shaping into the hook or a dedicated helper. +- [ ] Preserve contract-scoped enrollment filtering using `b2b_contract_id`. +- [ ] Preserve contract-scoped run filtering using `contract_id` query params and `contractId` run resolution. +- [ ] Preserve current language picker behavior and selected-language state. +- [ ] Use `dashboardAdapters.ts` to keep rendering through `DashboardCard` in this phase. +- [ ] Add or update tests proving: + - only contract programs are rendered, + - programs inside collections are not duplicated as standalone programs, + - collections render only when at least one program has valid contract runs, + - selected-language enrollment is preferred over contract next/best run, + - contract A does not display contract B enrollment, + - welcome/header/skeleton behavior is unchanged. +- [ ] Run: + +```bash +yarn test frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx +``` + +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. +- 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. + +If any of these fails, the phase has relocated complexity rather than reduced it. Stop and split before declaring done. + +## Phase 6: Shrink render components + +**Purpose:** Make `EnrollmentDisplay.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/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 `ContractContent.tsx`. +- [ ] Keep UI state that is truly UI-only in components: + - selected language key, + - expand/collapse state, + - local upgrade error state, + - welcome message expansion state. +- [ ] Keep rendering through existing cards. +- [ ] Verify imports no longer include language-resolution helpers directly in render components unless temporarily necessary. +- [ ] Verify render components do not call `selectBestEnrollment` or `selectBestContractEnrollmentForLanguage` directly. +- [ ] Run: + +```bash +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.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 +``` + +Expected: all tests pass. + +## Phase 7: Unify cards via `CoursewareCard` and delete the legacy cards + +**Purpose:** Replace `DashboardCard.tsx` and `ModuleCard.tsx` — two parallel implementations that have drifted — with a single `CoursewareCard` component that dispatches to explicit variant cases. The deliverable includes deletion of the legacy files; this phase is **not** complete until the duplication is gone. + +This is a structural unification, not a redesign. Each variant must reproduce today's rendering exactly. Visible copy, button width, certificate link text, CTA target selection, upgrade banner behavior, context menu items — all preserved. Behavior changes (multi-run UI, button-label convergence, course-level certificate aggregation, "Completed badge + Continue" CTA) remain out of scope and would be follow-up PRs against `CoursewareCard` after this phase ships. + +**Why now and not earlier:** Phases 1–6 give every callsite a stable slot-shaped input. Without that, a unified card would have had to absorb the V2/V3 reconciliation that the slot model now owns. With it, `CoursewareCard`'s job shrinks to "render this slot in this variant." + +**Hypothesised approach** (verify before executing): + +- [ ] Create `frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/CoursewareCard.tsx`. Props use a discriminated union over a `variant` field. Suggested variants — confirm by walking the existing render sites: + - `courseEnrollment` — user is enrolled (today: `DashboardCard` with a `CourseRunEnrollmentV3`-derived run). + - `unenrolledCourse` — user not yet enrolled (today: `DashboardCard` with a V2 course run / product). + - `program` — program enrollment row (today: `DashboardCard` with `program` resource type). + - `moduleRow` — compact program-as-course row (today: `ModuleCard`). + - `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. + 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. +- [ ] Once every callsite uses `CoursewareCard`, delete: + - `DashboardCard.tsx` + - `DashboardCard.test.tsx` + - `ModuleCard.tsx` + - `ModuleCard.test.tsx` +- [ ] Resolve `DashboardCardRoot` import in `OrganizationCards.tsx` — either inline a styled div, re-export the styled root from `CoursewareCard`, or move the styled root into a small shared file. The choice depends on whether `OrganizationCards` still belongs in this restructuring. +- [ ] Port `DashboardDialogs.test.tsx` if it still references the legacy cards; keep the test if it covers behavior that survives, delete it if it covers only the old wrapper. +- [ ] Migrate any remaining helpers from `helpers.ts` and `languageOptions.ts` into `model/dashboardViewModel.ts`. The `EnrollmentStatus` enum (consumed by `EnrollmentStatusIndicator.tsx` and `ProgressBadge.tsx`) is re-exported from viewModel; presentational consumers update their imports. +- [ ] Delete now-orphaned files and helpers: + - `helpers.ts` and its test file — fully absorbed into viewModel by this point. + - `languageOptions.ts` and its test file — fully absorbed into viewModel by this point. + - `selectBestEnrollment` (replaced by Phase 1's display-policy helper, now in viewModel). + - `selectBestContractEnrollmentForLanguage` (replaced by Phase 2's composite resolver). + - Case 1 of `getResolvedRunForSelectedLanguage` (V3-to-V2 enrolled-run synthesis). Cases 2 and 3 remain in viewModel — Case 3 is a permanent workaround for missing per-language unenrolled run metadata in the V2 API. + - The `dashboardAdapters.ts` adapter introduced in Phase 1, if `CoursewareCard` consumes the slot directly. If the adapter is still load-bearing for any variant, narrow it rather than deleting it. +- [ ] Check the LoC delta. Expected order of magnitude: net negative on the order of ~1500–2000 lines once both legacy cards and their tests are deleted. If the delta is much smaller, investigate before declaring the phase done — the duplication may not have actually been removed. + +**Migration safety:** + +- This phase can be split across multiple PRs along the callsite-migration boundary. PR 7a builds `CoursewareCard` and migrates home + program-as-course. PR 7b migrates program dashboard. PR 7c migrates contract dashboard. PR 7d does the deletion. Each is independently revertible. +- If a variant turns out to need behavior that's genuinely different from "render today's card with the slot's data" — escalate to product before encoding it. That's the signal that an apparent unification is actually a redesign. + +**Run:** + +```bash +yarn test frontends/main/src/app-pages/DashboardPage/ +yarn workspace frontends run typecheck +``` + +Expected: all tests pass; type checking clean; `DashboardCard.tsx` and `ModuleCard.tsx` no longer exist; line count meaningfully drops. + +## Explicitly deferred product decisions + +Do not answer these in this refactor: + +- how users discover alternate enrolled runs, +- whether alternate runs appear inline, in a dropdown, in a dialog, or in expanded mode, +- how to label alternate runs, +- whether language selection and run selection are one control or two controls, +- whether status/certificate display is course-level or displayed-run-level, +- whether CTA should be annotated with a run reference, +- whether the default visible run should prefer active work, certificate completion, newest run, or some other policy, +- whether program and contract dashboards should expose identical multi-run affordances. + +## Validation plan + +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/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 +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.test.tsx +``` + +For Phase 7 (and any earlier phase that touches the legacy cards), also run: + +```bash +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx # until deleted in Phase 7 +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ModuleCard.test.tsx # until deleted in Phase 7 +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/CoursewareCard.test.tsx # added in Phase 7 +yarn test frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx +``` + +Run type checking before final review: + +```bash +yarn workspace frontends run typecheck +``` + +## Implementation notes + +- Keep all changes in frontend files; OpenAPI regeneration is not expected. +- Use `setMockResponse` and factories from `api/test-utils` / `api/mitxonline-test-utils` in tests. +- Prefer role/text queries through `renderWithProviders`. +- Keep `import React from "react"` in frontend tests where existing tests use it. +- Use root-relative `@/` imports within `frontends/main`. +- Avoid broad behavior changes in the same PR as structural extraction. +- If a phase requires an intentional user-visible change, split that into its own PR with tests and product acknowledgement. + +## Recommended PR sequence + +1. Pure model and adapter helpers. +2. Composite slot/language resolver + language-union bug fix. +3. Home data hook extraction. +4. Program dashboard data hook extraction. +5. Contract dashboard data hook extraction. +6. Render component simplification. +7. `CoursewareCard` unification + legacy card deletion (may split into 7a–7d along callsite-migration boundaries). + +After Phase 6 the data architecture is sound; after Phase 7 the duplication is gone. Phase 7 is committed scope, not optional. Behavior changes that depend on multi-run UX direction (button-label convergence, multi-run affordances, course-level certificate aggregation) remain explicitly deferred to follow-up PRs against `CoursewareCard`. + +## File structure: before vs. after + +Diff-style listing of the dashboard-area files this plan touches. Page-level files outside scope (`DashboardLayout.tsx`, `HomeContent.tsx`, `ProgramContent.tsx`, `SettingsContent.tsx`, `ProfileContent.tsx`, `UserListDetailsContent.tsx`, `OrganizationRedirect.tsx`, `EnrollmentRedirectAlert.tsx`) are unchanged and omitted. + +Markers: `+` new, `-` deleted, `~` modified, blank = unchanged. + +Test files (`*.test.tsx`, `*.test.ts`) are omitted for brevity. Each non-test source file has a corresponding test that is added/modified/deleted alongside it. + +```text + frontends/main/src/app-pages/DashboardPage/ +~ ContractContent.tsx shrinks (Phase 5–6) + + CoursewareDisplay/ +- 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) +~ ProgramAsCourseCard.tsx uses CoursewareCard moduleRow (Phase 7) +~ OrganizationCards.tsx decouples DashboardCardRoot (Phase 7) +~ DashboardDialogs.tsx may shift if dialog wiring moves (Phase 7) + EnrollmentStatusIndicator.tsx unchanged (consumed by CoursewareCard) + ProgressBadge.tsx unchanged + receiptMenuItem.ts unchanged +~ test-utils.ts likely gains slot/variant helpers +- helpers.ts deleted (Phase 7) — absorbed into viewModel +- languageOptions.ts deleted (Phase 7) — absorbed into viewModel + — Cases 2, 3 of getResolvedRunForSelectedLanguage migrate + — Case 1 (V3→V2 synthesis) deleted + — selectBestContractEnrollmentForLanguage deleted + ++ model/ new directory (Phase 1, grows through Phase 7) ++ dashboardViewModel.ts single pure-model home: types, grouping helpers, + slot constructors, requirement-section builders, + composite resolver (Phase 2), display policy, + 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 +``` + +**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. +- 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/helpers.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts index 1e26fe98af..f84afb6da1 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts @@ -5,6 +5,7 @@ import { V3UserProgramEnrollment, } from "@mitodl/mitxonline-api-axios/v2" import { getBestRun } from "@/common/mitxonline" +import { pickDisplayedEnrollmentForLegacyDashboard } from "./model/dashboardViewModel" export { getBestRun } const ResourceType = { @@ -51,25 +52,7 @@ const selectBestEnrollment = ( course: CourseWithCourseRunsSerializerV2, enrollments: CourseRunEnrollmentV3[], ): CourseRunEnrollmentV3 | null => { - const courseEnrollments = enrollments.filter((enrollment) => - course.courseruns.some((run) => run.id === enrollment.run.id), - ) - if (courseEnrollments.length === 0) { - return null - } - return courseEnrollments.reduce((best, current) => { - const bestHasCert = !!best.certificate?.uuid - const currentHasCert = !!current.certificate?.uuid - - // Prioritize having a certificate - if (currentHasCert && !bestHasCert) return current - if (bestHasCert && !currentHasCert) return best - - // If both have or don't have certificates, compare grades - const bestGrade = Math.max(0, ...best.grades.map((g) => g.grade ?? 0)) - const currentGrade = Math.max(0, ...current.grades.map((g) => g.grade ?? 0)) - return currentGrade > bestGrade ? current : best - }, courseEnrollments[0]) + return pickDisplayedEnrollmentForLegacyDashboard(course, enrollments) } const getEnrollmentStatus = ( diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.test.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.test.ts new file mode 100644 index 0000000000..67453aba94 --- /dev/null +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.test.ts @@ -0,0 +1,191 @@ +import { factories } from "api/mitxonline-test-utils" +import { + adaptCourseSlotToLegacyDashboardCardProps, + type LegacyDashboardCardAdapterOutput, +} from "./dashboardAdapters" +import { + pickDisplayedEnrollmentForLegacyDashboard, + type DashboardCourseSlot, +} from "./dashboardViewModel" + +const makeSlot = ( + overrides: Partial, +): DashboardCourseSlot => { + const defaultRun = factories.courses.courseRun({ + id: 1, + courseware_url: "/courseware/default-run/", + }) + const defaultCourse = factories.courses.course({ + id: 10, + courseruns: [defaultRun], + }) + + return { + course: defaultCourse, + enrollments: [], + selectedLanguageKey: "", + availableLanguages: [], + displayedEnrollment: null, + displayedRun: defaultRun, + ...overrides, + } +} + +const expectEnrollmentResource = ( + adapted: LegacyDashboardCardAdapterOutput, + runId: number, +) => { + expect(adapted.resource.type).toBe("courserun-enrollment") + if (adapted.resource.type !== "courserun-enrollment") { + throw new Error("expected courserun-enrollment resource") + } + expect(adapted.resource.data.run.id).toBe(runId) +} + +describe("dashboardAdapters", () => { + test("no enrollment: adapts to course resource", () => { + const slot = makeSlot({ displayedEnrollment: null }) + + const adapted = adaptCourseSlotToLegacyDashboardCardProps(slot) + + expect(adapted.resource.type).toBe("course") + expect(adapted.selectedCourseRun?.id).toBe(slot.displayedRun?.id) + expect(adapted.buttonHref).toBe(slot.displayedRun?.courseware_url) + }) + + test("one enrollment: adapts to enrollment resource", () => { + const run = factories.courses.courseRun({ + id: 2, + courseware_url: "/courseware/enrolled/", + }) + const course = factories.courses.course({ id: 20, courseruns: [run] }) + const enrollment = factories.enrollment.courseEnrollment({ run }) + + const slot = makeSlot({ + course, + enrollments: [enrollment], + displayedEnrollment: enrollment, + displayedRun: run, + }) + const adapted = adaptCourseSlotToLegacyDashboardCardProps(slot) + + expectEnrollmentResource(adapted, run.id) + expect(adapted.buttonHref).toBe(run.courseware_url) + }) + + test("multiple enrollments with certificate: adapter uses policy-selected enrollment", () => { + const run = factories.courses.courseRun({ id: 3 }) + const course = factories.courses.course({ id: 30, courseruns: [run] }) + const noCertificate = factories.enrollment.courseEnrollment({ + run: { id: 3 }, + certificate: null, + }) + const withCertificate = factories.enrollment.courseEnrollment({ + run: { id: 3 }, + certificate: { uuid: "cert-3" }, + }) + + const displayedEnrollment = pickDisplayedEnrollmentForLegacyDashboard( + course, + [noCertificate, withCertificate], + ) + const slot = makeSlot({ + course, + enrollments: [noCertificate, withCertificate], + displayedEnrollment, + displayedRun: run, + }) + const adapted = adaptCourseSlotToLegacyDashboardCardProps(slot) + + expectEnrollmentResource(adapted, withCertificate.run.id) + }) + + test("multiple enrollments highest grade: adapter uses policy-selected enrollment", () => { + const run = factories.courses.courseRun({ id: 4 }) + const course = factories.courses.course({ id: 40, courseruns: [run] }) + const lowGrade = factories.enrollment.courseEnrollment({ + run: { id: 4 }, + certificate: null, + grades: [factories.enrollment.grade({ grade: 0.5 })], + }) + const highGrade = factories.enrollment.courseEnrollment({ + run: { id: 4 }, + certificate: null, + grades: [factories.enrollment.grade({ grade: 0.95 })], + }) + + const displayedEnrollment = pickDisplayedEnrollmentForLegacyDashboard( + course, + [lowGrade, highGrade], + ) + const slot = makeSlot({ + course, + enrollments: [lowGrade, highGrade], + displayedEnrollment, + displayedRun: run, + }) + const adapted = adaptCourseSlotToLegacyDashboardCardProps(slot) + + expectEnrollmentResource(adapted, highGrade.run.id) + }) + + test("selected-language enrollment: adapter prefers selected enrollment", () => { + const enRun = factories.courses.courseRun({ + id: 5, + language: "en", + courseware_url: "/courseware/en/", + }) + const esRun = factories.courses.courseRun({ + id: 6, + language: "es", + courseware_url: "/courseware/es/", + }) + const course = factories.courses.course({ + id: 50, + courseruns: [enRun, esRun], + }) + const selectedLanguageEnrollment = factories.enrollment.courseEnrollment({ + run: esRun, + }) + + const slot = makeSlot({ + course, + enrollments: [selectedLanguageEnrollment], + selectedLanguageKey: "language:es", + displayedEnrollment: selectedLanguageEnrollment, + displayedRun: esRun, + }) + const adapted = adaptCourseSlotToLegacyDashboardCardProps(slot) + + expectEnrollmentResource(adapted, esRun.id) + expect(adapted.buttonHref).toBe(esRun.courseware_url) + }) + + test("contract-scoped selected-language enrollment: passes contract/program context", () => { + const run = factories.courses.courseRun({ + id: 7, + courseware_url: "/courseware/contract-es/", + }) + const course = factories.courses.course({ id: 60, courseruns: [run] }) + const contractEnrollment = factories.enrollment.courseEnrollment({ + run, + b2b_contract_id: 999, + }) + const programEnrollment = factories.enrollment.programEnrollmentV3() + + const slot = makeSlot({ + course, + enrollments: [contractEnrollment], + selectedLanguageKey: "language:es", + displayedEnrollment: contractEnrollment, + displayedRun: run, + contractId: 999, + ancestorContext: { programEnrollment }, + }) + const adapted = adaptCourseSlotToLegacyDashboardCardProps(slot) + + expectEnrollmentResource(adapted, run.id) + expect(adapted.contractId).toBe(999) + expect(adapted.programEnrollment).toEqual(programEnrollment) + }) +}) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.ts new file mode 100644 index 0000000000..242ba17880 --- /dev/null +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardAdapters.ts @@ -0,0 +1,44 @@ +import type { + CourseRunEnrollmentV3, + CourseRunV2, + CourseWithCourseRunsSerializerV2, + V3UserProgramEnrollment, +} from "@mitodl/mitxonline-api-axios/v2" +import type { DashboardCourseSlot } from "./dashboardViewModel" + +export type LegacyDashboardCardResource = + | { type: "course"; data: CourseWithCourseRunsSerializerV2 } + | { type: "courserun-enrollment"; data: CourseRunEnrollmentV3 } + +export type LegacyDashboardCardAdapterOutput = { + resource: LegacyDashboardCardResource + selectedCourseRun: CourseRunV2 | null + buttonHref: string | null + contractId?: number + programEnrollment?: V3UserProgramEnrollment +} + +const adaptCourseSlotToLegacyDashboardCardProps = ( + slot: DashboardCourseSlot, +): LegacyDashboardCardAdapterOutput => { + return { + resource: slot.displayedEnrollment + ? { + type: "courserun-enrollment", + data: slot.displayedEnrollment, + } + : { + type: "course", + data: slot.course, + }, + selectedCourseRun: slot.displayedRun, + buttonHref: + slot.displayedEnrollment?.run.courseware_url ?? + slot.displayedRun?.courseware_url ?? + null, + contractId: slot.contractId, + programEnrollment: slot.ancestorContext?.programEnrollment, + } +} + +export { adaptCourseSlotToLegacyDashboardCardProps } 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 new file mode 100644 index 0000000000..aaf1bd9313 --- /dev/null +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.test.ts @@ -0,0 +1,109 @@ +import { factories } from "api/mitxonline-test-utils" +import { + groupCourseRunEnrollmentsByCourseId, + groupProgramEnrollmentsByProgramId, + pickDisplayedEnrollmentForLegacyDashboard, +} from "./dashboardViewModel" + +describe("dashboardViewModel", () => { + describe("pickDisplayedEnrollmentForLegacyDashboard", () => { + test("returns null when there are no matching course enrollments", () => { + const course = factories.courses.course({ + courseruns: [factories.courses.courseRun({ id: 1 })], + }) + const enrollments = [ + factories.enrollment.courseEnrollment({ run: { id: 999 } }), + ] + + expect( + pickDisplayedEnrollmentForLegacyDashboard(course, enrollments), + ).toBeNull() + }) + + test("prefers enrollment with certificate over one without", () => { + const run = factories.courses.courseRun({ id: 1 }) + const course = factories.courses.course({ courseruns: [run] }) + const noCertificate = factories.enrollment.courseEnrollment({ + run: { id: 1 }, + certificate: null, + }) + const withCertificate = factories.enrollment.courseEnrollment({ + run: { id: 1 }, + certificate: { uuid: "cert-123" }, + }) + + expect( + pickDisplayedEnrollmentForLegacyDashboard(course, [ + noCertificate, + withCertificate, + ]), + ).toEqual(withCertificate) + }) + + test("prefers highest grade when certificate state is tied", () => { + const run = factories.courses.courseRun({ id: 1 }) + const course = factories.courses.course({ courseruns: [run] }) + const lower = factories.enrollment.courseEnrollment({ + run: { id: 1 }, + certificate: null, + grades: [factories.enrollment.grade({ grade: 0.6 })], + }) + const higher = factories.enrollment.courseEnrollment({ + run: { id: 1 }, + certificate: null, + grades: [factories.enrollment.grade({ grade: 0.9 })], + }) + + expect( + pickDisplayedEnrollmentForLegacyDashboard(course, [lower, higher]), + ).toEqual(higher) + }) + }) + + 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 }, + }) + const enrollment2 = factories.enrollment.courseEnrollment({ + run: { id: 22 }, + }) + const unknownRunEnrollment = factories.enrollment.courseEnrollment({ + run: { id: 999 }, + }) + + const grouped = groupCourseRunEnrollmentsByCourseId( + [course1, course2], + [enrollment1, enrollment2, unknownRunEnrollment], + ) + + expect(grouped[1]).toEqual([enrollment1]) + expect(grouped[2]).toEqual([enrollment2]) + expect(grouped[999]).toBeUndefined() + }) + }) + + describe("groupProgramEnrollmentsByProgramId", () => { + test("creates a map keyed by program id", () => { + const enrollment1 = factories.enrollment.programEnrollmentV3({ + program: factories.programs.program({ id: 101 }), + }) + const enrollment2 = factories.enrollment.programEnrollmentV3({ + program: factories.programs.program({ id: 202 }), + }) + + const grouped = groupProgramEnrollmentsByProgramId([ + enrollment1, + enrollment2, + ]) + + expect(grouped[101]).toEqual(enrollment1) + expect(grouped[202]).toEqual(enrollment2) + }) + }) +}) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts new file mode 100644 index 0000000000..2383f26c25 --- /dev/null +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/model/dashboardViewModel.ts @@ -0,0 +1,123 @@ +/** + * Pure-model layer for the dashboard. + * + * Owns the canonical types (e.g. DashboardCourseSlot) and the pure transforms + * — grouping, slot construction, display policy — that data hooks compose into + * render-ready shapes. No React, no queries; everything is synchronous and + * unit-testable in isolation. + */ +import type { SimpleSelectOption } from "ol-components" +import type { + CourseRunEnrollmentV3, + CourseRunV2, + CourseWithCourseRunsSerializerV2, + V3UserProgramEnrollment, +} from "@mitodl/mitxonline-api-axios/v2" + +/** + * A program/contract dashboard's view of a course: every enrollment whose run + * belongs to this course, plus a derived display choice for the legacy + * single-enrollment card UI. + * + * `enrollments` must be course-matched (by run id) with no language filter — + * language is a display selection (`selectedLanguageKey` → `displayedEnrollment` + * / `displayedRun`), not a filter on the underlying list. Contract scoping, + * when applicable, must be applied by the slot constructor before the list + * reaches the slot. + */ +export type DashboardCourseSlot = { + course: CourseWithCourseRunsSerializerV2 + enrollments: CourseRunEnrollmentV3[] + selectedLanguageKey: string + availableLanguages: SimpleSelectOption[] + contractId?: number + isContractPageResource?: boolean + ancestorContext?: { + programEnrollment?: V3UserProgramEnrollment + parentProgramReadableIds?: string[] + useVerifiedEnrollment?: boolean + } + // Whether these fields survive past the legacy-card removal is an open tied + // to new card UX (run selection controlled by card or parent.) + displayedEnrollment: CourseRunEnrollmentV3 | null + displayedRun: CourseRunV2 | null +} + +const getMaxEnrollmentGrade = (enrollment: CourseRunEnrollmentV3): number => { + return Math.max(0, ...enrollment.grades.map((grade) => grade.grade ?? 0)) +} + +/** + * Legacy display policy used by dashboard cards. + * + * Priority: + * 1. Prefer enrollment with a certificate + * 2. If tied, prefer highest grade + * 3. Otherwise take first match + */ +const pickDisplayedEnrollmentForLegacyDashboard = ( + course: CourseWithCourseRunsSerializerV2, + enrollments: CourseRunEnrollmentV3[], +): CourseRunEnrollmentV3 | null => { + const courseEnrollments = enrollments.filter((enrollment) => + course.courseruns.some((run) => run.id === enrollment.run.id), + ) + if (courseEnrollments.length === 0) { + return null + } + + return courseEnrollments.reduce((best, current) => { + const bestHasCert = !!best.certificate?.uuid + const currentHasCert = !!current.certificate?.uuid + + if (currentHasCert && !bestHasCert) return current + if (bestHasCert && !currentHasCert) return best + + const bestGrade = getMaxEnrollmentGrade(best) + const currentGrade = getMaxEnrollmentGrade(current) + return currentGrade > bestGrade ? current : best + }, courseEnrollments[0]) +} + +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 + } + + acc[courseId] = [...(acc[courseId] ?? []), enrollment] + return acc + }, + {}, + ) +} + +const groupProgramEnrollmentsByProgramId = ( + programEnrollments: V3UserProgramEnrollment[], +): Record => { + return programEnrollments.reduce>( + (acc, enrollment) => { + acc[enrollment.program.id] = enrollment + return acc + }, + {}, + ) +} + +export { + pickDisplayedEnrollmentForLegacyDashboard, + groupCourseRunEnrollmentsByCourseId, + groupProgramEnrollmentsByProgramId, +} From 587a45bcdd851d43e22d639db9e6364b5cd5e3fe Mon Sep 17 00:00:00 2001 From: Zaman Afzal Date: Mon, 11 May 2026 21:17:33 +0500 Subject: [PATCH 2/4] feat: added welcome to learner email once he login for the first time (#3324) * feat: added welcome to learner email once he login for the first time --- authentication/api.py | 11 +------ authentication/api_test.py | 29 ------------------- authentication/views.py | 2 ++ authentication/views_test.py | 20 +++++++++++++ main/middleware/apisix_user_test.py | 45 ----------------------------- main/settings_pluggy.py | 2 +- profiles/plugins.py | 16 ---------- profiles/plugins_test.py | 17 ----------- 8 files changed, 24 insertions(+), 118 deletions(-) delete mode 100644 profiles/plugins_test.py diff --git a/authentication/api.py b/authentication/api.py index a8f73ea18f..396eb0dd8f 100644 --- a/authentication/api.py +++ b/authentication/api.py @@ -33,17 +33,8 @@ def create_user(username, email, profile_data=None, user_extra=None): defaults.update({"username": username}) with transaction.atomic(): - user, created = User.objects.get_or_create(email=email, defaults=defaults) - + user, _ = User.objects.get_or_create(email=email, defaults=defaults) profile_api.ensure_profile(user, profile_data=profile_data) - if created: - transaction.on_commit( - lambda: user_created_actions( - user=user, - is_new=True, - details=profile_data or {}, - ) - ) return user diff --git a/authentication/api_test.py b/authentication/api_test.py index 29ded90065..aef0f9fafe 100644 --- a/authentication/api_test.py +++ b/authentication/api_test.py @@ -37,35 +37,6 @@ def test_create_user(profile_data): assert user.profile.name is None -@pytest.mark.django_db(transaction=True) -def test_create_user_triggers_plugins_for_new_users(mocker): - """create_user should trigger user_created plugins for brand new users.""" - mock_pm = mocker.Mock() - mocker.patch("authentication.api.get_plugin_manager", return_value=mock_pm) - - user = api.create_user("new-user", "new@localhost", {"name": "New User"}) - - mock_pm.hook.user_created.assert_called_once_with( - user=user, - user_data={"profile": {"name": "New User"}}, - ) - - -@pytest.mark.django_db(transaction=True) -def test_create_user_does_not_retrigger_plugins_for_existing_users(mocker): - """create_user should not trigger user_created plugins for existing users.""" - user = UserFactory.create(email="existing@localhost") - mock_pm = mocker.Mock() - mocker.patch("authentication.api.get_plugin_manager", return_value=mock_pm) - - resolved = api.create_user( - "another-username", user.email, {"name": "Existing User"} - ) - - assert resolved.id == user.id - mock_pm.hook.user_created.assert_not_called() - - @pytest.mark.parametrize( "mock_method", ["profiles.api.ensure_profile"], diff --git a/authentication/views.py b/authentication/views.py index fb68e9c78c..ccd3b34015 100644 --- a/authentication/views.py +++ b/authentication/views.py @@ -9,6 +9,7 @@ from django.views import View from main.middleware.apisix_user import ApisixUserMiddleware, decode_apisix_headers +from profiles.tasks import send_welcome_email log = logging.getLogger(__name__) @@ -100,6 +101,7 @@ def get( redirect_url = f"{settings.MITOL_NEW_USER_LOGIN_URL}?{params}" profile.save() + send_welcome_email.delay(request.user.id) profile.has_logged_in = True profile.save() diff --git a/authentication/views_test.py b/authentication/views_test.py index 8ed8545d4f..92401d9aef 100644 --- a/authentication/views_test.py +++ b/authentication/views_test.py @@ -179,6 +179,9 @@ def test_custom_login_view_authenticated_user_needs_onboarding( "authentication.views.settings.MITOL_NEW_USER_LOGIN_URL", "/onboarding" ) mocker.patch("authentication.views.decode_apisix_headers", return_value={}) + mock_send_welcome_email = mocker.patch( + "authentication.views.send_welcome_email.delay" + ) response = CustomLoginView().get(request) @@ -188,6 +191,7 @@ def test_custom_login_view_authenticated_user_needs_onboarding( assert response.url == f"/onboarding?{urlencode({'next': expected_redirect})}" else: assert response.url == expected_redirect + mock_send_welcome_email.assert_called_once_with(request.user.id) def test_custom_login_view_authenticated_user_who_has_logged_in_before(mocker): @@ -199,11 +203,15 @@ def test_custom_login_view_authenticated_user_who_has_logged_in_before(mocker): ) request.user = MagicMock(is_anonymous=False) request.user.profile = MagicMock(has_logged_in=True) + mock_send_welcome_email = mocker.patch( + "authentication.views.send_welcome_email.delay" + ) response = CustomLoginView().get(request) assert response.status_code == 302 assert response.url == "/should-be-redirect?foo" + mock_send_welcome_email.assert_not_called() def test_custom_login_view_anonymous_user(mocker): @@ -231,9 +239,13 @@ def test_custom_login_view_first_time_login_sets_has_logged_in(mocker): mock_user = MagicMock() mock_user.is_anonymous = False + mock_user.id = 123 mock_user.profile = mock_profile request.user = mock_user + mock_send_welcome_email = mocker.patch( + "authentication.views.send_welcome_email.delay" + ) response = CustomLoginView().get(request) @@ -243,6 +255,7 @@ def test_custom_login_view_first_time_login_sets_has_logged_in(mocker): # Verify that has_logged_in was set to True and profile was saved assert mock_profile.has_logged_in is True mock_profile.save.assert_called_once() + mock_send_welcome_email.assert_called_once_with(mock_user.id) class LoginOrgUserRedirectParams(NamedTuple): @@ -286,6 +299,9 @@ def test_login_org_user_redirect( # Set up user profile based on test scenario user.profile.has_logged_in = has_logged_in user.profile.save() + mock_send_welcome_email = mocker.patch( + "authentication.views.send_welcome_email.delay" + ) header_str = b64encode( json.dumps( @@ -319,3 +335,7 @@ def test_login_org_user_redirect( user.profile.refresh_from_db() assert user.profile.has_logged_in is True + if has_logged_in: + mock_send_welcome_email.assert_not_called() + else: + mock_send_welcome_email.assert_called_once_with(user.id) diff --git a/main/middleware/apisix_user_test.py b/main/middleware/apisix_user_test.py index aed867a296..50cdbdc31c 100644 --- a/main/middleware/apisix_user_test.py +++ b/main/middleware/apisix_user_test.py @@ -84,51 +84,6 @@ def test_get_request_no_posthog_key(mocker, mock_login, settings): mock_posthog_cls.assert_not_called() -@pytest.mark.django_db(transaction=True) -def test_get_request_queues_welcome_email_for_new_sso_user(mocker, mock_login): - """New APISIX users should queue welcome email once.""" - close_old_connections() - mock_delay = mocker.patch("profiles.plugins.send_welcome_email.delay") - mock_request = mocker.Mock( - META={ - "HTTP_X_USERINFO": b64encode(json.dumps(apisix_user_info).encode()), - }, - user=AnonymousUser(), - ) - apisix_middleware = ApisixUserMiddleware(mocker.Mock()) - apisix_middleware.process_request(mock_request) - - user = User.objects.get(email=apisix_user_info["email"]) - mock_login.assert_called_once() - mock_delay.assert_called_once_with(user.id) - - -@pytest.mark.django_db(transaction=True) -def test_get_request_does_not_queue_welcome_email_for_existing_sso_user( - mocker, mock_login -): - """Existing APISIX users should not queue welcome email again.""" - close_old_connections() - existing_user = UserFactory.create( - email=apisix_user_info["email"], - username=apisix_user_info["preferred_username"], - global_id=apisix_user_info["sub"], - ) - mock_delay = mocker.patch("profiles.plugins.send_welcome_email.delay") - mock_request = mocker.Mock( - META={ - "HTTP_X_USERINFO": b64encode(json.dumps(apisix_user_info).encode()), - }, - user=AnonymousUser(), - ) - apisix_middleware = ApisixUserMiddleware(mocker.Mock()) - apisix_middleware.process_request(mock_request) - - existing_user.refresh_from_db() - mock_login.assert_called_once() - mock_delay.assert_not_called() - - @pytest.mark.django_db(transaction=True) def test_get_request_existing_user_no_globalid(mocker, mock_login): """Test that a valid request updates existing user with same email, no global_id.""" diff --git a/main/settings_pluggy.py b/main/settings_pluggy.py index 9b121b402d..da6aad907f 100644 --- a/main/settings_pluggy.py +++ b/main/settings_pluggy.py @@ -2,7 +2,7 @@ MITOL_AUTHENTICATION_PLUGINS = get_string( "MITOL_AUTHENTICATION_PLUGINS", - "learning_resources.plugins.FavoritesListPlugin,profiles.plugins.CreateProfilePlugin,profiles.plugins.WelcomeEmailPlugin", + "learning_resources.plugins.FavoritesListPlugin,profiles.plugins.CreateProfilePlugin", ) MITOL_LEARNING_RESOURCES_PLUGINS = get_string( "MITOL_LEARNING_RESOURCES_PLUGINS", diff --git a/profiles/plugins.py b/profiles/plugins.py index 9286f3e28a..7faccab4e3 100644 --- a/profiles/plugins.py +++ b/profiles/plugins.py @@ -3,7 +3,6 @@ from django.apps import apps from profiles.api import ensure_profile -from profiles.tasks import send_welcome_email class CreateProfilePlugin: @@ -20,18 +19,3 @@ def user_created(self, user, user_data): """ profile_data = user_data.get("profile", {}) ensure_profile(user, profile_data) - - -class WelcomeEmailPlugin: - hookimpl = apps.get_app_config("authentication").hookimpl - - @hookimpl - def user_created(self, user, user_data): # noqa: ARG002 - """ - Send a welcome email when a new user account is created. - - Args: - user(User): the user that was created - user_data(dict): the user data - """ - send_welcome_email.delay(user.id) diff --git a/profiles/plugins_test.py b/profiles/plugins_test.py deleted file mode 100644 index 99a9171652..0000000000 --- a/profiles/plugins_test.py +++ /dev/null @@ -1,17 +0,0 @@ -"""Tests for profiles plugins.""" - -import pytest - -from main.factories import UserFactory -from profiles.plugins import WelcomeEmailPlugin - - -@pytest.mark.django_db -def test_welcome_email_plugin_user_created(mocker): - """WelcomeEmailPlugin should queue welcome email task on user creation.""" - user = UserFactory.create(email="new.user@example.com", first_name="New") - mocked_delay = mocker.patch("profiles.plugins.send_welcome_email.delay") - - WelcomeEmailPlugin().user_created(user, user_data={}) - - mocked_delay.assert_called_once_with(user.id) From 62a2ef171d194a6473456fdc873fd41e1f3010db Mon Sep 17 00:00:00 2001 From: Danielle Frappier Date: Mon, 11 May 2026 12:30:22 -0400 Subject: [PATCH 3/4] fix: 404 on product pages when live=false (#3295) * fix: 404 on product pages when live=false * fix: prevent metadata generation for non-live product pages --- .../ProductPages/CoursePage.test.tsx | 21 ++++++++++++-- .../src/app-pages/ProductPages/CoursePage.tsx | 2 +- .../ProductPages/ProgramAsCoursePage.test.tsx | 29 +++++++++++++++++-- .../ProductPages/ProgramAsCoursePage.tsx | 2 +- .../ProductPages/ProgramPage.test.tsx | 27 +++++++++++++++-- .../app-pages/ProductPages/ProgramPage.tsx | 2 +- .../(products)/courses/[readable_id]/page.tsx | 21 ++++++++------ .../courses/p/[readable_id]/page.tsx | 21 ++++++++------ .../programs/[readable_id]/page.tsx | 21 ++++++++------ frontends/main/src/common/metadata.ts | 12 +++++++- 10 files changed, 121 insertions(+), 37 deletions(-) diff --git a/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx b/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx index eb9007dbad..97bd76d09f 100644 --- a/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx @@ -43,7 +43,7 @@ const setupApis = ({ page: CoursePageItem }) => { setMockResponse.get( - urls.courses.coursesList({ readable_id: course.readable_id }), + urls.courses.coursesList({ readable_id: course.readable_id, live: true }), { results: [course] }, ) @@ -352,7 +352,7 @@ describe("CoursePage", () => { { courses: [], pages: [] }, ])("Returns 404 if no course found", async ({ courses, pages }) => { setMockResponse.get( - urls.courses.coursesList({ readable_id: "readable_id" }), + urls.courses.coursesList({ readable_id: "readable_id", live: true }), { results: courses }, ) if (courses.length > 0) { @@ -375,6 +375,23 @@ describe("CoursePage", () => { }) }) + test("Returns 404 if course has live=false", async () => { + const course = makeCourse() + const page = makePage({ course_details: course }) + // Simulate live=false: the API filters it out, returning empty results + setMockResponse.get( + urls.courses.coursesList({ readable_id: course.readable_id, live: true }), + { results: [] }, + ) + setMockResponse.get(urls.pages.coursePages(course.readable_id), { + items: [page], + }) + renderWithProviders() + await waitFor(() => { + expect(notFound).toHaveBeenCalled() + }) + }) + describe("Stay Updated button", () => { useStayUpdatedEnv() diff --git a/frontends/main/src/app-pages/ProductPages/CoursePage.tsx b/frontends/main/src/app-pages/ProductPages/CoursePage.tsx index 8b6f531f58..b011bfe24d 100644 --- a/frontends/main/src/app-pages/ProductPages/CoursePage.tsx +++ b/frontends/main/src/app-pages/ProductPages/CoursePage.tsx @@ -43,7 +43,7 @@ const PrerequisitesSection = styled.section({ const CoursePage: React.FC = ({ readableId }) => { const pages = useQuery(pagesQueries.coursePages(readableId)) const courses = useQuery( - coursesQueries.coursesList({ readable_id: readableId }), + coursesQueries.coursesList({ readable_id: readableId, live: true }), ) const page = pages.data?.items[0] const course = courses.data?.results?.[0] diff --git a/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.test.tsx b/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.test.tsx index 188754fba3..d1de6f3ea7 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.test.tsx @@ -53,7 +53,10 @@ const setupApis = ({ childPrograms: V2ProgramDetail[] } => { setMockResponse.get( - urls.programs.programsList({ readable_id: program.readable_id }), + urls.programs.programsList({ + readable_id: program.readable_id, + live: true, + }), { results: [program] }, ) setMockResponse.get(urls.pages.programPages(program.readable_id), { @@ -163,7 +166,7 @@ describe("ProgramAsCoursePage", () => { test("Returns 404 if no program found", async () => { setMockResponse.get( - urls.programs.programsList({ readable_id: "readable_id" }), + urls.programs.programsList({ readable_id: "readable_id", live: true }), { results: [] }, ) setMockResponse.get(urls.pages.programPages("readable_id"), { @@ -175,6 +178,28 @@ describe("ProgramAsCoursePage", () => { }) }) + test("Returns 404 if program has live=false", async () => { + const program = makeProgramAsCourse() + const page = makePage({ program_details: program }) + // Simulate live=false: the API filters it out, returning empty results + setMockResponse.get( + urls.programs.programsList({ + readable_id: program.readable_id, + live: true, + }), + { results: [] }, + ) + setMockResponse.get(urls.pages.programPages(program.readable_id), { + items: [page], + }) + renderWithProviders( + , + ) + await waitFor(() => { + expect(notFound).toHaveBeenCalled() + }) + }) + test("Renders Modules section with course titles (single root)", async () => { const reqTree = new RequirementTreeBuilder() const op = reqTree.addOperator({ operator: "all_of", title: "All Modules" }) diff --git a/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx b/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx index 4bc11cd387..b7080c0f2f 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx @@ -215,7 +215,7 @@ const ProgramAsCoursePage: React.FC = ({ }) => { const pages = useQuery(pagesQueries.programPages(readableId)) const programs = useQuery( - programsQueries.programsList({ readable_id: readableId }), + programsQueries.programsList({ readable_id: readableId, live: true }), ) const page = pages.data?.items[0] diff --git a/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx b/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx index c05a23a0c5..596a6a402a 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx @@ -162,7 +162,10 @@ const setupApis = ({ childPrograms: V2ProgramDetail[] } => { setMockResponse.get( - urls.programs.programsList({ readable_id: program.readable_id }), + urls.programs.programsList({ + readable_id: program.readable_id, + live: true, + }), { results: [program] }, ) @@ -639,7 +642,7 @@ describe("ProgramPage", () => { }, ])("Returns 404 if no program found", async ({ programs, pages }) => { setMockResponse.get( - urls.programs.programsList({ readable_id: "readable_id" }), + urls.programs.programsList({ readable_id: "readable_id", live: true }), { results: programs }, ) setMockResponse.get(urls.pages.programPages("readable_id"), { @@ -652,6 +655,26 @@ describe("ProgramPage", () => { }) }) + test("Returns 404 if program has live=false", async () => { + const program = makeProgram({ ...makeReqs() }) + const page = makePage({ program_details: program }) + // Simulate live=false: the API filters it out, returning empty results + setMockResponse.get( + urls.programs.programsList({ + readable_id: program.readable_id, + live: true, + }), + { results: [] }, + ) + setMockResponse.get(urls.pages.programPages(program.readable_id), { + items: [page], + }) + renderWithProviders() + await waitFor(() => { + expect(notFound).toHaveBeenCalled() + }) + }) + test("clicking a requirement card fires course_card_clicked", async () => { allowConsoleErrors() const program = makeProgram({ diff --git a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx index 946c9e4c4f..a5133db254 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx @@ -220,7 +220,7 @@ const RequirementsSection: React.FC = ({ const ProgramPage: React.FC = ({ readableId }) => { const pages = useQuery(pagesQueries.programPages(readableId)) const programs = useQuery( - programsQueries.programsList({ readable_id: readableId }), + programsQueries.programsList({ readable_id: readableId, live: true }), ) const page = pages.data?.items[0] diff --git a/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx b/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx index d61064a8b4..293d8b284c 100644 --- a/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx +++ b/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx @@ -1,6 +1,10 @@ import React from "react" import { HydrationBoundary, dehydrate } from "@tanstack/react-query" -import { safeGenerateMetadata, standardizeMetadata } from "@/common/metadata" +import { + safeGenerateMetadata, + standardizeMetadata, + MetadataNotFound, +} from "@/common/metadata" import CoursePage from "@/app-pages/ProductPages/CoursePage" import { notFound } from "next/navigation" import { pagesQueries } from "api/mitxonline-hooks/pages" @@ -18,17 +22,16 @@ export const generateMetadata = async ( return safeGenerateMetadata(async () => { const queryClient = getQueryClient() - const coursePages = await queryClient.fetchQuery( - pagesQueries.coursePages(readableId), + const { results: courses } = await queryClient.fetchQuery( + coursesQueries.coursesList({ readable_id: readableId, live: true }), ) - if (coursePages.items.length === 0) { - notFound() + if (courses.length === 0) { + throw new MetadataNotFound() } - const [course] = coursePages.items + const [course] = courses - const image = - course.course_details.page.feature_image_src || DEFAULT_RESOURCE_IMG + const image = course.page?.feature_image_src || DEFAULT_RESOURCE_IMG return standardizeMetadata({ title: course.title, @@ -50,7 +53,7 @@ const Page: React.FC> = async (props) => { const [coursePages, courses] = await Promise.all([ queryClient.fetchQuery(pagesQueries.coursePages(readableId)), queryClient.fetchQuery( - coursesQueries.coursesList({ readable_id: readableId }), + coursesQueries.coursesList({ readable_id: readableId, live: true }), ), ]) diff --git a/frontends/main/src/app/(products)/courses/p/[readable_id]/page.tsx b/frontends/main/src/app/(products)/courses/p/[readable_id]/page.tsx index 7d8d03da2d..8e0ae5899a 100644 --- a/frontends/main/src/app/(products)/courses/p/[readable_id]/page.tsx +++ b/frontends/main/src/app/(products)/courses/p/[readable_id]/page.tsx @@ -1,6 +1,10 @@ import React from "react" import { HydrationBoundary, dehydrate } from "@tanstack/react-query" -import { safeGenerateMetadata, standardizeMetadata } from "@/common/metadata" +import { + safeGenerateMetadata, + standardizeMetadata, + MetadataNotFound, +} from "@/common/metadata" import ProgramAsCoursePage from "@/app-pages/ProductPages/ProgramAsCoursePage" import { notFound, redirect } from "next/navigation" import { pagesQueries } from "api/mitxonline-hooks/pages" @@ -19,17 +23,16 @@ export const generateMetadata = async ( return safeGenerateMetadata(async () => { const queryClient = getQueryClient() - const programPages = await queryClient.fetchQuery( - pagesQueries.programPages(readableId), + const { results: programs } = await queryClient.fetchQuery( + programsQueries.programsList({ readable_id: readableId, live: true }), ) - if (programPages.items.length === 0) { - notFound() + if (programs.length === 0) { + throw new MetadataNotFound() } - const [program] = programPages.items + const [program] = programs - const image = - program.program_details.page.feature_image_src || DEFAULT_RESOURCE_IMG + const image = program.page?.feature_image_src || DEFAULT_RESOURCE_IMG return standardizeMetadata({ title: program.title, @@ -51,7 +54,7 @@ const Page: React.FC> = async (props) => { const [programPages, programs] = await Promise.all([ queryClient.fetchQuery(pagesQueries.programPages(readableId)), queryClient.fetchQuery( - programsQueries.programsList({ readable_id: readableId }), + programsQueries.programsList({ readable_id: readableId, live: true }), ), ]) diff --git a/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx b/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx index cbce7040ce..572af7bd21 100644 --- a/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx +++ b/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx @@ -1,6 +1,10 @@ import React from "react" import { HydrationBoundary, dehydrate } from "@tanstack/react-query" -import { safeGenerateMetadata, standardizeMetadata } from "@/common/metadata" +import { + safeGenerateMetadata, + standardizeMetadata, + MetadataNotFound, +} from "@/common/metadata" import ProgramPage from "@/app-pages/ProductPages/ProgramPage" import { notFound, redirect } from "next/navigation" import { DisplayModeEnum } from "@mitodl/mitxonline-api-axios/v2" @@ -19,17 +23,16 @@ export const generateMetadata = async ( return safeGenerateMetadata(async () => { const queryClient = getQueryClient() - const programPages = await queryClient.fetchQuery( - pagesQueries.programPages(readableId), + const { results: programs } = await queryClient.fetchQuery( + programsQueries.programsList({ readable_id: readableId, live: true }), ) - if (programPages.items.length === 0) { - notFound() + if (programs.length === 0) { + throw new MetadataNotFound() } - const [program] = programPages.items + const [program] = programs - const image = - program.program_details.page.feature_image_src || DEFAULT_RESOURCE_IMG + const image = program.page?.feature_image_src || DEFAULT_RESOURCE_IMG return standardizeMetadata({ title: program.title, @@ -51,7 +54,7 @@ const Page: React.FC> = async (props) => { const [programPages, programs] = await Promise.all([ queryClient.fetchQuery(pagesQueries.programPages(readableId)), queryClient.fetchQuery( - programsQueries.programsList({ readable_id: readableId }), + programsQueries.programsList({ readable_id: readableId, live: true }), ), ]) diff --git a/frontends/main/src/common/metadata.ts b/frontends/main/src/common/metadata.ts index 1a7adce35d..29b796cb64 100644 --- a/frontends/main/src/common/metadata.ts +++ b/frontends/main/src/common/metadata.ts @@ -20,6 +20,13 @@ type MetadataAsyncProps = { social?: boolean } & Metadata +/** + * Throw inside safeGenerateMetadata to trigger a 404 not-found response. + * Use this instead of Next.js's notFound(), which throws an internal error + * that safeGenerateMetadata's catch block cannot intercept. + */ +export class MetadataNotFound extends Error {} + /** * Wraps the metadata generation function in a try/catch block. Uncaught or * rethrown errors in generateMetadata result in showing the fallback error page, @@ -38,12 +45,15 @@ export async function safeGenerateMetadata( try { return await fn() } catch (error: unknown) { + if (error instanceof MetadataNotFound) { + return notFound() + } if ((error as AxiosError)?.response?.status === 404) { return notFound() } console.error("Error fetching page metadata", error) Sentry.captureException(error) - return await standardizeMetadata() + return standardizeMetadata() } } From 2ecabc5d4dfef0e3a9b58883643d9d60a9b63176 Mon Sep 17 00:00:00 2001 From: Doof Date: Mon, 11 May 2026 16:52:23 +0000 Subject: [PATCH 4/4] Release 0.67.1 --- RELEASE.rst | 7 +++++++ main/settings.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index 1269fce8f0..2f0548fb3f 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,13 @@ Release Notes ============= +Version 0.67.1 +-------------- + +- fix: 404 on product pages when live=false (#3295) +- feat: added welcome to learner email once he login for the first time (#3324) +- dashboard refactor phase 1 (#3320) + Version 0.67.0 (Released May 11, 2026) -------------- diff --git a/main/settings.py b/main/settings.py index 1bc8b49d5e..b85b79e485 100644 --- a/main/settings.py +++ b/main/settings.py @@ -35,7 +35,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.67.0" +VERSION = "0.67.1" log = logging.getLogger()