Fix API callback error handling

* Abstract callback handling into generic dispatchAPICallbackToast * Test dispatchAPICallbackToast
ATIX-AG · Apr 16, 2024 · 42d4a4a · 42d4a4a
1 parent 30341df
commit 42d4a4a
Show file tree

Hide file tree

Showing 7 changed files with 173 additions and 76 deletions.
diff --git a/webpack/api_helper.js b/webpack/api_helper.js
@@ -1,3 +1,5 @@
+import { addToast } from 'foremanReact/components/ToastsList';
+import { translate as __ } from 'foremanReact/common/I18n';
 import { get, put, post } from 'foremanReact/redux/API';
 
 /* API constants */
@@ -64,10 +66,48 @@ const apiUpdateResourceQuota = (
     handleError: response => stateCallback(false, response, componentCallback),
   });
 
+/**
+ * Handles the callback response from an asynchronous operation, displaying a toast message accordingly.
+ * @param {function} dispatcher - The dispatcher function to dispatch actions.
+ * @param {boolean} isSuccess - Indicates whether the operation was successful or not.
+ * @param {object} response - The response object returned from the operation.
+ * @param {string} successMessage - The success message to display in case of success.
+ * @param {string} errorMessage - The error message to display in case of failure.
+ */
+const dispatchAPICallbackToast = (
+  dispatch,
+  isSuccess,
+  response,
+  successMessage,
+  errorMessage
+) => {
+  if (isSuccess) {
+    dispatch(
+      addToast({
+        type: 'success',
+        message: __(successMessage),
+      })
+    );
+  } else {
+    let printErrorMessage = __(errorMessage);
+    /* eslint-disable-next-line camelcase */
+    if (response?.response?.data?.error?.full_messages) {
+      printErrorMessage += __(` ${response.response.data.error.full_messages}`);
+    }
+    dispatch(
+      addToast({
+        type: 'warning',
+        message: printErrorMessage,
+      })
+    );
+  }
+};
+
 export {
   apiListResourceQuotas,
   apiGetResourceQuota,
   apiGetResourceQuotaUtilization,
   apiCreateResourceQuota,
   apiUpdateResourceQuota,
+  dispatchAPICallbackToast,
 };
