fix: make filter reset behavior consistent

This fixes some of the incorrect behavior with filter clear or reset
buttons. They should now only be shown when the state differs from
default and use correct terms for restoring and clearing (depends on
default filters).

Author: dominikvagner

playwright/test-utils/helpers/filters.ts

@@ -332,6 +332,11 @@ export const removeFilter = async (page: Page, filter: FilterConfig) => {
 export const resetFilters = async (page: Page) => {
   // Close any open dropdowns first
   await page.keyboard.press('Escape');
-  await page.getByRole('button', { name: /Reset filters/i }).click();
+  try {
+    await page.getByRole('button', { name: /(Reset|Clear) filters/i }).waitFor({ timeout: 5000 });
+  } catch {
+    return;
+  }
+  await page.getByRole('button', { name: /(Reset|Clear) filters/i }).click();
   await waitForTableLoad(page);
 };

src/Messages.ts

@@ -166,7 +166,7 @@ export default defineMessages({
   labelsFiltersClear: {
     id: 'labelsFiltersClear',
     description: 'label for remove filter chips',
-    defaultMessage: 'Reset filters',
+    defaultMessage: 'Clear filters',
   },
   labelsFiltersCvesSearchPlaceHolder: {
     id: 'labelsFiltersCvesSearch',

src/PresentationalComponents/TableView/TableView.js

@@ -4,11 +4,9 @@ import { PrimaryToolbar } from '@redhat-cloud-services/frontend-components/Prima
 import { SkeletonTable } from '@redhat-cloud-services/frontend-components/SkeletonTable';
 import PropTypes from 'prop-types';
 import React from 'react';
-import messages from '../../Messages';
 import AsyncRemediationButton from '../../SmartComponents/Remediation/AsyncRemediationButton';
-import { arrayFromObj, buildFilterChips, convertLimitOffset } from '../../Utilities/Helpers';
+import { arrayFromObj, buildActiveFilterConfig, convertLimitOffset } from '../../Utilities/Helpers';
 import { useRemoveFilter, useBulkSelectConfig } from '../../Utilities/hooks';
-import { intl } from '../../Utilities/IntlProvider';
 import TableFooter from './TableFooter';
 import ErrorHandler from '../../PresentationalComponents/Snippets/ErrorHandler';
 import { Skeleton, ToolbarItem } from '@patternfly/react-core';
@@ -50,6 +48,10 @@ const TableView = ({
   const selectedCount = selectedRows && arrayFromObj(selectedRows).length;
   const { code, hasError, isLoading } = status;
   const bulkSelectConfig = useBulkSelectConfig(selectedCount, onSelect, metadata, rows, onCollapse);
+  const activeFiltersConfig = React.useMemo(
+    () => buildActiveFilterConfig(filter, search, deleteFilters, searchChipLabel, defaultFilters),
+    [defaultFilters, deleteFilters, filter, search, searchChipLabel],
+  );
 
   const [isColumnMgmtModalOpen, setColumnMgmtModalOpen] = React.useState(false);
   const [appliedColumns, setAppliedColumns] = React.useState(columns);
@@ -102,13 +104,7 @@ const TableView = ({
               )
             }
             filterConfig={filterConfig}
-            activeFiltersConfig={{
-              filters: buildFilterChips(filter, search, searchChipLabel),
-              onDelete: deleteFilters,
-              deleteTitle: intl.formatMessage(
-                (defaultFilters && messages.labelsFiltersReset) || messages.labelsFiltersClear,
-              ),
-            }}
+            activeFiltersConfig={activeFiltersConfig}
             actionsConfig={{
               actions: [
                 remediationProvider && (

src/PresentationalComponents/TableView/TableView.test.js

@@ -2,9 +2,10 @@ import TableView from './TableView';
 import { render, screen, waitFor } from '@testing-library/react';
 import { Provider, useSelector } from 'react-redux';
 import configureStore from 'redux-mock-store';
-import { storeListDefaults } from '../../Utilities/constants';
+import { pageDefaultFilters, storeListDefaults } from '../../Utilities/constants';
 import { systemPackages } from '../../Utilities/RawDataForTesting';
 import userEvent from '@testing-library/user-event';
+import '@testing-library/jest-dom';
 
 const testObj = {
   columns: [],
@@ -134,6 +135,67 @@ describe('TableView', () => {
     expect(asFragment()).toMatchSnapshot();
   });
 
+  it('should keep default filter chips visible while hiding reset at the default state', () => {
+    render(
+      <Provider store={store}>
+        <TableView
+          {...testObj}
+          defaultFilters={pageDefaultFilters.packages}
+          searchChipLabel='Package'
+          store={{
+            rows: [],
+            metadata: { total_items: 10, limit: 20, offset: 0 },
+            status: { isLoading: false, code: 200, hasError: false },
+            queryParams: { filter: { systems_applicable: ['gt:0'] } },
+          }}
+        />
+      </Provider>,
+    );
+
+    expect(screen.getByText('Systems with patches available')).toBeInTheDocument();
+    expect(screen.queryByRole('button', { name: /reset filters/i })).not.toBeInTheDocument();
+  });
+
+  it('should show a reset button when active filters differ from defaults', () => {
+    render(
+      <Provider store={store}>
+        <TableView
+          {...testObj}
+          defaultFilters={pageDefaultFilters.packages}
+          searchChipLabel='Package'
+          store={{
+            rows: [],
+            metadata: { total_items: 10, limit: 20, offset: 0 },
+            status: { isLoading: false, code: 200, hasError: false },
+            queryParams: { filter: { systems_applicable: ['eq:0'] } },
+          }}
+        />
+      </Provider>,
+    );
+
+    expect(screen.getByRole('button', { name: /reset filters/i })).toBeInTheDocument();
+  });
+
+  it('should show a clear button for pages without defaults', () => {
+    render(
+      <Provider store={store}>
+        <TableView
+          {...testObj}
+          defaultFilters={pageDefaultFilters.advisories}
+          searchChipLabel='Advisory'
+          store={{
+            rows: [],
+            metadata: { total_items: 10, limit: 20, offset: 0 },
+            status: { isLoading: false, code: 200, hasError: false },
+            queryParams: { filter: { advisory_type_name: 'bugfix' } },
+          }}
+        />
+      </Provider>,
+    );
+
+    expect(screen.getByRole('button', { name: /clear filters/i })).toBeInTheDocument();
+  });
+
   it('Should unselect', async () => {
     await render(
       <Provider store={store}>

src/SmartComponents/Advisories/Advisories.js

@@ -16,6 +16,7 @@ import {
   selectAdvisoryRow,
 } from '../../store/Actions/Actions';
 import { exportAdvisoriesCSV, exportAdvisoriesJSON } from '../../Utilities/api/api';
+import { pageDefaultFilters } from '../../Utilities/constants';
 import { createAdvisoriesRows } from '../../Utilities/DataMappers';
 import {
   createSortBy,
@@ -200,6 +201,7 @@ const Advisories = () => {
               rebootFilter(apply, queryParams?.filter),
             ],
           }}
+          defaultFilters={pageDefaultFilters.advisories}
           searchChipLabel={intl.formatMessage(messages.labelsFiltersSearchAdvisoriesTitle)}
           isRemediationLoading={isRemediationLoading}
           hasColumnManagement

src/SmartComponents/AdvisoryDetail/CvesModal.tsx

@@ -6,6 +6,7 @@ import TableView from '../../PresentationalComponents/TableView/TableView';
 import searchFilter from '../../PresentationalComponents/Filters/SearchFilter';
 import { cvesTableColumns } from '../../PresentationalComponents/TableView/TableViewAssets';
 import { fetchCves } from '../../store/Actions/VulnerabilityActions';
+import { pageDefaultFilters } from '../../Utilities/constants';
 import { createCvesRows } from '../../Utilities/DataMappers';
 import { sortCves } from '../../Utilities/Helpers';
 import { SortByDirection } from '@patternfly/react-table';
@@ -108,6 +109,7 @@ const CvesModal = ({ cveIds }: CvesModalProps) => {
             status,
             queryParams: { filter: {}, search },
           }}
+          defaultFilters={pageDefaultFilters.cves}
           filterConfig={{
             items: [
               searchFilter(

src/SmartComponents/AdvisorySystems/AdvisorySystemsTable.js

@@ -17,7 +17,7 @@ import {
   exportAdvisorySystemsJSON,
   fetchAdvisorySystems,
 } from '../../Utilities/api/api';
-import { remediationIdentifiers } from '../../Utilities/constants';
+import { pageDefaultFilters, remediationIdentifiers } from '../../Utilities/constants';
 import {
   arrayFromObj,
   persistantParams,
@@ -64,7 +64,11 @@ const AdvisorySystemsTable = ({
     (newColumns) => setAppliedColumns(newColumns),
   );
 
-  const [deleteFilters] = useRemoveFilter({ search, ...filter }, apply);
+  const [deleteFilters] = useRemoveFilter(
+    { search, ...filter },
+    apply,
+    pageDefaultFilters.advisorySystems,
+  );
 
   const filterConfig = {
     items: [
@@ -78,7 +82,12 @@ const AdvisorySystemsTable = ({
     ],
   };
 
-  const activeFiltersConfig = buildActiveFiltersConfig(filter, search, deleteFilters);
+  const activeFiltersConfig = buildActiveFiltersConfig(
+    filter,
+    search,
+    deleteFilters,
+    pageDefaultFilters.advisorySystems,
+  );
 
   const onSelect = useOnSelect(systems, selectedRows, {
     endpoint: ID_API_ENDPOINTS.advisorySystems(advisoryName),

src/SmartComponents/AdvisorySystems/AdvisorySystemsTable.test.js

@@ -31,6 +31,10 @@ const initStore = (state) => {
   return mockStore(state);
 };
 
+beforeEach(() => {
+  InventoryTable.mockClear();
+});
+
 const renderComponent = async (mockedStore) => {
   render(
     <ComponentWithContext renderOptions={{ store: initStore(mockedStore) }}>
@@ -179,6 +183,21 @@ describe('AdvisorySystemsTable.js', () => {
     );
   });
 
+  it('should keep active filters empty when the page is at its default state', async () => {
+    await renderComponent(mockState);
+
+    expect(InventoryTable).toHaveBeenCalledWith(
+      expect.objectContaining({
+        activeFiltersConfig: {
+          deleteTitle: 'Clear filters',
+          filters: [],
+          onDelete: expect.any(Function),
+        },
+      }),
+      {},
+    );
+  });
+
   it('should provide activeFilters config', async () => {
     const filteredState = {
       ...mockState,
@@ -194,7 +213,7 @@ describe('AdvisorySystemsTable.js', () => {
     expect(InventoryTable).toHaveBeenCalledWith(
       expect.objectContaining({
         activeFiltersConfig: {
-          deleteTitle: 'Reset filters',
+          deleteTitle: 'Clear filters',
           filters: [
             {
               category: 'Status',

src/SmartComponents/PackageSystems/PackageSystems.js

@@ -26,10 +26,10 @@ import {
   fetchPackageSystems,
   fetchPackageVersions,
 } from '../../Utilities/api/api';
-import { remediationIdentifiers } from '../../Utilities/constants';
+import { pageDefaultFilters, remediationIdentifiers } from '../../Utilities/constants';
 import {
   arrayFromObj,
-  buildFilterChips,
+  buildActiveFilterConfig,
   decodeQueryparams,
   filterRemediatablePackageSystems,
   persistantParams,
@@ -91,7 +91,11 @@ const PackageSystems = ({ packageName }) => {
     (newColumns) => setAppliedColumns(newColumns),
   );
 
-  const [deleteFilters] = useRemoveFilter({ ...filter, search }, apply);
+  const [deleteFilters] = useRemoveFilter(
+    { ...filter, search },
+    apply,
+    pageDefaultFilters.packageSystems,
+  );
 
   const filterConfig = {
     items: [
@@ -107,15 +111,15 @@ const PackageSystems = ({ packageName }) => {
   };
 
   const activeFiltersConfig = useMemo(
-    () => ({
-      filters: buildFilterChips(
+    () =>
+      buildActiveFilterConfig(
         filter,
         search,
+        deleteFilters,
         intl.formatMessage(messages.labelsFiltersSystemsSearchTitle),
+        pageDefaultFilters.packageSystems,
       ),
-      onDelete: deleteFilters,
-    }),
-    [filter, search],
+    [deleteFilters, filter, search],
   );
 
   const constructFilename = (system) => `${system.available_evra}`;

src/SmartComponents/PackageSystems/PackageSystems.test.js

@@ -3,7 +3,8 @@ import { systemRows } from '../../Utilities/RawDataForTesting';
 import { initMocks } from '../../Utilities/unitTestingUtilities.js';
 import PackageSystems from './PackageSystems';
 import { ComponentWithContext } from '../../Utilities/TestingUtilities.js';
-import { render, screen } from '@testing-library/react';
+import { render, screen, waitFor } from '@testing-library/react';
+import { InventoryTable } from '@redhat-cloud-services/frontend-components/Inventory';
 
 initMocks();
 
@@ -61,26 +62,82 @@ const mockState = {
   },
 };
 
-const initStore = () => {
+const initStore = (state = mockState) => {
   const mockStore = configureStore([]);
-  return mockStore(mockState);
+  return mockStore(state);
 };
 
-const store = initStore(mockState);
-
-beforeEach(() => {
+const renderComponent = async (state = mockState) => {
   render(
-    <ComponentWithContext renderOptions={{ store }}>
+    <ComponentWithContext renderOptions={{ store: initStore(state) }}>
       <PackageSystems packageName='testName' />
     </ComponentWithContext>,
   );
+
+  await waitFor(() => {
+    expect(screen.getByTestId('inventory-mock-component')).toBeVisible();
+  });
+};
+
+beforeEach(() => {
+  InventoryTable.mockClear();
 });
 
 // TODO: find a meaningful way of testing InventoryTable fed module
 describe('PackageSystems.js', () => {
-  it('Should render inventory table', () => {
+  it('Should render inventory table', async () => {
+    await renderComponent();
     expect(screen.getByTestId('inventory-mock-component')).toBeVisible();
   });
+
+  it('should keep active filters empty when there are no non-default filters', async () => {
+    await renderComponent();
+
+    expect(InventoryTable).toHaveBeenCalledWith(
+      expect.objectContaining({
+        activeFiltersConfig: {
+          deleteTitle: 'Clear filters',
+          filters: [],
+          onDelete: expect.any(Function),
+        },
+      }),
+      {},
+    );
+  });
+
+  it('should provide a clear filters action when filters are active', async () => {
+    await renderComponent({
+      ...mockState,
+      PackageSystemsStore: {
+        queryParams: {
+          filter: { status: ['Applicable'] },
+        },
+      },
+    });
+
+    expect(InventoryTable).toHaveBeenCalledWith(
+      expect.objectContaining({
+        activeFiltersConfig: {
+          deleteTitle: 'Clear filters',
+          filters: [
+            {
+              category: 'Status',
+              chips: [
+                {
+                  id: 'status',
+                  name: 'Applicable',
+                  value: 'Applicable',
+                },
+              ],
+              id: 'status',
+            },
+          ],
+          onDelete: expect.any(Function),
+        },
+      }),
+      {},
+    );
+  });
   // it('Should dispatch change package systems params  action once only', () => {
   //     const dispatchedActions = store.getActions();
   //     expect(dispatchedActions.filter(item => item.type === 'CHANGE_PACKAGE_SYSTEMS_PARAMS')).toHaveLength(1);