From d4cba7f92a7d3b8e740d93104c42367b2477c1a2 Mon Sep 17 00:00:00 2001 From: Artem_Blazhko Date: Mon, 7 Oct 2024 16:38:03 +0300 Subject: [PATCH] Use `instanceId` param for ILR from items response --- CHANGELOG.md | 1 + src/RequestForm.js | 24 ++-- src/RequestForm.test.js | 113 ++++++++++++++---- src/RequestFormContainer.js | 9 +- src/RequestFormContainer.test.js | 9 +- .../InstanceInformation.js | 4 +- .../InstanceInformation.test.js | 34 +----- .../ItemInformation/ItemInformation.js | 4 +- .../ItemInformation/ItemInformation.test.js | 13 +- src/constants.js | 1 - src/routes/RequestsRoute.js | 5 - src/routes/RequestsRoute.test.js | 23 ---- 12 files changed, 128 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e19619b1..e685b9be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ * Add sorting to 'Printed' and '# Copies' columns in the Request App. Refs UIREQ-1140. * Implement "Print Status" filters as server-side filters. Refs UIREQ-1147. * Navigate to first page when saving the pick slip print log from beyond the first page. Refs UIREQ-1145. +* Use `instanceId` param for ILR from items response. Refs UIREQ-1149. ## [9.1.2] (https://github.com/folio-org/ui-requests/tree/v9.1.2) (2024-09-13) [Full Changelog](https://github.com/folio-org/ui-requests/compare/v9.1.1...v9.1.2) diff --git a/src/RequestForm.js b/src/RequestForm.js index c4b91915..aacc5e43 100644 --- a/src/RequestForm.js +++ b/src/RequestForm.js @@ -143,7 +143,6 @@ class RequestForm extends React.Component { values: PropTypes.object.isRequired, form: PropTypes.object.isRequired, blocked: PropTypes.bool.isRequired, - instanceId: PropTypes.string.isRequired, isPatronBlocksOverridden: PropTypes.bool.isRequired, onSubmit: PropTypes.func, parentMutator: PropTypes.shape({ @@ -160,7 +159,6 @@ class RequestForm extends React.Component { onSetSelectedInstance: PropTypes.func.isRequired, onSetBlocked: PropTypes.func.isRequired, onSetIsPatronBlocksOverridden: PropTypes.func.isRequired, - onSetInstanceId: PropTypes.func.isRequired, }; static defaultProps = { @@ -717,22 +715,21 @@ class RequestForm extends React.Component { findItemRelatedResources(item) { const { findResource, - onSetInstanceId, } = this.props; - if (!item) return null; + + if (!item) { + return null; + } return Promise.all( [ findResource('loan', item.id), findResource('requestsForItem', item.id), - findResource(RESOURCE_TYPES.HOLDING, item.holdingsRecordId), ], ).then((results) => { const selectedLoan = results[0]?.loans?.[0]; const itemRequestCount = results[1]?.requests?.length; - const holdingsRecord = results[2]?.holdingsRecords?.[0]; - onSetInstanceId(holdingsRecord?.instanceId); this.setState({ itemRequestCount, selectedLoan, @@ -811,6 +808,11 @@ class RequestForm extends React.Component { return foundItem; }) + .catch(() => { + onSetSelectedItem(null); + + return null; + }) .then(item => { if (item && selectedUser?.id) { const requester = getRequester(proxy, selectedUser); @@ -888,6 +890,11 @@ class RequestForm extends React.Component { return instance; }) + .catch(() => { + onSetSelectedInstance(null); + + return null; + }) .then(instance => { if (instance && selectedUser?.id) { const requester = getRequester(proxy, selectedUser); @@ -1093,7 +1100,6 @@ class RequestForm extends React.Component { selectedUser, selectedInstance, isPatronBlocksOverridden, - instanceId, blocked, values, onCancel, @@ -1283,7 +1289,6 @@ class RequestForm extends React.Component { onSetSelectedInstance={onSetSelectedInstance} isLoading={isItemOrInstanceLoading} instanceRequestCount={instanceRequestCount} - instanceId={instanceId} /> @@ -1305,7 +1310,6 @@ class RequestForm extends React.Component { onSetSelectedItem={onSetSelectedItem} values={values} itemRequestCount={itemRequestCount} - instanceId={instanceId} selectedLoan={selectedLoan} isLoading={isItemOrInstanceLoading} /> diff --git a/src/RequestForm.test.js b/src/RequestForm.test.js index d3324567..ace72ad8 100644 --- a/src/RequestForm.test.js +++ b/src/RequestForm.test.js @@ -19,6 +19,8 @@ import RequestForm, { } from './RequestForm'; import FulfilmentPreference from './components/FulfilmentPreference'; import RequesterInformation from './components/RequesterInformation'; +import ItemInformation from './components/ItemInformation'; +import InstanceInformation from './components/InstanceInformation'; import { REQUEST_LEVEL_TYPES, @@ -52,6 +54,8 @@ const testIds = { closePatronModalButton: 'closePatronModalButton', itemDialogCloseButton: 'itemDialogCloseButton', itemDialogRow: 'itemDialogRow', + itemEnterButton: 'itemEnterButton', + instanceEnterButton: 'instanceEnterButton', }; const fieldValue = 'value'; const idResourceType = 'id'; @@ -214,7 +218,6 @@ describe('RequestForm', () => { onSetSelectedInstance: jest.fn(), onSetSelectedItem: jest.fn(), onSetSelectedUser: jest.fn(), - onSetInstanceId: jest.fn(), onSetIsPatronBlocksOverridden: jest.fn(), onSetBlocked: jest.fn(), onShowErrorModal: jest.fn(), @@ -1232,6 +1235,7 @@ describe('RequestForm', () => { id: 'itemId', barcode: initialItemBarcode, holdingsRecordId: 'holdingsRecordId', + instanceId, } ], }; @@ -1253,13 +1257,6 @@ describe('RequestForm', () => { const itemRequestsResult = { requests: [], }; - const holdingsRecordResult = { - holdingsRecords: [ - { - instanceId, - } - ], - }; let findResource; beforeEach(() => { @@ -1269,7 +1266,6 @@ describe('RequestForm', () => { .mockResolvedValueOnce(requestTypesResult) .mockResolvedValueOnce(loanResult) .mockResolvedValueOnce(itemRequestsResult) - .mockResolvedValueOnce(holdingsRecordResult) .mockResolvedValue({}); const props = { @@ -1338,19 +1334,6 @@ describe('RequestForm', () => { expect(findResource).toHaveBeenCalledWith(...expectedArgs); }); - it('should get information about holdings', () => { - const expectedArgs = [ - RESOURCE_TYPES.HOLDING, - itemResult.items[0].holdingsRecordId - ]; - - expect(findResource).toHaveBeenCalledWith(...expectedArgs); - }); - - it('should set instance id', () => { - expect(basicProps.onSetInstanceId).toHaveBeenCalledWith(holdingsRecordResult.holdingsRecords[0].instanceId); - }); - it('should handle item barcode field change', () => { const expectedArgs = [RESOURCE_TYPES.ITEM, fieldValue, RESOURCE_KEYS.id]; const itemField = screen.getByTestId(testIds.itemField); @@ -1420,6 +1403,47 @@ describe('RequestForm', () => { expect(findResource).not.toHaveBeenCalledWith(...expectedArgs); }); }); + + describe('When error during item finding', () => { + beforeEach(() => { + const props = { + ...basicProps, + request: undefined, + findResource: jest.fn() + .mockRejectedValue({}), + }; + + ItemInformation.mockImplementationOnce(({ + findItem, + }) => { + const handleClick = () => { + findItem(idResourceType, 'id', false); + }; + + return ( + + ); + }); + + renderComponent(props); + }); + + it('should reset item information', async () => { + const itemField = screen.getByTestId(testIds.itemEnterButton); + + fireEvent.click(itemField); + + await waitFor(() => { + expect(basicProps.onSetSelectedItem).toHaveBeenCalledWith(null); + }); + }); + }); }); describe('Component updating', () => { @@ -1697,6 +1721,51 @@ describe('RequestForm', () => { expect(resetFieldState).not.toHaveBeenCalledWith(...expectedArgs); }); }); + + describe('When error during instance finding', () => { + beforeEach(() => { + const props = { + ...basicProps, + request: undefined, + values: { + ...basicProps.values, + createTitleLevelRequest: true, + }, + findResource: jest.fn() + .mockRejectedValue({}), + }; + + InstanceInformation.mockImplementationOnce(({ + findInstance, + }) => { + const handleClick = () => { + findInstance('id', false); + }; + + return ( + + ); + }); + + renderComponent(props); + }); + + it('should reset instance information', async () => { + const itemField = screen.getByTestId(testIds.instanceEnterButton); + + fireEvent.click(itemField); + + await waitFor(() => { + expect(basicProps.onSetSelectedInstance).toHaveBeenCalledWith(null); + }); + }); + }); }); describe('Component updating', () => { diff --git a/src/RequestFormContainer.js b/src/RequestFormContainer.js index 0c062603..c2810738 100644 --- a/src/RequestFormContainer.js +++ b/src/RequestFormContainer.js @@ -39,7 +39,6 @@ const RequestFormContainer = ({ const [selectedUser, setSelectedUser] = useState({ ...requester, id: requesterId }); const [selectedInstance, setSelectedInstance] = useState(request?.instance); const [isPatronBlocksOverridden, setIsPatronBlocksOverridden] = useState(false); - const [instanceId, setInstanceId] = useState(''); const [blocked, setBlocked] = useState(false); const setItem = (optedItem) => { @@ -62,10 +61,6 @@ const RequestFormContainer = ({ setIsPatronBlocksOverridden(value); }; - const setStateInstanceId = (id) => { - setInstanceId(id); - }; - const getPatronManualBlocks = (resources) => { return (resources?.patronBlocks?.records || []) .filter(b => b.requests === true) @@ -142,7 +137,7 @@ const RequestFormContainer = ({ }; } - requestData.instanceId = request?.instanceId || instanceId || selectedInstance?.id; + requestData.instanceId = request?.instanceId || selectedInstance?.id || selectedItem?.instanceId; requestData.requestLevel = request?.requestLevel || getRequestLevelValue(requestData.createTitleLevelRequest); if (requestData.requestLevel === REQUEST_LEVEL_TYPES.ITEM) { @@ -179,7 +174,6 @@ const RequestFormContainer = ({ selectedUser={selectedUser} selectedInstance={selectedInstance} isPatronBlocksOverridden={isPatronBlocksOverridden} - instanceId={instanceId} onGetPatronManualBlocks={getPatronManualBlocks} onGetAutomatedPatronBlocks={getAutomatedPatronBlocks} onSetBlocked={setIsBlocked} @@ -187,7 +181,6 @@ const RequestFormContainer = ({ onSetSelectedUser={setUser} onSetSelectedInstance={setInstance} onSetIsPatronBlocksOverridden={setStateIsPatronBlocksOverridden} - onSetInstanceId={setStateInstanceId} onSubmit={handleSubmit} /> ); diff --git a/src/RequestFormContainer.test.js b/src/RequestFormContainer.test.js index 64af25fb..e0c4c3f5 100644 --- a/src/RequestFormContainer.test.js +++ b/src/RequestFormContainer.test.js @@ -55,7 +55,6 @@ describe('RequestFormContainer', () => { }, selectedInstance: defaultProps.request.instance, isPatronBlocksOverridden: false, - instanceId: '', onGetPatronManualBlocks: expect.any(Function), onGetAutomatedPatronBlocks: expect.any(Function), onSetBlocked: expect.any(Function), @@ -63,7 +62,6 @@ describe('RequestFormContainer', () => { onSetSelectedUser: expect.any(Function), onSetSelectedInstance: expect.any(Function), onSetIsPatronBlocksOverridden: expect.any(Function), - onSetInstanceId: expect.any(Function), onSubmit: expect.any(Function), }; @@ -100,6 +98,10 @@ describe('RequestFormContainer', () => { }; const props = { ...defaultProps, + request: { + ...defaultProps.request, + instanceId: null, + }, itemId: 'itemId', item: { id: 'id', @@ -107,6 +109,7 @@ describe('RequestFormContainer', () => { }; const selectedItem = { holdingsRecordId: 'holdingsRecordId', + instanceId: 'instanceId', }; const selectItemLabel = 'Select Item'; @@ -144,7 +147,7 @@ describe('RequestFormContainer', () => { const expectedArg = { holdingsRecordId: selectedItem.holdingsRecordId, fulfillmentPreference: fulfillmentTypeMap.HOLD_SHELF, - instanceId: defaultProps.request.instanceId, + instanceId: selectedItem.instanceId, requestLevel: REQUEST_LEVEL_TYPES.ITEM, pickupServicePointId: submitData.pickupServicePointId, item: submitData.item, diff --git a/src/components/InstanceInformation/InstanceInformation.js b/src/components/InstanceInformation/InstanceInformation.js index 0bf66afa..745b0d81 100644 --- a/src/components/InstanceInformation/InstanceInformation.js +++ b/src/components/InstanceInformation/InstanceInformation.js @@ -37,7 +37,6 @@ class InstanceInformation extends Component { values: PropTypes.object.isRequired, onSetSelectedInstance: PropTypes.func.isRequired, isLoading: PropTypes.bool.isRequired, - instanceId: PropTypes.string.isRequired, request: PropTypes.object, instanceRequestCount: PropTypes.number, selectedInstance: PropTypes.object, @@ -159,7 +158,6 @@ class InstanceInformation extends Component { values, isLoading, instanceRequestCount, - instanceId, } = this.props; const { isInstanceClicked, @@ -247,7 +245,7 @@ class InstanceInformation extends Component { { isTitleInfoVisible && { }); describe('when instance is selected', () => { - it('should render "TitleInformation" with correct props', () => { - render( - - ); - - const expectedProps = { - instanceId: basicProps.instanceId, - titleLevelRequestsCount: basicProps.instanceRequestCount, - title: basicProps.selectedInstance.title, - contributors: basicProps.selectedInstance.contributors, - publications: basicProps.selectedInstance.publication, - editions: basicProps.selectedInstance.editions, - identifiers: basicProps.selectedInstance.identifiers, - }; - - expect(TitleInformation).toHaveBeenCalledWith(expectedProps, {}); - }); - it('should render "TitleInformation" with "request.instanceId"', () => { const instanceId = 'instanceId'; const props = { @@ -627,21 +607,13 @@ describe('InstanceInformation', () => { }); it('should render "TitleInformation" with "selectedInstance.id"', () => { - const selectedInstanceId = 'selectedInstanceId'; - const props = { - ...basicProps, - selectedInstance: { - ...basicProps.selectedInstance, - id: selectedInstanceId, - }, - }; const expectedProps = { - instanceId: selectedInstanceId, + instanceId: basicProps.selectedInstance.id, }; render( ); diff --git a/src/components/ItemInformation/ItemInformation.js b/src/components/ItemInformation/ItemInformation.js index c6bf74bf..3b1e9f1a 100644 --- a/src/components/ItemInformation/ItemInformation.js +++ b/src/components/ItemInformation/ItemInformation.js @@ -35,7 +35,6 @@ class ItemInformation extends Component { request: PropTypes.object.isRequired, onSetSelectedItem: PropTypes.func.isRequired, itemRequestCount: PropTypes.number.isRequired, - instanceId: PropTypes.string.isRequired, isLoading: PropTypes.bool.isRequired, submitting: PropTypes.bool.isRequired, isItemIdRequest: PropTypes.bool.isRequired, @@ -162,7 +161,6 @@ class ItemInformation extends Component { isLoading, selectedItem, request, - instanceId, selectedLoan, itemRequestCount, } = this.props; @@ -235,7 +233,7 @@ class ItemInformation extends Component { selectedItem && { }); describe('when item is selected', () => { + const props = { + ...basicProps, + selectedItem: { + id: 'itemId', + instanceId: 'instanceId', + }, + }; + beforeEach(() => { render( ); }); @@ -557,7 +564,7 @@ describe('ItemInformation', () => { it('should render "ItemDetail" with correct props', () => { const expectedProps = { request: basicProps.request, - currentInstanceId: basicProps.instanceId, + currentInstanceId: basicProps.selectedItem.instanceId, item: basicProps.selectedItem, loan: basicProps.selectedLoan, requestCount: basicProps.itemRequestCount, diff --git a/src/constants.js b/src/constants.js index a2890983..c328eca4 100644 --- a/src/constants.js +++ b/src/constants.js @@ -329,7 +329,6 @@ export const RESOURCE_TYPES = { ITEM: 'item', INSTANCE: 'instance', USER: 'user', - HOLDING: 'holding', REQUEST_TYPES: 'requestTypes', ECS_TLR_SETTINGS: 'ecsTlrSettings', }; diff --git a/src/routes/RequestsRoute.js b/src/routes/RequestsRoute.js index 9275ae21..5dbe6072 100644 --- a/src/routes/RequestsRoute.js +++ b/src/routes/RequestsRoute.js @@ -202,11 +202,6 @@ export const urls = { return `request-preference-storage/request-preference?${query}`; }, - holding: (value, idType) => { - const query = stringify({ query: `(${idType}=="${value}")` }); - - return `holdings-storage/holdings?${query}`; - }, requestTypes: ({ requesterId, itemId, diff --git a/src/routes/RequestsRoute.test.js b/src/routes/RequestsRoute.test.js index a6a4d2cf..f895c1f4 100644 --- a/src/routes/RequestsRoute.test.js +++ b/src/routes/RequestsRoute.test.js @@ -1827,29 +1827,6 @@ describe('RequestsRoute', () => { }); }); - describe('holding', () => { - const value = 'value'; - let queryString; - - beforeEach(() => { - queryString = urls.holding(value, idType); - }); - - it('should trigger "stringify" with correct argument', () => { - const expectedArgument = { - query: `(${idType}=="${value}")`, - }; - - expect(stringify).toHaveBeenCalledWith(expectedArgument); - }); - - it('should return correct url', () => { - const expectedResult = `holdings-storage/holdings?${mockedQueryValue}`; - - expect(queryString).toBe(expectedResult); - }); - }); - describe('requestTypes', () => { const requesterId = 'requesterIdUrl'; const operation = REQUEST_OPERATIONS.CREATE;