Skip to content

Commit

Permalink
UISACQCOMP-218: add "Amount must be a positive number" validation for…
Browse files Browse the repository at this point in the history
… "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
  • Loading branch information
alisher-epam authored Sep 30, 2024
1 parent 8a6d401 commit d0dde18
Show file tree
Hide file tree
Showing 13 changed files with 127 additions and 74 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions lib/AcqUnitFilter/AcqUnitFilter.test.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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]/);

Expand Down
1 change: 1 addition & 0 deletions lib/AffiliationsSelection/AffiliationsSelection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down
5 changes: 2 additions & 3 deletions lib/CountryFilter/CountryFilter.test.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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"]');
Expand Down
15 changes: 9 additions & 6 deletions lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -118,9 +121,9 @@ const CurrencyExchangeRateFields = ({
<TooltippedControl
controlComponent={Checkbox}
checked={isExchangeRateEnabled}
data-testid="use-set-exhange-rate"
data-testid="use-set-exchange-rate"
readOnly={isSetUseExangeRateDisabled}
id="use-set-exhange-rate"
id="use-set-exchange-rate"
label={<FormattedMessage id="stripes-acq-components.useSetExchangeRate" />}
onChange={enableExchangeRate}
vertical
Expand All @@ -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}
/>
Expand Down
24 changes: 21 additions & 3 deletions lib/CurrencyExchangeRateFields/CurrencyExchangeRateFields.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -27,21 +28,38 @@ const renderForm = ({ handleSubmit }) => (

const FormCmpt = stripesFinalForm({})(renderForm);

const renderComponent = (props = {}) => (render(
const renderComponent = (props = {}) => render(
<MemoryRouter>
<FormCmpt onSubmit={() => { }} initialValues={{}} {...props} />
</MemoryRouter>,
));
);

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();

renderComponent({ onSubmit });

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'));

Expand Down
3 changes: 2 additions & 1 deletion lib/DynamicSelection/DynamicSelection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({}));
});
Expand All @@ -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/));

Expand Down
7 changes: 6 additions & 1 deletion lib/FieldHolding/FieldHolding.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
12 changes: 10 additions & 2 deletions lib/FieldLocation/FieldLocationFinalContainer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]/);

Expand All @@ -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]/);

Expand Down
5 changes: 2 additions & 3 deletions lib/FundFilter/FundFilter.test.js
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
8 changes: 4 additions & 4 deletions lib/LanguageFilter/LanguageFilter.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -17,8 +19,6 @@ const renderFilter = (disabled = false, onChange = noop) => (render(
));

describe('LanguageFilter component', () => {
afterEach(cleanup);

it('should display passed title', () => {
const { getByText } = renderFilter();

Expand Down
8 changes: 8 additions & 0 deletions lib/utils/validateRequired.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ export function validateRequiredPositiveAmount(value) {
: <FormattedMessage id="stripes-acq-components.validation.shouldBePositiveAmount" />;
}

export function validatePositiveAmount(value) {
if (value === '' || value === undefined || value === null) return undefined;

return Number(value) > 0
? undefined
: <FormattedMessage id="stripes-acq-components.validation.shouldBePositiveAmount" />;
}

export const validateRequiredMinNumber = ({ minNumber, value }) => {
return value >= minNumber
? undefined
Expand Down
107 changes: 59 additions & 48 deletions lib/utils/validateRequired.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
validateNoSpaces,
validatePositiveAmount,
validateRequired,
validateRequiredMaxNumber,
validateRequiredMinNumber,
Expand All @@ -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();
});
});

0 comments on commit d0dde18

Please sign in to comment.