diff --git a/.eslintrc b/.eslintrc index 0e379d345..2bdde7210 100644 --- a/.eslintrc +++ b/.eslintrc @@ -16,7 +16,8 @@ { "files": [ "src/**"], "rules": { - "react-hooks/exhaustive-deps": "off" + "react-hooks/exhaustive-deps": "off", + "react/forbid-prop-types": [ 1, { "forbid": [ "array" ] } ] } } ], diff --git a/package.json b/package.json index a6354af4d..3cd4a0caa 100644 --- a/package.json +++ b/package.json @@ -830,6 +830,7 @@ "file-saver": "^2.0.0", "final-form": "^4.18.2", "final-form-arrays": "^3.0.1", + "history": "^5.0.0", "ky": "^0.23.0", "lodash": "^4.17.4", "moment": "~2.24.0", diff --git a/src/Holding/EditHolding/EditHolding.js b/src/Holding/EditHolding/EditHolding.js index e64599d0e..645753f21 100644 --- a/src/Holding/EditHolding/EditHolding.js +++ b/src/Holding/EditHolding/EditHolding.js @@ -1,4 +1,4 @@ -import React, { useCallback, useMemo } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; import PropTypes from 'prop-types'; import { FormattedMessage } from 'react-intl'; import { cloneDeep } from 'lodash'; @@ -17,23 +17,27 @@ import { } from '../../hooks'; import HoldingsForm from '../../edit/holdings/HoldingsForm'; import withLocation from '../../withLocation'; +import { parseHttpError } from '../../utils'; const EditHolding = ({ goTo, history, holdingId, instanceId, - location: { search, state: locationState }, + location, referenceTables, }) => { const callout = useCallout(); + const { search, state: locationState } = location; const stripes = useStripes(); + const [httpError, setHttpError] = useState(); const { instance, isLoading: isInstanceLoading } = useInstanceQuery(instanceId); const { holding, isLoading: isHoldingLoading } = useHolding(holdingId); const { totalRecords: itemCount, isLoading: isItemsLoading } = useHoldingItemsQuery(holdingId, { searchParams: { limit: 1 }, }); + const isMARCRecord = useMemo(() => ( referenceTables?.holdingsSources?.find(source => source.id === holding?.sourceId)?.name === 'MARC' ), [holding]); @@ -57,6 +61,11 @@ const EditHolding = ({ }); }, [onCancel, callout]); + const onError = async error => { + const parsedError = await parseHttpError(error.response); + setHttpError(parsedError); + }; + const { mutateHolding } = useHoldingMutation({ onSuccess }); const onSubmit = useCallback(holdingValues => { @@ -65,13 +74,15 @@ const EditHolding = ({ if (clonedHolding.permanentLocationId === '') delete clonedHolding.permanentLocationId; if (clonedHolding.temporaryLocationId === '') delete clonedHolding.temporaryLocationId; - return mutateHolding(clonedHolding); + return mutateHolding(clonedHolding).catch(onError); }, [mutateHolding]); if (isInstanceLoading || isItemsLoading || isHoldingLoading) return ; return ( render( ); describe('EditHolding', () => { - const mockMutate = jest.fn(); + const mutateHolding = jest.fn(() => Promise.resolve({ data: {} })); beforeEach(() => { useInstanceQuery.mockClear(); useHolding.mockClear(); useHoldingItemsQuery.mockClear(); - useHoldingMutation.mockClear().mockReturnValue({ mutateHolding: mockMutate }); + useHoldingMutation.mockClear().mockReturnValue({ mutateHolding }); }); it('should render HoldingsForm', () => { @@ -87,6 +87,21 @@ describe('EditHolding', () => { temporaryLocationId: '', }); - expect(mockMutate).toHaveBeenCalled(); + expect(mutateHolding).toHaveBeenCalled(); + }); + + it('should handle optimistic locking exception', () => { + const error = new Error({ response: 'optimistic locking' }); + mutateHolding.mockClear().mockImplementation(() => Promise.reject(error)); + + renderEditHolding(); + + HoldingsForm.mock.calls[0][0].onSubmit({ + id: 'holdingId', + permanentLocationId: '', + temporaryLocationId: '', + }); + + expect(mutateHolding).toHaveBeenCalled(); }); }); diff --git a/src/Instance/InstanceEdit/InstanceEdit.js b/src/Instance/InstanceEdit/InstanceEdit.js index 7f6e25882..2d0e04895 100644 --- a/src/Instance/InstanceEdit/InstanceEdit.js +++ b/src/Instance/InstanceEdit/InstanceEdit.js @@ -24,6 +24,7 @@ import InstanceForm from '../../edit/InstanceForm'; import { marshalInstance, unmarshalInstance, + parseHttpError, } from '../../utils'; import useLoadSubInstances from '../../hooks/useLoadSubInstances'; import useCallout from '../../hooks/useCallout'; @@ -63,11 +64,16 @@ const InstanceEdit = ({ goBack(); }, [callout, goBack]); + const onError = async error => { + const parsedError = await parseHttpError(error); + setHttpError(parsedError); + }; + const onSubmit = useCallback((updatedInstance) => { return mutator.instanceEdit.PUT(marshalInstance(updatedInstance, identifierTypesByName)) .then(() => onSuccess(updatedInstance)) - .catch(err => setHttpError(err)); - }, [identifierTypesByName, setHttpError]); + .catch(onError); + }, [identifierTypesByName, onError]); if (isInstanceLoading) return ; @@ -77,13 +83,14 @@ const InstanceEdit = ({ <> - {httpError && + {httpError && !httpError?.errorType && } diff --git a/src/Instance/InstanceEdit/InstanceEdit.test.js b/src/Instance/InstanceEdit/InstanceEdit.test.js index edfa4c0e6..308511ffd 100644 --- a/src/Instance/InstanceEdit/InstanceEdit.test.js +++ b/src/Instance/InstanceEdit/InstanceEdit.test.js @@ -16,7 +16,6 @@ import translations from '../../../test/jest/helpers/translationsProperties'; import { instance } from '../../../test/fixtures'; import InstanceEdit from './InstanceEdit'; - const queryClient = new QueryClient(); const referenceData = { locationsById: {}, diff --git a/src/Item/EditItem/EditItem.js b/src/Item/EditItem/EditItem.js index dc27ddc80..5ab98b20e 100644 --- a/src/Item/EditItem/EditItem.js +++ b/src/Item/EditItem/EditItem.js @@ -1,4 +1,4 @@ -import React, { useCallback } from 'react'; +import React, { useCallback, useState } from 'react'; import PropTypes from 'prop-types'; import { useHistory, @@ -17,6 +17,7 @@ import { } from '../../common'; import ItemForm from '../../edit/items/ItemForm'; import useCallout from '../../hooks/useCallout'; +import { parseHttpError } from '../../utils'; import { useItem, useItemMutation, @@ -30,7 +31,7 @@ const EditItem = ({ }) => { const history = useHistory(); const location = useLocation(); - + const [httpError, setHttpError] = useState(); const { isLoading: isInstanceLoading, instance } = useInstanceQuery(instanceId); const { isLoading: isHoldingLoading, holding } = useHolding(holdingId); const { isLoading: isItemLoading, item } = useItem(itemId); @@ -57,6 +58,13 @@ const EditItem = ({ }); }, [callout, instanceId, holdingId]); + const onError = async error => { + const parsedError = await parseHttpError(error.response); + + setHttpError(parsedError); + }; + + const { mutateItem } = useItemMutation({ onSuccess }); const onSubmit = useCallback((values) => { @@ -64,13 +72,14 @@ const EditItem = ({ delete item.barcode; } - return mutateItem(values); + return mutateItem(values).catch(onError); }, [mutateItem]); if (isInstanceLoading || isHoldingLoading || isItemLoading) return ; return ( jest.fn().mockReturnValue('ItemForm')); jest.mock('../../hooks/useCallout', () => jest.fn().mockReturnValue({ sendCallout: jest.fn() })); @@ -22,6 +23,7 @@ jest.mock('../../common/hooks', () => ({ jest.mock('../hooks', () => ({ ...jest.requireActual('../hooks'), useItem: jest.fn().mockReturnValue({ item: {}, isLoading: false }), + useItemMutation: jest.fn().mockReturnValue({ mutateItem: jest.fn() }), })); const defaultProps = { @@ -51,10 +53,13 @@ const renderEditItem = (props = {}) => render( describe('EditItem', () => { + const mutateItem = jest.fn(() => Promise.resolve({ data: {} })); + beforeEach(() => { useInstanceQuery.mockClear(); useHolding.mockClear(); useItem.mockClear(); + useItemMutation.mockClear().mockReturnValue({ mutateItem }); }); it('should render ItemForm', () => { @@ -70,4 +75,28 @@ describe('EditItem', () => { expect(screen.getByText('LoadingView')).toBeInTheDocument(); }); + + + it('should render LoadingView if page is loading', () => { + useInstanceQuery.mockReturnValue({ isLoading: true }); + + renderEditItem(); + + expect(screen.getByText('LoadingView')).toBeInTheDocument(); + }); + + it('should handle optimistic locking exception', () => { + const error = new Error({ response: 'optimistic locking' }); + mutateItem.mockClear().mockImplementation(() => Promise.reject(error)); + + renderEditItem(); + + ItemForm.mock.calls[0][0].onSubmit({ + id: 'itemId', + permanentLocationId: '', + temporaryLocationId: '', + }); + + expect(mutateItem).toHaveBeenCalled(); + }); }); diff --git a/src/components/OptimisticLockingBanner/OptimisticLockingBanner.js b/src/components/OptimisticLockingBanner/OptimisticLockingBanner.js new file mode 100644 index 000000000..0170c7b5c --- /dev/null +++ b/src/components/OptimisticLockingBanner/OptimisticLockingBanner.js @@ -0,0 +1,27 @@ +import { useRef } from 'react'; +import PropTypes from 'prop-types'; + +import { ConflictDetectionBanner } from '@folio/stripes/components'; + +import { ERROR_TYPES } from '../../constants'; + +const OptimisticLockingBanner = ({ latestVersionLink, httpError }) => { + const conflictDetectionBannerRef = useRef(null); + const focusConflictDetectionBanner = () => conflictDetectionBannerRef.current.focus(); + + return ( + httpError?.errorType === ERROR_TYPES.OPTIMISTIC_LOCKING && + + ); +}; + +OptimisticLockingBanner.propTypes = { + latestVersionLink: PropTypes.string, + httpError: PropTypes.object, +}; + +export default OptimisticLockingBanner; diff --git a/src/components/OptimisticLockingBanner/index.js b/src/components/OptimisticLockingBanner/index.js new file mode 100644 index 000000000..5eb885a5f --- /dev/null +++ b/src/components/OptimisticLockingBanner/index.js @@ -0,0 +1 @@ +export { default } from './OptimisticLockingBanner'; diff --git a/src/constants.js b/src/constants.js index 113fdc7da..5a7634176 100644 --- a/src/constants.js +++ b/src/constants.js @@ -283,3 +283,7 @@ export const FACETS_SETTINGS = { [FACETS_CQL.HOLDINGS_PERMANENT_LOCATION]: FACETS_OPTIONS.HOLDINGS_PERMANENT_LOCATION_OPTIONS, [FACETS_CQL.STATISTICAL_CODES]: FACETS_OPTIONS.STATISTICAL_CODES_OPTIONS, }; + +export const ERROR_TYPES = { + OPTIMISTIC_LOCKING: 'optimisticLocking', +}; diff --git a/src/edit/InstanceForm.js b/src/edit/InstanceForm.js index defc12aad..1794311df 100644 --- a/src/edit/InstanceForm.js +++ b/src/edit/InstanceForm.js @@ -31,9 +31,11 @@ import { collapseAllSections, expandAllSections, } from '@folio/stripes/components'; + import stripesFinalForm from '@folio/stripes/final-form'; import RepeatableField from '../components/RepeatableField'; +import OptimisticLockingBanner from '../components/OptimisticLockingBanner'; import AlternativeTitles from './alternativeTitles'; import AdministrativeNoteFields from './administrativeNoteFields'; @@ -249,6 +251,7 @@ class InstanceForm extends React.Component { pristine, submitting, history, + httpError, id, } = this.props; @@ -344,6 +347,12 @@ class InstanceForm extends React.Component { actionMenu={this.getActionMenu} id={id} > +
{ @@ -309,6 +316,12 @@ class HoldingsForm extends React.Component { : null } > + diff --git a/src/edit/items/ItemForm.js b/src/edit/items/ItemForm.js index dc1d55d72..c4414f510 100644 --- a/src/edit/items/ItemForm.js +++ b/src/edit/items/ItemForm.js @@ -40,6 +40,7 @@ import { import { effectiveCallNumber } from '@folio/stripes/util'; import RepeatableField from '../../components/RepeatableField'; +import OptimisticLockingBanner from '../../components/OptimisticLockingBanner'; import ElectronicAccessFields from '../electronicAccessFields'; import { memoize, mutators } from '../formUtils'; import { handleKeyCommand, validateOptionalField } from '../../utils'; @@ -181,6 +182,7 @@ class ItemForm extends React.Component { initialValues, instance, holdingsRecord, + httpError, referenceTables: { locationsById, @@ -335,6 +337,12 @@ class ItemForm extends React.Component { } > + fields.every(item => (isArray(item) @@ -713,3 +714,22 @@ export const batchRequest = (requestFn, items, buildQuery = buildQueryByIds, _pa */ export const accentFold = (str = '') => str.normalize('NFD').replace(/[\u0300-\u036f]/g, ''); +/** + * Parses http error to json and attaches an error type. + * + * @param httpError object + * @returns object + */ +export const parseHttpError = async httpError => { + try { + const jsonError = await httpError.json(); + + if (jsonError.message.match(/optimistic locking/i)) { + jsonError.errorType = ERROR_TYPES.OPTIMISTIC_LOCKING; + } + + return jsonError; + } catch (err) { + return httpError; + } +};