From e9c5856431d1477c4abf5961408a374a1d6b6127 Mon Sep 17 00:00:00 2001 From: Alessio Biancalana Date: Mon, 29 May 2023 16:39:33 +0200 Subject: [PATCH] Persist selected filters in checks results inside Redux (#1445) Save filters in redux when filtering checks results --- .../ExecutionResults/ChecksResultFilters.jsx | 20 +++++++-- .../ExecutionResults/ExecutionHeader.jsx | 8 +++- .../ExecutionResults/ExecutionResults.jsx | 12 ++++++ .../ExecutionResults.test.jsx | 42 +++++++++++++++++++ .../ExecutionResults/ExecutionResultsPage.jsx | 8 ++++ assets/js/state/checksResultsFilters.js | 17 ++++++++ assets/js/state/checksResultsFilters.test.js | 24 +++++++++++ assets/js/state/index.js | 3 ++ .../state/selectors/checksResultsFilters.js | 4 ++ .../selectors/checksResultsFilters.test.js | 25 +++++++++++ 10 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 assets/js/state/checksResultsFilters.js create mode 100644 assets/js/state/checksResultsFilters.test.js create mode 100644 assets/js/state/selectors/checksResultsFilters.js create mode 100644 assets/js/state/selectors/checksResultsFilters.test.js diff --git a/assets/js/components/ExecutionResults/ChecksResultFilters.jsx b/assets/js/components/ExecutionResults/ChecksResultFilters.jsx index cc66e4a3e7..5e17f50682 100644 --- a/assets/js/components/ExecutionResults/ChecksResultFilters.jsx +++ b/assets/js/components/ExecutionResults/ChecksResultFilters.jsx @@ -4,14 +4,25 @@ import Filter from '@components/Table/Filter'; export const RESULT_FILTER_FIELD = 'result'; -function ChecksResultFilters({ onChange }) { +const defaultSavedFilters = []; + +const getFilters = (savedFilters, searchParams) => + savedFilters.length >= 0 && searchParams.getAll('health').length === 0 + ? savedFilters + : searchParams.getAll('health'); + +function ChecksResultFilters({ + savedFilters = defaultSavedFilters, + onChange = () => {}, + onSave = () => {}, +}) { const [filtersForField, setFiltersForField] = useState({}); const [searchParams, setSearchParams] = useSearchParams(); // This structure is the foundation for a multi field filters // we can reuse later this structure in other parts of the application useEffect(() => { - const selectedFilters = searchParams.getAll('health'); + const selectedFilters = getFilters(savedFilters, searchParams); setFiltersForField({ RESULT_FILTER_FIELD: { @@ -21,7 +32,7 @@ function ChecksResultFilters({ onChange }) { values: selectedFilters, }, }); - }, [searchParams]); + }, [searchParams, savedFilters]); useEffect(() => { if (Object.keys(filtersForField).length >= 0) { @@ -40,8 +51,9 @@ function ChecksResultFilters({ onChange }) { key={RESULT_FILTER_FIELD} title="checks result" options={['passing', 'warning', 'critical', 'unknown']} - value={searchParams.getAll('health')} + value={getFilters(savedFilters, searchParams)} onChange={(list) => { + onSave(list); setSearchParams(createSearchParams({ health: list })); }} /> diff --git a/assets/js/components/ExecutionResults/ExecutionHeader.jsx b/assets/js/components/ExecutionResults/ExecutionHeader.jsx index 13f6caae9d..2faeb8d9d9 100644 --- a/assets/js/components/ExecutionResults/ExecutionHeader.jsx +++ b/assets/js/components/ExecutionResults/ExecutionHeader.jsx @@ -13,7 +13,9 @@ function ExecutionHeader({ clusterName, cloudProvider, clusterScenario, + savedFilters, onFilterChange = () => {}, + onFilterSave = () => {}, }) { return ( <> @@ -31,7 +33,11 @@ function ExecutionHeader({ {clusterName} - + {cloudProvider === UNKNOWN_PROVIDER && ( diff --git a/assets/js/components/ExecutionResults/ExecutionResults.jsx b/assets/js/components/ExecutionResults/ExecutionResults.jsx index 2283584f70..cc23ec507f 100644 --- a/assets/js/components/ExecutionResults/ExecutionResults.jsx +++ b/assets/js/components/ExecutionResults/ExecutionResults.jsx @@ -23,6 +23,13 @@ import CheckResultOutline from './CheckResultOutline'; import ExecutionHeader from './ExecutionHeader'; import ExecutionContainer from './ExecutionContainer'; +// To have an array as a default prop that is also used in a useEffect's dependency +// array we need to declare it outside the scope as a `const`, in order to prevent +// rerendering loops. +// +// https://github.com/facebook/react/issues/18123 +const defaultSavedFilters = []; + const resultsTableConfig = { usePadding: false, rowClassName: 'tn-check-result-row', @@ -101,9 +108,11 @@ function ExecutionResults({ executionData, executionError, clusterSelectedChecks = [], + savedFilters = defaultSavedFilters, onCatalogRefresh = () => {}, onLastExecutionUpdate = () => {}, onStartExecution = () => {}, + onSaveFilters = () => {}, }) { const [predicates, setPredicates] = useState([]); const [selectedCheck, setSelectedCheck] = useState(null); @@ -117,6 +126,7 @@ function ExecutionResults({ onLastExecutionUpdate(); } }; + const checksResults = getCheckResults(executionData); const catalogCategoryList = getCatalogCategoryList(catalog, checksResults); const tableData = checksResults @@ -168,7 +178,9 @@ function ExecutionResults({ clusterName={clusterName} cloudProvider={cloudProvider} clusterScenario={clusterScenario} + savedFilters={savedFilters} onFilterChange={(newPredicates) => setPredicates(newPredicates)} + onFilterSave={onSaveFilters} /> { expect(screen.queryByText(checkID2)).toBeNull(); }); + it("should render ExecutionResults with saved 'passing' filter", async () => { + const { + clusterID, + clusterHosts, + checks: [checkID1, checkID2], + loading, + catalog, + error, + executionLoading, + executionData, + executionError, + executionStarted, + } = prepareStateData('passing'); + + renderWithRouter( + + ); + + expect(screen.getAllByText('test-cluster')).toHaveLength(2); + expect(screen.getByText('HANA scale-up')).toBeTruthy(); + expect(screen.getByText('Azure')).toBeTruthy(); + expect(screen.getByText(clusterHosts[0].hostname)).toBeTruthy(); + expect(screen.getByText(clusterHosts[1].hostname)).toBeTruthy(); + expect(screen.getAllByText(checkID1)).toHaveLength(1); + expect(screen.queryByText(checkID2)).toBeNull(); + }); + it("should render ExecutionResults with successfully filtered 'passing' and 'critical' results", async () => { const { clusterID, @@ -518,6 +559,7 @@ describe('ExecutionResults', () => { executionData, checks, } = prepareStateData('completed'); + renderWithRouter( { @@ -62,9 +66,13 @@ function ExecutionResultsPage() { executionData={executionData} executionError={executionError} clusterSelectedChecks={cluster?.selected_checks} + savedFilters={savedFilters} onStartExecution={(clusterId, hosts, selectedChecks, navigate) => dispatch(executionRequested(clusterId, hosts, selectedChecks, navigate)) } + onSaveFilters={(filters) => + dispatch(setSelectedFilters({ resourceID: clusterID, filters })) + } /> ); } diff --git a/assets/js/state/checksResultsFilters.js b/assets/js/state/checksResultsFilters.js new file mode 100644 index 0000000000..5671ebb97b --- /dev/null +++ b/assets/js/state/checksResultsFilters.js @@ -0,0 +1,17 @@ +import { createSlice } from '@reduxjs/toolkit'; + +const initialState = {}; + +export const checksResultsFiltersSlice = createSlice({ + name: 'checksResultsFilters', + initialState, + reducers: { + setSelectedFilters: (state, { payload: { resourceID, filters } }) => { + state[resourceID] = filters; + }, + }, +}); + +export const { setSelectedFilters } = checksResultsFiltersSlice.actions; + +export default checksResultsFiltersSlice.reducer; diff --git a/assets/js/state/checksResultsFilters.test.js b/assets/js/state/checksResultsFilters.test.js new file mode 100644 index 0000000000..97fbb0de1a --- /dev/null +++ b/assets/js/state/checksResultsFilters.test.js @@ -0,0 +1,24 @@ +import { faker } from '@faker-js/faker'; +import checksResultsFiltersReducer, { + setSelectedFilters, +} from './checksResultsFilters'; + +describe('Catalog reducer', () => { + it('should set catalog on loading state', () => { + const initialState = {}; + + const resourceID = faker.datatype.uuid(); + + const filters = ['warning']; + + const action = setSelectedFilters({ resourceID, filters }); + + const expectedState = { + [resourceID]: filters, + }; + + expect(checksResultsFiltersReducer(initialState, action)).toEqual( + expectedState + ); + }); +}); diff --git a/assets/js/state/index.js b/assets/js/state/index.js index fb8aa63952..3d92a4585d 100644 --- a/assets/js/state/index.js +++ b/assets/js/state/index.js @@ -1,9 +1,11 @@ import { configureStore } from '@reduxjs/toolkit'; import createSagaMiddleware from 'redux-saga'; + import sapSystemsHealthSummaryReducer from './healthSummary'; import hostsListReducer from './hosts'; import clustersListReducer from './clusters'; import clusterChecksSelectionReducer from './clusterChecksSelection'; +import checksResultsFiltersReducer from './checksResultsFilters'; import sapSystemListReducer from './sapSystems'; import databasesListReducer from './databases'; import catalogReducer from './catalog'; @@ -21,6 +23,7 @@ export const store = configureStore({ hostsList: hostsListReducer, clustersList: clustersListReducer, clusterChecksSelection: clusterChecksSelectionReducer, + checksResultsFilters: checksResultsFiltersReducer, sapSystemsList: sapSystemListReducer, databasesList: databasesListReducer, catalog: catalogReducer, diff --git a/assets/js/state/selectors/checksResultsFilters.js b/assets/js/state/selectors/checksResultsFilters.js new file mode 100644 index 0000000000..0eefcfedd9 --- /dev/null +++ b/assets/js/state/selectors/checksResultsFilters.js @@ -0,0 +1,4 @@ +export const getSelectedFilters = + (resourceID) => + ({ checksResultsFilters }) => + checksResultsFilters[resourceID] || []; diff --git a/assets/js/state/selectors/checksResultsFilters.test.js b/assets/js/state/selectors/checksResultsFilters.test.js new file mode 100644 index 0000000000..75f8aa2b75 --- /dev/null +++ b/assets/js/state/selectors/checksResultsFilters.test.js @@ -0,0 +1,25 @@ +import { faker } from '@faker-js/faker'; + +import { getSelectedFilters } from './checksResultsFilters'; + +describe('getSelectedFilters', () => { + it('should return an empty array if the cluster ID is not found', () => { + const resourceID = faker.datatype.uuid(); + + expect( + getSelectedFilters(resourceID)({ checksResultsFilters: {} }) + ).toEqual([]); + }); + + it('should return a list of selected filters when the cluster ID is found', () => { + const resourceID = faker.datatype.uuid(); + const state = { + checksResultsFilters: { [resourceID]: ['passing', 'critical'] }, + }; + + expect(getSelectedFilters(resourceID)(state)).toEqual([ + 'passing', + 'critical', + ]); + }); +});