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 Jul 26, 2024
1 parent 8c74914 commit 6811d96
Show file tree
Hide file tree
Showing 41 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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Diff not rendered.
Diff not rendered.
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 6811d96

Please sign in to comment.