From 61f9e8baf1b5fdf789db3706b48df74e8329ac72 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Mon, 21 Nov 2022 10:24:55 -0500 Subject: [PATCH] fix(Tables): Reset filters resets only filters (#450) 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. --- cypress/utils/components.js | 2 ++ cypress/utils/pagination.js | 20 +++++++++++++++++ cypress/utils/table.js | 3 +-- .../AffectedClustersTable.cy.js | 22 +++++++++++++++++++ .../AffectedClustersTable.js | 7 +++++- .../ClusterRules/ClusterRules.cy.js | 11 ++++++++++ src/Components/ClusterRules/ClusterRules.js | 3 ++- .../ClustersListTable/ClustersListTable.cy.js | 22 +++++++++++++++++++ .../ClustersListTable/ClustersListTable.js | 3 ++- .../RecsListTable/RecsListTable.cy.js | 18 +++++++++++++++ src/Components/RecsListTable/RecsListTable.js | 12 ++++++++-- src/Services/Filters.js | 15 +++++++++---- 12 files changed, 127 insertions(+), 11 deletions(-) diff --git a/cypress/utils/components.js b/cypress/utils/components.js index 5c6bf3a5..7ef434be 100644 --- a/cypress/utils/components.js +++ b/cypress/utils/components.js @@ -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"]'; @@ -27,6 +28,7 @@ export { CHIP, ROW, PAGINATION, + PAGINATION_BOTTOM, PAGINATION_MENU, DROPDOWN, MODAL, diff --git a/cypress/utils/pagination.js b/cypress/utils/pagination.js index b70f6ed8..ea9104df 100644 --- a/cypress/utils/pagination.js +++ b/cypress/utils/pagination.js @@ -1,6 +1,7 @@ import { DEFAULT_ROW_COUNT } from './defaults'; import { TOOLBAR, + PAGINATION, PAGINATION_MENU, DROPDOWN_TOGGLE, DROPDOWN_ITEM, @@ -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) @@ -52,6 +70,8 @@ function changePagination(textInItem) { export { itemsPerPage, checkPaginationTotal, + checkCurrentPage, + checkPaginationSelected, checkPaginationValues, changePagination, }; diff --git a/cypress/utils/table.js b/cypress/utils/table.js index 93a3f714..118a4cd8 100644 --- a/cypress/utils/table.js +++ b/cypress/utils/table.js @@ -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; @@ -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 diff --git a/src/Components/AffectedClustersTable/AffectedClustersTable.cy.js b/src/Components/AffectedClustersTable/AffectedClustersTable.cy.js index c1709950..78cff1a7 100644 --- a/src/Components/AffectedClustersTable/AffectedClustersTable.cy.js +++ b/src/Components/AffectedClustersTable/AffectedClustersTable.cy.js @@ -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'; @@ -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(); diff --git a/src/Components/AffectedClustersTable/AffectedClustersTable.js b/src/Components/AffectedClustersTable/AffectedClustersTable.js index 0302009e..d54b8cd2 100644 --- a/src/Components/AffectedClustersTable/AffectedClustersTable.js +++ b/src/Components/AffectedClustersTable/AffectedClustersTable.js @@ -29,6 +29,7 @@ import { import Loading from '../Loading/Loading'; import { AFFECTED_CLUSTERS_INITIAL_STATE, + resetFilters, updateAffectedClustersFilters, } from '../../Services/Filters'; import messages from '../../Messages'; @@ -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 = { diff --git a/src/Components/ClusterRules/ClusterRules.cy.js b/src/Components/ClusterRules/ClusterRules.cy.js index b97d4c8f..b3c05479 100644 --- a/src/Components/ClusterRules/ClusterRules.cy.js +++ b/src/Components/ClusterRules/ClusterRules.cy.js @@ -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'); diff --git a/src/Components/ClusterRules/ClusterRules.js b/src/Components/ClusterRules/ClusterRules.js index 84c9bb94..728906d5 100644 --- a/src/Components/ClusterRules/ClusterRules.js +++ b/src/Components/ClusterRules/ClusterRules.js @@ -48,6 +48,7 @@ import { } from '../Common/Tables'; import { CLUSTER_RULES_INITIAL_STATE, + resetFilters, updateClusterRulesFilters, } from '../../Services/Filters'; import { getErrorKey, getPluginName } from '../../Utilities/Rule'; @@ -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 = { diff --git a/src/Components/ClustersListTable/ClustersListTable.cy.js b/src/Components/ClustersListTable/ClustersListTable.cy.js index 382fca20..2f9ad9df 100644 --- a/src/Components/ClustersListTable/ClustersListTable.cy.js +++ b/src/Components/ClustersListTable/ClustersListTable.cy.js @@ -43,6 +43,8 @@ import { CLUSTERS_LIST_COLUMNS } from '../../AppConstants'; import { itemsPerPage, checkPaginationTotal, + checkCurrentPage, + checkPaginationSelected, checkPaginationValues, changePagination, } from '../../../cypress/utils/pagination'; @@ -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({ diff --git a/src/Components/ClustersListTable/ClustersListTable.js b/src/Components/ClustersListTable/ClustersListTable.js index a05a49d3..5111c7f7 100644 --- a/src/Components/ClustersListTable/ClustersListTable.js +++ b/src/Components/ClustersListTable/ClustersListTable.js @@ -23,6 +23,7 @@ import { conditionalFilterType } from '@redhat-cloud-services/frontend-component import { CLUSTERS_LIST_INITIAL_STATE, + resetFilters, updateClustersListFilters, } from '../../Services/Filters'; import { @@ -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) => { diff --git a/src/Components/RecsListTable/RecsListTable.cy.js b/src/Components/RecsListTable/RecsListTable.cy.js index f23f17fe..8f1b5c33 100644 --- a/src/Components/RecsListTable/RecsListTable.cy.js +++ b/src/Components/RecsListTable/RecsListTable.cy.js @@ -31,6 +31,8 @@ import { import { cumulativeCombinations } from '../../../cypress/utils/combine'; import { checkPaginationTotal, + checkCurrentPage, + checkPaginationSelected, checkPaginationValues, changePagination, itemsPerPage, @@ -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', () => { diff --git a/src/Components/RecsListTable/RecsListTable.js b/src/Components/RecsListTable/RecsListTable.js index 6611d6b6..37b81134 100644 --- a/src/Components/RecsListTable/RecsListTable.js +++ b/src/Components/RecsListTable/RecsListTable.js @@ -37,6 +37,7 @@ import { import messages from '../../Messages'; import { RECS_LIST_INITIAL_STATE, + resetFilters, updateRecsListFilters, } from '../../Services/Filters'; import RuleLabels from '../Labels/RuleLabels'; @@ -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, @@ -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) => { diff --git a/src/Services/Filters.js b/src/Services/Filters.js index c8146768..2a2903ab 100644 --- a/src/Services/Filters.js +++ b/src/Services/Filters.js @@ -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, @@ -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, @@ -79,8 +88,6 @@ const filters = createSlice({ export const { updateAffectedClustersFilters, updateRecsListFilters, - updateRecsListSortIndex, - updateRecListSortDirection, updateClustersListFilters, updateClusterRulesFilters, } = filters.actions;