Skip to content

Commit 7c490c0

Browse files
Rylan-cgiclaude
andcommitted
fix(schedule10): mirror the ballast D branch and disable the N-discarded inputs
Addresses both "worth fixing in this PR" items from SScholefield's review of #325. 1. Ballast method D had the same echo mismatch N was fixed for ------------------------------------------------------------- buildStabilizing mirrored the server coercion only on N, so on D the request carried whatever material had been picked BEFORE D was chosen -- the combo is disabled on D, but the form still held the old value -- and Schedule10Service:557-560 then silently replaced it with NA. That is precisely the mismatch the N mirroring exists to prevent, reached by the other branch. The material decision now goes through ballastForcesMaterialNa, which already covered both branches, and is made independently of the figure zeroing. Only the FIGURES are N-specific: D stores its figures as submitted, and still does. 2. Method N left the discarded inputs enabled --------------------------------------------- The four dimensions plus actual cost and other transfer stayed editable while buildStabilizing sent 0 for them, even though the material combo was disabled in exactly the same situation. They are now disabled too, so the rule reads the same way everywhere. stTtTransfer is deliberately NOT in that set -- the server keeps it on the N branch -- and the hint now names the affected fields and says so, rather than leaving the reporter to work out which figures "dimensions, actual cost and other transfer" meant. The set lives in validation.ts as BALLAST_ZEROED_FIELDS, beside the builder that zeroes them, so the disable list and the request body cannot drift apart. Tests ----- Three added, and all three were mutation-tested -- each fix reverted, the new test confirmed failing, then restored: - D sends NA for the material while every figure survives (this one fails with the old ballastZeroesFigures gate). - N disables the six discarded inputs and leaves TtT Transfer editable. - D disables only the material, leaving the figures editable. Plus a drift guard: the fields buildStabilizing actually zeroes on N are compared against BALLAST_ZEROED_FIELDS, so the disable list cannot silently diverge from the request in either direction. Verified: 202 tests green across Schedule 10 and every file this branch touches; full frontend suite 1045/1046 with the one failure PRE-EXISTING (Schedule8 Helicopter, passes in isolation); build clean; lint and Prettier clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 93fcb4c commit 7c490c0

4 files changed

Lines changed: 164 additions & 11 deletions

File tree

