fix(print): stale-context guard, friendly 404, deferred revoke, Clear button

Addresses PR #303 review (SScholefield + Rylan-cgi):

- Stale-context guard: handleGenerate captures the dispatch-time isCurrent()
  and gates the download + success/error banners + busy release on it, so
  switching mill/year mid-render can't download the old context's PDF or
  repaint a "Done" banner under the new context. A reset-on-context-change
  effect releases the lock/banners for the new context.
- Friendly 404: a valid mill/year with no rows in the ticked schedules
  (404 ERR-005) now shows "No data to print for the selected schedules."
  instead of the verbatim "Schedule not found."; 400/409 keep the verbatim
  problem+json detail (ERR-002/003/004).
- triggerDownload defers URL.revokeObjectURL via setTimeout(…, 0) so
  Firefox/Safari don't cancel the download by revoking in the click's task;
  drops the now-unneeded appendChild/remove. Test uses fake timers.
- Clear button resets to the S06 default (Schedule Information re-checked,
  everything else cleared); Schedule Information now defaults checked on load.

Tests: added 404-message, stale-context-no-download, and Clear-reset cases;
guard mocked directly to control isCurrent/contextMissing. Suite 821 passing,
lint clean, build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: gpascucci

frontend/src/components/printSchedules/__tests__/PrintSchedules.test.tsx

@@ -9,10 +9,17 @@ import PrintSchedules from '../index'
 
 const PRINT_URL = 'http://localhost:3000/api/v1/reports/print'
 
-// Working mill/year comes from the mill/year context; drive it directly so the page under test does
-// not depend on the provider or a mill-context fetch.
-const ctx = vi.hoisted(() => ({ millId: 514 as number | null, year: 2021 as number | null }))
-vi.mock('@/context/millYear/useMillYear', () => ({ default: () => ({ ...ctx }) }))
+// Drive the mill/year context guard directly so the page under test does not depend on the provider or
+// a mill-context fetch — and so `isCurrent` / `contextMissing` can be controlled per test.
+const guard = vi.hoisted(() => ({
+  millId: 514 as number | null,
+  year: 2021 as number | null,
+  contextMissing: false,
+  isCurrent: () => true,
+}))
+vi.mock('@/hooks/useScheduleContextGuard', () => ({
+  useScheduleContextGuard: () => guard,
+}))
 
 // The tombstone self-sources the working context (and would fetch /v1/mill-context); stub it — this
 // suite is about the selection form + download, not the header.
@@ -27,8 +34,10 @@ vi.mock('@/utils/download', async (importOriginal) => ({
 }))
 
 beforeEach(() => {
-  ctx.millId = 514
-  ctx.year = 2021
+  guard.millId = 514
+  guard.year = 2021
+  guard.contextMissing = false
+  guard.isCurrent = () => true
   vi.mocked(triggerDownload).mockReset()
 })
 
