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..dcb873b03 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, + Schema, + signingBonusSchema, + 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, @@ -206,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, @@ -275,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 new file mode 100644 index 000000000..399ced2b1 --- /dev/null +++ b/src/common/tests/jsfEngineSituations.ts @@ -0,0 +1,167 @@ +import { FieldValues } from 'react-hook-form'; +import { screen, within } from '@testing-library/react'; +import { UserEvent } from '@testing-library/user-event'; + +export type Schema = Record; + +export type Situation = { + situation: string; + schema: Schema; + fill: (user: UserEvent) => Promise; + assert: (submitted: FieldValues) => Promise; +}; + +const money = (title: string) => ({ + title, + type: ['integer', 'null'], + 'x-jsf-presentation': { inputType: 'money', currency: 'EUR' }, +}); + +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, + 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: monthlyAllowanceSchema, + fill: async (user) => { + await user.type( + await screen.findByLabelText('Annual gross salary'), + '1234.56', + ); + await chooseInGroup(user, /Pay a monthly allowance/i, 'Yes'); + }, + assert: async (submitted) => { + expect( + await screen.findByText(/additional 102.88 EUR monthly/), + ).toBeInTheDocument(); + expect(submitted.monthly_allowance).toBe(10288); + }, + }, + { + situation: 'radio reveals a conditional money field', + schema: signingBonusSchema, + 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); + }, + }, +];