diff --git a/src/common/createHeadlessForm.tsx b/src/common/createHeadlessForm.tsx index ef0619a5c..4604b3941 100644 --- a/src/common/createHeadlessForm.tsx +++ b/src/common/createHeadlessForm.tsx @@ -3,7 +3,7 @@ import { createHeadlessForm as baseCreateHeadlessForm, modify, } from '@remoteoss/remote-json-schema-form-kit'; -import { convertToCents } from '@/src/components/form/utils'; +import { convertToCents, isNumericValue } from '@/src/components/form/utils'; import { JSFModify, JSONSchemaFormResultWithFieldsets, @@ -60,6 +60,7 @@ export const createHeadlessForm = ( } let moneyFieldsData: Record = {}; + let numberFieldsData: Record = {}; if (fieldValues) { const moneyFields = findFieldsByType(jsfSchema.properties || {}, 'money'); @@ -70,6 +71,22 @@ export const createHeadlessForm = ( }, {}, ); + + const numberFields = findFieldsByType(jsfSchema.properties || {}, 'number'); + numberFieldsData = numberFields.reduce>( + (acc, field) => { + // Only unambiguous numbers are injected. Conditionals are evaluated + // against these values before any validation runs, so a lenient cast + // (Number(' ') === 0) would open branches the user never filled in. + // Empty and untouched fields are left out entirely rather than + // materialised as 0. + if (isNumericValue(fieldValues[field])) { + acc[field] = Number(fieldValues[field]); + } + return acc; + }, + {}, + ); } /** @@ -80,6 +97,7 @@ export const createHeadlessForm = ( JSON.stringify({ ...fieldValues, ...moneyFieldsData, + ...numberFieldsData, }), ); diff --git a/src/common/tests/createHeadlessForm.test.ts b/src/common/tests/createHeadlessForm.test.ts new file mode 100644 index 000000000..85a85943c --- /dev/null +++ b/src/common/tests/createHeadlessForm.test.ts @@ -0,0 +1,166 @@ +import { createHeadlessForm } from '../createHeadlessForm'; + +const schemaWithNumberGuard = { + additionalProperties: false, + type: 'object', + 'x-rmt-meta': { + jsfVersion: '1', + }, + properties: { + clause_apply: { + title: 'Apply clause', + type: 'string', + oneOf: [ + { const: 'yes', title: 'Yes' }, + { const: 'no', title: 'No' }, + ], + 'x-jsf-presentation': { + inputType: 'radio', + }, + }, + compensation_percentage: { + title: 'Compensation percentage', + type: 'number', + 'x-jsf-presentation': { + inputType: 'number', + }, + }, + compensation_amount: { + title: 'Compensation amount', + type: 'integer', + 'x-jsf-presentation': { + currency: 'EUR', + inputType: 'money', + }, + }, + }, + required: ['clause_apply'], + allOf: [ + { + if: { + properties: { + clause_apply: { const: 'yes' }, + compensation_percentage: { type: 'number' }, + }, + required: ['clause_apply', 'compensation_percentage'], + }, + then: { + required: ['compensation_amount'], + }, + else: { + properties: { + compensation_amount: false, + }, + }, + }, + ], +}; + +// The kit only treats a schema as v1 when jsfVersion is exactly '1'; anything +// else goes through the v0 path, which casts values with yup before evaluating +// conditionals. +const schemaWithNumberGuardV0 = { + ...schemaWithNumberGuard, + 'x-rmt-meta': { + jsfVersion: '0', + }, +}; + +function getField(form: ReturnType, name: string) { + return form.fields?.find((field) => field.name === name); +} + +describe('createHeadlessForm', () => { + describe('number field coercion', () => { + it('should reveal a conditional field when the number guard value is a string', () => { + const form = createHeadlessForm(schemaWithNumberGuard, { + clause_apply: 'yes', + compensation_percentage: '50', + }); + + expect(getField(form, 'compensation_amount')?.isVisible).toBe(true); + }); + + it('should reveal a conditional field when the number guard value is already a number', () => { + const form = createHeadlessForm(schemaWithNumberGuard, { + clause_apply: 'yes', + compensation_percentage: 50, + }); + + expect(getField(form, 'compensation_amount')?.isVisible).toBe(true); + }); + + it('should keep the conditional field hidden when the number field is empty', () => { + const form = createHeadlessForm(schemaWithNumberGuard, { + clause_apply: 'yes', + compensation_percentage: '', + }); + + expect(getField(form, 'compensation_amount')?.isVisible).toBe(false); + }); + + it('should keep the conditional field hidden when the number field is not numeric', () => { + const form = createHeadlessForm(schemaWithNumberGuard, { + clause_apply: 'yes', + compensation_percentage: 'abc', + }); + + expect(getField(form, 'compensation_amount')?.isVisible).toBe(false); + }); + + it('should keep the conditional field hidden when the number field only contains whitespace', () => { + const form = createHeadlessForm(schemaWithNumberGuard, { + clause_apply: 'yes', + compensation_percentage: ' ', + }); + + expect(getField(form, 'compensation_amount')?.isVisible).toBe(false); + }); + + it('should keep the conditional field hidden for values Number() would coerce to 0', () => { + const booleanForm = createHeadlessForm(schemaWithNumberGuard, { + clause_apply: 'yes', + compensation_percentage: true, + }); + const arrayForm = createHeadlessForm(schemaWithNumberGuard, { + clause_apply: 'yes', + compensation_percentage: [], + }); + + expect(getField(booleanForm, 'compensation_amount')?.isVisible).toBe( + false, + ); + expect(getField(arrayForm, 'compensation_amount')?.isVisible).toBe(false); + }); + + it('should keep the conditional field hidden when the clause does not apply', () => { + const form = createHeadlessForm(schemaWithNumberGuard, { + clause_apply: 'no', + compensation_percentage: '50', + }); + + expect(getField(form, 'compensation_amount')?.isVisible).toBe(false); + }); + + // Nothing in createHeadlessForm branches on jsfVersion, so the coercion also + // runs on v0 schemas. These lock in that it stays a no-op there. + it('should not change how v0 schemas evaluate the same guard', () => { + const numeric = createHeadlessForm(schemaWithNumberGuardV0, { + clause_apply: 'yes', + compensation_percentage: '50', + }); + const empty = createHeadlessForm(schemaWithNumberGuardV0, { + clause_apply: 'yes', + compensation_percentage: '', + }); + const invalid = createHeadlessForm(schemaWithNumberGuardV0, { + clause_apply: 'yes', + compensation_percentage: 'abc', + }); + + expect(getField(numeric, 'compensation_amount')?.isVisible).toBe(true); + expect(getField(empty, 'compensation_amount')?.isVisible).toBe(false); + expect(getField(invalid, 'compensation_amount')?.isVisible).toBe(false); + }); + }); +}); diff --git a/src/components/form/JSONSchemaForm.tsx b/src/components/form/JSONSchemaForm.tsx index 8f720f674..44cd1cb9e 100644 --- a/src/components/form/JSONSchemaForm.tsx +++ b/src/components/form/JSONSchemaForm.tsx @@ -74,7 +74,8 @@ export const JSONSchemaFormFields = ({ { - setValue(name, value); - }, [name, value, setValue]); + setValue(name, forcedValue); + }, [name, forcedValue, setValue]); const isHiddenValue = !forcedValueDescription && !statement?.title; diff --git a/src/components/form/fields/tests/ForcedValueField.test.tsx b/src/components/form/fields/tests/ForcedValueField.test.tsx index ec669de4c..27fc58902 100644 --- a/src/components/form/fields/tests/ForcedValueField.test.tsx +++ b/src/components/form/fields/tests/ForcedValueField.test.tsx @@ -193,6 +193,31 @@ describe('ForcedValueField Component', () => { expect(mockSetValue).toHaveBeenCalledWith('testField', 'forced-value'); }); + it('converts money consts from cents to units before setting the form value', () => { + const mockSetValue = vi.fn(); + + const TestComponent = () => { + const methods = { + ...useForm(), + setValue: mockSetValue, + }; + return ( + + + + ); + }; + + render(); + + expect(mockSetValue).toHaveBeenCalledWith('compensation_amount', 1250); + }); + it('still sets form value even when field is hidden (no description and no title)', () => { const mockSetValue = vi.fn(); diff --git a/src/components/form/utils.ts b/src/components/form/utils.ts index 650bc6533..9314326b9 100644 --- a/src/components/form/utils.ts +++ b/src/components/form/utils.ts @@ -186,6 +186,37 @@ function extractFieldsetFieldsValues( }, {}); } +/** + * Tells whether a value unambiguously represents a number. + * + * Stricter than `Number()`, which happily turns booleans, arrays and blank + * strings into 0. Use this whenever coercing silently would change behaviour + * without going through validation first. + */ +export function isNumericValue(value: $TSFixMe): boolean { + if (typeof value === 'number') { + return Number.isFinite(value); + } + + if (typeof value !== 'string' || value.trim() === '') { + return false; + } + + return Number.isFinite(Number(value)); +} + +export function castNumberValue(value: $TSFixMe) { + // this prevents values with letters such as "2r" from being considered valid + // if the input is invalid, number().cast will return NaN + const castValue = Number(value); + + if (Number.isNaN(castValue)) { + return value; + } + + return castValue; +} + export const fieldTypesTransformations: Record = { [supportedTypes.COUNTRIES]: { /** @@ -231,17 +262,7 @@ export const fieldTypesTransformations: Record = { }, }, [supportedTypes.NUMBER]: { - transformValueToAPI: () => (value: string) => { - // this prevents values with letters such as "2r" from being considered valid - // if the input is invalid, number().cast will return NaN - const castValue = Number(value); - - if (Number.isNaN(castValue)) { - return value; - } - - return castValue; - }, + transformValueToAPI: () => castNumberValue, }, [supportedTypes.MONEY]: { transformValueFromAPI: () => (value: string | number) =>