From 7ad74fba7309282468b366610fcea56a69e6e6fc Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Mon, 6 Feb 2023 13:41:18 +0100 Subject: [PATCH 1/5] Added a missing validator check for items in an array Signed-off-by: bnechyporenko --- .../TemplatePage/createValidator.test.ts | 59 ++++++++++++++++++- .../TemplatePage/createValidator.ts | 26 +++++++- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts b/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts index 177697eda0..da7a79b1b9 100644 --- a/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts +++ b/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts @@ -17,14 +17,32 @@ import { createValidator } from './createValidator'; import { CustomFieldValidator } from '@backstage/plugin-scaffolder-react'; import { ApiHolder } from '@backstage/core-plugin-api'; -import { FormValidation } from '@rjsf/core'; +import { FieldValidation, FormValidation } from '@rjsf/core'; describe('createValidator', () => { const validators: Record> = { - CustomPicker: (value, validation, _context) => { + CustomPicker: ( + value: unknown, + fieldValidation: FieldValidation, + _context: { apiHolder: ApiHolder }, + ) => { if (!value || !(value as { value?: unknown }).value) { - validation.addError('Error !'); + fieldValidation.addError('Error !'); + } + }, + TagPicker: ( + value: unknown, + fieldValidation: FieldValidation, + _context: { apiHolder: ApiHolder }, + ) => { + if (!value) { + fieldValidation.addError('A tag name can not be empty'); + } + if (!/^[a-z0-9-]+$/.test(value as string)) { + fieldValidation.addError( + 'A tag name can only contain lowercase letters, numeric characters or dashes', + ); } }, }; @@ -70,4 +88,39 @@ describe('createValidator', () => { expect(result).not.toBeNull(); expect(result.p1.addError).toHaveBeenCalledTimes(1); }); + + it('Should call validator for array property from a custom field extension', () => { + /* GIVEN */ + const rootSchema = { + title: 'My form', + properties: { + tags: { + title: 'Tags', + type: 'array', + items: { + type: 'string', + 'ui:field': 'TagPicker', + }, + }, + }, + }; + const validator = createValidator(rootSchema, validators, context); + + const formData = { + tags: ['invalid-tag$$'], + }; + const errors = { + addError: jest.fn(), + tags: { + addError: jest.fn(), + } as unknown as FormValidation, + } as unknown as FormValidation; + + /* WHEN */ + const result = validator(formData, errors); + + /* THEN */ + expect(result).not.toBeNull(); + expect(result.tags.addError).toHaveBeenCalledTimes(1); + }); }); diff --git a/plugins/scaffolder/src/components/TemplatePage/createValidator.ts b/plugins/scaffolder/src/components/TemplatePage/createValidator.ts index 4a64481fc3..98273da43a 100644 --- a/plugins/scaffolder/src/components/TemplatePage/createValidator.ts +++ b/plugins/scaffolder/src/components/TemplatePage/createValidator.ts @@ -23,6 +23,10 @@ function isObject(obj: unknown): obj is JsonObject { return typeof obj === 'object' && obj !== null && !Array.isArray(obj); } +function isArray(obj: unknown): obj is JsonObject { + return typeof obj === 'object' && obj !== null && Array.isArray(obj); +} + export const createValidator = ( rootSchema: JsonObject, validators: Record>, @@ -46,8 +50,8 @@ export const createValidator = ( for (const [key, propData] of Object.entries(formData)) { const propValidation = errors[key]; + const propSchemaProps = schemaProps[key]; if (isObject(propData)) { - const propSchemaProps = schemaProps[key]; if (isObject(propSchemaProps)) { validate( propSchemaProps, @@ -55,10 +59,26 @@ export const createValidator = ( propValidation as FormValidation, ); } + } else if (isArray(propData)) { + if (isObject(propSchemaProps)) { + const { items } = propSchemaProps; + if (isObject(items)) { + for (const propItem of propData as JsonObject[]) { + const fieldName = items['ui:field'] as string; + if (fieldName && typeof validators[fieldName] === 'function') { + validators[fieldName]!( + propItem as JsonValue, + propValidation, + context, + ); + } + } + } + } } else { - const propSchema = schemaProps[key]; const fieldName = - isObject(propSchema) && (propSchema['ui:field'] as string); + isObject(propSchemaProps) && + (propSchemaProps['ui:field'] as string); if (fieldName && typeof validators[fieldName] === 'function') { validators[fieldName]!( propData as JsonValue, From 074f7e81b5033ed68f75eec17a9ebc6c7f4a02fd Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Mon, 6 Feb 2023 13:49:44 +0100 Subject: [PATCH 2/5] Added a changeset Signed-off-by: bnechyporenko --- .changeset/lucky-papayas-allow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/lucky-papayas-allow.md diff --git a/.changeset/lucky-papayas-allow.md b/.changeset/lucky-papayas-allow.md new file mode 100644 index 0000000000..5a896efd0b --- /dev/null +++ b/.changeset/lucky-papayas-allow.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': minor +--- + +Added a missing validator check for items in an array From 8c63cebe70106a17efa3c7a3f3f2859591b6be6a Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Mon, 6 Feb 2023 14:05:02 +0100 Subject: [PATCH 3/5] Incorporated a feedback Signed-off-by: bnechyporenko --- .changeset/slow-moles-sin.md | 2 +- .../TemplatePage/createValidator.test.ts | 15 +++++++++------ .../components/TemplatePage/createValidator.ts | 16 +++++++--------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/.changeset/slow-moles-sin.md b/.changeset/slow-moles-sin.md index 140d9e7435..02e1952312 100644 --- a/.changeset/slow-moles-sin.md +++ b/.changeset/slow-moles-sin.md @@ -2,4 +2,4 @@ '@backstage/plugin-circleci': patch --- -Making workflow a link +Making workflow a link diff --git a/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts b/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts index da7a79b1b9..d9b9d1c0c3 100644 --- a/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts +++ b/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts @@ -32,17 +32,20 @@ describe('createValidator', () => { } }, TagPicker: ( - value: unknown, + values: unknown, fieldValidation: FieldValidation, _context: { apiHolder: ApiHolder }, ) => { - if (!value) { + const input = values as string[]; + if (input.length === 0) { fieldValidation.addError('A tag name can not be empty'); } - if (!/^[a-z0-9-]+$/.test(value as string)) { - fieldValidation.addError( - 'A tag name can only contain lowercase letters, numeric characters or dashes', - ); + for (const item of input) { + if (!/^[a-z0-9-]+$/.test(item)) { + fieldValidation.addError( + 'A tag name can only contain lowercase letters, numeric characters or dashes', + ); + } } }, }; diff --git a/plugins/scaffolder/src/components/TemplatePage/createValidator.ts b/plugins/scaffolder/src/components/TemplatePage/createValidator.ts index 98273da43a..123a7e800e 100644 --- a/plugins/scaffolder/src/components/TemplatePage/createValidator.ts +++ b/plugins/scaffolder/src/components/TemplatePage/createValidator.ts @@ -63,15 +63,13 @@ export const createValidator = ( if (isObject(propSchemaProps)) { const { items } = propSchemaProps; if (isObject(items)) { - for (const propItem of propData as JsonObject[]) { - const fieldName = items['ui:field'] as string; - if (fieldName && typeof validators[fieldName] === 'function') { - validators[fieldName]!( - propItem as JsonValue, - propValidation, - context, - ); - } + const fieldName = items['ui:field'] as string; + if (fieldName && typeof validators[fieldName] === 'function') { + validators[fieldName]!( + propData as JsonObject[], + propValidation, + context, + ); } } } From 7c0b64e8b9c84a51a2b9b3e7c66e64adb8bb8c0d Mon Sep 17 00:00:00 2001 From: bogdannechyporenko Date: Mon, 6 Feb 2023 23:27:00 +0100 Subject: [PATCH 4/5] Updated createValidator.test.ts Signed-off-by: bogdannechyporenko --- .../src/components/TemplatePage/createValidator.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts b/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts index d9b9d1c0c3..777de0cc36 100644 --- a/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts +++ b/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts @@ -37,9 +37,6 @@ describe('createValidator', () => { _context: { apiHolder: ApiHolder }, ) => { const input = values as string[]; - if (input.length === 0) { - fieldValidation.addError('A tag name can not be empty'); - } for (const item of input) { if (!/^[a-z0-9-]+$/.test(item)) { fieldValidation.addError( From 5a2faa801fd5d4ac8b9df68834c4cf8c86f12192 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Fri, 10 Feb 2023 14:29:04 +0100 Subject: [PATCH 5/5] Incorporated feedback Signed-off-by: bnechyporenko --- .../Stepper/createAsyncValidators.test.ts | 55 +++++++++++++++++++ .../Stepper/createAsyncValidators.ts | 38 +++++++++---- .../TemplatePage/createValidator.test.ts | 4 +- 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder-react/src/next/components/Stepper/createAsyncValidators.test.ts b/plugins/scaffolder-react/src/next/components/Stepper/createAsyncValidators.test.ts index fe8b41a9f3..3c4442afea 100644 --- a/plugins/scaffolder-react/src/next/components/Stepper/createAsyncValidators.test.ts +++ b/plugins/scaffolder-react/src/next/components/Stepper/createAsyncValidators.test.ts @@ -272,4 +272,59 @@ describe('createAsyncValidators', () => { }, }); }); + + it('should call a validator for array property from a custom field extension', async () => { + const schema: JsonObject = { + type: 'object', + properties: { + tags: { + title: 'Tags', + type: 'array', + items: { + type: 'string', + 'ui:field': 'TagField', + }, + }, + }, + }; + + const validators = { TagField: jest.fn() }; + + const validate = createAsyncValidators(schema, validators, { + apiHolder: { get: jest.fn() }, + }); + + await validate({ + tags: ['tag-1', 'tag-2'], + }); + + expect(validators.TagField).toHaveBeenCalled(); + }); + + it('should does not call a validator if no ui field specified', async () => { + const schema: JsonObject = { + type: 'object', + properties: { + tags: { + title: 'Tags', + type: 'array', + items: { + type: 'string', + }, + }, + }, + }; + + const validators = { TagField: jest.fn() }; + + const validate = createAsyncValidators(schema, validators, { + apiHolder: { get: jest.fn() }, + }); + + await validate({ + tags: ['asd', 'asd$'], + }); + + expect(validators.TagField).not.toHaveBeenCalled(); + }); }); diff --git a/plugins/scaffolder-react/src/next/components/Stepper/createAsyncValidators.ts b/plugins/scaffolder-react/src/next/components/Stepper/createAsyncValidators.ts index 86b7619b34..b524944719 100644 --- a/plugins/scaffolder-react/src/next/components/Stepper/createAsyncValidators.ts +++ b/plugins/scaffolder-react/src/next/components/Stepper/createAsyncValidators.ts @@ -15,7 +15,7 @@ */ import { FieldValidation } from '@rjsf/utils'; -import type { JsonObject } from '@backstage/types'; +import type { JsonObject, JsonValue } from '@backstage/types'; import { ApiHolder } from '@backstage/core-plugin-api'; import { Draft07 as JSONSchema } from 'json-schema-library'; import { createFieldValidation } from '../../lib'; @@ -44,20 +44,38 @@ export const createAsyncValidators = ( const parsedSchema = new JSONSchema(rootSchema); const formValidation: FormValidation = {}; + const validateForm = async ( + validatorName: string, + key: string, + value: JsonValue | undefined, + ) => { + const validator = validators[validatorName]; + if (validator) { + const fieldValidation = createFieldValidation(); + try { + await validator(value, fieldValidation, { ...context, formData }); + } catch (ex) { + fieldValidation.addError(ex.message); + } + formValidation[key] = fieldValidation; + } + }; + for (const [key, value] of Object.entries(current)) { const path = `${pathPrefix}/${key}`; const definitionInSchema = parsedSchema.getSchema(path, formData); if (definitionInSchema && 'ui:field' in definitionInSchema) { - const validator = validators[definitionInSchema['ui:field']]; - if (validator) { - const fieldValidation = createFieldValidation(); - try { - await validator(value, fieldValidation, { ...context, formData }); - } catch (ex) { - fieldValidation.addError(ex.message); - } - formValidation[key] = fieldValidation; + if ('ui:field' in definitionInSchema) { + await validateForm(definitionInSchema['ui:field'], key, value); + } + } else if ( + definitionInSchema && + definitionInSchema.items && + 'ui:field' in definitionInSchema.items + ) { + if ('ui:field' in definitionInSchema.items) { + await validateForm(definitionInSchema.items['ui:field'], key, value); } } else if (isObject(value)) { formValidation[key] = await validate(formData, path, value); diff --git a/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts b/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts index 777de0cc36..f5be77811a 100644 --- a/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts +++ b/plugins/scaffolder/src/components/TemplatePage/createValidator.test.ts @@ -57,7 +57,7 @@ describe('createValidator', () => { apiHolder: apiHolderMock, }; - it('Should call validator for object property from a custom field extension', () => { + it('should call validator for object property from a custom field extension', () => { /* GIVEN */ const rootSchema = { title: 'Title', @@ -89,7 +89,7 @@ describe('createValidator', () => { expect(result.p1.addError).toHaveBeenCalledTimes(1); }); - it('Should call validator for array property from a custom field extension', () => { + it('should call validator for array property from a custom field extension', () => { /* GIVEN */ const rootSchema = { title: 'My form',