Skip to content

Commit 3bbd4aa

Browse files
gpascucciclaude
andcommitted
feat(29.6): route Schedules 2/3/4/5/8 through the shared guarded run()
Story 1.6 (tech hardening), completing the sweep started with Schedule 1. Every schedule save/delete/check-status now dispatches through useScheduleBanners.run() (via useScheduleMutations), so a stale in-flight write can no longer repaint a newly-switched mill/year context, and the request/error/lock scaffolding lives in one place. - Schedules 2, 3, 4, 8: adopt useScheduleMutations; drop the hand-rolled apiService...put/post/delete().then().catch().finally() chains and (where present) the separate `checking` lock (folds into the single `saving`). - Schedule 5: drop its local runMutation and adopt the shared hook. To keep its silent copy-hint resolve, useScheduleBanners.run() now accepts a null fallback = "fail silently, no banner" (backward-compatible; every existing caller passes a string). All five call sites reproduce their URLs verbatim. - Delete convergence (AC6): one shared mechanism; each page's post-delete empty-state stays at the call site — Schedules 1/3 reset in place (a re-GET would 404), list pages 2/4/8 re-seed from a reload, Schedule 5 re-seeds from its DELETE response. - Added a Schedule 1 stale-response regression test (AC1): a mill/year switch mid-save no longer applies the late PUT. Behavior unchanged (AC7): full frontend suite 887 -> 888 green, tsc + eslint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f74956a commit 3bbd4aa

8 files changed

Lines changed: 448 additions & 414 deletions

File tree