diff --git a/webpack/api_helper.test.js b/webpack/api_helper.test.js
@@ -0,0 +1,96 @@
+import { dispatchAPICallbackToast } from './api_helper';
+
+jest.mock('foremanReact/components/ToastsList', () => ({
+  addToast: jest.fn(param => param),
+}));
+
+jest.mock('foremanReact/common/I18n', () => ({
+  translate: jest.fn(param => param),
+}));
+
+const dispatch = jest.fn();
+
+describe('dispatchAPICallbackToast', () => {
+  afterEach(() => {
+    jest.clearAllMocks(); // Reset mock calls after each test
+  });
+
+  it('should dispatch a success toast', () => {
+    const isSuccess = true;
+    const response = {};
+    const successMessage = 'Success message';
+    const errorMessage = 'Error message';
+
+    dispatchAPICallbackToast(
+      dispatch,
+      isSuccess,
+      response,
+      successMessage,
+      errorMessage
+    );
+
+    expect(dispatch).toHaveBeenCalledTimes(1);
+    expect(dispatch).toHaveBeenCalledWith({
+      type: 'success',
+      message: 'Success message',
+    });
+  });
+
+  it('should dispatch a warning toast with error', () => {
+    const isSuccess = false;
+    const response = {
+      response: {
+        data: {
+          error: {
+            full_messages: 'Some nice error',
+          },
+        },
+      },
+    };
+    const successMessage = 'Success message';
+    const errorMessage = 'Error message';
+
+    dispatchAPICallbackToast(
+      dispatch,
+      isSuccess,
+      response,
+      successMessage,
+      errorMessage
+    );
+
+    expect(dispatch).toHaveBeenCalledTimes(1);
+    expect(dispatch).toHaveBeenCalledWith({
+      type: 'warning',
+      message: 'Error message Some nice error',
+    });
+  });
+
+  it('should dispatch a warning toast without error', () => {
+    const isSuccess = false;
+    const response = {
+      response: {
+        data: {
+          notTheExpectedErrorFormat: {
+            full_messages: 'Some nice error',
+          },
+        },
+      },
+    };
+    const successMessage = 'Success message';
+    const errorMessage = 'Error message';
+
+    dispatchAPICallbackToast(
+      dispatch,
+      isSuccess,
+      response,
+      successMessage,
+      errorMessage
+    );
+
+    expect(dispatch).toHaveBeenCalledTimes(1);
+    expect(dispatch).toHaveBeenCalledWith({
+      type: 'warning',
+      message: 'Error message',
+    });
+  });
+});
diff --git a/webpack/components/ResourceQuotaForm/ResourceQuotaFormConstants.js b/webpack/components/ResourceQuotaForm/ResourceQuotaFormConstants.js
@@ -20,13 +20,13 @@ export const RESOURCE_NAME_DISK = 'Disk space';
 export const RESOURCE_UNIT_CPU = [{ symbol: 'cores', factor: 1 }];
 export const RESOURCE_UNIT_MEMORY = [
   { symbol: 'MB', factor: 1 },
-  { symbol: 'GB', factor: 1000 },
-  { symbol: 'TB', factor: 1000000 },
+  { symbol: 'GB', factor: 1024 },
+  { symbol: 'TB', factor: 1024 * 1024 },
 ];
 export const RESOURCE_UNIT_DISK = [
   { symbol: 'GB', factor: 1 },
-  { symbol: 'TB', factor: 1000 },
-  { symbol: 'PB', factor: 1000000 },
+  { symbol: 'TB', factor: 1024 },
+  { symbol: 'PB', factor: 1024 * 1024 },
 ];
 
 /* Resource value bounds */

diff --git a/webpack/components/ResourceQuotaForm/components/Properties/TextInputField.js b/webpack/components/ResourceQuotaForm/components/Properties/TextInputField.js
@@ -3,10 +3,10 @@ import { useDispatch } from 'react-redux';
 import PropTypes from 'prop-types';
 
 import { translate as __ } from 'foremanReact/common/I18n';
-import { addToast } from 'foremanReact/components/ToastsList';
 
 import ActionableDetail from '../../../../lib/ActionableDetail';
 import StaticDetail from './StaticDetail';
