From 76c0e727856150dd0c5cf425a75d0f038475973c Mon Sep 17 00:00:00 2001 From: Brendan Bond Date: Fri, 2 Aug 2024 16:46:48 -0400 Subject: [PATCH] Merge pull request #126 from formio/fix/FIO-8731_validation_hidden_comp_in_nested_forms --- Changelog.md | 4 + Readme.md | 4 + src/process/__tests__/fixtures/index.ts | 5 +- .../skipValidForConditionallyHiddenComp.json | 67 +++++++++++++++ .../skipValidForLogicallyHiddenComp.json | 86 +++++++++++++++++++ .../skipValidWithHiddenParentComp.json | 55 ++++++++++++ src/process/__tests__/process.test.ts | 64 +++++++++++++- src/process/validation/index.ts | 8 +- src/utils/formUtil.ts | 19 +++- src/utils/isParentHidden.ts | 5 +- 10 files changed, 305 insertions(+), 12 deletions(-) create mode 100644 src/process/__tests__/fixtures/skipValidForConditionallyHiddenComp.json create mode 100644 src/process/__tests__/fixtures/skipValidForLogicallyHiddenComp.json create mode 100644 src/process/__tests__/fixtures/skipValidWithHiddenParentComp.json diff --git a/Changelog.md b/Changelog.md index 5b93d20a..689bfdc3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +## [Unreleased: 2.2.0-rc.8] +### Changed + - FIO-8731: Fixes component gets validated when being in a hidden parent + ## 2.2.0-rc.7 ### Changed - FIO-8597: fixed an issue with an empty array value for a number component with multiple values enabled diff --git a/Readme.md b/Readme.md index 16429cea..d35b42e4 100644 --- a/Readme.md +++ b/Readme.md @@ -60,3 +60,7 @@ FormioCore.render(document.getElementById('data-table'), { ``` See https://formio.github.io/core for more examples of how to use this library. + +### Debug + +[Instructions on how to debug with API Server](https://formio.atlassian.net/wiki/spaces/SD/pages/184025089/Debugging+formio+core) diff --git a/src/process/__tests__/fixtures/index.ts b/src/process/__tests__/fixtures/index.ts index 1d532970..05ef6349 100644 --- a/src/process/__tests__/fixtures/index.ts +++ b/src/process/__tests__/fixtures/index.ts @@ -1,7 +1,10 @@ import clearOnHideWithCustomCondition from './clearOnHideWithCustomCondition.json'; import clearOnHideWithHiddenParent from './clearOnHideWithHiddenParent.json'; +import skipValidForConditionallyHiddenComp from './skipValidForConditionallyHiddenComp.json'; +import skipValidForLogicallyHiddenComp from './skipValidForLogicallyHiddenComp.json'; +import skipValidWithHiddenParentComp from './skipValidWithHiddenParentComp.json'; import data1a from './data1a.json'; import form1 from './form1.json'; import subs from './subs.json'; -export { clearOnHideWithCustomCondition, clearOnHideWithHiddenParent, data1a, form1, subs }; +export { clearOnHideWithCustomCondition, clearOnHideWithHiddenParent, skipValidForLogicallyHiddenComp, skipValidForConditionallyHiddenComp, skipValidWithHiddenParentComp, data1a, form1, subs }; diff --git a/src/process/__tests__/fixtures/skipValidForConditionallyHiddenComp.json b/src/process/__tests__/fixtures/skipValidForConditionallyHiddenComp.json new file mode 100644 index 00000000..0812334e --- /dev/null +++ b/src/process/__tests__/fixtures/skipValidForConditionallyHiddenComp.json @@ -0,0 +1,67 @@ +{ + "form": { + "name": "conditional", + "path": "conditional", + "type": "form", + "display": "form", + "components": [ + { + "label": "Checkbox", + "tableView": false, + "validateWhenHidden": false, + "key": "checkbox", + "type": "checkbox", + "input": true + }, + { + "collapsible": false, + "key": "panel", + "conditional": { + "show": false, + "conjunction": "all", + "conditions": [ + { + "component": "checkbox", + "operator": "isEqual", + "value": true + } + ] + }, + "type": "panel", + "label": "Panel", + "input": false, + "tableView": false, + "components": [ + { + "label": "Text Field", + "applyMaskOn": "change", + "tableView": true, + "validate": { + "required": true + }, + "validateWhenHidden": false, + "key": "textField", + "type": "textfield", + "input": true + } + ] + }, + { + "type": "button", + "label": "Submit", + "key": "submit", + "disableOnInvalid": true, + "input": true, + "tableView": false + } + ], + "created": "2024-08-02T10:28:35.696Z", + "modified": "2024-08-02T10:28:35.704Z" + }, + "submission": { + "data": { + "checkbox": true, + "submit": true + } + } +} \ No newline at end of file diff --git a/src/process/__tests__/fixtures/skipValidForLogicallyHiddenComp.json b/src/process/__tests__/fixtures/skipValidForLogicallyHiddenComp.json new file mode 100644 index 00000000..ee9a887b --- /dev/null +++ b/src/process/__tests__/fixtures/skipValidForLogicallyHiddenComp.json @@ -0,0 +1,86 @@ +{ + "form": { + "type": "form", + "display": "form", + "components": [ + { + "label": "Checkbox", + "tableView": false, + "validateWhenHidden": false, + "key": "checkbox", + "type": "checkbox", + "input": true + }, + { + "collapsible": false, + "key": "panel", + "logic": [ + { + "name": "1", + "trigger": { + "type": "simple", + "simple": { + "show": true, + "conjunction": "all", + "conditions": [ + { + "component": "checkbox", + "operator": "isEqual", + "value": true + } + ] + } + }, + "actions": [ + { + "name": "12", + "type": "property", + "property": { + "label": "Hidden", + "value": "hidden", + "type": "boolean" + }, + "state": true + } + ] + } + ], + "type": "panel", + "label": "Panel", + "input": false, + "tableView": false, + "components": [ + { + "label": "Text Field", + "applyMaskOn": "change", + "tableView": true, + "validate": { + "required": true + }, + "validateWhenHidden": false, + "key": "textField", + "type": "textfield", + "input": true + } + ] + }, + { + "type": "button", + "label": "Submit", + "key": "submit", + "disableOnInvalid": true, + "input": true, + "tableView": false + } + ], + "created": "2024-08-02T10:28:35.696Z", + "modified": "2024-08-02T10:28:35.704Z" + }, + "submission": { + "data": { + "checkbox": true, + "submit": true + } + } +} + diff --git a/src/process/__tests__/fixtures/skipValidWithHiddenParentComp.json b/src/process/__tests__/fixtures/skipValidWithHiddenParentComp.json new file mode 100644 index 00000000..94f9fcb9 --- /dev/null +++ b/src/process/__tests__/fixtures/skipValidWithHiddenParentComp.json @@ -0,0 +1,55 @@ +{ + "form": { + "type": "form", + "display": "form", + "components": [ + { + "label": "Checkbox", + "tableView": false, + "validateWhenHidden": false, + "key": "checkbox", + "type": "checkbox", + "input": true + }, + { + "collapsible": false, + "hidden": true, + "key": "panel", + "type": "panel", + "label": "Panel", + "input": false, + "tableView": false, + "components": [ + { + "label": "Text Field", + "applyMaskOn": "change", + "tableView": true, + "validate": { + "required": true + }, + "validateWhenHidden": false, + "key": "textField", + "type": "textfield", + "input": true + } + ] + }, + { + "type": "button", + "label": "Submit", + "key": "submit", + "disableOnInvalid": true, + "input": true, + "tableView": false + } + ], + "created": "2024-08-02T11:13:50.020Z", + "modified": "2024-08-02T11:14:14.155Z" + }, + "submission": { + "data": { + "checkbox": true, + "submit": true + } + } +} \ No newline at end of file diff --git a/src/process/__tests__/process.test.ts b/src/process/__tests__/process.test.ts index e4190060..a4839c59 100644 --- a/src/process/__tests__/process.test.ts +++ b/src/process/__tests__/process.test.ts @@ -2,8 +2,8 @@ import { expect } from 'chai'; import assert from 'node:assert' import type { ContainerComponent, ValidationScope } from 'types'; import { getComponent } from 'utils/formUtil'; -import { processSync, ProcessTargets } from '../index'; -import { clearOnHideWithCustomCondition, clearOnHideWithHiddenParent, } from './fixtures' +import { process, processSync, ProcessTargets } from '../index'; +import { clearOnHideWithCustomCondition, clearOnHideWithHiddenParent, skipValidForConditionallyHiddenComp, skipValidForLogicallyHiddenComp, skipValidWithHiddenParentComp } from './fixtures' /* describe('Process Tests', () => { it('Should perform the processes using the processReduced method.', async () => { @@ -2825,6 +2825,66 @@ describe('Process Tests', () => { }); }); + it('Should skip child validation with conditional', async () => { + const { form, submission } = skipValidForConditionallyHiddenComp; + const context = { + form, + submission, + data: submission.data, + components: form.components, + processors: ProcessTargets.submission, + scope: {}, + config: { + server: true, + }, + }; + + processSync(context); + context.processors = ProcessTargets.evaluator; + processSync(context); + expect((context.scope as ValidationScope).errors).to.have.length(0); + }); + + it('Should skip child validation with hidden parent component', async () => { + const { form, submission } = skipValidWithHiddenParentComp; + const context = { + form, + submission, + data: submission.data, + components: form.components, + processors: ProcessTargets.submission, + scope: {}, + config: { + server: true, + }, + }; + + await process(context); + context.processors = ProcessTargets.evaluator; + await process(context); + expect((context.scope as ValidationScope).errors).to.have.length(0); + }); + + it('Should skip child validation with logic', async () => { + const { form, submission } = skipValidForLogicallyHiddenComp; + const context = { + form, + submission, + data: submission.data, + components: form.components, + processors: ProcessTargets.submission, + scope: {}, + config: { + server: true, + }, + }; + + processSync(context as any); + context.processors = ProcessTargets.evaluator; + processSync(context as any); + expect((context.scope as ValidationScope).errors).to.have.length(0); + }); + it('Should not return fields from conditionally hidden containers, clearOnHide = false', async () => { const { form, submission } = clearOnHideWithCustomCondition; const containerComponent = getComponent(form.components, 'section6') as ContainerComponent; diff --git a/src/process/validation/index.ts b/src/process/validation/index.ts index 43f737c6..1e84be67 100644 --- a/src/process/validation/index.ts +++ b/src/process/validation/index.ts @@ -2,13 +2,12 @@ import { ConditionsScope, ProcessorFn, ProcessorFnSync, ProcessorInfo, Validatio import { evaluationRules, rules, serverRules } from "./rules"; import find from "lodash/find"; import get from "lodash/get"; -import set from "lodash/set"; import pick from "lodash/pick"; import { getComponentAbsolutePath, getComponentPath } from "utils/formUtil"; import { getErrorMessage } from "utils/error"; import { FieldError } from "error"; import { ConditionallyHidden, isConditionallyHidden, isCustomConditionallyHidden, isSimpleConditionallyHidden } from "processes/conditions"; -import { validate } from 'fast-json-patch'; +import { isParentHidden } from 'utils/isParentHidden'; // Cleans up validation errors to remove unnessesary parts // and make them transferable to ivm. @@ -96,6 +95,7 @@ export function isForcedHidden(context: ValidationContext, isConditionallyHidden export const _shouldSkipValidation = (context: ValidationContext, isConditionallyHidden: ConditionallyHidden) => { const { component, scope, path } = context; + if ( (scope as ConditionsScope)?.conditionals && find((scope as ConditionsScope).conditionals, { @@ -115,9 +115,11 @@ export const _shouldSkipValidation = (context: ValidationContext, isConditionall () => isValueHidden(context), // Force valid if component is hidden. () => isForcedHidden(context, isConditionallyHidden) && !validateWhenHidden, + // Do not validate if 'validateWhenHidden' is disabled and some component parent is hidden + () => !validateWhenHidden && isParentHidden(component), ]; - return rules.some(pred => pred()); + return rules.some(pred => pred());; }; export const shouldSkipValidationCustom: SkipValidationFn = (context: ValidationContext) => { diff --git a/src/utils/formUtil.ts b/src/utils/formUtil.ts index 4d3e65c3..63db67be 100644 --- a/src/utils/formUtil.ts +++ b/src/utils/formUtil.ts @@ -287,9 +287,14 @@ export const eachComponentDataAsync = async ( await eachComponentDataAsync(component.components, data, fn, componentDataPath(component, path, compPath), index, component, includeAll); } return true; - } else { - return false; } + // This way layout components are added as parents to the child components + if (isComponentModelType(component, 'layout')) { + await eachComponentDataAsync(component.components, data, fn, componentDataPath(component, path, compPath), index, component, includeAll); + return true; + } + + return false; }, true, path, @@ -341,9 +346,15 @@ export const eachComponentData = ( eachComponentData(component.components, data, fn, componentDataPath(component, path, compPath), index, component, includeAll); } return true; - } else { - return false; } + + // This way layout components are added as parents to the child components + if (isComponentModelType(component, 'layout')) { + eachComponentData(component.components, data, fn, componentDataPath(component, path, compPath), index, component, includeAll); + return true; + } + + return false }, true, path, diff --git a/src/utils/isParentHidden.ts b/src/utils/isParentHidden.ts index bdfa2077..7972da09 100644 --- a/src/utils/isParentHidden.ts +++ b/src/utils/isParentHidden.ts @@ -1,4 +1,4 @@ -import { BaseComponent, Component } from 'types'; +import type { BaseComponent, Component } from 'types'; export const isParentHidden = (comp: Component) => { let parentComponent: BaseComponent | undefined = comp.parent; @@ -7,7 +7,8 @@ export const isParentHidden = (comp: Component) => { if (parentComponent.hidden) { return true; } - parentComponent = parentComponent.parent; + // Exit if there's a circular reference in 'parent' prop + parentComponent = parentComponent === parentComponent.parent ? undefined : parentComponent.parent; } return false;