From 42d4a4a4e937e544e0d46242d393e348fe77fcf2 Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Tue, 20 Feb 2024 17:00:01 +0100 Subject: [PATCH] Fix API callback error handling * Abstract callback handling into generic dispatchAPICallbackToast * Test dispatchAPICallbackToast --- webpack/api_helper.js | 40 ++++++++ webpack/api_helper.test.js | 96 +++++++++++++++++++ .../ResourceQuotaFormConstants.js | 8 +- .../components/Properties/TextInputField.js | 26 ++--- .../components/Properties/index.js | 26 ++--- .../components/Resource/index.js | 26 ++--- .../ResourceQuotaForm/components/Submit.js | 27 ++---- 7 files changed, 173 insertions(+), 76 deletions(-) create mode 100644 webpack/api_helper.test.js diff --git a/webpack/api_helper.js b/webpack/api_helper.js index 083c25f..cc3a6ea 100644 --- 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 new file mode 100644 index 0000000..eb30b64 --- /dev/null +++ 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 index d5d4465..af9e9a9 100644 --- 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 index 976d4c3..017b30a 100644 --- 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 index dfbae1f..93ae871 100644 --- 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 index 9bf8f52..8db30e0 100644 --- 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 index df2c5db..8918620 100644 --- 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); };