From 78f76ddf762dbd3734bebb3a5fe8b10a4f58539f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Mon, 13 Jan 2025 14:27:56 +0100 Subject: [PATCH] Fix: Allow to set the filter via URL query parameter Drop `usePageFilter` `locationQueryFilterString` because the query param filter gathered from `useSearchParams` is the only valid value. Drop `FilterProvider` `locationQuery` prop because a string was passed instead of the expected query object and because usePageFilter already handles its use case internally via `useSearchParams`. --- src/web/entities/__tests__/filterprovider.jsx | 34 +++++++++++++++---- src/web/entities/filterprovider.jsx | 5 --- src/web/entities/withEntitiesContainer.jsx | 8 +---- src/web/hooks/__tests__/usePageFilter.jsx | 25 +++++++++----- src/web/hooks/usePageFilter.js | 13 ++----- 5 files changed, 46 insertions(+), 39 deletions(-) diff --git a/src/web/entities/__tests__/filterprovider.jsx b/src/web/entities/__tests__/filterprovider.jsx index 0fe64b81f5..7bc3b6e5bc 100644 --- a/src/web/entities/__tests__/filterprovider.jsx +++ b/src/web/entities/__tests__/filterprovider.jsx @@ -5,6 +5,7 @@ import {describe, test, expect, testing} from '@gsa/testing'; import Filter, {DEFAULT_FALLBACK_FILTER} from 'gmp/models/filter'; +import {beforeEach, vi} from 'vitest'; import {pageFilter} from 'web/store/pages/actions'; import {defaultFilterLoadingActions} from 'web/store/usersettings/defaultfilters/actions'; import {loadingActions} from 'web/store/usersettings/defaults/actions'; @@ -12,11 +13,34 @@ import {rendererWith, screen} from 'web/utils/testing'; import FilterProvider from '../filterprovider'; +let mockSearchParams = {}; +const mockUseNavigate = vi.fn(); +const mockUseSearchParams = vi + .fn() + .mockReturnValue([ + {get: key => mockSearchParams[key]}, + {set: (key, value) => (mockSearchParams[key] = value)}, + ]); + +vi.mock('react-router-dom', async () => { + const actual = await vi.importActual('react-router-dom'); + return { + ...actual, + useLocation: () => ({pathname: '/'}), + useNavigate: () => mockUseNavigate(), + useSearchParams: () => mockUseSearchParams(), + }; +}); + +beforeEach(() => { + mockSearchParams = {}; +}); + describe('FilterProvider component tests', () => { - test('should prefer locationQuery filter over defaultSettingFilter', async () => { + test('should prefer search params filter over defaultSettingFilter', async () => { const resultingFilter = Filter.fromString('location=query rows=42'); - const locationQuery = {filter: 'location=query'}; + mockSearchParams['filter'] = 'location=query'; const defaultSettingFilter = Filter.fromString('foo=bar'); @@ -42,11 +66,7 @@ describe('FilterProvider component tests', () => { .fn() .mockReturnValue(); - render( - - {renderFunc} - , - ); + render({renderFunc}); await screen.findByTestId('awaiting-span'); diff --git a/src/web/entities/filterprovider.jsx b/src/web/entities/filterprovider.jsx index b214df7670..57bb651e48 100644 --- a/src/web/entities/filterprovider.jsx +++ b/src/web/entities/filterprovider.jsx @@ -13,11 +13,9 @@ const FilterProvider = ({ fallbackFilter, gmpname, pageName = gmpname, - locationQuery = {}, }) => { const [returnedFilter, isLoadingFilter] = usePageFilter(pageName, gmpname, { fallbackFilter, - locationQueryFilterString: locationQuery?.filter, }); return ( @@ -29,9 +27,6 @@ const FilterProvider = ({ FilterProvider.propTypes = { fallbackFilter: PropTypes.filter, gmpname: PropTypes.string, - locationQuery: PropTypes.shape({ - filter: PropTypes.string, - }), pageName: PropTypes.string, }; diff --git a/src/web/entities/withEntitiesContainer.jsx b/src/web/entities/withEntitiesContainer.jsx index de92725b3c..8acff4be97 100644 --- a/src/web/entities/withEntitiesContainer.jsx +++ b/src/web/entities/withEntitiesContainer.jsx @@ -5,7 +5,6 @@ import React from 'react'; import {connect} from 'react-redux'; -import {useSearchParams} from 'react-router-dom'; import withDownload from 'web/components/form/withDownload'; import Reload from 'web/components/loading/reload'; import withDialogNotification from 'web/components/notification/withDialogNotifiaction'; @@ -91,15 +90,10 @@ const withEntitiesContainer = )(EntitiesContainerWrapper); return props => { - const [searchParams] = useSearchParams(); return ( {({notify}) => ( - + {({filter}) => ( mockSearchParams[key]}, + {set: (key, value) => (mockSearchParams[key] = value)}, + ]); vi.mock('react-router-dom', async () => { const actual = await vi.importActual('react-router-dom'); @@ -21,14 +28,18 @@ vi.mock('react-router-dom', async () => { ...actual, useLocation: () => ({pathname: '/'}), useNavigate: () => mockUseNavigate(), + useSearchParams: () => mockUseSearchParams(), }; }); -describe('usePageFilter tests', () => { - test('should prefer locationQuery filter over defaultSettingFilter', async () => { - const locationQuery = {filter: 'location=query'}; +beforeEach(() => { + mockSearchParams = {}; +}); +describe('usePageFilter tests', () => { + test('should prefer search params filter over defaultSettingFilter', async () => { const defaultSettingFilter = Filter.fromString('foo=bar'); + mockSearchParams['filter'] = 'location=query'; const getSetting = testing.fn().mockResolvedValue({}); const gmp = { @@ -48,11 +59,7 @@ describe('usePageFilter tests', () => { defaultFilterLoadingActions.success('somePage', defaultSettingFilter), ); - const {result} = renderHook(() => - usePageFilter('somePage', 'gmpName', { - locationQueryFilterString: locationQuery.filter, - }), - ); + const {result} = renderHook(() => usePageFilter('somePage', 'gmpName')); expect(result.current[0]).toEqual( Filter.fromString('location=query rows=42'), diff --git a/src/web/hooks/usePageFilter.js b/src/web/hooks/usePageFilter.js index d6e3c4b3f3..69b685bde6 100644 --- a/src/web/hooks/usePageFilter.js +++ b/src/web/hooks/usePageFilter.js @@ -46,14 +46,7 @@ const useDefaultFilter = pageName => * filter and function to reset the filter */ -const usePageFilter = ( - pageName, - gmpName, - { - fallbackFilter, - locationQueryFilterString: initialLocationQueryFilterString, - } = {}, -) => { +const usePageFilter = (pageName, gmpName, {fallbackFilter} = {}) => { const gmp = useGmp(); const dispatch = useDispatch(); const [searchParams, setSearchParams] = useSearchParams(); @@ -73,9 +66,7 @@ const usePageFilter = ( // use null as value for not set at all let returnedFilter; - // only use searchParams directly if locationQueryFilterString is undefined - const locationQueryFilterString = - initialLocationQueryFilterString || searchParams.get('filter'); + const locationQueryFilterString = searchParams.get('filter'); const [locationQueryFilter, setLocationQueryFilter] = useState( hasValue(locationQueryFilterString)