Skip to content

Commit

Permalink
UIIN-1872: Display a Conflict Detection banner (folio-org#1568)
Browse files Browse the repository at this point in the history
* UIIN-1872: Display a Conflict Detection banner
  • Loading branch information
mkuklis authored Jan 26, 2022
1 parent aae92aa commit 7bd7c51
Show file tree
Hide file tree
Showing 15 changed files with 171 additions and 15 deletions.
3 changes: 2 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
{
"files": [ "src/**"],
"rules": {
"react-hooks/exhaustive-deps": "off"
"react-hooks/exhaustive-deps": "off",
"react/forbid-prop-types": [ 1, { "forbid": [ "array" ] } ]
}
}
],
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
17 changes: 14 additions & 3 deletions src/Holding/EditHolding/EditHolding.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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]);
Expand All @@ -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 => {
Expand All @@ -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 <LoadingView />;

return (
<HoldingsForm
httpError={httpError}
location={location}
initialValues={holding}
onSubmit={onSubmit}
onCancel={onCancel}
Expand Down
21 changes: 18 additions & 3 deletions src/Holding/EditHolding/EditHolding.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ const renderEditHolding = (props = {}) => 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', () => {
Expand All @@ -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();
});
});
13 changes: 10 additions & 3 deletions src/Instance/InstanceEdit/InstanceEdit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 <LoadingView />;

Expand All @@ -77,13 +83,14 @@ const InstanceEdit = ({
<>
<InstanceForm
onSubmit={onSubmit}
httpError={httpError}
initialValues={initialValues}
instanceSource={instance?.source}
referenceTables={referenceData}
stripes={stripes}
onCancel={goBack}
/>
{httpError &&
{httpError && !httpError?.errorType &&
<ErrorModal
open
label={<FormattedMessage id="ui-inventory.instance.saveError" />}
Expand Down
1 change: 0 additions & 1 deletion src/Instance/InstanceEdit/InstanceEdit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {},
Expand Down
15 changes: 12 additions & 3 deletions src/Item/EditItem/EditItem.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback } from 'react';
import React, { useCallback, useState } from 'react';
import PropTypes from 'prop-types';
import {
useHistory,
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -57,20 +58,28 @@ 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) => {
if (!values.barcode) {
delete item.barcode;
}

return mutateItem(values);
return mutateItem(values).catch(onError);
}, [mutateItem]);

if (isInstanceLoading || isHoldingLoading || isItemLoading) return <LoadingView />;

return (
<ItemForm
httpError={httpError}
form={`itemform_${holding.id}`}
id={holding.id}
key={holding.id}
Expand Down
31 changes: 30 additions & 1 deletion src/Item/EditItem/EditItem.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import {
useInstanceQuery,
useHolding,
} from '../../common/hooks';
import { useItem } from '../hooks';
import { useItem, useItemMutation } from '../hooks';
import EditItem from './EditItem';
import ItemForm from '../../edit/items/ItemForm';

jest.mock('../../edit/items/ItemForm', () => jest.fn().mockReturnValue('ItemForm'));
jest.mock('../../hooks/useCallout', () => jest.fn().mockReturnValue({ sendCallout: jest.fn() }));
Expand All @@ -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 = {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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();
});
});
27 changes: 27 additions & 0 deletions src/components/OptimisticLockingBanner/OptimisticLockingBanner.js
Original file line number Diff line number Diff line change
@@ -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 &&
<ConflictDetectionBanner
latestVersionLink={latestVersionLink}
conflictDetectionBannerRef={conflictDetectionBannerRef}
focusConflictDetectionBanner={focusConflictDetectionBanner}
/>
);
};

OptimisticLockingBanner.propTypes = {
latestVersionLink: PropTypes.string,
httpError: PropTypes.object,
};

export default OptimisticLockingBanner;
1 change: 1 addition & 0 deletions src/components/OptimisticLockingBanner/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from './OptimisticLockingBanner';
4 changes: 4 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
};
10 changes: 10 additions & 0 deletions src/edit/InstanceForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -249,6 +251,7 @@ class InstanceForm extends React.Component {
pristine,
submitting,
history,
httpError,
id,
} = this.props;

Expand Down Expand Up @@ -344,6 +347,12 @@ class InstanceForm extends React.Component {
actionMenu={this.getActionMenu}
id={id}
>
<OptimisticLockingBanner
httpError={httpError}
latestVersionLink={`/inventory/view/${initialValues.id}`}
conflictDetectionBannerRef={this.conflictDetectionBannerRef}
focusConflictDetectionBanner={this.focusConflictDetectionBanner}
/>
<div>
<Headline
size="large"
Expand Down Expand Up @@ -767,6 +776,7 @@ InstanceForm.propTypes = {
instanceSource: PropTypes.string,
history: PropTypes.object.isRequired,
id: PropTypes.string,
httpError: PropTypes.object,
};
InstanceForm.defaultProps = {
instanceSource: 'FOLIO',
Expand Down
Loading

0 comments on commit 7bd7c51

Please sign in to comment.