Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8e4776f
fix(onboarding): submit computed money forced values in the right unit
cammellos Sep 29, 2026
ccf0275
refactor(forms): default transformMoneyFields per key
gabrielseco Sep 29, 2026
77136fc
refactor(invoice-schedules): drop redundant money flag
gabrielseco Sep 29, 2026
c28068d
test(onboarding): cover money forced values on the legacy contract de…
gabrielseco Sep 29, 2026
6896273
refactor(form): drop getForcedValue
gabrielseco Sep 29, 2026
2d2809d
test(form): pin that forced values are submitted as their const
gabrielseco Sep 29, 2026
a5b4d35
Merge PR #1426 (refactor/create-headless-form-money-default)
gabrielseco Sep 29, 2026
8ac5bcb
refactor(onboarding): drop explicit transformMoneyFields
gabrielseco Sep 29, 2026
39b2adc
test(form): drop parseSubmitValues forced value test
gabrielseco Sep 29, 2026
8e1421d
refactor(form): add useHeadlessForm with rebuild and buildOnce strate…
gabrielseco Sep 30, 2026
c3cc3b1
refactor(onboarding): build legacy contract details through useHeadle…
gabrielseco Sep 30, 2026
fbdf5a6
fix(form): stop buildOnce from rebuilding on every render with inline…
gabrielseco Sep 30, 2026
f1e9034
refactor(onboarding): build jsf v1 contract details through useHeadle…
gabrielseco Sep 30, 2026
60b4ecb
fix(onboarding): refetch the contract details schema per employment
gabrielseco Sep 30, 2026
e5fc865
test(form): cover options shapes in the jsf engine contract
gabrielseco Sep 30, 2026
5575dbc
docs(form): add useHeadlessForm rollout plan
gabrielseco Sep 30, 2026
956e0f2
Merge branch 'main' into refactor/use-headless-form
gabrielseco Oct 5, 2026
b718cf6
refactor(form): seed buildOnce forms from initialValues and move basi…
gabrielseco Oct 5, 2026
11dc6f5
test(form): fold useHeadlessForm lifecycle cases into the jsf engine …
gabrielseco Oct 5, 2026
d3e0a35
docs(form): mark basic information as migrated to buildOnce in the ro…
gabrielseco Oct 5, 2026
53da62f
test(onboarding): drop the contract details employment_id query key test
gabrielseco Oct 5, 2026
587aa4c
test(form): move jsf engine situations into their own file and docume…
gabrielseco Oct 5, 2026
0cafa51
docs(architecture): describe the four test layers and the planned smo…
gabrielseco Oct 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .cursor/BUGBOT.md
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ flows/[FlowName]/
- Missing `waitFor` for async assertions
- Not cleaning up React Query cache between tests
- Testing implementation details instead of behavior
- A fix for JSON Schema form behaviour (conditional visibility, computed or forced values, money conversion, `initialValues`, `jsfModify`) without a new situation in `src/common/tests/jsfEngineSituations.ts`, or reproduced only through a flow integration test. The situation must fail without the fix

### 9. Build and Release Issues

Expand Down
44 changes: 40 additions & 4 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -1099,7 +1099,43 @@ Consumers can override text in JSON schemas without code changes:

## 7. Testing Philosophy

### 7.1 Vitest Globals
### 7.1 Test Layers

Four layers, each catching a kind of failure the others can't. A layer earns its place only if it does.

| Layer | Catches | Runs against | When |
| ---------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------ | ----------------- |
| **Unit (schema situations)** | How a JSON Schema form behaves: conditional fields, computed or forced values, money conversion, `initialValues`, `jsfModify` | Minimal inline schemas | Every PR |
| **Integration** | Flow wiring: submitting, going back, moving between steps, error handling | Fixture schemas, endpoints mocked with MSW | Every PR |
| **E2E** | Whether the SDK and the backend still work together on the happy path | Sandbox | Every PR, nightly |
| **Smoke** (planned) | Real country schemas using something no situation covers yet | Every country's real schema, in sandbox | Scheduled |

**Unit.** [`jsfEngineSituations.ts`](src/common/tests/jsfEngineSituations.ts) lists situations: a small schema, what the user does, and what they should see and submit. [`jsfEngineContract.test.tsx`](src/common/tests/jsfEngineContract.test.tsx) runs each one against every jsf engine and every `useHeadlessForm` strategy. The idea comes from json-schema-form's own tests: schema in, behaviour out, no flow around it. How to add one: [Adding a situation](docs/USE_HEADLESS_FORM_ROLLOUT.md#adding-a-situation). Plain helper tests, such as [`dates.test.ts`](src/common/tests/dates.test.ts), sit at this layer too.

