Merge pull request #11247 from AlexSCorey/11227-fix

Removes disassociate button on details view and fine tunes disassociate button on list view

awx/ui/src/components/DisassociateButton/DisassociateButton.js

@@ -33,18 +33,30 @@ function DisassociateButton({
     }
   }, [isKebabified, isOpen, onKebabModalChange]);
 
-  function cannotDisassociate(item) {
+  function cannotDisassociateAllOthers(item) {
     return !item.summary_fields?.user_capabilities?.delete;
   }
+  function cannotDisassociateInstances(item) {
+    return item.node_type === 'control';
+  }
+
+  const cannotDisassociate = itemsToDisassociate.some(
+    (i) => i.type === 'instance'
+  )
+    ? cannotDisassociateInstances
+    : cannotDisassociateAllOthers;
 
   function renderTooltip() {
     if (verifyCannotDisassociate) {
       const itemsUnableToDisassociate = itemsToDisassociate
         .filter(cannotDisassociate)
-        .map((item) => item.name)
+        .map((item) => item.name ?? item.hostname)
         .join(', ');
-
-      if (itemsToDisassociate.some(cannotDisassociate)) {
+      if (
+        cannotDisassociate
+          ? itemsToDisassociate.some(cannotDisassociateInstances)
+          : itemsToDisassociate.some(cannotDisassociateAllOthers)
+      ) {
         return (
           <div>
             {t`You do not have permission to disassociate the following: ${itemsUnableToDisassociate}`}
@@ -79,12 +91,17 @@ function DisassociateButton({
           aria-label={t`disassociate`}
           isDisabled={isDisabled}
           component="button"
+          ouiaId="disassociate-tooltip"
           onClick={() => setIsOpen(true)}
         >
           {t`Disassociate`}
         </DropdownItem>
       ) : (
-        <Tooltip content={renderTooltip()} position="top">
+        <Tooltip
+          content={renderTooltip()}
+          ouiaId="disassociate-tooltip"
+          position="top"
+        >
           <div>
             <Button
               ouiaId="disassociate-button"

awx/ui/src/components/DisassociateButton/DisassociateButton.test.js

@@ -108,5 +108,21 @@ describe('<DisassociateButton />', () => {
       );
       expect(wrapper.find('button[disabled]')).toHaveLength(1);
     });
+
+    test('should disable button for control instance', () => {
+      const wrapper = mountWithContexts(
+        <DisassociateButton
+          onDisassociate={() => {}}
+          itemsToDelete={[
+            {
+              id: 1,
+              hostname: 'awx',
+              node_type: 'control',
+            },
+          ]}
+        />
+      );
+      expect(wrapper.find('button[disabled]')).toHaveLength(1);
+    });
   });
 });

awx/ui/src/screens/InstanceGroup/InstanceDetails/InstanceDetails.js

@@ -273,13 +273,15 @@ function InstanceDetails({ setBreadcrumb, instanceGroup }) {
               {t`Health Check`}
             </Button>
           </Tooltip>
-          <DisassociateButton
-            verifyCannotDisassociate={!me.is_superuser}
-            key="disassociate"
-            onDisassociate={disassociateInstance}
-            itemsToDisassociate={[instance]}
-            modalTitle={t`Disassociate instance from instance group?`}
-          />
+          {me.is_superuser && instance.node_type !== 'control' && (
+            <DisassociateButton
+              verifyCannotDisassociate={!me.is_superuser}
+              key="disassociate"
+              onDisassociate={disassociateInstance}
+              itemsToDisassociate={[instance]}
+              modalTitle={t`Disassociate instance from instance group?`}
+            />
+          )}
           <InstanceToggle
             css="display: inline-flex;"
             fetchInstances={fetchDetails}

awx/ui/src/screens/InstanceGroup/InstanceDetails/InstanceDetails.test.js

@@ -230,9 +230,9 @@ describe('<InstanceDetails/>', () => {
       );
     });
     await waitForElement(wrapper, 'ContentLoading', (el) => el.length === 0);
-    expect(
-      wrapper.find("Button[ouiaId='disassociate-button']").prop('isDisabled')
-    ).toBe(true);
+    expect(wrapper.find("Button[ouiaId='disassociate-button']")).toHaveLength(
+      0
+    );
     expect(
       wrapper.find("Button[ouiaId='health-check-button']").prop('isDisabled')
     ).toBe(true);
@@ -412,7 +412,7 @@ describe('<InstanceDetails/>', () => {
       },
     });
     jest.spyOn(ConfigContext, 'useConfig').mockImplementation(() => ({
-      me: { is_system_auditor: true },
+      me: { is_superuser: true },
     }));
     await act(async () => {
       wrapper = mountWithContexts(
@@ -463,7 +463,7 @@ describe('<InstanceDetails/>', () => {
       },
     });
     jest.spyOn(ConfigContext, 'useConfig').mockImplementation(() => ({
-      me: { is_system_auditor: true },
+      me: { is_superuser: true },
     }));
     await act(async () => {
       wrapper = mountWithContexts(
@@ -474,6 +474,7 @@ describe('<InstanceDetails/>', () => {
       );
     });
     await waitForElement(wrapper, 'ContentLoading', (el) => el.length === 0);
+
     await act(async () =>
       wrapper.find('Button[ouiaId="disassociate-button"]').prop('onClick')()
     );

awx/ui/src/screens/InstanceGroup/Instances/InstanceList.js

@@ -207,12 +207,12 @@ function InstanceList() {
                   ]
                 : []),
               <DisassociateButton
-                verifyCannotDisassociate={false}
+                verifyCannotDisassociate={selected.some(
+                  (s) => s.node_type === 'control'
+                )}
                 key="disassociate"
                 onDisassociate={handleDisassociate}
-                itemsToDisassociate={selected.filter(
-                  (s) => s.node_type !== 'control'
-                )}
+                itemsToDisassociate={selected}
                 modalTitle={t`Disassociate instance from instance group?`}
               />,
               <Tooltip

awx/ui/src/screens/InstanceGroup/Instances/InstanceList.test.js

@@ -41,6 +41,7 @@ const instances = [
     jobs_running: 0,
     jobs_total: 68,
     cpu: 6,
+    node_type: 'control',
     memory: 2087469056,
     cpu_capacity: 24,
     mem_capacity: 1,
@@ -67,6 +68,7 @@ const instances = [
     jobs_running: 0,
     jobs_total: 68,
     cpu: 6,
+    node_type: 'hybrid',
     memory: 2087469056,
     cpu_capacity: 24,
     mem_capacity: 1,
@@ -93,6 +95,7 @@ const instances = [
     jobs_running: 0,
     jobs_total: 68,
     cpu: 6,
+    node_type: 'execution',
     memory: 2087469056,
     cpu_capacity: 24,
     mem_capacity: 1,
@@ -197,4 +200,29 @@ describe('<InstanceList/>', () => {
     wrapper.update();
     expect(wrapper.find('AlertModal')).toHaveLength(1);
   });
+
+  test('should disable disassociate button', async () => {
+    expect(
+      wrapper.find('Button[ouiaId="disassociate-button"]').prop('isDisabled')
+    ).toBe(true);
+    await act(async () =>
+      wrapper.find('DataListToolbar').prop('onSelectAll')(instances)
+    );
+
+    wrapper.update();
+    expect(
+      wrapper.find('Button[ouiaId="disassociate-button"]').prop('isDisabled')
+    ).toBe(true);
+    await act(async () =>
+      wrapper
+        .find('Tr#instance-row-1')
+        .find('SelectColumn[aria-label="Select row 0"]')
+        .prop('onSelect')(false)
+    );
+
+    wrapper.update();
+    expect(
+      wrapper.find('Button[ouiaId="disassociate-button"]').prop('isDisabled')
+    ).toBe(false);
+  });
 });

awx/ui/src/screens/InstanceGroup/Instances/InstanceListItem.js

@@ -121,9 +121,8 @@ function InstanceListItem({
         <Td
           select={{
             rowIndex,
-            isSelected: isSelected && instance.node_type !== 'control',
+            isSelected,
             onSelect,
-            disable: instance.node_type === 'control',
           }}
           dataLabel={t`Selected`}
         />

awx/ui/src/screens/InstanceGroup/Instances/InstanceListItem.test.js

@@ -201,24 +201,6 @@ describe('<InstanceListItem/>', () => {
     expect(wrapper.find('Td').at(1).prop('select').onSelect).toEqual(onSelect);
   });
 
-  test('should disable checkbox', async () => {
-    const onSelect = jest.fn();
-    await act(async () => {
-      wrapper = mountWithContexts(
-        <table>
-          <tbody>
-            <InstanceListItem
-              instance={instance[1]}
-              onSelect={onSelect}
-              fetchInstances={() => {}}
-            />
-          </tbody>
-        </table>
-      );
-    });
-    expect(wrapper.find('Td').at(1).prop('select').disable).toEqual(true);
-  });
-
   test('should display instance toggle', () => {
     expect(wrapper.find('InstanceToggle').length).toBe(1);
   });