fix(ESSNTL-5172): Alter permissions for add systems button (RedHatIns…

…ights#1967) Fixes https://issues.redhat.com/browse/ESSNTL-5172.
LiorKGOW · Aug 10, 2023 · a02e399 · a02e399
1 parent 823b2aa
commit a02e399
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 85 deletions.
diff --git 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
@@ -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
@@ -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: (
+                  <ActionDropdownItem
+                    requiredPermissions={REQUIRED_PERMISSIONS_TO_MODIFY_GROUP(
+                      groupId
+                    )}
+                    noAccessTooltip={NO_MODIFY_GROUP_TOOLTIP_MESSAGE}
+                    onClick={() => {
+                      setCurrentSystem([row]);
+                      setRemoveHostsFromGroupModalOpen(true);
+                    }}
+                  >
+                    Remove from group
+                  </ActionDropdownItem>
+                ),
+                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)
-                <Tooltip content={NO_MODIFY_GROUP_TOOLTIP_MESSAGE}>
-                  <Button isAriaDisabled>Add systems</Button>
-                </Tooltip>
-              ) : (
-                {
-                  label: 'Add systems',
-                  onClick: () => {
-                    resetTable();
-                    setAddToGroupModalOpen(true);
-                  },
-                }
-              ),
+              <ActionButton
+                key="add-systems-button"
+                requiredPermissions={REQUIRED_PERMISSIONS_TO_MODIFY_GROUP(
+                  groupId
+                )}
+                noAccessTooltip={NO_MODIFY_GROUP_TOOLTIP_MESSAGE}
+                onClick={() => {
+                  resetTable();
+                  setAddToGroupModalOpen(true);
+                }}
+              >
+                Add systems
+              </ActionButton>,
               {
                 label: 'Remove from group',
                 props: {

diff --git 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
@@ -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 }) => {
       <EmptyStateBody>
         To manage systems more effectively, add systems to the group.
       </EmptyStateBody>
-      {enableAddSystems ? (
-        <Button variant="primary" onClick={() => setIsModalOpen(true)}>
-          Add systems
-        </Button>
-      ) : (
-        <Tooltip content="You do not have the necessary permissions to modify this group. Contact your organization administrator.">
-          <Button isAriaDisabled>Add systems</Button>
-        </Tooltip>
-      )}
+      <ActionButton
+        requiredPermissions={REQUIRED_PERMISSIONS_TO_MODIFY_GROUP(groupId)}
+        noAccessTooltip={NO_MODIFY_GROUP_TOOLTIP_MESSAGE}
+        variant="primary"
+        onClick={() => setIsModalOpen(true)}
+      >
+        Add systems
+      </ActionButton>
       <EmptyStateSecondaryActions>
         <Button
           variant="link"

diff --git a/src/components/InventoryGroups/Modals/AddSystemsToGroupModal.js b/src/components/InventoryGroups/Modals/AddSystemsToGroupModal.js
@@ -101,9 +101,7 @@ const AddSystemsToGroupModal = ({
                   <Alert
                     variant="warning"
                     isInline
-                    title="One or more of the selected systems
-                                    already belong to a group. Adding these systems
-                                    to a different group may impact system configuration."
+                    title="One or more of the selected systems already belong to a group. Only ungrouped systems can be added. Unselect these systems to move forward."
                   />
                 </FlexItem>
               )}