test/docs(404): add unrelated-edit round-trip test; correct MultiCollection-only rationale
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 7s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 12s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 23s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 7m20s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 7s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 18m48s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 15m10s
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 7s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 12s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 23s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 7m20s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 7s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 18m48s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 15m10s
Review + live-E2E follow-ups (no behavior change): - Add a test proving the canonical silent-reset trap directly: a rename-only save round-trips both weights untouched (the prior test only edited the weight it asserted). Cold review nit. - Correct the rationale in itemRules.ts + decisions.md: WeightedShuffle is MultiCollection-only in the SPA for *meaningfulness* (per-source weights need 2+ sources), NOT because the classic write path rejects it — live-E2E confirmed the classic engine ACCEPTS it on a plain Collection (200) and degrades to fair-share. The rejection is on the separate playlist/block write paths, whose editors keep their own order lists and already omit it. Live-E2E (real API): weighted multi-collection create + read round-trips weights; rename-only PUT preserves them (no silent reset); WeightedShuffle persists on a classic MultiCollection schedule item. Ratio itself is pinned by the existing PlayoutBuildGoldenTests.Classic_weighted (3:1), untouched by this SPA change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+7
-4
@@ -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,
|
||||
|
||||
@@ -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'];
|
||||
|
||||
@@ -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(<MultiCollectionsScreen />);
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user