From d0dde185380ad09c2d7ca01628beb1a0c226b6b5 Mon Sep 17 00:00:00 2001 From: Alisher Musurmonov Date: Mon, 30 Sep 2024 10:50:13 +0500 Subject: [PATCH] UISACQCOMP-218: add "Amount must be a positive number" validation for "Set exchange rate" field. (#815) * UISACQCOMP-218: add "Amount must be a positive number" validation for "Set exchange rate" field. * test: fix failing tests * test: add test case * test: fix failing test * test: fix failing tests * refactor: fix code quality based on comments * remove required message * add not required positive amount validation * updated validation with number type conversion --- CHANGELOG.md | 1 + lib/AcqUnitFilter/AcqUnitFilter.test.js | 5 +- .../AffiliationsSelection.test.js | 1 + lib/CountryFilter/CountryFilter.test.js | 5 +- .../CurrencyExchangeRateFields.js | 15 ++- .../CurrencyExchangeRateFields.test.js | 24 +++- lib/DynamicSelection/DynamicSelection.test.js | 3 +- lib/FieldHolding/FieldHolding.test.js | 7 +- .../FieldLocationFinalContainer.test.js | 12 +- lib/FundFilter/FundFilter.test.js | 5 +- lib/LanguageFilter/LanguageFilter.test.js | 8 +- lib/utils/validateRequired.js | 8 ++ lib/utils/validateRequired.test.js | 107 ++++++++++-------- 13 files changed, 127 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d60b190a..9b7f55cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ * Support using custom list of tenants when open the locations modal. Refs UISACQCOMP-210. * ECS - Display all consortium tenants in the affiliation selection of the location lookup. Refs UISACQCOMP-202. * ECS - Add `isLoading` prop for `ConsortiumFieldInventory` component. Refs UISACQCOMP-215. +* Add "Amount must be a positive number" validation for "Set exchange rate" field. Refs UISACQCOMP-218. ## [5.1.2](https://github.com/folio-org/stripes-acq-components/tree/v5.1.2) (2024-09-13) [Full Changelog](https://github.com/folio-org/stripes-acq-components/compare/v5.1.1...v5.1.2) diff --git a/lib/AcqUnitFilter/AcqUnitFilter.test.js b/lib/AcqUnitFilter/AcqUnitFilter.test.js index 29019936..b73c4047 100644 --- a/lib/AcqUnitFilter/AcqUnitFilter.test.js +++ b/lib/AcqUnitFilter/AcqUnitFilter.test.js @@ -1,6 +1,5 @@ -import React from 'react'; -import { fireEvent, render, cleanup } from '@testing-library/react'; import { noop } from 'lodash'; +import { fireEvent, render, cleanup } from '@testing-library/react'; import AcqUnitFilter from './AcqUnitFilter'; @@ -25,7 +24,7 @@ describe('AcqUnitFilter component', () => { it('should render all passed options', async () => { const { findAllByText, getByText } = renderAcqUnitFilter(acqUnitsRecords); - await fireEvent.click(getByText('stripes-components.selection.controlLabel')); + fireEvent.click(getByText('stripes-components.selection.controlLabel')); const renderedFilterOptions = await findAllByText(/Unit #[0-9]/); diff --git a/lib/AffiliationsSelection/AffiliationsSelection.test.js b/lib/AffiliationsSelection/AffiliationsSelection.test.js index 0d209615..e2175546 100644 --- a/lib/AffiliationsSelection/AffiliationsSelection.test.js +++ b/lib/AffiliationsSelection/AffiliationsSelection.test.js @@ -43,6 +43,7 @@ describe('AffiliationsSelection', () => { within(document.getElementById('test-affiliations-select')) .getByText(affiliations[2].tenantName), ).toBeInTheDocument(); + affiliations.forEach(({ tenantName, isPrimary }) => { expect( within(document.getElementById('test-affiliations-select')) diff --git a/lib/CountryFilter/CountryFilter.test.js b/lib/CountryFilter/CountryFilter.test.js index bdac6a53..c42d2455 100644 --- a/lib/CountryFilter/CountryFilter.test.js +++ b/lib/CountryFilter/CountryFilter.test.js @@ -1,6 +1,5 @@ -import React from 'react'; -import { render, cleanup, fireEvent } from '@testing-library/react'; import { noop } from 'lodash'; +import { fireEvent, render, cleanup } from '@testing-library/react'; import CountryFilter from './CountryFilter'; @@ -25,7 +24,7 @@ describe('CountryFilter component', () => { expect(getByText('ui-organizations.filterConfig.country')).toBeDefined(); }); - it('should invoke onChange callback when something is selected', async () => { + it('should invoke onChange callback when something is selected', () => { const onChangeFilter = jest.fn(); const { container, getByText } = renderFilter(false, onChangeFilter); const button = container.querySelector('[id="org-filter-country-selection"]'); diff --git a/lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.js b/lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.js index 3ea28920..a5f94f68 100644 --- a/lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.js +++ b/lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.js @@ -19,16 +19,19 @@ import { Row, } from '@folio/stripes/components'; +import { FILED_NAMES } from './constants'; import { FieldCurrency } from '../Currency'; +import CurrentExchangeRate from './CurrentExchangeRate'; import { TextField } from '../Fields'; import { TooltippedControl } from '../TooltippedControl'; import { IfFieldVisible, VisibilityControl, } from '../VisibilityControl'; -import CurrentExchangeRate from './CurrentExchangeRate'; -import { FILED_NAMES } from './constants'; -import { validateRequired } from '../utils'; +import { + validatePositiveAmount, + validateRequiredPositiveAmount, +} from '../utils'; const CurrencyExchangeRateFields = ({ currencyFieldName, @@ -118,9 +121,9 @@ const CurrencyExchangeRateFields = ({ } onChange={enableExchangeRate} vertical @@ -146,7 +149,7 @@ const CurrencyExchangeRateFields = ({ readOnly={!isExchangeRateEnabled} tooltipText={tooltipTextExchangeRate} required={isExchangeRateRequired} - validate={isExchangeRateRequired ? validateRequired : undefined} + validate={isExchangeRateRequired ? validateRequiredPositiveAmount : validatePositiveAmount} key={isExchangeRateRequired ? 1 : 0} isNonInteractive={isSetExchangeRateNonIntaractive ?? isNonInteractive} /> diff --git a/lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.test.js b/lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.test.js index 356752d7..043681a8 100644 --- a/lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.test.js +++ b/lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.test.js @@ -9,6 +9,7 @@ import { MemoryRouter } from 'react-router-dom'; import { Button } from '@folio/stripes/components'; import stripesFinalForm from '@folio/stripes/final-form'; +import CurrentExchangeRate from './CurrentExchangeRate'; import CurrencyExchangeRateFields from './CurrencyExchangeRateFields'; jest.mock('./CurrentExchangeRate', () => { @@ -27,13 +28,30 @@ const renderForm = ({ handleSubmit }) => ( const FormCmpt = stripesFinalForm({})(renderForm); -const renderComponent = (props = {}) => (render( +const renderComponent = (props = {}) => render( { }} initialValues={{}} {...props} /> , -)); +); describe('CurrencyExchangeRateFields', () => { + it('should display validation messages', async () => { + const onSubmit = jest.fn(); + + renderComponent({ onSubmit }); + + await user.click(screen.getByText('stripes-components.selection.controlLabel')); + await user.click(screen.getByText('BYN (BYN)')); + await user.click(screen.getByTestId('use-set-exchange-rate')); + + await waitFor(() => expect(screen.getByTestId('use-set-exchange-rate')).toBeChecked()); + + CurrentExchangeRate.mock.calls[0][0].setExchangeRateRequired(true); + await user.type(screen.getByTestId('exchange-rate'), '-1'); + await user.click(screen.getByText('Save')); + await waitFor(() => expect(screen.getByText('stripes-acq-components.validation.shouldBePositiveAmount')).toBeInTheDocument()); + }); + it('should pass user input to API', async () => { const onSubmit = jest.fn(); @@ -41,7 +59,7 @@ describe('CurrencyExchangeRateFields', () => { user.click(screen.getByText('stripes-components.selection.controlLabel')); user.click(screen.getByText('BYN (BYN)')); - user.click(screen.getByTestId('use-set-exhange-rate')); + user.click(screen.getByTestId('use-set-exchange-rate')); user.type(screen.getByTestId('exchange-rate'), '2.66'); user.click(screen.getByText('Save')); diff --git a/lib/DynamicSelection/DynamicSelection.test.js b/lib/DynamicSelection/DynamicSelection.test.js index edf7be95..fffe5f1f 100644 --- a/lib/DynamicSelection/DynamicSelection.test.js +++ b/lib/DynamicSelection/DynamicSelection.test.js @@ -50,6 +50,7 @@ describe('DynamicSelection', () => { await user.type(input, '1'); jest.advanceTimersByTime(1500); }); + await user.click(screen.getByText('stripes-components.selection.controlLabel')); expect(kyMock.get).toHaveBeenCalledWith(ORDERS_API, expect.objectContaining({})); }); @@ -63,7 +64,7 @@ describe('DynamicSelection', () => { await user.type(input, '1'); jest.advanceTimersByTime(1500); }); - + user.click(screen.getByText('stripes-components.selection.controlLabel')); user.click(screen.getByText(/11111/)); diff --git a/lib/FieldHolding/FieldHolding.test.js b/lib/FieldHolding/FieldHolding.test.js index bd4517b4..9299e4d2 100644 --- a/lib/FieldHolding/FieldHolding.test.js +++ b/lib/FieldHolding/FieldHolding.test.js @@ -1,5 +1,10 @@ -import { render, screen, within, fireEvent } from '@testing-library/react'; import keyBy from 'lodash/keyBy'; +import { + fireEvent, + render, + screen, + within, +} from '@testing-library/react'; import { Form } from 'react-final-form'; import { useInstanceHoldings } from '../hooks'; diff --git a/lib/FieldLocation/FieldLocationFinalContainer.test.js b/lib/FieldLocation/FieldLocationFinalContainer.test.js index 0dbd2244..3b80b458 100644 --- a/lib/FieldLocation/FieldLocationFinalContainer.test.js +++ b/lib/FieldLocation/FieldLocationFinalContainer.test.js @@ -35,13 +35,19 @@ describe('FieldLocationFinalContainer component', () => { it('should display passed label', () => { renderFieldLocationFinalContainer({ labelId: 'Location' }); + const button = screen.getByText('stripes-components.selection.controlLabel'); + + fireEvent.click(button); + expect(screen.getByText(fieldLocationLabel)).toBeDefined(); }); it('should render options based on passed locationIds', async () => { renderFieldLocationFinalContainer(); - fireEvent.click(screen.getByText('stripes-components.selection.controlLabel')); + const button = screen.getByText('stripes-components.selection.controlLabel'); + + fireEvent.click(button); const renderedLocationOptions = await screen.findAllByText(/Location #[0-9]/); @@ -53,7 +59,9 @@ describe('FieldLocationFinalContainer component', () => { filterLocations: (records) => records.slice(0, 2), }); - fireEvent.click(screen.getByText('stripes-components.selection.controlLabel')); + const button = screen.getByText('stripes-components.selection.controlLabel'); + + fireEvent.click(button); const renderedLocationOptions = await screen.findAllByText(/Location #[0-9]/); diff --git a/lib/FundFilter/FundFilter.test.js b/lib/FundFilter/FundFilter.test.js index 800b9f15..d7bd2f77 100644 --- a/lib/FundFilter/FundFilter.test.js +++ b/lib/FundFilter/FundFilter.test.js @@ -1,7 +1,6 @@ -import React from 'react'; -import { render, fireEvent } from '@testing-library/react'; -import { IntlProvider } from 'react-intl'; import { noop } from 'lodash'; +import { fireEvent, render } from '@testing-library/react'; +import { IntlProvider } from 'react-intl'; import FundFilter from './FundFilter'; diff --git a/lib/LanguageFilter/LanguageFilter.test.js b/lib/LanguageFilter/LanguageFilter.test.js index d1993f7e..a9f3acfb 100644 --- a/lib/LanguageFilter/LanguageFilter.test.js +++ b/lib/LanguageFilter/LanguageFilter.test.js @@ -1,5 +1,7 @@ -import React from 'react'; -import { render, cleanup, fireEvent } from '@testing-library/react'; +import { + fireEvent, + render, +} from '@testing-library/react'; import { noop } from 'lodash'; import LanguageFilter from './LanguageFilter'; @@ -17,8 +19,6 @@ const renderFilter = (disabled = false, onChange = noop) => (render( )); describe('LanguageFilter component', () => { - afterEach(cleanup); - it('should display passed title', () => { const { getByText } = renderFilter(); diff --git a/lib/utils/validateRequired.js b/lib/utils/validateRequired.js index cf6693ca..0e677812 100644 --- a/lib/utils/validateRequired.js +++ b/lib/utils/validateRequired.js @@ -36,6 +36,14 @@ export function validateRequiredPositiveAmount(value) { : ; } +export function validatePositiveAmount(value) { + if (value === '' || value === undefined || value === null) return undefined; + + return Number(value) > 0 + ? undefined + : ; +} + export const validateRequiredMinNumber = ({ minNumber, value }) => { return value >= minNumber ? undefined diff --git a/lib/utils/validateRequired.test.js b/lib/utils/validateRequired.test.js index 07a1184f..63e7046f 100644 --- a/lib/utils/validateRequired.test.js +++ b/lib/utils/validateRequired.test.js @@ -1,5 +1,6 @@ import { validateNoSpaces, + validatePositiveAmount, validateRequired, validateRequiredMaxNumber, validateRequiredMinNumber, @@ -8,58 +9,68 @@ import { validateRequiredPositiveAmount, } from './validateRequired'; -test('validateNoSpaces', () => { - expect(validateNoSpaces('some code')).toBeTruthy(); - expect(validateNoSpaces('some_code')).toBe(undefined); -}); +describe('validateRequired', () => { + it('validateNoSpaces', () => { + expect(validateNoSpaces('some code')).toBeTruthy(); + expect(validateNoSpaces('some_code')).toBe(undefined); + }); -test('validateRequired', () => { - expect(validateRequired('')).toBeTruthy(); - expect(validateRequired(null)).toBeTruthy(); - expect(validateRequired(undefined)).toBeTruthy(); - expect(validateRequired('some value')).toBe(undefined); -}); + it('validateRequired', () => { + expect(validateRequired('')).toBeTruthy(); + expect(validateRequired(null)).toBeTruthy(); + expect(validateRequired(undefined)).toBeTruthy(); + expect(validateRequired('some value')).toBe(undefined); + }); -test('validateRequiredNotNegative', () => { - expect(validateRequiredNotNegative('')).toBeTruthy(); - expect(validateRequiredNotNegative(null)).toBeTruthy(); - expect(validateRequiredNotNegative(undefined)).toBeTruthy(); - expect(validateRequiredNotNegative(-1)).toBeTruthy(); - expect(validateRequiredNotNegative(0)).toBe(undefined); - expect(validateRequiredNotNegative('0')).toBe(undefined); - expect(validateRequiredNotNegative(1)).toBe(undefined); - expect(validateRequiredNotNegative('some value')).toBeTruthy(); -}); + it('validateRequiredNotNegative', () => { + expect(validateRequiredNotNegative('')).toBeTruthy(); + expect(validateRequiredNotNegative(null)).toBeTruthy(); + expect(validateRequiredNotNegative(undefined)).toBeTruthy(); + expect(validateRequiredNotNegative(-1)).toBeTruthy(); + expect(validateRequiredNotNegative(0)).toBe(undefined); + expect(validateRequiredNotNegative('0')).toBe(undefined); + expect(validateRequiredNotNegative(1)).toBe(undefined); + expect(validateRequiredNotNegative('some value')).toBeTruthy(); + }); -test('validateRequiredNumber', () => { - expect(validateRequiredNumber('')).toBeTruthy(); - expect(validateRequiredNumber(null)).toBeTruthy(); - expect(validateRequiredNumber(undefined)).toBeTruthy(); - expect(validateRequiredNumber(-1)).toBe(undefined); - expect(validateRequiredNumber(0)).toBe(undefined); - expect(validateRequiredNumber(1)).toBe(undefined); - expect(validateRequiredNumber('some value')).toBeTruthy(); -}); + it('validateRequiredNumber', () => { + expect(validateRequiredNumber('')).toBeTruthy(); + expect(validateRequiredNumber(null)).toBeTruthy(); + expect(validateRequiredNumber(undefined)).toBeTruthy(); + expect(validateRequiredNumber(-1)).toBe(undefined); + expect(validateRequiredNumber(0)).toBe(undefined); + expect(validateRequiredNumber(1)).toBe(undefined); + expect(validateRequiredNumber('some value')).toBeTruthy(); + }); -test('validateRequiredPositiveAmount', () => { - expect(validateRequiredPositiveAmount('')).toBeTruthy(); - expect(validateRequiredPositiveAmount(null)).toBeTruthy(); - expect(validateRequiredPositiveAmount(undefined)).toBeTruthy(); - expect(validateRequiredPositiveAmount(-1)).toBeTruthy(); - expect(validateRequiredPositiveAmount(0)).toBeTruthy(); - expect(validateRequiredPositiveAmount(0.1)).toBe(undefined); - expect(validateRequiredPositiveAmount(1)).toBe(undefined); - expect(validateRequiredPositiveAmount('some value')).toBeTruthy(); -}); + it('validateRequiredPositiveAmount', () => { + expect(validateRequiredPositiveAmount('')).toBeTruthy(); + expect(validateRequiredPositiveAmount(null)).toBeTruthy(); + expect(validateRequiredPositiveAmount(undefined)).toBeTruthy(); + expect(validateRequiredPositiveAmount(-1)).toBeTruthy(); + expect(validateRequiredPositiveAmount(0)).toBeTruthy(); + expect(validateRequiredPositiveAmount(0.1)).toBe(undefined); + expect(validateRequiredPositiveAmount(1)).toBe(undefined); + expect(validateRequiredPositiveAmount('some value')).toBeTruthy(); + }); -test('validateRequiredMinNumber', () => { - expect(validateRequiredMinNumber({ minNumber: 1, value: 0 })).toBeTruthy(); - expect(validateRequiredMinNumber({ minNumber: 1, value: 1 })).toBe(undefined); - expect(validateRequiredMinNumber({ minNumber: 1, value: 2 })).toBe(undefined); -}); + it('validateRequiredMinNumber', () => { + expect(validateRequiredMinNumber({ minNumber: 1, value: 0 })).toBeTruthy(); + expect(validateRequiredMinNumber({ minNumber: 1, value: 1 })).toBe(undefined); + expect(validateRequiredMinNumber({ minNumber: 1, value: 2 })).toBe(undefined); + }); + + it('validateRequiredMaxNumber', () => { + expect(validateRequiredMaxNumber({ maxNumber: 1, value: 0 })).toBe(undefined); + expect(validateRequiredMaxNumber({ maxNumber: 1, value: 1 })).toBe(undefined); + expect(validateRequiredMaxNumber({ maxNumber: 1, value: 2 })).toBeTruthy(); + }); -test('validateRequiredMaxNumber', () => { - expect(validateRequiredMaxNumber({ maxNumber: 1, value: 0 })).toBe(undefined); - expect(validateRequiredMaxNumber({ maxNumber: 1, value: 1 })).toBe(undefined); - expect(validateRequiredMaxNumber({ maxNumber: 1, value: 2 })).toBeTruthy(); + it('validatePositiveAmount', () => { + expect(validatePositiveAmount('')).toBe(undefined); + expect(validatePositiveAmount(null)).toBe(undefined); + expect(validatePositiveAmount(undefined)).toBe(undefined); + expect(validatePositiveAmount(-1)).toBeTruthy(); + expect(validatePositiveAmount(0)).toBeTruthy(); + }); });