From 206bfd277ecc24812fb52a81b6285a63f9721203 Mon Sep 17 00:00:00 2001 From: Georgii Karataev Date: Thu, 7 Dec 2023 10:17:18 +0100 Subject: [PATCH 1/4] refactor: Create useGlobalFilter This extracts global filter enablement logic to a separate hook. --- config/overrideChrome.js | 3 +- config/setupTests.js | 1 + .../ConventionalSystemsTab.js | 31 +------------ src/components/filters/useGlobalFilter.js | 44 +++++++++++++++++++ 4 files changed, 49 insertions(+), 30 deletions(-) create mode 100644 src/components/filters/useGlobalFilter.js diff --git a/config/overrideChrome.js b/config/overrideChrome.js index 3040a159a..6e034ebe3 100644 --- a/config/overrideChrome.js +++ b/config/overrideChrome.js @@ -3,7 +3,7 @@ const chromeMock = { isBeta: () => false, appAction: () => {}, appObjectId: () => {}, - on: () => {}, + on: () => () => {}, getApp: () => 'inventory', getBundle: () => 'insights', getUserPermissions: () => [{ permission: 'inventory:*:*' }], @@ -25,6 +25,7 @@ const chromeMock = { }, }), }, + hideGlobalFilter: () => {}, }; export default () => chromeMock; diff --git a/config/setupTests.js b/config/setupTests.js index c207562e4..c304b0552 100644 --- a/config/setupTests.js +++ b/config/setupTests.js @@ -43,6 +43,7 @@ jest.mock('@redhat-cloud-services/frontend-components/useChrome', () => ({ getUserPermissions: () => Promise.resolve(['inventory:*:*']), getApp: jest.fn(), getBundle: jest.fn(), + hideGlobalFilter: jest.fn(), }), })); diff --git a/src/components/InventoryTabs/ConventionalSystems/ConventionalSystemsTab.js b/src/components/InventoryTabs/ConventionalSystems/ConventionalSystemsTab.js index b28bfcfb1..c27ab650c 100644 --- a/src/components/InventoryTabs/ConventionalSystems/ConventionalSystemsTab.js +++ b/src/components/InventoryTabs/ConventionalSystems/ConventionalSystemsTab.js @@ -9,7 +9,6 @@ import { generateFilter } from '../../../Utilities/constants'; import { InventoryTable as InventoryTableCmp } from '../../InventoryTable'; import useChrome from '@redhat-cloud-services/frontend-components/useChrome'; import AddSelectedHostsToGroupModal from '../../InventoryGroups/Modals/AddSelectedHostsToGroupModal'; -import useFeatureFlag from '../../../Utilities/useFeatureFlag'; import { useBulkSelectConfig } from '../../../Utilities/hooks/useBulkSelectConfig'; import RemoveHostsFromGroupModal from '../../InventoryGroups/Modals/RemoveHostsFromGroupModal'; import { @@ -27,6 +26,7 @@ import uniq from 'lodash/uniq'; import useInsightsNavigate from '@redhat-cloud-services/frontend-components-utilities/useInsightsNavigate/useInsightsNavigate'; import useTableActions from './useTableActions'; import { calculateFilters, calculatePagination } from './Utilities'; +import useGlobalFilter from '../../filters/useGlobalFilter'; const BulkDeleteButton = ({ selectedSystems, ...props }) => { const requiredPermissions = selectedSystems.map(({ groups }) => @@ -87,7 +87,7 @@ const ConventionalSystemsTab = ({ const [addHostGroupModalOpen, setAddHostGroupModalOpen] = useState(false); const [removeHostsFromGroupModalOpen, setRemoveHostsFromGroupModalOpen] = useState(false); - const [globalFilter, setGlobalFilter] = useState(); + const globalFilter = useGlobalFilter(); const rows = useSelector(({ entities }) => entities?.rows, shallowEqual); const loaded = useSelector(({ entities }) => entities?.loaded); const selected = useSelector(({ entities }) => entities?.selected); @@ -118,37 +118,10 @@ const ConventionalSystemsTab = ({ } }; - const EdgeParityFilterDeviceEnabled = useFeatureFlag( - 'edgeParity.inventory-list-filter' - ); - useEffect(() => { chrome.updateDocumentTitle('Systems | Red Hat Insights'); - chrome?.hideGlobalFilter?.(false); chrome.appAction('system-list'); chrome.appObjectId(); - chrome.on('GLOBAL_FILTER_UPDATE', ({ data }) => { - const [workloads, SID, tags] = chrome.mapGlobalFilter(data, false, true); - setGlobalFilter({ - tags, - filter: { - ...globalFilter?.filter, - system_profile: { - ...globalFilter?.filter?.system_profile, - ...(workloads?.SAP?.isSelected && { sap_system: true }), - ...(workloads && - workloads['Ansible Automation Platform']?.isSelected && { - ansible: 'not_nil', - }), - ...(workloads?.['Microsoft SQL']?.isSelected && { - mssql: 'not_nil', - }), - ...(EdgeParityFilterDeviceEnabled && { host_type: 'nil' }), - ...(SID?.length > 0 && { sap_sids: SID }), - }, - }, - }); - }); dispatch(actions.clearNotifications()); if (perPage || page) { diff --git a/src/components/filters/useGlobalFilter.js b/src/components/filters/useGlobalFilter.js new file mode 100644 index 000000000..ce7b505f4 --- /dev/null +++ b/src/components/filters/useGlobalFilter.js @@ -0,0 +1,44 @@ +import { useEffect, useState } from 'react'; +import useFeatureFlag from '../../Utilities/useFeatureFlag'; +import useChrome from '@redhat-cloud-services/frontend-components/useChrome'; + +const useGlobalFilter = () => { + const chrome = useChrome(); + const edgeParityFilterDeviceEnabled = useFeatureFlag( + 'edgeParity.inventory-list-filter' + ); + const [globalFilter, setGlobalFilter] = useState(); + + useEffect(() => { + chrome.hideGlobalFilter(false); + const unlisten = chrome.on('GLOBAL_FILTER_UPDATE', ({ data }) => { + const [workloads, SID, tags] = chrome.mapGlobalFilter(data, false, true); + + setGlobalFilter({ + tags, + filter: { + ...globalFilter?.filter, + system_profile: { + ...globalFilter?.filter?.system_profile, + ...(workloads?.SAP?.isSelected && { sap_system: true }), + ...(workloads && + workloads['Ansible Automation Platform']?.isSelected && { + ansible: 'not_nil', + }), + ...(workloads?.['Microsoft SQL']?.isSelected && { + mssql: 'not_nil', + }), + ...(edgeParityFilterDeviceEnabled && { host_type: 'nil' }), + ...(SID?.length > 0 && { sap_sids: SID }), + }, + }, + }); + }); + + return () => unlisten(); + }, [edgeParityFilterDeviceEnabled]); + + return globalFilter; +}; + +export default useGlobalFilter; From f6ed963fc8d40b259eb6b90cddbc75502ecae084 Mon Sep 17 00:00:00 2001 From: Georgii Karataev Date: Wed, 6 Dec 2023 23:46:48 +0100 Subject: [PATCH 2/4] feat(RHINENG-4614): Global filter on group details This enables global filter on the group details page. --- src/components/GroupSystems/GroupSystems.js | 7 ++++++- src/components/InventoryGroupDetail/index.js | 8 +------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/components/GroupSystems/GroupSystems.js b/src/components/GroupSystems/GroupSystems.js index 6e6280d29..af4614465 100644 --- a/src/components/GroupSystems/GroupSystems.js +++ b/src/components/GroupSystems/GroupSystems.js @@ -20,6 +20,8 @@ import { clearEntitiesAction } from '../../store/actions'; import { useBulkSelectConfig } from '../../Utilities/hooks/useBulkSelectConfig'; import difference from 'lodash/difference'; import map from 'lodash/map'; +import useGlobalFilter from '../filters/useGlobalFilter'; + export const prepareColumns = ( initialColumns, hideGroupColumn, @@ -74,6 +76,7 @@ export const prepareColumns = ( const GroupSystems = ({ groupName, groupId }) => { const dispatch = useDispatch(); + const globalFilter = useGlobalFilter(); const [removeHostsFromGroupModalOpen, setRemoveHostsFromGroupModalOpen] = useState(false); const [currentSystem, setCurrentSystem] = useState([]); @@ -104,7 +107,7 @@ const GroupSystems = ({ groupName, groupId }) => { const bulkSelectConfig = useBulkSelectConfig( selected, - null, + globalFilter, total, rows, true, @@ -220,6 +223,8 @@ const GroupSystems = ({ groupName, groupId }) => { showTags ref={inventory} showCentosVersions + customFilters={{ globalFilter }} + autoRefresh /> )} diff --git a/src/components/InventoryGroupDetail/index.js b/src/components/InventoryGroupDetail/index.js index c9f32209b..fb3643d2c 100644 --- a/src/components/InventoryGroupDetail/index.js +++ b/src/components/InventoryGroupDetail/index.js @@ -1,15 +1,9 @@ -import useChrome from '@redhat-cloud-services/frontend-components/useChrome'; -import React, { useEffect } from 'react'; +import React from 'react'; import { useParams } from 'react-router-dom'; import InventoryGroupDetail from './InventoryGroupDetail'; const InventoryGroupDetailWrapper = () => { const { groupId } = useParams(); - const chrome = useChrome(); - - useEffect(() => { - chrome?.hideGlobalFilter?.(); - }, []); return ; }; From 942598f053f7e1333dbd1a66493603970cc6aa03 Mon Sep 17 00:00:00 2001 From: Georgii Karataev Date: Wed, 6 Dec 2023 12:40:23 +0100 Subject: [PATCH 3/4] fix(Create group): Ignore empty input This makes the create group modal not to validate group name presence if the input is empty. It previously led to the modal crash sometimes if the input field was empty. --- .../Modals/CreateGroupModal.js | 6 ++++- .../Modals/CreateGroupModal.test.js | 23 +++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/components/InventoryGroups/Modals/CreateGroupModal.js b/src/components/InventoryGroups/Modals/CreateGroupModal.js index 87458538c..9df6c760f 100644 --- a/src/components/InventoryGroups/Modals/CreateGroupModal.js +++ b/src/components/InventoryGroups/Modals/CreateGroupModal.js @@ -7,7 +7,11 @@ import { createGroup, validateGroupName } from '../utils/api'; import { useDispatch } from 'react-redux'; import awesomeDebouncePromise from 'awesome-debounce-promise'; -export const validate = async (value) => { +export const validate = async (value = '') => { + if (value.length === 0) { + return undefined; // the input is empty + } + const results = await validateGroupName(value.trim()); if (results === true) { throw 'Group name already exists'; diff --git a/src/components/InventoryGroups/Modals/CreateGroupModal.test.js b/src/components/InventoryGroups/Modals/CreateGroupModal.test.js index 9ae4a7ad0..f5ab8a224 100644 --- a/src/components/InventoryGroups/Modals/CreateGroupModal.test.js +++ b/src/components/InventoryGroups/Modals/CreateGroupModal.test.js @@ -4,22 +4,41 @@ import { validate } from './CreateGroupModal'; jest.mock('../utils/api'); describe('validate function', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + it('works with basic input', async () => { const result = await validate('test'); - expect(result).toBe(undefined); + expect(result).toBeUndefined(); expect(validateGroupName).toHaveBeenCalledWith('test'); }); it('trims input', async () => { const result = await validate(' test '); - expect(result).toBe(undefined); + expect(result).toBeUndefined(); expect(validateGroupName).toHaveBeenCalledWith('test'); }); it('throws error if the name is present', async () => { validateGroupName.mockResolvedValue(true); + await expect(validate('test')).rejects.toBe('Group name already exists'); }); + + it('does not check on undefined input', async () => { + const result = await validate(undefined); + + expect(result).toBeUndefined(); + expect(validateGroupName).not.toHaveBeenCalled(); + }); + + it('does not check on empty input', async () => { + const result = await validate(''); + + expect(result).toBeUndefined(); + expect(validateGroupName).not.toHaveBeenCalled(); + }); }); From 64c5f6c28676d779a3e2a1ce097f49c9f4a65de4 Mon Sep 17 00:00:00 2001 From: Georgii Karataev Date: Wed, 6 Dec 2023 13:22:14 +0100 Subject: [PATCH 4/4] fix(RHINENG-1414): Sync create group schema with hints Fixes https://issues.redhat.com/browse/RHINENG-1414. --- .../Modals/CreateGroupModal.test.js | 102 +++++++++++++++++- .../Modals/ModalSchemas/schemes.js | 2 +- .../InventoryGroups/helpers/validate.js | 4 +- 3 files changed, 104 insertions(+), 4 deletions(-) diff --git a/src/components/InventoryGroups/Modals/CreateGroupModal.test.js b/src/components/InventoryGroups/Modals/CreateGroupModal.test.js index f5ab8a224..1fcb0d123 100644 --- a/src/components/InventoryGroups/Modals/CreateGroupModal.test.js +++ b/src/components/InventoryGroups/Modals/CreateGroupModal.test.js @@ -1,7 +1,12 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; import { validateGroupName } from '../utils/api'; -import { validate } from './CreateGroupModal'; +import CreateGroupModal, { validate } from './CreateGroupModal'; +import userEvent from '@testing-library/user-event'; +import '@testing-library/jest-dom'; jest.mock('../utils/api'); +jest.mock('react-redux'); describe('validate function', () => { afterEach(() => { @@ -42,3 +47,98 @@ describe('validate function', () => { expect(validateGroupName).not.toHaveBeenCalled(); }); }); + +describe('create group modal', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + const setIsModalOpen = jest.fn(); + const reloadData = jest.fn(); + + it('create button is initially disabled', () => { + render( + + ); + + expect( + screen.getByRole('button', { + name: /create/i, + }) + ).toBeDisabled(); + }); + + it('can create a group with new name', async () => { + validateGroupName.mockResolvedValue(false); + + render( + + ); + + await userEvent.type( + screen.getByRole('textbox', { + name: /group name/i, + }), + '_abc' + ); + + await waitFor(() => { + expect( + screen.getByRole('button', { + name: /create/i, + }) + ).toBeEnabled(); + }); + }); + + it('cannot create a group with incorrect name', async () => { + render( + + ); + + expect( + screen.getByRole('button', { + name: /create/i, + }) + ).toBeDisabled(); + + await userEvent.type( + screen.getByRole('textbox', { + name: /group name/i, + }), + '###' + ); + + expect( + screen.getByRole('button', { + name: /create/i, + }) + ).toBeDisabled(); + + await userEvent.click( + screen.getByRole('button', { + name: /create/i, + }) + ); // must change focus for the hint to appear (DDF implementation) + + await waitFor(() => { + expect( + screen.getByText( + 'Valid characters include letters, numbers, spaces, hyphens ( - ), and underscores ( _ ).' + ) + ).toBeVisible(); + }); + }); +}); diff --git a/src/components/InventoryGroups/Modals/ModalSchemas/schemes.js b/src/components/InventoryGroups/Modals/ModalSchemas/schemes.js index 2068c9d5a..f4509ee40 100644 --- a/src/components/InventoryGroups/Modals/ModalSchemas/schemes.js +++ b/src/components/InventoryGroups/Modals/ModalSchemas/schemes.js @@ -13,7 +13,7 @@ export const createGroupSchema = (namePresenceValidator) => ({ name: 'name', label: 'Group name', helperText: - 'Can only contain letters, numbers, spaces, hyphens ( - ), and underscores( _ ).', + 'Can only contain letters, numbers, spaces, hyphens ( - ), and underscores ( _ ).', isRequired: true, autoFocus: true, validate: [ diff --git a/src/components/InventoryGroups/helpers/validate.js b/src/components/InventoryGroups/helpers/validate.js index 98f41211d..bef6ab301 100644 --- a/src/components/InventoryGroups/helpers/validate.js +++ b/src/components/InventoryGroups/helpers/validate.js @@ -2,7 +2,7 @@ import validatorTypes from '@data-driven-forms/react-form-renderer/validator-typ export const nameValidator = { type: validatorTypes.PATTERN, - pattern: /^[A-Za-z0-9]+[A-Za-z0-9_\-\s]*$/, + pattern: /^[A-Za-z0-9_\-\s]+[A-Za-z0-9_\-\s]*$/, message: - 'Must start with a letter or number. Valid characters include lowercase letters, numbers, hyphens ( - ), and underscores ( _ ).', + 'Valid characters include letters, numbers, spaces, hyphens ( - ), and underscores ( _ ).', };