**Integration.** Full flows rendered with the endpoints mocked (see [7.3](#73-msw-for-api-mocking)). This is where several things are tested together: a step submits, the next one loads, going back keeps the values.

**E2E.** Playwright in [`example/e2e/`](example/e2e/), driving the deployed example app against the sandbox environment, on every PR and nightly ([`e2e-nightly.yml`](.github/workflows/e2e-nightly.yml)). Few tests, happy paths only. When a PR's own unit and integration tests pass but E2E fails, the backend has usually changed.

**Smoke (planned, not built yet).** Every country's contract details schema, fetched from sandbox. For each one: generate values the schema accepts, run them through `useHeadlessForm` and `parseFormValues`, and check that sandbox accepts the payload. Headless, no UI. It runs on a schedule rather than on PRs: running every country is too slow for each PR, and one country's schema changing in sandbox shouldn't block unrelated work. A green run means sandbox schemas work with the SDK; sandbox can differ from production.

Platform's FE already has smoke tests: Playwright, one test per country, run on merge requests against a real backend in CI. Ours would differ in running headless and on a schedule, but three things are worth copying:

- Values are generated from the schema, not written per country: fill the fields, validate, and repeat until the schema accepts them.
- Each country is its own test, so one broken schema doesn't hide the others.
- On failure, the generator is re-run on the saved schema to tell an SDK regression apart from a backend schema regression.

One thing to avoid: about 41 of the ~91 countries in Platform's FE smoke list are commented out, many with "flaky for some reason that is unknown". Skipped countries here should each have a reason and a ticket, so the list shrinks instead of growing.

#### Where a bug's test goes

- Reproducible with a schema alone → a unit situation.
- Needs steps, navigation or API calls → an integration test.
- Needs the real backend → E2E.
- Only some countries break and you don't know which → smoke finds it.

Failures move down the layers. When E2E or smoke finds a schema bug, the fix comes with a situation that reproduces it on a minimal schema. That keeps the slow layers finding new problems instead of guarding old ones.

### 7.2 Vitest Globals

Vitest globals are **enabled** in `vitest.config.ts`:

Expand Down Expand Up @@ -1132,7 +1168,7 @@ describe('MyComponent', () => {

**Why:** Globals reduce boilerplate. This is a project convention - follow it.

### 7.2 MSW for API Mocking
### 7.3 MSW for API Mocking

All API calls are mocked using MSW (Mock Service Worker):

Expand Down Expand Up @@ -1184,7 +1220,7 @@ export const mockEmployment = {
};
```

### 7.3 Strict Equality Assertions
### 7.4 Strict Equality Assertions

Always use **exact object matching**, never `expect.objectContaining()`:

Expand All @@ -1204,7 +1240,7 @@ expect(mockApiCall).toHaveBeenCalledWith({

**Why:** `objectContaining` lets tests pass even if we're passing extra unexpected fields. Strict equality catches bugs.

### 7.4 QueryClient Management
### 7.5 QueryClient Management

React Query tests must wrap components in a provider:

Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ Tailwind v4 (`@tailwindcss/postcss`) + CSS variables. Public component classes a
- **Strict equality in mocks** — use `toHaveBeenCalledWith({ exact: 'shape' })`, not `expect.objectContaining()`.
- **MSW for API mocking** in tests — never `vi.mock('@/src/client')`. Override endpoints with `server.use(http.get(...))` inside `beforeEach`; defaults live in [src/tests/handlers.ts](src/tests/handlers.ts) and are reset after each test by [src/tests/server.ts](src/tests/server.ts). This holds for headless-hook tests (`renderHook` + `useFlowName()`) exactly as much as for rendered-flow tests — [OnboardingFlowGermanyDynamicSteps.test.tsx](src/flows/Onboarding/tests/OnboardingFlowGermanyDynamicSteps.test.tsx) and [useOnboardingJsfV1ContractDetails.test.tsx](src/flows/Onboarding/tests/useOnboardingJsfV1ContractDetails.test.tsx) are the references. Mocking the generated client bypasses the real query/serialization path and silently drops any endpoint the flow starts calling (the server is set to `onUnhandledRequest: 'error'`, so MSW surfaces those instead). Reserve `vi.fn()` for consumer callbacks (`onSubmit`, `onSuccess`, `render`) and for the rare test that asserts on request _arguments_ rather than behaviour.
- **Schema fixtures already include the response envelope** — a `contractDetailsSchemaV1<Country>` is `{ data: <schema> }`, so a handler is just `HttpResponse.json(fixture)`. Add new ones under `tests/fixtures/<form>/` and re-export from that folder's `index.ts`.
- **Form behaviour bugs get a situation, not an integration test** — when the bug is in how a JSON Schema form behaves (conditional visibility, computed or forced values, money conversion, `initialValues`, `jsfModify`), reproduce it first as a failing entry in [jsfEngineSituations.ts](src/common/tests/jsfEngineSituations.ts), then fix it. Each situation runs against every jsf engine and `useHeadlessForm` strategy in [jsfEngineContract.test.tsx](src/common/tests/jsfEngineContract.test.tsx). Use a minimal inline schema unless the bug depends on a specific country's fixture, and name the situation after the behaviour, not the country or ticket. Integration tests are for flow wiring (steps, API calls, submit payloads). How to write one: [docs/USE_HEADLESS_FORM_ROLLOUT.md](docs/USE_HEADLESS_FORM_ROLLOUT.md#adding-a-situation).
- **Tests must wrap React Query consumers** in a `QueryClientProvider` and `queryClient.clear()` between tests. Use the shared `queryClient` + `TestProviders` from [src/tests/testHelpers.tsx](src/tests/testHelpers.tsx) rather than constructing a local `QueryClient`.
- **`no-console: warn`** in oxlint — only `console.warn` / `console.error` permitted.
- **No `any`** — `typescript/no-explicit-any: error`. Use the `$TSFixMe` type from `src/types/remoteFlows.ts` for genuine workarounds.
Expand Down
101 changes: 101 additions & 0 deletions docs/USE_HEADLESS_FORM_ROLLOUT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# useHeadlessForm rollout

Every flow builds its JSON Schema forms through one shared hook, [`src/common/useHeadlessForm.ts`](../src/common/useHeadlessForm.ts), instead of calling `createHeadlessForm` directly. Once that's done, every form builds once and resolves conditional fields through validation. This doc tracks the migration so it can happen over many PRs.

## Why

- Each flow has its own hand-written copy of the build and validate logic. Each copy can break on its own (the money-in-cents bug fixed in #1429 is one example), and no shared test catches it.
- With one hook, [`jsfEngineContract.test.tsx`](../src/common/tests/jsfEngineContract.test.tsx) tests schema situations once, across engines and strategies. Each flow only needs one smoke test proving it goes through the hook.
- Rebuilding the form on every change is why the money pre-fill conversion and `transformMoneyFields` exist. Building once removes the need for both.

## Strategies

- **`rebuild`** (old): `createHeadlessForm(schema, values, options)` again whenever values change. Money values are converted to cents at build time.
- **`buildOnce`** (new): built once per `schema`/`options`, with `transformMoneyFields: false`. Conditionals are resolved by `handleValidation` with `isPartialValidation: true`. The first build is seeded with `initialValues` (saved values in API units), so visibility is right without a mounted step. When `jsfModify` changes, the form is rebuilt from the last validated values; a new schema starts again from `initialValues`.

## Adding a situation

When a form misbehaves because of the schema or the hook (a field doesn't appear, a computed value is wrong, money is converted twice, `initialValues` or `jsfModify` are lost), reproduce it as a situation before fixing it. A situation runs against every engine and strategy in [`jsfEngineContract.test.tsx`](../src/common/tests/jsfEngineContract.test.tsx), so one entry covers what would otherwise take an integration test per flow and country.

Add an entry to `SITUATIONS` in [`jsfEngineSituations.ts`](../src/common/tests/jsfEngineSituations.ts):

- `situation`: the behaviour, e.g. "fieldset hidden by a radio keeps its children's values". Not the country, the flow or the ticket.
- `schema`: the smallest schema that shows the bug, written inline. Use a country fixture only when the bug depends on that country's schema. Leave out `x-rmt-meta`; the test adds one per engine.
- `fill`: what the user does, with `userEvent` and `chooseInGroup`.
- `assert`: what the user sees and what `parseFormValues` returns.

Run it before the fix and check that it fails. A situation that passes either way doesn't protect anything.

Behaviour that only exists in one strategy (rebuilding when `initialValues` or `jsfModify` change, falling back when the schema changes) goes in the `buildOnce lifecycle` block of the test instead.

Keep integration tests for flow wiring: steps, API calls and submit payloads.

## Phases

### Phase 1: move every call site onto the hook, same behaviour

Pure refactor. Each call site keeps the strategy it effectively has today, so nothing a user sees should change. Each PR adds one smoke test per flow. The test must fail when the hook is broken, not just pass with it.

Call sites that don't pass values already behave like `buildOnce`, so they go straight onto it.

A `rebuild` step can also skip this phase and go straight onto `buildOnce` when it passes the phase 2 checklist. Basic information did this in #1433.

### Phase 2: switch `rebuild` steps to `buildOnce`, one step per PR

For each step, test all of these before switching (they come from #1430):

- [ ] Starting from scratch
- [ ] `initialValues` provided by the partner
- [ ] Existing `employmentId` (server values already in cents, so they must not be converted twice)
- [ ] Read-only employment that jumps straight to review without mounting the step (`prettifyFormValues` drops invisible fields)
- [ ] Partner passes `options` inline, which creates a new `jsfModify` reference on every render
- [ ] Headless `use<Flow>()` consumers with a custom UI, which don't mount our step components

### Phase 3: cleanup

- [ ] Remove the `rebuild` strategy from `useHeadlessForm`
- [ ] Remove `transformMoneyFields` and the pre-fill money conversion from `createHeadlessForm`
- [ ] Add an oxlint `no-restricted-imports` rule that bans importing `createHeadlessForm` in `src/flows/**`

## Call sites

Status: `todo`, `phase 1` (on the hook, old behaviour), `done` (on the hook with `buildOnce`).

### Pass values (rebuild today)

| Call site | Status | Notes |
| ------------------------------------------------------------------ | ------- | ------------------------------------------------------------------------------------------------------------------------------ |
| Onboarding `useLegacyContractDetailsSchema` | phase 1 | #1433. Only building moved; validation and submit parsing still go through the hand-written branches in `Onboarding/hooks.tsx` |
| Onboarding basic information `useBasicInformationSchema` | done | #1433. Went straight to `buildOnce`; building, validation and submit parsing all go through the hook |
| Onboarding `useJSONSchemaForm` | todo | No Onboarding step uses it anymore; only `JsonSchemaComparison` does |
| Onboarding `useBenefitOffersSchema` | todo | |
| Onboarding `useEngagementAgreementDetailsSchema` | todo | |
| Contractor contract details `useContractorOnboardingDetailsSchema` | todo | Money-sensitive |
| ContractorOnboarding `useGetContractDocumentSignatureSchema` | todo | |
| ContractorOnboarding `useGetEligibilityQuestionnaire` | todo | |
| ContractorOnboarding `useGetContractOriginSchema` | todo | |
| Invoice schedules `useGetCreateInvoiceScheduleSchema` | todo | Money-sensitive |
| ContractAmendment `useContractAmendmentSchemaQuery` | todo | Money-sensitive |
| CreateCompany `useAddressDetailsSchema` | todo | |
| Termination `useTerminationSchema` | todo | |
| PayrollAdminOnboarding `useGPFormSchema` | todo | |
| PayrollAdminOnboarding `useGPCountrySelectSchema` | todo | |
| PayrollEmployeeOnboarding `useGPEmployeeFormSchema` | todo | |
| JsonSchemaPlayground `useJsonSchemaPlayground` | todo | Only exported from `internals`, so it's the lowest priority |

### Already build once (no values)

| Call site | Status | Notes |
| ----------------------------------------------------------- | ------ | --------------------------------------------------------- |
| Onboarding `useContractDetailsSchema` (jsf v1) | done | #1433 |
| Onboarding `useCountriesSchemaField` | todo | |
| ContractorOnboarding `useCountriesSchemaField` | todo | |
| ContractorOnboarding `useContractorSubscriptionSchemaField` | todo | |
| ContractorOnboarding `useGetInvoiceScheduleSchema` | todo | |
| CreateCompany `useCountriesSchemaField` (basic information) | todo | |
| CostCalculator `useStaticSchema` + `useRegionFields` | todo | Combines several forms; needs a design per sub-form first |
| Termination full schema in `useTermination` | todo | Combines several forms; needs a design per sub-form first |

## Known behaviour until phase 2

On a prefilled legacy contract details step, the first render converts server values that are already in cents a second time. A probe on a PRT employment computed the allowance as `8125770` before it settled on `81257` once the step mounted. Users never see this. Anything that reads the form before the step mounts could, and that hasn't been verified. The problem goes away once that step switches to `buildOnce`.
Loading
Loading