refactor(onboarding): build the basic information schema once - #1430
gabrielseco wants to merge 3 commits into
Conversation
Basic information no longer rebuilds its headless form from the current values on every change. The form is built once and conditional fields are resolved through handleValidation, the same way the jsf v1 contract details already work. Pre-filled values (employment data or the initialValues prop) are validated when the step mounts, and before a read-only employment jumps straight to review, so conditional fields such as seniority_date still show without the user typing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
📦 Bundle Size Report
Size Limits
Largest Files (Top 5)
View All Files (75 total)
✅ Bundle size check passed |
|
Deploy preview for remote-flows ready!
Deployed with vercel-action |
…ebuilt A new jsfModify reference, for example options passed inline and a parent re-render, rebuilds the basic information form with every field back in its default visibility. Replay the last validated values onto the new form so conditional fields such as seniority_date stay visible. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Deploy preview for adp-cost-calculator ready!
Deployed with vercel-action |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
📊 Coverage Report✅ Coverage increased! 🎉
Detailed BreakdownLines Coverage
Statements Coverage
Functions Coverage
Branches Coverage
✅ Coverage check passed |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit de2efb7. Configure here.
| ); | ||
| return basicInformationForm?.handleValidation(parsedValues); | ||
| lastBasicInformationValuesRef.current = parsedValues; | ||
| const result = basicInformationForm?.handleValidation(parsedValues); |
There was a problem hiding this comment.
Stripped values hide nested conditionals
Medium Severity
Visibility for basic information now depends on handleValidation, but values are parsed with isPartialValidation: false and then stored in lastBasicInformationValuesRef. That drops currently hidden fields, so a nested conditional cannot see the parent value it needs on mount or after a form rebuild. The v1 contract-details path keeps invisible values for this reason, and the new review jump already uses isPartialValidation: true.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit de2efb7. Configure here.
| onboardingBag?.checkFieldUpdates(form.getValues()); | ||
| } | ||
| // Pre-filled values need to reach useStepState and be validated so conditional | ||
| // fields show before the user types. |
There was a problem hiding this comment.
Multi-line implementation comment
Low Severity
The new comment in OnboardingForm spans two lines. Implementation comments are limited to a single short line that explains why, with no multi-line blocks.
Triggered by project rule: Code Review Guidelines
Reviewed by Cursor Bugbot for commit de2efb7. Configure here.
1 participant
Summary
The Onboarding basic information step now builds its form once instead of rebuilding it every time a value changes. Nothing changes for users: conditional fields still show from pre-filled data and from what the user types.
Why
Rebuilding the form on every change is the old pattern. It's also where #1195's money-unit regression came from, fixed in #1429. The jsf v1 contract details (FRA, ITA, DEU, ESP) already build once and rely on validation to show or hide fields. This moves basic information to the same pattern as a first, small step, to see what breaks before doing the other steps.
What changed
Toggle details
useBasicInformationSchemainOnboarding/api.ts, modelled onuseContractDetailsSchema: fetchesemployment_basic_information, builds the form once with{}andtransformMoneyFields: false.useJSONSchemaFormis left untouched; it's still used for legacy contract details, ContractorOnboarding andJsonSchemaComparison.checkFieldUpdatesnow runshandleValidationon basic information too, not only on jsf v1 contract details. The basic information branch ofhandleValidationforces a re-render like the v1 branch does, so newly visible fields appear.OnboardingFormmount effect now always callscheckFieldUpdates(form.getValues()), not only when there's anemploymentId. Every step after basic information already has anemploymentId, so in practice this only adds basic information on a fresh onboarding (whereinitialValuescan pre-fill it) and select country.jsfModifyreference (e.g. a partner passingoptionsinline, then a parent re-render) rebuilds the form with every field back in its default visibility. The legacy path didn't have this problem because it rebuilt with the current values. The last validated values are now replayed onto the rebuilt form during render, so nothing flashes.seniority_datedisappeared from the review.Public API: no signature changes. One behaviour to look at: a headless
useOnboarding()consumer with a custom form that never callscheckFieldUpdateson mount would no longer get conditional fields resolved from pre-filled basic information values. The legacy path did it by rebuilding the form. The jsf v1 contract details already have this requirement today.Known limitation: the mount validation runs in a
useEffect, so visibility is corrected right after the first paint. The jsf v1 contract details behave the same.Screenshots
N/A
Related Resources
Testing
New tests in
OnboardingFlow.test.tsx(basic information conditional fields from pre-filled values), using Portugal'shas_seniority_date→seniority_dateconditional:seniority_datestays hidden.initialValuesprop on a fresh onboarding:seniority_dateshows without typing.employmentIdwith seniority data:seniority_dateshows without typing.seniority_dateis in the review.optionsobject:seniority_datestays visible (passes on the legacy path, failed on this PR before the replay fix).Each fix was removed one at a time to confirm a test fails without it:
OnboardingForminitialValues,employmentIdhandleValidationincheckFieldUpdatesfor basic informationinitialValues,employmentIdoptionsre-renderexample/app in a browser🤖 Generated with Claude Code
Note
Medium Risk
Touches core onboarding form lifecycle (schema fetch, conditional visibility, auto-review navigation) for a high-traffic step; behavior is intended to be unchanged but custom headless consumers that never call
checkFieldUpdateson mount may lose pre-filled conditional fields.Overview
Onboarding basic information now follows the same “build schema once, drive visibility via validation” pattern as JSF v1 contract details, instead of rebuilding the headless form on every value change.
useBasicInformationSchema(inapi.ts, modeled onuseContractDetailsSchema) fetchesemployment_basic_informationand callscreateHeadlessFormwith empty values andtransformMoneyFields: false.useOnboardingwires that hook in place of the per-changeuseJSONSchemapath.To keep conditional fields correct without rebuilds:
OnboardingFormalways runscheckFieldUpdateson mount so pre-filled/initialValuesdata is validated before the user types;checkFieldUpdatesalso runshandleValidationon the basic information step (with a re-render bump like contract details v1). Whenoptions/jsfModifychanges and the form is rebuilt, the last validated values are replayed from a ref so fields likeseniority_datedo not disappear. Read-only employments that skip straight to review validate basic information against initial values before building review meta so conditionally visible fields still appear on review.New tests cover Portugal
has_seniority_date→seniority_datefor scratch,initialValues,employmentId, review skip, and inlineoptionsre-render.Reviewed by Cursor Bugbot for commit de2efb7. Bugbot is set up for automated code reviews on this repo. Configure here.