From eedb997338caf301a31adbd2d817c08224de4e79 Mon Sep 17 00:00:00 2001 From: Artem Blazhko Date: Sat, 25 Jan 2025 08:00:47 +0200 Subject: [PATCH] UIU-2701: Return appropriate error message when item for manual fee/fine can't be found (#2847) --- CHANGELOG.md | 1 + .../Accounts/ChargeFeeFine/ChargeFeeFine.js | 32 +++- .../ChargeFeeFine/ChargeFeeFine.test.js | 77 +++++++++ .../Accounts/ChargeFeeFine/ChargeForm.js | 18 ++- .../Accounts/ChargeFeeFine/ChargeForm.test.js | 3 + .../Accounts/ChargeFeeFine/ItemInfo.js | 152 ++++++++++++++++-- .../Accounts/ChargeFeeFine/ItemInfo.test.js | 149 ++++++++++++++--- src/constants.js | 5 + translations/ui-users/en.json | 1 + 9 files changed, 397 insertions(+), 41 deletions(-) create mode 100644 src/components/Accounts/ChargeFeeFine/ChargeFeeFine.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 61986c0ef..abd8b0dbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * Update fee/fine actions column UX for accessibility. Refs UIU-3027. * Change import of `exportToCsv` from `stripes-util` to `stripes-components`. Refs UIU-3202. * Provide `itemToString` to create unique `key` attributes. Refs UIU-3312. +* Return appropriate error message when item for manual fee/fine can't be found. Refs UIU-2701. ## [11.0.11](https://github.com/folio-org/ui-users/tree/v11.0.11) (2025-01-15) [Full Changelog](https://github.com/folio-org/ui-users/compare/v11.0.10...v11.0.11) diff --git a/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.js b/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.js index 4093da56b..87ff31e0e 100644 --- a/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.js +++ b/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.js @@ -21,7 +21,10 @@ import { loadServicePoints, deleteOptionalActionFields, } from '../accountFunctions'; -import { SHARED_OWNER } from '../../../constants'; +import { + SHARED_OWNER, + NEW_FEE_FINE_FIELD_NAMES, +} from '../../../constants'; class ChargeFeeFine extends React.Component { static propTypes = { @@ -72,6 +75,7 @@ class ChargeFeeFine extends React.Component { showConfirmDialog: false, notify: null, paymentNotify: null, + itemBarcode: '', }; this.chargeAction = this.chargeAction.bind(this); this.payAction = this.payAction.bind(this); @@ -232,7 +236,10 @@ class ChargeFeeFine extends React.Component { onClickSelectItem(barcode) { if (barcode !== '') { - this.props.mutator.activeRecord.update({ barcode }); + this.props.mutator.activeRecord.update({ + barcode, + isBarcodeValidated: true, + }); if ((this.props.resources.activeRecord || {}).barcode === barcode) { this.setState({ lookup: true, @@ -241,7 +248,7 @@ class ChargeFeeFine extends React.Component { } } - onChangeOwner(ownerId) { + onChangeOwner(ownerId, itemBarcode = '') { const { resources, mutator, @@ -253,7 +260,11 @@ class ChargeFeeFine extends React.Component { mutator.activeRecord.update({ shared }); } mutator.activeRecord.update({ ownerId }); - this.setState({ ownerId }); + + this.setState({ + ownerId, + itemBarcode, + }); } onChangeItem(item) { @@ -395,7 +406,12 @@ class ChargeFeeFine extends React.Component { ); } - onSubmitCharge = (data) => { + onSubmitCharge = (dataToSend) => { + const data = _.cloneDeep(dataToSend); + + _.unset(data, NEW_FEE_FINE_FIELD_NAMES.ITEM_BARCODE); + _.unset(data, NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE); + if (data.pay) { delete data.pay; this.type.remaining = data.amount; @@ -433,6 +449,7 @@ class ChargeFeeFine extends React.Component { ownerId, feeFineTypeId, pay, + itemBarcode, } = this.state; this.item = _.get(resources, ['items', 'records', [0]], {}); const feefines = _.get(resources, ['allfeefines', 'records'], []); @@ -461,7 +478,7 @@ class ChargeFeeFine extends React.Component { parseFloat(selected).toFixed(2); let item; - if (this.item && (loanid || barcode)) { + if (this.item && (loanid || barcode || !resources.activeRecord?.isBarcodeValidated)) { item = { id: this.item.id || '', instance: this.item.title || '', @@ -490,7 +507,8 @@ class ChargeFeeFine extends React.Component { ownerId: initialOwnerId, notify: !!(selectedFeeFine?.chargeNoticeId || selectedOwner?.defaultChargeNoticeId), feeFineId: '', - amount: '' + amount: '', + itemBarcode, }; const initialActionValues = { diff --git a/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.test.js b/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.test.js new file mode 100644 index 000000000..73f45de87 --- /dev/null +++ b/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.test.js @@ -0,0 +1,77 @@ +import { + screen, + render, +} from '@folio/jest-config-stripes/testing-library/react'; +import userEvent from '@folio/jest-config-stripes/testing-library/user-event'; + +import ChargeFeeFine from './ChargeFeeFine'; +import ChargeForm from './ChargeForm'; + +const basicProps = { + resources: { + activeRecord: { + isBarcodeValidated: true, + }, + }, + mutator: { + activeRecord: { + update: jest.fn(), + }, + }, + user: {}, + stripes: {}, + location: {}, + history: {}, + intl: {}, + match: { + params: {}, + }, + okapi: { + currentUser: {}, + }, +}; +const testIds = { + selectItem: 'selectItem', +}; +const itemBarcode = 'itemBarcode'; + +jest.mock('./ChargeForm', () => jest.fn(({ + onClickSelectItem, +}) => ( + <> + + +))); +jest.mock('./ItemLookup', () => jest.fn(() =>
)); +jest.mock('../Actions/ActionModal', () => jest.fn(() =>
)); + +describe('ChargeFeeFine', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + beforeEach(() => { + render(); + }); + + it('should trigger ChargeForm', () => { + expect(ChargeForm).toHaveBeenCalled(); + }); + + it('should update activeRecord', async () => { + const selectItemButton = screen.getByTestId(testIds.selectItem); + + await userEvent.click(selectItemButton); + + expect(basicProps.mutator.activeRecord.update).toHaveBeenCalledWith({ + barcode: itemBarcode, + isBarcodeValidated: true, + }); + }); +}); diff --git a/src/components/Accounts/ChargeFeeFine/ChargeForm.js b/src/components/Accounts/ChargeFeeFine/ChargeForm.js index 0887ee5e3..b16d0dc32 100644 --- a/src/components/Accounts/ChargeFeeFine/ChargeForm.js +++ b/src/components/Accounts/ChargeFeeFine/ChargeForm.js @@ -22,7 +22,10 @@ import { effectiveCallNumber } from '@folio/stripes/util'; import UserInfo from './UserInfo'; import FeeFineInfo from './FeeFineInfo'; import ItemInfo from './ItemInfo'; -import { SHARED_OWNER } from '../../../constants'; +import { + SHARED_OWNER, + NEW_FEE_FINE_FIELD_NAMES, +} from '../../../constants'; import { formatCurrencyAmount } from '../../util'; function showValidationErrors({ feeFineId, ownerId, amount }) { @@ -62,6 +65,7 @@ class ChargeForm extends React.Component { handleSubmit: PropTypes.func.isRequired, onClickSelectItem: PropTypes.func.isRequired, onFindShared: PropTypes.func.isRequired, + values: PropTypes.object.isRequired, pristine: PropTypes.bool, submitting: PropTypes.bool, invalid: PropTypes.bool, @@ -122,9 +126,17 @@ class ChargeForm extends React.Component { } onChangeOwner(ownerId) { - const { form: { change, reset } } = this.props; + const { + form: { + change, + reset, + }, + values, + onChangeOwner, + } = this.props; + reset(); - this.props.onChangeOwner(ownerId); + onChangeOwner(ownerId, values[NEW_FEE_FINE_FIELD_NAMES.ITEM_BARCODE]); let showNotify = false; const owner = this.props.owners.find(o => o.id === ownerId) || {}; if (owner?.defaultChargeNoticeId) { diff --git a/src/components/Accounts/ChargeFeeFine/ChargeForm.test.js b/src/components/Accounts/ChargeFeeFine/ChargeForm.test.js index 6ac81b57d..36f2f3143 100644 --- a/src/components/Accounts/ChargeFeeFine/ChargeForm.test.js +++ b/src/components/Accounts/ChargeFeeFine/ChargeForm.test.js @@ -139,6 +139,9 @@ const propData = { ownerId: '109155d9-3b60-4a3d-a6b1-066cf1b1356b', metadata: { createdDate: '2022-03-01T03:22:48.262+00:00', updatedDate: '2022-03-01T03:22:48.262+00:00' }, }], + resources: { + items: {}, + }, }; describe('ChargeForm component', () => { diff --git a/src/components/Accounts/ChargeFeeFine/ItemInfo.js b/src/components/Accounts/ChargeFeeFine/ItemInfo.js index f39676461..3e51e36e7 100644 --- a/src/components/Accounts/ChargeFeeFine/ItemInfo.js +++ b/src/components/Accounts/ChargeFeeFine/ItemInfo.js @@ -1,7 +1,9 @@ import React from 'react'; +import { Field } from 'react-final-form'; import PropTypes from 'prop-types'; import { Link } from 'react-router-dom'; import { FormattedMessage } from 'react-intl'; + import { Button, Row, @@ -9,8 +11,26 @@ import { TextField, } from '@folio/stripes/components'; +import { NEW_FEE_FINE_FIELD_NAMES } from '../../../constants'; + class ItemInfo extends React.Component { static propTypes = { + resources: PropTypes.shape({ + items: PropTypes.shape({ + records: PropTypes.arrayOf(PropTypes.object), + isPending: PropTypes.bool, + }), + activeRecord: PropTypes.shape({ + barcode: PropTypes.string, + }), + }).isRequired, + mutator: PropTypes.shape({ + activeRecord: PropTypes.shape({ + replace: PropTypes.func, + }), + }), + values: PropTypes.object.isRequired, + form: PropTypes.object.isRequired, onClickSelectItem: PropTypes.func, item: PropTypes.object, editable: PropTypes.bool, @@ -25,6 +45,29 @@ class ItemInfo extends React.Component { this.onClickSelectItem = this.onClickSelectItem.bind(this); this.onChangeSelectItem = this.onChangeSelectItem.bind(this); this.query = ''; + this.state = { + isBarcodeChangedAfterValidation: false, + }; + } + + componentDidUpdate(prevProps) { + const { + resources: { + items, + }, + } = this.props; + const { + resources: { + items: prevItems, + }, + } = prevProps; + + if (prevItems.records !== items.records && this.state.isBarcodeChangedAfterValidation) { + this.triggerItemBarcodeValidation(); + this.setState({ + isBarcodeChangedAfterValidation: false, + }); + } } onClickSelectItem(e) { @@ -33,10 +76,66 @@ class ItemInfo extends React.Component { } onChangeSelectItem(e) { - if (e) e.preventDefault(); - this.query = e.target.value; + const value = e.target.value; + const { + form, + resources, + } = this.props; + const { + isBarcodeChangedAfterValidation, + } = this.state; + + if (e) { + e.preventDefault(); + } + + if (resources.activeRecord?.barcode && resources.activeRecord?.barcode !== value) { + this.props.mutator.activeRecord.replace({ + ...resources.activeRecord, + isBarcodeValidated: false, + barcode: '', + }); + } + + if (!isBarcodeChangedAfterValidation) { + this.setState({ + isBarcodeChangedAfterValidation: true, + }); + } + + form.change(NEW_FEE_FINE_FIELD_NAMES.ITEM_BARCODE, value); + this.query = value; } + triggerItemBarcodeValidation = () => { + const { + form, + values, + } = this.props; + + form.change(NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE, values[NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE] ? 0 : 1); + }; + + validateBarcode = (barcode) => { + const { + resources: { + items, + activeRecord, + }, + } = this.props; + + if (barcode && barcode === activeRecord.barcode && items.records.length === 0) { + return ( + + ); + } + + return undefined; + }; + render() { const { item: { @@ -46,8 +145,17 @@ class ItemInfo extends React.Component { instance, barcode, id, - } + }, + values, + editable, + resources: { + items, + }, } = this.props; + const { + isBarcodeChangedAfterValidation, + } = this.state; + const isEnterButtonDisabled = !editable || items.isPending; return (
@@ -57,20 +165,42 @@ class ItemInfo extends React.Component { - {placeholder => ( - - )} + {placeholder => { + const key = values[NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE] ?? 0; + + return ( + + { + ({ input, meta }) => { + const validationError = !isBarcodeChangedAfterValidation && meta.error; + + return ( + + ); + } + } + + ); + }} diff --git a/src/components/Accounts/ChargeFeeFine/ItemInfo.test.js b/src/components/Accounts/ChargeFeeFine/ItemInfo.test.js index cf627d713..958203e14 100644 --- a/src/components/Accounts/ChargeFeeFine/ItemInfo.test.js +++ b/src/components/Accounts/ChargeFeeFine/ItemInfo.test.js @@ -1,39 +1,148 @@ -import { screen } from '@folio/jest-config-stripes/testing-library/react'; +import { + screen, + render, +} from '@folio/jest-config-stripes/testing-library/react'; import userEvent from '@folio/jest-config-stripes/testing-library/user-event'; -import renderWithRouter from 'helpers/renderWithRouter'; import ItemInfo from './ItemInfo'; +import { NEW_FEE_FINE_FIELD_NAMES } from '../../../constants'; import '__mock__/stripesCore.mock'; import '__mock__/stripesSmartComponent.mock'; jest.unmock('@folio/stripes/components'); -const renderItemInfo = (props) => renderWithRouter( +const renderItemInfo = (props) => render( ); -const selectMock = jest.fn(); - -const props = { - onClickSelectItem: selectMock, +const basicProps = { + onClickSelectItem: jest.fn(), item: {}, - editable: true + editable: true, + resources: { + items: { + records: [], + }, + activeRecord: { + barcode: 'itemBarcode', + }, + }, + mutator: { + activeRecord: { + replace: jest.fn(), + }, + }, + form: { + change: jest.fn(), + }, + values: {}, +}; +const labelIds = { + itemTitle: 'ui-users.charge.item.title', + chargeButton: 'ui-users.charge.item.button', +}; +const testIds = { + itemBarcodeField: 'itemBarcodeField', }; -describe('Item Info component', () => { - it('if it renders', () => { - renderItemInfo(props); - expect(screen.getByText('ui-users.charge.item.title')).toBeInTheDocument(); +jest.mock('react-final-form', () => ({ + Field: jest.fn(({ + children, + 'data-testid': testId, + validate, + }) => { + return children({ + meta: {}, + input: { + validate, + 'data-testid': testId, + value: '', + }, + }); + }), +})); +jest.mock('react-router-dom', () => ({ + Link: jest.fn(() =>
), +})); + +describe('ItemInfo', () => { + afterEach(() => { + jest.clearAllMocks(); }); - it('Charge Item Button', async () => { - renderItemInfo(props); - await userEvent.click(screen.getByText('ui-users.charge.item.button')); - expect(selectMock).toHaveBeenCalled(); + + describe('Initial render', () => { + beforeEach(() => { + renderItemInfo(basicProps); + }); + + it('should render item title', () => { + const itemTitle = screen.getByText(labelIds.itemTitle); + + expect(itemTitle).toBeInTheDocument(); + }); + + it('should get item information', async () => { + const chargeButton = screen.getByText(labelIds.chargeButton); + + await userEvent.click(chargeButton); + + expect(basicProps.onClickSelectItem).toHaveBeenCalled(); + }); }); - it('charge item text field', async () => { - renderItemInfo(props); - await userEvent.type(document.querySelector('[placeholder="ui-users.charge.item.placeholder"]'), 'Test'); - expect(document.querySelector('[placeholder="ui-users.charge.item.placeholder"]').value).toBe('Test'); + + describe('Component updating', () => { + const itemBarcode = 'newItemBarcode'; + const newProps = { + ...basicProps, + resources: { + ...basicProps.resources, + items: { + records: [ + { + id: 'itemId', + } + ] + } + }, + }; + + beforeEach(async () => { + const { rerender } = renderItemInfo(basicProps); + + const itemBarcodeField = screen.getByTestId(testIds.itemBarcodeField); + + await userEvent.type(itemBarcodeField, itemBarcode); + + rerender(); + }); + + it('should change "key" value of barcode field', () => { + expect(basicProps.form.change).toHaveBeenCalledWith(NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE, basicProps.values[NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE] ? 0 : 1); + }); + }); + + describe('Item barcode changing', () => { + const itemBarcode = 'newItemBarcode'; + + beforeEach(async () => { + renderItemInfo(basicProps); + + const itemBarcodeField = screen.getByTestId(testIds.itemBarcodeField); + + await userEvent.type(itemBarcodeField, itemBarcode); + }); + + it('should replace activeRecord', () => { + expect(basicProps.mutator.activeRecord.replace).toHaveBeenCalledWith({ + ...basicProps.resources.activeRecord, + isBarcodeValidated: false, + barcode: '', + }); + }); + + it('should change item barcode field', () => { + expect(basicProps.form.change).toHaveBeenCalledWith(NEW_FEE_FINE_FIELD_NAMES.ITEM_BARCODE, itemBarcode); + }); }); }); diff --git a/src/constants.js b/src/constants.js index 36ed514a2..949991aeb 100644 --- a/src/constants.js +++ b/src/constants.js @@ -401,3 +401,8 @@ export const KEYCLOAK_USER_EXISTANCE = { nonExist: 'nonExist', error: 'error', }; + +export const NEW_FEE_FINE_FIELD_NAMES = { + ITEM_BARCODE: 'itemBarcode', + KEY_OF_ITEM_BARCODE: 'keyOfItemBarcode', +}; diff --git a/translations/ui-users/en.json b/translations/ui-users/en.json index 12c6ac386..5312347ae 100644 --- a/translations/ui-users/en.json +++ b/translations/ui-users/en.json @@ -581,6 +581,7 @@ "charge.item.placeholder": "Scan or enter item barcode", "charge.item.button": "Enter", "charge.item.barcode": "Barcode", + "charge.item.barcode.error": "No item with barcode {barcode} could be found", "charge.item.instance": "Instance", "charge.item.status": "Item Status", "charge.item.callNumber": "Effective call number string",