From c01e546b51cfc8b96b6bf52b7cc5eb88c992c28b Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Sat, 7 Nov 2026 13:14:37 +0100 Subject: [PATCH] fix(console): keep the source switch panel truthful while pending While a switch request was in flight, a topology poll that already saw the new source made the panel say the selected source was no longer available and show a different source in the picker. The panel now renders from what the operator submitted until the request settles: the picker is locked on the submitted target, the stale-target hint and the unknown-source line stay hidden, and the panel stays mounted through a failed or malformed poll. Before submit and after it settles, the checks are unchanged. Confirm buttons in ReasonConfirm also show a progress indicator and announce the pending state. Closes dmfdeploy/dmfdeploy#861 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- frontend/src/__tests__/reasonConfirm.test.tsx | 17 ++ .../__tests__/switchSourcePending.test.tsx | 247 ++++++++++++++++++ frontend/src/components/ReasonConfirm.tsx | 19 ++ .../MediaWorkloads/stages/ConfigureStage.tsx | 48 +++- 4 files changed, 322 insertions(+), 9 deletions(-) create mode 100644 frontend/src/__tests__/switchSourcePending.test.tsx diff --git a/frontend/src/__tests__/reasonConfirm.test.tsx b/frontend/src/__tests__/reasonConfirm.test.tsx index 401d93a..cf42aab 100644 --- a/frontend/src/__tests__/reasonConfirm.test.tsx +++ b/frontend/src/__tests__/reasonConfirm.test.tsx @@ -551,3 +551,20 @@ describe('ReasonConfirm capacity refusal', () => { expect(screen.getByText(/Check your access, then retry/)).toBeTruthy() }) }) + +describe('ReasonConfirm pending indicator (dmfdeploy/dmfdeploy#861)', () => { + it('shows a progress indicator and announces progress only while pending', () => { + const props = { title: 'T', description: 'D', pendingLabel: 'Working…', onConfirm: vi.fn(), onCancel: () => {} } + const { rerender } = render() + expect(screen.queryByTestId('reason-confirm-spinner')).toBeNull() + expect(screen.getByRole('status').textContent).toBe('') + + rerender() + expect(screen.getByTestId('reason-confirm-spinner')).toBeTruthy() + expect(screen.getByRole('status').textContent).toBe('In progress') + expect(document.querySelector('.throbber-spin')).not.toBeNull() + + rerender() + expect(screen.queryByTestId('reason-confirm-spinner')).toBeNull() + }) +}) diff --git a/frontend/src/__tests__/switchSourcePending.test.tsx b/frontend/src/__tests__/switchSourcePending.test.tsx new file mode 100644 index 0000000..aaf228f --- /dev/null +++ b/frontend/src/__tests__/switchSourcePending.test.tsx @@ -0,0 +1,247 @@ +/** + * dmfdeploy/dmfdeploy#861: a source switch in progress must not say the + * selected source is gone. + * + * The topology query polls while the panel is armed. A poll that lands while + * the switch request is in flight can show the target as active_source, fail, + * return malformed data or leave no other source. While the request is + * pending the panel keeps the pending indicator and shows the submitted + * target in a disabled select, with no pre-submit "unknown or stale" line. + * Before submit, and after an error settles while the control is still + * rendered, the stale-target check applies to the current target. + */ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import ConfigureStage from '../pages/MediaWorkloads/stages/ConfigureStage' +import { InventoryWritesBlockedProvider } from '../lib/inventoryFreshness' +import type { MediaWorkload } from '../api/types' + +afterEach(() => { + cleanup() + vi.restoreAllMocks() + vi.unstubAllGlobals() +}) + +function json(b: unknown, status = 200) { + return new Response(JSON.stringify(b), { status, headers: { 'Content-Type': 'application/json' } }) +} + +const WORKLOAD = { + slug: 'studio-a', + name: 'studio-a', + lifecycle: 'operate', + health: 'ok', + instances: [ + { + instance: 'viewer-1', + netbox_id: 1, + function_key: 'crosspoint', + live_view: false, + requested_state: 'active', + observed_state: 'running', + reconcile_pending: false, + placement: { node: 'node-1', ports: [], protocol: null }, + workload_assignment: 'ok', + }, + ], + functions: [{ function_key: 'crosspoint', count: 1, running: 1, reconcile_pending: 0 }], +} as unknown as MediaWorkload + +const SOURCES = [ + { id: 'source-a', flow_id: 'f1', pattern: 'smpte' }, + { id: 'source-b', flow_id: 'f2', pattern: 'ball' }, +] + +const KEY = ['media-workloads-topology', 'viewer-1'] +type TopologyMode = 'ok' | 'error' | 'malformed' + +function setup() { + let active = 'source-a' + let sources = SOURCES + let mode: TopologyMode = 'ok' + let settlePost: (outcome: 'ok' | 'fail') => void = () => {} + const gate = new Promise<'ok' | 'fail'>((resolve) => { + settlePost = resolve + }) + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = (typeof input === 'string' ? input : (input as Request).url).toString() + if (init?.method === 'POST') { + if ((await gate) === 'fail') return json({ error: 'upstream failed' }, 500) + return json({ + request_id: 'r1', + status: 'active', + outcome: 'active', + source_instance: 'source-b', + reason: 'go', + actor: 'ops', + role: 'engineer', + }) + } + if (url.endsWith('/topology')) { + if (mode === 'error') return json({ error: 'boom' }, 500) + return json({ + receiver_instance: 'viewer-1', + sources: mode === 'malformed' ? 'not-a-list' : sources, + active_source: active, + provenance: 'observed-flow', + observed_at: new Date().toISOString(), + }) + } + return json({}) + }), + ) + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const tree = () => ( + + + {}} + onSwitchResult={() => {}} + onJobStart={() => {}} + /> + + + ) + const view = render(tree()) + // Delivers a poll result, then waits until the query holds it, so the + // assertions that follow cannot pass just because the poll never landed. + const poll = async (next: { active?: string; sources?: typeof SOURCES; mode?: TopologyMode }) => { + if (next.active) active = next.active + if (next.sources) sources = next.sources + mode = next.mode ?? 'ok' + await act(async () => { + await qc.refetchQueries({ queryKey: KEY }) + }) + await waitFor(() => { + const state = qc.getQueryState(KEY) + if (mode === 'error') expect(state?.status).toBe('error') + else if (mode === 'malformed') expect(qc.getQueryData<{ sources: unknown }>(KEY)?.sources).toBe('not-a-list') + else expect(qc.getQueryData<{ active_source: string }>(KEY)?.active_source).toBe(active) + }) + // The query notifies React on a later task; let that render happen. + await act(async () => { + await new Promise((r) => setTimeout(r, 0)) + }) + } + return { poll, settlePost, qc, view, tree } +} + +async function armAndPick(target: string) { + fireEvent.click(await screen.findByRole('button', { name: 'Switch source' })) + fireEvent.change(screen.getByTestId('reason-switch-source'), { target: { value: 'go' } }) + fireEvent.change(screen.getByRole('combobox'), { target: { value: target } }) +} + +async function submitAndWaitPending() { + fireEvent.click(screen.getByTestId('confirm-switch-source')) + await waitFor(() => + expect(screen.getByTestId('confirm-switch-source').textContent).toContain('Switching…'), + ) +} + +function expectPendingPanel() { + expect(screen.queryByText(HINT)).toBeNull() + expect(screen.queryByText(/Live source is unknown or stale/)).toBeNull() + const select = screen.getByRole('combobox') as HTMLSelectElement + expect(select.value).toBe('source-b') + expect(select.disabled).toBe(true) + expect(screen.getByTestId('confirm-switch-source').textContent).toContain('Switching…') + expect(screen.getByTestId('reason-confirm-spinner')).toBeTruthy() +} + +const HINT = /no longer available/ + +describe('switch source while the request is pending (#861)', () => { + it('a poll that shows the target as active keeps the submitted target and raises no hint', async () => { + const { poll, settlePost } = setup() + await armAndPick('source-b') + await submitAndWaitPending() + await poll({ active: 'source-b' }) + expectPendingPanel() + settlePost('ok') + }) + + it('a poll that fails keeps the panel, the pending state and the submitted target', async () => { + const { poll, settlePost } = setup() + await armAndPick('source-b') + await submitAndWaitPending() + await poll({ mode: 'error' }) + expectPendingPanel() + settlePost('ok') + }) + + it('a poll with malformed data keeps the panel, the pending state and the submitted target', async () => { + const { poll, settlePost } = setup() + await armAndPick('source-b') + await submitAndWaitPending() + await poll({ mode: 'malformed' }) + expectPendingPanel() + settlePost('ok') + }) + + it('a poll that leaves no other source keeps the panel, the pending state and the submitted target', async () => { + const { poll, settlePost } = setup() + await armAndPick('source-b') + await submitAndWaitPending() + await poll({ active: 'source-b', sources: [SOURCES[1]] }) + expectPendingPanel() + settlePost('ok') + }) + + it('an error settle after a poll made the target active shows the hint and disables Confirm', async () => { + const { poll, settlePost } = setup() + await armAndPick('source-b') + await submitAndWaitPending() + await poll({ active: 'source-b' }) + settlePost('fail') + expect(await screen.findByText(HINT)).toBeTruthy() + expect((screen.getByTestId('confirm-switch-source') as HTMLButtonElement).disabled).toBe(true) + expect((screen.getByRole('combobox') as HTMLSelectElement).disabled).toBe(false) + }) + + it('a success settle closes the panel and shows the result', async () => { + const { poll, settlePost } = setup() + await armAndPick('source-b') + await submitAndWaitPending() + await poll({ active: 'source-b' }) + settlePost('ok') + expect(await screen.findByText('Active source: source-b')).toBeTruthy() + expect(screen.queryByRole('combobox')).toBeNull() + }) + + it('unmounting while pending and remounting shows no stale snapshot or stuck pending state', async () => { + const { settlePost, view, tree } = setup() + await armAndPick('source-b') + await submitAndWaitPending() + view.unmount() + render(tree()) + const arm = await screen.findByRole('button', { name: 'Switch source' }) + expect(screen.queryByText('Switching…')).toBeNull() + fireEvent.click(arm) + fireEvent.change(screen.getByTestId('reason-switch-source'), { target: { value: 'again' } }) + const select = screen.getByRole('combobox') as HTMLSelectElement + expect(select.disabled).toBe(false) + expect(within(select).getAllByRole('option').map((o) => (o as HTMLOptionElement).value)).toEqual(['', 'source-b']) + fireEvent.change(select, { target: { value: 'source-b' } }) + expect((screen.getByTestId('confirm-switch-source') as HTMLButtonElement).disabled).toBe(false) + settlePost('ok') + await act(async () => {}) + }) +}) + +describe('switch source before submit (#861)', () => { + it('a poll that drops the selected target shows the hint and disables Confirm', async () => { + const { poll } = setup() + await armAndPick('source-b') + expect((screen.getByTestId('confirm-switch-source') as HTMLButtonElement).disabled).toBe(false) + await poll({ active: 'source-b' }) + expect(await screen.findByText(HINT)).toBeTruthy() + expect((screen.getByTestId('confirm-switch-source') as HTMLButtonElement).disabled).toBe(true) + }) +}) diff --git a/frontend/src/components/ReasonConfirm.tsx b/frontend/src/components/ReasonConfirm.tsx index 0197a50..0fdd98f 100644 --- a/frontend/src/components/ReasonConfirm.tsx +++ b/frontend/src/components/ReasonConfirm.tsx @@ -1,4 +1,5 @@ import { useId, useState } from 'react' +import { Disc3 } from 'lucide-react' import { Input, Select, Textarea } from './FormField' import { capacityRefusalMessage } from '../lib/capacityRefusal' import { useInventoryWritesBlocked } from '../lib/inventoryFreshness' @@ -69,6 +70,8 @@ export interface ReasonConfirmExtraField { onChange: (value: string) => void invalid?: boolean invalidHint?: string + // Locks the field, e.g. while the write it feeds is in flight. + disabled?: boolean // umbrella #201 WP5: when provided, renders a of these options // (e.g. the switch-source target picker) instead of the default free-text // — every other consumer (the #239 workload field) omits this and @@ -159,6 +162,7 @@ export default function ReasonConfirm({ id={extraFieldId} className="mt-1.5" value={extraField.value} + disabled={extraField.disabled} onChange={(e) => extraField.onChange(e.target.value)} > @@ -177,6 +181,7 @@ export default function ReasonConfirm({ className="mt-1.5" placeholder={extraField.placeholder} value={extraField.value} + disabled={extraField.disabled} onChange={(e) => extraField.onChange(e.target.value)} /> )} @@ -215,8 +220,22 @@ export default function ReasonConfirm({ onClick={() => onConfirm(reason.trim())} {...(confirmId ? { 'data-testid': confirmId } : {})} > + {pending && ( + // umbrella #861: the label swap alone was the only sign of work in + // progress. Same glyph and class as AutomationInProgressNotice, so + // the reduced-motion rule in index.css stops it. + + + + )} {pending ? pendingLabel : confirmLabel} + {/* The live region is always mounted so the change to pending is + announced; the button's own text swap is not. It carries its own + wording so the pending label stays unique on the page. */} + + {pending ? 'In progress' : ''} + Cancel diff --git a/frontend/src/pages/MediaWorkloads/stages/ConfigureStage.tsx b/frontend/src/pages/MediaWorkloads/stages/ConfigureStage.tsx index 9466389..78fe1b6 100644 --- a/frontend/src/pages/MediaWorkloads/stages/ConfigureStage.tsx +++ b/frontend/src/pages/MediaWorkloads/stages/ConfigureStage.tsx @@ -126,6 +126,8 @@ export default function ConfigureStage({ ) } +type SourceOption = { value: string; label: string } + function InstanceSwitchControl({ instance, hasTopology, @@ -152,6 +154,7 @@ function InstanceSwitchControl({ const recordAwxWrite = useActivityStore((s) => s.recordAwxWrite) const [arming, setArming] = useState(false) const [target, setTarget] = useState('') + const [submitted, setSubmitted] = useState<{ target: string; options: SourceOption[] } | null>(null) useEffect(() => onPendingChange(switchMutation.isPending), [switchMutation.isPending, onPendingChange]) @@ -172,9 +175,20 @@ function InstanceSwitchControl({ // regardless of isError (observed_at goes stale on its own), but the // withdraw-on-error contract should read the same everywhere this // topology shape is consumed. - if (topology.failed || !topology.data || !Array.isArray(topology.data.sources)) return null + // umbrella #861: while the switch request is in flight the panel renders + // from what the operator submitted, so a poll that errors, returns bad + // data or leaves no other source cannot remove the pending indicator or + // change the target shown, and the pre-submit "unknown or stale" line + // stays hidden. `frozen` is null whenever nothing is pending, so the checks + // below behave as before then. + const pending = switchMutation.isPending + const frozen = pending ? submitted : null + const live = + topology.failed || !topology.data || !Array.isArray(topology.data.sources) ? null : topology.data + if (!live && !frozen) return null - const { sources, active_source, provenance, observed_at } = topology.data + const sources = live?.sources ?? [] + const { active_source, provenance, observed_at } = live ?? {} const isObservedFresh = provenance === 'observed-flow' && !!active_source && @@ -182,10 +196,12 @@ function InstanceSwitchControl({ Date.now() - new Date(observed_at).getTime() < OBSERVED_SOURCE_STALE_MS const otherSources = isObservedFresh ? sources.filter((s) => s.id !== active_source) : [] - if (isObservedFresh && otherSources.length === 0) return null + if (!frozen && isObservedFresh && otherSources.length === 0) return null const result = switchMutation.data + const targetOffered = otherSources.some((s) => s.id === target) + const submit = (reason: string) => { if (!target) return const stillFresh = @@ -199,6 +215,10 @@ function InstanceSwitchControl({ } catch { return } + setSubmitted({ + target, + options: otherSources.map((s) => ({ value: s.id, label: `${s.id} (${s.pattern})` })), + }) onJobStart() switchMutation.mutate( { instance, sourceInstance: target, reason }, @@ -265,7 +285,7 @@ function InstanceSwitchControl({ stale under the operator mid-form, Confirm goes dead — and a dead Confirm with no stated reason is the honesty gap, not the block itself (Art. 1). */} - {!isObservedFresh && ( + {!frozen && !isObservedFresh && ( {OBSERVED_SOURCE_UNKNOWN_MESSAGE} )} {!arming && isObservedFresh && !allowed && !result && ( @@ -282,7 +302,7 @@ function InstanceSwitchControl({ description="Coarse reconfigure/reconnect actuator — not a live IS-05 switch. Re-points this viewer to a different source and is recorded in the audit trail with your reason." confirmLabel="Confirm switch" pendingLabel="Switching…" - pending={switchMutation.isPending} + pending={pending} error={switchMutation.isError ? switchMutation.error : undefined} errorMessage={SWITCH_REQUEST_FAILED_TEXT} onConfirm={submit} @@ -294,7 +314,7 @@ function InstanceSwitchControl({ extraField={{ label: 'Target source', placeholder: 'Select a source…', - value: target, + value: frozen ? frozen.target : target, // umbrella #402: `topology` now polls while armed (it didn't // before), so `target` — plain state, not re-derived from // `otherSources` — can point at a source that a poll landing @@ -309,13 +329,23 @@ function InstanceSwitchControl({ // submit()'s own freshness re-check, both out of scope here — // this only widens the existing Confirm-disable condition to a // fact already available at render time. - invalid: target === '' || !isObservedFresh || !otherSources.some((s) => s.id === target), + // + // umbrella #861: none of the above applies once the request is + // in flight. The select is disabled and shows the submitted + // target from the options shown at submit. After the request + // settles with an error the live check applies again: if the + // control is still rendered, a target that became active or + // vanished mid-request shows the hint and disables Confirm. + // If the early returns above withdraw the control (the poll + // failed, or the target is now the only source), nothing shows. + invalid: frozen ? false : target === '' || !isObservedFresh || !targetOffered, invalidHint: - target !== '' && isObservedFresh && !otherSources.some((s) => s.id === target) + !frozen && target !== '' && isObservedFresh && !targetOffered ? 'Selected source is no longer available — choose again.' : undefined, + disabled: pending, onChange: setTarget, - options: otherSources.map((s) => ({ value: s.id, label: `${s.id} (${s.pattern})` })), + options: frozen ? frozen.options : otherSources.map((s) => ({ value: s.id, label: `${s.id} (${s.pattern})` })), }} />
{OBSERVED_SOURCE_UNKNOWN_MESSAGE}