From 48c157c9f52ad4b631828fa20813bfe0ebc3d837 Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Sat, 7 Nov 2026 22:45:28 +0100 Subject: [PATCH 1/2] fix(console): show a finished teardown without waiting for the tab The operation poll skipped its interval while the tab was hidden, and the app turns window-focus refetching off, so returning to the tab did not refresh it either. A teardown that finished in under a minute showed as in progress for minutes. The operation status now polls in the background and refetches on focus until it settles. After the job finishes, the inventory can still list the entry as active. The review then said the teardown completed and offered Teardown again, and nothing re-read the catalog until the next focus. Each entry whose teardown finished now waits on its own inventory record: the catalog is polled every 3 s, also in the background, until that entry stops reading active, and the review says the job finished and is waiting for the inventory to confirm. Closes dmfdeploy/dmfdeploy#862 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- .../finaliseInventoryConfirm.test.tsx | 425 ++++++++++++++++++ .../__tests__/operationTerminalStall.test.tsx | 9 +- frontend/src/api/hooks.ts | 37 +- .../MediaWorkloads/stages/FinaliseStage.tsx | 53 ++- .../MediaWorkloads/stages/JobProgress.tsx | 3 +- 5 files changed, 515 insertions(+), 12 deletions(-) create mode 100644 frontend/src/__tests__/finaliseInventoryConfirm.test.tsx diff --git a/frontend/src/__tests__/finaliseInventoryConfirm.test.tsx b/frontend/src/__tests__/finaliseInventoryConfirm.test.tsx new file mode 100644 index 0000000..be71bde --- /dev/null +++ b/frontend/src/__tests__/finaliseInventoryConfirm.test.tsx @@ -0,0 +1,425 @@ +/** + * A finished teardown used to show late on the workload page, for several + * independent reasons, and these tests pin each of them: + * + * 1. The operation poll did not run while the tab was hidden, and the app's + * global `refetchOnWindowFocus: false` meant coming back to the tab did + * not refresh it either (T3a-T3c). + * 2. The automation job can finish before the inventory (catalog) record + * changes. The stage then claimed "Teardown of X completed." and offered + * Teardown again while the entry still read `active`, and nothing + * re-read the catalog until the next window focus (T1, T2, T4). + * 3. With two functions, each pending entry has to stay guarded until its own + * inventory record changes, whatever happens to the other entry's teardown + * (T5a, T5b), and the catalog poll has to keep running in a hidden tab (T6). + * + * jsdom and TanStack Query: @tanstack/query-core 5.100.7 listens only for + * `visibilitychange` on `window` and reads `document.visibilityState`. It no + * longer listens for `focus`, so the `focus` events dispatched below are + * what a browser also sends but are inert here; `visibilitychange` is the + * event that drives the refetch. jsdom reports `visible` by default, so the + * hidden state is simulated by redefining `document.visibilityState`. + */ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { MemoryRouter, Route, Routes } from 'react-router-dom' +import WorkloadSetup from '../pages/MediaWorkloads/WorkloadSetup' +import { OperationStatusLine } from '../pages/MediaWorkloads/stages/JobProgress' +import HeaderSlotProbe from './testUtils/HeaderSlotProbe' +import type { CatalogEntry, MediaWorkload, MediaWorkloadsGroupedResponse } from '../api/types' + +function catalogEntry(overrides: Partial = {}): CatalogEntry { + return { + key: 'crosspoint', + display_name: 'MXL Crosspoint', + summary: '', + ebu_layer: null, + ebu_vertical: null, + ebu_media_function_type: null, + ebu_lifecycle_owner: null, + lifecycle: 'active', + provision_image: null, + provision_netbox_service: null, + configure_awx_job_template: 'deploy-crosspoint', + finalise_awx_job_template: 'teardown-crosspoint', + dependencies: [], + ingress_url: null, + ...overrides, + } +} + +function json(body: unknown, status = 200) { + return new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } }) +} + +function operation(state: string) { + return { + operation_id: 'op-inv', action: 'teardown', target: 'crosspoint', state, + job_id: 9001, error: null, created_at: 't0', updated_at: 't1', + } +} + +// The same QueryClient defaults as main.tsx, except retry: main.tsx sets +// `retry: 1`; retries are off here so a failed read never hides behind one. +function newQueryClient() { + return new QueryClient({ defaultOptions: { queries: { retry: false, refetchOnWindowFocus: false } } }) +} + +interface Server { + catalogLifecycle: CatalogEntry['lifecycle'] + operationState: string + catalogReads: number + operationReads: number +} + +function stubServer(initial: Partial = {}): Server { + const server: Server = { + catalogLifecycle: 'active', + operationState: 'run_complete', + catalogReads: 0, + operationReads: 0, + ...initial, + } + const grouped: MediaWorkloadsGroupedResponse = { + configured: true, degraded: false, scope: [], invalid_instances: [], + workloads: [offFlowWorkload()], + } + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL) => { + const url = (typeof input === 'string' ? input : (input as Request).url).toString() + if (url.endsWith('/api/catalog')) { + server.catalogReads += 1 + return json({ entries: [catalogEntry({ lifecycle: server.catalogLifecycle })] }) + } + if (url.endsWith('/api/media-workloads/grouped')) return json(grouped) + if (url.match(/\/api\/catalog\/crosspoint\/teardown$/)) { + return json({ ...operation('waking'), job_id: null, request_id: 'req-inv' }) + } + if (url.endsWith('/api/operations/op-inv')) { + server.operationReads += 1 + return json(operation(server.operationState)) + } + return json({}) + }), + ) + return server +} + +const KEY_A = 'crosspoint' +const KEY_B = 'switch' + +interface TwoServer { + lifecycle: Record + outcome: Record +} + +// Two active functions, each torn down by its own operation (`op-`). +// Tearing an entry down moves its inventory record to `bootstrapped` only when +// the test says so, which is how a lagging inventory is simulated per entry. +function stubTwoFunctionServer(initial: Partial = {}): TwoServer { + const server: TwoServer = { + lifecycle: { [KEY_A]: 'active', [KEY_B]: 'active' }, + outcome: { [KEY_A]: 'run_complete', [KEY_B]: 'run_complete' }, + ...initial, + } + const workload: MediaWorkload = { + ...offFlowWorkload(), + functions: [ + { function_key: KEY_A, count: 1, running: 1, reconcile_pending: 0 }, + { function_key: KEY_B, count: 1, running: 1, reconcile_pending: 0 }, + ], + } + const grouped: MediaWorkloadsGroupedResponse = { + configured: true, degraded: false, scope: [], invalid_instances: [], workloads: [workload], + } + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL) => { + const url = (typeof input === 'string' ? input : (input as Request).url).toString() + if (url.endsWith('/api/catalog')) { + return json({ + entries: [ + catalogEntry({ key: KEY_A, display_name: 'MXL Crosspoint', lifecycle: server.lifecycle[KEY_A] }), + catalogEntry({ key: KEY_B, display_name: 'MXL Switch', lifecycle: server.lifecycle[KEY_B] }), + ], + }) + } + if (url.endsWith('/api/media-workloads/grouped')) return json(grouped) + const teardown = url.match(/\/api\/catalog\/([a-z]+)\/teardown$/) + if (teardown) { + const key = teardown[1] + return json({ + operation_id: `op-${key}`, action: 'teardown', target: key, state: 'waking', + job_id: null, error: null, created_at: 't0', updated_at: 't0', request_id: `req-${key}`, + }) + } + const op = url.match(/\/api\/operations\/op-([a-z]+)$/) + if (op) { + const key = op[1] + return json({ + operation_id: `op-${key}`, action: 'teardown', target: key, state: server.outcome[key], + job_id: 9001, error: server.outcome[key] === 'run_failed' ? 'ansible-timeout' : null, + created_at: 't0', updated_at: 't1', + }) + } + return json({}) + }), + ) + return server +} + +function waitingFor(key: string): RegExp { + return new RegExp(`The teardown job for ${key} finished\\. Waiting for the inventory to confirm\\.`) +} + +function teardownButtons(finalise: HTMLElement): HTMLElement[] { + return within(finalise).queryAllByRole('button', { name: /Teardown/ }) +} + +function offFlowWorkload(): MediaWorkload { + return { + slug: 'studio-a', + name: 'studio-a', + lifecycle: 'operate', + health: 'ok', + instances: [], + functions: [{ function_key: 'crosspoint', count: 1, running: 1, reconcile_pending: 0 }], + } +} + +function renderDetail(queryClient: QueryClient) { + return render( + + + + } /> + + + + , + ) +} + +function stageSection(label: string): HTMLElement { + return screen.getByRole('heading', { name: label, level: 2 }).closest('[data-step-state]') as HTMLElement +} + +async function mountFinalise(queryClient: QueryClient) { + const view = renderDetail(queryClient) + await screen.findByRole('navigation', { name: 'Media workload lifecycle' }) + const finalise = stageSection('Finalise & Review') + await within(finalise).findAllByRole('button', { name: /Teardown/ }) + return { view, finalise } +} + +function armAndConfirmTeardown(finalise: HTMLElement) { + fireEvent.click(within(finalise).getAllByRole('button', { name: /Teardown/ })[0]) + fireEvent.change(within(finalise).getAllByRole('textbox')[0], { target: { value: 'go' } }) + fireEvent.click(within(finalise).getByRole('button', { name: /Confirm teardown/ })) +} + +const WAITING = /The teardown job for crosspoint finished\. Waiting for the inventory to confirm\./ +const COMPLETED = 'Teardown of crosspoint completed.' + +function setTabHidden(hidden: boolean) { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: () => (hidden ? 'hidden' : 'visible'), + }) + document.dispatchEvent(new Event('visibilitychange', { bubbles: true })) + if (!hidden) window.dispatchEvent(new Event('focus')) +} + +afterEach(() => { + cleanup() + // Remove the own-property override so jsdom's `visible` default applies. + delete (document as { visibilityState?: unknown }).visibilityState + vi.restoreAllMocks() + vi.unstubAllGlobals() + vi.useRealTimers() +}) + +describe('Finalise & Review: the inventory has to confirm a finished teardown', () => { + it('T1: run_complete with the catalog still active shows the waiting line, not "completed", and no Teardown button', async () => { + stubServer({ catalogLifecycle: 'active', operationState: 'run_complete' }) + const { finalise } = await mountFinalise(newQueryClient()) + armAndConfirmTeardown(finalise) + + // The operation reaches run_complete on the first poll; the hand-off to + // the review is paced (about one second). + await waitFor(() => expect(finalise.textContent).toMatch(WAITING), { timeout: 4000 }) + expect(finalise.textContent).not.toContain(COMPLETED) + expect(within(finalise).queryByRole('button', { name: /Teardown/ })).toBeNull() + }, 20_000) + + it('T2: once the catalog stops reporting active, the stage confirms within 5 s with no focus event', async () => { + const server = stubServer({ catalogLifecycle: 'active', operationState: 'run_complete' }) + const { finalise } = await mountFinalise(newQueryClient()) + armAndConfirmTeardown(finalise) + await waitFor(() => expect(finalise.textContent).toMatch(WAITING), { timeout: 4000 }) + + server.catalogLifecycle = 'bootstrapped' + + await waitFor(() => expect(within(finalise).getByText('Not currently deployed.')).toBeTruthy(), { timeout: 5000 }) + expect(finalise.textContent).not.toMatch(WAITING) + expect(finalise.textContent).toContain(COMPLETED) + expect(within(finalise).queryByRole('button', { name: /Teardown/ })).toBeNull() + }, 20_000) + + it('T4: mount -> pending -> unmount while pending -> remount: no leaked poll, and the remount reads the catalog alone', async () => { + const server = stubServer({ catalogLifecycle: 'active', operationState: 'run_complete' }) + const queryClient = newQueryClient() + const first = await mountFinalise(queryClient) + armAndConfirmTeardown(first.finalise) + await waitFor(() => expect(first.finalise.textContent).toMatch(WAITING), { timeout: 4000 }) + + // Unmount while the inventory is still pending. + first.view.unmount() + const readsAtUnmount = server.catalogReads + // Longer than one 3 s poll interval: a leaked interval would read again. + await new Promise((resolve) => setTimeout(resolve, 3600)) + expect(server.catalogReads).toBe(readsAtUnmount) + + // Remount: the catalog still says active, and the stage must not remember + // the earlier pending state or the earlier outcome. + const second = await mountFinalise(queryClient) + expect(second.finalise.textContent).not.toMatch(WAITING) + expect(second.finalise.textContent).not.toContain(COMPLETED) + // The control is available again: a single use does not retire it. + fireEvent.click(within(second.finalise).getByRole('button', { name: /Teardown/ })) + expect(within(second.finalise).getByRole('button', { name: /Confirm teardown/ })).toBeTruthy() + + // The remounted stage is not polling either. + const readsAtRemount = server.catalogReads + await new Promise((resolve) => setTimeout(resolve, 3600)) + expect(server.catalogReads).toBe(readsAtRemount) + }, 30_000) +}) + +describe('Finalise & Review: two entries pending on the inventory are tracked independently', () => { + // Entry A finishes while its inventory record still says `active`; entry B + // is then torn down and reaches `outcomeB`. A must keep its guard, its + // waiting line and the catalog poll, until A's own record changes. + async function pendingAThenB(outcomeB: 'run_complete' | 'run_failed') { + const server = stubTwoFunctionServer() + const { finalise } = await mountFinalise(newQueryClient()) + armAndConfirmTeardown(finalise) + await waitFor(() => expect(finalise.textContent).toMatch(waitingFor(KEY_A)), { timeout: 4000 }) + // A's button is withheld, so the only Teardown button now belongs to B. + expect(teardownButtons(finalise)).toHaveLength(1) + + server.outcome[KEY_B] = outcomeB + if (outcomeB === 'run_complete') server.lifecycle[KEY_B] = 'bootstrapped' + armAndConfirmTeardown(finalise) + await waitFor( + () => expect(finalise.textContent).toMatch( + outcomeB === 'run_complete' ? /Teardown of switch completed\./ : /Teardown of switch did not complete/, + ), + { timeout: 4000 }, + ) + return { server, finalise } + } + + function entryOf(button: HTMLElement): string { + return button.closest('.border-t')?.textContent ?? '' + } + + it('T5a: B succeeding while A is still active leaves A guarded, then A confirms', async () => { + const { server, finalise } = await pendingAThenB('run_complete') + + expect(finalise.textContent).toMatch(waitingFor(KEY_A)) + expect(teardownButtons(finalise)).toHaveLength(0) + + server.lifecycle[KEY_A] = 'bootstrapped' + await waitFor(() => expect(finalise.textContent).not.toMatch(waitingFor(KEY_A)), { timeout: 5000 }) + expect(within(finalise).getAllByText('Not currently deployed.')).toHaveLength(2) + expect(teardownButtons(finalise)).toHaveLength(0) + }, 30_000) + + it('T5b: B failing while A is still active leaves A guarded, then A confirms', async () => { + const { server, finalise } = await pendingAThenB('run_failed') + + expect(finalise.textContent).toMatch(waitingFor(KEY_A)) + // Only B, whose teardown failed and which is still active, offers Teardown. + const buttons = teardownButtons(finalise) + expect(buttons).toHaveLength(1) + expect(entryOf(buttons[0])).toContain('MXL Switch') + + server.lifecycle[KEY_A] = 'bootstrapped' + await waitFor(() => expect(finalise.textContent).not.toMatch(waitingFor(KEY_A)), { timeout: 5000 }) + expect(within(finalise).getAllByText('Not currently deployed.')).toHaveLength(1) + expect(teardownButtons(finalise)).toHaveLength(1) + expect(entryOf(teardownButtons(finalise)[0])).toContain('MXL Switch') + }, 30_000) +}) + +describe('Finalise & Review: the inventory poll runs in a hidden tab', () => { + it('T6: run_complete while hidden, then the catalog flips while still hidden: the stage confirms without the tab returning', async () => { + const server = stubServer({ catalogLifecycle: 'active', operationState: 'running' }) + const { finalise } = await mountFinalise(newQueryClient()) + armAndConfirmTeardown(finalise) + await within(finalise).findByText('Running', {}, { timeout: 4000 }) + + setTabHidden(true) + server.operationState = 'run_complete' + await waitFor(() => expect(finalise.textContent).toMatch(WAITING), { timeout: 8000 }) + + server.catalogLifecycle = 'bootstrapped' + const readsWhenFlipped = server.catalogReads + await waitFor(() => expect(within(finalise).getByText('Not currently deployed.')).toBeTruthy(), { timeout: 6000 }) + expect(server.catalogReads).toBeGreaterThan(readsWhenFlipped) + expect(finalise.textContent).not.toMatch(WAITING) + }, 40_000) +}) + +describe('useOperationStatus: a hidden tab does not delay the outcome', () => { + function renderLine() { + const queryClient = newQueryClient() + render( + + {}} onError={() => {}} /> + , + ) + } + + it('T3a: becoming visible again fetches the outcome faster than the next 3 s tick', async () => { + const server = stubServer({ operationState: 'running' }) + renderLine() + await screen.findByText('Running') + const seenAt = performance.now() + + setTabHidden(true) + server.operationState = 'run_complete' + setTabHidden(false) + + await screen.findByText('Complete', {}, { timeout: 1500 }) + // The interval tick is 3 s after the first read; this arrived well before. + expect(performance.now() - seenAt).toBeLessThan(2500) + }, 20_000) + + it('T3b: polling continues while the tab stays hidden', async () => { + const server = stubServer({ operationState: 'running' }) + renderLine() + await screen.findByText('Running') + + setTabHidden(true) + server.operationState = 'run_complete' + + await screen.findByText('Complete', {}, { timeout: 5000 }) + }, 20_000) + + it('T3c: a settled operation is not refetched when the tab becomes visible', async () => { + const server = stubServer({ operationState: 'run_complete' }) + renderLine() + await screen.findByText('Complete') + const reads = server.operationReads + + setTabHidden(true) + setTabHidden(false) + await new Promise((resolve) => setTimeout(resolve, 300)) + + expect(server.operationReads).toBe(reads) + }) +}) diff --git a/frontend/src/__tests__/operationTerminalStall.test.tsx b/frontend/src/__tests__/operationTerminalStall.test.tsx index e3266b8..0d8d630 100644 --- a/frontend/src/__tests__/operationTerminalStall.test.tsx +++ b/frontend/src/__tests__/operationTerminalStall.test.tsx @@ -112,13 +112,20 @@ describe('Finalise & Review: a teardown operation that never passes through `lau const grouped: MediaWorkloadsGroupedResponse = { configured: true, degraded: false, scope: [], workloads: [wl], invalid_instances: [], } + // The inventory records the teardown as soon as the job is launched, as + // it does once it catches up. While it still said `active` after + // run_complete, the stage would (correctly) hold back "completed". + let tornDown = false vi.stubGlobal( 'fetch', vi.fn(async (input: RequestInfo | URL) => { const url = (typeof input === 'string' ? input : (input as Request).url).toString() - if (url.endsWith('/api/catalog')) return json({ entries: [catalogEntry()] }) + if (url.endsWith('/api/catalog')) { + return json({ entries: [catalogEntry({ lifecycle: tornDown ? 'bootstrapped' : 'active' })] }) + } if (url.endsWith('/api/media-workloads/grouped')) return json(grouped) if (url.match(/\/api\/catalog\/crosspoint\/teardown$/)) { + tornDown = true return json({ operation_id: 'op-t1', action: 'teardown', target: 'crosspoint', state: 'waking', job_id: null, error: null, created_at: 't0', updated_at: 't0', request_id: 'req-t1', diff --git a/frontend/src/api/hooks.ts b/frontend/src/api/hooks.ts index 0bfa050..abf78c7 100644 --- a/frontend/src/api/hooks.ts +++ b/frontend/src/api/hooks.ts @@ -138,18 +138,30 @@ export const _WATCHED_TERMINAL_STATES: OperationState[] = [ 'run_complete', 'run_failed', 'failed_rollback_required', 'rollback_incomplete', 'run_status_unknown', ] +// True once polling an operation can no longer change what the page shows. +function isOperationSettled(data: Operation | undefined): boolean { + if (!data) return false + if (data.state === 'error') return true + if (data.action === 'launch' && data.state === 'launched') return true + return _WATCHED_TERMINAL_STATES.includes(data.state) +} + export function useOperationStatus(operationId: string | null) { return useQuery({ queryKey: ['operation', operationId], queryFn: () => apiCall(`/api/operations/${operationId}`), enabled: operationId !== null, + // TanStack skips interval ticks while the tab is hidden. A teardown takes + // minutes, so the operator often switches away, and the outcome would then + // wait for the tab to come back and show up late. + refetchIntervalInBackground: true, + // The app turns window-focus refetching off globally. An operation that is + // still running needs it on, so returning to the tab refreshes the status + // at once instead of waiting for the next tick. + refetchOnWindowFocus: (query) => !isOperationSettled(query.state.data), refetchInterval: (query) => { // Stop polling when operation reaches terminal state - const data = query.state.data - if (!data) return 3000 - if (data.state === 'error') return false - if (data.action === 'launch' && data.state === 'launched') return false - if (_WATCHED_TERMINAL_STATES.includes(data.state)) return false + if (isOperationSettled(query.state.data)) return false return 3000 // Poll every 3s for non-terminal states }, }) @@ -349,12 +361,25 @@ export function useAdminGroups() { // Catalog (Media Functions) // ------------------------------------------------------------------ -export function useCatalog() { +const NO_KEYS: readonly string[] = [] + +// `awaitingKeys` names entries whose automation job has finished but whose +// inventory record may not have caught up. While any of them still reports +// `active`, the catalog is re-read every 3 s, tab hidden or not: a hidden tab +// would otherwise skip every tick and a return within the stale time would not +// refetch either. The poll stops once none of them reads `active`, or when the +// component that passed the keys unmounts. +export function useCatalog(awaitingKeys: readonly string[] = NO_KEYS) { return useQuery({ queryKey: ['catalog'], queryFn: () => apiCall('/api/catalog'), refetchOnWindowFocus: true, staleTime: 30_000, + refetchIntervalInBackground: true, + refetchInterval: (query) => { + const entries = query.state.data?.entries ?? [] + return entries.some((e) => awaitingKeys.includes(e.key) && e.lifecycle === 'active') ? 3000 : false + }, }) } diff --git a/frontend/src/pages/MediaWorkloads/stages/FinaliseStage.tsx b/frontend/src/pages/MediaWorkloads/stages/FinaliseStage.tsx index 2f92d4f..cb4fe98 100644 --- a/frontend/src/pages/MediaWorkloads/stages/FinaliseStage.tsx +++ b/frontend/src/pages/MediaWorkloads/stages/FinaliseStage.tsx @@ -134,12 +134,18 @@ export default function FinaliseStage({ */ runningReadout: AutomationRunningReadout }) { + // `awaitingKeys` are the entries whose teardown job finished but whose + // inventory record may still read `active`: while any does, the catalog is + // polled (see useCatalog). Each key is tracked on its own, so a later + // teardown of another entry never releases it. It is component state on + // purpose, so a remount starts from the catalog alone. + const [awaitingKeys, setAwaitingKeys] = useState([]) // `catalogFailed` threaded into // the absence claim below — see ProvisionStage.tsx's identical fix for // the full reasoning (same shared PlanStage/CreateWorkload precedent). // // (umbrella #385) retrofitted onto settleQuery. - const { data: catalogData, loading: catalogLoading, failed: catalogFailed } = settleQuery(useCatalog()) + const { data: catalogData, loading: catalogLoading, failed: catalogFailed } = settleQuery(useCatalog(awaitingKeys)) const guardWrite = useGuardWrite() const teardownMutation = useTeardownCatalog() const purgeMutation = usePurgeWorkload() @@ -247,6 +253,26 @@ export default function FinaliseStage({ const entries = (catalogData?.entries ?? []).filter((e) => functionKeys.includes(e.key)) + // A key leaves the list the moment its own entry stops reporting as active. + // Clearing the key then, not just hiding the text, keeps a later redeploy of + // the same entry from reading as a teardown still unconfirmed. + const stillActive = (key: string) => + catalogData?.entries.some((e) => e.key === key && e.lifecycle === 'active') ?? false + const isAwaitingInventory = (key: string) => awaitingKeys.includes(key) && stillActive(key) + const awaitingLine = (key: string) => ( +

