Repository navigation
test(form): move jsf engine situations into their own file and document when to add one - #1443
Merged
gabrielseco merged 2 commits intoOct 5, 2026
Merged
Conversation
…nt when to add one Form behaviour bugs (visibility, computed values, money conversion, initialValues, jsfModify) should be reproduced as a situation in the jsf engine contract test instead of a per-country integration test. The rule now lives in CLAUDE.md, the BUGBOT review guide and the useHeadlessForm rollout doc, and situations live in jsfEngineSituations.ts so adding one doesn't touch the test logic. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
📦 Bundle Size Report
Size Limits
Largest Files (Top 5)
View All Files (283 total)
✅ Bundle size check passed |
Contributor
|
Deploy preview for remote-flows ready!
Deployed with vercel-action |
Contributor
|
Deploy preview for adp-cost-calculator ready!
Deployed with vercel-action |
Contributor
📊 Coverage Report⚪ Coverage unchanged
Detailed BreakdownLines Coverage
Statements Coverage
Functions Coverage
Branches Coverage
✅ Coverage check passed |
…in the jsf engine situations Each situation now uses a schema that only contains the fields the behaviour needs, matching the guidance in CLAUDE.md and the rollout doc, and gives the next contributor an inline example to copy that works on jsf v0, v1 and schemas without meta. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 tasks
gabrielseco
added a commit
that referenced
this pull request
Oct 5, 2026
…ic information onto it (#1433) * fix(onboarding): submit computed money forced values in the right unit Portugal's extended work hours allowance is a forced money value computed from the salary. Onboarding built its render-time form without converting money fields to cents, so the allowance was computed from the salary in major units (100x too small), and the forced value was then written into the form state in cents while the form keeps money in major units. Validation recomputed the correct value and rejected the submission on a field with no input to show the error, blocking the step. - Onboarding's useJSONSchemaForm passes transformMoneyFields: true, since createHeadlessForm only defaults it on when no options are passed. - Forced money values are converted from cents before being set in the form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(forms): default transformMoneyFields per key Default transformMoneyFields to true whenever it's not explicitly set, instead of only when the whole options object is missing. Callers that pass options without transformMoneyFields (e.g. only jsfModify) now get money fields converted to cents too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(invoice-schedules): drop redundant money flag Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(onboarding): cover money forced values on the legacy contract details path The flow regression test only covers the legacy path while PRT is outside JSF_V1_CONTRACT_DETAILS_COUNTRIES. These test the Onboarding useJSONSchemaForm hook and JSONSchemaFormFields directly, so they keep covering the fix whichever countries are on v1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(form): drop getForcedValue Submitting a forced value already uses the field's `const` in cents (parseFormValuesToAPI overwrites the form value with it), and the default forced value component doesn't render `value`. The `transformMoneyFields` change alone fixes the PRT allowance; getForcedValue would only change what custom forcedValue components receive in `fieldData.value`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(form): pin that forced values are submitted as their const Correct submission of forced money values relies on parseFormValuesToAPI overwriting the form value with the field's const. Covers a top-level field and one nested in a fieldset; both fail with 100x the amount without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(onboarding): drop explicit transformMoneyFields createHeadlessForm now defaults transformMoneyFields per key (#1426), so the explicit override in useJSONSchemaForm is redundant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(form): drop parseSubmitValues forced value test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(form): add useHeadlessForm with rebuild and buildOnce strategies Every flow wires createHeadlessForm, parseJSFToValidate and handleValidation by hand, and each copy can drift on its own (the money unit regression lived in one of them). useHeadlessForm puts that wiring behind one interface with the two strategies the codebase uses today: - rebuild: rebuild the form whenever values change (current behaviour of most call sites). - buildOnce: build once per schema/options and resolve conditionals through handleValidation. It validates once after every build, so visibility is right without a mounted step and an inline jsfModify reference does not reset it; a replaced form's pending validation is cancelled before it mutates the old fields. The engine contract test renders the real useJSONSchemaForm + JSONSchemaFormFields (so ForcedValueField's setValue loop runs) and runs each schema situation across jsf v0, jsf v1, no meta and both strategies. New situations are one row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(onboarding): build legacy contract details through useHeadlessForm First call site on the shared hook, with the rebuild strategy so behaviour is unchanged. useLegacyContractDetailsSchema fetches the raw schema and leaves building to the hook; the shared useJSONSchema stays as it is for the other steps. mergedFormValues moves out of that wrapper so both can use it. The existing PRT extended hours allowance flow test covers this step: it fails when the hook's rebuild stops converting money to cents. The jsonSchemaVersion test's mocked schema gains x-jsf-presentation, like every real schema has: the hook builds during render instead of inside a React Query select, so the money conversion's lookup of it now surfaces instead of turning into a silent query error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(form): stop buildOnce from rebuilding on every render with inline options buildOnce rebuilt the form whenever the options object changed identity, and every build validates and re-renders. A caller that creates options during render (as the Onboarding contract details hooks do) looped forever: 132 renders in 500ms in a probe, "Too many re-renders" in the test. Options are now compared by value with fast-deep-equal, the helper useJSONSchemaForm already uses. Functions and components inside jsfModify keep their identity across our own renders, so they compare equal by reference. React elements cannot reach jsfModify: the jsf engine deep-clones it and rejects them, so the React-aware variant is not needed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(onboarding): build jsf v1 contract details through useHeadlessForm The v1 contract details (FRA, ITA, DEU, ESP) already built the form once and resolved conditionals through handleValidation with invisible values kept, which is the hook's buildOnce strategy. useContractDetailsSchema now hands building, validation and submit parsing to the hook, so contract details covers both strategies: the legacy step with rebuild, v1 with buildOnce. The fieldsCount state that forced a re-render after each v1 validation goes away, since the hook re-renders itself. The comment on why invisible values are kept moves with that decision into the hook. The replay after a rebuild uses the step's form values, not the server employment data, so money is never converted from cents a second time on this path. France's flow tests and useOnboardingJsfV1ContractDetails fail when the hook's buildOnce validation is broken, so they cover this step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(onboarding): refetch the contract details schema per employment Both contract details schema requests send employment_id, but their query keys only held the country and schema version, so React Query served the first employment's schema to every other employment of the same country in the session, and never refetched once an employment was created mid-flow. The keys now include the request query. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(form): cover options shapes in the jsf engine contract The contract only built forms without options, which is the one shape where createHeadlessForm's money default is irrelevant, so reverting that default to the old `options || { transformMoneyFields: true }` stayed green here and was only caught by flow and hook tests. Run every situation with no options, with an options object carrying no jsfModify (what Onboarding passes when the consumer gives none), and with a real jsfModify whose effect is asserted. Mutation checked: the old default fails the 6 rebuild money rows, and dropping options inside useHeadlessForm fails the 6 jsfModify rows. The Onboarding useJSONSchemaForm money forced value test is removed: its engine check is now in the contract and its wiring is covered end to end by the OnboardingFlow allowance-in-cents test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(form): add useHeadlessForm rollout plan Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(form): seed buildOnce forms from initialValues and move basic information onto useHeadlessForm useHeadlessForm's buildOnce strategy now takes initialValues and resolves conditional visibility at build time instead of replaying values in a post-mount effect. Rebuilds triggered by jsfModify reuse the last validated values for the same schema, while a new schema starts from initialValues. Onboarding's basic_information step now goes through useBasicInformationSchema (buildOnce) and contract details receives saved employment values as initialValues rather than the live fieldValues. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(form): fold useHeadlessForm lifecycle cases into the jsf engine contract test The buildOnce rebuild/seed rules (initialValues on first render and when they arrive late, last validated values surviving a jsfModify change and beating initialValues, a new schema starting from initialValues) now run through the real form on jsf v0, jsf v1 and no meta, instead of a renderHook test on the jsf v1 fixture only. The render-count guard is dropped: the hook no longer validates after each build, so recreated options cannot cause a render loop anymore. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(form): mark basic information as migrated to buildOnce in the rollout Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(onboarding): drop the contract details employment_id query key test Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(form): move jsf engine situations into their own file and document when to add one (#1443) * test(form): move jsf engine situations into their own file and document when to add one Form behaviour bugs (visibility, computed values, money conversion, initialValues, jsfModify) should be reproduced as a situation in the jsf engine contract test instead of a per-country integration test. The rule now lives in CLAUDE.md, the BUGBOT review guide and the useHeadlessForm rollout doc, and situations live in jsfEngineSituations.ts so adding one doesn't touch the test logic. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(form): replace the Portugal fixture with minimal inline schemas in the jsf engine situations Each situation now uses a schema that only contains the fields the behaviour needs, matching the guidance in CLAUDE.md and the rollout doc, and gives the next contributor an inline example to copy that works on jsf v0, v1 and schemas without meta. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(architecture): describe the four test layers and the planned smoke tests (#1444) Adds a Test Layers section to the testing chapter: what unit tests and schema situations, integration, E2E and smoke tests each catch, where a bug's test belongs, and how failures found by the slow layers turn into situations. Smoke tests are marked as planned (headless, scheduled, against sandbox), with what to copy from Platform's FE smoke tests and what to avoid. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Andrea Maria Piana <andrea.maria.piana@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Gives form bugs one obvious place to get a regression test, so a schema bug is reproduced once instead of in an integration test for every flow and country.
Why
CLAUDE.mdfor agents, BUGBOT for review, and the rollout doc for people.What changed
Toggle details
src/common/tests/jsfEngineSituations.ts(new):SITUATIONS, theSituation/Schematypes andchooseInGroup, moved out of the test file. The Portugal contract details fixture is replaced by two minimal inline schemas:signingBonusSchema(a radio reveals a money field) and a monthly allowance schema (a money value computed from another money field throughx-jsf-logic). The second keeps the computed-value path the Portugal case exercised: 1234.56 EUR a year becomes a forced 102.88 EUR (10288 cents) a month.jsfEngineContract.test.tsx: imports them, and thebuildOnce lifecycletests usesigningBonusSchemainstead of Portugal. Assertions are unchanged apart from the new allowance figures. All 51 cases pass on jsf v0, v1 and no meta, and they fail whensetRevisionis removed from the hook.CLAUDE.md: a Conventions bullet. Form behaviour bugs (visibility, computed/forced values, money conversion,initialValues,jsfModify) are reproduced as a failing situation first. Prefer a minimal inline schema, name situations after the behaviour, and keep integration tests for flow wiring..cursor/BUGBOT.md: a Testing Issues bullet flagging such fixes that lack a situation, or one that doesn't fail without the fix.docs/USE_HEADLESS_FORM_ROLLOUT.md: an "Adding a situation" section covering each field of a situation, red-first, and when a case belongs in thebuildOnce lifecycleblock instead.No public API change. Stacked on #1433.
Screenshots
N/A
Related Resources
Testing
example/app in a browser: N/A, test and docs only🤖 Generated with Claude Code
Note
Low Risk
Test and documentation changes only; no runtime or public API surface.
Overview
Centralizes JSON Schema form regression cases in
jsfEngineSituations.tsand makes “add a situation first” the expected workflow for form-engine bugs.jsfEngineContract.test.tsxnow importsSITUATIONS, shared helpers (chooseInGroup), and schemas from that module instead of inlining them. Situations use minimal inline schemas (e.g. signing bonus visibility, computed monthly allowance) rather than the Portugal contract fixture;buildOncelifecycle tests likewise usesigningBonusSchema.Docs and review policy are aligned:
CLAUDE.md,.cursor/BUGBOT.md, anddocs/USE_HEADLESS_FORM_ROLLOUT.mdexplain when to add a situation (visibility, computed/forced values, money,initialValues,jsfModify), how to write one red-first, and that flow integration tests stay for wiring/API/submit—not duplicate per-country schema behaviour.Reviewed by Cursor Bugbot for commit 3d5b443. Bugbot is set up for automated code reviews on this repo. Configure here.