From 19e0310df73cfc37c4ba73ad0e87c23fe2cb368d Mon Sep 17 00:00:00 2001 From: Gabriel Garcia Date: Mon, 5 Oct 2026 14:05:47 +0200 Subject: [PATCH 1/2] 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) --- .cursor/BUGBOT.md | 1 + CLAUDE.md | 1 + docs/USE_HEADLESS_FORM_ROLLOUT.md | 17 +++++ src/common/tests/jsfEngineContract.test.tsx | 70 +++------------------ src/common/tests/jsfEngineSituations.ts | 63 +++++++++++++++++++ 5 files changed, 90 insertions(+), 62 deletions(-) create mode 100644 src/common/tests/jsfEngineSituations.ts diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index ea31288c8..5b1fe1fda 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -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 diff --git a/CLAUDE.md b/CLAUDE.md index 7f27dff0d..fab38da48 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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` is `{ data: }`, so a handler is just `HttpResponse.json(fixture)`. Add new ones under `tests/fixtures/
/` 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. diff --git a/docs/USE_HEADLESS_FORM_ROLLOUT.md b/docs/USE_HEADLESS_FORM_ROLLOUT.md index 29805a5cd..56c19f373 100644 --- a/docs/USE_HEADLESS_FORM_ROLLOUT.md +++ b/docs/USE_HEADLESS_FORM_ROLLOUT.md @@ -13,6 +13,23 @@ Every flow builds its JSON Schema forms through one shared hook, [`src/common/us - **`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 diff --git a/src/common/tests/jsfEngineContract.test.tsx b/src/common/tests/jsfEngineContract.test.tsx index cccf7f292..9313ef5f4 100644 --- a/src/common/tests/jsfEngineContract.test.tsx +++ b/src/common/tests/jsfEngineContract.test.tsx @@ -1,7 +1,13 @@ import { useState } from 'react'; import { FieldValues } from 'react-hook-form'; -import { render, screen, waitFor, within } from '@testing-library/react'; -import userEvent, { UserEvent } from '@testing-library/user-event'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { + chooseInGroup, + portugal, + Schema, + SITUATIONS, +} from '@/src/common/tests/jsfEngineSituations'; import { HeadlessFormStrategy, useHeadlessForm, @@ -10,18 +16,8 @@ import { JSONSchemaFormFields } from '@/src/components/form/JSONSchemaForm'; import { useJSONSchemaForm } from '@/src/components/form/useJSONSchemaForm'; import { Form } from '@/src/components/ui/form'; import { JSFModify } from '@/src/flows/types'; -import { contractDetailsSchemaV1Portugal } from '@/src/flows/Onboarding/tests/fixtures'; import { queryClient, TestProviders } from '@/src/tests/testHelpers'; -type Schema = Record; - -type Situation = { - situation: string; - schema: Schema; - fill: (user: UserEvent) => Promise; - assert: (submitted: FieldValues) => Promise; -}; - const ENGINES = [ { engine: 'jsf v0', meta: { jsfOldVersion: true } }, { engine: 'jsf v1', meta: { jsfVersion: '1' } }, @@ -52,56 +48,6 @@ const OPTIONS: { }, ]; -const withoutMeta = ({ 'x-rmt-meta': _meta, ...schema }: Schema) => schema; - -const portugal = withoutMeta(contractDetailsSchemaV1Portugal.data); - -const chooseInGroup = async ( - user: UserEvent, - group: RegExp, - option: string, -) => { - const radioGroup = await screen.findByRole('radiogroup', { name: group }); - await user.click(within(radioGroup).getByRole('radio', { name: option })); -}; - -const SITUATIONS: Situation[] = [ - { - situation: 'forced money value computed from another money field', - schema: portugal, - fill: async (user) => { - await user.type( - await screen.findByLabelText('Annual gross salary'), - '71703.77', - ); - await chooseInGroup(user, /Type of employee/i, 'Full-time'); - await chooseInGroup(user, /work outside regular work hours/i, 'Yes'); - await chooseInGroup(user, /more than 8 hours a day/i, 'Yes'); - }, - assert: async (submitted) => { - expect( - await screen.findByText(/additional 812.57 EUR monthly/), - ).toBeInTheDocument(); - expect(submitted.working_hours_exemption_allowance).toBe(81257); - }, - }, - { - situation: 'radio reveals a conditional money field', - schema: portugal, - fill: async (user) => { - await chooseInGroup(user, /Offer a signing bonus/i, 'Yes'); - await user.type( - await screen.findByLabelText('Signing bonus amount'), - '1000.50', - ); - }, - assert: async (submitted) => { - expect(screen.getByLabelText('Signing bonus amount')).toBeVisible(); - expect(submitted.signing_bonus_amount).toBe(100050); - }, - }, -]; - function Harness({ schema, strategy, diff --git a/src/common/tests/jsfEngineSituations.ts b/src/common/tests/jsfEngineSituations.ts new file mode 100644 index 000000000..8bfbd4945 --- /dev/null +++ b/src/common/tests/jsfEngineSituations.ts @@ -0,0 +1,63 @@ +import { FieldValues } from 'react-hook-form'; +import { screen, within } from '@testing-library/react'; +import { UserEvent } from '@testing-library/user-event'; +import { contractDetailsSchemaV1Portugal } from '@/src/flows/Onboarding/tests/fixtures'; + +export type Schema = Record; + +export type Situation = { + situation: string; + schema: Schema; + fill: (user: UserEvent) => Promise; + assert: (submitted: FieldValues) => Promise; +}; + +const withoutMeta = ({ 'x-rmt-meta': _meta, ...schema }: Schema) => schema; + +export const portugal = withoutMeta(contractDetailsSchemaV1Portugal.data); + +export const chooseInGroup = async ( + user: UserEvent, + group: RegExp, + option: string, +) => { + const radioGroup = await screen.findByRole('radiogroup', { name: group }); + await user.click(within(radioGroup).getByRole('radio', { name: option })); +}; + +export const SITUATIONS: Situation[] = [ + { + situation: 'forced money value computed from another money field', + schema: portugal, + fill: async (user) => { + await user.type( + await screen.findByLabelText('Annual gross salary'), + '71703.77', + ); + await chooseInGroup(user, /Type of employee/i, 'Full-time'); + await chooseInGroup(user, /work outside regular work hours/i, 'Yes'); + await chooseInGroup(user, /more than 8 hours a day/i, 'Yes'); + }, + assert: async (submitted) => { + expect( + await screen.findByText(/additional 812.57 EUR monthly/), + ).toBeInTheDocument(); + expect(submitted.working_hours_exemption_allowance).toBe(81257); + }, + }, + { + situation: 'radio reveals a conditional money field', + schema: portugal, + fill: async (user) => { + await chooseInGroup(user, /Offer a signing bonus/i, 'Yes'); + await user.type( + await screen.findByLabelText('Signing bonus amount'), + '1000.50', + ); + }, + assert: async (submitted) => { + expect(screen.getByLabelText('Signing bonus amount')).toBeVisible(); + expect(submitted.signing_bonus_amount).toBe(100050); + }, + }, +]; From 3d5b443718ded99f4c649fa0281b454a61377af4 Mon Sep 17 00:00:00 2001 From: Gabriel Garcia Date: Mon, 5 Oct 2026 14:10:54 +0200 Subject: [PATCH 2/2] 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) --- src/common/tests/jsfEngineContract.test.tsx | 6 +- src/common/tests/jsfEngineSituations.ts | 126 ++++++++++++++++++-- 2 files changed, 118 insertions(+), 14 deletions(-) diff --git a/src/common/tests/jsfEngineContract.test.tsx b/src/common/tests/jsfEngineContract.test.tsx index 9313ef5f4..dcb873b03 100644 --- a/src/common/tests/jsfEngineContract.test.tsx +++ b/src/common/tests/jsfEngineContract.test.tsx @@ -4,8 +4,8 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { chooseInGroup, - portugal, Schema, + signingBonusSchema, SITUATIONS, } from '@/src/common/tests/jsfEngineSituations'; import { @@ -152,7 +152,7 @@ describe.each(ENGINES)('buildOnce lifecycle on $engine', ({ meta }) => { const renderHarness = ( props: Partial[0]> = {}, ) => { - const schema = withMeta(portugal); + const schema = withMeta(signingBonusSchema); const harnessProps = { schema, strategy: 'buildOnce' as const, @@ -221,7 +221,7 @@ describe.each(ENGINES)('buildOnce lifecycle on $engine', ({ meta }) => { await chooseInGroup(user, /Offer a signing bonus/i, 'Yes'); await screen.findByLabelText('Signing bonus amount'); - rerenderWith({ schema: withMeta(portugal) }); + rerenderWith({ schema: withMeta(signingBonusSchema) }); await waitFor(() => expect(screen.queryByLabelText('Signing bonus amount')).toBeNull(), diff --git a/src/common/tests/jsfEngineSituations.ts b/src/common/tests/jsfEngineSituations.ts index 8bfbd4945..399ced2b1 100644 --- a/src/common/tests/jsfEngineSituations.ts +++ b/src/common/tests/jsfEngineSituations.ts @@ -1,7 +1,6 @@ import { FieldValues } from 'react-hook-form'; import { screen, within } from '@testing-library/react'; import { UserEvent } from '@testing-library/user-event'; -import { contractDetailsSchemaV1Portugal } from '@/src/flows/Onboarding/tests/fixtures'; export type Schema = Record; @@ -12,9 +11,116 @@ export type Situation = { assert: (submitted: FieldValues) => Promise; }; -const withoutMeta = ({ 'x-rmt-meta': _meta, ...schema }: Schema) => schema; +const money = (title: string) => ({ + title, + type: ['integer', 'null'], + 'x-jsf-presentation': { inputType: 'money', currency: 'EUR' }, +}); -export const portugal = withoutMeta(contractDetailsSchemaV1Portugal.data); +const yesNo = (title: string) => ({ + title, + type: 'string', + oneOf: [ + { const: 'yes', title: 'Yes' }, + { const: 'no', title: 'No' }, + ], + 'x-jsf-presentation': { inputType: 'radio', direction: 'row' }, +}); + +const monthlySalaryInCents = { + '-': [ + { '/': [{ var: 'annual_gross_salary' }, 12] }, + { '%': [{ '/': [{ var: 'annual_gross_salary' }, 12] }, 1] }, + ], +}; + +export const signingBonusSchema: Schema = { + type: 'object', + additionalProperties: false, + properties: { + annual_gross_salary: money('Annual gross salary'), + has_signing_bonus: yesNo('Offer a signing bonus?'), + signing_bonus_amount: money('Signing bonus amount'), + }, + required: ['annual_gross_salary', 'has_signing_bonus'], + allOf: [ + { + if: { + properties: { has_signing_bonus: { const: 'yes' } }, + required: ['has_signing_bonus'], + }, + then: { required: ['signing_bonus_amount'] }, + else: { properties: { signing_bonus_amount: false } }, + }, + ], + 'x-jsf-order': [ + 'annual_gross_salary', + 'has_signing_bonus', + 'signing_bonus_amount', + ], +}; + +const monthlyAllowanceSchema: Schema = { + type: 'object', + additionalProperties: false, + properties: { + annual_gross_salary: money('Annual gross salary'), + pays_monthly_allowance: yesNo('Pay a monthly allowance?'), + monthly_allowance: money('Monthly allowance'), + }, + required: ['annual_gross_salary', 'pays_monthly_allowance'], + allOf: [ + { + if: { + properties: { + annual_gross_salary: { minimum: 1 }, + pays_monthly_allowance: { const: 'yes' }, + }, + required: ['annual_gross_salary', 'pays_monthly_allowance'], + }, + then: { + properties: { + monthly_allowance: { + 'x-jsf-logic-computedAttrs': { + const: 'monthly_allowance_in_cents', + default: 'monthly_allowance_in_cents', + 'x-jsf-presentation': { + statement: { + description: + 'You need to pay an additional {{monthly_allowance_value}} EUR monthly.', + severity: 'info', + }, + }, + }, + }, + }, + required: ['monthly_allowance'], + }, + else: { properties: { monthly_allowance: false } }, + }, + ], + 'x-jsf-order': [ + 'annual_gross_salary', + 'pays_monthly_allowance', + 'monthly_allowance', + ], + 'x-jsf-logic': { + computedValues: { + monthly_allowance_in_cents: { + rule: { if: [{ var: 'annual_gross_salary' }, monthlySalaryInCents, 0] }, + }, + monthly_allowance_value: { + rule: { + if: [ + { var: 'annual_gross_salary' }, + { '/': [monthlySalaryInCents, 100] }, + 0, + ], + }, + }, + }, + }, +}; export const chooseInGroup = async ( user: UserEvent, @@ -28,26 +134,24 @@ export const chooseInGroup = async ( export const SITUATIONS: Situation[] = [ { situation: 'forced money value computed from another money field', - schema: portugal, + schema: monthlyAllowanceSchema, fill: async (user) => { await user.type( await screen.findByLabelText('Annual gross salary'), - '71703.77', + '1234.56', ); - await chooseInGroup(user, /Type of employee/i, 'Full-time'); - await chooseInGroup(user, /work outside regular work hours/i, 'Yes'); - await chooseInGroup(user, /more than 8 hours a day/i, 'Yes'); + await chooseInGroup(user, /Pay a monthly allowance/i, 'Yes'); }, assert: async (submitted) => { expect( - await screen.findByText(/additional 812.57 EUR monthly/), + await screen.findByText(/additional 102.88 EUR monthly/), ).toBeInTheDocument(); - expect(submitted.working_hours_exemption_allowance).toBe(81257); + expect(submitted.monthly_allowance).toBe(10288); }, }, { situation: 'radio reveals a conditional money field', - schema: portugal, + schema: signingBonusSchema, fill: async (user) => { await chooseInGroup(user, /Offer a signing bonus/i, 'Yes'); await user.type(