+import dispatchAPICallbackToast from '../../../../api_helper';
 
 const TextInputField = ({
   initialValue,
@@ -36,23 +36,13 @@ const TextInputField = ({
 
   const callback = (success, response) => {
     setIsLoading(false);
-    if (success) {
-      dispatch(
-        addToast({
-          type: 'success',
-          message: __(`Sucessfully applied ${label}`),
-        })
-      );
-    } else {
-      dispatch(
-        addToast({
-          type: 'warning',
-          message: __(
-            `An error occurred appyling ${label}: ${response.response.data.error.full_messages}`
-          ),
-        })
-      );
-    }
+    dispatchAPICallbackToast(
+      dispatch,
+      success,
+      response,
+      `Sucessfully applied ${label}.`,
+      `An error occurred appyling ${label}.`
+    );
   };
 
   /* Guard setting of isInputValid to prevent re-render reace condition */

diff --git a/webpack/components/ResourceQuotaForm/components/Properties/index.js b/webpack/components/ResourceQuotaForm/components/Properties/index.js
@@ -22,7 +22,7 @@ import ClusterIcon from '@patternfly/react-icons/dist/esm/icons/cluster-icon';
 import SyncAltIcon from '@patternfly/react-icons/dist/esm/icons/sync-alt-icon';
 
 import { translate as __ } from 'foremanReact/common/I18n';
-import { addToast } from 'foremanReact/components/ToastsList';
+import dispatchAPICallbackToast from '../../../../api_helper';
 
 import './Properties.scss';
 import StatusPropertiesLabel from './StatusPropertiesLabel';
@@ -58,23 +58,13 @@ const Properties = ({
 
   const callbackFetch = (success, response) => {
     setIsFetchLoading(false);
-    if (success) {
-      dispatch(
-        addToast({
-          type: 'success',
-          message: __(`Sucessfully fetched latest data.`),
-        })
-      );
-    } else {
-      dispatch(
-        addToast({
-          type: 'warning',
-          message: __(
-            `An error occurred fetching quota information: ${response.response.data.error.full_messages}`
-          ),
-        })
-      );
-    }
+    dispatchAPICallbackToast(
+      dispatch,
+      success,
+      response,
+      `Sucessfully fetched latest data.`,
+      `An error occurred fetching quota information.`
+    );
   };
 
   return (

diff --git a/webpack/components/ResourceQuotaForm/components/Resource/index.js b/webpack/components/ResourceQuotaForm/components/Resource/index.js
@@ -17,7 +17,6 @@ import {
   FormGroup,
 } from '@patternfly/react-core';
 
-import { addToast } from 'foremanReact/components/ToastsList';
 import LabelIcon from 'foremanReact/components/common/LabelIcon';
 import { translate as __ } from 'foremanReact/common/I18n';
 
@@ -26,6 +25,7 @@ import UnitInputField from './UnitInputField';
 import UtilizationProgress from './UtilizationProgress';
 
 import { resourceAttributesByIdentifier } from '../../ResourceQuotaFormConstants';
+import dispatchAPICallbackToast from '../../../../api_helper';
 
 // TODO: Visualize maximum resource (tooltip?)
 // TODO: Add error message if given quota limit exceeds present quota utilization (consumed resources)
@@ -70,23 +70,13 @@ const Resource = ({
       setInputValue(response.data[resourceIdentifier]);
       setIsEnabled(response.data[resourceIdentifier] !== null);
     }
-    if (success) {
-      dispatch(
-        addToast({
-          type: 'success',
-          message: __(`Sucessfully applied ${resourceTitle}`),
-        })
-      );
-    } else {
-      dispatch(
-        addToast({
-          type: 'warning',
-          message: __(
-            `An error occurred appyling ${resourceTitle}: ${response.response.data.error.full_messages}`
-          ),
-        })
-      );
-    }
+    dispatchAPICallbackToast(
+      dispatch,
+      success,
+      response,
+      `Sucessfully applied ${resourceTitle}.`,
+      `An error occurred appyling ${resourceTitle}.`
+    );
   };
 
   /* apply user input changes to state variables */

diff --git a/webpack/components/ResourceQuotaForm/components/Submit.js b/webpack/components/ResourceQuotaForm/components/Submit.js
@@ -4,7 +4,8 @@ import { useDispatch } from 'react-redux';
 import { Button, Flex, FlexItem } from '@patternfly/react-core';
 
 import { translate as __ } from 'foremanReact/common/I18n';
-import { addToast } from 'foremanReact/components/ToastsList';
+
+import dispatchAPICallbackToast from '../../../api_helper';
 
 import {
   RESOURCE_IDENTIFIER_ID,
@@ -26,23 +27,13 @@ const Submit = ({ isValid, onCreate, onSubmit }) => {
 
   const onCreateCallback = (success, response) => {
     setIsSubmitLoading(false);
-    if (success) {
-      dispatch(
-        addToast({
-          type: 'success',
-          message: __(`Sucessfully created new Resource Quota`),
-        })
-      );
-    } else {
-      dispatch(
-        addToast({
-          type: 'danger',
-          message: __(
-            `An error occurred while creating new Resource Quota: ${response.response.data.error.full_messages}`
-          ),
-        })
-      );
-    }
+    dispatchAPICallbackToast(
+      dispatch,
+      success,
+      response,
+      `Sucessfully created new Resource Quota`,
+      `An error occurred while creating new Resource Quota.`
+    );
     if (onSubmit) onSubmit(success);
   };