Skip to content

Commit e666ac8

Browse files
Rylan-cgiclaude
andcommitted
fix(schedule6): import groupInput — the blur re-mask shipped inert (#291)
CI surfaced 16 Vitest unhandled errors, all one root cause: `commitRate` called `groupInput` without importing it, so every blur threw a ReferenceError. The throw landed AFTER `apply(rateInputsOf(form))`, so the rate itself updated and the defect-291 fix looked correct in manual testing — only the re-mask was lost, leaving `50000` unmasked beside a freshly formatted rate. Nothing caught it. `tsc --noEmit` aborts on this repo's tsconfig (TS5107/TS6310) before reading a single source file; eslint has no-undef off for TS, as standard; and vite build uses esbuild, which does not type-check. An unhandled error in a React event handler fails no test, so the suite stayed green. Three failure-path assertions expected the unmasked value — they had encoded the broken behaviour. Grouping on blur is the established convention here (schedule9, schedule1OtherCosts, CommaNumberInput all assert it), so the expectations were stale, not the fix. Also adds a test that asserts the FIELD, not just the rate — mutation-verified to fail with the original ReferenceError when the import is removed. Separately, OtherAcceptableSubtotal becomes a type alias: Schedule3SubPage's deriveSummary prop is `=> Readonly<Record<string, number | null>>` and TypeScript grants an implicit index signature to a type alias but never to an interface. That was a second error the broken type-check hid. Verified against an origin/main baseline: zero type-error regressions in any file this branch touches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5fd9c17 commit e666ac8

3 files changed

Lines changed: 46 additions & 6 deletions

File tree

frontend/src/components/schedule3OtherAcceptableCosts/derived.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,16 @@ import { committedNum, sumAsZero, wholeDollars } from '@/utils/derivedMath'
2020
* unit test of its own — it was the only new figure in that batch pinned by nothing.
2121
*/
2222

23-
/** The footer triple, keyed so it cannot be mis-paired with the summary labels positionally. */
24-
export interface OtherAcceptableSubtotal {
23+
/**
24+
* The footer triple, keyed so it cannot be mis-paired with the summary labels positionally.
25+
*
26+
* `type`, NOT `interface`: `Schedule3SubPage`'s `deriveSummary` prop is typed
27+
* `=> Readonly<Record<string, number | null>>` because it looks the triple up by key, and TypeScript
28+
* grants an implicit index signature to a type alias but never to an interface. As an interface this
29+
* assignment does not type-check — and it shipped that way, because `tsc --noEmit` aborts on this
30+
* repo's tsconfig before reading a single source file (see deferred-work.md).
31+
*/
32+
export type OtherAcceptableSubtotal = {
2533
readonly harvest: number | null
2634
readonly pop: number | null
2735
readonly crown: number | null

frontend/src/components/schedule6/__tests__/Schedule6.test.tsx

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,38 @@ describe('Schedule 6 page (Story 8.3)', () => {
209209
expect(rateIn(rowPanel(1))).toBe('') // the mask renders a blank, not an em dash
210210
})
211211

212+
test('the same blur re-applies the comma mask to the field itself (#291)', async () => {
213+
// Legacy's handlers re-rendered the INPUT alongside the rate (`render="vol cal ..."`,
214+
// schedule6.xhtml:153,163,364,383), which re-ran the converter and put the mask back. Schedule 6
215+
// is the only page whose blur moves a derived cell, so it was the only one that could leave
216+
// `50000` sitting unmasked next to a freshly-formatted rate.
217+
//
218+
// This test exists because that re-mask shipped INERT: `commitRate` called `groupInput` without
219+
// importing it, so every blur threw a ReferenceError *after* the rate had been applied. The rate
220+
// updated, the mask never came back, and the throw surfaced only as a Vitest unhandled error --
221+
// which fails no test. Assert the field, not just the rate.
222+
server.use(http.get(URL, () => HttpResponse.json(doc())))
223+
render(<Schedule6 />)
224+
const user = userEvent.setup()
225+
226+
await screen.findByRole('button', { name: 'Road Maintenance report Id: 1' })
227+
await user.click(within(rowPanel(1)).getByRole('button', { name: /^edit$/i }))
228+
229+
const volume = within(rowPanel(1)).getByLabelText('Volume m³')
230+
await user.clear(volume)
231+
await user.type(volume, '1000000')
232+
expect(volume).toHaveValue('1000000') // mid-entry: untouched, so the caret is not moved
233+
await user.tab()
234+
expect(within(rowPanel(1)).getByLabelText('Volume m³')).toHaveValue('1,000,000')
235+
236+
const cost = within(rowPanel(1)).getByLabelText('Cost $')
237+
await user.clear(cost)
238+
await user.type(cost, '2500000')
239+
await user.tab()
240+
expect(within(rowPanel(1)).getByLabelText('Cost $')).toHaveValue('2,500,000')
241+
expect(rateIn(rowPanel(1))).toBe('2.50') // 2,500,000 / 1,000,000
242+
})
243+
212244
test('the footer totals do NOT move during entry — legacy left them until Save (#291)', async () => {
213245
// The deliberate boundary: totalVol/totalCos/totalCal appear in NO legacy render or update
214246
// target, so refreshing them from the document is already faithful and they are not mirrored.
@@ -1172,7 +1204,7 @@ describe('Schedule 6 page (Story 8.3)', () => {
11721204
// Values stay put so the entry can be corrected and resubmitted.
11731205
expect(within(panel).getByLabelText('TSA or TFL')).toHaveValue('TFL')
11741206
expect(within(panel).getByLabelText('TFL')).toHaveValue('99')
1175-
expect(within(panel).getByLabelText('Volume m³')).toHaveValue('1000')
1207+
expect(within(panel).getByLabelText('Volume m³')).toHaveValue('1,000')
11761208
})
11771209

11781210
test('a load failure carrying no detail falls back to the generic load message (AC7)', async () => {
@@ -1205,7 +1237,7 @@ describe('Schedule 6 page (Story 8.3)', () => {
12051237
expect(await screen.findByText('Schedule could not be saved.')).toBeInTheDocument()
12061238
// add-is-save: the panel and its values survive a server-side failure so the entry is not retyped.
12071239
expect(screen.getByRole('region', { name: 'Add Road Maintenance report' })).toBeInTheDocument()
1208-
expect(within(panel).getByLabelText('Volume m³')).toHaveValue('1000')
1240+
expect(within(panel).getByLabelText('Volume m³')).toHaveValue('1,000')
12091241
})
12101242

12111243
test('an edit failure renders the detail verbatim and leaves the editor open (AC4 / AC10)', async () => {
@@ -1227,7 +1259,7 @@ describe('Schedule 6 page (Story 8.3)', () => {
12271259
expect(await screen.findByText(detail)).toBeInTheDocument()
12281260
// The failure branch must not run onSuccess: the editor stays open holding the rejected value so
12291261
// it can be corrected, page state is not replaced, and no success banner appears alongside.
1230-
expect(within(rowPanel(1)).getByLabelText('Volume m³')).toHaveValue('2000')
1262+
expect(within(rowPanel(1)).getByLabelText('Volume m³')).toHaveValue('2,000')
12311263
expect(screen.queryByText('Data saved successfully')).not.toBeInTheDocument()
12321264
// The in-flight lock releases on the error path too, or Save is dead until reload.
12331265
await waitFor(() =>

frontend/src/components/schedule6/index.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import apiService from '@/service/api-service'
1818
import { useScheduleContextGuard } from '@/hooks/useScheduleContextGuard'
1919
import { useScheduleDocument } from '@/hooks/useScheduleDocument'
2020
import { extractDetail } from '@/utils/error'
21-
import { numStr } from '@/utils/number'
21+
import { groupInput, numStr } from '@/utils/number'
2222
import LoadingScreen from '@/components/core/LoadingScreen'
2323
import NotificationColumn from '@/components/core/NotificationColumn'
2424
import PageState from '@/components/core/PageState'

0 commit comments

Comments
 (0)