diff --git a/docs/decisions.md b/docs/decisions.md index ec6a62284..74ccb02c0 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -1956,10 +1956,13 @@ is purely SPA (+ docs). classic schedule item's Playback Order options. Putting weights on the schedule item was rejected: the same multi-collection can be referenced by many schedule items, and weights are shared, not per-reference. - **`WeightedShuffle` is offered ONLY for `collectionType === 'MultiCollection'`** (`itemRules.ts` - `MULTI_COLLECTION_ORDERS`). It needs per-source weights, which only exist on multi-collection members, so it - is meaningless on the single-source collection types — and the classic write path *rejects* it for - playlist/block items (decisions.md 2026-07-17), so offering it there would only produce a 400. This is the - SPA mirror of that write-path gate. + `MULTI_COLLECTION_ORDERS`) — a *meaningfulness* scope, not a rejection mirror. Weights only exist on + multi-collection members, so the order only does something with 2+ weighted sources; on a single collection + the classic engine still **accepts** it (verified by live-E2E: `PUT /schedules/{id}/items` with a plain + Collection + `WeightedShuffle` → 200) but it degrades to fair-share (≈ `Shuffle`), a confusing no-op — so + the SPA doesn't offer it there. The *rejection* the #70 entry describes is on the separate **playlist and + block** write paths (different engines); the playlist/block editors keep their own order lists (not + `MULTI_COLLECTION_ORDERS`) and already omit `WeightedShuffle`, so nothing extra was needed there. - **`WeightedShuffle` joins `ShuffleInOrder` in the `fillWithGroup` exclusion.** `fillWithGroupModeEligible` already excluded `ShuffleInOrder`; WeightedShuffle is excluded for the same structural reason — `PlayoutBuilder` splits a fill-with-group item into per-group enumerators scheduled one group at a time, diff --git a/web/src/schedules/itemRules.ts b/web/src/schedules/itemRules.ts index 76968027c..61740a54e 100644 --- a/web/src/schedules/itemRules.ts +++ b/web/src/schedules/itemRules.ts @@ -43,10 +43,12 @@ export function nextDraftKey(): string { // ---- Option lists -------------------------------------------------------- -// WeightedShuffle (#70) is offered ONLY here: it needs per-source weights, which live on -// MultiCollectionItem/SmartItem, so it is meaningless on the single-source collection types. The -// classic write path rejects it everywhere else (playlist/block items), so the SPA must not offer it -// there either (docs/decisions.md 2026-07-17). +// WeightedShuffle (#70) is offered ONLY for MultiCollection: it distributes airtime across per-source +// weights, which live on MultiCollectionItem/SmartItem, so it is only *meaningful* with 2+ weighted +// sources. On a single collection the classic engine still accepts it but it degrades to fair-share +// (≈ Shuffle), a confusing no-op — hence MultiCollection-only here. (Distinct concern: the playlist and +// block editors — which keep their own order lists, not this one — must also omit it because those +// write paths reject it outright; docs/decisions.md 2026-07-17.) const MULTI_COLLECTION_ORDERS: PlaybackOrder[] = ['Shuffle', 'ShuffleInOrder', 'WeightedShuffle']; const COLLECTION_LIKE_ORDERS: PlaybackOrder[] = ['Chronological', 'Random', 'Shuffle', 'ShuffleInOrder', 'Marathon']; const TV_SHOW_ORDERS: PlaybackOrder[] = ['Chronological', 'SeasonEpisode', 'Random', 'Shuffle', 'MultiEpisodeShuffle']; diff --git a/web/src/screens/MultiCollectionsScreen.test.tsx b/web/src/screens/MultiCollectionsScreen.test.tsx index 5b4f93998..cf4ab1e3d 100644 --- a/web/src/screens/MultiCollectionsScreen.test.tsx +++ b/web/src/screens/MultiCollectionsScreen.test.tsx @@ -162,20 +162,60 @@ describe('MultiCollectionsScreen', () => { }); }); + // Weighted multi-collection fixture reused by the round-trip tests: 3:1 split (Favorites : Action). + const weightedMulti = [ + { + id: 1, + items: [ + { collectionId: 5, name: 'Favorites', playbackOrder: 'Chronological', scheduleAsGroup: false, smartCollectionId: null, weight: 3 }, + { collectionId: null, name: 'Action', playbackOrder: 'Chronological', scheduleAsGroup: true, smartCollectionId: 9, weight: 1 } + ], + name: 'Saturday Bundle' + } + ]; + + it('preserves weights when only an unrelated field is edited (the silent-reset trap)', async () => { + // The canonical #404 trap: the replace-all PUT resubmits the whole item list, so editing ONLY the + // name must still round-trip each source's weight untouched — dropping it in the draft mapping + // would silently reset every weight to 1 on this save (docs/decisions.md 2026-07-17). + const fetchMock = mockApi({ + multi: weightedMulti, + onRequest: (url, method) => + url === '/api/v1/multi-collections/1' && method === 'PUT' + ? jsonResponse({ id: 1, items: [], name: 'Weekend Bundle' }) + : null + }); + + render(); + fireEvent.click(await screen.findByText('Saturday Bundle')); + await screen.findByLabelText('Weight for Favorites'); + + // Edit ONLY the name — never touch a weight input. + fireEvent.change(screen.getByPlaceholderText('Multi-collection name'), { target: { value: 'Weekend Bundle' } }); + fireEvent.click(screen.getByRole('button', { name: 'Save multi-collection' })); + + await waitFor(() => { + const putCall = fetchMock.mock.calls.find( + ([u, init]) => u === '/api/v1/multi-collections/1' && (init?.method ?? '').toUpperCase() === 'PUT' + ); + expect(putCall).toBeDefined(); + const parsed = JSON.parse(String(putCall?.[1]?.body)); + expect(parsed.name).toBe('Weekend Bundle'); + // Both original weights survive unchanged — NOT reset to 1. + expect(parsed.items).toEqual( + expect.arrayContaining([ + { collectionId: 5, playbackOrder: 'Chronological', scheduleAsGroup: false, smartCollectionId: null, weight: 3 }, + { collectionId: null, playbackOrder: 'Chronological', scheduleAsGroup: true, smartCollectionId: 9, weight: 1 } + ]) + ); + }); + }); + it('round-trips per-source weights on save (no silent reset), and edits them', async () => { // Weighted multi-collection: the API returns each item's weight and the update REPLACES the item // list, so the editor must carry weight through untouched or every source silently resets to 1 // (docs/decisions.md 2026-07-17, #404). - const weighted = [ - { - id: 1, - items: [ - { collectionId: 5, name: 'Favorites', playbackOrder: 'Chronological', scheduleAsGroup: false, smartCollectionId: null, weight: 3 }, - { collectionId: null, name: 'Action', playbackOrder: 'Chronological', scheduleAsGroup: true, smartCollectionId: 9, weight: 1 } - ], - name: 'Saturday Bundle' - } - ]; + const weighted = weightedMulti; const fetchMock = mockApi({ multi: weighted,