Merge pull request #12123 from marshmalien/12109-fix-user-role-association

Fix user role association in access modal

awx/ui/src/components/AddRole/AddResourceRole.js

@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import { useHistory } from 'react-router-dom';
 import { t } from '@lingui/macro';
 import { TeamsAPI, UsersAPI } from 'api';
+import useSelected from 'hooks/useSelected';
 import SelectableCard from '../SelectableCard';
 import Wizard from '../Wizard';
 import SelectResourceStep from './SelectResourceStep';
@@ -71,51 +72,31 @@ const teamSortColumns = [
 function AddResourceRole({ onSave, onClose, roles, resource, onError }) {
   const history = useHistory();
 
-  const [selectedResource, setSelectedResource] = useState(null);
+  const {
-  const [selectedResourceRows, setSelectedResourceRows] = useState([]);
+    selected: resourcesSelected,
-  const [selectedRoleRows, setSelectedRoleRows] = useState([]);
+    handleSelect: handleResourceSelect,
+    clearSelected: clearResources,
+  } = useSelected([]);
+  const {
+    selected: rolesSelected,
+    handleSelect: handleRoleSelect,
+    clearSelected: clearRoles,
+  } = useSelected([]);
+
+  const [resourceType, setResourceType] = useState(null);
   const [currentStepId, setCurrentStepId] = useState(1);
   const [maxEnabledStep, setMaxEnabledStep] = useState(1);
 
-  const handleResourceCheckboxClick = (user) => {
-    const selectedIndex = selectedResourceRows.findIndex(
-      (selectedRow) => selectedRow.id === user.id
-    );
-    if (selectedIndex > -1) {
-      selectedResourceRows.splice(selectedIndex, 1);
-      if (selectedResourceRows.length === 0) {
-        setMaxEnabledStep(currentStepId);
-      }
-      setSelectedRoleRows(selectedResourceRows);
-    } else {
-      setSelectedResourceRows([...selectedResourceRows, user]);
-    }
-  };
-
   useEffect(() => {
     if (currentStepId === 1 && maxEnabledStep > 1) {
       history.push(history.location.pathname);
     }
   }, [currentStepId, history, maxEnabledStep]);
 
-  const handleRoleCheckboxClick = (role) => {
+  const handleResourceTypeSelect = (type) => {
-    const selectedIndex = selectedRoleRows.findIndex(
+    setResourceType(type);
-      (selectedRow) => selectedRow.id === role.id
+    clearResources();
-    );
+    clearRoles();
-
-    if (selectedIndex > -1) {
-      setSelectedRoleRows(
-        selectedRoleRows.filter((r, index) => index !== selectedIndex)
-      );
-    } else {
-      setSelectedRoleRows([...selectedRoleRows, role]);
-    }
-  };
-
-  const handleResourceSelect = (resourceType) => {
-    setSelectedResource(resourceType);
-    setSelectedResourceRows([]);
-    setSelectedRoleRows([]);
   };
 
   const handleWizardNext = (step) => {
@@ -131,20 +112,20 @@ function AddResourceRole({ onSave, onClose, roles, resource, onError }) {
     try {
       const roleRequests = [];
 
-      for (let i = 0; i < selectedResourceRows.length; i++) {
+      for (let i = 0; i < resourcesSelected.length; i++) {
-        for (let j = 0; j < selectedRoleRows.length; j++) {
+        for (let j = 0; j < rolesSelected.length; j++) {
-          if (selectedResource === 'users') {
+          if (resourceType === 'users') {
             roleRequests.push(
               UsersAPI.associateRole(
-                selectedResourceRows[i].id,
+                resourcesSelected[i].id,
-                selectedRoleRows[j].id
+                rolesSelected[j].id
               )
             );
-          } else if (selectedResource === 'teams') {
+          } else if (resourceType === 'teams') {
             roleRequests.push(
               TeamsAPI.associateRole(
-                selectedResourceRows[i].id,
+                resourcesSelected[i].id,
-                selectedRoleRows[j].id
+                rolesSelected[j].id
               )
             );
           }
@@ -162,7 +143,7 @@ function AddResourceRole({ onSave, onClose, roles, resource, onError }) {
   // Object roles can be user only, so we remove them when
   // showing role choices for team access
   const selectableRoles = { ...roles };
-  if (selectedResource === 'teams') {
+  if (resourceType === 'teams') {
     Object.keys(roles).forEach((key) => {
       if (selectableRoles[key].user_only) {
         delete selectableRoles[key];
@@ -172,7 +153,7 @@ function AddResourceRole({ onSave, onClose, roles, resource, onError }) {
 
   let wizardTitle = '';
 
-  switch (selectedResource) {
+  switch (resourceType) {
     case 'users':
       wizardTitle = t`Add User Roles`;
       break;
@@ -193,60 +174,60 @@ function AddResourceRole({ onSave, onClose, roles, resource, onError }) {
             {t`Choose the type of resource that will be receiving new roles.  For example, if you'd like to add new roles to a set of users please choose Users and click Next.  You'll be able to select the specific resources in the next step.`}
           </div>
           <SelectableCard
-            isSelected={selectedResource === 'users'}
+            isSelected={resourceType === 'users'}
             label={t`Users`}
             ariaLabel={t`Users`}
             dataCy="add-role-users"
-            onClick={() => handleResourceSelect('users')}
+            onClick={() => handleResourceTypeSelect('users')}
           />
           {resource?.type === 'team' ||
           (resource?.type === 'credential' &&
             !resource?.organization) ? null : (
             <SelectableCard
-              isSelected={selectedResource === 'teams'}
+              isSelected={resourceType === 'teams'}
               label={t`Teams`}
               ariaLabel={t`Teams`}
               dataCy="add-role-teams"
-              onClick={() => handleResourceSelect('teams')}
+              onClick={() => handleResourceTypeSelect('teams')}
             />
           )}
         </div>
       ),
       nextButtonText: t`Next`,
-      enableNext: selectedResource !== null,
+      enableNext: resourceType !== null,
     },
     {
       id: 2,
       name: t`Select Items from List`,
       component: (
         <>
-          {selectedResource === 'users' && (
+          {resourceType === 'users' && (
             <SelectResourceStep
               searchColumns={userSearchColumns}
               sortColumns={userSortColumns}
               displayKey="username"
-              onRowClick={handleResourceCheckboxClick}
+              onRowClick={handleResourceSelect}
               fetchItems={readUsers}
               fetchOptions={readUsersOptions}
               selectedLabel={t`Selected`}
-              selectedResourceRows={selectedResourceRows}
+              selectedResourceRows={resourcesSelected}
               sortedColumnKey="username"
             />
           )}
-          {selectedResource === 'teams' && (
+          {resourceType === 'teams' && (
             <SelectResourceStep
               searchColumns={teamSearchColumns}
               sortColumns={teamSortColumns}
-              onRowClick={handleResourceCheckboxClick}
+              onRowClick={handleResourceSelect}
               fetchItems={readTeams}
               fetchOptions={readTeamsOptions}
               selectedLabel={t`Selected`}
-              selectedResourceRows={selectedResourceRows}
+              selectedResourceRows={resourcesSelected}
             />
           )}
         </>
       ),
-      enableNext: selectedResourceRows.length > 0,
+      enableNext: resourcesSelected.length > 0,
       nextButtonText: t`Next`,
       canJumpTo: maxEnabledStep >= 2,
     },
@@ -255,16 +236,16 @@ function AddResourceRole({ onSave, onClose, roles, resource, onError }) {
       name: t`Select Roles to Apply`,
       component: (
         <SelectRoleStep
-          onRolesClick={handleRoleCheckboxClick}
+          onRolesClick={handleRoleSelect}
           roles={selectableRoles}
-          selectedListKey={selectedResource === 'users' ? 'username' : 'name'}
+          selectedListKey={resourceType === 'users' ? 'username' : 'name'}
           selectedListLabel={t`Selected`}
-          selectedResourceRows={selectedResourceRows}
+          selectedResourceRows={resourcesSelected}
-          selectedRoleRows={selectedRoleRows}
+          selectedRoleRows={rolesSelected}
         />
       ),
       nextButtonText: t`Save`,
-      enableNext: selectedRoleRows.length > 0,
+      enableNext: rolesSelected.length > 0,
       canJumpTo: maxEnabledStep >= 3,
     },
   ];

awx/ui/src/components/AddRole/AddResourceRole.test.js

@@ -42,6 +42,7 @@ describe('<_AddResourceRole />', () => {
         results: [
           { id: 1, username: 'foo', url: '' },
           { id: 2, username: 'bar', url: '' },
+          { id: 3, username: 'baz', url: '' },
         ],
       },
     });
@@ -95,14 +96,20 @@ describe('<_AddResourceRole />', () => {
     // Step 2
     await waitForElement(wrapper, 'EmptyStateBody', (el) => el.length === 0);
     expect(wrapper.find('Chip').length).toBe(0);
-    act(() =>
+    wrapper.find('CheckboxListItem[name="foo"]').invoke('onSelect')(true);
-      wrapper.find('CheckboxListItem[name="foo"]').invoke('onSelect')(true)
+    wrapper.find('CheckboxListItem[name="bar"]').invoke('onSelect')(true);
-    );
+    wrapper.find('CheckboxListItem[name="baz"]').invoke('onSelect')(true);
-    wrapper.update();
+    wrapper.find('CheckboxListItem[name="baz"]').invoke('onSelect')(false);
     expect(
       wrapper.find('CheckboxListItem[name="foo"]').prop('isSelected')
     ).toBe(true);
-    expect(wrapper.find('Chip').length).toBe(1);
+    expect(
+      wrapper.find('CheckboxListItem[name="bar"]').prop('isSelected')
+    ).toBe(true);
+    expect(
+      wrapper.find('CheckboxListItem[name="baz"]').prop('isSelected')
+    ).toBe(false);
+    expect(wrapper.find('Chip').length).toBe(2);
     act(() => wrapper.find('Button[type="submit"]').prop('onClick')());
     wrapper.update();
 
@@ -120,6 +127,8 @@ describe('<_AddResourceRole />', () => {
       wrapper.find('Button[type="submit"]').prop('onClick')()
     );
     expect(UsersAPI.associateRole).toBeCalledWith(1, 1);
+    expect(UsersAPI.associateRole).toBeCalledWith(2, 1);
+    expect(UsersAPI.associateRole).toBeCalledTimes(2);
   });
 
   test('should call on error properly', async () => {
@@ -189,7 +198,7 @@ describe('<_AddResourceRole />', () => {
     expect(onError).toBeCalled();
   });
 
-  test('should should update history properly', async () => {
+  test('should update history properly', async () => {
     let wrapper;
     const history = createMemoryHistory({
       initialEntries: ['organizations/2/access?resource.order_by=-username'],