From 839a421f0bf05141bb3c45f0bf4a3e60c1d8d1ab Mon Sep 17 00:00:00 2001 From: Yury Saukou Date: Thu, 3 Oct 2024 16:00:35 +0400 Subject: [PATCH 1/3] UIOR-1281 Applied 'Strategies' approach for error handling of order updates --- CHANGELOG.md | 1 + .../order/handleErrorsStrategies/index.js | 3 + .../noBudgetForFiscalYearStrategy.js | 20 +++++++ .../noExpenseClassesStrategy.js | 14 +++++ .../restrictedLocationViolationStrategy.js | 13 +++++ .../Utils/order/showUpdateOrderError.js | 58 +++++++------------ translations/ui-orders/en.json | 2 +- 7 files changed, 72 insertions(+), 39 deletions(-) create mode 100644 src/components/Utils/order/handleErrorsStrategies/index.js create mode 100644 src/components/Utils/order/handleErrorsStrategies/noBudgetForFiscalYearStrategy.js create mode 100644 src/components/Utils/order/handleErrorsStrategies/noExpenseClassesStrategy.js create mode 100644 src/components/Utils/order/handleErrorsStrategies/restrictedLocationViolationStrategy.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 15ab4a2b8..1e408fd17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ * ECS - handle error message appears when user without appropriate affiliation is trying to edit/open POL with location in such affiliation. Refs UIOR-1307. * UX: Removal of hyphen in Reexport and add refresh icon. UIOR-1246. * Improve the error message when attempting to open an order when no budget for the fiscal year exists for the fund specified in "Fund distribution". Refs UIOR-1313. +* Applied "Strategies" approach for error handling of order updates. Refs UIOR-1281. ## [6.0.4](https://github.com/folio-org/ui-orders/tree/v6.0.4) (2024-04-25) [Full Changelog](https://github.com/folio-org/ui-orders/compare/v6.0.3...v6.0.4) diff --git a/src/components/Utils/order/handleErrorsStrategies/index.js b/src/components/Utils/order/handleErrorsStrategies/index.js new file mode 100644 index 000000000..7044250db --- /dev/null +++ b/src/components/Utils/order/handleErrorsStrategies/index.js @@ -0,0 +1,3 @@ +export { noBudgetForFiscalYearStrategy } from './noBudgetForFiscalYearStrategy'; +export { noExpenseClassesStrategy } from './noExpenseClassesStrategy'; +export { restrictedLocationViolationStrategy } from './restrictedLocationViolationStrategy'; diff --git a/src/components/Utils/order/handleErrorsStrategies/noBudgetForFiscalYearStrategy.js b/src/components/Utils/order/handleErrorsStrategies/noBudgetForFiscalYearStrategy.js new file mode 100644 index 000000000..42aff9066 --- /dev/null +++ b/src/components/Utils/order/handleErrorsStrategies/noBudgetForFiscalYearStrategy.js @@ -0,0 +1,20 @@ +import { ERROR_CODES } from '../../../../common/constants'; + +export const noBudgetForFiscalYearStrategy = ({ callout }) => { + const handle = (errorsContainer) => { + const error = errorsContainer.getError(); + const fundCodes = error.getParameter('fundCodes'); + const fiscalYearCode = error.getParameter('fiscalYearCode'); + + callout.sendCallout({ + messageId: `ui-orders.errors.${ERROR_CODES[error.code]}`, + type: 'error', + values: { + fiscalYearCode, + fundCodes: JSON.parse(fundCodes)?.join(', '), + }, + }); + }; + + return { handle }; +}; diff --git a/src/components/Utils/order/handleErrorsStrategies/noExpenseClassesStrategy.js b/src/components/Utils/order/handleErrorsStrategies/noExpenseClassesStrategy.js new file mode 100644 index 000000000..793a888ce --- /dev/null +++ b/src/components/Utils/order/handleErrorsStrategies/noExpenseClassesStrategy.js @@ -0,0 +1,14 @@ +export const noExpenseClassesStrategy = ({ callout }) => { + const handle = (errorsContainer) => { + const fundCode = errorsContainer.getError().getParameter('fundCode'); + const expenseClassName = errorsContainer.getError().getParameter('expenseClassName'); + + callout.sendCallout({ + messageId: `ui-orders.errors.${errorsContainer.code}`, + type: 'error', + values: { fundCode, expenseClassName }, + }); + }; + + return { handle }; +}; diff --git a/src/components/Utils/order/handleErrorsStrategies/restrictedLocationViolationStrategy.js b/src/components/Utils/order/handleErrorsStrategies/restrictedLocationViolationStrategy.js new file mode 100644 index 000000000..bbea7f242 --- /dev/null +++ b/src/components/Utils/order/handleErrorsStrategies/restrictedLocationViolationStrategy.js @@ -0,0 +1,13 @@ +export const restrictedLocationViolationStrategy = ({ callout }) => { + const handle = (errorsContainer) => { + const polNumber = errorsContainer.getError().getParameter('poLineNumber'); + + callout.sendCallout({ + messageId: 'ui-orders.errors.openOrder.fundLocationRestrictionViolation', + type: 'error', + values: { polNumber }, + }); + }; + + return { handle }; +}; diff --git a/src/components/Utils/order/showUpdateOrderError.js b/src/components/Utils/order/showUpdateOrderError.js index d99fafe06..1af38df45 100644 --- a/src/components/Utils/order/showUpdateOrderError.js +++ b/src/components/Utils/order/showUpdateOrderError.js @@ -1,13 +1,20 @@ +import get from 'lodash/get'; import React from 'react'; import { FormattedMessage } from 'react-intl'; -import { get } from 'lodash'; + +import { ResponseErrorsContainer } from '@folio/stripes-acq-components'; import { ERROR_CODES } from '../../../common/constants'; +import { + noBudgetForFiscalYearStrategy, + noExpenseClassesStrategy, + restrictedLocationViolationStrategy, +} from './handleErrorsStrategies'; const POL_NUMBER_KEY = 'poLineNumber'; -const showMessage = (callout, code, error, path) => { - const title = get(error, 'errors.0.parameters.0.value', ''); +const showMessage = (callout, code, errors, path) => { + const title = get(errors.getError().parameters, '0.value', ''); callout.sendCallout({ type: 'error', @@ -28,15 +35,9 @@ const showUpdateOrderError = async ( genericCode = ERROR_CODES.orderGenericError1, toggleDeletePieces = null, ) => { - let error; - - try { - error = await response.clone().json(); - } catch (parsingException) { - error = response; - } + const { handler } = await ResponseErrorsContainer.create(response); - const errorCode = get(error, 'errors.0.code'); + const errorCode = handler.getError().code; const code = get(ERROR_CODES, errorCode, genericCode); switch (code) { @@ -49,7 +50,7 @@ const showUpdateOrderError = async ( case ERROR_CODES.accessProviderIsInactive: case ERROR_CODES.accessProviderNotFound: { let errors = - get(error, 'errors.0.parameters', []) + handler.getError().parameters .filter(({ key }) => key === POL_NUMBER_KEY) .map(({ value }) => ({ code, poLineNumber: value })); @@ -62,52 +63,33 @@ const showUpdateOrderError = async ( break; } case ERROR_CODES.missingInstanceStatus: { - showMessage(callout, code, error, 'instanceStatusTypes'); + showMessage(callout, code, handler, 'instanceStatusTypes'); break; } case ERROR_CODES.missingInstanceType: { - showMessage(callout, code, error, 'resourcetypes'); + showMessage(callout, code, handler, 'resourcetypes'); break; } case ERROR_CODES.missingLoanType: { - showMessage(callout, code, error, 'loantypes'); + showMessage(callout, code, handler, 'loantypes'); break; } case ERROR_CODES.budgetExpenseClassNotFound: { - const fundCode = error?.errors?.[0]?.parameters?.find(({ key }) => key === 'fundCode')?.value; - const expenseClassName = error?.errors?.[0]?.parameters?.find(({ key }) => key === 'expenseClassName')?.value; - - callout.sendCallout({ - messageId: `ui-orders.errors.${code}`, - type: 'error', - values: { fundCode, expenseClassName }, - }); + handler.handle(noExpenseClassesStrategy({ callout })); break; } case ERROR_CODES.fundCannotBePaid: { - const fundCodes = error?.errors?.[0]?.parameters?.find(({ key }) => key === 'finance.funds')?.value; + const fundCodes = handler.getError().getParameter('finance.funds'); callout.sendCallout({ messageId: `ui-orders.errors.${ERROR_CODES[code]}`, type: 'error', values: { fundCodes } }); break; } case ERROR_CODES.fundLocationRestrictionViolation: { - const polNumber = error?.errors?.[0]?.parameters?.find(({ key }) => key === 'poLineNumber')?.value; - - callout.sendCallout({ - messageId: 'ui-orders.errors.openOrder.fundLocationRestrictionViolation', - type: 'error', - values: { polNumber }, - }); + handler.handle(restrictedLocationViolationStrategy({ callout })); break; } case ERROR_CODES.budgetNotFoundForFiscalYear: { - const fundCodes = error?.errors?.[0]?.parameters?.find(({ key }) => key === 'fundCodes')?.value; - - callout.sendCallout({ - messageId: `ui-orders.errors.${ERROR_CODES[code]}`, - type: 'error', - values: { fundCodes: JSON.parse(fundCodes)?.join(', ') }, - }); + handler.handle(noBudgetForFiscalYearStrategy({ callout })); break; } default: { diff --git a/translations/ui-orders/en.json b/translations/ui-orders/en.json index ea5b2e483..605a1ada2 100644 --- a/translations/ui-orders/en.json +++ b/translations/ui-orders/en.json @@ -145,7 +145,7 @@ "errors.accessProviderNotFound": "POL {poLineNumber} is using an Access Provider that is inactive or has been removed, please select an active Access Provider to continue.", "errors.budgetExpenseClassNotFound": "{expenseClassName} expense class not found on {fundCode} Fund", "errors.budgetIsInactive": "Cannot create encumbrance from the not active budget", - "errors.budgetNotFoundForFiscalYear": "Order cannot be opened because there is no current budget for fund {fundCodes}.", + "errors.budgetNotFoundForFiscalYear": "Order cannot be opened because there is no current budget for fund(s) {fundCodes} for fiscal year {fiscalYearCode}.", "errors.budgetNotFoundForTransaction": "One or more fund distribution(s) on this order can not be encumbered, because there is no current budget", "errors.clone.error": "The purchase order was not cloned", "errors.costAdditionalCostInvalid": "Cost's additional cost must be positive number", From cdf478d3bc99ebad593a0c2fd7e0ce419b61aa16 Mon Sep 17 00:00:00 2001 From: Yury Saukou Date: Thu, 3 Oct 2024 16:17:02 +0400 Subject: [PATCH 2/3] properly handle JSON parsing --- .../noBudgetForFiscalYearStrategy.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/components/Utils/order/handleErrorsStrategies/noBudgetForFiscalYearStrategy.js b/src/components/Utils/order/handleErrorsStrategies/noBudgetForFiscalYearStrategy.js index 42aff9066..708df4c1a 100644 --- a/src/components/Utils/order/handleErrorsStrategies/noBudgetForFiscalYearStrategy.js +++ b/src/components/Utils/order/handleErrorsStrategies/noBudgetForFiscalYearStrategy.js @@ -2,16 +2,22 @@ import { ERROR_CODES } from '../../../../common/constants'; export const noBudgetForFiscalYearStrategy = ({ callout }) => { const handle = (errorsContainer) => { + let fundCodes; const error = errorsContainer.getError(); - const fundCodes = error.getParameter('fundCodes'); const fiscalYearCode = error.getParameter('fiscalYearCode'); + try { + fundCodes = JSON.parse(error.getParameter('fundCodes')).join(', '); + } catch { + fundCodes = error.getParameter('fundCodes'); + } + callout.sendCallout({ messageId: `ui-orders.errors.${ERROR_CODES[error.code]}`, type: 'error', values: { fiscalYearCode, - fundCodes: JSON.parse(fundCodes)?.join(', '), + fundCodes, }, }); }; From 54e7953a1b00d230ee7c373333d7cb0b01a55a12 Mon Sep 17 00:00:00 2001 From: Yury Saukou Date: Thu, 3 Oct 2024 17:31:08 +0400 Subject: [PATCH 3/3] update tests --- .../Utils/order/showUpdateOrderError.test.js | 104 +++++++++--------- 1 file changed, 50 insertions(+), 54 deletions(-) diff --git a/src/components/Utils/order/showUpdateOrderError.test.js b/src/components/Utils/order/showUpdateOrderError.test.js index e7eef5df1..317ebf964 100644 --- a/src/components/Utils/order/showUpdateOrderError.test.js +++ b/src/components/Utils/order/showUpdateOrderError.test.js @@ -1,11 +1,21 @@ import showUpdateOrderError from './showUpdateOrderError'; +const getResponseMock = ({ code, message, parameters }) => { + return { + clone: jest.fn().mockReturnThis(), + json: jest.fn().mockResolvedValue({ + errors: [{ + code, + parameters, + message, + }], + }), + status: 400, + }; +}; + const params = { - response: { - errors: [{ - code: 'errorCode', - }], - }, + response: getResponseMock({ code: 'genericError' }), callout: { sendCallout: jest.fn(), }, @@ -13,83 +23,69 @@ const params = { }; describe('showUpdateOrderError', () => { - it('should handle error and open modal', () => { - const response = { - errors: [{ - code: 'vendorIsInactive', - }], - }; + it('should handle error and open modal', async () => { + const response = getResponseMock({ code: 'vendorIsInactive' }); - showUpdateOrderError(response, params.callout, params.openModal); + await showUpdateOrderError(response, params.callout, params.openModal); expect(params.openModal).toHaveBeenCalled(); }); - it('should handle error and show message', () => { - const response = { - errors: [{ - code: 'missingInstanceStatus', - }], - }; + it('should handle error and show message', async () => { + const response = getResponseMock({ code: 'missingInstanceStatus' }); - showUpdateOrderError(response, params.callout, params.openModal); + await showUpdateOrderError(response, params.callout, params.openModal); expect(params.callout.sendCallout).toHaveBeenCalled(); }); - it('should handle `fundLocationRestrictionViolation` error and show message', () => { - const response = { - errors: [{ - code: 'fundLocationRestrictionViolation', - parameters: [{ - key: 'poLineNumber', - value: 'value', - }], + it('should handle `fundLocationRestrictionViolation` error and show message', async () => { + const response = getResponseMock({ + code: 'fundLocationRestrictionViolation', + parameters: [{ + key: 'poLineNumber', + value: 'value', }], - }; + }); - showUpdateOrderError(response, params.callout, params.openModal); + await showUpdateOrderError(response, params.callout, params.openModal); expect(params.callout.sendCallout).toHaveBeenCalled(); }); - it('should handle `budgetExpenseClassNotFound` error and show message', () => { - const response = { - errors: [{ - code: 'budgetExpenseClassNotFound', - parameters: [{ - key: 'fundCode', - value: 'value', - }, { - key: 'expenseClassName', - value: 'value', - }], + it('should handle `budgetExpenseClassNotFound` error and show message', async () => { + const response = getResponseMock({ + code: 'budgetExpenseClassNotFound', + parameters: [{ + key: 'fundCode', + value: 'value', + }, { + key: 'expenseClassName', + value: 'value', }], - }; + }); - showUpdateOrderError(response, params.callout, params.openModal); + await showUpdateOrderError(response, params.callout, params.openModal); expect(params.callout.sendCallout).toHaveBeenCalled(); }); - it('should handle `budgetNotFoundForFiscalYear` error and show message', () => { - const response = { - errors: [{ - code: 'budgetNotFoundForFiscalYear', - parameters: [{ - key: 'fundCodes', - value: '[1,2]', - }], + it('should handle `budgetNotFoundForFiscalYear` error and show message', async () => { + const response = getResponseMock({ + code: 'budgetNotFoundForFiscalYear', + parameters: [{ + key: 'fundCodes', + value: '[1,2]', }], - }; + }); - showUpdateOrderError(response, params.callout, params.openModal); + await showUpdateOrderError(response, params.callout, params.openModal); expect(params.callout.sendCallout).toHaveBeenCalled(); }); - it('should handle default error case', () => { - showUpdateOrderError(...Object.values(params)); + it('should handle default error case', async () => { + await showUpdateOrderError(...Object.values(params)); expect(params.callout.sendCallout).toHaveBeenCalled(); });