Validation global error (#2743)

* global error * Apply suggestions from code review Co-authored-by: Nelson Kopliku <nelson.kopliku@suse.com> * format * Return CVEs from `Errata Details` endpoint (#2733) * Return CVEs from `v1/software_updates/errata_details/:advisory_name` * Create CVE factory * global error * Apply suggestions from code review Co-authored-by: Nelson Kopliku <nelson.kopliku@suse.com> * format --------- Co-authored-by: Nelson Kopliku <nelson.kopliku@suse.com> Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com>
trento-project · Jul 8, 2024 · ae5ba8f · ae5ba8f
1 parent 317de10
commit ae5ba8f
Show file tree

Hide file tree

Showing 7 changed files with 133 additions and 22 deletions.
diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx
@@ -7,7 +7,7 @@ import { InputNumber } from '@common/Input';
 import Select from '@common/Select';
 import Label from '@common/Label';
 
-import { getError } from '@lib/api/validationErrors';
+import { getError, getGlobalError } from '@lib/api/validationErrors';
 
 const defaultErrors = [];
 
@@ -22,9 +22,7 @@ const toRetentionTimeErrorMessage = (errors) =>
     .filter(Boolean)
     .join('; ');
 
-const toGenericErrorMessage = (errors) =>
-  // the first error of type string is considered the generic error
-  errors.find((error) => typeof error === 'string');
+const toGlobalErrorMessage = (errors) => capitalize(getGlobalError(errors));
 
 function TimeSpan({ time: initialTime, error = false, onChange = noop }) {
   const [time, setTime] = useState(initialTime);
@@ -90,7 +88,7 @@ function ActivityLogsSettingsModal({
   const [retentionTime, setRetentionTime] = useState(initialRetentionTime);
 
   const retentionTimeError = toRetentionTimeErrorMessage(errors);
-  const genericError = toGenericErrorMessage(errors);
+  const genericError = toGlobalErrorMessage(errors);
 
   return (
     <Modal

diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.stories.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.stories.jsx
@@ -64,7 +64,7 @@ export const WithCompositeFieldValidationError = {
   },
 };
 
-export const WithGenericError = {
+export const WithGlobalError = {
   args: {
     open: false,
     initialRetentionTime: { value: 1, unit: 'month' },

diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx
@@ -4,6 +4,7 @@ import { render, screen, act } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import '@testing-library/jest-dom';
 
+import { defaultGlobalError } from '@lib/api/validationErrors';
 import ActivityLogsSettingsModal from '.';
 
 const positiveInt = () => faker.number.int({ min: 1 });
@@ -125,8 +126,8 @@ describe('ActivityLogsSettingsModal component', () => {
     ${'unit error'}                        | ${[unitError]}                     | ${unitError.detail}
     ${'value and unit errors (1)'}         | ${[valueError, unitError]}         | ${valueError.detail}
     ${'value and unit errors (2)'}         | ${[valueError, unitError]}         | ${unitError.detail}
-    ${'generic error'}                     | ${['a generic error']}             | ${'a generic error'}
-    ${'generic error and value error (1)'} | ${['a generic error', valueError]} | ${'a generic error'}
+    ${'generic error'}                     | ${['a generic error']}             | ${defaultGlobalError.detail}
+    ${'generic error and value error (1)'} | ${['a generic error', valueError]} | ${defaultGlobalError.detail}
     ${'generic error and value error (2)'} | ${['a generic error', valueError]} | ${valueError.detail}
   `(
     'should display errors on $scenario',

diff --git a/assets/js/lib/api/validationErrors.js b/assets/js/lib/api/validationErrors.js
@@ -1,20 +1,32 @@
 import { flow, first } from 'lodash';
-import { get, filter } from 'lodash/fp';
+import { get, filter, map } from 'lodash/fp';
 
-export const hasError = (keyword, errors) =>
-  errors.some((error) => {
-    const pointer = get(['source', 'pointer'], error);
+const selectField = (keyword) => (error) => {
+  const pointer = get(['source', 'pointer'], error);
 
-    return pointer === `/${keyword}`;
-  });
+  return pointer === `/${keyword}`;
+};
+
+export const hasError = (keyword, errors) => errors.some(selectField(keyword));
 
 export const getError = (keyword, errors) =>
-  flow([
-    filter((error) => {
-      const pointer = get(['source', 'pointer'], error);
+  flow([filter(selectField(keyword)), first, get('detail')])(errors);
 
-      return pointer === `/${keyword}`;
-    }),
+export const defaultGlobalError = {
+  title: 'Unexpected error',
+  detail: 'Something went wrong.',
+};
+
+export const getGlobalError = (errors) =>
+  flow([
+    filter(
+      (error) => !(typeof error === 'object' && error && 'source' in error)
+    ),
+    map((error) =>
+      typeof error === 'object' && error && 'detail' in error
+        ? error
+        : defaultGlobalError
+    ),
     first,
     get('detail'),
   ])(errors);
diff --git a/assets/js/lib/api/validationErrors.test.js b/assets/js/lib/api/validationErrors.test.js
@@ -1,4 +1,9 @@
-import { hasError, getError } from './validationErrors';
+import {
+  hasError,
+  getError,
+  getGlobalError,
+  defaultGlobalError,
+} from './validationErrors';
 
 describe('hasError', () => {
   it('should tell that a list contains an error about a specific field', () => {
@@ -91,3 +96,58 @@ describe('getError', () => {
     expect(getError('url', errors)).toBe(undefined);
   });
 });
+
+describe('getGlobalError', () => {
+  it('should return the first global error', () => {
+    const errors = [
+      {
+        detail: 'a detail',
+        title: 'a title',
+      },
+      {
+        detail: 'another detail',
+        source: { pointer: '/some_field' },
+        title: 'another title',
+      },
+      {
+        detail: 'do not return this detail',
+        title: 'do not return this title',
+      },
+    ];
+
+    expect(getGlobalError(errors)).toBe('a detail');
+  });
+
+  it('should return undefined when there is no global error', () => {
+    const errors = [
+      {
+        detail: "can't be blank",
+        source: { pointer: '/some_value' },
+        title: 'Invalid value',
+      },
+    ];
+
+    expect(getGlobalError(errors)).not.toBeDefined();
+  });
+
+  it('should return undefined when no error', () => {
+    const errors = [];
+
+    expect(getGlobalError(errors)).not.toBeDefined();
+  });
+
+  it.each`
+    input
+    ${{ malformed: true }}
+    ${undefined}
+    ${null}
+    ${'string'}
+    ${1234 /* number */}
+    ${[12, 34] /* array */}
+  `(
+    'should return the default error if the received error is malformed',
+    ({ input }) => {
+      expect(getGlobalError([input])).toBe(defaultGlobalError.detail);
+    }
+  );
+});
diff --git a/assets/js/state/sagas/activityLogsSettings.js b/assets/js/state/sagas/activityLogsSettings.js
@@ -11,6 +11,7 @@ import {
   setEditingActivityLogsSettings,
   setNetworkError,
 } from '@state/activityLogsSettings';
+import { defaultGlobalError } from '@lib/api/validationErrors';
 
 export function* fetchActivityLogsSettings() {
   yield put(startLoadingActivityLogsSettings());
@@ -34,7 +35,7 @@ export function* updateActivityLogsSettings({ payload }) {
     const errors = get(
       error,
       ['response', 'data', 'errors'],
-      ['An error occurred while saving the settings']
+      [defaultGlobalError]
     );
     yield put(setActivityLogsSettingsErrors(errors));
   }

diff --git a/assets/js/state/sagas/activityLogsSettings.test.js b/assets/js/state/sagas/activityLogsSettings.test.js
@@ -13,6 +13,7 @@ import {
   setNetworkError,
 } from '@state/activityLogsSettings';
 
+import { defaultGlobalError } from '@lib/api/validationErrors';
 import {
   fetchActivityLogsSettings,
   updateActivityLogsSettings,
@@ -93,6 +94,44 @@ describe('Activity Logs Settings saga', () => {
       ]);
     });
 
+    it('should have generic errors on update (receiving empty body)', async () => {
+      const axiosMock = new MockAdapter(networkClient);
+      const payload = activityLogsSettingsFactory.build();
+
+      axiosMock.onPut('/settings/activity_log', payload).reply(500);
+
+      const dispatched = await recordSaga(updateActivityLogsSettings, {
+        payload,
+      });
+
+      expect(dispatched).toEqual([
+        startLoadingActivityLogsSettings(),
+        setActivityLogsSettingsErrors([defaultGlobalError]),
+      ]);
+    });
+
+    it('should have generic errors on update', async () => {
+      const axiosMock = new MockAdapter(networkClient);
+      const payload = activityLogsSettingsFactory.build();
+
+      axiosMock.onPut('/settings/activity_log', payload).reply(500, {
+        errors: [
+          { title: 'Internal Server Error', detail: 'Something went wrong.' },
+        ],
+      });
+
+      const dispatched = await recordSaga(updateActivityLogsSettings, {
+        payload,
+      });
+
+      expect(dispatched).toEqual([
+        startLoadingActivityLogsSettings(),
+        setActivityLogsSettingsErrors([
+          { title: 'Internal Server Error', detail: 'Something went wrong.' },
+        ]),
+      ]);
+    });
+
     it.each([403, 404, 500, 502, 504])(
       'should put a network error flag on failed saving',
       async (status) => {
@@ -107,7 +146,7 @@ describe('Activity Logs Settings saga', () => {
 
         expect(dispatched).toEqual([
           startLoadingActivityLogsSettings(),
-          setActivityLogsSettingsErrors([expect.any(String)]),
+          setActivityLogsSettingsErrors([defaultGlobalError]),
         ]);
       }
     );