Skip to content

fix(SDK-1284): persist 0 when an additional-earning field is cleared - #2649

Draft
krisxcrash wants to merge 1 commit into
mainfrom
kw/fix/sdk-1284-clearing-additional-earning-does-not-zero
Draft

fix(SDK-1284): persist 0 when an additional-earning field is cleared#2649
krisxcrash wants to merge 1 commit into
mainfrom
kw/fix/sdk-1284-clearing-additional-earning-does-not-zero

Conversation

@krisxcrash

@krisxcrash krisxcrash commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Fixes SDK-1284 (High, Pay v2 / RRoP Test Fest).

Correction to the ticket

The ticket attributes this to the gws-flows NumberInput adapter forwarding NaN. That component is not on this path. Additional earnings render as TextInputField type="number" min={0}, so the values are strings and NumberInput never enters the picture. No adapter change is needed; the cause is entirely in this component.

Root cause

buildCompensationFromFormData gated on formAmount !== '':

if (formAmount !== undefined && formAmount !== '') {

A cleared field is '', so the entry was skipped entirely and never added to updatedFixedCompensations. PayrollEditEmployee then spreads that object into the payrollUpdate, so the API received a fixed_compensations array with the entry absent — and the payroll update API leaves anything it is not sent untouched. The previously saved amount survived: the box looked empty, the save appeared to succeed, and the old value came back on refetch. That is exactly the reporter's "the pre and post are ultimately the same screenshot."

Typing an explicit 0 worked because that took the existingFixedCompensation branch and sent amount: "0".

Nothing caught it client-side either: the field is marked isRequired, but the schema is z.record(z.string(), z.string().optional()), so empty passes. (Field-level rules are ignored when a resolver is present, so isRequired is presentational here.)

Fix

An entry that already has a saved compensation now resolves a cleared field to '0', so clearing behaves exactly like typing 0 — the path testers already know works. Entries with no saved compensation stay omitted; there is nothing to zero out.

This also fixes the live gross-pay preview, which runs through the same builder, so the total drops the moment the field is cleared instead of showing the stale amount.

Testing

Two new cases in PayrollEditEmployeePresentation.test.tsx, sitting next to the existing "submits existing compensations even when set to zero":

  • clearing an existing Bonus sends amount: '0' and leaves Commission untouched
  • clearing an earning type that never had a saved amount still omits it

npx vitest run src/components/Payroll/PayrollEditEmployee — 70 pass. Both new tests also pass in isolation (-t), not just when an earlier test has warmed the i18n namespace. npm run tsc, eslint, prettier — clean.

Merge note

SDK-1285 touches the same file and the same schema/builder block. Whichever of the two merges second needs a small rebase there.

buildCompensationFromFormData skipped any fixed compensation whose form value
was an empty string, so clearing a field dropped the entry from the payload
entirely. The payroll update API leaves anything it is not sent untouched, so the
previously saved amount survived: the box looked empty, the save appeared to
succeed, and the old value came back on refetch. Typing an explicit 0 worked
because that took the existing-compensation branch.

An entry that already has a saved amount now resolves a cleared field to '0', so
clearing behaves exactly like typing 0. Entries with no saved compensation stay
omitted -- there is nothing to zero out.

This also fixes the live gross-pay preview, which runs through the same builder,
so the total now drops the moment the field is cleared instead of showing the
stale amount.

Note for reviewers: the ticket attributed this to the gws-flows NumberInput
adapter forwarding NaN. That component is not on this path -- additional earnings
render as TextInputField type="number", so the values are strings and the cause
is entirely in this builder.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant