From c17fa4983e6baff968953e0e40c5139f71e98c0a Mon Sep 17 00:00:00 2001 From: Yury Saukou Date: Tue, 22 Nov 2022 14:36:02 +0400 Subject: [PATCH] UIOR-1038 Some shortcut keys do not work in the "Orders" app (#1424) * UIOR-1038 fix 'new order' and 'duplicate order' shortcuts * UIOR-1038 fix toggle handling of 'ongoing order information' accordion * UIOR-1038 fix accordion toggle handling in POLineForm * UIOR-1038 async validation of field-arrays always set Promise as error in form and prevents to close accordion * UIOR-1038 update changelog * add unit test for 'new' shortcut --- CHANGELOG.md | 1 + src/OrdersList/OrdersList.js | 2 +- src/OrdersList/OrdersList.test.js | 21 +++++++- src/common/utils/index.js | 1 + .../utils/omitFieldArraysAsyncErrors.js | 27 ++++++++++ .../utils/omitFieldArraysAsyncErrors.test.js | 49 +++++++++++++++++++ src/components/POLine/POLineForm.js | 6 ++- src/components/POLine/POLineView.js | 1 + src/components/POLine/const.js | 3 +- src/components/PurchaseOrder/PO.js | 4 +- .../OrderTemplatesEditor.js | 5 +- 11 files changed, 112 insertions(+), 8 deletions(-) create mode 100644 src/common/utils/omitFieldArraysAsyncErrors.js create mode 100644 src/common/utils/omitFieldArraysAsyncErrors.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 16f664851..b8a056933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Use Orders Export History API (mod-orders). Refs UIOR-1034. * Provide local translations to `ControlledVocab`. Refs UIOR-1018. +* Some shortcut keys do not work in the "Orders" application. Refs UIOR-1038. ## [3.3.0](https://github.com/folio-org/ui-orders/tree/v3.3.0) (2022-10-27) [Full Changelog](https://github.com/folio-org/ui-orders/compare/v3.2.2...v3.3.0) diff --git a/src/OrdersList/OrdersList.js b/src/OrdersList/OrdersList.js index 2eece390f..8f4ee0c5b 100644 --- a/src/OrdersList/OrdersList.js +++ b/src/OrdersList/OrdersList.js @@ -168,7 +168,7 @@ function OrdersList({ { name: 'new', handler: handleKeyCommand(() => { - if (stripes.hasPerm('ui-orders.order.create')) { + if (stripes.hasPerm('ui-orders.orders.create')) { history.push('/orders/create'); } }), diff --git a/src/OrdersList/OrdersList.test.js b/src/OrdersList/OrdersList.test.js index b1e56eebb..113ae5dea 100644 --- a/src/OrdersList/OrdersList.test.js +++ b/src/OrdersList/OrdersList.test.js @@ -1,5 +1,5 @@ import React from 'react'; -import { render, screen } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; import { HasCommand } from '@folio/stripes/components'; @@ -22,6 +22,11 @@ const mockLocalStorageFilters = { searchIndex: '', }; +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + // eslint-disable-next-line react/prop-types + withRouter: (Component) => (props) => , +})); jest.mock('@folio/stripes/smart-components', () => ({ ...jest.requireActual('@folio/stripes/smart-components'), // eslint-disable-next-line react/prop-types @@ -50,6 +55,9 @@ const defaultProps = { resetData: jest.fn(), refreshList: jest.fn(), history, + location: { + search: '', + }, }; const renderOrdersList = (props = {}) => render( @@ -62,6 +70,7 @@ const renderOrdersList = (props = {}) => render( describe('OrdersList', () => { beforeEach(() => { + defaultProps.history.push.mockClear(); HasCommand.mockClear(); useModalToggle.mockClear(); }); @@ -99,6 +108,16 @@ describe('OrdersList', () => { expect(screen.getByText('OrderExportSettingsModalContainer')).toBeInTheDocument(); }); }); + + describe('shortcuts', () => { + it('should handle \'new\' shortcut', async () => { + renderOrdersList(); + + await act(async () => HasCommand.mock.calls[0][0].commands.find(c => c.name === 'new').handler()); + + expect(history.push).toHaveBeenCalled(); + }); + }); }); describe('resultsFormatter', () => { diff --git a/src/common/utils/index.js b/src/common/utils/index.js index 319a2dfa9..aa1cbef8c 100644 --- a/src/common/utils/index.js +++ b/src/common/utils/index.js @@ -8,4 +8,5 @@ export * from './getUserNameById'; export * from './getExportAccountNumbers'; export * from './getRecordMap'; export * from './fetchExportDataByIds'; +export * from './omitFieldArraysAsyncErrors'; export * from './validateDuplicateLines'; diff --git a/src/common/utils/omitFieldArraysAsyncErrors.js b/src/common/utils/omitFieldArraysAsyncErrors.js new file mode 100644 index 000000000..de611fdd8 --- /dev/null +++ b/src/common/utils/omitFieldArraysAsyncErrors.js @@ -0,0 +1,27 @@ +import { ARRAY_ERROR } from 'final-form'; +import { + cloneDeep, + get, + unset, +} from 'lodash'; + +/* + Final form async validation of field array itself return a Promise instead of resolved value + and set it as "FINAL_FORM/array-error". So form always contain this promise in form's + errors object even if there no actual validation error. + + Issue: https://github.com/final-form/react-final-form-arrays/issues/176 +*/ +export const omitFieldArraysAsyncErrors = (formErrors, asyncFieldArrays = []) => { + const cloned = cloneDeep(formErrors); + + asyncFieldArrays.forEach((field) => { + const arrayFieldAsyncError = get(formErrors, `${field}[${ARRAY_ERROR}].then`); + + if (arrayFieldAsyncError && !get(formErrors, field, []).filter(Boolean).length) { + unset(cloned, field); + } + }); + + return cloned; +}; diff --git a/src/common/utils/omitFieldArraysAsyncErrors.test.js b/src/common/utils/omitFieldArraysAsyncErrors.test.js new file mode 100644 index 000000000..dd60e0beb --- /dev/null +++ b/src/common/utils/omitFieldArraysAsyncErrors.test.js @@ -0,0 +1,49 @@ +import { ARRAY_ERROR } from 'final-form'; + +import { omitFieldArraysAsyncErrors } from './omitFieldArraysAsyncErrors'; + +const ASYNC_FIELD_ARRAY = 'fieldArrayWithAsyncValidation'; + +describe('omitFieldArraysAsyncErrors', () => { + it('should omit field-array from form errors if it contains only async error of array itself', () => { + const fieldArrayErrors = []; + + fieldArrayErrors[ARRAY_ERROR] = Promise.resolve(null); + + const formErrors = { + [ASYNC_FIELD_ARRAY]: fieldArrayErrors, + }; + + const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + + expect(errors).toEqual({}); + }); + + it('should keep field-array in form errors object if it contains sync error of array itself', () => { + const fieldArrayErrors = []; + + fieldArrayErrors[ARRAY_ERROR] = 'Invalid array'; + + const formErrors = { + [ASYNC_FIELD_ARRAY]: fieldArrayErrors, + }; + + const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + + expect(ASYNC_FIELD_ARRAY in errors).toBeTruthy(); + }); + + it('should keep field-array in form errors object if it contains its fields\' errors', () => { + const fieldArrayErrors = ['Test field is invalid']; + + fieldArrayErrors[ARRAY_ERROR] = Promise.resolve(null); + + const formErrors = { + [ASYNC_FIELD_ARRAY]: fieldArrayErrors, + }; + + const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + + expect(ASYNC_FIELD_ARRAY in errors).toBeTruthy(); + }); +}); diff --git a/src/components/POLine/POLineForm.js b/src/components/POLine/POLineForm.js index 2dc520bec..1cd0aeede 100644 --- a/src/components/POLine/POLineForm.js +++ b/src/components/POLine/POLineForm.js @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import PropTypes from 'prop-types'; import { get, mapValues, pick } from 'lodash'; import { FormattedMessage } from 'react-intl'; @@ -39,6 +39,7 @@ import { isOtherResource, } from '../../common/POLFields'; import { isOngoing } from '../../common/POFields'; +import { omitFieldArraysAsyncErrors } from '../../common/utils'; import LocationForm from './Location/LocationForm'; import { EresourcesForm } from './Eresources'; import { PhysicalForm } from './Physical'; @@ -276,7 +277,8 @@ function POLineForm({ ); }; - const errors = form.getState()?.errors; + const formErrors = form.getState()?.errors; + const errors = useMemo(() => omitFieldArraysAsyncErrors(formErrors, ['fundDistribution']), [formErrors]); const [ expandAll, diff --git a/src/components/POLine/POLineView.js b/src/components/POLine/POLineView.js index 3be9d4ea3..4afe8833c 100644 --- a/src/components/POLine/POLineView.js +++ b/src/components/POLine/POLineView.js @@ -124,6 +124,7 @@ const POLineView = ({ [ACCORDION_ID.poLine]: true, [ACCORDION_ID.linkedInstances]: false, [ACCORDION_ID.exportDetails]: false, + [ACCORDION_ID.ongoingOrder]: true, }); const [showConfirmDelete, toggleConfirmDelete] = useModalToggle(); const [showConfirmCancel, toggleConfirmCancel] = useModalToggle(); diff --git a/src/components/POLine/const.js b/src/components/POLine/const.js index b57d88935..b7c8d9e90 100644 --- a/src/components/POLine/const.js +++ b/src/components/POLine/const.js @@ -32,6 +32,7 @@ export const MAP_FIELD_ACCORDION = { details: ACCORDION_ID.itemDetails, eresource: ACCORDION_ID.eresources, fundDistribution: ACCORDION_ID.fundDistribution, + 'fundDistribution-error': ACCORDION_ID.fundDistribution, locations: ACCORDION_ID.location, orderFormat: ACCORDION_ID.lineDetails, other: ACCORDION_ID.other, @@ -58,6 +59,6 @@ export const POL_TEMPLATE_FIELDS_MAP = { 'tags.tagList': 'polTags.tagList', }; -export const INITIAL_SECTIONS = Object.keys(ACCORDION_ID).reduce( +export const INITIAL_SECTIONS = Object.values(ACCORDION_ID).reduce( (accum, id) => ({ ...accum, [id]: true }), {}, ); diff --git a/src/components/PurchaseOrder/PO.js b/src/components/PurchaseOrder/PO.js index 470a7a333..351f26871 100644 --- a/src/components/PurchaseOrder/PO.js +++ b/src/components/PurchaseOrder/PO.js @@ -648,7 +648,7 @@ const PO = ({ { name: 'new', handler: handleKeyCommand(() => { - if (stripes.hasPerm('ui-orders.order.create')) { + if (stripes.hasPerm('ui-orders.orders.create')) { history.push('/orders/create'); } }), @@ -666,7 +666,7 @@ const PO = ({ { name: 'duplicateRecord', handler: handleKeyCommand(() => { - if (stripes.hasPerm('ui-orders.order.create')) { + if (stripes.hasPerm('ui-orders.orders.create')) { toggleCloneConfirmation(); } }), diff --git a/src/settings/OrderTemplates/OrderTemplatesEditor/OrderTemplatesEditor.js b/src/settings/OrderTemplates/OrderTemplatesEditor/OrderTemplatesEditor.js index 470c080b8..8fcd1727b 100644 --- a/src/settings/OrderTemplates/OrderTemplatesEditor/OrderTemplatesEditor.js +++ b/src/settings/OrderTemplates/OrderTemplatesEditor/OrderTemplatesEditor.js @@ -1,4 +1,5 @@ import PropTypes from 'prop-types'; +import { useMemo } from 'react'; import { FormattedMessage } from 'react-intl'; import { mapValues } from 'lodash'; @@ -38,6 +39,7 @@ import { } from '../../../common/POLFields'; import { WORKFLOW_STATUS } from '../../../common/constants'; import { useFundDistributionValidation } from '../../../common/hooks'; +import { omitFieldArraysAsyncErrors } from '../../../common/utils'; import { ItemForm } from '../../../components/POLine/Item'; import { CostForm } from '../../../components/POLine/Cost'; import { OngoingOrderForm } from '../../../components/POLine/OngoingOrder'; @@ -82,7 +84,8 @@ const OrderTemplatesEditor = ({ submitting, }) => { const { validateFundDistributionTotal } = useFundDistributionValidation(formValues); - const errors = getState()?.errors; + const formErrors = getState()?.errors; + const errors = useMemo(() => omitFieldArraysAsyncErrors(formErrors, ['fundDistribution']), [formErrors]); const [ expandAll,