Merge pull request #8339 from keithjgrant/7515-form-error-polish

Refactor FormSubmitError for easier testing, better error display

Reviewed-by: John Hill <johill@redhat.com>
             https://github.com/unlikelyzero

awx/ui_next/src/components/FormField/FormSubmitError.jsx

@@ -2,62 +2,21 @@ import React, { useState, useEffect } from 'react';
 import { useFormikContext } from 'formik';
 import { Alert } from '@patternfly/react-core';
 import { FormFullWidthLayout } from '../FormLayout';
-
+import sortErrorMessages from './sortErrorMessages';
-const findErrorStrings = (obj, messages = []) => {
-  if (typeof obj === 'string') {
-    messages.push(obj);
-  } else if (typeof obj === 'object') {
-    Object.keys(obj).forEach(key => {
-      const value = obj[key];
-      if (typeof value === 'string') {
-        messages.push(value);
-      } else if (Array.isArray(value)) {
-        value.forEach(arrValue => {
-          messages = findErrorStrings(arrValue, messages);
-        });
-      } else if (typeof value === 'object') {
-        messages = findErrorStrings(value, messages);
-      }
-    });
-  }
-  return messages;
-};
 
 function FormSubmitError({ error }) {
   const [errorMessage, setErrorMessage] = useState(null);
-  const { setErrors } = useFormikContext();
+  const { values, setErrors } = useFormikContext();
 
   useEffect(() => {
-    if (!error) {
+    const { formError, fieldErrors } = sortErrorMessages(error, values);
-      return;
+    if (formError) {
+      setErrorMessage(formError);
     }
-    if (
+    if (fieldErrors) {
-      error?.response?.data &&
+      setErrors(fieldErrors);
-      typeof error.response.data === 'object' &&
-      Object.keys(error.response.data).length > 0
-    ) {
-      const errorMessages = {};
-      Object.keys(error.response.data).forEach(fieldName => {
-        const errors = error.response.data[fieldName];
-        if (!errors) {
-          return;
-        }
-        if (Array.isArray(errors.length)) {
-          errorMessages[fieldName] = errors.join(' ');
-        } else {
-          errorMessages[fieldName] = errors;
-        }
-      });
-      setErrors(errorMessages);
-
-      const messages = findErrorStrings(error.response.data);
-      setErrorMessage(messages.length > 0 ? messages : null);
-    } else {
-      /* eslint-disable-next-line no-console */
-      console.error(error);
-      setErrorMessage(error.message);
     }
-  }, [error, setErrors]);
+  }, [error, setErrors, values]);
 
   if (!errorMessage) {
     return null;

awx/ui_next/src/components/FormField/FormSubmitError.test.jsx

@@ -21,7 +21,7 @@ describe('<FormSubmitError>', () => {
       },
     };
     const wrapper = mountWithContexts(
-      <Formik>
+      <Formik initialValues={{ name: '' }}>
         {({ errors }) => (
           <div>
             <p>{errors.name}</p>
@@ -52,30 +52,4 @@ describe('<FormSubmitError>', () => {
     expect(global.console.error).toHaveBeenCalledWith(error);
     global.console = realConsole;
   });
-
-  test('should display error message if field error is nested', async () => {
-    const error = {
-      response: {
-        data: {
-          name: 'There was an error with name',
-          inputs: {
-            url: 'Error with url',
-          },
-        },
-      },
-    };
-    let wrapper;
-    await act(async () => {
-      wrapper = mountWithContexts(
-        <Formik>{() => <FormSubmitError error={error} />}</Formik>
-      );
-    });
-    wrapper.update();
-    expect(
-      wrapper.find('Alert').contains(<div>There was an error with name</div>)
-    ).toEqual(true);
-    expect(wrapper.find('Alert').contains(<div>Error with url</div>)).toEqual(
-      true
-    );
-  });
 });

awx/ui_next/src/components/FormField/sortErrorMessages.js

@@ -0,0 +1,58 @@
+export default function sortErrorMessages(error, formValues = {}) {
+  if (!error) {
+    return {};
+  }
+
+  if (
+    error?.response?.data &&
+    typeof error.response.data === 'object' &&
+    Object.keys(error.response.data).length > 0
+  ) {
+    const parsed = parseFieldErrors(error.response.data, formValues);
+    return {
+      formError: parsed.formErrors.join('; '),
+      fieldErrors: Object.keys(parsed.fieldErrors).length
+        ? parsed.fieldErrors
+        : null,
+    };
+  }
+  /* eslint-disable-next-line no-console */
+  console.error(error);
+  return {
+    formError: error.message,
+    fieldErrors: null,
+  };
+}
+
+// Recursively traverse field errors object and build up field/form errors
+function parseFieldErrors(obj, formValues) {
+  let fieldErrors = {};
+  let formErrors = [];
+  Object.keys(obj).forEach(key => {
+    const value = obj[key];
+    if (typeof value === 'string') {
+      if (typeof formValues[key] === 'undefined') {
+        formErrors.push(value);
+      } else {
+        fieldErrors[key] = value;
+      }
+    } else if (Array.isArray(value)) {
+      if (typeof formValues[key] === 'undefined') {
+        formErrors = formErrors.concat(value);
+      } else {
+        fieldErrors[key] = value.join('; ');
+      }
+    } else if (typeof value === 'object') {
+      const parsed = parseFieldErrors(value, formValues[key] || {});
+      if (Object.keys(parsed.fieldErrors).length) {
+        fieldErrors = {
+          ...fieldErrors,
+          [key]: parsed.fieldErrors,
+        };
+      }
+      formErrors = formErrors.concat(parsed.formErrors);
+    }
+  });
+
+  return { fieldErrors, formErrors };
+}

awx/ui_next/src/components/FormField/sortErrorMessages.test.js

@@ -0,0 +1,146 @@
+import sortErrorMessages from './sortErrorMessages';
+
+describe('sortErrorMessages', () => {
+  let consoleError;
+  beforeEach(() => {
+    // Component logs errors to console. Hide those during testing.
+    consoleError = global.console.error;
+    global.console.error = () => {};
+  });
+
+  afterEach(() => {
+    global.console.error = consoleError;
+  });
+
+  test('should give general error message', () => {
+    const error = {
+      message: 'An error occurred',
+    };
+    const parsed = sortErrorMessages(error);
+
+    expect(parsed).toEqual({
+      formError: 'An error occurred',
+      fieldErrors: null,
+    });
+  });
+
+  test('should give field error messages', () => {
+    const error = {
+      response: {
+        data: {
+          foo: 'bar',
+          baz: 'bam',
+        },
+      },
+    };
+    const parsed = sortErrorMessages(error, { foo: '', baz: '' });
+    expect(parsed).toEqual({
+      formError: '',
+      fieldErrors: {
+        foo: 'bar',
+        baz: 'bam',
+      },
+    });
+  });
+
+  test('should give form error for nonexistent field', () => {
+    const error = {
+      response: {
+        data: {
+          alpha: 'oopsie',
+          baz: 'bam',
+        },
+      },
+    };
+    const parsed = sortErrorMessages(error, { foo: '', baz: '' });
+    expect(parsed).toEqual({
+      formError: 'oopsie',
+      fieldErrors: {
+        baz: 'bam',
+      },
+    });
+  });
+
+  test('should join multiple field error messages', () => {
+    const error = {
+      response: {
+        data: {
+          foo: ['bar', 'bar2'],
+          baz: 'bam',
+        },
+      },
+    };
+    const parsed = sortErrorMessages(error, { foo: '', baz: '' });
+    expect(parsed).toEqual({
+      formError: '',
+      fieldErrors: {
+        foo: 'bar; bar2',
+        baz: 'bam',
+      },
+    });
+  });
+
+  test('should give nested field error messages', () => {
+    const error = {
+      response: {
+        data: {
+          inputs: {
+            url: ['URL Error'],
+            other: {
+              stuff: ['Other stuff error'],
+            },
+          },
+        },
+      },
+    };
+    const formValues = {
+      inputs: {
+        url: '',
+        other: {
+          stuff: '',
+        },
+      },
+    };
+    const parsed = sortErrorMessages(error, formValues);
+    expect(parsed).toEqual({
+      formError: '',
+      fieldErrors: {
+        inputs: {
+          url: 'URL Error',
+          other: {
+            stuff: 'Other stuff error',
+          },
+        },
+      },
+    });
+  });
+
+  test('should give unknown nested field error as form error', () => {
+    const error = {
+      response: {
+        data: {
+          inputs: {
+            url: ['URL Error'],
+            other: {
+              stuff: ['Other stuff error'],
+            },
+          },
+        },
+      },
+    };
+    const formValues = {
+      inputs: {
+        url: '',
+      },
+    };
+    const parsed = sortErrorMessages(error, formValues);
+    expect(parsed).toEqual({
+      formError: 'Other stuff error',
+      fieldErrors: {
+        inputs: {
+          url: 'URL Error',
+        },
+      },
+    });
+  });
+});