diff --git a/src/calculateConditionalProperties.js b/src/calculateConditionalProperties.js index 5471c9876..cf684a156 100644 --- a/src/calculateConditionalProperties.js +++ b/src/calculateConditionalProperties.js @@ -69,7 +69,7 @@ function rebuildFieldset(fields, property) { * schema dependencies, and conditional logic. * * @param {Object} params - Parameters - * @param {Object} params.fieldParams - Current field parameters + * @param {Object} params.fieldParams - The field attributes from the first render (root) * @param {Object} params.customProperties - Custom field properties from schema * @param {Object} params.logic - JSON-logic * @param {Object} params.config - Form configuration @@ -77,6 +77,11 @@ function rebuildFieldset(fields, property) { * @returns {Function} A function that calculates conditional properties */ export function calculateConditionalProperties({ fieldParams, customProperties, logic, config }) { + /** + * @typedef {calculateConditionalPropertiesReturn} + * @property {rootFieldAttrs} - The field attributes from the first render (root) + * @property {newAttributes} - Attributes from the matched conditional + */ /** * Runs dynamic property calculation on a field based on a conditional that has been calculated * @@ -85,11 +90,11 @@ export function calculateConditionalProperties({ fieldParams, customProperties, * @param {Object} params.conditionBranch - Condition branch * @param {Object} params.formValues - Current form values * - * @returns {Object} Updated field parameters + * @returns {calculateConditionalPropertiesReturn} */ return ({ isRequired, conditionBranch, formValues }) => { // Check if the current field is conditionally declared in the schema - + // console.log('::calc (closure original)', fieldParams.description); const conditionalProperty = conditionBranch?.properties?.[fieldParams.name]; if (conditionalProperty) { @@ -142,20 +147,26 @@ export function calculateConditionalProperties({ fieldParams, customProperties, ), }; - return omit(merge(base, presentation, newFieldParams), ['inputType']); + return { + rootFieldAttrs: fieldParams, + newAttributes: omit(merge(base, presentation, newFieldParams), ['inputType']), + }; } // If field is not conditionally declared it should be visible if it's required const isVisible = isRequired; return { - isVisible, - required: isRequired, - schema: buildYupSchema({ - ...fieldParams, - ...extractParametersFromNode(conditionBranch), + rootFieldAttrs: fieldParams, + newAttributes: { + isVisible, required: isRequired, - }), + schema: buildYupSchema({ + ...fieldParams, + ...extractParametersFromNode(conditionBranch), + required: isRequired, + }), + }, }; }; } diff --git a/src/helpers.js b/src/helpers.js index 4ba02c593..1a86783ac 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -12,6 +12,41 @@ import { processJSONLogicNode } from './jsonLogic'; import { hasProperty } from './utils'; import { buildCompleteYupSchema, buildYupSchema } from './yupSchema'; +/** + * List of custom JSF's attributes for field + * that are added dynamically after the first parsing. + */ +const dynamicInternalJsfAttrs = [ + 'isVisible', // Driven from conditionals state + 'fields', // driven from group-array + 'getComputedAttributes', // From json-logic + 'computedAttributes', // From json-logic + 'calculateConditionalProperties', // driven from conditionals + 'calculateCustomValidationProperties', // To be deprecated in favor of json-logic + 'scopedJsonSchema', // The respective JSON Schema +]; +const dynamicInternalJsfAttrsObj = Object.fromEntries( + dynamicInternalJsfAttrs.map((k) => [k, true]) +); + +/** + * + * @param {Object} field - Current field attributes + * @param {Object} conditionalAttrs - Attributes from the matched conditional + * @param {Object} rootAttrs - Original field attributes from the root. + */ +function removeConditionalStaleAttributes(field, conditionalAttrs, rootAttrs) { + Object.keys(field).forEach((key) => { + if ( + conditionalAttrs[key] === undefined && + rootAttrs[key] === undefined && // Don't remove attrs that were declared in the root field. + dynamicInternalJsfAttrsObj[key] === undefined // ignore these because they are internal + ) { + field[key] = undefined; + } + }); +} + /** * @typedef {import('./createHeadlessForm').FieldParameters} FieldParameters * @typedef {import('./createHeadlessForm').FieldValues} FieldValues @@ -202,9 +237,9 @@ function updateField(field, requiredFields, node, formValues, logic, config) { field.isVisible = true; } - const updateValues = (fieldValues) => - Object.entries(fieldValues).forEach(([key, value]) => { - // some values (eg "schema") are a function, so we need to call it here + const updateAttributes = (fieldAttrs) => { + Object.entries(fieldAttrs).forEach(([key, value]) => { + // some attributes' value (eg "schema") are a function, so we need to call it here field[key] = typeof value === 'function' ? value() : value; if (key === 'value') { @@ -212,9 +247,9 @@ function updateField(field, requiredFields, node, formValues, logic, config) { // unless it's a read-only field // If the readOnly property has changed, use that updated value, // otherwise use the start value of the property - const readOnlyPropertyWasUpdated = typeof fieldValues.readOnly !== 'undefined'; + const readOnlyPropertyWasUpdated = typeof fieldAttrs.readOnly !== 'undefined'; const isReadonlyByDefault = field.readOnly; - const isReadonly = readOnlyPropertyWasUpdated ? fieldValues.readOnly : isReadonlyByDefault; + const isReadonly = readOnlyPropertyWasUpdated ? fieldAttrs.readOnly : isReadonlyByDefault; // Needs field.type check because otherwise checkboxes will have an initial // value of "true" when they should be not checked. !8755 for full context @@ -226,9 +261,10 @@ function updateField(field, requiredFields, node, formValues, logic, config) { } } }); + }; if (field.getComputedAttributes) { - const computedFieldValues = field.getComputedAttributes({ + const newAttributes = field.getComputedAttributes({ field, isRequired: fieldIsRequired, node, @@ -236,26 +272,27 @@ function updateField(field, requiredFields, node, formValues, logic, config) { config, logic, }); - updateValues(computedFieldValues); + updateAttributes(newAttributes); } // If field has a calculateConditionalProperties closure, run it and update the field properties if (field.calculateConditionalProperties) { - const newFieldValues = field.calculateConditionalProperties({ + const { rootFieldAttrs, newAttributes } = field.calculateConditionalProperties({ isRequired: fieldIsRequired, conditionBranch: node, formValues, }); - updateValues(newFieldValues); + updateAttributes(newAttributes); + removeConditionalStaleAttributes(field, newAttributes, rootFieldAttrs); } if (field.calculateCustomValidationProperties) { - const newFieldValues = field.calculateCustomValidationProperties( + const newAttributes = field.calculateCustomValidationProperties( fieldIsRequired, node, formValues ); - updateValues(newFieldValues); + updateAttributes(newAttributes); } } diff --git a/src/tests/conditions.test.js b/src/tests/conditions.test.js index 2f00789bd..9114e5e89 100644 --- a/src/tests/conditions.test.js +++ b/src/tests/conditions.test.js @@ -27,12 +27,7 @@ it('Should allow check of a nested property in a conditional', () => { additionalProperties: false, properties: { child: { - oneOf: [ - { - const: 'yes', - }, - { const: 'no' }, - ], + oneOf: [{ const: 'yes' }, { const: 'no' }], type: 'string', }, }, @@ -56,3 +51,311 @@ it('Should allow check of a nested property in a conditional', () => { undefined ); }); + +describe('Conditional attributes updated', () => { + it('Update basic case with const, default, maximum', () => { + const { fields, handleValidation } = createHeadlessForm( + { + properties: { + is_full_time: { type: 'string', oneOf: [{ const: 'yes' }, { const: 'no' }] }, + hours: { type: 'number' }, + }, + allOf: [ + { + if: { + properties: { is_full_time: { const: 'yes' } }, + required: ['is_full_time'], + }, + then: { + properties: { + hours: { + const: 8, + default: 8, + }, + }, + }, + else: { + properties: { + hours: { + maximum: 4, + }, + }, + }, + }, + ], + }, + { strictInputType: false } + ); + + // Given "Yes" it applies "const" and "default" + expect(handleValidation({ is_full_time: 'yes', hours: 4 }).formErrors).toEqual({ + hours: 'The only accepted value is 8.', + }); + expect(fields[1]).toMatchObject({ const: 8, default: 8 }); + expect(fields[1].maximum).toBeUndefined(); + + // Changing to "No", applies the "maximum" and cleans "const" and "default" + expect(handleValidation({ is_full_time: 'no', hours: 4 }).formErrors).toBeUndefined(); + expect(fields[1]).toMatchObject({ maximum: 4 }); + expect(fields[1].const).toBeUndefined(); + expect(fields[1].default).toBeUndefined(); + + // Changing back to "Yes", it removes "maximum", and applies "const" and "default" + expect(handleValidation({}).formErrors).toBeUndefined(); + expect(handleValidation({ is_full_time: 'yes', hours: 8 }).formErrors).toBeUndefined(); + expect(fields[1].maximum).toBeUndefined(); + expect(fields[1]).toMatchObject({ const: 8, default: 8 }); + }); + + it('Update a new attribute (eg description)', () => { + const { fields, handleValidation } = createHeadlessForm( + { + properties: { + is_full_time: { type: 'string', oneOf: [{ const: 'yes' }, { const: 'no' }] }, + hours: { + type: 'number', + }, + }, + allOf: [ + { + if: { + properties: { is_full_time: { const: 'yes' } }, + required: ['is_full_time'], + }, + then: { + properties: { + hours: { + description: 'We recommend 8 hours.', + }, + }, + }, + }, + ], + }, + { strictInputType: false } + ); + + // By default the attribute is not set. + expect(fields[1].description).toBeUndefined(); + + // Given "Yes" it applies the conditional attribute + expect(handleValidation({ is_full_time: 'yes' }).formErrors).toBeUndefined(); + expect(fields[1].description).toBe('We recommend 8 hours.'); + + // Changing to "No", removes the description + expect(handleValidation({ is_full_time: 'no' }).formErrors).toBeUndefined(); + expect(fields[1].description).toBeUndefined(); + + // Changing back to "Yes", it sets the attribute again + expect(handleValidation({ is_full_time: 'yes' }).formErrors).toBeUndefined(); + expect(fields[1].description).toBe('We recommend 8 hours.'); + }); + + it('Update an existing attribute (eg description)', () => { + const { fields, handleValidation } = createHeadlessForm( + { + properties: { + is_full_time: { type: 'string', oneOf: [{ const: 'yes' }, { const: 'no' }] }, + hours: { + type: 'number', + description: 'Any value works.', + }, + }, + allOf: [ + { + if: { + properties: { is_full_time: { const: 'yes' } }, + required: ['is_full_time'], + }, + then: { + properties: { + hours: { + description: 'We recommend 8 hours.', + }, + }, + }, + }, + ], + }, + { strictInputType: false } + ); + + // By default the attribute is set the base value. + expect(fields[1].description).toBe('Any value works.'); + + // Given "Yes" it applies the conditional attribute + expect(handleValidation({ is_full_time: 'yes', hours: 4 }).formErrors).toBeUndefined(); + expect(fields[1].description).toBe('We recommend 8 hours.'); + + // Changing to "No", it applies the base value again. + expect(handleValidation({ is_full_time: 'no', hours: 4 }).formErrors).toBeUndefined(); + expect(fields[1].description).toBe('Any value works.'); + + // Changing back to "Yes", it sets the attribute again + expect(handleValidation({ is_full_time: 'yes', hours: 8 }).formErrors).toBeUndefined(); + expect(fields[1].description).toBe('We recommend 8 hours.'); + }); + + it('Update a nested attribute', () => { + const { fields, handleValidation } = createHeadlessForm( + { + properties: { + is_full_time: { type: 'string', oneOf: [{ const: 'yes' }, { const: 'no' }] }, + hours: { + type: 'number', + presentation: { + inputType: 'number', + anything: 'info', + }, + }, + }, + allOf: [ + { + if: { + properties: { is_full_time: { const: 'yes' } }, + required: ['is_full_time'], + }, + then: { + properties: { + hours: { + presentation: { + anything: 'danger', + }, + }, + }, + }, + }, + ], + }, + { strictInputType: false } + ); + + // By default the attribute is set the base value. + expect(fields[1].anything).toBe('info'); + + // Given "Yes" it applies the conditional attribute + expect(handleValidation({ is_full_time: 'yes' }).formErrors).toBeUndefined(); + expect(fields[1].anything).toBe('danger'); + + // Changing to "No", it applies the base value again. + expect(handleValidation({ is_full_time: 'no' }).formErrors).toBeUndefined(); + expect(fields[1].anything).toBe('info'); + + // Changing back to "Yes", it sets the attribute again + expect(handleValidation({ is_full_time: 'yes' }).formErrors).toBeUndefined(); + expect(fields[1].anything).toBe('danger'); + }); + + it('Keeps existing attributes in matches that dont change the attr', () => { + const { fields, handleValidation } = createHeadlessForm( + { + properties: { + is_full_time: { type: 'string', oneOf: [{ const: 'yes' }, { const: 'no' }] }, + hours: { + type: 'number', + description: 'Any value works.', + }, + }, + allOf: [ + { + if: { + properties: { is_full_time: { const: 'yes' } }, + required: ['is_full_time'], + }, + then: { + required: ['hours'], + }, + else: { + properties: { + hours: false, + }, + }, + }, + ], + }, + { strictInputType: false } + ); + + // By default the attribute is set the base value, even though the field is invisible. + expect(fields[1].description).toBe('Any value works.'); + expect(fields[1].isVisible).toBe(false); + + // Given "Yes" it keeps the base value + expect(handleValidation({ is_full_time: 'yes' }).formErrors).toEqual({ + hours: 'Required field', + }); + expect(fields[1].description).toBe('Any value works.'); + expect(fields[1].isVisible).toBe(true); + + // Changing to "No" it keeps the base value + expect(handleValidation({ is_full_time: 'no' }).formErrors).toBeUndefined(); + expect(fields[1].description).toBe('Any value works.'); + expect(fields[1].isVisible).toBe(false); + }); + + it('Keeps internal attributes (fieldAttrsFromJsf)', () => { + // This is necessary while we keep supporting "type", even if deprecated + // otherwise our Remote app will break because it didn't migrate + // from "type" to "inputType" yet. + const { fields, handleValidation } = createHeadlessForm( + { + properties: { + is_full_time: { type: 'string', oneOf: [{ const: 'yes' }, { const: 'no' }] }, + salary_period: { + type: 'string', + title: 'Salary period', + oneOf: [ + { title: 'Weekly', const: 'weekly' }, + { title: 'Monthly', const: 'monthly' }, + ], + }, + }, + allOf: [ + { + if: { + properties: { is_full_time: { const: 'yes' } }, + required: ['is_full_time'], + }, + then: { + properties: { + salary_period: { + description: 'We recommend montlhy.', + }, + }, + }, + }, + ], + }, + { strictInputType: false } + ); + + // Given "Yes" it keeps the "type" + handleValidation({ is_full_time: 'yes' }); + + // All the following attrs are never removed + // during conditionals because they are core. + expect(fields[1]).toMatchObject({ + name: 'salary_period', + label: 'Salary period', + required: false, + type: 'radio', + inputType: 'radio', + jsonType: 'string', + computedAttributes: {}, + calculateConditionalProperties: expect.any(Function), + schema: expect.any(Object), + scopedJsonSchema: expect.any(Object), + isVisible: true, + options: [ + { + label: 'Weekly', + value: 'weekly', + }, + { + label: 'Monthly', + value: 'monthly', + }, + ], + }); + }); +}); diff --git a/src/tests/createHeadlessForm.test.js b/src/tests/createHeadlessForm.test.js index 6cb4a0cb5..415846420 100644 --- a/src/tests/createHeadlessForm.test.js +++ b/src/tests/createHeadlessForm.test.js @@ -446,7 +446,7 @@ describe('createHeadlessForm', () => { [fieldName]: 'The option "blah-blah" is not valid.', }); - // Given undefined, it says it's a required field. + // Given undefined, it says it's a required field. expect(validateForm({})).toEqual({ [fieldName]: 'Required field', }); @@ -856,6 +856,7 @@ describe('createHeadlessForm', () => { has_car: 'yes', }) ).toBeUndefined(); + expect(validateForm({})).toEqual({ has_siblings: 'Required field', }); @@ -1419,6 +1420,50 @@ describe('createHeadlessForm', () => { ], }); }); + + it('can be a conditional field', () => { + const { fields, handleValidation } = createHeadlessForm({ + properties: { + yes_or_no: { + title: 'Show the dependents or not?', + oneOf: [{ const: 'yes' }, { const: 'no' }], + 'x-jsf-presentation': { inputType: 'radio' }, + }, + dependent_details: mockGroupArrayInput, + }, + allOf: [ + { + if: { + properties: { + yes_or_no: { const: 'yes' }, + }, + required: ['yes_or_no'], + }, + then: { + required: ['dependent_details'], + }, + else: { + properties: { + dependent_details: false, + }, + }, + }, + ], + }); + + // By default is hidden but the fields are accessible + expect(getField(fields, 'dependent_details').isVisible).toBe(false); + expect(getField(fields, 'dependent_details').fields).toEqual(expect.any(Function)); + + // When the condition matches... + const { formErrors } = handleValidation({ yes_or_no: 'yes' }); + expect(formErrors).toEqual({ + dependent_details: 'Required field', + }); + // it gets visible with its inner fields. + expect(getField(fields, 'dependent_details').isVisible).toBe(true); + expect(getField(fields, 'dependent_details').fields).toEqual(expect.any(Function)); + }); }); it('supports "radio" field type with its "card" and "card-expandable" variants', () => { @@ -1808,11 +1853,8 @@ describe('createHeadlessForm', () => { expect(foodField.options).toHaveLength(4); // ...Food description was back to the original expect(foodField.description).toBeUndefined(); - - // @BUG RMT-58 PTO description should disappear, but it didn't. - expect(getField(fields, 'pto').description).toBe( - 'Above 30 hours, the PTO needs to be at least 20 days.' - ); + // ...PTO Description is removed too. + expect(getField(fields, 'pto').description).toBeUndefined(); // Given again "low perks", the form valid. expect( diff --git a/src/tests/jsonLogic.fixtures.js b/src/tests/jsonLogic.fixtures.js index 11c749224..781bb9026 100644 --- a/src/tests/jsonLogic.fixtures.js +++ b/src/tests/jsonLogic.fixtures.js @@ -250,7 +250,7 @@ export const schemaWithComputedAttributeThatDoesntExist = { field_a: { type: 'number', 'x-jsf-logic-computedAttrs': { - default: 'iDontExist', + minimum: 'iDontExist', }, }, },