Improved error handling in recommendation modals (#18524)

fixes https://github.com/TryGhost/Product/issues/3990

- Show errors on blur
- Show toast on submit (final modal only)
- Do an initial validation on mount

apps/admin-x-settings/src/admin-x-ds/global/form/URLTextField.tsx

@@ -26,7 +26,7 @@ export const formatUrl = (value: string, baseUrl?: string) => {
 
     if (!baseUrl) {
         // Absolute URL with no base URL
-        if (!url.startsWith('http://') && !url.startsWith('https://')) {
+        if (!url.startsWith('http')) {
             url = `https://${url}`;
         }
     }

apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx

@@ -4,7 +4,7 @@ import Modal from '../../../../admin-x-ds/global/modal/Modal';
 import NiceModal, {useModal} from '@ebay/nice-modal-react';
 import React, {useEffect, useState} from 'react';
 import TextField from '../../../../admin-x-ds/global/form/TextField';
-import useForm from '../../../../hooks/useForm';
+import useForm, {ErrorMessages} from '../../../../hooks/useForm';
 import useRouting from '../../../../hooks/useRouting';
 import {AlreadyExistsError} from '../../../../utils/errors';
 import {EditOrAddRecommendation, RecommendationResponseType, useGetRecommendationByUrl} from '../../../../api/recommendations';
@@ -25,6 +25,22 @@ const doFormatUrl = (url: string) => {
     return formatUrl(url).save;
 };
 
+const validateUrl = function (errors: ErrorMessages, url: string) {
+    try {
+        const u = new URL(url);
+
+        // Check domain includes a dot
+        if (!u.hostname.includes('.')) {
+            errors.url = 'Please enter a valid URL.';
+        } else {
+            delete errors.url;
+        }
+    } catch (e) {
+        errors.url = 'Please enter a valid URL.';
+    }
+    return errors;
+};
+
 const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModalProps> = ({searchParams, recommendation, animate}) => {
     const [enterPressed, setEnterPressed] = useState(false);
     const modal = useModal();
@@ -41,7 +57,7 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
     const didInitialSubmit = React.useRef(false);
     const [showLoadingView, setShowLoadingView] = React.useState(!!initialUrlCleaned);
 
-    const {formState, updateForm, handleSave, errors, saveState, clearError} = useForm({
+    const {formState, updateForm, handleSave, errors, saveState, clearError, setErrors} = useForm({
         initialState: recommendation ?? {
             title: '',
             url: initialUrlCleaned,
@@ -119,16 +135,7 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
         onValidate: () => {
             const newErrors: Record<string, string> = {};
 
-            try {
-                const u = new URL(formState.url);
-
-                // Check domain includes a dot
-                if (!u.hostname.includes('.')) {
-                    newErrors.url = 'Please enter a valid URL.';
-                }
-            } catch (e) {
-                newErrors.url = 'Please enter a valid URL.';
-            }
+            validateUrl(newErrors, formState.url);
 
             // If we have errors: close direct submit view
             if (showLoadingView) {
@@ -225,7 +232,13 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
                 placeholder='https://www.example.com'
                 title='URL'
                 value={formState.url}
-                onBlur={() => updateForm(state => ({...state, url: doFormatUrl(formState.url)}))}
+                onBlur={() => {
+                    const url = doFormatUrl(formState.url);
+                    setErrors(
+                        validateUrl(errors, url)
+                    );
+                    updateForm(state => ({...state, url: url}));
+                }}
                 onChange={(e) => {
                     clearError?.('url');
                     updateForm(state => ({...state, url: e.target.value}));

apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModalConfirm.tsx

@@ -2,7 +2,7 @@ import AddRecommendationModal from './AddRecommendationModal';
 import Modal from '../../../../admin-x-ds/global/modal/Modal';
 import NiceModal, {useModal} from '@ebay/nice-modal-react';
 import React from 'react';
-import RecommendationReasonForm from './RecommendationReasonForm';
+import RecommendationReasonForm, {validateReasonForm} from './RecommendationReasonForm';
 import useForm from '../../../../hooks/useForm';
 import useHandleError from '../../../../utils/api/handleError';
 import useRouting from '../../../../hooks/useRouting';
@@ -20,12 +20,12 @@ const AddRecommendationModalConfirm: React.FC<AddRecommendationModalProps> = ({r
     const {mutateAsync: addRecommendation} = useAddRecommendation();
     const handleError = useHandleError();
 
-    const {formState, updateForm, handleSave, saveState, errors, clearError} = useForm({
+    const {formState, updateForm, handleSave, saveState, errors, clearError, setErrors} = useForm({
         initialState: {
             ...recommendation
         },
-        onSave: async () => {
-            await addRecommendation(formState);
+        onSave: async (state) => {
+            await addRecommendation(state);
             modal.remove();
             showToast({
                 message: 'Successfully added a recommendation',
@@ -34,15 +34,16 @@ const AddRecommendationModalConfirm: React.FC<AddRecommendationModalProps> = ({r
             updateRoute('recommendations');
         },
         onSaveError: handleError,
-        onValidate: () => {
-            const newErrors: Record<string, string> = {};
-            if (!formState.title) {
-                newErrors.title = 'Title is required';
+        onValidate: (state) => {
+            const newErrors = validateReasonForm(state);
+
+            if (Object.keys(newErrors).length !== 0) {
+                showToast({
+                    type: 'pageError',
+                    message: 'Can\'t add recommendation, please double check that you\'ve filled all mandatory fields correctly.'
+                });
             }
 
-            if (formState.reason && formState.reason.length > 200) {
-                newErrors.reason = 'Description cannot be longer than 200 characters';
-            }
             return newErrors;
         }
     });
@@ -122,7 +123,7 @@ const AddRecommendationModalConfirm: React.FC<AddRecommendationModalProps> = ({r
             }
         }}
     >
-        <RecommendationReasonForm clearError={clearError} errors={errors} formState={formState} showURL={false} updateForm={updateForm}/>
+        <RecommendationReasonForm clearError={clearError} errors={errors} formState={formState} setErrors={setErrors} showURL={false} updateForm={updateForm}/>
     </Modal>;
 };
 

apps/admin-x-settings/src/components/settings/site/recommendations/EditRecommendationModal.tsx

@@ -2,7 +2,7 @@ import ConfirmationModal from '../../../../admin-x-ds/global/modal/ConfirmationM
 import Modal from '../../../../admin-x-ds/global/modal/Modal';
 import NiceModal, {useModal} from '@ebay/nice-modal-react';
 import React from 'react';
-import RecommendationReasonForm from './RecommendationReasonForm';
+import RecommendationReasonForm, {validateReasonForm} from './RecommendationReasonForm';
 import useForm from '../../../../hooks/useForm';
 import useHandleError from '../../../../utils/api/handleError';
 import useRouting from '../../../../hooks/useRouting';
@@ -22,25 +22,24 @@ const EditRecommendationModal: React.FC<RoutingModalProps & EditRecommendationMo
     const {mutateAsync: deleteRecommendation} = useDeleteRecommendation();
     const handleError = useHandleError();
 
-    const {formState, updateForm, handleSave, saveState, errors, clearError} = useForm({
+    const {formState, updateForm, handleSave, saveState, errors, clearError, setErrors} = useForm({
         initialState: {
             ...recommendation
         },
-        onSave: async () => {
-            await editRecommendation(formState);
+        onSave: async (state) => {
+            await editRecommendation(state);
             modal.remove();
             updateRoute('recommendations');
         },
         onSaveError: handleError,
-        onValidate: () => {
-            const newErrors: Record<string, string> = {};
+        onValidate: (state) => {
+            const newErrors = validateReasonForm(state);
 
-            if (!formState.title) {
-                newErrors.title = 'Title is required';
-            }
-
-            if (formState.reason && formState.reason.length > 200) {
-                newErrors.reason = 'Description cannot be longer than 200 characters';
+            if (Object.keys(newErrors).length !== 0) {
+                showToast({
+                    type: 'pageError',
+                    message: 'Can\'t edit recommendation, please double check that you\'ve filled all mandatory fields correctly.'
+                });
             }
 
             return newErrors;
@@ -120,7 +119,7 @@ const EditRecommendationModal: React.FC<RoutingModalProps & EditRecommendationMo
             }
         }}
     >
-        <RecommendationReasonForm clearError={clearError} errors={errors} formState={formState} showURL={true} updateForm={updateForm as any}/>
+        <RecommendationReasonForm clearError={clearError} errors={errors} formState={formState} setErrors={setErrors} showURL={true} updateForm={updateForm as any}/>
     </Modal>;
 };
 

apps/admin-x-settings/src/components/settings/site/recommendations/RecommendationReasonForm.tsx

@@ -14,12 +14,56 @@ interface Props<T extends EditOrAddRecommendation> {
     formState: T,
     errors: ErrorMessages,
     updateForm: (fn: (state: T) => T) => void,
-    clearError?: (key: keyof ErrorMessages) => void
+    clearError?: (key: keyof ErrorMessages) => void,
+    setErrors: (errors: ErrorMessages) => void
 }
 
-const RecommendationReasonForm: React.FC<Props<EditOrAddRecommendation | Recommendation>> = ({showURL, formState, updateForm, errors, clearError}) => {
+export const validateReasonFormField = function (errors: ErrorMessages, field: 'title'|'reason', value: string|null) {
+    const cloned = {...errors};
+    switch (field) {
+    case 'title':
+        if (!value) {
+            cloned.title = 'Title is required';
+        } else {
+            delete cloned.title;
+        }
+        break;
+    case 'reason':
+        if (value && value.length > 200) {
+            cloned.reason = 'Description cannot be longer than 200 characters';
+        } else {
+            delete cloned.reason;
+        }
+        break;
+    default:
+        // Will throw a compile error if we forget to add a case for a field
+        const f: never = field;
+        throw new Error(`Unknown field ${f}`);
+    }
+    return cloned;
+};
+
+export const validateReasonForm = function (formState: EditOrAddRecommendation) {
+    let newErrors: ErrorMessages = {};
+    newErrors = validateReasonFormField(newErrors, 'title', formState.title);
+    newErrors = validateReasonFormField(newErrors, 'reason', formState.reason);
+    return newErrors;
+};
+
+const RecommendationReasonForm: React.FC<Props<EditOrAddRecommendation | Recommendation>> = ({showURL, formState, updateForm, errors, clearError, setErrors}) => {
     const [reasonLength, setReasonLength] = React.useState(formState?.reason?.length || 0);
     const reasonLengthColor = reasonLength > 200 ? 'text-red' : 'text-green';
+
+    // Do an intial validation on mounting
+    const didValidate = React.useRef(false);
+    React.useEffect(() => {
+        if (didValidate.current) {
+            return;
+        }
+        didValidate.current = true;
+        setErrors(validateReasonForm(formState));
+    }, [formState, setErrors]);
+
     return <Form
         marginBottom={false}
         marginTop
@@ -63,6 +107,9 @@ const RecommendationReasonForm: React.FC<Props<EditOrAddRecommendation | Recomme
             hint={errors.title}
             title="Title"
             value={formState.title ?? ''}
+            onBlur={() => setErrors(
+                validateReasonFormField(errors, 'title', formState.title)
+            )}
             onChange={(e) => {
                 clearError?.('title');
                 updateForm(state => ({...state, title: e.target.value}));
@@ -71,10 +118,14 @@ const RecommendationReasonForm: React.FC<Props<EditOrAddRecommendation | Recomme
         <TextArea
             clearBg={true}
             error={Boolean(errors.reason)}
-            hint={errors.reason || <>Max: <strong>200</strong> characters. You&#8217;ve used <strong className={reasonLengthColor}>{reasonLength}</strong></>}
+            // Note: we don't show the error text here, because errors are related to the character count
+            hint={<>Max: <strong>200</strong> characters. You&#8217;ve used <strong className={reasonLengthColor}>{reasonLength}</strong></>}
             rows={4}
             title="Short description"
             value={formState.reason ?? ''}
+            onBlur={() => setErrors(
+                validateReasonFormField(errors, 'reason', formState.reason)
+            )}
             onChange={(e) => {
                 clearError?.('reason');
                 setReasonLength(e.target.value.length);

apps/admin-x-settings/src/hooks/useForm.ts

@@ -29,13 +29,14 @@ export interface FormHook<State> {
     clearError: (field: string) => void;
     isValid: boolean;
     errors: ErrorMessages;
+    setErrors: (errors: ErrorMessages) => void;
 }
 
 const useForm = <State>({initialState, onSave, onSaveError, onValidate}: {
     initialState: State,
-    onSave: () => void | Promise<void>
+    onSave: (state: State) => void | Promise<void>
     onSaveError?: (error: unknown) => void | Promise<void>
-    onValidate?: () => ErrorMessages
+    onValidate?: (state: State) => ErrorMessages
 }): FormHook<State> => {
     const [formState, setFormState] = useState(initialState);
     const [saveState, setSaveState] = useState<SaveState>('');
@@ -52,37 +53,43 @@ const useForm = <State>({initialState, onSave, onSaveError, onValidate}: {
 
     const isValid = (errs: ErrorMessages) => Object.values(errs).filter(Boolean).length === 0;
 
-    const validate = () => {
-        if (!onValidate) {
-            return true;
-        }
+    const validate = useCallback(
+        () => {
+            if (!onValidate) {
+                return true;
+            }
 
-        const newErrors = onValidate();
-        setErrors(newErrors);
-        return isValid(newErrors);
-    };
+            const newErrors = onValidate(formState);
+            setErrors(newErrors);
+            return isValid(newErrors);
+        },
+        [formState, onValidate]
+    );
 
     // function to save the changed settings via API
-    const handleSave = async (options: {force?: boolean} = {}) => {
-        if (!validate()) {
-            return false;
-        }
+    const handleSave = useCallback(
+        async (options: {force?: boolean} = {}) => {
+            if (!validate()) {
+                return false;
+            }
 
-        if (saveState !== 'unsaved' && !options.force) {
-            return true;
-        }
+            if (saveState !== 'unsaved' && !options.force) {
+                return true;
+            }
 
-        setSaveState('saving');
-        try {
-            await onSave();
-            setSaveState('saved');
-            return true;
-        } catch (e) {
-            await onSaveError?.(e);
-            setSaveState('unsaved');
-            throw e;
-        }
-    };
+            setSaveState('saving');
+            try {
+                await onSave(formState);
+                setSaveState('saved');
+                return true;
+            } catch (e) {
+                await onSaveError?.(e);
+                setSaveState('unsaved');
+                throw e;
+            }
+        },
+        [formState, saveState, onSave, onSaveError, validate]
+    );
 
     const updateForm = useCallback((updater: (state: State) => State) => {
         setFormState(updater);
@@ -104,7 +111,8 @@ const useForm = <State>({initialState, onSave, onSaveError, onValidate}: {
         clearError: (field: string) => {
             setErrors(state => ({...state, [field]: ''}));
         },
-        errors
+        errors,
+        setErrors
     };
 };