Skip to content

Commit

Permalink
fix(Tables): Reset filters resets only filters (#450)
Browse files Browse the repository at this point in the history
Fixes https://issues.redhat.com/browse/OCPADVISOR-6. 

When you click on "Reset filters" values set for pagination and sorting get also reset. This is on every table in the app.

This PR ensures that on "Reset filters" click, only filters get reset and not pagination or sorting.
  • Loading branch information
johnsonm325 authored and gkarat committed Nov 28, 2022
1 parent e72946f commit 61f9e8b
Show file tree
Hide file tree
Showing 12 changed files with 127 additions and 11 deletions.
2 changes: 2 additions & 0 deletions cypress/utils/components.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const CHIP = '[data-ouia-component-type="PF4/Chip"]';
const ROW =
'[data-ouia-component-type="PF4/TableRow"]:not([class~="pf-c-table__expandable-row"])';
const PAGINATION = 'div[data-ouia-component-type="PF4/Pagination"]';
const PAGINATION_BOTTOM = '.pf-m-bottom';
const PAGINATION_MENU =
'div[data-ouia-component-type="PF4/PaginationOptionsMenu"]';
const DROPDOWN = '[data-ouia-component-type="PF4/Dropdown"]';
Expand All @@ -27,6 +28,7 @@ export {
CHIP,
ROW,
PAGINATION,
PAGINATION_BOTTOM,
PAGINATION_MENU,
DROPDOWN,
MODAL,
Expand Down
20 changes: 20 additions & 0 deletions cypress/utils/pagination.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { DEFAULT_ROW_COUNT } from './defaults';
import {
TOOLBAR,
PAGINATION,
PAGINATION_MENU,
DROPDOWN_TOGGLE,
DROPDOWN_ITEM,
Expand All @@ -27,6 +28,23 @@ function checkPaginationTotal(n) {
.should('have.text', n);
}

function checkCurrentPage(page) {
cy.get(PAGINATION)
.find('[aria-label="Current page"]')
.should('have.value', `${page}`);
}

function checkPaginationSelected(index) {
cy.get(TOOLBAR).find(PAGINATION_MENU).find(DROPDOWN_TOGGLE).click();
cy.get(TOOLBAR)
.find(PAGINATION_MENU)
.find('ul[class=pf-c-options-menu__menu]')
.children()
.eq(index)
.find('button')
.should('have.class', 'pf-m-selected pf-c-options-menu__menu-item');
}

function checkPaginationValues(expectedValues) {
cy.get(TOOLBAR).find(PAGINATION_MENU).find(DROPDOWN_TOGGLE).click();
cy.get(TOOLBAR)
Expand All @@ -52,6 +70,8 @@ function changePagination(textInItem) {
export {
itemsPerPage,
checkPaginationTotal,
checkCurrentPage,
checkPaginationSelected,
checkPaginationValues,
changePagination,
};
3 changes: 1 addition & 2 deletions cypress/utils/table.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _ from 'lodash';

import { ROW, TBODY, TABLE, TITLE, CHIP_GROUP, CHIP } from './components';
import { removeAllChips, applyFilters, filter } from './filters';
import { removeAllChips, applyFilters } from './filters';

function checkTableHeaders(expectedHeaders) {
/* patternfly/react-table-4.71.16, for some reason, renders extra empty `th` container;
Expand Down Expand Up @@ -131,7 +131,6 @@ function checkFiltering(
cy.get('button').contains('Reset filters').should('exist');
}


/**
* A column is sorted and then data and a reference column (which can be different than the
* sorting one) are compared to see if order matches
Expand Down
22 changes: 22 additions & 0 deletions src/Components/AffectedClustersTable/AffectedClustersTable.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ import {
import {
itemsPerPage,
checkPaginationTotal,
checkPaginationSelected,
checkPaginationValues,
changePagination,
checkCurrentPage,
} from '../../../cypress/utils/pagination';
import rule from '../../../cypress/fixtures/api/insights-results-aggregator/v2/rule/external.rules.rule|ERROR_KEY.json';
import { AFFECTED_CLUSTERS_COLUMNS } from '../../AppConstants';
Expand Down Expand Up @@ -522,6 +524,26 @@ describe('non-empty successful affected clusters table', () => {
checkRowCounts(Math.min(DEFAULT_ROW_COUNT, filterData({}).length));
});

it('will reset filters but not pagination and sorting', () => {
filterApply({ name: 'a' });
changePagination(PAGINATION_VALUES[0]);
cy.get(TOOLBAR)
.find(PAGINATION)
.find('button[data-action="next"]')
.then(($button) => {
cy.wrap($button).click();
});

cy.get('th[data-label="Name"]').find('button').click();
cy.get(TOOLBAR).find('button').contains('Reset filters').click();
cy.get(TOOLBAR).find(CHIP_GROUP).should('not.exist');
checkPaginationSelected(0);
checkCurrentPage(2);
cy.get('th[data-label="Name"]')
.should('have.attr', 'aria-sort')
.and('contain', 'ascending');
});

it('empty state is displayed when filters do not match any rule', () => {
filterApply({ name: 'Not existing cluster' });
checkNoMatchingClusters();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
import Loading from '../Loading/Loading';
import {
AFFECTED_CLUSTERS_INITIAL_STATE,
resetFilters,
updateAffectedClustersFilters,
} from '../../Services/Filters';
import messages from '../../Messages';
Expand Down Expand Up @@ -322,7 +323,11 @@ const AffectedClustersTable = ({ query, rule, afterDisableFn }) => {
deleteTitle: intl.formatMessage(messages.resetFilters),
onDelete: (event, itemsToRemove, isAll) => {
if (isAll) {
updateFilters(AFFECTED_CLUSTERS_INITIAL_STATE);
resetFilters(
filters,
AFFECTED_CLUSTERS_INITIAL_STATE,
updateFilters
);
} else {
itemsToRemove.map((item) => {
const newFilter = {
Expand Down
11 changes: 11 additions & 0 deletions src/Components/ClusterRules/ClusterRules.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,17 @@ describe('cluster rules table', () => {
.should('have.length', RULES_ENABLED);
});

it('will reset filters but not pagination and sorting', () => {
filterApply({ description: 'Lo' });

cy.get('th[data-label="Description"]').find('button').click();
cy.get(TOOLBAR).find('button').contains('Reset filters').click();
cy.get(TOOLBAR).find(CHIP_GROUP).should('not.exist');
cy.get('th[data-label="Description"]')
.should('have.attr', 'aria-sort')
.and('contain', 'ascending');
});

it('chips can be cleared', () => {
filterApply(filterCombos[0]);
cy.get(CHIP_GROUP).should('exist');
Expand Down
3 changes: 2 additions & 1 deletion src/Components/ClusterRules/ClusterRules.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
} from '../Common/Tables';
import {
CLUSTER_RULES_INITIAL_STATE,
resetFilters,
updateClusterRulesFilters,
} from '../../Services/Filters';
import { getErrorKey, getPluginName } from '../../Utilities/Rule';
Expand Down Expand Up @@ -409,7 +410,7 @@ const ClusterRules = ({ cluster }) => {
filters: buildFilterChips(),
onDelete: (_event, itemsToRemove, isAll) => {
if (isAll) {
updateFilters(CLUSTER_RULES_INITIAL_STATE);
resetFilters(filters, CLUSTER_RULES_INITIAL_STATE, updateFilters);
} else {
itemsToRemove.map((item) => {
const newFilter = {
Expand Down
22 changes: 22 additions & 0 deletions src/Components/ClustersListTable/ClustersListTable.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import { CLUSTERS_LIST_COLUMNS } from '../../AppConstants';
import {
itemsPerPage,
checkPaginationTotal,
checkCurrentPage,
checkPaginationSelected,
checkPaginationValues,
changePagination,
} from '../../../cypress/utils/pagination';
Expand Down Expand Up @@ -433,6 +435,26 @@ describe('clusters list table', () => {
checkRowCounts(DEFAULT_DISPLAYED_SIZE);
});

it('will reset filters but not pagination and sorting', () => {
filterApply({ name: '0' });
changePagination(PAGINATION_VALUES[0]);
cy.get(TOOLBAR)
.find(PAGINATION)
.find('button[data-action="next"]')
.then(($button) => {
cy.wrap($button).click();
});

cy.get('th[data-label="Name"]').find('button').click();
cy.get(TOOLBAR).find('button').contains('Reset filters').click();
cy.get(CHIP_GROUP).should('have.length', 1);
checkPaginationSelected(0);
checkCurrentPage(2);
cy.get('th[data-label="Name"]')
.should('have.attr', 'aria-sort')
.and('contain', 'ascending');
});

it('empty state is displayed when filters do not match any rule', () => {
removeAllChips();
filterApply({
Expand Down
3 changes: 2 additions & 1 deletion src/Components/ClustersListTable/ClustersListTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { conditionalFilterType } from '@redhat-cloud-services/frontend-component

import {
CLUSTERS_LIST_INITIAL_STATE,
resetFilters,
updateClustersListFilters,
} from '../../Services/Filters';
import {
Expand Down Expand Up @@ -281,7 +282,7 @@ const ClustersListTable = ({
if (isEqual(filters, CLUSTERS_LIST_INITIAL_STATE)) {
refetch();
} else {
updateFilters(CLUSTERS_LIST_INITIAL_STATE);
resetFilters(filters, CLUSTERS_LIST_INITIAL_STATE, updateFilters);
}
} else {
itemsToRemove.map((item) => {
Expand Down
18 changes: 18 additions & 0 deletions src/Components/RecsListTable/RecsListTable.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import {
import { cumulativeCombinations } from '../../../cypress/utils/combine';
import {
checkPaginationTotal,
checkCurrentPage,
checkPaginationSelected,
checkPaginationValues,
changePagination,
itemsPerPage,
Expand Down Expand Up @@ -621,6 +623,22 @@ describe('successful non-empty recommendations list table', () => {
});
cy.get(TOOLBAR_FILTER).find('.pf-c-form-control').should('be.empty');
});

it('will reset filters but not pagination and sorting', () => {
filterApply({ name: '2' });
changePagination(PAGINATION_VALUES[0]);

cy.get('th[data-label="Name"]').find('button').click();
cy.get(TOOLBAR).find('button').contains('Reset filters').click();
cy.get(TOOLBAR)
.find(CHIP_GROUP)
.should('have.length', Object.keys(DEFAULT_FILTERS).length);
checkPaginationSelected(0);
checkCurrentPage(1);
cy.get('th[data-label="Name"]')
.should('have.attr', 'aria-sort')
.and('contain', 'ascending');
});
});

describe('enabling/disabling', () => {
Expand Down
12 changes: 10 additions & 2 deletions src/Components/RecsListTable/RecsListTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
import messages from '../../Messages';
import {
RECS_LIST_INITIAL_STATE,
resetFilters,
updateRecsListFilters,
} from '../../Services/Filters';
import RuleLabels from '../Labels/RuleLabels';
Expand Down Expand Up @@ -111,7 +112,14 @@ const RecsListTable = ({ query }) => {
]);

useEffect(() => {
setFilteredRows(buildFilteredRows(recs, filters));
let filteredRows = buildFilteredRows(recs, filters);
if (filteredRows.length && filteredRows.length <= filters.offset) {
updateFilters({
...filters,
offset: 0,
});
}
setFilteredRows(filteredRows);
}, [
data,
filters.category,
Expand Down Expand Up @@ -481,7 +489,7 @@ const RecsListTable = ({ query }) => {
if (isEqual(filters, RECS_LIST_INITIAL_STATE)) {
refetch();
} else {
updateFilters(RECS_LIST_INITIAL_STATE);
resetFilters(filters, RECS_LIST_INITIAL_STATE, updateFilters);
}
} else {
itemsToRemove.map((item) => {
Expand Down
15 changes: 11 additions & 4 deletions src/Services/Filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ export const CLUSTERS_LIST_INITIAL_STATE = {

// single cluster page
export const CLUSTER_RULES_INITIAL_STATE = {
limit: 20,
offset: 0,
// default sorting by total risk
// TODO: use a constant instead
sortIndex: -1,
Expand All @@ -53,6 +51,17 @@ const filtersInitialState = {
clusterRulesState: CLUSTER_RULES_INITIAL_STATE,
};

export const resetFilters = (filters, initialState, updateFilters) => {
const { limit, offset, sortIndex, sortDirection } = filters;
updateFilters({
...initialState,
...(limit !== undefined && { limit }),
...(limit !== undefined && { offset }),
sortIndex,
sortDirection,
});
};

const filters = createSlice({
name: 'filters',
initialState: filtersInitialState,
Expand All @@ -79,8 +88,6 @@ const filters = createSlice({
export const {
updateAffectedClustersFilters,
updateRecsListFilters,
updateRecsListSortIndex,
updateRecListSortDirection,
updateClustersListFilters,
updateClusterRulesFilters,
} = filters.actions;
Expand Down

0 comments on commit 61f9e8b

Please sign in to comment.