Decouple practice item from a single track (#4)#6
Open
sjiang1 wants to merge 1 commit into
Open
Conversation
Replace PracticeItem.trackId with trackChoices: string[], so an item can reference a pool of tracks (the foundation for the dice-picks-a-track feature, #5). A new primaryTrackId(item) helper in plans.ts is the single seam every consumer reads through: PracticeTasksList, teacher-data aggregation, plan-validation, and the editor (PlanEditor / PracticeItemRow / replace-item-track / NewPlanButton) all resolve their lone track via it, so single-element pools behave exactly as before. #5 swaps this helper for "the rolled track." actions.getPlans gains normalizePlans, a read-time upgrade that maps legacy {trackId} items (the shape still stored in prod Redis) to {trackChoices}, so deploying needs no manual Redis migration. data/plans.json and the test seed are rewritten to the array shape. Tests: trackId PracticeItem literals migrated to trackChoices across the suite (validation-error and TrackReport trackId fields are unchanged); adds primaryTrackId unit tests and getPlans legacy-normalization tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Jun 26, 2026
There was a problem hiding this comment.
Pull request overview
This PR updates the plans data model so a PracticeItem can reference a pool of tracks via trackChoices: string[] instead of a single trackId, introducing primaryTrackId(item) as the current “active track” seam and adding a read-time normalization step to keep legacy Redis data working without a manual migration.
Changes:
- Replaced
PracticeItem.trackIdwithtrackChoices: string[]across rendering, editing, validation, teacher aggregation, and seed data. - Added
primaryTrackId(item)and migrated consumers to read the “active” track via this helper. - Added
normalizePlans()ingetPlans()to upgrade legacy{ trackId }items to{ trackChoices: [trackId] }, plus tests for both new and legacy shapes.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/test-support/seed.ts | Updates test seed plans to use trackChoices. |
| src/app/teacher-data.ts | Aggregation now uses primaryTrackId(item) instead of item.trackId. |
| src/app/teacher-data.test.ts | Migrates teacher report tests to trackChoices. |
| src/app/seed-data.test.ts | Validates all trackChoices track IDs exist in committed seed data. |
| src/app/PracticeTasksList.tsx | Renders practice items using primaryTrackId(item) as the track identifier. |
| src/app/plans/replace-item-track.ts | Replaces a practice item’s track by setting a single-element trackChoices pool. |
| src/app/plans/replace-item-track.test.ts | Updates replace-track tests for trackChoices. |
| src/app/plans/PracticeItemRow.tsx | Editor row now identifies items via primaryTrackId(item). |
| src/app/plans/PlanEditor.tsx | Editor create/drag/lookup paths migrated to trackChoices + primaryTrackId. |
| src/app/plans/plan-validation.ts | Validation errors now key by primaryTrackId(item). |
| src/app/plans/plan-validation.test.ts | Updates validation tests to build plans with trackChoices. |
| src/app/plans/NewPlanButton.tsx | Copies trackChoices when cloning plans. |
| src/app/plans.ts | Defines trackChoices and introduces primaryTrackId(item) seam. |
| src/app/plans.test.ts | Adds unit tests for primaryTrackId; updates plan literals to trackChoices. |
| src/app/actions.ts | Adds normalizePlans() and applies it in getPlans() for legacy back-compat. |
| src/app/actions.test.ts | Adds tests for legacy normalization + updates affected plan literals. |
| data/plans.json | Migrates committed seed plans to trackChoices. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+22
| export function primaryTrackId(item: PracticeItem): string { | ||
| return item.trackChoices[0]; | ||
| } |
Comment on lines
+126
to
+131
| items: plan.items.map((item) => { | ||
| if (Array.isArray(item.trackChoices)) return item; | ||
| const legacy = item as unknown as { trackId?: string }; | ||
| const { trackId, ...rest } = legacy; | ||
| return { ...(rest as object), trackChoices: trackId ? [trackId] : [] } as typeof item; | ||
| }), |
Comment on lines
173
to
+177
| {draft.items.map((item, idx) => ( | ||
| <PracticeItemRow | ||
| key={item.trackId} | ||
| key={primaryTrackId(item)} | ||
| item={item} | ||
| track={trackList.find((t) => t.id === item.trackId)} | ||
| track={trackList.find((t) => t.id === primaryTrackId(item))} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4.
What
Replaces
PracticeItem.trackIdwithtrackChoices: string[], so a practice item can reference a pool of tracks instead of exactly one. This is the data-model foundation for the dice-picks-a-track feature (#5) — no roll UI here, decoupling only.How it stays behavior-preserving
Every existing item becomes a one-element pool, so nothing changes for users today.
primaryTrackId(item) → item.trackChoices[0]inplans.tsis the single seam every consumer reads through (rendering, done-keys, notes, teacher progress aggregation, validation, editor). Shared practice-page die with per-item face customization (dice picks a track/rhythm) #5 will swap this one function for "the rolled track."Prod data migration
Plans live in Upstash Redis in prod, still in the old
{trackId}shape.getPlansnow runsnormalizePlans, a read-time upgrade mapping legacy{trackId}→{trackChoices: [trackId]}, so no manual Redis migration is needed on deploy.data/plans.jsonand the test seed are rewritten to the array shape.Tests
trackIdPracticeItemliterals migrated totrackChoicesacross the suite (validation-error andTrackReporttrackIdfields are intentionally unchanged).primaryTrackIdunit tests, andgetPlanslegacy-normalization tests (legacy item upgrades; already-migrated item untouched).Review focus
primaryTrackIdseam — confirm it's the right place for Shared practice-page die with per-item face customization (dice picks a track/rhythm) #5 to later inject the rolled track.normalizePlansinactions.ts— the prod-Redis back-compat shim.🤖 Generated with Claude Code