From 9746c81dab04b6b73c604b70b1af448c560d943e Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Tue, 5 Dec 2023 16:11:29 +0200 Subject: [PATCH 01/13] Add new example for conditional required --- packages/examples/src/examples/if-allOf.ts | 85 ++++++++++++++++++++++ packages/examples/src/index.ts | 2 + 2 files changed, 87 insertions(+) create mode 100644 packages/examples/src/examples/if-allOf.ts diff --git a/packages/examples/src/examples/if-allOf.ts b/packages/examples/src/examples/if-allOf.ts new file mode 100644 index 000000000..3f3c95b20 --- /dev/null +++ b/packages/examples/src/examples/if-allOf.ts @@ -0,0 +1,85 @@ +/* + The MIT License + + Copyright (c) 2017-2019 EclipseSource Munich + https://github.com/eclipsesource/jsonforms + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. +*/ +import { registerExamples } from '../register'; + +export const schema = { + type: 'object', + properties: { + foo: { type: 'string' }, + bar: { type: 'string' }, + baz: { type: 'string' }, + }, + allOf: [ + { + if: { + properties: { + foo: { const: 'bar' }, + }, + }, + then: { required: ['bar'] }, + }, + { + if: { + properties: { + foo: { const: 'baz' }, + }, + }, + then: { required: ['baz'] }, + }, + ], +}; + +export const uischema = { + type: 'VerticalLayout', + elements: [ + { + label: 'Foo', + type: 'Control', + scope: '#/properties/foo', + }, + { + type: 'Control', + label: 'bar', + scope: '#/properties/bar', + }, + { + type: 'Control', + label: 'baz', + scope: '#/properties/baz', + }, + ], +}; + +const data = {}; + +registerExamples([ + { + name: 'if-allOf', + label: 'If AllOf', + data, + schema, + uischema, + }, +]); diff --git a/packages/examples/src/index.ts b/packages/examples/src/index.ts index f4c5ed51b..1c88a99d7 100644 --- a/packages/examples/src/index.ts +++ b/packages/examples/src/index.ts @@ -75,11 +75,13 @@ export * from './register'; export * from './example'; import * as ifThenElse from './examples/if_then_else'; +import * as allOfIf from './examples/if-allOf'; export { issue_1948, defaultExample, allOf, + allOfIf, anyOf, oneOf, oneOfArray, From 67ab199892cf8434291cce3e740ab7db1aad6d99 Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Tue, 5 Dec 2023 21:13:14 +0200 Subject: [PATCH 02/13] Fix required not showing with conditional subschemas Fix the isRequired function's required check when having conditional subschemas - whether it is a single if-the-else or multiple if-then-else inside allOf combinator. --- packages/core/src/util/renderer.ts | 80 ++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index 1346efdef..d6a731af0 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -28,6 +28,8 @@ import { ControlElement, isLabelable, JsonSchema, + JsonSchema4, + JsonSchema7, LabelElement, UISchemaElement, } from '../models'; @@ -80,11 +82,67 @@ import { } from '../i18n/arrayTranslations'; import { resolveSchema } from './resolvers'; import cloneDeep from 'lodash/cloneDeep'; +import { has } from 'lodash'; +import { all, any } from 'lodash/fp'; + +/** + * Check if property's required attribute is set based on if-then-else condition + * + */ +const checkRequiredInIf = (schema: JsonSchema, segment: string, data: any) => { + const propertiesCondition = get(get(schema, 'if'), 'properties') as any; + + const condition = all((property) => { + //TODO: add pattern + if (has(propertiesCondition[property], 'const')) { + return data[property] === get(propertiesCondition[property], 'const'); + } else if (has(propertiesCondition[property], 'enum')) { + return (get(propertiesCondition[property], 'enum') as unknown[]).includes( + data[property] + ); + } + + return false; + }, Object.keys(propertiesCondition)); + + if (has(get(schema, 'then'), 'required') && condition) { + const requiredProperties = get(get(schema, 'then'), 'required') as string[]; + + return requiredProperties.includes(segment); + } else if (has(get(schema, 'else'), 'required') && !condition) { + const requiredProperties = get(get(schema, 'else'), 'required') as string[]; + + return requiredProperties.includes(segment); + } + + return false; +}; + +/** + * Check if property becomes required based on some if-then-else condition + * that is part of allOf combinator + */ +const conditionallyRequired = ( + schema: JsonSchema, + segment: string, + data: any +) => { + const nestedAllOfSchema = get(schema, 'allOf'); + + return any((subschema: JsonSchema4 | JsonSchema7) => { + if (has(subschema, 'if')) { + return checkRequiredInIf(subschema, segment, data); + } + + return false; + }, nestedAllOfSchema); +}; const isRequired = ( schema: JsonSchema, schemaPath: string, - rootSchema: JsonSchema + rootSchema: JsonSchema, + data: any ): boolean => { const pathSegments = schemaPath.split('/'); const lastSegment = pathSegments[pathSegments.length - 1]; @@ -100,10 +158,22 @@ const isRequired = ( rootSchema ); + const requiredInIf = + has(nextHigherSchema, 'if') && + checkRequiredInIf(nextHigherSchema, lastSegment, data); + + const requiredConditionally = conditionallyRequired( + nextHigherSchema, + lastSegment, + data + ); + return ( - nextHigherSchema !== undefined && - nextHigherSchema.required !== undefined && - nextHigherSchema.required.indexOf(lastSegment) !== -1 + (nextHigherSchema !== undefined && + nextHigherSchema.required !== undefined && + nextHigherSchema.required.indexOf(lastSegment) !== -1) || + requiredInIf || + requiredConditionally ); }; @@ -500,7 +570,7 @@ export const mapStateToControlProps = ( const rootSchema = getSchema(state); const required = controlElement.scope !== undefined && - isRequired(ownProps.schema, controlElement.scope, rootSchema); + isRequired(ownProps.schema, controlElement.scope, rootSchema, rootData); const resolvedSchema = Resolve.schema( ownProps.schema || rootSchema, controlElement.scope, From 9afddab8c15b93e80a6085652ee64ea7bc76d4d3 Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Wed, 6 Dec 2023 11:34:19 +0200 Subject: [PATCH 03/13] Add pattern support and nested checks Make the if condition work with pattern and fix the check to work with nested properties --- packages/core/src/util/renderer.ts | 24 ++++++++++---- packages/examples/src/examples/if-allOf.ts | 37 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index d6a731af0..97bdca771 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -60,7 +60,7 @@ import type { CombinatorKeyword } from './combinators'; import { moveDown, moveUp } from './array'; import type { AnyAction, Dispatch } from './type'; import { Resolve, convertDateToString, hasType } from './util'; -import { composePaths, composeWithUi } from './path'; +import { composePaths, composeWithUi, toDataPath } from './path'; import { CoreActions, update } from '../actions'; import type { ErrorObject } from 'ajv'; import type { JsonFormsState } from '../store'; @@ -93,13 +93,22 @@ const checkRequiredInIf = (schema: JsonSchema, segment: string, data: any) => { const propertiesCondition = get(get(schema, 'if'), 'properties') as any; const condition = all((property) => { - //TODO: add pattern if (has(propertiesCondition[property], 'const')) { - return data[property] === get(propertiesCondition[property], 'const'); + return ( + has(data, property) && + data[property] === get(propertiesCondition[property], 'const') + ); } else if (has(propertiesCondition[property], 'enum')) { - return (get(propertiesCondition[property], 'enum') as unknown[]).includes( - data[property] + return ( + has(data, property) && + (get(propertiesCondition[property], 'enum') as unknown[]).includes( + data[property] + ) ); + } else if (has(propertiesCondition[property], 'pattern')) { + const pattern = new RegExp(get(propertiesCondition[property], 'pattern')); + + return has(data, property) && pattern.test(data[property]); } return false; @@ -157,15 +166,16 @@ const isRequired = ( nextHigherSchemaPath, rootSchema ); + const currentData = Resolve.data(data, toDataPath(nextHigherSchemaPath)); const requiredInIf = has(nextHigherSchema, 'if') && - checkRequiredInIf(nextHigherSchema, lastSegment, data); + checkRequiredInIf(nextHigherSchema, lastSegment, currentData); const requiredConditionally = conditionallyRequired( nextHigherSchema, lastSegment, - data + currentData ); return ( diff --git a/packages/examples/src/examples/if-allOf.ts b/packages/examples/src/examples/if-allOf.ts index 3f3c95b20..720aee095 100644 --- a/packages/examples/src/examples/if-allOf.ts +++ b/packages/examples/src/examples/if-allOf.ts @@ -30,6 +30,23 @@ export const schema = { foo: { type: 'string' }, bar: { type: 'string' }, baz: { type: 'string' }, + nested: { + type: 'object', + properties: { + foo: { type: 'string' }, + bar: { type: 'string' }, + }, + allOf: [ + { + if: { + properties: { + foo: { const: 'bar' }, + }, + }, + then: { required: ['bar'] }, + }, + ], + }, }, allOf: [ { @@ -48,6 +65,16 @@ export const schema = { }, then: { required: ['baz'] }, }, + { + if: { + properties: { + foo: { pattern: 'foo.' }, + }, + }, + then: { + required: ['baz', 'bar'], + }, + }, ], }; @@ -69,6 +96,16 @@ export const uischema = { label: 'baz', scope: '#/properties/baz', }, + { + type: 'Control', + label: 'foo1', + scope: '#/properties/nested/properties/foo', + }, + { + type: 'Control', + label: 'bar1', + scope: '#/properties/nested/properties/bar', + }, ], }; From 2cf58243e587a447e2b289f828b53ba179044ffb Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Wed, 6 Dec 2023 16:18:27 +0200 Subject: [PATCH 04/13] Add support for nested allOf and if-then-else --- packages/core/src/util/renderer.ts | 57 ++++++++++++++-------- packages/examples/src/examples/if-allOf.ts | 18 ++++--- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index 97bdca771..18c82275e 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -89,8 +89,15 @@ import { all, any } from 'lodash/fp'; * Check if property's required attribute is set based on if-then-else condition * */ -const checkRequiredInIf = (schema: JsonSchema, segment: string, data: any) => { - const propertiesCondition = get(get(schema, 'if'), 'properties') as any; +const checkRequiredInIf = ( + schema: JsonSchema, + segment: string, + data: Record +): boolean => { + const propertiesCondition = get(get(schema, 'if'), 'properties') as Record< + string, + unknown + >; const condition = all((property) => { if (has(propertiesCondition[property], 'const')) { @@ -108,23 +115,36 @@ const checkRequiredInIf = (schema: JsonSchema, segment: string, data: any) => { } else if (has(propertiesCondition[property], 'pattern')) { const pattern = new RegExp(get(propertiesCondition[property], 'pattern')); - return has(data, property) && pattern.test(data[property]); + return ( + has(data, property) && + typeof data[property] === 'string' && + pattern.test(data[property] as string) + ); } return false; }, Object.keys(propertiesCondition)); - if (has(get(schema, 'then'), 'required') && condition) { - const requiredProperties = get(get(schema, 'then'), 'required') as string[]; - - return requiredProperties.includes(segment); - } else if (has(get(schema, 'else'), 'required') && !condition) { - const requiredProperties = get(get(schema, 'else'), 'required') as string[]; - - return requiredProperties.includes(segment); - } + const requiredInThen = has(get(schema, 'then'), 'required'); + const requiredInElse = has(get(schema, 'else'), 'required'); + const ifInThen = has(get(schema, 'then'), 'if'); + const ifInElse = has(get(schema, 'else'), 'if'); + const allOfInThen = has(get(schema, 'then'), 'allOf'); + const allOfInElse = has(get(schema, 'else'), 'allOf'); - return false; + return ( + (requiredInThen && + condition && + (get(get(schema, 'then'), 'required') as string[]).includes(segment)) || + (requiredInElse && + !condition && + (get(get(schema, 'else'), 'required') as string[]).includes(segment)) || + (ifInThen && checkRequiredInIf(get(schema, 'then'), segment, data)) || + (ifInElse && checkRequiredInIf(get(schema, 'else'), segment, data)) || + (allOfInThen && + conditionallyRequired(get(schema, 'then'), segment, data)) || + (allOfInElse && conditionallyRequired(get(schema, 'else'), segment, data)) + ); }; /** @@ -138,12 +158,11 @@ const conditionallyRequired = ( ) => { const nestedAllOfSchema = get(schema, 'allOf'); - return any((subschema: JsonSchema4 | JsonSchema7) => { - if (has(subschema, 'if')) { - return checkRequiredInIf(subschema, segment, data); - } - - return false; + return any((subschema: JsonSchema4 | JsonSchema7): boolean => { + return ( + (has(subschema, 'if') && checkRequiredInIf(subschema, segment, data)) || + conditionallyRequired(subschema, segment, data) + ); }, nestedAllOfSchema); }; diff --git a/packages/examples/src/examples/if-allOf.ts b/packages/examples/src/examples/if-allOf.ts index 720aee095..13102d006 100644 --- a/packages/examples/src/examples/if-allOf.ts +++ b/packages/examples/src/examples/if-allOf.ts @@ -66,14 +66,18 @@ export const schema = { then: { required: ['baz'] }, }, { - if: { - properties: { - foo: { pattern: 'foo.' }, + allOf: [ + { + if: { + properties: { + foo: { pattern: 'foo.' }, + }, + }, + then: { + required: ['baz', 'bar'], + }, }, - }, - then: { - required: ['baz', 'bar'], - }, + ], }, ], }; From 4455ed2afba5e9b9f392971e88c267440375b3f5 Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Mon, 11 Dec 2023 11:52:13 +0200 Subject: [PATCH 05/13] Address comments about nested structures --- packages/core/src/util/renderer.ts | 143 +++++++++++++++++++++++------ 1 file changed, 114 insertions(+), 29 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index 18c82275e..29e5f9879 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -85,6 +85,118 @@ import cloneDeep from 'lodash/cloneDeep'; import { has } from 'lodash'; import { all, any } from 'lodash/fp'; +const checkPropertyCondition = ( + propertiesCondition: Record, + property: string, + data: Record +): boolean => { + if (has(propertiesCondition[property], 'not')) { + return !checkPropertyCondition( + get(propertiesCondition[property], 'not'), + property, + data + ); + } + + if (has(propertiesCondition[property], 'properties')) { + const nextPropertyConditions = get( + propertiesCondition[property], + 'properties' + ); + + return all( + (prop) => + checkPropertyCondition( + nextPropertyConditions, + prop, + data[property] as Record + ), + Object.keys(nextPropertyConditions) + ); + } + + if (has(propertiesCondition[property], 'const')) { + return ( + has(data, property) && + data[property] === get(propertiesCondition[property], 'const') + ); + } else if (has(propertiesCondition[property], 'enum')) { + return ( + has(data, property) && + (get(propertiesCondition[property], 'enum') as unknown[]).includes( + data[property] + ) + ); + } else if (has(propertiesCondition[property], 'pattern')) { + const pattern = new RegExp(get(propertiesCondition[property], 'pattern')); + + return ( + has(data, property) && + typeof data[property] === 'string' && + pattern.test(data[property] as string) + ); + } + + return false; +}; + +const evaluateCondition = ( + schema: JsonSchema, + data: Record +): boolean => { + if (has(schema, 'allOf')) { + return all( + (subschema: JsonSchema) => evaluateCondition(subschema, data), + get(schema, 'allOf') + ); + } + + if (has(schema, 'anyOf')) { + return any( + (subschema: JsonSchema) => evaluateCondition(subschema, data), + get(schema, 'anyOf') + ); + } + + if (has(schema, 'oneOf')) { + const subschemas = get(schema, 'oneOf'); + + let satisfied = false; + + for (let i = 0; i < subschemas.length; i++) { + if (satisfied) { + return false; + } + + satisfied = evaluateCondition(subschemas[i], data); + } + + return satisfied; + } + + let requiredProperties: string[] = []; + if (has(schema, 'required')) { + requiredProperties = get(schema, 'required'); + } + + const requiredCondition = all( + (property) => data[property], + requiredProperties + ); + + const propertiesCondition = get(schema, 'properties') as Record< + string, + unknown + >; + + const valueCondition = all( + (property) => checkPropertyCondition(propertiesCondition, property, data), + Object.keys(propertiesCondition) + ); + + return requiredCondition && valueCondition; +}; + /** * Check if property's required attribute is set based on if-then-else condition * @@ -94,36 +206,9 @@ const checkRequiredInIf = ( segment: string, data: Record ): boolean => { - const propertiesCondition = get(get(schema, 'if'), 'properties') as Record< - string, - unknown - >; + const propertiesConditionSchema = get(schema, 'if'); - const condition = all((property) => { - if (has(propertiesCondition[property], 'const')) { - return ( - has(data, property) && - data[property] === get(propertiesCondition[property], 'const') - ); - } else if (has(propertiesCondition[property], 'enum')) { - return ( - has(data, property) && - (get(propertiesCondition[property], 'enum') as unknown[]).includes( - data[property] - ) - ); - } else if (has(propertiesCondition[property], 'pattern')) { - const pattern = new RegExp(get(propertiesCondition[property], 'pattern')); - - return ( - has(data, property) && - typeof data[property] === 'string' && - pattern.test(data[property] as string) - ); - } - - return false; - }, Object.keys(propertiesCondition)); + const condition = evaluateCondition(propertiesConditionSchema, data); const requiredInThen = has(get(schema, 'then'), 'required'); const requiredInElse = has(get(schema, 'else'), 'required'); From ea425a6774b596c9eed7a69f1917d451b2cdb924 Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Mon, 11 Dec 2023 15:21:04 +0200 Subject: [PATCH 06/13] Fix wrong calling of condition --- packages/core/src/util/renderer.ts | 66 ++++++++++++++++++------------ 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index 29e5f9879..b2bbcb028 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -85,13 +85,40 @@ import cloneDeep from 'lodash/cloneDeep'; import { has } from 'lodash'; import { all, any } from 'lodash/fp'; +const checkDataCondition = ( + propertyCondition: unknown, + property: string, + data: Record +) => { + if (has(propertyCondition, 'const')) { + return ( + has(data, property) && data[property] === get(propertyCondition, 'const') + ); + } else if (has(propertyCondition, 'enum')) { + return ( + has(data, property) && + (get(propertyCondition, 'enum') as unknown[]).includes(data[property]) + ); + } else if (has(propertyCondition, 'pattern')) { + const pattern = new RegExp(get(propertyCondition, 'pattern')); + + return ( + has(data, property) && + typeof data[property] === 'string' && + pattern.test(data[property] as string) + ); + } + + return false; +}; + const checkPropertyCondition = ( propertiesCondition: Record, property: string, data: Record ): boolean => { if (has(propertiesCondition[property], 'not')) { - return !checkPropertyCondition( + return !checkDataCondition( get(propertiesCondition[property], 'not'), property, data @@ -115,29 +142,7 @@ const checkPropertyCondition = ( ); } - if (has(propertiesCondition[property], 'const')) { - return ( - has(data, property) && - data[property] === get(propertiesCondition[property], 'const') - ); - } else if (has(propertiesCondition[property], 'enum')) { - return ( - has(data, property) && - (get(propertiesCondition[property], 'enum') as unknown[]).includes( - data[property] - ) - ); - } else if (has(propertiesCondition[property], 'pattern')) { - const pattern = new RegExp(get(propertiesCondition[property], 'pattern')); - - return ( - has(data, property) && - typeof data[property] === 'string' && - pattern.test(data[property] as string) - ); - } - - return false; + return checkDataCondition(propertiesCondition[property], property, data); }; const evaluateCondition = ( @@ -224,11 +229,18 @@ const checkRequiredInIf = ( (requiredInElse && !condition && (get(get(schema, 'else'), 'required') as string[]).includes(segment)) || - (ifInThen && checkRequiredInIf(get(schema, 'then'), segment, data)) || - (ifInElse && checkRequiredInIf(get(schema, 'else'), segment, data)) || + (ifInThen && + condition && + checkRequiredInIf(get(schema, 'then'), segment, data)) || + (ifInElse && + !condition && + checkRequiredInIf(get(schema, 'else'), segment, data)) || (allOfInThen && + condition && conditionallyRequired(get(schema, 'then'), segment, data)) || - (allOfInElse && conditionallyRequired(get(schema, 'else'), segment, data)) + (allOfInElse && + !condition && + conditionallyRequired(get(schema, 'else'), segment, data)) ); }; From 1e18a5e9bd11d14344de9de2e519469ce904b378 Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Tue, 12 Dec 2023 20:58:21 +0200 Subject: [PATCH 07/13] Add check for required in parent schema --- packages/core/src/util/renderer.ts | 131 ++++++++++++++++++++++++++--- 1 file changed, 117 insertions(+), 14 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index b2bbcb028..a4f6c2b25 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -202,6 +202,35 @@ const evaluateCondition = ( return requiredCondition && valueCondition; }; +/** + * Go through parent's properties untill the segment is found at the exact level it is defined and check if it is required + */ +const extractRequired = ( + schema: JsonSchema, + segment: string, + prevSegments: string[] +) => { + let i = 0; + let currentSchema = schema; + while ( + i < prevSegments.length && + has(currentSchema, 'properties') && + has(get(currentSchema, 'properties'), prevSegments[i]) + ) { + currentSchema = get(get(currentSchema, 'properties'), prevSegments[i]); + ++i; + } + + if (i < prevSegments.length) { + return false; + } + + return ( + has(currentSchema, 'required') && + (get(currentSchema, 'required') as string[]).includes(segment) + ); +}; + /** * Check if property's required attribute is set based on if-then-else condition * @@ -209,38 +238,42 @@ const evaluateCondition = ( const checkRequiredInIf = ( schema: JsonSchema, segment: string, + prevSegments: string[], data: Record ): boolean => { const propertiesConditionSchema = get(schema, 'if'); const condition = evaluateCondition(propertiesConditionSchema, data); - const requiredInThen = has(get(schema, 'then'), 'required'); - const requiredInElse = has(get(schema, 'else'), 'required'); const ifInThen = has(get(schema, 'then'), 'if'); const ifInElse = has(get(schema, 'else'), 'if'); const allOfInThen = has(get(schema, 'then'), 'allOf'); const allOfInElse = has(get(schema, 'else'), 'allOf'); return ( - (requiredInThen && + (has(schema, 'then') && condition && - (get(get(schema, 'then'), 'required') as string[]).includes(segment)) || - (requiredInElse && + extractRequired(get(schema, 'then'), segment, prevSegments)) || + (has(schema, 'else') && !condition && - (get(get(schema, 'else'), 'required') as string[]).includes(segment)) || + extractRequired(get(schema, 'else'), segment, prevSegments)) || (ifInThen && condition && - checkRequiredInIf(get(schema, 'then'), segment, data)) || + checkRequiredInIf(get(schema, 'then'), segment, prevSegments, data)) || (ifInElse && !condition && - checkRequiredInIf(get(schema, 'else'), segment, data)) || + checkRequiredInIf(get(schema, 'else'), segment, prevSegments, data)) || (allOfInThen && condition && - conditionallyRequired(get(schema, 'then'), segment, data)) || + conditionallyRequired( + get(schema, 'then'), + segment, + prevSegments, + data + )) || (allOfInElse && !condition && - conditionallyRequired(get(schema, 'else'), segment, data)) + conditionallyRequired(get(schema, 'else'), segment, prevSegments, data)) ); }; @@ -251,18 +284,77 @@ const checkRequiredInIf = ( const conditionallyRequired = ( schema: JsonSchema, segment: string, + prevSegments: string[], data: any ) => { const nestedAllOfSchema = get(schema, 'allOf'); return any((subschema: JsonSchema4 | JsonSchema7): boolean => { return ( - (has(subschema, 'if') && checkRequiredInIf(subschema, segment, data)) || - conditionallyRequired(subschema, segment, data) + (has(subschema, 'if') && + checkRequiredInIf(subschema, segment, prevSegments, data)) || + conditionallyRequired(subschema, segment, prevSegments, data) ); }, nestedAllOfSchema); }; +/** + * Check if property is being required in the parent schema + */ +const isRequiredInParent = ( + schema: JsonSchema, + rootSchema: JsonSchema, + path: string, + segment: string, + prevSegments: string[], + data: Record +): boolean => { + const pathSegments = path.split('/'); + const lastSegment = pathSegments[pathSegments.length - 3]; + const nextHigherSchemaSegments = pathSegments.slice( + 0, + pathSegments.length - 4 + ); + + if (!nextHigherSchemaSegments.length) { + return false; + } + + const nextHigherSchemaPath = nextHigherSchemaSegments.join('/'); + + const nextHigherSchema = Resolve.schema( + schema, + nextHigherSchemaPath, + rootSchema + ); + + const currentData = Resolve.data(data, toDataPath(nextHigherSchemaPath)); + + return ( + conditionallyRequired( + nextHigherSchema, + segment, + [lastSegment, ...prevSegments], + currentData + ) || + (has(nextHigherSchema, 'if') && + checkRequiredInIf( + nextHigherSchema, + segment, + [lastSegment, ...prevSegments], + currentData + )) || + isRequiredInParent( + schema, + rootSchema, + nextHigherSchemaPath, + segment, + [lastSegment, ...prevSegments], + currentData + ) + ); +}; + const isRequired = ( schema: JsonSchema, schemaPath: string, @@ -286,20 +378,31 @@ const isRequired = ( const requiredInIf = has(nextHigherSchema, 'if') && - checkRequiredInIf(nextHigherSchema, lastSegment, currentData); + checkRequiredInIf(nextHigherSchema, lastSegment, [], currentData); const requiredConditionally = conditionallyRequired( nextHigherSchema, lastSegment, + [], currentData ); + const requiredConditionallyInParent = isRequiredInParent( + rootSchema, + rootSchema, + schemaPath, + lastSegment, + [], + data + ); + return ( (nextHigherSchema !== undefined && nextHigherSchema.required !== undefined && nextHigherSchema.required.indexOf(lastSegment) !== -1) || requiredInIf || - requiredConditionally + requiredConditionally || + requiredConditionallyInParent ); }; From d11e2b6e7ce30048056bcf7fc3f2bb1fbd6a5495 Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Tue, 9 Jan 2024 16:06:39 +0200 Subject: [PATCH 08/13] Add tests for conditionally required logic --- packages/core/src/util/renderer.ts | 44 +- packages/core/test/util/renderer.test.ts | 578 ++++++++++++++++++++++- 2 files changed, 602 insertions(+), 20 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index a4f6c2b25..fd3cff620 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -82,8 +82,10 @@ import { } from '../i18n/arrayTranslations'; import { resolveSchema } from './resolvers'; import cloneDeep from 'lodash/cloneDeep'; -import { has } from 'lodash'; -import { all, any } from 'lodash/fp'; +import isEqual from 'lodash/isEqual'; +import has from 'lodash/has'; +import any from 'lodash/fp/any'; +import all from 'lodash/fp/all'; const checkDataCondition = ( propertyCondition: unknown, @@ -92,12 +94,15 @@ const checkDataCondition = ( ) => { if (has(propertyCondition, 'const')) { return ( - has(data, property) && data[property] === get(propertyCondition, 'const') + has(data, property) && + isEqual(data[property], get(propertyCondition, 'const')) ); } else if (has(propertyCondition, 'enum')) { return ( has(data, property) && - (get(propertyCondition, 'enum') as unknown[]).includes(data[property]) + (get(propertyCondition, 'enum') as unknown[]).find((value) => + isEqual(value, data[property]) + ) !== undefined ); } else if (has(propertyCondition, 'pattern')) { const pattern = new RegExp(get(propertyCondition, 'pattern')); @@ -169,11 +174,14 @@ const evaluateCondition = ( let satisfied = false; for (let i = 0; i < subschemas.length; i++) { - if (satisfied) { + const current = evaluateCondition(subschemas[i], data); + if (current && satisfied) { return false; } - satisfied = evaluateCondition(subschemas[i], data); + if (current && !satisfied) { + satisfied = true; + } } return satisfied; @@ -185,21 +193,25 @@ const evaluateCondition = ( } const requiredCondition = all( - (property) => data[property], + (property) => has(data, property), requiredProperties ); - const propertiesCondition = get(schema, 'properties') as Record< - string, - unknown - >; + if (has(schema, 'properties')) { + const propertiesCondition = get(schema, 'properties') as Record< + string, + unknown + >; - const valueCondition = all( - (property) => checkPropertyCondition(propertiesCondition, property, data), - Object.keys(propertiesCondition) - ); + const valueCondition = all( + (property) => checkPropertyCondition(propertiesCondition, property, data), + Object.keys(propertiesCondition) + ); + + return requiredCondition && valueCondition; + } - return requiredCondition && valueCondition; + return requiredCondition; }; /** diff --git a/packages/core/test/util/renderer.test.ts b/packages/core/test/util/renderer.test.ts index afbebaffd..5b834addd 100644 --- a/packages/core/test/util/renderer.test.ts +++ b/packages/core/test/util/renderer.test.ts @@ -1,19 +1,19 @@ /* The MIT License - + Copyright (c) 2017-2019 EclipseSource Munich https://github.com/eclipsesource/jsonforms - + Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: - + The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. - + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE @@ -72,6 +72,8 @@ import { defaultJsonFormsI18nState } from '../../src/reducers/i18n'; import { i18nJsonSchema } from '../../src/i18n/i18nTypes'; import { convertDateToString } from '../../src/util'; +type Combinator = 'allOf' | 'anyOf' | 'oneOf'; + const middlewares: Redux.Middleware[] = []; const mockStore = configureStore(middlewares); @@ -443,6 +445,574 @@ test('mapStateToControlProps - id', (t) => { t.is(props.id, '#/properties/firstName'); }); +test('mapStateToControlProps - required based on single condition', (t) => { + const schema = { + type: 'object', + properties: { + notifications: { type: 'boolean' }, + firstName: { type: 'string' }, + }, + required: ['notifications'], + if: { + properties: { + notifications: { const: true }, + }, + }, + then: { + required: ['firstName'], + }, + }; + + const ownProps: OwnPropsOfControl = { + uischema: coreUISchema, + schema, + }; + + const createState = (data: { + notifications: boolean; + firstName: string; + }) => ({ + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + }, + }); + + const stateTrue = createState({ notifications: true, firstName: 'Bart' }); + const stateFalse = createState({ notifications: false, firstName: 'Bart' }); + + const propsTrue = mapStateToControlProps(stateTrue, ownProps); + const propsFalse = mapStateToControlProps(stateFalse, ownProps); + t.true(propsTrue.required); + t.false(propsFalse.required); +}); + +test('mapStateToControlProps - required based on condition with combinators', (t) => { + const createSchema = (combinator: Combinator) => ({ + type: 'object', + properties: { + notifications: { type: 'boolean' }, + email: { type: 'string' }, + firstName: { type: 'string' }, + }, + required: ['notifications'], + if: { + [combinator]: [ + { + properties: { + notifications: { const: true }, + }, + }, + { + required: ['email'], + }, + ], + }, + then: { + required: ['firstName'], + }, + }); + + const createState = (data: { + combinator: Combinator; + notifications: boolean; + email?: string; + firstName: string; + }) => { + const { email, ...rest } = data; + + return { + jsonforms: { + core: { + schema: createSchema(data.combinator), + uischema: coreUISchema, + data: email ? data : rest, + errors: [] as ErrorObject[], + }, + }, + }; + }; + + const stateAnyOfTrue = createState({ + combinator: 'anyOf', + notifications: false, + email: 'bart@email.com', + firstName: 'Bart', + }); + + const stateAllOfTrue = createState({ + combinator: 'allOf', + notifications: true, + email: 'bart@email.com', + firstName: 'Bart', + }); + + const stateOneOfTrue = createState({ + combinator: 'oneOf', + notifications: true, + firstName: 'Bart', + }); + + const stateAnyOfFalse = createState({ + combinator: 'anyOf', + notifications: false, + firstName: 'Bart', + }); + + const stateAllOfFalse = createState({ + combinator: 'allOf', + notifications: true, + firstName: 'Bart', + }); + + const stateOneOfFalse = createState({ + combinator: 'oneOf', + notifications: true, + email: 'bart@email.com', + firstName: 'Bart', + }); + + const ownPropsAnyOf: OwnPropsOfControl = { + uischema: coreUISchema, + schema: createSchema('anyOf'), + }; + + const ownPropsAllOf: OwnPropsOfControl = { + uischema: coreUISchema, + schema: createSchema('allOf'), + }; + + const ownPropsOneOf: OwnPropsOfControl = { + uischema: coreUISchema, + schema: createSchema('oneOf'), + }; + + const propsAnyOfTrue = mapStateToControlProps(stateAnyOfTrue, ownPropsAnyOf); + const propsAllOfTrue = mapStateToControlProps(stateAllOfTrue, ownPropsAllOf); + const propsOneOfTrue = mapStateToControlProps(stateOneOfTrue, ownPropsOneOf); + const propsAnyOfFalse = mapStateToControlProps( + stateAnyOfFalse, + ownPropsAnyOf + ); + const propsAllOfFalse = mapStateToControlProps( + stateAllOfFalse, + ownPropsAllOf + ); + const propsOneOfFalse = mapStateToControlProps( + stateOneOfFalse, + ownPropsOneOf + ); + + t.true(propsAnyOfTrue.required); + t.true(propsAllOfTrue.required); + t.true(propsOneOfTrue.required); + t.false(propsAnyOfFalse.required); + t.false(propsAllOfFalse.required); + t.false(propsOneOfFalse.required); +}); + +test('mapStateToControlProps - required based on multiple conditions', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + notifications: { type: 'boolean' }, + role: { type: 'string' }, + firstName: { type: 'string' }, + }, + required: ['notifications'], + allOf: [ + { + if: { + properties: { + notifications: { const: true }, + }, + }, + then: { + required: ['firstName'], + }, + }, + { + if: { + properties: { + role: { const: 'manager' }, + }, + }, + then: { + required: ['firstName'], + }, + }, + ], + }; + + const ownProps: OwnPropsOfControl = { + uischema: coreUISchema, + schema, + }; + + const createState = (data: { + notifications: boolean; + role: string; + firstName: string; + }) => ({ + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + }, + }); + + const stateFirstMatches = createState({ + notifications: true, + role: 'employee', + firstName: 'Bart', + }); + + const stateSecondMatches = createState({ + notifications: false, + role: 'manager', + firstName: 'Bart', + }); + + const propsFirstMatches = mapStateToControlProps(stateFirstMatches, ownProps); + const propsSecondMatcher = mapStateToControlProps( + stateSecondMatches, + ownProps + ); + t.true(propsFirstMatches.required); + t.true(propsSecondMatcher.required); +}); + +test('mapStateToControlProps - required based on nested conditions', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + notifications: { type: 'boolean' }, + role: { type: 'string' }, + firstName: { type: 'string' }, + }, + required: ['notifications'], + if: { + properties: { + notifications: { const: true }, + }, + }, + then: { + if: { + properties: { + role: { + pattern: '[a-zA-Z]+', + }, + }, + }, + then: { + required: ['firstName'], + }, + }, + }; + + const ownProps: OwnPropsOfControl = { + uischema: coreUISchema, + schema, + }; + + const createState = (data: { + notifications: boolean; + role: string; + firstName: string; + }) => ({ + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + }, + }); + + const stateAllTrue = createState({ + notifications: true, + role: 'manager', + firstName: 'Bart', + }); + const stateInnerFalse = createState({ + notifications: true, + role: '398', + firstName: 'Bart', + }); + const stateOuterFalse = createState({ + notifications: false, + role: 'manager', + firstName: 'Bart', + }); + + const propsAllTrue = mapStateToControlProps(stateAllTrue, ownProps); + const propsInnerFalse = mapStateToControlProps(stateInnerFalse, ownProps); + const propsOuterFalse = mapStateToControlProps(stateOuterFalse, ownProps); + + t.true(propsAllTrue.required); + t.false(propsInnerFalse.required); + t.false(propsOuterFalse.required); +}); + +test('mapStateToControlProps - required based on multiple conditions in nested "allOf"-s', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + notifications: { type: 'boolean' }, + role: { type: 'string' }, + qualifies: { type: 'boolean' }, + firstName: { type: 'string' }, + }, + required: ['notifications'], + allOf: [ + { + allOf: [ + { + if: { + properties: { + notifications: { const: true }, + }, + }, + then: { + required: ['firstName'], + }, + }, + { + if: { + properties: { + role: { enum: ['manager', 'lead'] }, + }, + }, + then: { + required: ['firstName'], + }, + }, + ], + }, + { + if: { + properties: { + qualified: { const: true }, + }, + }, + then: { + required: ['firstName'], + }, + }, + ], + }; + + const ownProps: OwnPropsOfControl = { + uischema: coreUISchema, + schema, + }; + + const createState = (data: { + notifications: boolean; + qualified: boolean; + role: string; + firstName: string; + }) => ({ + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + }, + }); + + const stateFirstMatches = createState({ + notifications: true, + qualified: false, + role: 'employee', + firstName: 'Bart', + }); + + const stateSecondMatches = createState({ + notifications: false, + qualified: false, + role: 'manager', + firstName: 'Bart', + }); + + const stateThirdMatches = createState({ + notifications: false, + qualified: true, + role: 'employee', + firstName: 'Bart', + }); + + const propsFirstMatches = mapStateToControlProps(stateFirstMatches, ownProps); + const propsSecondMatcher = mapStateToControlProps( + stateSecondMatches, + ownProps + ); + const propsThirdMatcher = mapStateToControlProps(stateThirdMatches, ownProps); + + t.true(propsFirstMatches.required); + t.true(propsSecondMatcher.required); + t.true(propsThirdMatcher.required); +}); + +test('mapStateToControlProps - required based on condition for nested object', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + role: { + type: 'object', + properties: { + position: { type: 'string' }, + experience: { type: 'number' }, + }, + }, + firstName: { type: 'string' }, + }, + required: ['notifications'], + if: { + properties: { + role: { + properties: { + position: { enum: ['manager', 'lead'] }, + }, + }, + }, + }, + then: { + required: ['firstName'], + }, + }; + + const ownProps: OwnPropsOfControl = { + uischema: coreUISchema, + schema, + }; + + const createState = (data: { + role: { + position: string; + experience: number; + }; + firstName: string; + }) => ({ + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + }, + }); + + const stateTrue = createState({ + role: { + position: 'lead', + experience: 5, + }, + firstName: 'Bart', + }); + + const stateFalse = createState({ + role: { + position: 'employee', + experience: 1, + }, + firstName: 'Bart', + }); + + const propsTrue = mapStateToControlProps(stateTrue, ownProps); + const propsFalse = mapStateToControlProps(stateFalse, ownProps); + + t.true(propsTrue.required); + t.false(propsFalse.required); +}); + +test('mapStateToControlProps - required based on parent condition', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + notifications: { type: 'boolean' }, + role: { + type: 'object', + properties: { + position: { type: 'string' }, + firstName: { type: 'string' }, + }, + }, + }, + required: ['notifications'], + if: { + properties: { + notifications: { const: true }, + }, + }, + then: { + properties: { + role: { + required: ['firstName'], + }, + }, + }, + }; + + const ownProps: OwnPropsOfControl = { + uischema: { + type: 'Control', + scope: '#/properties/role/properties/firstName', + }, + schema, + }; + + const createState = (data: { + notifications: boolean; + role: { + position: string; + firstName: string; + }; + }) => ({ + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + }, + }); + + const stateTrue = createState({ + notifications: true, + role: { + position: 'lead', + firstName: 'Bart', + }, + }); + + const stateFalse = createState({ + notifications: false, + role: { + position: 'lead', + firstName: 'Bart', + }, + }); + + const propsTrue = mapStateToControlProps(stateTrue, ownProps); + const propsFalse = mapStateToControlProps(stateFalse, ownProps); + + t.true(propsTrue.required); + t.false(propsFalse.required); +}); + test('mapStateToControlProps - hide errors in hide validation mode', (t) => { const schema = { type: 'object', From 0d3f614bb8a92ddd4eb1d9c159ddd4dfb1ff03a7 Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Wed, 10 Jan 2024 12:26:20 +0200 Subject: [PATCH 09/13] Add more tests and make conditionally required check optional --- packages/core/src/configDefault.ts | 14 +- packages/core/src/util/renderer.ts | 21 +- packages/core/test/util/renderer.test.ts | 262 ++++++++++++++++++----- 3 files changed, 235 insertions(+), 62 deletions(-) diff --git a/packages/core/src/configDefault.ts b/packages/core/src/configDefault.ts index 3761c314e..f75b95611 100644 --- a/packages/core/src/configDefault.ts +++ b/packages/core/src/configDefault.ts @@ -1,19 +1,19 @@ /* The MIT License - + Copyright (c) 2017-2019 EclipseSource Munich https://github.com/eclipsesource/jsonforms - + Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: - + The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. - + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE @@ -45,4 +45,10 @@ export const configDefault = { * [text] if asterisks in labels for required fields should be hidden */ hideRequiredAsterisk: false, + + /** + * [text] if dynamic checks for conditional application of properties + * should be performed (e.g. check for conditional required) + */ + allowDynamicCheck: false, }; diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index fd3cff620..222ba6450 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -371,7 +371,8 @@ const isRequired = ( schema: JsonSchema, schemaPath: string, rootSchema: JsonSchema, - data: any + data: any, + config: any ): boolean => { const pathSegments = schemaPath.split('/'); const lastSegment = pathSegments[pathSegments.length - 1]; @@ -388,6 +389,14 @@ const isRequired = ( ); const currentData = Resolve.data(data, toDataPath(nextHigherSchemaPath)); + if (!config?.allowDynamicCheck) { + return ( + nextHigherSchema !== undefined && + nextHigherSchema.required !== undefined && + nextHigherSchema.required.indexOf(lastSegment) !== -1 + ); + } + const requiredInIf = has(nextHigherSchema, 'if') && checkRequiredInIf(nextHigherSchema, lastSegment, [], currentData); @@ -809,9 +818,16 @@ export const mapStateToControlProps = ( const controlElement = uischema as ControlElement; const id = ownProps.id; const rootSchema = getSchema(state); + const config = getConfig(state); const required = controlElement.scope !== undefined && - isRequired(ownProps.schema, controlElement.scope, rootSchema, rootData); + isRequired( + ownProps.schema, + controlElement.scope, + rootSchema, + rootData, + config + ); const resolvedSchema = Resolve.schema( ownProps.schema || rootSchema, controlElement.scope, @@ -824,7 +840,6 @@ export const mapStateToControlProps = ( const data = Resolve.data(rootData, path); const labelDesc = createLabelDescriptionFrom(uischema, resolvedSchema); const label = labelDesc.show ? labelDesc.text : ''; - const config = getConfig(state); const enabled: boolean = isInherentlyEnabled( state, ownProps, diff --git a/packages/core/test/util/renderer.test.ts b/packages/core/test/util/renderer.test.ts index 5b834addd..c27af9529 100644 --- a/packages/core/test/util/renderer.test.ts +++ b/packages/core/test/util/renderer.test.ts @@ -71,6 +71,7 @@ import { JsonSchema7 } from '../../src/models/jsonSchema7'; import { defaultJsonFormsI18nState } from '../../src/reducers/i18n'; import { i18nJsonSchema } from '../../src/i18n/i18nTypes'; import { convertDateToString } from '../../src/util'; +import { configReducer, setConfig } from '../../src'; type Combinator = 'allOf' | 'anyOf' | 'oneOf'; @@ -445,7 +446,7 @@ test('mapStateToControlProps - id', (t) => { t.is(props.id, '#/properties/firstName'); }); -test('mapStateToControlProps - required based on single condition', (t) => { +test('mapStateToControlProps - required not checked dynamically if not explicitly specified in config', (t) => { const schema = { type: 'object', properties: { @@ -487,11 +488,122 @@ test('mapStateToControlProps - required based on single condition', (t) => { const propsTrue = mapStateToControlProps(stateTrue, ownProps); const propsFalse = mapStateToControlProps(stateFalse, ownProps); + t.false(propsTrue.required); + t.false(propsFalse.required); +}); + +test('mapStateToControlProps - required with dynamic check based on single condition', (t) => { + const schema = { + type: 'object', + properties: { + notifications: { type: 'boolean' }, + firstName: { type: 'string' }, + }, + required: ['notifications'], + if: { + properties: { + notifications: { const: true }, + }, + }, + then: { + required: ['firstName'], + }, + }; + + const ownProps: OwnPropsOfControl = { + uischema: coreUISchema, + schema, + }; + + const config = setConfig({ + allowDynamicCheck: true, + }); + + const createState = (data: { + notifications: boolean; + firstName: string; + }) => ({ + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + config: configReducer(undefined, config), + }, + }); + + const stateTrue = createState({ notifications: true, firstName: 'Bart' }); + const stateFalse = createState({ notifications: false, firstName: 'Bart' }); + + const propsTrue = mapStateToControlProps(stateTrue, ownProps); + const propsFalse = mapStateToControlProps(stateFalse, ownProps); + t.true(propsTrue.required); + t.false(propsFalse.required); +}); + +test('mapStateToControlProps - required with dynamic check based on condition with missing data', (t) => { + const schema = { + type: 'object', + properties: { + role: { type: 'string' }, + firstName: { type: 'string' }, + }, + required: ['role'], + if: { + properties: { + role: { + not: { + const: 'manager', + }, + }, + }, + }, + then: { + required: ['firstName'], + }, + }; + + const ownProps: OwnPropsOfControl = { + uischema: coreUISchema, + schema, + }; + + const createState = (data: { role?: string; firstName: string }) => { + const { role, ...rest } = data; + + const config = setConfig({ + allowDynamicCheck: true, + }); + + return { + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data: role ? data : rest, + errors: [] as ErrorObject[], + }, + config: configReducer(undefined, config), + }, + }; + }; + + const stateMissing = createState({ firstName: 'Bart' }); + const stateTrue = createState({ role: 'lead', firstName: 'Bart' }); + const stateFalse = createState({ role: 'manager', firstName: 'Bart' }); + + const propsMissing = mapStateToControlProps(stateMissing, ownProps); + const propsTrue = mapStateToControlProps(stateTrue, ownProps); + const propsFalse = mapStateToControlProps(stateFalse, ownProps); + + t.true(propsMissing.required); t.true(propsTrue.required); t.false(propsFalse.required); }); -test('mapStateToControlProps - required based on condition with combinators', (t) => { +test('mapStateToControlProps - required with dynamic check based on condition with combinators', (t) => { const createSchema = (combinator: Combinator) => ({ type: 'object', properties: { @@ -524,6 +636,9 @@ test('mapStateToControlProps - required based on condition with combinators', (t firstName: string; }) => { const { email, ...rest } = data; + const config = setConfig({ + allowDynamicCheck: true, + }); return { jsonforms: { @@ -533,6 +648,7 @@ test('mapStateToControlProps - required based on condition with combinators', (t data: email ? data : rest, errors: [] as ErrorObject[], }, + config: configReducer(undefined, config), }, }; }; @@ -615,7 +731,7 @@ test('mapStateToControlProps - required based on condition with combinators', (t t.false(propsOneOfFalse.required); }); -test('mapStateToControlProps - required based on multiple conditions', (t) => { +test('mapStateToControlProps - required with dynamic check based on multiple conditions', (t) => { const schema: JsonSchema = { type: 'object', properties: { @@ -654,19 +770,26 @@ test('mapStateToControlProps - required based on multiple conditions', (t) => { }; const createState = (data: { - notifications: boolean; + notifications?: boolean; role: string; firstName: string; - }) => ({ - jsonforms: { - core: { - schema, - uischema: coreUISchema, - data, - errors: [] as ErrorObject[], + }) => { + const config = setConfig({ + allowDynamicCheck: true, + }); + + return { + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + config: configReducer(undefined, config), }, - }, - }); + }; + }; const stateFirstMatches = createState({ notifications: true, @@ -689,7 +812,7 @@ test('mapStateToControlProps - required based on multiple conditions', (t) => { t.true(propsSecondMatcher.required); }); -test('mapStateToControlProps - required based on nested conditions', (t) => { +test('mapStateToControlProps - required with dynamic check based on nested conditions', (t) => { const schema: JsonSchema = { type: 'object', properties: { @@ -723,19 +846,28 @@ test('mapStateToControlProps - required based on nested conditions', (t) => { }; const createState = (data: { - notifications: boolean; + notifications?: boolean; role: string; firstName: string; - }) => ({ - jsonforms: { - core: { - schema, - uischema: coreUISchema, - data, - errors: [] as ErrorObject[], + }) => { + const { notifications, ...rest } = data; + + const config = setConfig({ + allowDynamicCheck: true, + }); + + return { + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data: notifications !== undefined ? data : rest, + errors: [] as ErrorObject[], + }, + config: configReducer(undefined, config), }, - }, - }); + }; + }; const stateAllTrue = createState({ notifications: true, @@ -748,7 +880,6 @@ test('mapStateToControlProps - required based on nested conditions', (t) => { firstName: 'Bart', }); const stateOuterFalse = createState({ - notifications: false, role: 'manager', firstName: 'Bart', }); @@ -762,7 +893,7 @@ test('mapStateToControlProps - required based on nested conditions', (t) => { t.false(propsOuterFalse.required); }); -test('mapStateToControlProps - required based on multiple conditions in nested "allOf"-s', (t) => { +test('mapStateToControlProps - required with dynamic check based on multiple conditions in nested "allOf"-s', (t) => { const schema: JsonSchema = { type: 'object', properties: { @@ -820,16 +951,23 @@ test('mapStateToControlProps - required based on multiple conditions in nested " qualified: boolean; role: string; firstName: string; - }) => ({ - jsonforms: { - core: { - schema, - uischema: coreUISchema, - data, - errors: [] as ErrorObject[], + }) => { + const config = setConfig({ + allowDynamicCheck: true, + }); + + return { + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + config: configReducer(undefined, config), }, - }, - }); + }; + }; const stateFirstMatches = createState({ notifications: true, @@ -864,7 +1002,7 @@ test('mapStateToControlProps - required based on multiple conditions in nested " t.true(propsThirdMatcher.required); }); -test('mapStateToControlProps - required based on condition for nested object', (t) => { +test('mapStateToControlProps - required with dynamic check based on condition for nested object', (t) => { const schema: JsonSchema = { type: 'object', properties: { @@ -903,16 +1041,23 @@ test('mapStateToControlProps - required based on condition for nested object', ( experience: number; }; firstName: string; - }) => ({ - jsonforms: { - core: { - schema, - uischema: coreUISchema, - data, - errors: [] as ErrorObject[], + }) => { + const config = setConfig({ + allowDynamicCheck: true, + }); + + return { + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + config: configReducer(undefined, config), }, - }, - }); + }; + }; const stateTrue = createState({ role: { @@ -937,7 +1082,7 @@ test('mapStateToControlProps - required based on condition for nested object', ( t.false(propsFalse.required); }); -test('mapStateToControlProps - required based on parent condition', (t) => { +test('mapStateToControlProps - required with dynamic check based on parent condition', (t) => { const schema: JsonSchema = { type: 'object', properties: { @@ -979,16 +1124,23 @@ test('mapStateToControlProps - required based on parent condition', (t) => { position: string; firstName: string; }; - }) => ({ - jsonforms: { - core: { - schema, - uischema: coreUISchema, - data, - errors: [] as ErrorObject[], + }) => { + const config = setConfig({ + allowDynamicCheck: true, + }); + + return { + jsonforms: { + core: { + schema, + uischema: coreUISchema, + data, + errors: [] as ErrorObject[], + }, + config: configReducer(undefined, config), }, - }, - }); + }; + }; const stateTrue = createState({ notifications: true, From a94ef354eabba2a0cef2463dc86c2b6579da678f Mon Sep 17 00:00:00 2001 From: Kaloyan Manolov Date: Thu, 15 Feb 2024 16:13:01 +0200 Subject: [PATCH 10/13] Modify conditionallyRequired logic to handle arrays --- packages/core/src/util/renderer.ts | 170 ++++++++++++++++++++--------- 1 file changed, 117 insertions(+), 53 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index 222ba6450..ca296ff2d 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -60,7 +60,7 @@ import type { CombinatorKeyword } from './combinators'; import { moveDown, moveUp } from './array'; import type { AnyAction, Dispatch } from './type'; import { Resolve, convertDateToString, hasType } from './util'; -import { composePaths, composeWithUi, toDataPath } from './path'; +import { composePaths, composeWithUi } from './path'; import { CoreActions, update } from '../actions'; import type { ErrorObject } from 'ajv'; import type { JsonFormsState } from '../store'; @@ -87,6 +87,27 @@ import has from 'lodash/has'; import any from 'lodash/fp/any'; import all from 'lodash/fp/all'; +const dataPathToJsonPointer = (dataPath: string): string => { + // Split the path into parts + const parts = dataPath.split('.'); + + // Initialize the JSON Pointer with a hash symbol + let jsonPointer = '#'; + + // Iterate over the parts to build the JSON Pointer + parts.forEach((part) => { + if (part.match(/^\d+$/)) { + // Check if the part is a numeric index, indicating an array + jsonPointer += '/items'; + } else { + // For non-numeric parts, prefix with "/properties" + jsonPointer += `/properties/${part}`; + } + }); + + return jsonPointer; +}; + const checkDataCondition = ( propertyCondition: unknown, property: string, @@ -215,7 +236,7 @@ const evaluateCondition = ( }; /** - * Go through parent's properties untill the segment is found at the exact level it is defined and check if it is required + * Go through parent's properties until the segment is found at the exact level it is defined and check if it is required */ const extractRequired = ( schema: JsonSchema, @@ -226,10 +247,15 @@ const extractRequired = ( let currentSchema = schema; while ( i < prevSegments.length && - has(currentSchema, 'properties') && - has(get(currentSchema, 'properties'), prevSegments[i]) + (has(currentSchema, prevSegments[i]) || + (has(currentSchema, 'properties') && + has(get(currentSchema, 'properties'), prevSegments[i]))) ) { - currentSchema = get(get(currentSchema, 'properties'), prevSegments[i]); + // TODO: Verify this always works in trees with `items` + if (has(currentSchema, 'properties')) { + currentSchema = get(currentSchema, 'properties'); + } + currentSchema = get(currentSchema, prevSegments[i]); ++i; } @@ -310,37 +336,49 @@ const conditionallyRequired = ( }, nestedAllOfSchema); }; +const getNextHigherSchemaPath = (schemaPath: string): string => { + const pathSegments = schemaPath.split('/'); + const lastSegment = pathSegments[pathSegments.length - 1]; + + // We'd normally jump two segments back, but if we're in an `items` key, we want to check its parent + // Example path: '#/properties/anotherObject/properties/myArray/items/properties/propertyName' + const nextHigherSegmentIndexDifference = lastSegment === 'items' ? 1 : 2; + const nextHigherSchemaSegments = pathSegments.slice( + 0, + pathSegments.length - nextHigherSegmentIndexDifference + ); + + return nextHigherSchemaSegments.join('/'); +}; + +const getNextHigherDataPath = (dataPath: string): string => { + const dataPathSegments = dataPath.split('.'); + return dataPathSegments.slice(0, dataPathSegments.length - 1).join('.'); +}; + /** * Check if property is being required in the parent schema */ const isRequiredInParent = ( schema: JsonSchema, - rootSchema: JsonSchema, - path: string, + schemaPath: string, segment: string, prevSegments: string[], - data: Record + data: Record, + dataPath: string ): boolean => { - const pathSegments = path.split('/'); - const lastSegment = pathSegments[pathSegments.length - 3]; - const nextHigherSchemaSegments = pathSegments.slice( - 0, - pathSegments.length - 4 - ); + const pathSegments = schemaPath.split('/'); + const lastSegment = pathSegments[pathSegments.length - 1]; + const nextHigherSchemaPath = getNextHigherSchemaPath(schemaPath); - if (!nextHigherSchemaSegments.length) { + if (!nextHigherSchemaPath) { return false; } - const nextHigherSchemaPath = nextHigherSchemaSegments.join('/'); + const nextHigherSchema = Resolve.schema(schema, nextHigherSchemaPath, schema); - const nextHigherSchema = Resolve.schema( - schema, - nextHigherSchemaPath, - rootSchema - ); - - const currentData = Resolve.data(data, toDataPath(nextHigherSchemaPath)); + const nextHigherDataPath = getNextHigherDataPath(dataPath); + const currentData = Resolve.data(data, nextHigherDataPath); return ( conditionallyRequired( @@ -356,26 +394,38 @@ const isRequiredInParent = ( [lastSegment, ...prevSegments], currentData )) || + // TODO: Were we previously skipping each other parent + // when calling `isRequiredInParent` recursively (because of + // the `slice` call with (pathSegments.length - 4) - hence going + // back two times on each call) isRequiredInParent( schema, - rootSchema, nextHigherSchemaPath, segment, [lastSegment, ...prevSegments], - currentData + // TODO: Why was this currentData? + data, + nextHigherDataPath ) ); }; +const isRequiredInSchema = (schema: JsonSchema, segment: string): boolean => { + return ( + schema !== undefined && + schema.required !== undefined && + schema.required.indexOf(segment) !== -1 + ); +}; + const isRequired = ( schema: JsonSchema, schemaPath: string, - rootSchema: JsonSchema, - data: any, - config: any + rootSchema: JsonSchema ): boolean => { const pathSegments = schemaPath.split('/'); const lastSegment = pathSegments[pathSegments.length - 1]; + // TODO: This does not work correctly with "items" in the schemaPath // Skip "properties", "items" etc. to resolve the parent const nextHigherSchemaSegments = pathSegments.slice( 0, @@ -387,15 +437,32 @@ const isRequired = ( nextHigherSchemaPath, rootSchema ); - const currentData = Resolve.data(data, toDataPath(nextHigherSchemaPath)); + return isRequiredInSchema(nextHigherSchema, lastSegment); +}; - if (!config?.allowDynamicCheck) { - return ( - nextHigherSchema !== undefined && - nextHigherSchema.required !== undefined && - nextHigherSchema.required.indexOf(lastSegment) !== -1 - ); - } +// TODO: This may now be combinable with `isRequiredInParent` in a +// single function +const isConditionallyRequired = ( + rootSchema: JsonSchema, + schemaPath: string, + data: any, + dataPath: string +): boolean => { + const pathSegments = schemaPath.split('/'); + const lastSegment = pathSegments[pathSegments.length - 1]; + + const nextHigherSchemaPath = getNextHigherSchemaPath(schemaPath); + const nextHigherSchema = Resolve.schema( + rootSchema, + nextHigherSchemaPath, + rootSchema + ); + + // We need the `dataPath` to be able to resolve data in arrays, + // for example `myObject.myArray.0.myProperty` has no + // equivalent for the index in the schema syntax + const nextHigherDataPath = getNextHigherDataPath(dataPath); + const currentData = Resolve.data(data, nextHigherDataPath); const requiredInIf = has(nextHigherSchema, 'if') && @@ -410,21 +477,15 @@ const isRequired = ( const requiredConditionallyInParent = isRequiredInParent( rootSchema, - rootSchema, - schemaPath, + // TODO: Why was this previously schemaPath + nextHigherSchemaPath, lastSegment, [], - data + data, + nextHigherDataPath ); - return ( - (nextHigherSchema !== undefined && - nextHigherSchema.required !== undefined && - nextHigherSchema.required.indexOf(lastSegment) !== -1) || - requiredInIf || - requiredConditionally || - requiredConditionallyInParent - ); + return requiredInIf || requiredConditionally || requiredConditionallyInParent; }; /** @@ -821,12 +882,15 @@ export const mapStateToControlProps = ( const config = getConfig(state); const required = controlElement.scope !== undefined && - isRequired( - ownProps.schema, - controlElement.scope, - rootSchema, - rootData, - config + !!( + isRequired(ownProps.schema, controlElement.scope, rootSchema) || + (config?.allowDynamicCheck && + isConditionallyRequired( + rootSchema, + dataPathToJsonPointer(path), + rootData, + path + )) ); const resolvedSchema = Resolve.schema( ownProps.schema || rootSchema, From 0cc6dda9c15230dce64757d59d430bb057a4b7d3 Mon Sep 17 00:00:00 2001 From: Kaloyan Manolov Date: Thu, 15 Feb 2024 16:30:50 +0200 Subject: [PATCH 11/13] Remove redundant comments --- packages/core/src/util/renderer.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index ca296ff2d..f9970efdd 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -88,19 +88,13 @@ import any from 'lodash/fp/any'; import all from 'lodash/fp/all'; const dataPathToJsonPointer = (dataPath: string): string => { - // Split the path into parts const parts = dataPath.split('.'); - - // Initialize the JSON Pointer with a hash symbol let jsonPointer = '#'; - // Iterate over the parts to build the JSON Pointer parts.forEach((part) => { if (part.match(/^\d+$/)) { - // Check if the part is a numeric index, indicating an array jsonPointer += '/items'; } else { - // For non-numeric parts, prefix with "/properties" jsonPointer += `/properties/${part}`; } }); From ad4614a368d566bceed2f687ff141a08236d90cf Mon Sep 17 00:00:00 2001 From: Kaloyan Manolov Date: Mon, 19 Feb 2024 13:39:40 +0200 Subject: [PATCH 12/13] Add additional check to prevent accessing undefined data --- packages/core/src/util/renderer.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index f9970efdd..d4253e026 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -145,7 +145,7 @@ const checkPropertyCondition = ( ); } - if (has(propertiesCondition[property], 'properties')) { + if (has(propertiesCondition[property], 'properties') && has(data, property)) { const nextPropertyConditions = get( propertiesCondition[property], 'properties' @@ -245,7 +245,6 @@ const extractRequired = ( (has(currentSchema, 'properties') && has(get(currentSchema, 'properties'), prevSegments[i]))) ) { - // TODO: Verify this always works in trees with `items` if (has(currentSchema, 'properties')) { currentSchema = get(currentSchema, 'properties'); } From a6f0d7b943a34656c05db33fceda2b27f56d2731 Mon Sep 17 00:00:00 2001 From: Georgi Atanasov Date: Tue, 27 Aug 2024 11:09:38 +0300 Subject: [PATCH 13/13] Remove comments from renderer.ts --- packages/core/src/util/renderer.ts | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts index d4253e026..8a6c90020 100644 --- a/packages/core/src/util/renderer.ts +++ b/packages/core/src/util/renderer.ts @@ -237,22 +237,22 @@ const extractRequired = ( segment: string, prevSegments: string[] ) => { - let i = 0; + let segmentIndex = 0; let currentSchema = schema; while ( - i < prevSegments.length && - (has(currentSchema, prevSegments[i]) || + segmentIndex < prevSegments.length && + (has(currentSchema, prevSegments[segmentIndex]) || (has(currentSchema, 'properties') && - has(get(currentSchema, 'properties'), prevSegments[i]))) + has(get(currentSchema, 'properties'), prevSegments[segmentIndex]))) ) { if (has(currentSchema, 'properties')) { currentSchema = get(currentSchema, 'properties'); } - currentSchema = get(currentSchema, prevSegments[i]); - ++i; + currentSchema = get(currentSchema, prevSegments[segmentIndex]); + ++segmentIndex; } - if (i < prevSegments.length) { + if (segmentIndex < prevSegments.length) { return false; } @@ -264,7 +264,6 @@ const extractRequired = ( /** * Check if property's required attribute is set based on if-then-else condition - * */ const checkRequiredInIf = ( schema: JsonSchema, @@ -387,16 +386,11 @@ const isRequiredInParent = ( [lastSegment, ...prevSegments], currentData )) || - // TODO: Were we previously skipping each other parent - // when calling `isRequiredInParent` recursively (because of - // the `slice` call with (pathSegments.length - 4) - hence going - // back two times on each call) isRequiredInParent( schema, nextHigherSchemaPath, segment, [lastSegment, ...prevSegments], - // TODO: Why was this currentData? data, nextHigherDataPath ) @@ -418,7 +412,6 @@ const isRequired = ( ): boolean => { const pathSegments = schemaPath.split('/'); const lastSegment = pathSegments[pathSegments.length - 1]; - // TODO: This does not work correctly with "items" in the schemaPath // Skip "properties", "items" etc. to resolve the parent const nextHigherSchemaSegments = pathSegments.slice( 0, @@ -433,8 +426,6 @@ const isRequired = ( return isRequiredInSchema(nextHigherSchema, lastSegment); }; -// TODO: This may now be combinable with `isRequiredInParent` in a -// single function const isConditionallyRequired = ( rootSchema: JsonSchema, schemaPath: string, @@ -470,7 +461,6 @@ const isConditionallyRequired = ( const requiredConditionallyInParent = isRequiredInParent( rootSchema, - // TODO: Why was this previously schemaPath nextHigherSchemaPath, lastSegment, [],