Release 0.68.2#3353
Merged
Merged
Conversation
Co-authored-by: Ahtesham Quraish <ahtesham.quraish@A006-01455.local>
* dashboard refactor phase 3: extract home dashboard view-model helpers
Move home-only data orchestration out of the home dashboard component
and into a typed composer (private function in HomeEnrollmentsDisplay.tsx)
that calls named pure helpers from model/dashboardViewModel.ts.
New pure helpers in dashboardViewModel.ts (with unit tests):
bucketAndSortHomeEnrollments, enrollmentCourseIsInPrograms,
groupModuleCoursesByProgramId, getModuleCourseIdsFromPrograms,
getNonContractProgramEnrollments, getTopLevelProgramEnrollments,
isProgramAsCourse, isNonContractEnrollment.
ProgramEnrollmentDisplay's two inline reduces (enrollmentsByCourseId,
programEnrollmentsById) now use the simplified helpers added in Phase 1.
Drop the dead courses parameter from groupCourseRunEnrollmentsByCourseId
— its filter never tripped because both callers only look up by ids the
helper would have included.
Soften the plan's "zero inline transforms" exit check across Phases 3-5
to "no inline-defined domain logic" — composing a named predicate or
comparator with .filter() is glue, not orchestration. Add an Open
question noting Phase 3 inlined useHomeDashboardData as a private
function rather than its own file; Phase 4/5 settle the hook location
and test strategy before they begin.
Rename ProgramContent.test.tsx's mock testid enrollment-display →
program-enrollment-display for clarity now that two dashboards exist.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add home dashboard B2B course filter test
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* dashboard refactor phase 3 v3: extract useHomeDashboardData hook + assembleHomeCardList
Set the consistent composer/testing pattern for the dashboard refactor
(Phases 4-5 follow it):
- Add pure assembleHomeCardList(buckets+programEnrollments) ->
{ cards, initiallyVisibleCount } owning home card ordering + the
MIN_VISIBLE promotion rule (was stranded in EnrollmentExpandCollapse).
Isolated unit tests in dashboardViewModel.test.ts.
- Move useHomeDashboardData out of HomeEnrollmentsDisplay.tsx into a
separate exported hooks/useHomeDashboardData.ts; emits the durable
contract { cards, initiallyVisibleCount, ... }.
- Add hooks/useHomeDashboardData.test.tsx renderHook suite reusing the
shared setupEnrollments scenario (no duplicated mock setup).
- EnrollmentExpandCollapse reduced to zero-logic slice + toggle.
- HomeEnrollmentsDisplay.test.tsx left exactly untouched as the
behavior-preservation oracle.
- Update dashboardRefactorPlan.md: resolve the hook location/testing
open question, document Phase 3 v3, re-scope Phase 6 to a checkpoint,
add overall testing-strategy section.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* address phase 3 v3 review feedback
- B#1: resolveSlotForLanguage calls local pickDisplayedEnrollmentForLegacyDashboard
instead of round-tripping through helpers.selectBestEnrollment; drop the
now-unused ../helpers import. Removes the gratuitous helpers<->viewModel
cycle leg. Zero behavior change (resolveSlotForLanguage/Program/Contract
suites green).
- A: add assembleHomeCardList case (nonExpired>0, no expired ->
initiallyVisibleCount === cards.length), closing the case matrix.
- B#2: fix DashboardCourseSlot JSDoc typo.
- B#3: scope the type-only-import comment to that line (not a file-wide
no-cycle claim).
- Rewrite useHomeDashboardData JSDoc: timeless/descriptive, drop the
refactor-historical 'remains unchanged' framing.
- Plan: record the uniform durable-contract assertion rule (composer
suites assert the durable contract, never through the adapter;
displayed-*/run-selection pinned only in the resolveSlotForLanguage
unit suite) so Phase 4/5 inherit it.
Oracle (HomeEnrollmentsDisplay.test.tsx) still untouched. Full
DashboardPage suite 411/411.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* plan: language-picker decided (hook owns it, Phase 4); fold reviewer-C refinements
- Resolved decision: language picker = option (b) — useDashboardLanguagePicker
extracted in Phase 4, composed by useProgramDashboardData; component
presentational. Rationale recorded (hook needs the key to build slots
regardless, so the YAGNI-defer argument doesn't hold).
- Phase 4 pre-decisions block: (1) slot/adapter prove-or-delete, default
lean (A) build it for real on Program; (2) test-utils infra-only +
composable buildProgramScenario; (3) language picker (b).
- Phase 5: hard exit gate — no OrgProgramDisplay/OrgProgramCollectionDisplay
inline resolveSlotForLanguage / b2b_contract_id filtering.
- Phase 7: checkbox to migrate DashboardResource/DashboardType into
dashboardViewModel.ts before deleting DashboardCard.tsx.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* address gpt-5.4 review: doc accuracy + hook contract coverage
- A: scope the 'oracle untouched' wording in dashboardRefactorPlan.md.
HomeEnrollmentsDisplay.test.tsx is NOT modified by the v3 extraction
(empty diff for 8ec7d30^..HEAD), but it is new on this branch
(created cf3c0db, extended d812730 in Phase 3 v1). Plan now says
'not modified by the extraction commit' and flags the branch-vs-main
narrative for reviewers, instead of a blanket 'untouched'.
- B: add a renderHook test asserting the durable contract fields
courseProgramsById / moduleCoursesByProgramId (program-as-course
lookups the renderer needs) — previously only covered indirectly via
the component suite.
- C: no change. ProgramContent.test.tsx is modified on the branch only
by a mechanical test-id rename in Phase 3 v1 (cf3c0db), forced by
the EnrollmentDisplay->ProgramEnrollmentDisplay rename; its low-value
nature is pre-existing and out of refactor scope.
Oracle still not modified by v3. Full DashboardPage suite 412/412.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: correct adapter/slot wording in dashboard refactor plan
CoursewareCard consumes a per-row variant union, never a slot — fix the
two 'consumes the slot directly' phrasings (lines 214, 731) that
contradicted the Phase 7 variant-union design and stability rule. Also
fix two stale spots that drifted from the Resolved decision: the layer
table's composer-location cell and the net-file-count reading note
(Phase 3 v3 added a hooks/ directory of separate exported composers).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: correct layer-2 assertion rule for displayedEnrollment/displayedRun
The old rule banned displayedEnrollment/displayedRun from composer
renderHook suites and pinned them only in the pure resolveSlotForLanguage
suite — but those fields are returned, render-driving contract, and a
returned field that is 'wrong' to assert is a smell. The pure suite
proves the derivation; it cannot catch wiring (hook feeding the resolver
wrong-scoped enrollments or a stale language key). Split by which bug
each layer catches: composer suite asserts a representative
displayedEnrollment/displayedRun (wiring); pure suite owns the exhaustive
language-resolution matrix (derivation). Drop the stale 'only durable'
framing in the lead-in accordingly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Danielle Frappier
Chris Chudzicki
Ahtesham Quraish
Nathan Levesque