@@ -37,21 +46,21 @@ afterEach(() => {
 })
 
 describe('PrintSchedules', () => {
-  it('renders schedules/options with the deferred ones disabled ("coming soon"), Generate disabled', () => {
+  it('renders schedules/options (deferred disabled, Schedule information default-checked), Generate disabled', () => {
     render(<PrintSchedules />)
     expect(screen.getByRole('checkbox', { name: 'Select all schedules' })).toBeInTheDocument()
-    // Renderable schedules + content options are enabled.
     expect(screen.getByRole('checkbox', { name: 'Schedule 7A' })).toBeEnabled()
     expect(screen.getByRole('checkbox', { name: 'Schedule 11' })).toBeEnabled()
-    expect(screen.getByRole('checkbox', { name: 'Comments' })).toBeEnabled()
+    // S06 default: Schedule Information is pre-checked.
+    expect(screen.getByRole('checkbox', { name: 'Schedule information' })).toBeChecked()
     // Deferred schedules + the Mill info report are shown but disabled with a coming-soon note.
     expect(screen.getByRole('checkbox', { name: 'Schedule 1 (coming soon)' })).toBeDisabled()
-    expect(screen.getByRole('checkbox', { name: 'Schedule 8 (coming soon)' })).toBeDisabled()
     expect(
       screen.getByRole('checkbox', { name: 'Mill information report (coming soon)' }),
     ).toBeDisabled()
-    // Nothing selected yet → Generate is disabled.
+    // No schedule selected yet → Generate is disabled; Clear is available.
     expect(screen.getByRole('button', { name: /Generate PDF/ })).toBeDisabled()
+    expect(screen.getByRole('button', { name: 'Clear' })).toBeInTheDocument()
   })
 
   it('"Select all schedules" checks only the renderable schedules', async () => {
@@ -60,7 +69,6 @@ describe('PrintSchedules', () => {
     for (const label of ['Schedule 5', 'Schedule 7B', 'Schedule 9', 'Schedule 11']) {
       expect(screen.getByRole('checkbox', { name: label })).toBeChecked()
     }
-    // A deferred schedule stays disabled and unchecked.
     expect(screen.getByRole('checkbox', { name: 'Schedule 1 (coming soon)' })).not.toBeChecked()
   })
 
@@ -82,29 +90,89 @@ describe('PrintSchedules', () => {
 
     await waitFor(() => expect(vi.mocked(triggerDownload)).toHaveBeenCalledTimes(1))
     expect(screen.getByText(/generated and downloaded/i)).toBeInTheDocument()
-    expect(sentBody).toMatchObject({ schedule5: true, printComments: true, allSchedules: false })
+    expect(sentBody).toMatchObject({
+      schedule5: true,
+      printComments: true,
+      printScheduleInformation: true,
+      allSchedules: false,
+    })
     expect(vi.mocked(triggerDownload).mock.calls[0][1]).toBe('schedules_print.pdf')
   })
 
-  it('shows the server error detail when the selection is rejected', async () => {
+  it('passes through the verbatim server detail for a 400 rejection', async () => {
     server.use(
       http.post(PRINT_URL, () =>
         HttpResponse.json({ detail: 'Select at least one print option.' }, { status: 400 }),
       ),
     )
     render(<PrintSchedules />)
 
+    // Schedule Information is on by default, so ticking a schedule is enough to enable Generate.
     await userEvent.click(screen.getByRole('checkbox', { name: 'Schedule 5' }))
-    await userEvent.click(screen.getByRole('checkbox', { name: 'Schedule information' }))
     await userEvent.click(screen.getByRole('button', { name: /Generate PDF/ }))
 
     expect(await screen.findByText('Select at least one print option.')).toBeInTheDocument()
     expect(vi.mocked(triggerDownload)).not.toHaveBeenCalled()
   })
 
+  it('shows a friendly message (not "Schedule not found") when the selection has no data (404)', async () => {
+    server.use(
+      http.post(PRINT_URL, () =>
+        HttpResponse.json({ detail: 'Schedule not found.' }, { status: 404 }),
+      ),
+    )
+    render(<PrintSchedules />)
+
+    await userEvent.click(screen.getByRole('checkbox', { name: 'Schedule 5' }))
+    await userEvent.click(screen.getByRole('button', { name: /Generate PDF/ }))
+
+    expect(
+      await screen.findByText('No data to print for the selected schedules.'),
+    ).toBeInTheDocument()
+    expect(screen.queryByText('Schedule not found.')).toBeNull()
+    expect(vi.mocked(triggerDownload)).not.toHaveBeenCalled()
+  })
+
+  it('ignores a stale response when the mill/year context changed mid-render (no download)', async () => {
+    let handled = false
+    server.use(
+      http.post(PRINT_URL, () => {
+        handled = true
+        return new HttpResponse(new Blob(['%PDF-1.4 mock']), {
+          headers: { 'Content-Type': 'application/pdf' },
+        })
+      }),
+    )
+    // The dispatch-time guard reports the context is no longer current when the response comes back.
+    guard.isCurrent = () => false
+    render(<PrintSchedules />)
+
+    await userEvent.click(screen.getByRole('checkbox', { name: 'Schedule 5' }))
+    await userEvent.click(screen.getByRole('button', { name: /Generate PDF/ }))
+
+    await waitFor(() => expect(handled).toBe(true))
+    // The stale PDF must not download, and no "done" banner appears under the new context.
+    expect(vi.mocked(triggerDownload)).not.toHaveBeenCalled()
+    expect(screen.queryByText(/generated and downloaded/i)).toBeNull()
+  })
+
+  it('Clear resets to the default (schedules cleared, Schedule Information re-checked)', async () => {
+    render(<PrintSchedules />)
+
+    await userEvent.click(screen.getByRole('checkbox', { name: 'Schedule 5' }))
+    await userEvent.click(screen.getByRole('checkbox', { name: 'Comments' }))
+    await userEvent.click(screen.getByRole('checkbox', { name: 'Schedule information' })) // uncheck default
+    expect(screen.getByRole('checkbox', { name: 'Schedule information' })).not.toBeChecked()
+
+    await userEvent.click(screen.getByRole('button', { name: 'Clear' }))
+
+    expect(screen.getByRole('checkbox', { name: 'Schedule 5' })).not.toBeChecked()
+    expect(screen.getByRole('checkbox', { name: 'Comments' })).not.toBeChecked()
+    expect(screen.getByRole('checkbox', { name: 'Schedule information' })).toBeChecked()
+  })
+
   it('gates on a missing mill/year context instead of showing the form', () => {
-    ctx.millId = null
-    ctx.year = null
+    guard.contextMissing = true
     render(<PrintSchedules />)
     expect(screen.getByText('Select a mill and reporting year')).toBeInTheDocument()
     expect(screen.queryByRole('button', { name: /Generate PDF/ })).toBeNull()

frontend/src/components/printSchedules/index.scss

@@ -1,12 +1,15 @@
 @use '@carbon/react/scss/spacing' as *;
 
-// Print Schedules selection page: space the two checkbox groups and the action button apart.
+// Print Schedules selection page: space the two checkbox groups and the action buttons apart.
 .print-schedules {
   &__group {
     margin-block-end: $spacing-07;
   }
 
-  .#{'cds'}--btn {
+  // Generate + Clear sit on one row with a small gap.
+  &__actions {
+    display: flex;
+    gap: $spacing-03;
     margin-block-start: $spacing-03;
   }
 }

frontend/src/components/printSchedules/index.tsx

@@ -1,5 +1,5 @@
 import type { FC } from 'react'
-import { useState } from 'react'
+import { useEffect, useState } from 'react'
 import { Button, Checkbox, Column, FormGroup, Grid, InlineNotification } from '@carbon/react'
 import { Printer } from '@carbon/icons-react'
 import apiService from '@/service/api-service'
@@ -67,23 +67,43 @@ const comingSoon = (label: string) => `${label} (coming soon)`
 const noneSelected = <T extends string>(keys: readonly { key: T }[]): Record<T, boolean> =>
   Object.fromEntries(keys.map((k) => [k.key, false])) as Record<T, boolean>
 
+// The S06 default selection: Schedule Information pre-checked, everything else cleared. Used on first
+// load and by the Clear button.
+const defaultOptions = (): Record<OptionFlag, boolean> => ({
+  printScheduleInformation: true,
+  printComments: false,
+  printMillInformationReport: false,
+})
+
 /**
  * Print Schedules selection page (Epic 20.3). Mirrors the legacy PrintSchedulesMB screen: pick any of
  * the twelve schedules (+ "select all") and the print options, then download the combined bookmarked
  * PDF the backend assembles at {@code POST /api/v1/reports/print} for the working mill/year. Printing
  * is read-only for every role (BR-01); the server is authoritative for selection validation.
  */
 const PrintSchedules: FC = () => {
-  const { millId, year, contextMissing } = useScheduleContextGuard()
+  const { millId, year, contextMissing, isCurrent } = useScheduleContextGuard()
 
   const [schedules, setSchedules] = useState<Record<ScheduleFlag, boolean>>(() =>
     noneSelected(SCHEDULES),
   )
-  const [options, setOptions] = useState<Record<OptionFlag, boolean>>(() => noneSelected(OPTIONS))
+  const [options, setOptions] = useState<Record<OptionFlag, boolean>>(defaultOptions)
   const [busy, setBusy] = useState(false)
   const [message, setMessage] = useState<string | null>(null)
   const [error, setError] = useState<string | null>(null)
 
+  useEffect(() => {
+    // On a mill/year change, drop the in-flight lock + banners from the previous context, so a context
+    // switch can't leave a stale "Done"/error banner or a stuck Generate button; a late response is
+    // separately ignored via isCurrent() (in handleGenerate). The selection itself is intentionally kept.
+    // Deliberate reset-on-context-change — the synchronous setState here is the point.
+    /* eslint-disable @eslint-react/set-state-in-effect */
+    setBusy(false)
+    setMessage(null)
+    setError(null)
+    /* eslint-enable @eslint-react/set-state-in-effect */
+  }, [millId, year])
+
   // "All"/enable guards consider only the renderable schedules and available options — the disabled
   // ones can never be selected, so they must not gate (or be swept into) a generate.
   const allSelected = RENDERABLE_SCHEDULES.every((s) => schedules[s.key])
@@ -102,6 +122,10 @@ const PrintSchedules: FC = () => {
   }
 
   async function handleGenerate() {
+    // Capture the dispatch-time context guard: if the user switches mill/year while Jasper renders the
+    // sections, the late response must NOT download or repaint under the new context (the same
+    // stale-response guard the schedule pages apply to their writes).
+    const dispatchedCurrent = isCurrent
     setBusy(true)
     setError(null)
     setMessage(null)
@@ -115,16 +139,40 @@ const PrintSchedules: FC = () => {
         .post(`${PRINT_PATH}?millId=${String(millId)}&year=${String(year)}`, body, {
           responseType: 'blob',
         })
+      if (!dispatchedCurrent()) {
+        return
+      }
       triggerDownload(response.data as Blob, PDF_FILENAME)
       setMessage('Your Print Schedules PDF has been generated and downloaded.')
     } catch (err: unknown) {
-      // A 400/404/409 problem+json arrives as a Blob under responseType:'blob' — parse it for `detail`.
-      setError((await extractBlobDetail(err)) ?? 'Unable to generate the PDF. Please try again.')
+      if (!dispatchedCurrent()) {
+        return
+      }
+      // With selection validation client-gated, the real-world failure is a valid mill/year that simply
+      // has no rows in the ticked schedules → 404 ERR-005. Verbatim "Schedule not found." reads wrong for
+      // a print, so special-case it; keep the verbatim problem+json detail for 400/409 (ERR-002/003/004),
+      // where the legacy-verbatim-text rule actually applies. (Blob error body → extractBlobDetail.)
+      const status = (err as { response?: { status?: number } })?.response?.status
+      if (status === 404) {
+        setError('No data to print for the selected schedules.')
+      } else {
+        setError((await extractBlobDetail(err)) ?? 'Unable to generate the PDF. Please try again.')
+      }
     } finally {
-      setBusy(false)
+      if (dispatchedCurrent()) {
+        setBusy(false)
+      }
     }
   }
 
+  function handleClear() {
+    // Reset to the S06 default: Schedule Information re-checked, all schedules and other options cleared.
+    setSchedules(noneSelected(SCHEDULES))
+    setOptions(defaultOptions())
+    setMessage(null)
+    setError(null)
+  }
+
   return (
     <div className="app-page">
       <ScheduleTombstone title="Print Schedules" />
@@ -195,13 +243,18 @@ const PrintSchedules: FC = () => {
                 ))}
               </FormGroup>
 
-              <Button
-                renderIcon={Printer}
-                disabled={!canGenerate}
-                onClick={() => void handleGenerate()}
-              >
-                {busy ? 'Generating…' : 'Generate PDF'}
-              </Button>
+              <div className="print-schedules__actions">
+                <Button
+                  renderIcon={Printer}
+                  disabled={!canGenerate}
+                  onClick={() => void handleGenerate()}
+                >
+                  {busy ? 'Generating…' : 'Generate PDF'}
+                </Button>
+                <Button kind="secondary" disabled={busy} onClick={handleClear}>
+                  Clear
+                </Button>
+              </div>
             </>
           )}
         </Column>

frontend/src/utils/download.test.ts

@@ -7,7 +7,8 @@ describe('triggerDownload', () => {
     vi.unstubAllGlobals()
   })
 
-  it('creates an object URL, clicks a download anchor, and revokes the URL', () => {
+  it('creates an object URL, clicks a download anchor, and revokes the URL only after the click task', () => {
+    vi.useFakeTimers()
     const createObjectURL = vi.fn(() => 'blob:mock-url')
     const revokeObjectURL = vi.fn()
     vi.stubGlobal('URL', { createObjectURL, revokeObjectURL })
@@ -17,7 +18,12 @@ describe('triggerDownload', () => {
 
     expect(createObjectURL).toHaveBeenCalledTimes(1)
     expect(click).toHaveBeenCalledTimes(1)
+    // Revocation is deferred (revoking in the click's task cancels the download in Firefox/Safari).
+    expect(revokeObjectURL).not.toHaveBeenCalled()
+
+    vi.runAllTimers()
     expect(revokeObjectURL).toHaveBeenCalledWith('blob:mock-url')
+    vi.useRealTimers()
   })
 })
 

frontend/src/utils/download.ts

@@ -7,16 +7,14 @@ import { extractDetail } from '@/utils/error'
  */
 export function triggerDownload(blob: Blob, filename: string): void {
   const url = URL.createObjectURL(blob)
-  try {
-    const anchor = document.createElement('a')
-    anchor.href = url
-    anchor.download = filename
-    document.body.appendChild(anchor)
-    anchor.click()
-    anchor.remove()
-  } finally {
-    URL.revokeObjectURL(url)
-  }
+  const anchor = document.createElement('a')
+  anchor.href = url
+  anchor.download = filename
+  anchor.click()
+  // Defer revocation: revoking synchronously in the same task as the synthetic <a download> click can
+  // cancel the download in Firefox/Safari (Chrome tolerates it). Let the browser start reading the blob
+  // first. A detached anchor needs no appendChild/remove.
+  setTimeout(() => URL.revokeObjectURL(url), 0)
 }
 
 /**