From 20e066cd9d1e1b156ac793fe1e59be05669adf12 Mon Sep 17 00:00:00 2001 From: Josh <37798644+joshuaellis@users.noreply.github.com> Date: Fri, 10 May 2024 14:29:16 +0100 Subject: [PATCH] chore: refactor error types & validation messages --- .../core/admin/admin/src/components/Form.tsx | 43 ++++-- .../src/components/Layouts/GridLayout.tsx | 2 +- .../admin/src/components/Layouts/Layout.tsx | 4 +- .../admin/src/hooks/useDocument.ts | 27 ++-- .../admin/src/utils/tests/validation.test.ts | 125 ------------------ .../admin/src/utils/validation.ts | 44 +----- .../src/components/BulkLocaleActionModal.tsx | 52 +++++--- .../admin/src/components/CMHeaderActions.tsx | 7 +- 8 files changed, 81 insertions(+), 223 deletions(-) delete mode 100644 packages/core/content-manager/admin/src/utils/tests/validation.test.ts diff --git a/packages/core/admin/admin/src/components/Form.tsx b/packages/core/admin/admin/src/components/Form.tsx index a58514c11f5..df3d50277ed 100644 --- a/packages/core/admin/admin/src/components/Form.tsx +++ b/packages/core/admin/admin/src/components/Form.tsx @@ -228,18 +228,7 @@ const Form = React.forwardRef( return { data }; } catch (err) { if (isErrorYupValidationError(err)) { - let errors: FormErrors = {}; - - if (err.inner) { - if (err.inner.length === 0) { - return setIn(errors, err.path!, err.message); - } - for (const error of err.inner) { - if (!getIn(errors, error.path!)) { - errors = setIn(errors, error.path!, error.message); - } - } - } + const errors = getYupValidationErrors(err); if (shouldSetErrors) { setErrors(errors); @@ -470,11 +459,36 @@ const isErrorYupValidationError = (err: any): err is Yup.ValidationError => typeof err.name === 'string' && err.name === 'ValidationError'; +/* ------------------------------------------------------------------------------------------------- + * getYupValidationErrors + * -----------------------------------------------------------------------------------------------*/ + +/** + * @description handy utility to convert a yup validation error into a form + * error object. To be used elsewhere. + */ +const getYupValidationErrors = (err: Yup.ValidationError): FormErrors => { + let errors: FormErrors = {}; + + if (err.inner) { + if (err.inner.length === 0) { + return setIn(errors, err.path!, err.message); + } + for (const error of err.inner) { + if (!getIn(errors, error.path!)) { + errors = setIn(errors, error.path!, error.message); + } + } + } + + return errors; +}; + /* ------------------------------------------------------------------------------------------------- * reducer * -----------------------------------------------------------------------------------------------*/ -export type FormErrors = { +type FormErrors = { // is it a repeatable component or dynamic zone? [Key in keyof TFormValues]?: TFormValues[Key] extends any[] ? TFormValues[Key][number] extends object @@ -777,8 +791,9 @@ const Blocker = ({ onProceed = () => {}, onCancel = () => {} }: BlockerProps) => return null; }; -export { Form, Blocker, useField, useForm }; +export { Form, Blocker, useField, useForm, getYupValidationErrors }; export type { + FormErrors, FormHelpers, FormProps, FormValues, diff --git a/packages/core/admin/admin/src/components/Layouts/GridLayout.tsx b/packages/core/admin/admin/src/components/Layouts/GridLayout.tsx index fc6312ab0d0..73be05bd04c 100644 --- a/packages/core/admin/admin/src/components/Layouts/GridLayout.tsx +++ b/packages/core/admin/admin/src/components/Layouts/GridLayout.tsx @@ -34,4 +34,4 @@ const GridLayout = ({ size, children }: GridLayoutProps) => { }; export { GridLayout }; -export type { GridColSize }; +export type { GridLayoutProps, GridColSize }; diff --git a/packages/core/admin/admin/src/components/Layouts/Layout.tsx b/packages/core/admin/admin/src/components/Layouts/Layout.tsx index 009435aed67..1eb0b5d25c2 100644 --- a/packages/core/admin/admin/src/components/Layouts/Layout.tsx +++ b/packages/core/admin/admin/src/components/Layouts/Layout.tsx @@ -5,7 +5,7 @@ import { styled } from 'styled-components'; import { ActionLayout } from './ActionLayout'; import { ContentLayout } from './ContentLayout'; -import { GridLayout } from './GridLayout'; +import { GridLayout, GridLayoutProps } from './GridLayout'; import { HeaderLayout, BaseHeaderLayout } from './HeaderLayout'; interface LayoutProps { @@ -40,4 +40,4 @@ const Layouts = { Content: ContentLayout, }; -export { Layouts, type LayoutProps }; +export { Layouts, type LayoutProps, type GridLayoutProps }; diff --git a/packages/core/content-manager/admin/src/hooks/useDocument.ts b/packages/core/content-manager/admin/src/hooks/useDocument.ts index 549a11c50fc..0bdb6d2f74e 100644 --- a/packages/core/content-manager/admin/src/hooks/useDocument.ts +++ b/packages/core/content-manager/admin/src/hooks/useDocument.ts @@ -6,7 +6,13 @@ import * as React from 'react'; -import { useNotification, useAPIErrorHandler, useQueryParams } from '@strapi/admin/strapi-admin'; +import { + useNotification, + useAPIErrorHandler, + useQueryParams, + FormErrors, + getYupValidationErrors, +} from '@strapi/admin/strapi-admin'; import { Modules } from '@strapi/types'; import { useParams } from 'react-router-dom'; import { ValidationError } from 'yup'; @@ -14,13 +20,12 @@ import { ValidationError } from 'yup'; import { SINGLE_TYPES } from '../constants/collections'; import { useGetDocumentQuery } from '../services/documents'; import { buildValidParams } from '../utils/api'; -import { createYupSchema, getInnerErrors } from '../utils/validation'; +import { createYupSchema } from '../utils/validation'; import { useContentTypeSchema, ComponentsDictionary } from './useContentTypeSchema'; import type { FindOne } from '../../../shared/contracts/collection-types'; import type { ContentType } from '../../../shared/contracts/content-types'; -import type { MessageDescriptor, PrimitiveType } from 'react-intl'; interface UseDocumentArgs { collectionType: string; @@ -51,9 +56,7 @@ type UseDocument = ( * This is the schema of the content type, it is not the same as the layout. */ schema?: Schema; - validate: ( - document: Document - ) => null | Record }>; + validate: (document: Document) => null | FormErrors; }; /* ------------------------------------------------------------------------------------------------- @@ -118,9 +121,7 @@ const useDocument: UseDocument = (args, opts) => { }, [schema, components]); const validate = React.useCallback( - ( - document: Modules.Documents.AnyDocument - ): Record }> | null => { + (document: Modules.Documents.AnyDocument): FormErrors | null => { if (!validationSchema) { throw new Error( 'There is no validation schema generated, this is likely due to the schema not being loaded yet.' @@ -132,13 +133,7 @@ const useDocument: UseDocument = (args, opts) => { return null; } catch (error) { if (error instanceof ValidationError) { - const formattedErrors = getInnerErrors(error); - // Ensure that formattedErrors is of the correct type - // You may need to transform formattedErrors to match the expected type - return formattedErrors as unknown as Record< - string, - MessageDescriptor & { values?: Record } - >; + return getYupValidationErrors(error); } throw error; diff --git a/packages/core/content-manager/admin/src/utils/tests/validation.test.ts b/packages/core/content-manager/admin/src/utils/tests/validation.test.ts deleted file mode 100644 index 072ad4e767d..00000000000 --- a/packages/core/content-manager/admin/src/utils/tests/validation.test.ts +++ /dev/null @@ -1,125 +0,0 @@ -import { getInnerErrors } from '../validation'; - -import type { ValidationError } from 'yup'; - -describe('validation', () => { - describe('getInnerErrors', () => { - test('can extract relevant parameters from an error', () => { - const maxError = { - value: { number: 6 }, - name: 'ValidationError', - message: 'components.Input.error.validation.max', - errors: ['components.Input.error.validation.max'], - inner: [ - { - errors: ['components.Input.error.validation.max'], - message: 'components.Input.error.validation.max', - name: 'ValidationError', - params: { value: 6, originalValue: 6, path: 'number', max: 5 }, - path: 'number', - type: 'max', - value: 6, - inner: [], - }, - ], - } satisfies ValidationError; - - expect(getInnerErrors(maxError)).toMatchObject({ - number: { - id: 'components.Input.error.validation.max', - defaultMessage: 'components.Input.error.validation.max', - values: { max: 5 }, - }, - }); - }); - - test('can extract error messages from multiple errors', () => { - const multipleErrors = { - value: { number: 6, json: 'invalid json' }, - name: 'ValidationError', - message: '2 errors occurred', - errors: ['components.Input.error.validation.json', 'components.Input.error.validation.max'], - inner: [ - { - errors: ['components.Input.error.validation.json'], - message: 'components.Input.error.validation.json', - name: 'ValidationError', - params: { value: 'invalid json', originalValue: 'invalid json', path: 'json' }, - path: 'json', - type: 'isJSON', - value: 'invalid json', - inner: [], - }, - { - errors: ['components.Input.error.validation.max'], - message: 'components.Input.error.validation.max', - name: 'ValidationError', - params: { value: 6, originalValue: 6, path: 'number', max: 5 }, - path: 'number', - type: 'max', - value: 6, - inner: [], - }, - ], - } satisfies ValidationError; - - expect(getInnerErrors(multipleErrors)).toMatchObject({ - json: { - id: 'components.Input.error.validation.json', - defaultMessage: 'components.Input.error.validation.json', - values: {}, - }, - number: { - id: 'components.Input.error.validation.max', - defaultMessage: 'components.Input.error.validation.max', - values: { max: 5 }, - }, - }); - }); - - test('can extract errors from dynamic zones errors', () => { - const dynamicZoneErrors = { - errors: ['components.Input.error.validation.max'], - message: 'components.Input.error.validation.max', - name: 'ValidationError', - value: { - dynamicZone: [ - { - __component: 'component', - number: 6, - }, - ], - }, - inner: [ - { - errors: ['components.Input.error.validation.max'], - message: 'components.Input.error.validation.max', - name: 'ValidationError', - params: { value: 6, originalValue: 6, path: 'dynamicZone[0].number', max: 5 }, - path: 'dynamicZone[0].number', - type: 'max', - value: 6, - inner: [], - }, - ], - } satisfies ValidationError; - - expect(getInnerErrors(dynamicZoneErrors)).toMatchObject({ - dynamicZone: [ - { - number: { - defaultMessage: 'components.Input.error.validation.max', - id: 'components.Input.error.validation.max', - values: { - max: 5, - originalValue: 6, - path: 'dynamicZone[0].number', - value: 6, - }, - }, - }, - ], - }); - }); - }); -}); diff --git a/packages/core/content-manager/admin/src/utils/validation.ts b/packages/core/content-manager/admin/src/utils/validation.ts index 56f31422854..ecf4e94b66f 100644 --- a/packages/core/content-manager/admin/src/utils/validation.ts +++ b/packages/core/content-manager/admin/src/utils/validation.ts @@ -1,12 +1,9 @@ -import { translatedErrors, FormErrors } from '@strapi/admin/strapi-admin'; +import { translatedErrors } from '@strapi/admin/strapi-admin'; import pipe from 'lodash/fp/pipe'; -import { type MessageDescriptor } from 'react-intl'; import * as yup from 'yup'; import { DOCUMENT_META_FIELDS } from '../constants/attributes'; -import { getIn, setIn } from './objects'; - import type { ComponentsDictionary, Schema } from '../hooks/useDocument'; import type { Schema as SchemaUtils } from '@strapi/types'; import type { ObjectShape } from 'yup/lib/object'; @@ -338,41 +335,4 @@ const addRegexValidation: ValidationFn = return schema; }; -/* ------------------------------------------------------------------------------------------------- - * getInnerErrors - * -----------------------------------------------------------------------------------------------*/ - -const isErrorMessageDescriptor = (object?: string | object): object is MessageDescriptor => { - return ( - typeof object === 'object' && object !== null && 'id' in object && 'defaultMessage' in object - ); -}; - -const getInnerErrors = (error: yup.ValidationError): FormErrors => { - let errors: FormErrors = {}; - - if (error.inner) { - if (error.inner.length === 0 && error.path) { - errors = setIn(errors, error.path, error.message); - return errors; - } - - error.inner.forEach((innerError) => { - if (innerError.path && !getIn(errors, innerError.path)) { - const message = isErrorMessageDescriptor(innerError.message) - ? innerError.message - : { - id: innerError.message, - defaultMessage: innerError.message, - values: innerError.params, - }; - - errors = setIn(errors, innerError.path, message); - } - }); - } - - return errors; -}; - -export { createYupSchema, getInnerErrors }; +export { createYupSchema }; diff --git a/packages/plugins/i18n/admin/src/components/BulkLocaleActionModal.tsx b/packages/plugins/i18n/admin/src/components/BulkLocaleActionModal.tsx index bcf265518a3..b782205a71e 100644 --- a/packages/plugins/i18n/admin/src/components/BulkLocaleActionModal.tsx +++ b/packages/plugins/i18n/admin/src/components/BulkLocaleActionModal.tsx @@ -1,11 +1,11 @@ import * as React from 'react'; -import { Table, useTable } from '@strapi/admin/strapi-admin'; +import { FormErrors, Table, useTable } from '@strapi/admin/strapi-admin'; import { Box, Typography, IconButton, Flex, Tooltip, Status } from '@strapi/design-system'; import { Pencil, CheckCircle, CrossCircle, ArrowsCounterClockwise } from '@strapi/icons'; import { Modules } from '@strapi/types'; import { stringify } from 'qs'; -import { type MessageDescriptor, useIntl } from 'react-intl'; +import { type MessageDescriptor, useIntl, PrimitiveType } from 'react-intl'; import { useNavigate } from 'react-router-dom'; import { Locale } from '../../../shared/contracts/locales'; @@ -22,10 +22,14 @@ type Status = Modules.Documents.Params.PublicationStatus.Kind | 'modified'; interface EntryValidationTextProps { status: Status; - validationErrors?: Record; + validationErrors: FormErrors[string] | null; } -const isErrorMessageDescriptor = (object?: string | object): object is MessageDescriptor => { +interface TranslationMessage extends MessageDescriptor { + values?: Record; +} + +const isErrorMessageDescriptor = (object?: string | object): object is TranslationMessage => { return ( typeof object === 'object' && object !== null && 'id' in object && 'defaultMessage' in object ); @@ -34,20 +38,33 @@ const isErrorMessageDescriptor = (object?: string | object): object is MessageDe const EntryValidationText = ({ status = 'draft', validationErrors }: EntryValidationTextProps) => { const { formatMessage } = useIntl(); + /** + * TODO: Should this be extracted an made into a factory to recursively get + * error messages?? + */ + const getErrorStr = (key: string, value?: FormErrors[string]): string => { + if (typeof value === 'string') { + return `${key}: ${value}`; + } else if (isErrorMessageDescriptor(value)) { + return `${key}: ${formatMessage(value)}`; + } else if (Array.isArray(value)) { + return value.map((v) => getErrorStr(key, v)).join(' '); + } else if (typeof value === 'object' && !Array.isArray(value)) { + return Object.entries(value) + .map(([k, v]) => getErrorStr(k, v)) + .join(' '); + } else { + /** + * unlikely to happen, but we need to return something + */ + return ''; + } + }; + if (validationErrors) { const validationErrorsMessages = Object.entries(validationErrors) .map(([key, value]) => { - const builtKey = [key]; - - let currentValue = value; - while (typeof currentValue === 'object' && !isErrorMessageDescriptor(currentValue)) { - // We need to extract errors from nested components - const newKey = Object.keys(currentValue)[0]; - builtKey.push(newKey); - currentValue = currentValue[newKey]; - } - - return `${builtKey.join('.')}: ${formatMessage(currentValue as MessageDescriptor)}`; + return getErrorStr(key, value); }) .join(' '); @@ -128,10 +145,7 @@ interface BulkLocaleActionModalProps { }[]; onClose: () => void; localesMetadata: Locale[]; - validationErrors?: Record< - Modules.Documents.Params.Locale.StringNotation, - Record - >; + validationErrors?: FormErrors; } const BulkLocaleActionModal = ({ diff --git a/packages/plugins/i18n/admin/src/components/CMHeaderActions.tsx b/packages/plugins/i18n/admin/src/components/CMHeaderActions.tsx index 2855f8cb8d7..b62110fab17 100644 --- a/packages/plugins/i18n/admin/src/components/CMHeaderActions.tsx +++ b/packages/plugins/i18n/admin/src/components/CMHeaderActions.tsx @@ -5,6 +5,7 @@ import { useQueryParams, Table, useAPIErrorHandler, + FormErrors, } from '@strapi/admin/strapi-admin'; import { type HeaderActionComponent, @@ -16,7 +17,7 @@ import { import { Flex, Status, Typography, Button } from '@strapi/design-system'; import { WarningCircle, ListPlus, Trash } from '@strapi/icons'; import { Modules } from '@strapi/types'; -import { useIntl, type MessageDescriptor } from 'react-intl'; +import { useIntl } from 'react-intl'; import { useNavigate } from 'react-router-dom'; import { styled } from 'styled-components'; @@ -331,9 +332,7 @@ const BulkLocalePublishAction: DocumentActionComponent = ({ // Build the validation errors for each locale. const allDocuments = [document, ...(documentMeta?.availableLocales ?? [])]; - const errors = allDocuments.reduce< - Record> - >((errs, document) => { + const errors = allDocuments.reduce((errs, document) => { if (!document) { return errs; }