frontend/src/components/schedule1/__tests__/Schedule1.test.tsx

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,32 @@ vi.mock('@tanstack/react-router', () => ({
1616

1717
import Schedule1 from '@/components/schedule1'
1818
import MillYearProvider from '@/context/millYear/MillYearProvider'
19+
import useMillYear from '@/context/millYear/useMillYear'
20+
21+
// Drives a mid-save mill/year change so the shared run() stale-response guard can be exercised
22+
// (Story 29.6). Module-level so it is not re-created per render (an @eslint-react rule forbids nested
23+
// component definitions).
24+
const StaleRaceHarness = () => {
25+
const { setContext } = useMillYear()
26+
return (
27+
<>
28+
<button type="button" onClick={() => setContext(999, 2020)}>
29+
change
30+
</button>
31+
<Schedule1 />
32+
</>
33+
)
34+
}
35+
36+
// Deterministically drain the event loop so a settled-but-guarded request finishes its whole
37+
// then/catch/finally chain before a negative assertion runs (turn-based, never wall-clock).
38+
const flushAsync = async () => {
39+
for (let i = 0; i < 4; i += 1) {
40+
await new Promise((resolve) => {
41+
setTimeout(resolve, 0)
42+
})
43+
}
44+
}
1945

2046
const URL = 'http://localhost:3000/api/v1/schedule1'
2147

@@ -663,3 +689,56 @@ describe('Schedule1 Other Costs navigation (Story 2.5)', () => {
663689
expect(mockNavigate).toHaveBeenCalledWith({ to: '/schedule-1/other-costs' })
664690
})
665691
})
692+
693+
describe('Schedule1 stale-response guard (Story 29.6)', () => {
694+
test('a mill/year change mid-save does not apply the stale response (AC1)', async () => {
695+
// The PUT is gated on an explicit release, not a wall-clock delay, so the "stale response settles
696+
// after the context change" ordering holds under any CI load. Routing save through the shared
697+
// useScheduleMutations run() gives Schedule 1 the isCurrent() guard it previously lacked.
698+
let releasePut = () => {}
699+
const putGate = new Promise<void>((resolve) => {
700+
releasePut = resolve
701+
})
702+
server.use(
703+
http.get(URL, ({ request }) =>
704+
request.url.includes('millId=999')
705+
? HttpResponse.json({
706+
...schedule1Doc,
707+
millId: 999,
708+
year: 2020,
709+
editable: false,
710+
comments: 'Context 999/2020 loaded',
711+
})
712+
: HttpResponse.json(schedule1Doc),
713+
),
714+
http.put(URL, async () => {
715+
await putGate
716+
return HttpResponse.json({
717+
...schedule1Doc,
718+
message: { key: 'dataSavedSuccesfullyInfoMsg', text: 'Data saved successfully' },
719+
})
720+
}),
721+
)
722+
723+
render(
724+
<MillYearProvider initial={{ millId: 514, year: 2021 }}>
725+
<StaleRaceHarness />
726+
</MillYearProvider>,
727+
)
728+
const user = userEvent.setup()
729+
730+
// Editable 514 loaded → dispatch the save (PUT now in flight) → switch mill/year before it settles.
731+
await screen.findByLabelText('Standing Tree to Loaded Truck cost')
732+
await user.click(screen.getAllByRole('button', { name: /^save$/i })[0])
733+
await user.click(screen.getByRole('button', { name: /change/i }))
734+
735+
// The new context's document has rendered (read-only 999/2020).
736+
expect(await screen.findByText('Context 999/2020 loaded')).toBeInTheDocument()
737+
738+
// Release the stale PUT, let its chain settle, then confirm nothing from it landed on 999/2020.
739+
releasePut()
740+
await flushAsync()
741+
expect(screen.queryByText('Data saved successfully')).not.toBeInTheDocument()
742+
expect(screen.queryByLabelText('Standing Tree to Loaded Truck cost')).not.toBeInTheDocument()
743+
})
744+
})

frontend/src/components/schedule2/index.tsx

Lines changed: 51 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { FC } from 'react'
22
import type Schedule2Response from '@/interfaces/Schedule2Response'
33
import type { CostBlock, CheckStatusResponse } from '@/interfaces/Schedule2Response'
44
import type Schedule2Request from '@/interfaces/Schedule2Request'
5-
import { useCallback, useState } from 'react'
5+
import { useState } from 'react'
66
import {
77
Button,
88
Column,
@@ -18,9 +18,9 @@ import {
1818
TextArea,
1919
} from '@carbon/react'
2020
import apiService from '@/service/api-service'
21-
import useMillYear from '@/context/millYear/useMillYear'
21+
import { useScheduleContextGuard } from '@/hooks/useScheduleContextGuard'
2222
import { useScheduleDocument } from '@/hooks/useScheduleDocument'
23-
import { extractDetail } from '@/utils/error'
23+
import { useScheduleMutations } from '@/hooks/useScheduleMutations'
2424
import { fmtCurrency, fmtNumber, numStr, toNum } from '@/utils/number'
2525
import CommaNumberInput from '@/components/core/CommaNumberInput'
2626
import LoadingScreen from '@/components/core/LoadingScreen'
@@ -79,22 +79,28 @@ function buildRequest(doc: Schedule2Response, form: FieldValues): Schedule2Reque
7979
}
8080

8181
const Schedule2: FC = () => {
82-
const { millId, year } = useMillYear()
83-
const contextMissing = millId === null || year === null
82+
const { millId, year, contextMissing, isCurrent } = useScheduleContextGuard()
83+
84+
// Save/delete/check-status all run through the shared hook's guarded run() (Story 29.6): a stale
85+
// in-flight write can no longer repaint a newly-switched mill/year. `saving` is the single in-flight
86+
// lock for every write (it also gates Check Status). `checkResult` holds the Check Status response.
87+
const {
88+
saving,
89+
message: saveMessage,
90+
actionError: saveError,
91+
checkResult: statusMessages,
92+
setMessage: setSaveMessage,
93+
setActionError: setSaveError,
94+
setCheckResult: setStatusMessages,
95+
clearBanners,
96+
resetBanners,
97+
save,
98+
remove,
99+
checkStatus,
100+
} = useScheduleMutations<CheckStatusResponse>({ path: '/v1/schedule2', millId, year, isCurrent })
84101

85-
const [saving, setSaving] = useState(false)
86-
const [saveMessage, setSaveMessage] = useState<string | null>(null)
87-
const [saveError, setSaveError] = useState<string | null>(null)
88-
const [statusMessages, setStatusMessages] = useState<CheckStatusResponse | null>(null)
89102
const [confirmDeleteOpen, setConfirmDeleteOpen] = useState(false)
90103

91-
// Clear the mutation notifications whenever a fresh document loads (mill/year change).
92-
const resetMessages = useCallback(() => {
93-
setSaveMessage(null)
94-
setSaveError(null)
95-
setStatusMessages(null)
96-
}, [])
97-
98104
const { data, setData, form, setForm, setField, errorDetail, isLoading } =
99105
useScheduleDocument<Schedule2Response>({
100106
path: '/v1/schedule2',
@@ -103,7 +109,7 @@ const Schedule2: FC = () => {
103109
contextMissing,
104110
seedForm,
105111
mapLoadError,
106-
onReset: resetMessages,
112+
onReset: resetBanners,
107113
})
108114

109115
const handleSave = () => {
@@ -119,58 +125,43 @@ const Schedule2: FC = () => {
119125
setSaveError('Please correct the highlighted fields before saving.')
120126
return
121127
}
122-
setSaving(true)
123-
setSaveMessage(null)
124-
setSaveError(null)
125-
setStatusMessages(null)
126-
apiService
127-
.getAxiosInstance()
128-
.put<Schedule2Response>(
129-
`/v1/schedule2?millId=${millId}&year=${year}`,
130-
buildRequest(data, form),
131-
)
132-
.then((response) => {
133-
setData(response.data)
134-
setForm(seedForm(response.data))
128+
clearBanners() // drop any prior banners incl. a now-stale Check Status result
129+
save<Schedule2Response>(buildRequest(data, form), {
130+
fallback: 'Schedule could not be saved.',
131+
onSuccess: (doc) => {
132+
setData(doc)
133+
setForm(seedForm(doc))
135134
// Success text verbatim from the API message field (AD-8), never hardcoded.
136-
setSaveMessage(response.data.message?.text ?? null)
137-
})
138-
.catch((error: unknown) => {
139-
// Keep the entered values; surface the API's verbatim ProblemDetail.detail.
140-
setSaveError(extractDetail(error) || 'Schedule could not be saved.')
141-
})
142-
.finally(() => setSaving(false))
135+
setSaveMessage(doc.message?.text ?? null)
136+
},
137+
})
143138
}
144139

145140
const handleDelete = () => {
146141
if (saving) {
147142
return
148143
}
149144
setConfirmDeleteOpen(false)
150-
setSaving(true)
151-
setSaveMessage(null)
152-
setSaveError(null)
153-
setStatusMessages(null)
154-
const api = apiService.getAxiosInstance()
155-
api
156-
.delete<{ message?: { text?: string } }>(`/v1/schedule2?millId=${millId}&year=${year}`)
157-
.then((response) => {
158-
const deleteMessage = response.data?.message?.text ?? null
159-
// Schedule 2 never 404s: with the summary gone, a re-GET returns the 200 empty EDITABLE
160-
// document (revisionCount null). Reload it so the meta row / form reflect reality and the
161-
// Licensee can immediately re-enter data (legacy AF1), while keeping the API delete message.
162-
return api
145+
clearBanners() // the deleted schedule's check result / save banner are stale
146+
remove<{ message?: { text?: string } }>({
147+
fallback: 'Unable to delete Schedule 2.',
148+
// Schedule 2 never 404s: with the summary gone, a re-GET returns the 200 empty EDITABLE
149+
// document (revisionCount null). Reload it so the meta row / form reflect reality and the
150+
// Licensee can immediately re-enter data (legacy AF1), while keeping the API delete message.
151+
// This per-page empty-state lives at the call site (Story 29.6): list/re-GET pages re-seed
152+
// from the reload, single-doc reset-in-place pages (Schedules 1/3) reset in place instead.
153+
onSuccess: (delResp) => {
154+
const deleteMessage = delResp?.message?.text ?? null
155+
apiService
156+
.getAxiosInstance()
163157
.get<Schedule2Response>(`/v1/schedule2?millId=${millId}&year=${year}`)
164158
.then((reload) => {
165159
setData(reload.data)
166160
setForm(seedForm(reload.data))
167161
setSaveMessage(deleteMessage)
168162
})
169-
})
170-
.catch((error: unknown) => {
171-
setSaveError(extractDetail(error) || 'Unable to delete Schedule 2.')
172-
})
173-
.finally(() => setSaving(false))
163+
},
164+
})
174165
}
175166

176167
const handleCheckStatus = () => {
@@ -187,20 +178,11 @@ const Schedule2: FC = () => {
187178
setSaveError('Please correct the highlighted fields before checking status.')
188179
return
189180
}
190-
setSaving(true)
191-
setSaveMessage(null)
192-
setSaveError(null)
193-
setStatusMessages(null)
194-
apiService
195-
.getAxiosInstance()
196-
.post<CheckStatusResponse>(`/v1/schedule2/check-status?millId=${millId}&year=${year}`)
197-
.then((response) => {
198-
setStatusMessages(response.data)
199-
})
200-
.catch((error: unknown) => {
201-
setSaveError(extractDetail(error) || 'Unable to check status.')
202-
})
203-
.finally(() => setSaving(false))
181+
clearBanners() // don't leave a stale Save success banner beside a new check result
182+
checkStatus<CheckStatusResponse>({
183+
fallback: 'Unable to check status.',
184+
onSuccess: setStatusMessages,
185+
})
204186
}
205187

206188
if (contextMissing) {

0 commit comments

Comments
 (0)