From a02e3990fdb423b38ed613a8aff031bd6cf3297d Mon Sep 17 00:00:00 2001 From: Georgy Karataev Date: Thu, 10 Aug 2023 12:34:29 +0200 Subject: [PATCH] fix(ESSNTL-5172): Alter permissions for add systems button (#1967) Fixes https://issues.redhat.com/browse/ESSNTL-5172. --- cypress/support/commands.js | 12 ++++ .../GroupSystems/GroupSystems.cy.js | 40 +++-------- src/components/GroupSystems/GroupSystems.js | 68 +++++++++++-------- .../InventoryGroupDetail.cy.js | 5 ++ .../NoSystemsEmptyState.js | 35 ++++------ .../Modals/AddSystemsToGroupModal.js | 4 +- 6 files changed, 79 insertions(+), 85 deletions(-) diff --git a/cypress/support/commands.js b/cypress/support/commands.js index c9b522fb1..aee64e8f1 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -85,3 +85,15 @@ Cypress.Commands.add( ); } ); + +Cypress.Commands.add( + 'shouldHaveAriaDisabled', + { prevSubject: true }, + (subject) => cy.wrap(subject).should('have.attr', 'aria-disabled', 'true') +); + +Cypress.Commands.add( + 'shouldHaveAriaEnabled', + { prevSubject: true }, + (subject) => cy.wrap(subject).should('have.attr', 'aria-disabled', 'false') +); diff --git a/src/components/GroupSystems/GroupSystems.cy.js b/src/components/GroupSystems/GroupSystems.cy.js index 874dc5443..76e47a2b3 100644 --- a/src/components/GroupSystems/GroupSystems.cy.js +++ b/src/components/GroupSystems/GroupSystems.cy.js @@ -59,7 +59,11 @@ const checkSelectedNumber = (number) => checkSelectedNumber_(number, '#bulk-select-systems-toggle-checkbox-text'); const mountTable = () => - cy.mountWithContext(GroupSystems, {}, { groupName: GROUP_NAME }); + cy.mountWithContext( + GroupSystems, + {}, + { groupName: GROUP_NAME, groupId: TEST_ID } + ); const waitForTable = (waitNetwork = false) => { if (waitNetwork) { @@ -463,20 +467,14 @@ describe('integration with rbac', () => { }); it('no way to add or remove systems', () => { - cy.get('button') - .contains('Add systems') - .should('have.class', 'pf-m-aria-disabled'); + cy.get('button').contains('Add systems').shouldHaveAriaDisabled(); cy.get('.ins-c-primary-toolbar__actions [aria-label="Actions"]').click(); - cy.get('button') - .contains('Remove from group') - .should('have.class', 'pf-m-aria-disabled'); + cy.get('button').contains('Remove from group').shouldHaveAriaDisabled(); }); it('per-row dropdown should be disabled', () => { cy.get(ROW).eq(1).find(DROPDOWN).click(); - cy.get('button') - .contains('Remove from group') - .should('have.class', 'pf-m-aria-disabled'); + cy.get('button').contains('Remove from group').shouldHaveAriaDisabled(); }); }); @@ -500,26 +498,8 @@ describe('integration with rbac', () => { cy.get(DROPDOWN_ITEM).contains('Remove from group').should('be.enabled'); }); - it('no way to add systems', () => { - cy.get('button') - .contains('Add systems') - .should('have.class', 'pf-m-aria-disabled'); - }); - }); - - describe('has additional hosts read permissions', () => { - before(() => { - cy.mockWindowChrome({ - userPermissions: [ - ...READ_PERMISSIONS_WITH_RD, - ...WRITE_PERMISSIONS_WITH_RD, - 'inventory:hosts:read', - ], - }); - }); - - it('can add systems', () => { - cy.get('button').contains('Add systems').should('be.enabled'); + it('add systems button is enabled', () => { + cy.get('button').contains('Add systems').shouldHaveAriaEnabled(); }); }); }); diff --git a/src/components/GroupSystems/GroupSystems.js b/src/components/GroupSystems/GroupSystems.js index 53480877e..17f84e767 100644 --- a/src/components/GroupSystems/GroupSystems.js +++ b/src/components/GroupSystems/GroupSystems.js @@ -14,7 +14,10 @@ import { NO_MODIFY_GROUP_TOOLTIP_MESSAGE, REQUIRED_PERMISSIONS_TO_MODIFY_GROUP, } from '../../constants'; -import { Button, Tooltip } from '@patternfly/react-core'; +import { + ActionButton, + ActionDropdownItem, +} from '../InventoryTable/ActionWithRBAC'; export const bulkSelectConfig = ( dispatch, @@ -116,8 +119,6 @@ const GroupSystems = ({ groupName, groupId }) => { dispatch(selectEntity(-1, false)); }; - const enableAddSystems = canModify; - useEffect(() => { return () => { resetTable(); @@ -171,40 +172,47 @@ const GroupSystems = ({ groupName, groupId }) => { showTags ) } - actions={[ - { - title: 'Remove from group', - onClick: (event, index, rowData) => { - setCurrentSystem([rowData]); - setRemoveHostsFromGroupModalOpen(true); - }, - ...(!canModify && { - isAriaDisabled: true, - tooltip: NO_MODIFY_GROUP_TOOLTIP_MESSAGE, - }), - }, - ]} tableProps={{ isStickyHeader: true, variant: TableVariant.compact, canSelectAll: false, + actionResolver: (row) => [ + { + title: ( + { + setCurrentSystem([row]); + setRemoveHostsFromGroupModalOpen(true); + }} + > + Remove from group + + ), + style: { + padding: 0, // custom component creates extra padding space + }, + }, + ], }} actionsConfig={{ actions: [ - !enableAddSystems ? ( - // custom component needed since it's the first action to render (see primary toolbar implementation) - - - - ) : ( - { - label: 'Add systems', - onClick: () => { - resetTable(); - setAddToGroupModalOpen(true); - }, - } - ), + { + resetTable(); + setAddToGroupModalOpen(true); + }} + > + Add systems + , { label: 'Remove from group', props: { diff --git a/src/components/InventoryGroupDetail/InventoryGroupDetail.cy.js b/src/components/InventoryGroupDetail/InventoryGroupDetail.cy.js index 368a337b3..735397fd5 100644 --- a/src/components/InventoryGroupDetail/InventoryGroupDetail.cy.js +++ b/src/components/InventoryGroupDetail/InventoryGroupDetail.cy.js @@ -24,6 +24,9 @@ before(() => { cy.mockWindowChrome(); // with all permissions }); +const waitPageLoad = () => + cy.get('h1').should('not.have.text', 'Loading group details'); + describe('test data', () => { it('the group has no hosts', () => { groupDetailFixtures.results[0].host_count === 0; @@ -201,10 +204,12 @@ describe('integration with rbac', () => { beforeEach(() => { groupDetailInterceptors.successful(); mountPage(); + waitPageLoad(); }); it('actions are disabled', () => { cy.get(DROPDOWN).contains('Group actions').should('be.disabled'); + cy.get('button').contains('Add systems').shouldHaveAriaDisabled(); }); it('should allow to see systems', () => { diff --git a/src/components/InventoryGroupDetail/NoSystemsEmptyState.js b/src/components/InventoryGroupDetail/NoSystemsEmptyState.js index 36711ed93..5f28ef390 100644 --- a/src/components/InventoryGroupDetail/NoSystemsEmptyState.js +++ b/src/components/InventoryGroupDetail/NoSystemsEmptyState.js @@ -6,27 +6,19 @@ import { EmptyStateIcon, EmptyStateSecondaryActions, Title, - Tooltip, } from '@patternfly/react-core'; import { ExternalLinkAltIcon, PlusCircleIcon } from '@patternfly/react-icons'; import { global_palette_black_600 as globalPaletteBlack600 } from '@patternfly/react-tokens/dist/js/global_palette_black_600'; import AddSystemsToGroupModal from '../InventoryGroups/Modals/AddSystemsToGroupModal'; import PropTypes from 'prop-types'; -import { usePermissionsWithContext } from '@redhat-cloud-services/frontend-components-utilities/RBACHook'; -import { REQUIRED_PERMISSIONS_TO_MODIFY_GROUP } from '../../constants'; +import { + NO_MODIFY_GROUP_TOOLTIP_MESSAGE, + REQUIRED_PERMISSIONS_TO_MODIFY_GROUP, +} from '../../constants'; +import { ActionButton } from '../InventoryTable/ActionWithRBAC'; const NoSystemsEmptyState = ({ groupId, groupName }) => { - const { hasAccess: canViewHosts } = usePermissionsWithContext([ - 'inventory:hosts:read', - ]); - - const { hasAccess: canModify } = usePermissionsWithContext( - REQUIRED_PERMISSIONS_TO_MODIFY_GROUP(groupId) - ); - - const enableAddSystems = canModify && canViewHosts; - const [isModalOpen, setIsModalOpen] = useState(false); return ( @@ -51,15 +43,14 @@ const NoSystemsEmptyState = ({ groupId, groupName }) => { To manage systems more effectively, add systems to the group. - {enableAddSystems ? ( - - ) : ( - - - - )} + setIsModalOpen(true)} + > + Add systems +