From b2ce7f18c04905c1773dc7624a8c186d98a05c4e Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Feb 2021 11:23:25 +0100 Subject: [PATCH] feat: added recursive validation for the repository url picker component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Johan Haals Co-authored-by: Fredrik Adelöw Co-authored-by: Patrik Oldsberg Signed-off-by: Johan Haals --- .../MultistepJsonForm/MultistepJsonForm.tsx | 59 ++++++++-------- .../TemplatePage/TemplatePage.test.tsx | 37 +++++++++- .../components/TemplatePage/TemplatePage.tsx | 68 ++++++++++++++++-- .../components/fields/RepoUrlPicker/index.tsx | 70 +++++++++++-------- 4 files changed, 170 insertions(+), 64 deletions(-) diff --git a/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx b/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx index b33bd82ffa..876d6dfa66 100644 --- a/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx +++ b/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx @@ -71,34 +71,37 @@ export const MultistepJsonForm = ({ return ( <> - {steps.map(({ title, schema, ...formProps }) => ( - - - {title} - - -
{ - if (e.errors.length === 0) handleNext(); - }} - {...formProps} - {...transformSchemaToProps(schema)} - > - - -
- - - ))} + {steps.map(({ title, schema, ...formProps }) => { + return ( + + + {title} + + +
{ + if (e.errors.length === 0) handleNext(); + }} + {...formProps} + {...transformSchemaToProps(schema)} + > + + +
+ + + ); + })} {activeStep === steps.length && ( diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.test.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.test.tsx index 1d66e07c6a..0556dd1129 100644 --- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.test.tsx +++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.test.tsx @@ -22,7 +22,7 @@ import { act } from 'react-dom/test-utils'; import { MemoryRouter, Route } from 'react-router'; import { ScaffolderApi, scaffolderApiRef } from '../../api'; import { rootRouteRef } from '../../routes'; -import { TemplatePage } from './TemplatePage'; +import { TemplatePage, createValidator } from './TemplatePage'; jest.mock('react-router-dom', () => { return { @@ -122,3 +122,38 @@ describe('TemplatePage', () => { expect(rendered.queryByText('This is root')).toBeInTheDocument(); }); }); + +describe('createValidator', () => { + it('should validate deep schema', () => { + const validator = createValidator({ + type: 'object', + properties: { + foo: { + type: 'object', + properties: { + bar: { + type: 'string', + 'ui:field': 'RepoUrlPicker', + }, + }, + }, + }, + }); + + const errors = { foo: { bar: { addError: jest.fn() } } }; + validator({ foo: { bar: 'github.com?owner=a' } }, errors as any); + expect(errors.foo.bar.addError).toHaveBeenCalledWith( + 'Incomplete repository location provided', + ); + jest.resetAllMocks(); + + validator({ foo: { bar: 'github.com?repo=b' } }, errors as any); + expect(errors.foo.bar.addError).toHaveBeenCalledWith( + 'Incomplete repository location provided', + ); + jest.resetAllMocks(); + + validator({ foo: { bar: 'github.com?owner=a&repo=b' } }, errors as any); + expect(errors.foo.bar.addError).not.toHaveBeenCalled(); + }); +}); diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx index 282154c1e2..5d173173ee 100644 --- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx +++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx @@ -34,6 +34,7 @@ import { scaffolderApiRef } from '../../api'; import { rootRouteRef } from '../../routes'; import { MultistepJsonForm } from '../MultistepJsonForm'; import { RepoUrlPicker } from '../fields'; +import { JsonObject } from '@backstage/config'; const useTemplateParameterSchema = (templateName: string) => { const scaffolderApi = useApi(scaffolderApiRef); @@ -49,12 +50,67 @@ const useTemplateParameterSchema = (templateName: string) => { return { schema: value, loading, error }; }; +function isObject(obj: unknown): obj is JsonObject { + return typeof obj === 'object' && obj !== null && !Array.isArray(obj); +} + +export const createValidator = (rootSchema: JsonObject) => { + function validate( + schema: JsonObject, + formData: JsonObject, + errors: FormValidation, + ) { + const schemaProps = schema.properties; + if (!isObject(schemaProps)) { + return; + } + + for (const [key, propData] of Object.entries(formData)) { + const propErrors = errors[key]; + + if (isObject(propData)) { + const propSchemaProps = schemaProps[key]; + if (isObject(propSchemaProps)) { + validate( + propSchemaProps, + propData as JsonObject, + propErrors as FormValidation, + ); + } + } else { + const propSchema = schemaProps[key]; + if ( + isObject(propSchema) && + propSchema['ui:field'] === 'RepoUrlPicker' + ) { + try { + const { host, searchParams } = new URL(`https://${propData}`); + if ( + !host || + !searchParams.get('owner') || + !searchParams.get('repo') + ) { + propErrors.addError('Incomplete repository location provided'); + } + } catch { + propErrors.addError('Unable to parse the Repository URL'); + } + } + } + } + } + + return (formData: JsonObject, errors: FormValidation) => { + validate(rootSchema, formData, errors); + return errors; + }; +}; + const storePathValidator = ( formData: { storePath?: string }, errors: FormValidation, ) => { const { storePath } = formData; - if (!storePath) { errors.storePath.addError('Store path is required and not present'); return errors; @@ -139,15 +195,19 @@ export const TemplatePage = () => { onReset={handleFormReset} onFinish={handleCreate} steps={schema.steps.map(step => { - // TODO: Using this workaround to keep storePath validation, but we should replace - // it with a custom store path selection widget + // TODO: Can delete this function when the migration from v1 to v2 beta is completed + // And just have the default validator for all fields. if ((step.schema as any)?.properties?.storePath) { return { ...step, validate: (a, b) => storePathValidator(a, b), }; } - return step; + + return { + ...step, + validate: createValidator(step.schema), + }; })} /> diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/index.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/index.tsx index cb7eec017d..a20f748d3a 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/index.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/index.tsx @@ -14,16 +14,18 @@ * limitations under the License. */ import React, { useState, useCallback, useEffect } from 'react'; -import { Field, Widget } from '@rjsf/core'; +import { Field } from '@rjsf/core'; import { useApi, Progress } from '@backstage/core'; import { scaffolderApiRef } from '../../../api'; import { useAsync } from 'react-use'; -import TextField from '@material-ui/core/TextField'; -import MenuItem from '@material-ui/core/MenuItem'; - +import Select from '@material-ui/core/Select'; +import InputLabel from '@material-ui/core/InputLabel'; +import Input from '@material-ui/core/Input'; +import FormControl from '@material-ui/core/FormControl'; import { Typography } from '@material-ui/core'; +import { rest } from 'msw/lib/types'; -export const RepoUrlPicker: Field = ({ onChange, uiSchema }) => { +export const RepoUrlPicker: Field = ({ onChange, uiSchema, ...rest }) => { const api = useApi(scaffolderApiRef); const allowedHosts = uiSchema['ui:options']?.allowedHosts as string[]; @@ -31,14 +33,16 @@ export const RepoUrlPicker: Field = ({ onChange, uiSchema }) => { return await api.getIntegrationsList({ allowedHosts }); }); - const [hostname, setHostname] = useState(''); + console.log(rest); + + const [host, setHost] = useState(''); const [owner, setOwner] = useState(''); const [repo, setRepo] = useState(''); - const updateHostname = useCallback( + const updateHost = useCallback( (evt: React.ChangeEvent<{ name?: string; value: unknown }>) => - setHostname(evt.target.value as string), - [setHostname], + setHost(evt.target.value as string), + [setHost], ); const updateOwner = useCallback( @@ -54,18 +58,18 @@ export const RepoUrlPicker: Field = ({ onChange, uiSchema }) => { ); useEffect(() => { - if (hostname === '' && integrations?.length) { - setHostname(integrations[0].host); + if (host === '' && integrations?.length) { + setHost(integrations[0].host); } - }, [integrations, hostname]); + }, [integrations, host]); useEffect(() => { const params = new URLSearchParams(); params.set('owner', owner); params.set('repo', repo); - onChange(`${hostname}?${params.toString()}`); - }, [hostname, owner, repo, onChange]); + onChange(`${encodeURIComponent(host)}?${params.toString()}`); + }, [host, owner, repo, onChange]); if (loading) { return ; @@ -73,23 +77,27 @@ export const RepoUrlPicker: Field = ({ onChange, uiSchema }) => { return ( <> - Repository Location - - {integrations! - .filter(i => allowedHosts?.includes(i.host)) - .map(({ host, title }) => ( - - {title} - - ))} - - - + Repository Location + + Host + + + + Owner + + + + Repository + + ); };