+ The teardown job for {key} finished. + Waiting for the inventory to confirm. +

+ ) + useEffect(() => { + if (!catalogData) return + setAwaitingKeys((prev) => { + const next = prev.filter((key) => catalogData.entries.some((e) => e.key === key && e.lifecycle === 'active')) + return next.length === prev.length ? prev : next + }) + }, [catalogData]) + const allowed = actions.includes('tear-down') const purgeAllowed = actions.includes('delete-permanently') @@ -292,6 +318,11 @@ export default function FinaliseStage({ const handleOpTerminal = (key: string, operation: Operation) => { setTrack((prev) => ({ ...prev, [key]: EMPTY_TRACK })) setLastOpOutcome({ entryKey: key, operation }) + // Only this entry's own outcome changes its own pending state. + setAwaitingKeys((prev) => { + const others = prev.filter((k) => k !== key) + return operation.state === 'run_complete' ? [...others, key] : others + }) void queryClient.invalidateQueries({ queryKey: ['media-workloads-grouped'] }) void queryClient.invalidateQueries({ queryKey: ['catalog'] }) } @@ -347,6 +378,7 @@ export default function FinaliseStage({ entry={entry} allowed={allowed} track={entryTrack} + awaitingInventory={isAwaitingInventory(entry.key)} isTearingDown={teardownMutation.isPending && teardownMutation.variables?.key === entry.key} teardownError={teardownMutation.variables?.key === entry.key ? teardownMutation.error : null} onTeardown={(reason) => handleTeardown(entry, reason)} @@ -506,7 +538,9 @@ export default function FinaliseStage({ )} {lastOpOutcome && (
- {lastOpOutcome.operation.state === 'run_complete' ? ( + {lastOpOutcome.operation.state === 'run_complete' && isAwaitingInventory(lastOpOutcome.entryKey) ? ( + awaitingLine(lastOpOutcome.entryKey) + ) : lastOpOutcome.operation.state === 'run_complete' ? (

Teardown of {lastOpOutcome.entryKey} completed.

@@ -520,6 +554,15 @@ export default function FinaliseStage({ )}
)} + {/* The last outcome names one entry; any other entry still + waiting on the inventory keeps its own line. */} + {awaitingKeys + .filter((key) => key !== lastOpOutcome?.entryKey && isAwaitingInventory(key)) + .map((key) => ( +
+ {awaitingLine(key)} +
+ ))} {lastSwitchResult && (

@@ -577,6 +620,7 @@ function FinaliseEntry({ entry, allowed, track, + awaitingInventory, isTearingDown, teardownError, onTeardown, @@ -590,6 +634,9 @@ function FinaliseEntry({ entry: CatalogEntry allowed: boolean track: EntryTrack + // The teardown job finished but the inventory still lists the entry as + // active, so offering Teardown again would act on a stale record. + awaitingInventory: boolean isTearingDown: boolean teardownError: unknown onTeardown: (reason: string) => Promise @@ -663,7 +710,7 @@ function FinaliseEntry({ difference trying to encode both (which is also what btn-danger-outline vs btn-secondary encoded no better before §D2 — visually identical to "← Previous"). */} - {allowed && !inFlight && entry.lifecycle === 'active' && !arming && ( + {allowed && !inFlight && !awaitingInventory && entry.lifecycle === 'active' && !arming && ( diff --git a/frontend/src/pages/MediaWorkloads/stages/JobProgress.tsx b/frontend/src/pages/MediaWorkloads/stages/JobProgress.tsx index 7073e39..a5055f6 100644 --- a/frontend/src/pages/MediaWorkloads/stages/JobProgress.tsx +++ b/frontend/src/pages/MediaWorkloads/stages/JobProgress.tsx @@ -95,8 +95,7 @@ export function OperationStatusLine({ * for a watched action — `launched` (what onLaunched depends on) is * transient, and a watched action keeps running right past it toward a * real terminal state whether or not any poll happened to land during - * that window (a missed tick, a backgrounded tab — refetchInterval does - * not run there). Optional: existing callers, and + * that window (a missed tick or a failed read). Optional: existing callers, and * jobProgressHonesty.test.tsx, only ever needed onLaunched/onError. */ onTerminal?: (operation: Operation) => void From 80b430fb617ed5c7e7a71ef6990dc0f9c19121d6 Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Sat, 7 Nov 2026 23:24:09 +0100 Subject: [PATCH 2/2] fix(console): give the job-ID teardown path the same confirmation When the teardown operation hands off to its automation job, the page polls the job instead. That poll stopped in a hidden tab, and a successful job never waited for the inventory, so the stage could offer Teardown again while the entry still read active. The job status now polls in the background and refetches on focus until it's done. A successful teardown job adds its entry to the pending entries, as run_complete does, and a failed job releases only its own entry. The cleanup effect also reruns when the pending entries change, so an entry added after the inventory already caught up doesn't linger and hold back a later redeploy. Refs dmfdeploy/dmfdeploy#862 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- .../finaliseInventoryConfirm.test.tsx | 147 +++++++++++++++++- frontend/src/api/hooks.ts | 5 + .../MediaWorkloads/stages/FinaliseStage.tsx | 28 +++- .../MediaWorkloads/stages/JobProgress.tsx | 9 +- 4 files changed, 176 insertions(+), 13 deletions(-) diff --git a/frontend/src/__tests__/finaliseInventoryConfirm.test.tsx b/frontend/src/__tests__/finaliseInventoryConfirm.test.tsx index be71bde..399da70 100644 --- a/frontend/src/__tests__/finaliseInventoryConfirm.test.tsx +++ b/frontend/src/__tests__/finaliseInventoryConfirm.test.tsx @@ -12,6 +12,10 @@ * 3. With two functions, each pending entry has to stay guarded until its own * inventory record changes, whatever happens to the other entry's teardown * (T5a, T5b), and the catalog poll has to keep running in a hidden tab (T6). + * 4. A teardown can also hand off from the operation to its automation job + * (`launched` with a job id). That path has to reach the same inventory + * confirmation (T7a, T7b, T7c) and has to keep polling in a hidden tab + * (T8a, T8b). * * jsdom and TanStack Query: @tanstack/query-core 5.100.7 listens only for * `visibilitychange` on `window` and reads `document.visibilityState`. It no @@ -21,11 +25,11 @@ * hidden state is simulated by redefining `document.visibilityState`. */ import { afterEach, describe, expect, it, vi } from 'vitest' -import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { MemoryRouter, Route, Routes } from 'react-router-dom' import WorkloadSetup from '../pages/MediaWorkloads/WorkloadSetup' -import { OperationStatusLine } from '../pages/MediaWorkloads/stages/JobProgress' +import { JobStatusLine, OperationStatusLine } from '../pages/MediaWorkloads/stages/JobProgress' import HeaderSlotProbe from './testUtils/HeaderSlotProbe' import type { CatalogEntry, MediaWorkload, MediaWorkloadsGroupedResponse } from '../api/types' @@ -170,6 +174,55 @@ function stubTwoFunctionServer(initial: Partial = {}): TwoServer { return server } +interface JobServer { + catalogLifecycle: CatalogEntry['lifecycle'] + jobStatus: string + jobDone: boolean + catalogReads: number + jobReads: number +} + +// The `launched` -> job-id path: the teardown operation reports `launched` +// with a job id (and never moves on), and the job itself is polled by id. +function stubJobPathServer(initial: Partial = {}): JobServer { + const server: JobServer = { + catalogLifecycle: 'active', + jobStatus: 'successful', + jobDone: true, + catalogReads: 0, + jobReads: 0, + ...initial, + } + const grouped: MediaWorkloadsGroupedResponse = { + configured: true, degraded: false, scope: [], invalid_instances: [], + workloads: [offFlowWorkload()], + } + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL) => { + const url = (typeof input === 'string' ? input : (input as Request).url).toString() + if (url.endsWith('/api/catalog')) { + server.catalogReads += 1 + return json({ entries: [catalogEntry({ lifecycle: server.catalogLifecycle })] }) + } + if (url.endsWith('/api/media-workloads/grouped')) return json(grouped) + if (url.match(/\/api\/catalog\/crosspoint\/teardown$/)) { + return json({ ...operation('waking'), job_id: null, request_id: 'req-inv' }) + } + if (url.endsWith('/api/operations/op-inv')) return json({ ...operation('launched'), job_id: 7001 }) + if (url.match(/\/api\/catalog\/crosspoint\/status\/7001$/)) { + server.jobReads += 1 + return json({ + job_id: 7001, status: server.jobStatus, is_done: server.jobDone, + is_running: !server.jobDone, + }) + } + return json({}) + }), + ) + return server +} + function waitingFor(key: string): RegExp { return new RegExp(`The teardown job for ${key} finished\\. Waiting for the inventory to confirm\\.`) } @@ -374,6 +427,75 @@ describe('Finalise & Review: the inventory poll runs in a hidden tab', () => { }, 40_000) }) +describe('Finalise & Review: the launched -> job-id path confirms against the inventory too', () => { + it('T7a: a successful job with the catalog still active shows the waiting line and no Teardown button, then confirms', async () => { + const server = stubJobPathServer({ catalogLifecycle: 'active', jobStatus: 'successful', jobDone: true }) + const { finalise } = await mountFinalise(newQueryClient()) + armAndConfirmTeardown(finalise) + + // Hand-off to the job (about 1 s), then the paced completion (2 s). + await waitFor(() => expect(finalise.textContent).toMatch(waitingFor('crosspoint')), { timeout: 10_000 }) + expect(within(finalise).queryByRole('button', { name: /Teardown/ })).toBeNull() + + server.catalogLifecycle = 'bootstrapped' + await waitFor(() => expect(within(finalise).getByText('Not currently deployed.')).toBeTruthy(), { timeout: 5000 }) + expect(finalise.textContent).not.toMatch(waitingFor('crosspoint')) + expect(within(finalise).queryByRole('button', { name: /Teardown/ })).toBeNull() + }, 40_000) + + it('T7b: a failed job never claims the inventory is pending, and offers Teardown again', async () => { + stubJobPathServer({ catalogLifecycle: 'active', jobStatus: 'failed', jobDone: true }) + const { finalise } = await mountFinalise(newQueryClient()) + armAndConfirmTeardown(finalise) + + await waitFor(() => expect(finalise.textContent).toMatch(/Last job: #7001/), { timeout: 10_000 }) + // The in-flight tracker clears (the Teardown button returns) once the job completes. + await within(finalise).findByRole('button', { name: /Teardown/ }, { timeout: 5000 }) + expect(finalise.textContent).not.toMatch(waitingFor('crosspoint')) + }, 40_000) + + it('T7c: a pending key is released even when the catalog already read non-active, so a later redeploy is not held back', async () => { + const server = stubJobPathServer({ catalogLifecycle: 'active', jobStatus: 'running', jobDone: false }) + const queryClient = newQueryClient() + const refreshCatalog = () => act(() => queryClient.invalidateQueries({ queryKey: ['catalog'] })) + const { finalise } = await mountFinalise(queryClient) + armAndConfirmTeardown(finalise) + await within(finalise).findByText(/job #7001/, {}, { timeout: 5000 }) + + // The inventory catches up before the job is reported done, and the stage + // reads it. When the job then finishes, the catalog read it triggers is + // identical to the one already held, so the stage's data does not change. + server.catalogLifecycle = 'bootstrapped' + await refreshCatalog() + server.jobStatus = 'successful' + server.jobDone = true + await waitFor(() => expect(within(finalise).getByText('Not currently deployed.')).toBeTruthy(), { timeout: 10_000 }) + expect(finalise.textContent).not.toMatch(waitingFor('crosspoint')) + + // The entry is deployed again. Nothing about the finished teardown may + // still hold it back. + server.catalogLifecycle = 'active' + await refreshCatalog() + await within(finalise).findByRole('button', { name: /Teardown/ }, { timeout: 5000 }) + expect(finalise.textContent).not.toMatch(waitingFor('crosspoint')) + }, 40_000) + + it('T8a: the job finishes while the tab is hidden: the stage still reaches the waiting line, then confirms while hidden', async () => { + const server = stubJobPathServer({ catalogLifecycle: 'active', jobStatus: 'running', jobDone: false }) + const { finalise } = await mountFinalise(newQueryClient()) + armAndConfirmTeardown(finalise) + await within(finalise).findByText(/job #7001/, {}, { timeout: 5000 }) + + setTabHidden(true) + server.jobStatus = 'successful' + server.jobDone = true + await waitFor(() => expect(finalise.textContent).toMatch(waitingFor('crosspoint')), { timeout: 10_000 }) + + server.catalogLifecycle = 'bootstrapped' + await waitFor(() => expect(within(finalise).getByText('Not currently deployed.')).toBeTruthy(), { timeout: 6000 }) + }, 40_000) +}) + describe('useOperationStatus: a hidden tab does not delay the outcome', () => { function renderLine() { const queryClient = newQueryClient() @@ -423,3 +545,24 @@ describe('useOperationStatus: a hidden tab does not delay the outcome', () => { expect(server.operationReads).toBe(reads) }) }) + +describe('useCatalogJobStatus: a hidden tab does not delay the job outcome', () => { + it('T8b: becoming visible again fetches the job outcome faster than the next 2 s tick', async () => { + const server = stubJobPathServer({ jobStatus: 'running', jobDone: false }) + render( + + {}} /> + , + ) + await screen.findByText(/Running/) + const reads = server.jobReads + + setTabHidden(true) + server.jobStatus = 'successful' + server.jobDone = true + setTabHidden(false) + + await screen.findByText('Successful', {}, { timeout: 1500 }) + expect(server.jobReads).toBeGreaterThan(reads) + }, 20_000) +}) diff --git a/frontend/src/api/hooks.ts b/frontend/src/api/hooks.ts index abf78c7..ae0f8eb 100644 --- a/frontend/src/api/hooks.ts +++ b/frontend/src/api/hooks.ts @@ -412,6 +412,11 @@ export function useCatalogJobStatus(key: string, jobId: number | null) { queryKey: ['catalog-job', key, jobId], queryFn: () => apiCall(`/api/catalog/${key}/status/${jobId}`), enabled: jobId !== null, + // Same reasoning as useOperationStatus: a teardown outlasts the time an + // operator stays on the tab, so the poll keeps running while it is hidden, + // and returning to a job that is still running refreshes it at once. + refetchIntervalInBackground: true, + refetchOnWindowFocus: (query) => !query.state.data?.is_done, refetchInterval: 2000, }) } diff --git a/frontend/src/pages/MediaWorkloads/stages/FinaliseStage.tsx b/frontend/src/pages/MediaWorkloads/stages/FinaliseStage.tsx index cb4fe98..9e2f7b4 100644 --- a/frontend/src/pages/MediaWorkloads/stages/FinaliseStage.tsx +++ b/frontend/src/pages/MediaWorkloads/stages/FinaliseStage.tsx @@ -271,7 +271,7 @@ export default function FinaliseStage({ const next = prev.filter((key) => catalogData.entries.some((e) => e.key === key && e.lifecycle === 'active')) return next.length === prev.length ? prev : next }) - }, [catalogData]) + }, [catalogData, awaitingKeys]) const allowed = actions.includes('tear-down') const purgeAllowed = actions.includes('delete-permanently') @@ -301,8 +301,15 @@ export default function FinaliseStage({ })) } - const handleJobComplete = (key: string) => { + const handleJobComplete = (key: string, jobStatus: string) => { setTrack((prev) => ({ ...prev, [key]: EMPTY_TRACK })) + // A finished job is the same news as a `run_complete` operation: the + // inventory still has to confirm it. A failed job releases only its own + // entry. + setAwaitingKeys((prev) => { + const others = prev.filter((k) => k !== key) + return jobStatus === 'successful' ? [...others, key] : others + }) void queryClient.invalidateQueries({ queryKey: ['media-workloads-grouped'] }) void queryClient.invalidateQueries({ queryKey: ['catalog'] }) } @@ -395,7 +402,7 @@ export default function FinaliseStage({ onOpError={() => setTrack((prev) => ({ ...prev, [entry.key]: EMPTY_TRACK }))} onOpTerminal={(operation) => handleOpTerminal(entry.key, operation)} onStatusChange={statusCallbackFor(entry.key)} - onJobComplete={() => handleJobComplete(entry.key)} + onJobComplete={(status) => handleJobComplete(entry.key, status)} runningReadout={runningReadout} /> ) @@ -554,10 +561,15 @@ export default function FinaliseStage({ )}

)} - {/* The last outcome names one entry; any other entry still - waiting on the inventory keeps its own line. */} + {/* The last operation outcome renders its own waiting line; + any other entry still waiting on the inventory, including + one that finished through the job-id path, keeps its own. */} {awaitingKeys - .filter((key) => key !== lastOpOutcome?.entryKey && isAwaitingInventory(key)) + .filter( + (key) => + !(lastOpOutcome?.entryKey === key && lastOpOutcome.operation.state === 'run_complete') && + isAwaitingInventory(key), + ) .map((key) => (
{awaitingLine(key)} @@ -644,7 +656,7 @@ function FinaliseEntry({ onOpError: () => void onOpTerminal: (operation: Operation) => void onStatusChange: (status: string) => void - onJobComplete: () => void + onJobComplete: (status: string) => void runningReadout: AutomationRunningReadout }) { const [arming, setArming] = useState(false) @@ -803,7 +815,7 @@ function FinaliseEntry({ onJobComplete(status)} onStatusChange={onStatusChange} />
diff --git a/frontend/src/pages/MediaWorkloads/stages/JobProgress.tsx b/frontend/src/pages/MediaWorkloads/stages/JobProgress.tsx index a5055f6..a0a7d89 100644 --- a/frontend/src/pages/MediaWorkloads/stages/JobProgress.tsx +++ b/frontend/src/pages/MediaWorkloads/stages/JobProgress.tsx @@ -232,7 +232,9 @@ export function JobStatusLine({ }: { entryKey: string jobId: number - onComplete: (key: string) => void + // `status` is the job's final status, so a caller can tell success from + // failure without keeping its own copy of it. + onComplete: (key: string, status: string) => void onStatusChange?: (status: string) => void onDoneChange?: (done: boolean) => void }) { @@ -251,9 +253,10 @@ export function JobStatusLine({ useEffect(() => { if (!jobStatus?.is_done) return - const timer = setTimeout(() => onComplete(entryKey), 2000) + const finalStatus = jobStatus.status + const timer = setTimeout(() => onComplete(entryKey, finalStatus), 2000) return () => clearTimeout(timer) - }, [jobStatus?.is_done, onComplete, entryKey]) + }, [jobStatus?.is_done, jobStatus?.status, onComplete, entryKey]) if (!jobStatus) { return (