fix(SDK-1285): reject negative additional-earning amounts before submit - #2651
Draft
krisxcrash wants to merge 2 commits into
Draft
fix(SDK-1285): reject negative additional-earning amounts before submit#2651krisxcrash wants to merge 2 commits into
krisxcrash wants to merge 2 commits into
Conversation
The fields already pass min={0}, but a native min only constrains the stepper --
it does not stop a typed "-50" -- and the SDK's <form> is noValidate, so the
browser never enforces it either. The form schema accepted any string, so negative
amounts went to the platform and came back only as a submit-time error.
Adds a superRefine over the fixedCompensations record that flags any negative
value at field level, following the pattern SDK-1225 established for draft
reimbursements: a zod rule plus an errorMessage on the field, which useField only
surfaces once react-hook-form has actually flagged it. Server validation stays as
the backstop.
The rule covers every additional earning, not just correction payment: they all
share the fixedCompensations record, so one rule and one message is the natural
unit. Empty and zero values are untouched. hourlyCompensations and
timeOffCompensations have the same min-without-validation gap but are separate
fields with different rules, and are out of scope here.
Note for reviewers: the ticket said the SDK "isn't supplying min={0}". It already
does; the gap is that a native min cannot stop typed input.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes SDK-1285 (Medium, Pay v2 / RRoP Test Fest).
Correction to the ticket
The ticket says the SDK "isn't supplying
min={0}". It already does — the correction-payment field and every other additional earning passmin={0}today.The real gap: a native
minonly constrains the stepper. It does not stop a typed-50, and the SDK's<form>setsnoValidate, so the browser never enforces it either. The form schema accepted any string, so negative amounts reached the platform and came back only as a submit-time error."Correction payment" is
COMPENSATION_NAME_CORRECTION_PAYMENT, rendered in the same additional-earnings grid as bonus/tips/commission.Fix
Follows the pattern SDK-1225 established for draft reimbursements: a zod rule plus an
errorMessageon the field.useFieldreturnsisInvalid ? (errorMessage ?? fieldState.error?.message) : undefined, so a statically-passederrorMessageonly surfaces once react-hook-form has actually flagged the field — no conditional logic, and no need to threadtinto a module-level schema.superRefineover thefixedCompensationsrecord, adding an issue at['fixedCompensations', name]for any value parsing to< 0. A record needssuperRefinerather than a per-valuerefinebecause the keys are dynamic.validations.negativeAmountalongside the existingvalidations.reimbursementAmount.src/i18n/types.d.ts(CI'sderive-check-i18nfails if stale).Server validation is untouched and remains the backstop, which is the second AC.
Scope
The rule covers every additional earning, not just correction payment — they all share the
fixedCompensationsrecord, so one rule and one message is the natural unit rather than special-casing one key. Empty and zero values are untouched.hourlyCompensationsandtimeOffCompensationshave the same min-without-validation gap, but they are separate fields with different rules and are not touched here.Testing
Three new cases:
I verified the first two fail when the
superRefineis removed. The correction-payment case also confirms the error path resolves for a field name containing a space (fixedCompensations.Correction Payment), which was the non-obvious part.npx vitest run src/components/Payroll/PayrollEditEmployee— 71 pass.npm run tsc, eslint, prettier — clean.Merge note
SDK-1284 touches the same file and the same schema/builder block. Whichever of the two merges second needs a small rebase there.