Skip to content

Commit

Permalink
Fix: Allow to set the filter via URL query parameter
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
bjoernricks committed Jan 14, 2025
1 parent a54ad65 commit 010a144
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 43 deletions.
34 changes: 27 additions & 7 deletions src/web/entities/__tests__/filterprovider.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,42 @@

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

Expand All @@ -42,11 +66,7 @@ describe('FilterProvider component tests', () => {
.fn()
.mockReturnValue(<span data-testid="awaiting-span" />);

render(
<FilterProvider gmpname="task" locationQuery={locationQuery}>
{renderFunc}
</FilterProvider>,
);
render(<FilterProvider gmpname="task">{renderFunc}</FilterProvider>);

await screen.findByTestId('awaiting-span');

Expand Down
5 changes: 0 additions & 5 deletions src/web/entities/filterprovider.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@ const FilterProvider = ({
fallbackFilter,
gmpname,
pageName = gmpname,
locationQuery = {},
}) => {
const [returnedFilter, isLoadingFilter] = usePageFilter(pageName, gmpname, {
fallbackFilter,
locationQueryFilterString: locationQuery?.filter,
});
return (
<React.Fragment>
Expand All @@ -29,9 +27,6 @@ const FilterProvider = ({
FilterProvider.propTypes = {
fallbackFilter: PropTypes.filter,
gmpname: PropTypes.string,
locationQuery: PropTypes.shape({
filter: PropTypes.string,
}),
pageName: PropTypes.string,
};

Expand Down
8 changes: 1 addition & 7 deletions src/web/entities/withEntitiesContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -91,15 +90,10 @@ const withEntitiesContainer =
)(EntitiesContainerWrapper);

return props => {
const [searchParams] = useSearchParams();
return (
<SubscriptionProvider>
{({notify}) => (
<FilterProvider
fallbackFilter={fallbackFilter}
gmpname={gmpname}
locationQuery={searchParams.toString()}
>
<FilterProvider fallbackFilter={fallbackFilter} gmpname={gmpname}>
{({filter}) => (
<EntitiesContainerWrapper
{...props}
Expand Down
31 changes: 18 additions & 13 deletions src/web/hooks/__tests__/usePageFilter.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,43 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/



import {describe, test, expect, testing} from '@gsa/testing';
import Filter, {DEFAULT_FALLBACK_FILTER} from 'gmp/models/filter';
import {vi} from 'vitest';
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';
import {rendererWith, waitFor} from 'web/utils/testing';

import usePageFilter from '../usePageFilter';


const mockUseNavigate = testing.fn();
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(),
};
});

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 = {
Expand All @@ -51,16 +59,13 @@ 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'),
);
});

test('should prefer pageFilter over defaultSettingFilter', async () => {
const pFilter = Filter.fromString('page=filter');
const defaultSettingFilter = Filter.fromString('foo=bar');
Expand Down
13 changes: 2 additions & 11 deletions src/web/hooks/usePageFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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)
Expand Down

0 comments on commit 010a144

Please sign in to comment.