Skip to content

Commit

Permalink
UIOR-1038 Some shortcut keys do not work in the "Orders" app (#1424)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
usavkov-epam authored Nov 22, 2022
1 parent f395333 commit c17fa49
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/OrdersList/OrdersList.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}),
Expand Down
21 changes: 20 additions & 1 deletion src/OrdersList/OrdersList.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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) => <Component {...props} />,
}));
jest.mock('@folio/stripes/smart-components', () => ({
...jest.requireActual('@folio/stripes/smart-components'),
// eslint-disable-next-line react/prop-types
Expand Down Expand Up @@ -50,6 +55,9 @@ const defaultProps = {
resetData: jest.fn(),
refreshList: jest.fn(),
history,
location: {
search: '',
},
};

const renderOrdersList = (props = {}) => render(
Expand All @@ -62,6 +70,7 @@ const renderOrdersList = (props = {}) => render(

describe('OrdersList', () => {
beforeEach(() => {
defaultProps.history.push.mockClear();
HasCommand.mockClear();
useModalToggle.mockClear();
});
Expand Down Expand Up @@ -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', () => {
Expand Down
1 change: 1 addition & 0 deletions src/common/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ export * from './getUserNameById';
export * from './getExportAccountNumbers';
export * from './getRecordMap';
export * from './fetchExportDataByIds';
export * from './omitFieldArraysAsyncErrors';
export * from './validateDuplicateLines';
27 changes: 27 additions & 0 deletions src/common/utils/omitFieldArraysAsyncErrors.js
Original file line number Diff line number Diff line change
@@ -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;
};
49 changes: 49 additions & 0 deletions src/common/utils/omitFieldArraysAsyncErrors.test.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
6 changes: 4 additions & 2 deletions src/components/POLine/POLineForm.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/components/POLine/POLineView.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion src/components/POLine/const.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 }), {},
);
4 changes: 2 additions & 2 deletions src/components/PurchaseOrder/PO.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}),
Expand All @@ -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();
}
}),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import PropTypes from 'prop-types';
import { useMemo } from 'react';
import { FormattedMessage } from 'react-intl';
import { mapValues } from 'lodash';

Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit c17fa49

Please sign in to comment.