From 52f37242fcc961a439720c453a400e41d921f927 Mon Sep 17 00:00:00 2001
From: Keith Grant <keithjgrant@redhat.com>
Date: Tue, 6 Oct 2020 14:48:23 -0700
Subject: [PATCH] clean up & unit test form error handling

---
 .../components/FormField/FormSubmitError.jsx  | 57 ++-----------
 .../FormField/FormSubmitError.test.jsx        | 28 +------
 .../components/FormField/sortErrorMessages.js | 35 ++++++++
 .../FormField/sortErrorMessages.test.js       | 81 +++++++++++++++++++
 4 files changed, 125 insertions(+), 76 deletions(-)
 create mode 100644 awx/ui_next/src/components/FormField/sortErrorMessages.js
 create mode 100644 awx/ui_next/src/components/FormField/sortErrorMessages.test.js

diff --git a/awx/ui_next/src/components/FormField/FormSubmitError.jsx b/awx/ui_next/src/components/FormField/FormSubmitError.jsx
index 2453720246..67efb31e02 100644
--- a/awx/ui_next/src/components/FormField/FormSubmitError.jsx
+++ b/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';
-
-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;
-};
+import sortErrorMessages from './sortErrorMessages';
 
 function FormSubmitError({ error }) {
   const [errorMessage, setErrorMessage] = useState(null);
-  const { setErrors } = useFormikContext();
+  const { values, setErrors } = useFormikContext();
 
   useEffect(() => {
-    if (!error) {
-      return;
+    const { formError, fieldErrors } = sortErrorMessages(error, values);
+    if (formError) {
+      setErrorMessage(formError);
     }
-    if (
-      error?.response?.data &&
-      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);
+    if (fieldErrors) {
+      setErrors(fieldErrors);
     }
-  }, [error, setErrors]);
+  }, [error, setErrors, values]);
 
   if (!errorMessage) {
     return null;
diff --git a/awx/ui_next/src/components/FormField/FormSubmitError.test.jsx b/awx/ui_next/src/components/FormField/FormSubmitError.test.jsx
index 30656b4d62..86c6f39d8d 100644
--- a/awx/ui_next/src/components/FormField/FormSubmitError.test.jsx
+++ b/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
-    );
-  });
 });
diff --git a/awx/ui_next/src/components/FormField/sortErrorMessages.js b/awx/ui_next/src/components/FormField/sortErrorMessages.js
new file mode 100644
index 0000000000..dc1cd999cd
--- /dev/null
+++ b/awx/ui_next/src/components/FormField/sortErrorMessages.js
@@ -0,0 +1,35 @@
+export default function sortErrorMessages(error, formValues = {}) {
+  if (!error) {
+    return {};
+  }
+
+  const fieldErrors = {};
+  let formErrors = [];
+  if (
+    error?.response?.data &&
+    typeof error.response.data === 'object' &&
+    Object.keys(error.response.data).length > 0
+  ) {
+    Object.keys(error.response.data).forEach(fieldName => {
+      const errors = error.response.data[fieldName];
+      if (!errors) {
+        return;
+      }
+      const errorsArray = Array.isArray(errors) ? errors : [errors];
+      if (typeof formValues[fieldName] === 'undefined') {
+        formErrors = [...formErrors, ...errorsArray];
+      } else {
+        fieldErrors[fieldName] = errorsArray.join('; ');
+      }
+    });
+  } else {
+    /* eslint-disable-next-line no-console */
+    console.error(error);
+    formErrors.push(error.message);
+  }
+
+  return {
+    formError: formErrors.join('; '),
+    fieldErrors: Object.keys(fieldErrors).length ? fieldErrors : null,
+  };
+}
diff --git a/awx/ui_next/src/components/FormField/sortErrorMessages.test.js b/awx/ui_next/src/components/FormField/sortErrorMessages.test.js
new file mode 100644
index 0000000000..c16ed7cc9a
--- /dev/null
+++ b/awx/ui_next/src/components/FormField/sortErrorMessages.test.js
@@ -0,0 +1,81 @@
+import sortErrorMessages from './sortErrorMessages';
+
+describe('sortErrorMessages', () => {
+  let consoleError;
+  beforeEach(() => {
+    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',
+      },
+    });
+  });
+});