frontend/src/components/schedule10/RoadDetailFields.tsx

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import CommaNumberInput from '@/components/core/CommaNumberInput'
77
import { fmtCurrency, fmtWholeCost } from '@/utils/number'
88
import type { MaskedField, RoadDetailErrors, RoadDetailFormValues } from './validation'
99
import {
10+
BALLAST_ZEROED_FIELDS,
1011
COMMENTS_MAX,
1112
ROAD_NAME_MAX,
1213
ballastForcesMaterialNa,
@@ -72,6 +73,18 @@ const RoadDetailFields: FC<RoadDetailFieldsProps> = ({
7273
}) => {
7374
const id = (name: string) => `${idPrefix}-${name}`
7475

76+
// `N` and `D` both have their material forced to `NA`; `N` additionally has its dimensions and two
77+
// of its three costs zeroed, which `buildStabilizing` sends rather than leaving to the server.
78+
const materialForced = ballastForcesMaterialNa(form.stBallastMethodCode)
79+
const figuresZeroed = ballastZeroesFigures(form.stBallastMethodCode)
80+
81+
// An input whose entry `N` discards is disabled, so the rule reads the same way everywhere: the
82+
// material combo was already disabled in exactly this situation, and leaving these editable
83+
// invited entry that Save would silently replace with zero. `stTtTransfer` is NOT in the set —
84+
// the server keeps it on the `N` branch, so it stays editable.
85+
const zeroedByBallast = (key: MaskedField): boolean =>
86+
figuresZeroed && (BALLAST_ZEROED_FIELDS as readonly string[]).includes(key)
87+
7588
// A stored classification may have been de-listed since it was saved, in which case it is absent
7689
// from the offerable list. Appending it keeps the field showing what the row actually holds instead
7790
// of appearing unselected — and it is appended with the row's OWN label, which the response carries
@@ -130,7 +143,7 @@ const RoadDetailFields: FC<RoadDetailFieldsProps> = ({
130143
labelText={labelWithUnit}
131144
autoComplete="off"
132145
value={form[key]}
133-
disabled={disabled}
146+
disabled={disabled || zeroedByBallast(key)}
134147
invalid={Boolean(errors[key])}
135148
invalidText={errors[key] ?? ''}
136149
onValueChange={(raw) => onChange(key, raw)}
@@ -167,11 +180,6 @@ const RoadDetailFields: FC<RoadDetailFieldsProps> = ({
167180
)
168181
}
169182

170-
// `N` and `D` both have their material forced to `NA`; `N` additionally has its dimensions and two
171-
// of its three costs zeroed, which `buildStabilizing` now sends rather than leaving to the server.
172-
const materialForced = ballastForcesMaterialNa(form.stBallastMethodCode)
173-
const figuresZeroed = ballastZeroesFigures(form.stBallastMethodCode)
174-
175183
const engineeringCostsValue = form.detailedEngineeringCostInd === 'Y' ? 'Yes' : 'No'
176184

177185
const endHaulRate = previewCostPerVolumePerLength(
@@ -315,8 +323,9 @@ const RoadDetailFields: FC<RoadDetailFieldsProps> = ({
315323
)}
316324
{!readOnly && figuresZeroed && (
317325
<p className="schedule-10__hint">
318-
This Additional Stabilizing code stores its dimensions, actual cost and other transfer as
319-
zero.
326+
This Additional Stabilizing code stores its length, surface width, depth, distance to
327+
source, actual cost and other transfer as zero, so those fields are disabled. TtT Transfer
328+
is still recorded as entered.
320329
</p>
321330
)}
322331
</div>

frontend/src/components/schedule10/__tests__/Schedule10.test.tsx

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,6 +1307,59 @@ describe('regressions from the 2026-08-19 code review', () => {
13071307
})
13081308
})
13091309

1310+
test('review #325 — method N disables the figures it discards, but not TtT Transfer', async () => {
1311+
renderSchedule10('/schedule-10?pageId=8900')
1312+
await userEvent.click(await screen.findByRole('button', { name: 'Edit' }))
1313+
await screen.findByDisplayValue('Mainline A')
1314+
1315+
// Before choosing N every figure is editable.
1316+
expect(screen.getByLabelText('Additional Stabilizing Length (km)')).toBeEnabled()
1317+
1318+
await userEvent.click(screen.getByRole('combobox', { name: 'Ballast Method Code' }))
1319+
await userEvent.click(await screen.findByRole('option', { name: 'None' }))
1320+
1321+
for (const label of [
1322+
'Additional Stabilizing Length (km)',
1323+
'Additional Stabilizing Surface Width (m)',
1324+
'Depth (m)',
1325+
'Distance to Source (km)',
1326+
'Additional Stabilizing Actual Costs ($)',
1327+
'Additional Stabilizing Other Transfer ($)',
1328+
]) {
1329+
expect(screen.getByLabelText(label)).toBeDisabled()
1330+
}
1331+
// The server keeps this one on the N branch, so entry here is still recorded.
1332+
expect(screen.getByLabelText('Additional Stabilizing TtT Transfer ($)')).toBeEnabled()
1333+
})
1334+
1335+
test('review #325 — method D disables only the material, leaving the figures editable', async () => {
1336+
server.use(
1337+
getHandler(
1338+
doc({
1339+
codeLists: {
1340+
...doc().codeLists,
1341+
ballastMethods: [
1342+
{ code: 'C', description: 'Crushed' },
1343+
{ code: 'N', description: 'None' },
1344+
{ code: 'D', description: 'Dirt' },
1345+
],
1346+
},
1347+
}),
1348+
),
1349+
)
1350+
renderSchedule10('/schedule-10?pageId=8900')
1351+
await userEvent.click(await screen.findByRole('button', { name: 'Edit' }))
1352+
await screen.findByDisplayValue('Mainline A')
1353+
1354+
await userEvent.click(screen.getByRole('combobox', { name: 'Ballast Method Code' }))
1355+
await userEvent.click(await screen.findByRole('option', { name: 'Dirt' }))
1356+
1357+
expect(screen.getByRole('combobox', { name: 'Type' })).toBeDisabled()
1358+
// D stores its figures as submitted, so they must stay editable.
1359+
expect(screen.getByLabelText('Additional Stabilizing Length (km)')).toBeEnabled()
1360+
expect(screen.getByLabelText('Additional Stabilizing Actual Costs ($)')).toBeEnabled()
1361+
})
1362+
13101363
test('M7 — a de-listed BEC classification renders its label, not its catalogue id', async () => {
13111364
server.use(
13121365
getHandler(

frontend/src/components/schedule10/__tests__/validation.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, test } from 'vitest'
22
import type { ConstructionPage, RoadDetail } from '@/interfaces/Schedule10Response'
33
import {
4+
BALLAST_ZEROED_FIELDS,
45
MASK_DIGITS,
56
SCH10_MESSAGES,
67
ballastForcesMaterialNa,
@@ -722,6 +723,69 @@ describe('fixes from the 2026-08-19 code review', () => {
722723
})
723724
})
724725

726+
test('review #325 — method D forces the material to NA but keeps every figure', () => {
727+
// The N mirroring originally covered only N, so D sent whatever material was picked before D was
728+
// chosen and the server (Schedule10Service:557-560) silently replaced it with NA -- the exact
729+
// echo mismatch the mirroring exists to prevent. D coerces the material ONLY; its figures are
730+
// stored as submitted.
731+
const form = {
732+
...base(),
733+
stBallastMethodCode: 'D',
734+
stBallastMaterialCode: 'GR',
735+
stLength: '3',
736+
stSurfaceWidth: '6.5',
737+
stDepth: '0.3',
738+
stDistanceToSource: '12.4',
739+
stActualCost: '5000',
740+
stTtTransfer: '750',
741+
stOtherTransfer: '250',
742+
}
743+
expect(buildRoadDetailBody(form).stabilizing).toEqual({
744+
ballastMethodCode: 'D',
745+
ballastMaterialCode: 'NA',
746+
length: 3,
747+
surfaceWidth: 6.5,
748+
depth: 0.3,
749+
distanceToSource: 12.4,
750+
actualCost: 5000,
751+
ttTransfer: 750,
752+
otherTransfer: 250,
753+
})
754+
})
755+
756+
test('review #325 — the zeroed-field set matches what the N branch actually sends', () => {
757+
// BALLAST_ZEROED_FIELDS drives which inputs the form DISABLES. If it drifts from what
758+
// buildStabilizing zeroes, the UI either disables a field that is still recorded or leaves one
759+
// editable whose entry is discarded -- the bug this review caught, in the other direction.
760+
const filled = {
761+
...base(),
762+
stBallastMethodCode: 'N',
763+
stLength: '3',
764+
stSurfaceWidth: '6.5',
765+
stDepth: '0.3',
766+
stDistanceToSource: '12.4',
767+
stActualCost: '5000',
768+
stTtTransfer: '750',
769+
stOtherTransfer: '250',
770+
}
771+
const sent = buildRoadDetailBody(filled).stabilizing as Record<string, unknown>
772+
// Map each form key to the request field it lands in.
773+
const requestField: Record<string, string> = {
774+
stLength: 'length',
775+
stSurfaceWidth: 'surfaceWidth',
776+
stDepth: 'depth',
777+
stDistanceToSource: 'distanceToSource',
778+
stActualCost: 'actualCost',
779+
stOtherTransfer: 'otherTransfer',
780+
stTtTransfer: 'ttTransfer',
781+
}
782+
const zeroed = Object.keys(requestField).filter((key) => sent[requestField[key]] === 0)
783+
expect(zeroed.sort()).toEqual([...BALLAST_ZEROED_FIELDS].sort())
784+
// And the one deliberately left out is genuinely still carried.
785+
expect(BALLAST_ZEROED_FIELDS).not.toContain('stTtTransfer')
786+
expect(sent.ttTransfer).toBe(750)
787+
})
788+
725789
test('P2 — both N and D force the material to NA in the UI', () => {
726790
expect(ballastForcesMaterialNa('N')).toBe(true)
727791
expect(ballastForcesMaterialNa('D')).toBe(true)

frontend/src/components/schedule10/validation.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,20 @@ export const ballastMaterialRequired = (methodCode: string): boolean => {
188188
return code === '' || code === 'C'
189189
}
190190

191+
/**
192+
* The Additional Stabilizing figures ballast method `N` stores as zero, so the form can disable
193+
* exactly the inputs whose entry would be discarded. `stTtTransfer` is deliberately ABSENT: the
194+
* server keeps it on the `N` branch, so it stays editable.
195+
*/
196+
export const BALLAST_ZEROED_FIELDS = [
197+
'stLength',
198+
'stSurfaceWidth',
199+
'stDepth',
200+
'stDistanceToSource',
201+
'stActualCost',
202+
'stOtherTransfer',
203+
] as const
204+
191205
/** The material code the server substitutes on the `N` and `D` branches. */
192206
export const BALLAST_MATERIAL_NA = 'NA'
193207

@@ -197,7 +211,11 @@ export const ballastForcesMaterialNa = (methodCode: string): boolean => {
197211
return code === 'N' || code === 'D'
198212
}
199213

200-
/** Ballast method `N` is the branch whose dimensions and two costs the server forces to zero. */
214+
/**
215+
* Ballast method `N` is the branch whose dimensions and two costs the server forces to zero. The
216+
* MATERIAL is not this branch's business — `D` forces that too, so it goes through
217+
* {@link ballastForcesMaterialNa}.
218+
*/
201219
export const ballastZeroesFigures = (methodCode: string): boolean =>
202220
methodCode.trim().toUpperCase() === 'N'
203221

@@ -580,10 +598,19 @@ const buildSubGrade = (form: RoadDetailFormValues): SubGradeRequest => ({
580598
*/
581599
const buildStabilizing = (form: RoadDetailFormValues): StabilizingRequest => {
582600
const method = form.stBallastMethodCode.trim()
601+
// BOTH `N` and `D` have the material forced to `NA` server-side
602+
// (`Schedule10Service.java:557-560`), so both must send it. Mirroring only `N` left `D` with the
603+
// exact echo mismatch the mirroring exists to prevent: the combo is disabled on `D`, but the form
604+
// still held whatever material was picked BEFORE `D` was chosen, and that value was sent and then
605+
// silently replaced. Only the FIGURES are `N`-specific.
606+
const ballastMaterialCode = ballastForcesMaterialNa(method)
607+
? BALLAST_MATERIAL_NA
608+
: blankToNull(form.stBallastMaterialCode)
609+
583610
if (ballastZeroesFigures(method)) {
584611
return {
585612
ballastMethodCode: method,
586-
ballastMaterialCode: BALLAST_MATERIAL_NA,
613+
ballastMaterialCode,
587614
length: 0,
588615
surfaceWidth: 0,
589616
depth: 0,
@@ -596,7 +623,7 @@ const buildStabilizing = (form: RoadDetailFormValues): StabilizingRequest => {
596623
}
597624
return {
598625
ballastMethodCode: method,
599-
ballastMaterialCode: blankToNull(form.stBallastMaterialCode),
626+
ballastMaterialCode,
600627
length: numberOrNull(form.stLength),
601628
surfaceWidth: numberOrNull(form.stSurfaceWidth),
602629
depth: numberOrNull(form.stDepth),

0 commit comments

Comments
 (0)