From ebb87b3d45a8ba920d9f86374966cb67ab4962f8 Mon Sep 17 00:00:00 2001 From: Alisher Musurmonov Date: Mon, 25 Nov 2024 17:54:25 +0500 Subject: [PATCH 1/4] UIF-562: add `useDebouncedQuery` hook to fix endless request for `DynamicSelection` component --- CHANGELOG.md | 1 + lib/DynamicSelection/DynamicSelection.js | 63 +++++++------------ lib/hooks/index.js | 1 + lib/hooks/useDebouncedQuery/index.js | 1 + .../useDebouncedQuery/useDebouncedQuery.js | 61 ++++++++++++++++++ 5 files changed, 86 insertions(+), 41 deletions(-) create mode 100644 lib/hooks/useDebouncedQuery/index.js create mode 100644 lib/hooks/useDebouncedQuery/useDebouncedQuery.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 5680d86d..25cb15d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Add more reusable hooks and utilities. Refs UISACQCOMP-228. * Move reusable version history components to the ACQ lib. Refs UISACQCOMP-230. * Move reusable helper function to support version history functionality. Refs UISACQCOMP-232. +* Add `useDebouncedQuery` hook to fix endless request for `DynamicSelection` component. Refs UIF-562. ## [6.0.1](https://github.com/folio-org/stripes-acq-components/tree/v6.0.1) (2024-11-14) [Full Changelog](https://github.com/folio-org/stripes-acq-components/compare/v6.0.0...v6.0.1) diff --git a/lib/DynamicSelection/DynamicSelection.js b/lib/DynamicSelection/DynamicSelection.js index 7d5f7a1f..343a6fc0 100644 --- a/lib/DynamicSelection/DynamicSelection.js +++ b/lib/DynamicSelection/DynamicSelection.js @@ -1,13 +1,13 @@ -import { useCallback, useEffect, useState } from 'react'; -import { FormattedMessage } from 'react-intl'; -import { debounce } from 'lodash'; import PropTypes from 'prop-types'; +import { useCallback } from 'react'; +import { FormattedMessage } from 'react-intl'; -import { Loading, Selection } from '@folio/stripes/components'; -import { useOkapiKy } from '@folio/stripes/core'; +import { + Loading, + Selection, +} from '@folio/stripes/components'; -const LIST_ITEMS_LIMIT = 100; -const DEBOUNCE_DELAY = 500; +import { useDebouncedQuery } from '../hooks'; export const DynamicSelection = ({ api, @@ -19,46 +19,27 @@ export const DynamicSelection = ({ value, ...rest }) => { - const ky = useOkapiKy(); - const [filterValue, setFilterValue] = useState(''); - const [options, setOptions] = useState(initialOptions); - const [isLoading, setIsLoading] = useState(); - - const fetchData = useCallback(debounce(async (inputValue) => { - const searchParams = { - query: queryBuilder(inputValue), - limit: LIST_ITEMS_LIMIT, - }; - - try { - const res = await ky.get(api, { searchParams }).json(); - - setOptions(dataFormatter(res)); - } catch { - setOptions([]); - } - - setIsLoading(false); - }, DEBOUNCE_DELAY), []); - - const onFilter = useCallback((inputValue) => { - setIsLoading(true); - setFilterValue(inputValue); - fetchData(inputValue); + const { + options = initialOptions, + isLoading, + inputValue, + setInputValue, + } = useDebouncedQuery({ + api, + dataFormatter, + queryBuilder, + }); + + const onFilter = useCallback((filterValue) => { + setInputValue(filterValue); return options; - }, [options, fetchData]); - - useEffect(() => { - return () => { - fetchData.cancel(); - }; - }, []); + }, [options, setInputValue]); return ( } + emptyMessage={!inputValue && } loading={isLoading} loadingMessage={} name={name} diff --git a/lib/hooks/index.js b/lib/hooks/index.js index daf5dc20..19723f7f 100644 --- a/lib/hooks/index.js +++ b/lib/hooks/index.js @@ -9,6 +9,7 @@ export * from './useCampuses'; export * from './useCampusesQuery'; export * from './useCategories'; export * from './useContributorNameTypes'; +export * from './useDebouncedQuery'; export * from './useDefaultReceivingSearchSettings'; export * from './useEventEmitter'; export * from './useExchangeRateValue'; diff --git a/lib/hooks/useDebouncedQuery/index.js b/lib/hooks/useDebouncedQuery/index.js new file mode 100644 index 00000000..6543f80a --- /dev/null +++ b/lib/hooks/useDebouncedQuery/index.js @@ -0,0 +1 @@ +export { useDebouncedQuery } from './useDebouncedQuery'; diff --git a/lib/hooks/useDebouncedQuery/useDebouncedQuery.js b/lib/hooks/useDebouncedQuery/useDebouncedQuery.js new file mode 100644 index 00000000..76b14d82 --- /dev/null +++ b/lib/hooks/useDebouncedQuery/useDebouncedQuery.js @@ -0,0 +1,61 @@ +import debounce from 'lodash/debounce'; +import { + useMemo, + useState, +} from 'react'; +import { useQuery } from 'react-query'; + +import { + useNamespace, + useOkapiKy, +} from '@folio/stripes/core'; + +const LIST_ITEMS_LIMIT = 100; +const DEBOUNCE_DELAY = 500; + +export const useDebouncedQuery = ({ + api, + queryBuilder, + dataFormatter, + debounceDelay = DEBOUNCE_DELAY, + limit = LIST_ITEMS_LIMIT, +}) => { + const [inputValue, setInputValue] = useState(''); + const [options, setOptions] = useState([]); + const ky = useOkapiKy(); + const [namespace] = useNamespace({ key: 'locations' }); + + const debouncedSetInputValue = useMemo(() => { + return debounce((value) => setInputValue(value), debounceDelay); + }, [debounceDelay]); + + const { isLoading } = useQuery({ + queryKey: [namespace, inputValue], + queryFn: async ({ signal }) => { + if (!inputValue) return []; + + const searchParams = { + query: queryBuilder(inputValue), + limit, + }; + + const res = await ky.get(api, { searchParams, signal }).json(); + + return dataFormatter(res); + }, + enabled: Boolean(inputValue), + onSuccess: (data) => { + setOptions(data); + }, + onError: () => { + setOptions([]); + }, + }); + + return { + options, + isLoading, + inputValue, + setInputValue: debouncedSetInputValue, + }; +}; From 7e6a374b419d456dc2d2b5a43106d0f4078346b3 Mon Sep 17 00:00:00 2001 From: Alisher Musurmonov Date: Mon, 25 Nov 2024 18:32:47 +0500 Subject: [PATCH 2/4] test: fix failing tests and update changelog file with correct Jira ticket number --- CHANGELOG.md | 2 +- lib/DynamicSelection/DynamicSelection.test.js | 26 ++++++++++++++----- .../DynamicSelectionFilter.test.js | 14 +++++++--- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25cb15d2..739a4bc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ * Add more reusable hooks and utilities. Refs UISACQCOMP-228. * Move reusable version history components to the ACQ lib. Refs UISACQCOMP-230. * Move reusable helper function to support version history functionality. Refs UISACQCOMP-232. -* Add `useDebouncedQuery` hook to fix endless request for `DynamicSelection` component. Refs UIF-562. +* Add `useDebouncedQuery` hook to fix endless request for `DynamicSelection` component. Refs UISACQCOMP-233. ## [6.0.1](https://github.com/folio-org/stripes-acq-components/tree/v6.0.1) (2024-11-14) [Full Changelog](https://github.com/folio-org/stripes-acq-components/compare/v6.0.0...v6.0.1) diff --git a/lib/DynamicSelection/DynamicSelection.test.js b/lib/DynamicSelection/DynamicSelection.test.js index fffe5f1f..b8b73124 100644 --- a/lib/DynamicSelection/DynamicSelection.test.js +++ b/lib/DynamicSelection/DynamicSelection.test.js @@ -5,14 +5,20 @@ import { useOkapiKy } from '@folio/stripes/core'; import { ORDERS_API } from '../constants'; import { DynamicSelection } from './DynamicSelection'; - -jest.mock('@folio/stripes/core', () => ({ - ...jest.requireActual('@folio/stripes/core'), - useOkapiKy: jest.fn(), -})); +import { useDebouncedQuery } from '../hooks'; jest.useFakeTimers('modern'); +jest.mock('../hooks', () => ({ + ...jest.requireActual('../hooks'), + useDebouncedQuery: jest.fn(() => ({ + options: [], + isLoading: false, + inputValue: '', + setInputValue: jest.fn(), + })), +})); + const dataFormatter = ({ poLines }) => poLines.map(({ id, poLineNumber }) => ({ label: poLineNumber, value: id })); const defaultProps = { @@ -36,9 +42,17 @@ const kyMock = { })), }; +const mockSetInputValue = jest.fn(); + describe('DynamicSelection', () => { beforeEach(() => { useOkapiKy.mockClear().mockReturnValue(kyMock); + useDebouncedQuery.mockClear().mockReturnValue({ + isLoading: false, + options: [{ label: '11111', value: 'poLine-1' }], + inputValue: '', + setInputValue: mockSetInputValue, + }); }); it('should call debounced fetch function when \'onFilter\' was triggered', async () => { @@ -52,7 +66,7 @@ describe('DynamicSelection', () => { }); await user.click(screen.getByText('stripes-components.selection.controlLabel')); - expect(kyMock.get).toHaveBeenCalledWith(ORDERS_API, expect.objectContaining({})); + expect(mockSetInputValue).toHaveBeenCalledWith('1'); }); it('should call \'onChange\' when an option from list was selected', async () => { diff --git a/lib/DynamicSelectionFilter/DynamicSelectionFilter.test.js b/lib/DynamicSelectionFilter/DynamicSelectionFilter.test.js index 9c5cb6ea..4b882869 100644 --- a/lib/DynamicSelectionFilter/DynamicSelectionFilter.test.js +++ b/lib/DynamicSelectionFilter/DynamicSelectionFilter.test.js @@ -8,15 +8,21 @@ import { buildFiltersObj } from '../AcqList/utils'; import { ORDERS_API } from '../constants'; import { DynamicSelectionFilter } from './DynamicSelectionFilter'; -jest.mock('@folio/stripes/core', () => ({ - ...jest.requireActual('@folio/stripes/core'), - useOkapiKy: jest.fn(), -})); jest.mock('../AcqList/utils', () => ({ ...jest.requireActual('../AcqList/utils'), buildFiltersObj: jest.fn(), })); +jest.mock('../hooks', () => ({ + ...jest.requireActual('../hooks'), + useDebouncedQuery: jest.fn(() => ({ + options: [{ label: '11111', value: 'poLine-1' }], + isLoading: false, + inputValue: '', + setInputValue: jest.fn(), + })), +})); + jest.useFakeTimers('modern'); const dataFormatter = ({ poLines }) => poLines.map(({ id, poLineNumber }) => ({ label: poLineNumber, value: id })); From 3bfb85c27df518708ab5b1956037c5c4f36a1ecc Mon Sep 17 00:00:00 2001 From: Alisher Musurmonov Date: Mon, 25 Nov 2024 19:08:38 +0500 Subject: [PATCH 3/4] test: add test coverages --- .../useDebouncedQuery/useDebouncedQuery.js | 2 +- .../useDebouncedQuery.test.js | 67 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 lib/hooks/useDebouncedQuery/useDebouncedQuery.test.js diff --git a/lib/hooks/useDebouncedQuery/useDebouncedQuery.js b/lib/hooks/useDebouncedQuery/useDebouncedQuery.js index 76b14d82..ebc0cba3 100644 --- a/lib/hooks/useDebouncedQuery/useDebouncedQuery.js +++ b/lib/hooks/useDebouncedQuery/useDebouncedQuery.js @@ -23,7 +23,7 @@ export const useDebouncedQuery = ({ const [inputValue, setInputValue] = useState(''); const [options, setOptions] = useState([]); const ky = useOkapiKy(); - const [namespace] = useNamespace({ key: 'locations' }); + const [namespace] = useNamespace({ key: 'debounced-query' }); const debouncedSetInputValue = useMemo(() => { return debounce((value) => setInputValue(value), debounceDelay); diff --git a/lib/hooks/useDebouncedQuery/useDebouncedQuery.test.js b/lib/hooks/useDebouncedQuery/useDebouncedQuery.test.js new file mode 100644 index 00000000..265a1535 --- /dev/null +++ b/lib/hooks/useDebouncedQuery/useDebouncedQuery.test.js @@ -0,0 +1,67 @@ +import { + QueryClient, + QueryClientProvider, +} from 'react-query'; + +import { renderHook, act } from '@testing-library/react-hooks'; +import { useOkapiKy } from '@folio/stripes/core'; + +import { useDebouncedQuery } from './useDebouncedQuery'; + +const DELAY = 300; + +jest.useFakeTimers('modern'); +const mockDataFormatter = jest.fn(({ poLines }) => { + return poLines.map(({ id, poLineNumber }) => ({ label: poLineNumber, value: id })); +}); + +const queryClient = new QueryClient(); +const wrapper = ({ children }) => ( + + {children} + +); + +describe('useDebouncedQuery', () => { + beforeEach(() => { + jest.clearAllMocks(); + useOkapiKy.mockReturnValue({ + get: jest.fn(() => ({ + json: () => Promise.resolve({ poLines: [{ id: 'poLine-1', poLineNumber: '11111' }] }), + })), + }); + }); + + it('should not call `dataFormatter` and return empty []', async () => { + const { result } = renderHook(() => useDebouncedQuery({ + api: 'api', + queryBuilder: jest.fn(), + dataFormatter: mockDataFormatter, + debounceDelay: DELAY, + }), { wrapper }); + + await act(async () => { + await result.current.setInputValue(''); + jest.advanceTimersByTime(1500); + }); + + expect(mockDataFormatter).toHaveBeenCalledTimes(0); + expect(result.current.options).toEqual([]); + }); + + it('should call `dataFormatter` and return options', async () => { + const { result } = renderHook(() => useDebouncedQuery({ + api: 'api', + queryBuilder: jest.fn(), + dataFormatter: mockDataFormatter, + }), { wrapper }); + + await act(async () => { + await result.current.setInputValue('test'); + jest.advanceTimersByTime(1500); + }); + + expect(mockDataFormatter).toHaveBeenCalledTimes(1); + expect(result.current.options).toEqual([{ label: '11111', value: 'poLine-1' }]); + }); +}); From 5a40a9dc11accead96e2c65cd352f04a78717a81 Mon Sep 17 00:00:00 2001 From: Alisher Musurmonov Date: Mon, 25 Nov 2024 20:00:20 +0500 Subject: [PATCH 4/4] refactor: rename hook outputs and add default formatter --- lib/DynamicSelection/DynamicSelection.js | 10 ++++---- lib/DynamicSelection/DynamicSelection.test.js | 6 ++--- .../DynamicSelectionFilter.test.js | 4 ++-- .../useDebouncedQuery/useDebouncedQuery.js | 23 ++++++++++--------- .../useDebouncedQuery.test.js | 21 ++++++++++++++--- 5 files changed, 40 insertions(+), 24 deletions(-) diff --git a/lib/DynamicSelection/DynamicSelection.js b/lib/DynamicSelection/DynamicSelection.js index 343a6fc0..db0468f6 100644 --- a/lib/DynamicSelection/DynamicSelection.js +++ b/lib/DynamicSelection/DynamicSelection.js @@ -22,8 +22,8 @@ export const DynamicSelection = ({ const { options = initialOptions, isLoading, - inputValue, - setInputValue, + searchQuery, + setSearchQuery, } = useDebouncedQuery({ api, dataFormatter, @@ -31,15 +31,15 @@ export const DynamicSelection = ({ }); const onFilter = useCallback((filterValue) => { - setInputValue(filterValue); + setSearchQuery(filterValue); return options; - }, [options, setInputValue]); + }, [options, setSearchQuery]); return ( } + emptyMessage={!searchQuery && } loading={isLoading} loadingMessage={} name={name} diff --git a/lib/DynamicSelection/DynamicSelection.test.js b/lib/DynamicSelection/DynamicSelection.test.js index b8b73124..379b44cd 100644 --- a/lib/DynamicSelection/DynamicSelection.test.js +++ b/lib/DynamicSelection/DynamicSelection.test.js @@ -14,8 +14,8 @@ jest.mock('../hooks', () => ({ useDebouncedQuery: jest.fn(() => ({ options: [], isLoading: false, - inputValue: '', - setInputValue: jest.fn(), + searchQuery: '', + setSearchQuery: jest.fn(), })), })); @@ -51,7 +51,7 @@ describe('DynamicSelection', () => { isLoading: false, options: [{ label: '11111', value: 'poLine-1' }], inputValue: '', - setInputValue: mockSetInputValue, + setSearchQuery: mockSetInputValue, }); }); diff --git a/lib/DynamicSelectionFilter/DynamicSelectionFilter.test.js b/lib/DynamicSelectionFilter/DynamicSelectionFilter.test.js index 4b882869..563f788a 100644 --- a/lib/DynamicSelectionFilter/DynamicSelectionFilter.test.js +++ b/lib/DynamicSelectionFilter/DynamicSelectionFilter.test.js @@ -18,8 +18,8 @@ jest.mock('../hooks', () => ({ useDebouncedQuery: jest.fn(() => ({ options: [{ label: '11111', value: 'poLine-1' }], isLoading: false, - inputValue: '', - setInputValue: jest.fn(), + searchQuery: '', + setSearchQuery: jest.fn(), })), })); diff --git a/lib/hooks/useDebouncedQuery/useDebouncedQuery.js b/lib/hooks/useDebouncedQuery/useDebouncedQuery.js index ebc0cba3..fa187e79 100644 --- a/lib/hooks/useDebouncedQuery/useDebouncedQuery.js +++ b/lib/hooks/useDebouncedQuery/useDebouncedQuery.js @@ -12,30 +12,31 @@ import { const LIST_ITEMS_LIMIT = 100; const DEBOUNCE_DELAY = 500; +const DEFAULT_DATA_FORMATTER = (data) => data; export const useDebouncedQuery = ({ api, queryBuilder, - dataFormatter, + dataFormatter = DEFAULT_DATA_FORMATTER, debounceDelay = DEBOUNCE_DELAY, limit = LIST_ITEMS_LIMIT, }) => { - const [inputValue, setInputValue] = useState(''); + const [searchQuery, setSearchQuery] = useState(''); const [options, setOptions] = useState([]); + const [namespace] = useNamespace({ key: api }); const ky = useOkapiKy(); - const [namespace] = useNamespace({ key: 'debounced-query' }); - const debouncedSetInputValue = useMemo(() => { - return debounce((value) => setInputValue(value), debounceDelay); + const debounceSetSearchQuery = useMemo(() => { + return debounce((value) => setSearchQuery(value), debounceDelay); }, [debounceDelay]); const { isLoading } = useQuery({ - queryKey: [namespace, inputValue], + queryKey: [namespace, searchQuery], queryFn: async ({ signal }) => { - if (!inputValue) return []; + if (!searchQuery) return []; const searchParams = { - query: queryBuilder(inputValue), + query: queryBuilder(searchQuery), limit, }; @@ -43,7 +44,7 @@ export const useDebouncedQuery = ({ return dataFormatter(res); }, - enabled: Boolean(inputValue), + enabled: Boolean(searchQuery), onSuccess: (data) => { setOptions(data); }, @@ -55,7 +56,7 @@ export const useDebouncedQuery = ({ return { options, isLoading, - inputValue, - setInputValue: debouncedSetInputValue, + searchQuery, + setSearchQuery: debounceSetSearchQuery, }; }; diff --git a/lib/hooks/useDebouncedQuery/useDebouncedQuery.test.js b/lib/hooks/useDebouncedQuery/useDebouncedQuery.test.js index 265a1535..f2ea128b 100644 --- a/lib/hooks/useDebouncedQuery/useDebouncedQuery.test.js +++ b/lib/hooks/useDebouncedQuery/useDebouncedQuery.test.js @@ -9,6 +9,7 @@ import { useOkapiKy } from '@folio/stripes/core'; import { useDebouncedQuery } from './useDebouncedQuery'; const DELAY = 300; +const mockData = { poLines: [{ id: 'poLine-1', poLineNumber: '11111' }] }; jest.useFakeTimers('modern'); const mockDataFormatter = jest.fn(({ poLines }) => { @@ -27,7 +28,7 @@ describe('useDebouncedQuery', () => { jest.clearAllMocks(); useOkapiKy.mockReturnValue({ get: jest.fn(() => ({ - json: () => Promise.resolve({ poLines: [{ id: 'poLine-1', poLineNumber: '11111' }] }), + json: () => Promise.resolve(mockData), })), }); }); @@ -41,7 +42,7 @@ describe('useDebouncedQuery', () => { }), { wrapper }); await act(async () => { - await result.current.setInputValue(''); + await result.current.setSearchQuery(''); jest.advanceTimersByTime(1500); }); @@ -57,11 +58,25 @@ describe('useDebouncedQuery', () => { }), { wrapper }); await act(async () => { - await result.current.setInputValue('test'); + await result.current.setSearchQuery('test'); jest.advanceTimersByTime(1500); }); expect(mockDataFormatter).toHaveBeenCalledTimes(1); expect(result.current.options).toEqual([{ label: '11111', value: 'poLine-1' }]); }); + + it('should call default `dataFormatter` when `dataFormatter` is not present', async () => { + const { result } = renderHook(() => useDebouncedQuery({ + api: 'api', + queryBuilder: jest.fn(), + }), { wrapper }); + + await act(async () => { + await result.current.setSearchQuery('test'); + jest.advanceTimersByTime(1500); + }); + + expect(result.current.options).toEqual(mockData); + }); });