Refactor the health check button

awx/ui/src/components/HealthCheckButton/HealthCheckButton.js

@@ -5,37 +5,26 @@ import { useKebabifiedMenu } from 'contexts/Kebabified';
 
 function HealthCheckButton({ isDisabled, onClick, selectedItems }) {
   const { isKebabified } = useKebabifiedMenu();
-  const hopNodeSelected =
-    selectedItems.filter((instance) => instance.node_type === 'hop').length > 0;
-  const hasSelectedItems = selectedItems.length > 0;
 
-  const buildTooltip = () => {
+  const selectedItemsCount = selectedItems.length;
-    if (hopNodeSelected) {
+
-      return (
+  const buildTooltip = () =>
-        <Plural
+    selectedItemsCount ? (
-          value={hopNodeSelected}
-          one="Cannot run health check on a hop node.  Deselect the hop node to run a health check."
-          other="Cannot run health check on hop nodes.  Deselect the hop nodes to run health checks."
-        />
-      );
-    }
-    return selectedItems.length ? (
       <Plural
-        value={selectedItems.length}
+        value={selectedItemsCount}
         one="Click to run a health check on the selected instance."
         other="Click to run a health check on the selected instances."
       />
     ) : (
       t`Select an instance to run a health check.`
     );
-  };
 
   if (isKebabified) {
     return (
       <Tooltip data-cy="healthCheckTooltip" content={buildTooltip()}>
         <DropdownItem
           key="approve"
-          isDisabled={hopNodeSelected || isDisabled || !hasSelectedItems}
+          isDisabled={isDisabled || !selectedItemsCount}
           component="button"
           onClick={onClick}
           ouiaId="health-check"
@@ -49,7 +38,7 @@ function HealthCheckButton({ isDisabled, onClick, selectedItems }) {
     <Tooltip data-cy="healthCheckTooltip" content={buildTooltip()}>
       <div>
         <Button
-          isDisabled={hopNodeSelected || isDisabled || !hasSelectedItems}
+          isDisabled={isDisabled || !selectedItemsCount}
           variant="secondary"
           ouiaId="health-check"
           onClick={onClick}

awx/ui/src/components/PaginatedTable/HeaderRow.js

@@ -69,6 +69,7 @@ export function HeaderCell({
   idPrefix,
   className,
   children,
+  tooltip,
 }) {
   const sort = sortKey
     ? {
@@ -79,6 +80,11 @@ export function HeaderCell({
     : null;
   return (
     <Th
+      info={
+        tooltip && {
+          popover: <div>{tooltip}</div>,
+        }
+      }
       id={sortKey ? `${idPrefix}-${sortKey}` : null}
       className={className}
       sort={sort}

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

@@ -88,10 +88,14 @@ function InstanceList() {
     useCallback(async () => {
       await Promise.all(selected.map(({ id }) => InstancesAPI.healthCheck(id)));
       fetchInstances();
-      clearSelected();
+    }, [selected, fetchInstances])
-    }, [selected, clearSelected, fetchInstances])
   );
 
+  const handleHealthCheck = async () => {
+    await fetchHealthCheck();
+    clearSelected();
+  };
+
   const {
     isLoading: isDisassociateLoading,
     deleteItems: disassociateInstances,
@@ -231,7 +235,7 @@ function InstanceList() {
               />,
               <HealthCheckButton
                 isDisabled={!canAdd}
-                onClick={fetchHealthCheck}
+                onClick={handleHealthCheck}
                 selectedItems={selected}
               />,
             ]}
@@ -250,7 +254,7 @@ function InstanceList() {
             <HeaderCell sortKey="hostname">{t`Name`}</HeaderCell>
             <HeaderCell sortKey="errors">{t`Status`}</HeaderCell>
             <HeaderCell sortKey="node_type">{t`Node Type`}</HeaderCell>
-            <HeaderCell sortKey="capacity_adjustment">{t`Capacity Adjustment`}</HeaderCell>
+            <HeaderCell>{t`Capacity Adjustment`}</HeaderCell>
             <HeaderCell>{t`Used Capacity`}</HeaderCell>
             <HeaderCell>{t`Actions`}</HeaderCell>
           </HeaderRow>

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

@@ -19,6 +19,7 @@ import StatusLabel from 'components/StatusLabel';
 import { Instance } from 'types';
 import useRequest, { useDismissableError } from 'hooks/useRequest';
 import useDebounce from 'hooks/useDebounce';
+import computeForks from 'util/computeForks';
 import { InstancesAPI } from 'api';
 import { useConfig } from 'contexts/Config';
 import AlertModal from 'components/AlertModal';
@@ -42,15 +43,6 @@ const SliderForks = styled.div`
   text-align: center;
 `;
 
-function computeForks(memCapacity, cpuCapacity, selectedCapacityAdjustment) {
-  const minCapacity = Math.min(memCapacity, cpuCapacity);
-  const maxCapacity = Math.max(memCapacity, cpuCapacity);
-
-  return Math.floor(
-    minCapacity + (maxCapacity - minCapacity) * selectedCapacityAdjustment
-  );
-}
-
 function InstanceListItem({
   instance,
   isExpanded,
@@ -196,13 +188,23 @@ function InstanceListItem({
         <Td colSpan={7}>
           <ExpandableRowContent>
             <DetailList>
-              <Detail value={instance.jobs_running} label={t`Running Jobs`} />
-              <Detail value={instance.jobs_total} label={t`Total Jobs`} />
               <Detail
+                data-cy="running-jobs"
+                value={instance.jobs_running}
+                label={t`Running Jobs`}
+              />
+              <Detail
+                data-cy="total-jobs"
+                value={instance.jobs_total}
+                label={t`Total Jobs`}
+              />
+              <Detail
+                data-cy="policy-type"
                 label={t`Policy Type`}
                 value={instance.managed_by_policy ? t`Auto` : t`Manual`}
               />
               <Detail
+                data-cy="last-health-check"
                 label={t`Last Health Check`}
                 value={formatDateString(instance.last_health_check)}
               />

awx/ui/src/screens/Instances/Instance.js

@@ -33,7 +33,6 @@ function Instance({ setBreadcrumb }) {
           <Route path="/instances/:id/details" key="details">
             <InstanceDetail setBreadcrumb={setBreadcrumb} />
           </Route>
-          ,
           <Route path="*" key="not-found">
             <ContentError isNotFound>
               {match.params.id && (
@@ -49,5 +48,4 @@ function Instance({ setBreadcrumb }) {
   );
 }
 
-export { Instance as _Instance };
 export default Instance;

awx/ui/src/screens/Instances/InstanceDetail/InstanceDetail.js

@@ -134,124 +134,122 @@ function InstanceDetail({ setBreadcrumb }) {
   }
   const isHopNode = instance.node_type === 'hop';
   return (
-    <>
+    <CardBody>
-      <CardBody>
+      <DetailList gutter="sm">
-        <DetailList gutter="sm">
+        <Detail
-          <Detail
+          label={t`Host Name`}
-            label={t`Host Name`}
+          value={instance.hostname}
-            value={instance.hostname}
+          dataCy="instance-detail-name"
-            dataCy="instance-detail-name"
+        />
-          />
+        <Detail
-          <Detail
+          label={t`Status`}
-            label={t`Status`}
+          value={
-            value={
+            <StatusLabel status={healthCheck?.errors ? 'error' : 'healthy'} />
-              <StatusLabel status={healthCheck?.errors ? 'error' : 'healthy'} />
+          }
-            }
+        />
-          />
+        <Detail label={t`Node Type`} value={instance.node_type} />
-          <Detail label={t`Node Type`} value={instance.node_type} />
+        {!isHopNode && (
-          {!isHopNode && (
+          <>
-            <>
-              <Detail
-                label={t`Policy Type`}
-                value={instance.managed_by_policy ? t`Auto` : t`Manual`}
-              />
-              <Detail label={t`Running Jobs`} value={instance.jobs_running} />
-              <Detail label={t`Total Jobs`} value={instance.jobs_total} />
-              <Detail
-                label={t`Last Health Check`}
-                value={formatDateString(healthCheck?.last_health_check)}
-              />
-              <Detail
-                label={t`Capacity Adjustment`}
-                value={
-                  <SliderHolder data-cy="slider-holder">
-                    <div data-cy="cpu-capacity">{t`CPU ${instance.cpu_capacity}`}</div>
-                    <SliderForks data-cy="slider-forks">
-                      <div data-cy="number-forks">
-                        <Plural value={forks} one="# fork" other="# forks" />
-                      </div>
-                      <Slider
-                        areCustomStepsContinuous
-                        max={1}
-                        min={0}
-                        step={0.1}
-                        value={instance.capacity_adjustment}
-                        onChange={handleChangeValue}
-                        isDisabled={!me?.is_superuser || !instance.enabled}
-                        data-cy="slider"
-                      />
-                    </SliderForks>
-                    <div data-cy="mem-capacity">{t`RAM ${instance.mem_capacity}`}</div>
-                  </SliderHolder>
-                }
-              />
-              <Detail
-                label={t`Used Capacity`}
-                value={
-                  instance.enabled ? (
-                    <Progress
-                      title={t`Used capacity`}
-                      value={Math.round(
-                        100 - instance.percent_capacity_remaining
-                      )}
-                      measureLocation={ProgressMeasureLocation.top}
-                      size={ProgressSize.sm}
-                      aria-label={t`Used capacity`}
-                    />
-                  ) : (
-                    <Unavailable>{t`Unavailable`}</Unavailable>
-                  )
-                }
-              />
-            </>
-          )}
-          {healthCheck?.errors && (
             <Detail
-              fullWidth
+              label={t`Policy Type`}
-              label={t`Errors`}
+              value={instance.managed_by_policy ? t`Auto` : t`Manual`}
+            />
+            <Detail label={t`Running Jobs`} value={instance.jobs_running} />
+            <Detail label={t`Total Jobs`} value={instance.jobs_total} />
+            <Detail
+              label={t`Last Health Check`}
+              value={formatDateString(healthCheck?.last_health_check)}
+            />
+            <Detail
+              label={t`Capacity Adjustment`}
               value={
-                <CodeBlock>
+                <SliderHolder data-cy="slider-holder">
-                  <CodeBlockCode>{healthCheck?.errors}</CodeBlockCode>
+                  <div data-cy="cpu-capacity">{t`CPU ${instance.cpu_capacity}`}</div>
-                </CodeBlock>
+                  <SliderForks data-cy="slider-forks">
+                    <div data-cy="number-forks">
+                      <Plural value={forks} one="# fork" other="# forks" />
+                    </div>
+                    <Slider
+                      areCustomStepsContinuous
+                      max={1}
+                      min={0}
+                      step={0.1}
+                      value={instance.capacity_adjustment}
+                      onChange={handleChangeValue}
+                      isDisabled={!me?.is_superuser || !instance.enabled}
+                      data-cy="slider"
+                    />
+                  </SliderForks>
+                  <div data-cy="mem-capacity">{t`RAM ${instance.mem_capacity}`}</div>
+                </SliderHolder>
               }
             />
-          )}
+            <Detail
-        </DetailList>
+              label={t`Used Capacity`}
-        {!isHopNode && (
+              value={
-          <CardActionsRow>
+                instance.enabled ? (
-            <Tooltip content={t`Run a health check on the instance`}>
+                  <Progress
-              <Button
+                    title={t`Used capacity`}
-                isDisabled={!me.is_superuser}
+                    value={Math.round(
-                variant="primary"
+                      100 - instance.percent_capacity_remaining
-                ouiaId="health-check-button"
+                    )}
-                onClick={fetchHealthCheck}
+                    measureLocation={ProgressMeasureLocation.top}
-              >
+                    size={ProgressSize.sm}
-                {t`Health Check`}
+                    aria-label={t`Used capacity`}
-              </Button>
+                  />
-            </Tooltip>
+                ) : (
-            <InstanceToggle
+                  <Unavailable>{t`Unavailable`}</Unavailable>
-              css="display: inline-flex;"
+                )
-              fetchInstances={fetchDetails}
+              }
-              instance={instance}
             />
-          </CardActionsRow>
+          </>
         )}
+        {healthCheck?.errors && (
+          <Detail
+            fullWidth
+            label={t`Errors`}
+            value={
+              <CodeBlock>
+                <CodeBlockCode>{healthCheck?.errors}</CodeBlockCode>
+              </CodeBlock>
+            }
+          />
+        )}
+      </DetailList>
+      {!isHopNode && (
+        <CardActionsRow>
+          <Tooltip content={t`Run a health check on the instance`}>
+            <Button
+              isDisabled={!me.is_superuser}
+              variant="primary"
+              ouiaId="health-check-button"
+              onClick={fetchHealthCheck}
+            >
+              {t`Health Check`}
+            </Button>
+          </Tooltip>
+          <InstanceToggle
+            css="display: inline-flex;"
+            fetchInstances={fetchDetails}
+            instance={instance}
+          />
+        </CardActionsRow>
+      )}
 
-        {error && (
+      {error && (
-          <AlertModal
+        <AlertModal
-            isOpen={error}
+          isOpen={error}
-            onClose={dismissError}
+          onClose={dismissError}
-            title={t`Error!`}
+          title={t`Error!`}
-            variant="error"
+          variant="error"
-          >
+        >
-            {updateInstanceError
+          {updateInstanceError
-              ? t`Failed to update capacity adjustment.`
+            ? t`Failed to update capacity adjustment.`
-              : t`Failed to disassociate one or more instances.`}
+            : t`Failed to disassociate one or more instances.`}
-            <ErrorDetail error={error} />
+          <ErrorDetail error={error} />
-          </AlertModal>
+        </AlertModal>
-        )}
+      )}
-      </CardBody>
+    </CardBody>
-    </>
   );
 }
 

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

@@ -61,7 +61,7 @@ function InstanceList() {
   );
 
   const { selected, isAllSelected, handleSelect, clearSelected, selectAll } =
-    useSelected(instances);
+    useSelected(instances.filter((i) => i.node_type !== 'hop'));
 
   useEffect(() => {
     fetchInstances();
@@ -79,15 +79,16 @@ function InstanceList() {
           .map(({ id }) => InstancesAPI.healthCheck(id))
       );
       fetchInstances();
-      clearSelected();
+    }, [selected, fetchInstances])
-    }, [selected, clearSelected, fetchInstances])
   );
-
+  const handleHealthCheck = async () => {
+    await fetchHealthCheck();
+    clearSelected();
+  };
   const { error, dismissError } = useDismissableError(healthCheckError);
 
   const { expanded, isAllExpanded, handleExpand, expandAll } =
     useExpanded(instances);
-
   return (
     <>
       <PageSection>
@@ -135,7 +136,7 @@ function InstanceList() {
                 qsConfig={QS_CONFIG}
                 additionalControls={[
                   <HealthCheckButton
-                    onClick={fetchHealthCheck}
+                    onClick={handleHealthCheck}
                     selectedItems={selected}
                   />,
                 ]}
@@ -143,10 +144,13 @@ function InstanceList() {
             )}
             headerRow={
               <HeaderRow qsConfig={QS_CONFIG} isExpandable>
-                <HeaderCell sortKey="hostname">{t`Name`}</HeaderCell>
+                <HeaderCell
+                  tooltip={t`Cannot run health check on hop nodes.`}
+                  sortKey="hostname"
+                >{t`Name`}</HeaderCell>
                 <HeaderCell sortKey="errors">{t`Status`}</HeaderCell>
                 <HeaderCell sortKey="node_type">{t`Node Type`}</HeaderCell>
-                <HeaderCell sortKey="capacity_adjustment">{t`Capacity Adjustment`}</HeaderCell>
+                <HeaderCell>{t`Capacity Adjustment`}</HeaderCell>
                 <HeaderCell>{t`Used Capacity`}</HeaderCell>
                 <HeaderCell>{t`Actions`}</HeaderCell>
               </HeaderRow>

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

@@ -152,13 +152,6 @@ describe('<InstanceList/>', () => {
       wrapper.find('DataListToolbar').prop('onSelectAll')(instances)
     );
     wrapper.update();
-
-    // Ensures health check button is disabled because a hop node is among
-    // the selected.
-    expect(
-      wrapper.find('Button[ouiaId="health-check"]').prop('isDisabled')
-    ).toBe(true);
-
     await act(async () =>
       wrapper.find('input[aria-label="Select row 3"]').prop('onChange')(false)
     );
@@ -197,17 +190,4 @@ describe('<InstanceList/>', () => {
     wrapper.update();
     expect(wrapper.find('AlertModal')).toHaveLength(1);
   });
-
-  test('Health check button should remain disabled', async () => {
-    await act(async () =>
-      wrapper.find('input[aria-label="Select row 3"]').prop('onChange')(true)
-    );
-    wrapper.update();
-    expect(
-      wrapper.find('Button[ouiaId="health-check"]').prop('isDisabled')
-    ).toBe(true);
-    expect(wrapper.find('Tooltip[data-cy="healthCheckTooltip"]').length).toBe(
-      1
-    );
-  });
 });

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

@@ -13,6 +13,7 @@ import {
 } from '@patternfly/react-core';
 import { Tr, Td, ExpandableRowContent } from '@patternfly/react-table';
 import { formatDateString } from 'util/dates';
+import computeForks from 'util/computeForks';
 import { ActionsTd, ActionItem } from 'components/PaginatedTable';
 import InstanceToggle from 'components/InstanceToggle';
 import StatusLabel from 'components/StatusLabel';
@@ -42,15 +43,6 @@ const SliderForks = styled.div`
   text-align: center;
 `;
 
-function computeForks(memCapacity, cpuCapacity, selectedCapacityAdjustment) {
-  const minCapacity = Math.min(memCapacity, cpuCapacity);
-  const maxCapacity = Math.max(memCapacity, cpuCapacity);
-
-  return Math.floor(
-    minCapacity + (maxCapacity - minCapacity) * selectedCapacityAdjustment
-  );
-}
-
 function InstanceListItem({
   instance,
   isExpanded,
@@ -120,7 +112,6 @@ function InstanceListItem({
             expand={{
               rowIndex,
               isExpanded,
-              disabled: isHopNode,
               onToggle: onExpand,
             }}
           />
@@ -130,6 +121,7 @@ function InstanceListItem({
             rowIndex,
             isSelected,
             onSelect,
+            disable: isHopNode,
           }}
           dataLabel={t`Selected`}
         />
@@ -138,21 +130,23 @@ function InstanceListItem({
             <b>{instance.hostname}</b>
           </Link>
         </Td>
-        {!instance.node_type !== 'hop' && (
+
-          <Td dataLabel={t`Status`}>
+        <Td dataLabel={t`Status`}>
-            <Tooltip
+          <Tooltip
-              content={
+            content={
-                <div>
+              <div>
-                  {t`Last Health Check`}
+                {t`Last Health Check`}
-                  &nbsp;
+                &nbsp;
-                  {formatDateString(instance.last_health_check)}
+                {formatDateString(
-                </div>
+                  instance.last_health_check ?? instance.last_seen
-              }
+                )}
-            >
+              </div>
-              <StatusLabel status={instance.errors ? 'error' : 'healthy'} />
+            }
-            </Tooltip>
+          >
-          </Td>
+            <StatusLabel status={instance.errors ? 'error' : 'healthy'} />
-        )}
+          </Tooltip>
+        </Td>
+
         <Td dataLabel={t`Node Type`}>{instance.node_type}</Td>
         {!isHopNode && (
           <>
@@ -209,13 +203,23 @@ function InstanceListItem({
           <Td colSpan={7}>
             <ExpandableRowContent>
               <DetailList>
-                <Detail value={instance.jobs_running} label={t`Running Jobs`} />
-                <Detail value={instance.jobs_total} label={t`Total Jobs`} />
                 <Detail
+                  data-cy="running-jobs"
+                  value={instance.jobs_running}
+                  label={t`Running Jobs`}
+                />
+                <Detail
+                  data-cy="total-jobs"
+                  value={instance.jobs_total}
+                  label={t`Total Jobs`}
+                />
+                <Detail
+                  data-cy="policy-type"
                   label={t`Policy Type`}
                   value={instance.managed_by_policy ? t`Auto` : t`Manual`}
                 />
                 <Detail
+                  data-cy="last-health-check"
                   label={t`Last Health Check`}
                   value={formatDateString(instance.last_health_check)}
                 />

awx/ui/src/util/computeForks.js

@@ -0,0 +1,12 @@
+export default function computeForks(
+  memCapacity,
+  cpuCapacity,
+  selectedCapacityAdjustment
+) {
+  const minCapacity = Math.min(memCapacity, cpuCapacity);
+  const maxCapacity = Math.max(memCapacity, cpuCapacity);
+
+  return Math.floor(
+    minCapacity + (maxCapacity - minCapacity) * selectedCapacityAdjustment
+  );
+}