Skip to content

Commit

Permalink
Persist selected filters in checks results inside Redux (#1445)
Browse files Browse the repository at this point in the history
Save filters in redux when filtering checks results
  • Loading branch information
dottorblaster authored May 29, 2023
1 parent 5a499bf commit e9c5856
Show file tree
Hide file tree
Showing 10 changed files with 158 additions and 5 deletions.
20 changes: 16 additions & 4 deletions assets/js/components/ExecutionResults/ChecksResultFilters.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -21,7 +32,7 @@ function ChecksResultFilters({ onChange }) {
values: selectedFilters,
},
});
}, [searchParams]);
}, [searchParams, savedFilters]);

useEffect(() => {
if (Object.keys(filtersForField).length >= 0) {
Expand All @@ -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 }));
}}
/>
Expand Down
8 changes: 7 additions & 1 deletion assets/js/components/ExecutionResults/ExecutionHeader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ function ExecutionHeader({
clusterName,
cloudProvider,
clusterScenario,
savedFilters,
onFilterChange = () => {},
onFilterSave = () => {},
}) {
return (
<>
Expand All @@ -31,7 +33,11 @@ function ExecutionHeader({
{clusterName}
</span>
</h1>
<ChecksResultFilters onChange={onFilterChange} />
<ChecksResultFilters
savedFilters={savedFilters}
onChange={onFilterChange}
onSave={onFilterSave}
/>
</div>
{cloudProvider === UNKNOWN_PROVIDER && (
<WarningBanner>
Expand Down
12 changes: 12 additions & 0 deletions assets/js/components/ExecutionResults/ExecutionResults.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -101,9 +108,11 @@ function ExecutionResults({
executionData,
executionError,
clusterSelectedChecks = [],
savedFilters = defaultSavedFilters,
onCatalogRefresh = () => {},
onLastExecutionUpdate = () => {},
onStartExecution = () => {},
onSaveFilters = () => {},
}) {
const [predicates, setPredicates] = useState([]);
const [selectedCheck, setSelectedCheck] = useState(null);
Expand All @@ -117,6 +126,7 @@ function ExecutionResults({
onLastExecutionUpdate();
}
};

const checksResults = getCheckResults(executionData);
const catalogCategoryList = getCatalogCategoryList(catalog, checksResults);
const tableData = checksResults
Expand Down Expand Up @@ -168,7 +178,9 @@ function ExecutionResults({
clusterName={clusterName}
cloudProvider={cloudProvider}
clusterScenario={clusterScenario}
savedFilters={savedFilters}
onFilterChange={(newPredicates) => setPredicates(newPredicates)}
onFilterSave={onSaveFilters}
/>
<ResultsContainer
error={catalogError || executionError}
Expand Down
42 changes: 42 additions & 0 deletions assets/js/components/ExecutionResults/ExecutionResults.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,47 @@ describe('ExecutionResults', () => {
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(
<ExecutionResults
clusterID={clusterID}
clusterName="test-cluster"
clusterScenario="hana_scale_up"
cloudProvider="azure"
clusterHosts={clusterHosts}
catalogLoading={loading}
catalog={catalog}
executionStarted={executionStarted}
catalogError={error}
executionLoading={executionLoading}
executionData={executionData}
executionError={executionError}
savedFilters={['passing']}
/>
);

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,
Expand Down Expand Up @@ -518,6 +559,7 @@ describe('ExecutionResults', () => {
executionData,
checks,
} = prepareStateData('completed');

renderWithRouter(
<ExecutionResults
clusterID={clusterID}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { useParams } from 'react-router-dom';
import { setSelectedFilters } from '@state/checksResultsFilters';
import { getSelectedFilters } from '@state/selectors/checksResultsFilters';
import { getLastExecutionData } from '@state/selectors/lastExecutions';
import { updateCatalog } from '@state/actions/catalog';
import {
Expand Down Expand Up @@ -29,6 +31,8 @@ function ExecutionResultsPage() {
},
} = useSelector(getLastExecutionData(clusterID));

const savedFilters = useSelector(getSelectedFilters(clusterID));

const cloudProvider = cluster?.provider;

useEffect(() => {
Expand Down Expand Up @@ -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 }))
}
/>
);
}
Expand Down
17 changes: 17 additions & 0 deletions assets/js/state/checksResultsFilters.js
Original file line number Diff line number Diff line change
@@ -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;
24 changes: 24 additions & 0 deletions assets/js/state/checksResultsFilters.test.js
Original file line number Diff line number Diff line change
@@ -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
);
});
});
3 changes: 3 additions & 0 deletions assets/js/state/index.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -21,6 +23,7 @@ export const store = configureStore({
hostsList: hostsListReducer,
clustersList: clustersListReducer,
clusterChecksSelection: clusterChecksSelectionReducer,
checksResultsFilters: checksResultsFiltersReducer,
sapSystemsList: sapSystemListReducer,
databasesList: databasesListReducer,
catalog: catalogReducer,
Expand Down
4 changes: 4 additions & 0 deletions assets/js/state/selectors/checksResultsFilters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const getSelectedFilters =
(resourceID) =>
({ checksResultsFilters }) =>
checksResultsFilters[resourceID] || [];
25 changes: 25 additions & 0 deletions assets/js/state/selectors/checksResultsFilters.test.js
Original file line number Diff line number Diff line change
@@ -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',
]);
});
});

0 comments on commit e9c5856

Please sign in to comment.