From 56fffacfd4e041d85d1e5c8d5f7f14c9e0d4e64b Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 18 Jul 2026 01:29:58 +0200 Subject: [PATCH] fix(386): address cold-review findings on the DetailPanel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold adversarial review (no blockers, 3 real Mediums): - Query&size "Order" row ignored the Shuffle toggle (dead ?? fallback showing the static axis order); now reflects shuffleOn, consistent with the subtitle. - Clearing Name/Number to '' flagged the row Edited + armed the unsaved-changes guard, but the payload reverted to the proposal default — the two "edited" derivations disagreed. overrideEdited now ignores an empty (inherited) value. - runPreview did not clear per-channel overrides, so edits (incl. pinned numbers) silently carried into a re-generated batch → collision risk. Fresh preview now resets overrides/detailKey/logo cache. Tests: empty-name-not-Edited + re-preview-clears-overrides. Live-E2E (local instance, seeded TV library): preview → Configure → toggle Shuffle → Create verified end-to-end; the created channel's schedule shows PlaybackOrder=Shuffle (overridden) vs SeasonEpisode (axis default), confirming the per-channel override flows UI → SPA → backend → playout. Refs #386 --- web/src/screens/AutoTuneScreen.test.tsx | 37 +++++++++++++++++++++++++ web/src/screens/AutoTuneScreen.tsx | 16 ++++++++--- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/web/src/screens/AutoTuneScreen.test.tsx b/web/src/screens/AutoTuneScreen.test.tsx index 95caf9439..5fd6c3631 100644 --- a/web/src/screens/AutoTuneScreen.test.tsx +++ b/web/src/screens/AutoTuneScreen.test.tsx @@ -302,4 +302,41 @@ describe('AutoTuneScreen', () => { const comedy = body.channels.find((c: { value: string }) => c.value === 'Comedy'); expect(comedy).toEqual({ axis: 'TvGenre', value: 'Comedy', name: 'Comedy', number: '502' }); }); + + it('does not flag a row Edited when the name is typed then cleared back to empty', async () => { + mockAutoTuneApi(); + await openPreview(); + + fireEvent.click(screen.getAllByRole('button', { name: 'Configure' })[0]); + await screen.findByRole('dialog', { name: 'The Office' }); + + const nameInput = screen.getByDisplayValue('The Office'); + fireEvent.change(nameInput, { target: { value: 'Renamed' } }); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + expect(screen.getByText('Edited')).toBeInTheDocument(); + + // Reopen and clear the field — inheriting the default is not an edit. + fireEvent.click(screen.getAllByRole('button', { name: 'Configure' })[0]); + fireEvent.change(screen.getByDisplayValue('Renamed'), { target: { value: '' } }); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + expect(screen.queryByText('Edited')).not.toBeInTheDocument(); + }); + + it('drops per-channel edits when the preview is re-run', async () => { + mockAutoTuneApi(); + await openPreview(); + + fireEvent.click(screen.getAllByRole('button', { name: 'Configure' })[0]); + await screen.findByRole('dialog', { name: 'The Office' }); + fireEvent.change(screen.getByDisplayValue('The Office'), { target: { value: 'The Office (US)' } }); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + expect(screen.getByText('Edited')).toBeInTheDocument(); + + // Back to Configure, re-run Preview → a fresh batch starts with no carried-over overrides. + fireEvent.click(screen.getByRole('button', { name: 'Back' })); + fireEvent.click(screen.getByRole('button', { name: /Preview channels/ })); + await screen.findByText('TV Shows'); + expect(screen.queryByText('Edited')).not.toBeInTheDocument(); + expect(screen.getByRole('checkbox', { name: 'The Office' })).toBeInTheDocument(); + }); }); diff --git a/web/src/screens/AutoTuneScreen.tsx b/web/src/screens/AutoTuneScreen.tsx index e142fdfe9..59800a8a6 100644 --- a/web/src/screens/AutoTuneScreen.tsx +++ b/web/src/screens/AutoTuneScreen.tsx @@ -128,8 +128,11 @@ function overrideEdited(override: ChannelOverride | undefined, proposal: AutoTun return false; } return Boolean( - (override.name != null && override.name.trim() !== proposal.name) || - (override.number != null && override.number.trim() !== proposal.number) || + // A cleared field (trimmed to '') is NOT an edit — it inherits the proposal default, the same + // value buildChannels submits (`override.name?.trim() || proposal.name`). Only a non-empty value + // that differs counts, so the "Edited" badge + dirty guard match what actually gets created. + (override.name != null && override.name.trim() !== '' && override.name.trim() !== proposal.name) || + (override.number != null && override.number.trim() !== '' && override.number.trim() !== proposal.number) || override.templateId || override.logoFile || override.playbackOrder != null || @@ -249,6 +252,12 @@ export function AutoTuneScreen() { const id = ++previewSeqRef.current; setStep('preview'); setPreviewState({ status: 'loading' }); + // A fresh preview is a clean slate: drop any per-channel edits from a prior batch. They are + // keyed by axis+value and would silently re-attach to the re-enumerated proposals — including a + // pinned channel number that no longer fits the server's recomputed numbering (collision risk). + setOverrides({}); + setDetailKey(null); + uploadedLogosRef.current = new Map(); previewAutoTune({ axes: AXES.filter((axis) => axisIds.has(axis.id)).map((axis) => axis.id), minItems: Math.max(1, Number.parseInt(minItems, 10) || 1), @@ -1224,7 +1233,6 @@ function DetailPanel({ const effectiveStreaming = effectiveValue('streamingMode', template, override.advanced) as StreamingMode | undefined; const streamingLabel = effectiveStreaming ? (STREAMING_MODE_LABELS[effectiveStreaming] ?? effectiveStreaming) : '—'; - const axisMeta = AXES.find((axis) => axis.id === proposal.axis); const batchTemplateName = templates.find((candidate) => String(candidate.id) === batchTemplateId)?.name ?? 'batch template'; const overrideCount = Object.keys(override.advanced).length; @@ -1362,7 +1370,7 @@ function DetailPanel({ - + {proposal.itemCount}} />