diff --git a/docs/USE_HEADLESS_FORM_ROLLOUT.md b/docs/USE_HEADLESS_FORM_ROLLOUT.md index 1aa5ece07..76d516fcc 100644 --- a/docs/USE_HEADLESS_FORM_ROLLOUT.md +++ b/docs/USE_HEADLESS_FORM_ROLLOUT.md @@ -11,7 +11,13 @@ Every flow builds its JSON Schema forms through one shared hook, [`src/common/us ## 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`. Conditionals are resolved by `handleValidation` with `isPartialValidation: true`, and the latest values are replayed after each build so visibility is right without a mounted step. +- **`buildOnce`** (new): built once per `schema`/`options`. Conditionals are resolved by `handleValidation` with `isPartialValidation: true`, and the latest values are replayed after each build so visibility is right without a mounted step. `form` stays `null` (`isBuilding: true`) until that first replay finishes, so nothing ever reads a form with default visibility. After a `jsfModify` rebuild the previous form is kept until the new one is ready; a new `schema` goes back to `null`. + +### Adopting `buildOnce` in a step + +1. Pass `values` holding only that step's values, in form units: partner `initialValues`, then the saved employment's data for that step, then `stepState.values` for the step, then the live `fieldValues` while the step is mounted. Values from another step change visibility in the wrong form. Saved values in API units (money in cents) have to be converted to form units first. +2. Add `isBuilding` to the step's loading state, so the step never mounts with an unresolved form (its `defaultValues` would be computed from empty fields). +3. Route the step's `handleValidation` and `parseFormValues` through the hook, and call `onValuesChange` from `checkFieldUpdates` while the step is current. ## Phases @@ -47,7 +53,6 @@ Status: `todo`, `phase 1` (on the hook, old behaviour), `done` (on the hook with | 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 `useJSONSchemaForm` (basic information and other steps) | todo | #1430 tried `buildOnce` here and was closed | | Onboarding `useBenefitOffersSchema` | todo | | | Onboarding `useEngagementAgreementDetailsSchema` | todo | | | Contractor contract details `useContractorOnboardingDetailsSchema` | todo | Money-sensitive | @@ -65,16 +70,17 @@ Status: `todo`, `phase 1` (on the hook, old behaviour), `done` (on the hook with ### 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 | +| Call site | Status | Notes | +| ----------------------------------------------------------- | ------ | ---------------------------------------------------------------------------------------------------------------------------------------------- | +| Onboarding `useContractDetailsSchema` (jsf v1) | done | #1433. Replays only the mounted step's `fieldValues`, so a read-only review doesn't see saved values yet (needs cents converted to form units) | +| Onboarding `useBasicInformationSchema` | done | Was `rebuild` through `useJSONSchemaForm`; #1430 was the first attempt | +| 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 diff --git a/src/common/tests/useHeadlessForm.test.tsx b/src/common/tests/useHeadlessForm.test.tsx index f0b2b5dbb..f560c045a 100644 --- a/src/common/tests/useHeadlessForm.test.tsx +++ b/src/common/tests/useHeadlessForm.test.tsx @@ -1,20 +1,45 @@ import { renderHook, waitFor } from '@testing-library/react'; import { useHeadlessForm } from '@/src/common/useHeadlessForm'; import { JSFModify } from '@/src/flows/types'; -import { contractDetailsSchemaV1Portugal } from '@/src/flows/Onboarding/tests/fixtures'; +import { + basicInformationSchemaV3Portugal, + contractDetailsSchemaV1Portugal, +} from '@/src/flows/Onboarding/tests/fixtures'; import { $TSFixMe } from '@/src/types/remoteFlows'; const { 'x-rmt-meta': _meta, ...schema } = contractDetailsSchemaV1Portugal.data; +const findField = (fields: $TSFixMe[] | undefined, name: string) => + fields?.find((field) => field.name === name); + const isVisible = (fields: $TSFixMe[] | undefined, name: string) => - fields?.find((field) => field.name === name)?.isVisible; + findField(fields, name)?.isVisible; const renamedBonus = (title: string): JSFModify => ({ fields: { signing_bonus_amount: { title } }, }); describe('useHeadlessForm buildOnce', () => { - it('replays the latest values into a rebuilt form and leaves the replaced one untouched', async () => { + it('hands out the form only once its visibility matches the values', async () => { + const { result } = renderHook(() => + useHeadlessForm({ + schema, + values: { has_signing_bonus: 'yes' }, + strategy: 'buildOnce', + }), + ); + + expect(result.current.form).toBeNull(); + expect(result.current.isBuilding).toBe(true); + + await waitFor(() => expect(result.current.form).not.toBeNull()); + expect(result.current.isBuilding).toBe(false); + expect(isVisible(result.current.form?.fields, 'signing_bonus_amount')).toBe( + true, + ); + }); + + it('keeps the resolved form while a changed jsfModify rebuilds, then replays the latest values into the new one', async () => { const { result, rerender } = renderHook( ({ jsfModify }) => useHeadlessForm({ @@ -25,17 +50,44 @@ describe('useHeadlessForm buildOnce', () => { }), { initialProps: { jsfModify: renamedBonus('Bonus') } }, ); + await waitFor(() => expect(result.current.form).not.toBeNull()); const replacedForm = result.current.form; rerender({ jsfModify: renamedBonus('Welcome bonus') }); + expect(result.current.form).toBe(replacedForm); + expect(result.current.isBuilding).toBe(false); + await waitFor(() => expect(result.current.form).not.toBe(replacedForm)); + const rebuiltFields = result.current.form?.fields; + expect(findField(rebuiltFields, 'signing_bonus_amount')?.label).toBe( + 'Welcome bonus', + ); + expect(isVisible(rebuiltFields, 'signing_bonus_amount')).toBe(true); + }); + + it('stops handing out the previous form when the schema changes', async () => { + const { 'x-rmt-meta': _basicMeta, ...otherSchema } = + basicInformationSchemaV3Portugal.data; + const { result, rerender } = renderHook( + ({ currentSchema }) => + useHeadlessForm({ + schema: currentSchema, + values: {}, + strategy: 'buildOnce', + }), + { initialProps: { currentSchema: schema as Record } }, + ); + await waitFor(() => expect(result.current.form).not.toBeNull()); + + rerender({ currentSchema: otherSchema }); + + expect(result.current.form).toBeNull(); + expect(result.current.isBuilding).toBe(true); await waitFor(() => expect( - isVisible(result.current.form?.fields, 'signing_bonus_amount'), - ).toBe(true), + findField(result.current.form?.fields, 'has_seniority_date'), + ).toBeDefined(), ); - expect(result.current.form).not.toBe(replacedForm); - expect(isVisible(replacedForm?.fields, 'signing_bonus_amount')).toBe(false); }); it('keeps the form when options are recreated with the same content on every render', async () => { @@ -53,13 +105,9 @@ describe('useHeadlessForm buildOnce', () => { strategy: 'buildOnce', }); }); - const builtForm = result.current.form; - await waitFor(() => - expect( - isVisible(result.current.form?.fields, 'signing_bonus_amount'), - ).toBe(true), - ); + await waitFor(() => expect(result.current.form).not.toBeNull()); + const builtForm = result.current.form; await new Promise((resolve) => setTimeout(resolve, 100)); expect(result.current.form).toBe(builtForm); diff --git a/src/common/useHeadlessForm.ts b/src/common/useHeadlessForm.ts index 4b54061ee..b9c9d44e6 100644 --- a/src/common/useHeadlessForm.ts +++ b/src/common/useHeadlessForm.ts @@ -20,6 +20,7 @@ type UseHeadlessFormArgs = { export type HeadlessForm = { form: JSONSchemaFormResultWithFieldsets | null; + isBuilding: boolean; handleValidation: (values: FieldValues) => Promise; onValuesChange: (values: FieldValues) => Promise; parseFormValues: (values: FieldValues) => Promise; @@ -33,6 +34,10 @@ export function useHeadlessForm({ }: UseHeadlessFormArgs): HeadlessForm { const isBuildOnce = strategy === 'buildOnce'; const [, setRevision] = useState(0); + const [resolved, setResolved] = useState<{ + schema: Record; + form: JSONSchemaFormResultWithFieldsets; + } | null>(null); const [stableOptions, setStableOptions] = useState(options); if (!equal(stableOptions, options)) { setStableOptions(options); @@ -44,7 +49,7 @@ export function useHeadlessForm({ const buildValues = isBuildOnce ? undefined : values; - const form = useMemo(() => { + const builtForm = useMemo(() => { if (!schema) return null; return isBuildOnce ? createHeadlessForm( @@ -55,29 +60,39 @@ export function useHeadlessForm({ : createHeadlessForm(schema, buildValues, stableOptions); }, [schema, buildValues, stableOptions, isBuildOnce]); - const validate = useCallback( - async (nextValues: FieldValues, isCancelled: () => boolean) => { - if (!form) return null; + const form = !isBuildOnce + ? builtForm + : resolved && resolved.schema === schema + ? resolved.form + : null; + + const validateForm = useCallback( + async ( + target: JSONSchemaFormResultWithFieldsets | null, + nextValues: FieldValues, + isCancelled: () => boolean, + ) => { + if (!target) return null; // buildOnce keeps invisible values on purpose. The fields still hold the // visibility of the previous change, so dropping their values would hide // what a field that is about to become visible needs to compute itself: // a hidden fieldset coming back would lose the values driving its own // children. handleValidation resolves the visibility first and nulls // whatever it considers hidden afterwards, which is the right order. - const parsedValues = await parseJSFToValidate(nextValues, form.fields, { + const parsedValues = await parseJSFToValidate(nextValues, target.fields, { isPartialValidation: isBuildOnce, }); if (isCancelled()) return null; - const result = form.handleValidation(parsedValues); + const result = target.handleValidation(parsedValues); if (isBuildOnce) setRevision((revision) => revision + 1); return result; }, - [form, isBuildOnce], + [isBuildOnce], ); const handleValidation = useCallback( - (nextValues: FieldValues) => validate(nextValues, () => false), - [validate], + (nextValues: FieldValues) => validateForm(form, nextValues, () => false), + [validateForm, form], ); const onValuesChange = useCallback( @@ -88,13 +103,17 @@ export function useHeadlessForm({ ); useEffect(() => { - if (!isBuildOnce) return; + if (!isBuildOnce || !schema || !builtForm) return; let cancelled = false; - void validate(latestValues.current, () => cancelled); + void validateForm(builtForm, latestValues.current, () => cancelled).finally( + () => { + if (!cancelled) setResolved({ schema, form: builtForm }); + }, + ); return () => { cancelled = true; }; - }, [validate, isBuildOnce]); + }, [validateForm, builtForm, schema, isBuildOnce]); const parseFormValues = useCallback( async (nextValues: FieldValues) => @@ -102,5 +121,13 @@ export function useHeadlessForm({ [form], ); - return { form, handleValidation, onValuesChange, parseFormValues }; + const isBuilding = Boolean(schema) && !form; + + return { + form, + isBuilding, + handleValidation, + onValuesChange, + parseFormValues, + }; } diff --git a/src/flows/Onboarding/api.ts b/src/flows/Onboarding/api.ts index c8a920bdc..45d1cb845 100644 --- a/src/flows/Onboarding/api.ts +++ b/src/flows/Onboarding/api.ts @@ -329,7 +329,7 @@ export const useContractDetailsSchema = ({ options?: FlowOptions & { queryOptions?: { enabled?: boolean } }; query?: Record; jsonSchemaVersion?: number | 'latest'; -}): Omit & { +}): Omit & { data: JSONSchemaFormResultWithFieldsets | null; isLoading: boolean; } => { @@ -375,14 +375,95 @@ export const useContractDetailsSchema = ({ select: ({ data }) => data?.data, }); - const { form, handleValidation, parseFormValues } = useHeadlessForm({ + const { form, isBuilding, handleValidation, parseFormValues } = + useHeadlessForm({ + schema, + values: fieldValues, + options: { jsfModify: options?.jsfModify }, + strategy: 'buildOnce', + }); + + return { + data: form, + isLoading: isLoading || isBuilding, + handleValidation, + parseFormValues, + }; +}; + +export const useBasicInformationSchema = ({ + countryCode, + fieldValues, + options, + jsonSchemaVersion, +}: { + countryCode: string; + fieldValues: FieldValues; + options?: FlowOptions & { queryOptions?: { enabled?: boolean } }; + jsonSchemaVersion?: number | 'latest'; +}): Omit & { + data: JSONSchemaFormResultWithFieldsets | null; + isLoading: boolean; +} => { + const { client } = useClient(); + const jsonSchemaQueryParam = jsonSchemaVersion + ? { + json_schema_version: jsonSchemaVersion, + } + : {}; + const { data: schema, isLoading } = useQuery({ + queryKey: [ + 'onboarding-basic-information-schema', + countryCode, + jsonSchemaVersion, + ], + retry: false, + queryFn: async () => { + const response = await getV1CountriesCountryCodeForm({ + client: client as Client, + headers: { + Authorization: ``, + }, + path: { + country_code: countryCode, + form: 'employment_basic_information', + }, + query: { + skip_benefits: true, + ...jsonSchemaQueryParam, + }, + }); + + if (response.error || !response.data) { + throw new Error('Failed to fetch onboarding schema'); + } + + return response; + }, + enabled: options?.queryOptions?.enabled, + select: ({ data }) => data?.data, + }); + + const { + form, + isBuilding, + handleValidation, + onValuesChange, + parseFormValues, + } = useHeadlessForm({ schema, values: fieldValues, options: { jsfModify: options?.jsfModify }, strategy: 'buildOnce', }); - return { data: form, isLoading, handleValidation, parseFormValues }; + return { + data: form, + isLoading: isLoading || isBuilding, + handleValidation, + onValuesChange, + parseFormValues, + }; }; export const useBenefitOffersSchema = ( diff --git a/src/flows/Onboarding/hooks.tsx b/src/flows/Onboarding/hooks.tsx index 2b45ff247..b9c163c65 100644 --- a/src/flows/Onboarding/hooks.tsx +++ b/src/flows/Onboarding/hooks.tsx @@ -31,6 +31,7 @@ import { useBenefitOffers, useBenefitOffersSchema, useCompany, + useBasicInformationSchema, useContractDetailsSchema, useCountriesSchemaField, useCreateEmployment, @@ -39,7 +40,6 @@ import { useEmploymentOnboardingReservesStatus, useEngagementAgreementDetailsSchema, useGetPreOnboardingRequirements, - useJSONSchemaForm, useLegacyContractDetailsSchema, useUpdateBenefitsOffers, useUpdateEmployment, @@ -522,35 +522,6 @@ export const useOnboarding = ({ ...serverEmploymentData, }; - const useJSONSchema = ({ - form, - options: jsonSchemaOptions = {}, - query = {}, - jsonSchemaVersion, - }: { - form: JSONSchemaFormType; - options?: { - jsfModify?: JSFModify; - queryOptions?: { enabled?: boolean }; - }; - query?: Record; - jsonSchemaVersion?: number | 'latest'; - }) => { - return useJSONSchemaForm({ - countryCode: internalCountryCode as string, - form: form, - fieldValues: mergedFormValues, - query, - options: { - ...jsonSchemaOptions, - queryOptions: { - enabled: jsonSchemaOptions.queryOptions?.enabled ?? true, - }, - }, - jsonSchemaVersion, - }); - }; - const isBasicInformationDetailsEnabled = Boolean( internalCountryCode && (stepState.currentStep.name === 'basic_information' || @@ -570,11 +541,33 @@ export const useOnboarding = ({ Boolean(employmentId)), ); + const basicInformationFormValues = useMemo( + () => ({ + ...onboardingInitialValues, + ...employment?.basic_information, + ...stepState.values?.basic_information, + ...(stepState.currentStep.name === 'basic_information' + ? fieldValues + : {}), + }), + [ + onboardingInitialValues, + employment?.basic_information, + stepState.values?.basic_information, + stepState.currentStep.name, + fieldValues, + ], + ); + const { data: basicInformationForm, isLoading: isLoadingBasicInformationForm, - } = useJSONSchema({ - form: 'employment_basic_information', + handleValidation: handleBasicInformationValidation, + onValuesChange: onBasicInformationValuesChange, + parseFormValues: parseBasicInformationValues, + } = useBasicInformationSchema({ + countryCode: internalCountryCode as string, + fieldValues: basicInformationFormValues, options: { jsfModify: options?.jsfModify?.basic_information, queryOptions: { @@ -1086,9 +1079,7 @@ export const useOnboarding = ({ basicInformationForm && stepState.currentStep.name === 'basic_information' ) { - return await parseJSFToValidate(values, basicInformationForm?.fields, { - isPartialValidation: false, - }); + return await parseBasicInformationValues(values); } if ( @@ -1269,12 +1260,7 @@ export const useOnboarding = ({ basicInformationForm && stepState.currentStep.name === 'basic_information' ) { - const parsedValues = await parseJSFToValidate( - values, - basicInformationForm?.fields, - { isPartialValidation: false }, - ); - return basicInformationForm?.handleValidation(parsedValues); + return handleBasicInformationValidation(values); } if ( @@ -1317,6 +1303,7 @@ export const useOnboarding = ({ selectCountryForm, benefitOffersSchema, basicInformationForm, + handleBasicInformationValidation, engagementAgreementDetailsSchema, contractDetailsForm, contractDetailsFormV1, @@ -1328,6 +1315,9 @@ export const useOnboarding = ({ const checkFieldUpdates = useCallback( async (values: FieldValues) => { setFieldValues(values); + if (stepState.currentStep.name === 'basic_information') { + await onBasicInformationValuesChange(values); + } if ( isJsfV1ContractDetailsEnabled && stepState.currentStep.name === 'contract_details' @@ -1340,6 +1330,7 @@ export const useOnboarding = ({ isJsfV1ContractDetailsEnabled, stepState, handleValidation, + onBasicInformationValuesChange, ], ); diff --git a/src/flows/Onboarding/tests/basicInformationBuildOnce.test.tsx b/src/flows/Onboarding/tests/basicInformationBuildOnce.test.tsx new file mode 100644 index 000000000..b43f3c5d2 --- /dev/null +++ b/src/flows/Onboarding/tests/basicInformationBuildOnce.test.tsx @@ -0,0 +1,197 @@ +import { useState } from 'react'; +import { render, renderHook, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { http, HttpResponse } from 'msw'; +import { useOnboarding } from '@/src/flows/Onboarding/hooks'; +import { OnboardingFlow } from '@/src/flows/Onboarding/OnboardingFlow'; +import { + basicInformationSchemaV1Portugal, + contractDetailsSchemaV1Portugal, + employmentDefaultResponse, +} from '@/src/flows/Onboarding/tests/fixtures'; +import { generateUniqueEmploymentId } from '@/src/flows/Onboarding/tests/helpers'; +import { OnboardingRenderProps } from '@/src/flows/Onboarding/types'; +import { PrettifiedValuesRenderer } from '@/src/tests/components/PrettifiedValuesRenderer'; +import { server } from '@/src/tests/server'; +import { fillRadio, queryClient, TestProviders } from '@/src/tests/testHelpers'; +import { $TSFixMe } from '@/src/types/remoteFlows'; + +const findField = (fields: $TSFixMe[] = [], name: string) => + fields.find((field) => field.name === name); + +const mockEmployment = ( + employmentId: string, + overrides: Record = {}, +) => + server.use( + http.get(`*/v1/employments/${employmentId}`, () => + HttpResponse.json({ + ...employmentDefaultResponse, + data: { + ...employmentDefaultResponse.data, + employment: { + ...employmentDefaultResponse.data.employment, + id: employmentId, + basic_information: { + ...employmentDefaultResponse.data.employment.basic_information, + has_seniority_date: 'yes', + seniority_date: '2020-01-15', + }, + ...overrides, + }, + }, + }), + ), + ); + +const renderStep = ({ onboardingBag, components }: OnboardingRenderProps) => { + if (onboardingBag.isLoading) { + return
Loading...
; + } + const { BasicInformationStep, SubmitButton } = components; + switch (onboardingBag.stepState.currentStep.name) { + case 'basic_information': + return ( + <> + + Next Step + + ); + case 'review': + return ( + + ); + default: + return

Step: {onboardingBag.stepState.currentStep.name}

; + } +}; + +describe('Onboarding basic information built once', () => { + beforeEach(() => { + queryClient.clear(); + server.use( + http.get('*/v1/countries/*/employment_basic_information*', () => + HttpResponse.json(basicInformationSchemaV1Portugal), + ), + http.get('*/v1/countries/PRT/contract_details*', () => + HttpResponse.json(contractDetailsSchemaV1Portugal), + ), + ); + }); + + it('shows a conditional field revealed by the saved employment on first paint', async () => { + const employmentId = generateUniqueEmploymentId(); + mockEmployment(employmentId); + + render( + , + { wrapper: TestProviders }, + ); + + await screen.findByLabelText(/Personal email/i); + expect(screen.getByTestId('seniority_date')).toBeInTheDocument(); + }); + + it('shows a conditional field revealed by the partner initialValues on first paint', async () => { + render( + , + { wrapper: TestProviders }, + ); + + expect(await screen.findByTestId('seniority_date')).toBeInTheDocument(); + }); + + it('keeps a conditional field in the review of a read-only employment that never mounts the step', async () => { + const employmentId = generateUniqueEmploymentId(); + mockEmployment(employmentId, { status: 'invited' }); + + render( + , + { wrapper: TestProviders }, + ); + + expect(await screen.findByText(/^seniority_date:/)).toBeInTheDocument(); + }); + + it('keeps a revealed field when the partner passes options inline and re-renders', async () => { + const user = userEvent.setup(); + function Partner() { + const [, setRenders] = useState(0); + return ( + <> + + + + ); + } + + render(, { wrapper: TestProviders }); + await screen.findByLabelText('Employee name'); + await fillRadio('Does the employee have a seniority date?', 'Yes'); + await screen.findByTestId('seniority_date'); + + await user.click(screen.getByText('Re-render partner')); + + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(screen.getByTestId('seniority_date')).toBeInTheDocument(); + }); + + it('resolves conditional fields for headless consumers without mounting a step', async () => { + const employmentId = generateUniqueEmploymentId(); + mockEmployment(employmentId); + + const { result } = renderHook( + () => + useOnboarding({ + companyId: '1234', + countryCode: 'PRT', + employmentId, + skipSteps: ['select_country'], + }), + { wrapper: TestProviders }, + ); + + await waitFor(() => + expect(findField(result.current.fields, 'seniority_date')).toBeDefined(), + ); + expect(findField(result.current.fields, 'seniority_date').isVisible).toBe( + true, + ); + }); +});