From 2775e92ae1e9bf5b4f5372be32ecaeae0eb22eb2 Mon Sep 17 00:00:00 2001 From: Artem_Blazhko Date: Fri, 17 Jan 2025 16:10:20 +0200 Subject: [PATCH 1/3] Return appropriate error message when item for manual fee/fine can't be found --- .../Accounts/ChargeFeeFine/ChargeFeeFine.js | 46 ++++-- .../Accounts/ChargeFeeFine/ChargeForm.js | 18 ++- .../Accounts/ChargeFeeFine/ItemInfo.js | 136 ++++++++++++++++-- src/constants.js | 5 + translations/ui-users/en.json | 1 + 5 files changed, 178 insertions(+), 28 deletions(-) diff --git a/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.js b/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.js index 4093da56b..1d812bbae 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) { @@ -396,18 +407,23 @@ class ChargeFeeFine extends React.Component { } onSubmitCharge = (data) => { - if (data.pay) { - delete data.pay; - this.type.remaining = data.amount; + const dataToSend = _.cloneDeep(data); + + _.unset(dataToSend, NEW_FEE_FINE_FIELD_NAMES.ITEM_BARCODE); + _.unset(dataToSend, NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE); + + if (dataToSend.pay) { + delete dataToSend.pay; + this.type.remaining = dataToSend.amount; - return this.chargeAction(data) - .then(() => this.payAction(data)) + return this.chargeAction(dataToSend) + .then(() => this.payAction(dataToSend)) .then(() => this.goBack()); } else { - delete data.pay; + delete dataToSend.pay; - return this.chargeAction(data) - .then(() => this.showCalloutMessage(data)) + return this.chargeAction(dataToSend) + .then(() => this.showCalloutMessage(dataToSend)) .then(() => this.goBack()); } } @@ -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/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/ItemInfo.js b/src/components/Accounts/ChargeFeeFine/ItemInfo.js index f39676461..505ba2676 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,16 @@ 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), + }), + }).isRequired, + form: PropTypes.object.isRequired, onClickSelectItem: PropTypes.func, item: PropTypes.object, editable: PropTypes.bool, @@ -25,6 +35,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 +66,61 @@ 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 +130,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 +150,41 @@ 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/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..bfc89b451 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": "Item with this barcode does not exist", "charge.item.instance": "Instance", "charge.item.status": "Item Status", "charge.item.callNumber": "Effective call number string", From a7cb8556b8ff080d2e7cf93e2249b9a56f43ba69 Mon Sep 17 00:00:00 2001 From: Artem_Blazhko Date: Thu, 23 Jan 2025 13:52:45 +0200 Subject: [PATCH 2/3] Change error message --- src/components/Accounts/ChargeFeeFine/ItemInfo.js | 7 ++++++- translations/ui-users/en.json | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/components/Accounts/ChargeFeeFine/ItemInfo.js b/src/components/Accounts/ChargeFeeFine/ItemInfo.js index 505ba2676..e11583e04 100644 --- a/src/components/Accounts/ChargeFeeFine/ItemInfo.js +++ b/src/components/Accounts/ChargeFeeFine/ItemInfo.js @@ -115,7 +115,12 @@ class ItemInfo extends React.Component { } = this.props; if (barcode && barcode === activeRecord.barcode && items.records.length === 0) { - return ; + return ( + + ); } return undefined; diff --git a/translations/ui-users/en.json b/translations/ui-users/en.json index bfc89b451..5312347ae 100644 --- a/translations/ui-users/en.json +++ b/translations/ui-users/en.json @@ -581,7 +581,7 @@ "charge.item.placeholder": "Scan or enter item barcode", "charge.item.button": "Enter", "charge.item.barcode": "Barcode", - "charge.item.barcode.error": "Item with this barcode does not exist", + "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", From 39a6e6df354d2c34b7cef9330310e8e2e12b35b5 Mon Sep 17 00:00:00 2001 From: Artem_Blazhko Date: Thu, 23 Jan 2025 17:38:22 +0200 Subject: [PATCH 3/3] Add tests --- CHANGELOG.md | 1 + .../Accounts/ChargeFeeFine/ChargeFeeFine.js | 24 +-- .../ChargeFeeFine/ChargeFeeFine.test.js | 77 +++++++++ .../Accounts/ChargeFeeFine/ChargeForm.test.js | 3 + .../Accounts/ChargeFeeFine/ItemInfo.js | 13 +- .../Accounts/ChargeFeeFine/ItemInfo.test.js | 149 +++++++++++++++--- 6 files changed, 234 insertions(+), 33 deletions(-) create mode 100644 src/components/Accounts/ChargeFeeFine/ChargeFeeFine.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index fca290149..cc4266d73 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 1d812bbae..87ff31e0e 100644 --- a/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.js +++ b/src/components/Accounts/ChargeFeeFine/ChargeFeeFine.js @@ -406,24 +406,24 @@ class ChargeFeeFine extends React.Component { ); } - onSubmitCharge = (data) => { - const dataToSend = _.cloneDeep(data); + onSubmitCharge = (dataToSend) => { + const data = _.cloneDeep(dataToSend); - _.unset(dataToSend, NEW_FEE_FINE_FIELD_NAMES.ITEM_BARCODE); - _.unset(dataToSend, NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE); + _.unset(data, NEW_FEE_FINE_FIELD_NAMES.ITEM_BARCODE); + _.unset(data, NEW_FEE_FINE_FIELD_NAMES.KEY_OF_ITEM_BARCODE); - if (dataToSend.pay) { - delete dataToSend.pay; - this.type.remaining = dataToSend.amount; + if (data.pay) { + delete data.pay; + this.type.remaining = data.amount; - return this.chargeAction(dataToSend) - .then(() => this.payAction(dataToSend)) + return this.chargeAction(data) + .then(() => this.payAction(data)) .then(() => this.goBack()); } else { - delete dataToSend.pay; + delete data.pay; - return this.chargeAction(dataToSend) - .then(() => this.showCalloutMessage(dataToSend)) + return this.chargeAction(data) + .then(() => this.showCalloutMessage(data)) .then(() => this.goBack()); } } 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.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 e11583e04..3e51e36e7 100644 --- a/src/components/Accounts/ChargeFeeFine/ItemInfo.js +++ b/src/components/Accounts/ChargeFeeFine/ItemInfo.js @@ -18,8 +18,18 @@ class ItemInfo extends React.Component { 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, @@ -160,13 +170,14 @@ class ItemInfo extends React.Component { return ( { - ({input, meta}) => { + ({ 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); + }); }); });