Skip to content

Commit

Permalink
fix(SystemsPage): RHINENG-5977 - Fix URL params filters handling
Browse files Browse the repository at this point in the history
  • Loading branch information
bastilian committed Aug 15, 2024
1 parent 8c74914 commit 20314d9
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 72 deletions.
4 changes: 3 additions & 1 deletion src/Components/SmartComponents/CVEs/CVEs.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import { Alert, Stack, StackItem } from '@patternfly/react-core';
import PropTypes from 'prop-types';
import { CVES_ALLOWED_PARAMS, PATCHMAN_ADVISORY_DOCS_PATH, PERMISSIONS, SERVICE_NAME } from '../../../Helpers/constants';
import { createCveListByAccount } from '../../../Helpers/VulnerabilityHelper';
import { constructFilterParameters, useUrlParams } from '../../../Helpers/MiscHelper';
import { constructFilterParameters } from '../../../Helpers/MiscHelper';
import useUrlParams from '../../../Helpers/useUrlParams';

import BusinessRiskModal from '../Modals/BusinessRiskModal';
import StatusModal from '../Modals/CveStatusModal';
import CVEsTable from './CVEsTable';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useEffect } from 'react';
import propTypes from 'prop-types';
import { injectIntl } from 'react-intl';
import { useUrlParams } from '../../../../Helpers/MiscHelper';
import useUrlParams from '../../../../Helpers/useUrlParams';
import { useDispatch, useSelector } from 'react-redux';
import useInsightsNavigate from '@redhat-cloud-services/frontend-components-utilities/useInsightsNavigate';
import ReducerRegistry from '../../../../Utilities/ReducerRegistry';
Expand Down Expand Up @@ -47,7 +47,9 @@ const ImmutableDevices = ({ intl, cveName, filterRuleValues, inventoryRef, heade

useEffect(() => apply(urlParameters), []);

useEffect(() => setUrlParams({ ...parameters, ...headerFilters }), [parameters, headerFilters]);
useEffect(() => (
setUrlParams({ ...parameters, ...headerFilters })
), [JSON.stringify(parameters), JSON.stringify(headerFilters)]);

const getEntities = useGetEntities(
{
Expand Down
3 changes: 2 additions & 1 deletion src/Components/SmartComponents/SystemCves/SystemCves.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ import { useDispatch, useSelector } from 'react-redux';
import { CVES_ALLOWED_PARAMS, SYSTEM_DETAILS_HEADER } from '../../../Helpers/constants';
import {
constructFilterParameters,
useUrlParams,
updateRef
} from '../../../Helpers/MiscHelper';
import useUrlParams from '../../../Helpers/useUrlParams';

import { createCveListBySystem } from '../../../Helpers/VulnerabilityHelper';
import {
Stack,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import propTypes from 'prop-types';
import { injectIntl } from 'react-intl';
import messages from '../../../Messages';
import { translateUrlSortParameter, useUrlParams } from '../../../Helpers/MiscHelper';
import { translateUrlSortParameter } from '../../../Helpers/MiscHelper';
import useUrlParams from '../../../Helpers/useUrlParams';
import { useDispatch, useSelector, shallowEqual } from 'react-redux';
import React, { Fragment, useEffect, useState } from 'react';
import CvePairStatusModal from '../Modals/CvePairStatusModal';
Expand Down
8 changes: 3 additions & 5 deletions src/Components/SmartComponents/SystemsPage/SystemsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import {
fetchSystems,
fetchSystemsIds
} from '../../../Store/Actions/Actions';
import { translateUrlSortParameter, useUrlParams } from '../../../Helpers/MiscHelper';
import { translateUrlSortParameter } from '../../../Helpers/MiscHelper';
import useUrlParams from '../../../Helpers/useUrlParams';
import { InventoryTable } from '@redhat-cloud-services/frontend-components/Inventory';
import ErrorHandler from '../../PresentationalComponents/ErrorHandler/ErrorHandler';
import { TableVariant } from '@patternfly/react-table';
Expand Down Expand Up @@ -63,7 +64,6 @@ const SystemsPage = () => {

const items = useSelector(({ entities }) => entities?.rows || [], shallowEqual);
const totalItems = useSelector(({ entities }) => entities?.total);
const meta = useSelector(({ entities }) => entities?.meta);
const selectedRows = useSelector(({ entities }) => entities?.selectedRows || []);
const selectedRowsCount = useSelector(({ entities }) => entities?.selectedRowsCount ?? 0);
const isLoaded = useSelector(({ entities }) => entities?.loaded || false);
Expand Down Expand Up @@ -101,8 +101,6 @@ const SystemsPage = () => {

useEffect(() => apply(urlParameters), []);

useEffect(() => setUrlParams({ ...parameters, ...meta }), [setUrlParams, parameters, meta]);

const handleSelect = (payload, selecting) => dispatch(selectRows(payload, selecting));

const onRefreshInventory = () => {
Expand All @@ -114,7 +112,7 @@ const SystemsPage = () => {
};

const doOptOut = useOptOutSystems(onRefreshInventory);
const getEntities = useGetEntities(APIHelper.getSystems, {});
const getEntities = useGetEntities(APIHelper.getSystems, { setUrlParams });

const [columnCounter, setColumnCount] = useState(0);
useEffect(() => setColumnCount(columnCounter + 1), [columns]);
Expand Down
31 changes: 0 additions & 31 deletions src/Helpers/MiscHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import findIndex from 'lodash/findIndex';
import propTypes from 'prop-types';
import React from 'react';
import { impactList, PUBLIC_DATE_OPTIONS } from './constants';
import qs from 'query-string';
import { coerce, compare, rcompare } from 'semver';
import cloneDeep from 'lodash/cloneDeep';
import { Icon } from '@patternfly/react-core';
Expand Down Expand Up @@ -54,24 +53,6 @@ export function constructParameters(apiProps, allowedParams, shouldUseHybridSyst
return [];
}

// TODO DRY:similar to constructParameters
export function constructURLParameters(urlParams, allowedParams) {
if (urlParams) {
const params = { ...urlParams };
Object.keys(urlParams).forEach(
key => (
params[key] === undefined
|| params[key] === ''
|| !allowedParams.includes(key)
|| params[key] === false
)
&& delete params[key]

);
return params;
}
}

export function formatDate(date = new Date(), includeTime = false) {
const prepend = (number) => `${`${number}`.length === 1 ? '0' : ''}${number}`;
const toFormat = new Date(date);
Expand Down Expand Up @@ -173,18 +154,6 @@ export const updateStateSet = (stateSet, names, value) => {
return stateSet;
};

export const useUrlParams = (allowedParams) => {
const url = new URL(window.location);
const urlParams = qs.parse(url.search);

const setUrlParams = (parameters) => {
const searchParams = qs.stringify(constructURLParameters(parameters, allowedParams));
window.history.replaceState(null, null, `${url.origin}${url.pathname}?${searchParams}${url.hash}`);
};

return [urlParams, setUrlParams];
};

// TODO: Refactor
export const updateRef = (meta, params, apply) => {
const pages = parseInt(meta.pages);
Expand Down
32 changes: 1 addition & 31 deletions src/Helpers/MiscHelper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,26 +169,12 @@ describe('MiscHelper', () => {
${{ publish_date: 'last90' }} | ${{ public_from: formatDate(subtractDays(90)), public_to: undefined, publish_date: 'last90' }}
${{ publish_date: 'lastYear' }} | ${{ public_from: formatDate(subtractYears(1)), public_to: undefined, publish_date: 'lastYear' }}
${{ publish_date: 'MoreThanYear' }} | ${{ public_from: undefined, public_to: formatDate(subtractYears(1)), publish_date: 'MoreThanYear' }}
`('constructFilterParameters - publish_filter', ({ publish_filter, expected_data }) => {
const filter = constructFilterParameters(publish_filter);
expect(filter).toEqual(expected_data);
});

it('useUrlParams', () => {
const mockLocation = new URL('https://localhost:1337/insights/vulnerability/cves?page=1&severity=3');
window.location = mockLocation;

const [urlParams, setUrlParams] = useUrlParams(['a', 'b']);
setUrlParams({ filter: 'testFilter' });

expect(urlParams).toEqual({
page: '1',
severity: '3'
});
expect(mockHistory.replaceState).toHaveBeenCalled();
});

it('Should updateRef update with the same page', () => {
const testParameters = { page: 10 };
const testMeta = { page: 10, pages: 10, cvesCount: 10 };
Expand Down Expand Up @@ -217,20 +203,4 @@ describe('MiscHelper', () => {
const result = formatDate();
expect(result).toEqual(expect.anything());
});

it.each`
urlParams | expected_data
${undefined} | ${undefined}
${{}} | ${{}}
${{ a: 'testValue', b: undefined }} | ${{ a: 'testValue' }}
${{ a: false, b: 'testValue' }} | ${{ b: 'testValue' }}
${{ a: 'testValue', c: 'testValue' }} | ${{ a: 'testValue' }}
${{ a: '', b: 'testValue' }} | ${{ b: 'testValue' }}
`('constructURLParameters', ({ urlParams, expected_data }) => {
const result = constructURLParameters(urlParams, ['a', 'b']);
expect(result).toEqual(expected_data);
});

});
34 changes: 34 additions & 0 deletions src/Helpers/useUrlParams.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { useCallback } from 'react';
import { useSearchParams } from 'react-router-dom';

// TODO DRY:similar to constructParameters
export function constructURLParameters(urlParams, allowedParams) {
if (urlParams) {
const params = { ...urlParams };
Object.keys(urlParams).forEach(
key => (
params[key] === undefined
|| params[key] === ''
|| !allowedParams.includes(key)
|| params[key] === false
)
&& delete params[key]

);
return params;
}
}

export const useUrlParams = (allowedParams) => {
const [searchParams, setSearchParams] = useSearchParams();

const setUrlParams = useCallback((parameters) => {
const para = constructURLParameters(parameters, allowedParams);

setSearchParams(para);
}, [JSON.stringify(allowedParams)]);

return [searchParams, setUrlParams];
};

export default useUrlParams;
35 changes: 35 additions & 0 deletions src/Helpers/useUrlParams.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { renderHook, act } from '@testing-library/react';
import { createTestWrapper } from '../Utilities/TestWrapper';
import { MemoryRouter } from 'react-router-dom';

import useUrlParams, { constructURLParameters } from './useUrlParams';

describe('MiscHelper', () => {
it('useUrlParams', async () => {
const { result } = renderHook(() => useUrlParams(['a', 'b']), {
wrapper: createTestWrapper(MemoryRouter, { initialEntries: ['/'] })
});

await act(()=> {
result.current[1]({ a: 'testValue', b: 'testValue', c: 'testValue' });
});

expect(result.current[0].get('a')).toEqual('testValue');
expect(result.current[0].get('b')).toEqual('testValue');
expect(result.current[0].get('c')).toEqual(null);
});

it.each`
urlParams | expected_data
${undefined} | ${undefined}
${{}} | ${{}}
${{ a: 'testValue', b: undefined }} | ${{ a: 'testValue' }}
${{ a: false, b: 'testValue' }} | ${{ b: 'testValue' }}
${{ a: 'testValue', c: 'testValue' }} | ${{ a: 'testValue' }}
${{ a: '', b: 'testValue' }} | ${{ b: 'testValue' }}
`('constructURLParameters', ({ urlParams, expected_data }) => { // eslint-disable-line camelcase

const result = constructURLParameters(urlParams, ['a', 'b']);
expect(result).toEqual(expected_data);
});
});
6 changes: 6 additions & 0 deletions src/Utilities/TestWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,10 @@ TestWrapper.propTypes = {
store: propTypes.object
};

export const createTestWrapper = (Wrapper = TestWrapper, props) => {
return function CreatedWrapper({ children }) { // eslint-disable-line
return <Wrapper {...props}>{children}</Wrapper>;
};
};

export default TestWrapper;

0 comments on commit 20314d9

Please sign in to comment.