fix(machines): move "check power" to power action dropdown MAASENG-21…

…25 (#5196)

src/app/base/components/NodeActionMenu/NodeActionMenu.tsx

@@ -56,7 +56,12 @@ const actionGroups: ActionGroup[] = [
   },
   {
     name: "power",
-    actions: [NodeActions.ON, NodeActions.OFF],
+    actions: [
+      NodeActions.ON,
+      NodeActions.OFF,
+      NodeActions.SOFT_OFF,
+      NodeActions.CHECK_POWER,
+    ],
   },
   {
     name: "testing",

src/app/base/components/NodeActionMenuGroup/NodeActionMenuGroup.test.tsx

@@ -64,8 +64,8 @@ describe("NodeActionMenuGroup", () => {
       screen.queryByRole("button", { name: Labels.Lock })
     ).not.toBeInTheDocument();
     expect(
-      screen.queryByRole("button", { name: Labels.PowerCycle })
-    ).not.toBeInTheDocument();
+      screen.getByRole("button", { name: Labels.Power })
+    ).toBeInTheDocument();
     expect(
       screen.queryByRole("button", { name: Labels.Troubleshoot })
     ).not.toBeInTheDocument();

src/app/base/components/NodeActionMenuGroup/NodeActionMenuGroup.tsx

@@ -40,7 +40,7 @@ type Props = {
 
 export enum Labels {
   Actions = "Actions",
-  PowerCycle = "Power cycle",
+  Power = "Power",
   Troubleshoot = "Troubleshoot",
   Categorise = "Categorise",
   Lock = "Lock",
@@ -62,8 +62,13 @@ const actionGroups: ActionGroup[] = [
   },
   {
     name: "power",
-    actions: [NodeActions.ON, NodeActions.OFF, NodeActions.SOFT_OFF],
-    title: Labels.PowerCycle,
+    actions: [
+      NodeActions.ON,
+      NodeActions.OFF,
+      NodeActions.SOFT_OFF,
+      NodeActions.CHECK_POWER,
+    ],
+    title: Labels.Power,
   },
   {
     name: "testing",

src/app/base/hooks/node.ts

@@ -20,6 +20,7 @@ import { kebabToCamelCase } from "app/utils";
 // without needing the user to provide any additional information.
 export type MachineMenuAction = Exclude<
   MachineActions,
+  | NodeActions.CHECK_POWER
   | NodeActions.CLONE
   | NodeActions.SET_POOL
   | NodeActions.SET_ZONE

src/app/machines/components/MachineForms/MachineActionFormWrapper/MachineActionFormWrapper.tsx

@@ -184,6 +184,11 @@ export const MachineActionForm = ({
             {...commonNodeFormProps}
           />
         );
+      // No form should be opened for this, as it should only
+      // be available for machine details, and will be dispatched
+      // immediately on click.
+      case NodeActions.CHECK_POWER:
+        return null;
     }
   };
 

src/app/machines/constants.ts

@@ -4,6 +4,7 @@ import { NodeActions } from "app/store/types/node";
 export const MachineActionSidePanelViews = {
   ABORT_MACHINE: ["machineActionForm", NodeActions.ABORT],
   ACQUIRE_MACHINE: ["machineActionForm", NodeActions.ACQUIRE],
+  CHECK_MACHINE_POEWR: ["machineActionForm", NodeActions.CHECK_POWER],
   CLONE_MACHINE: ["machineActionForm", NodeActions.CLONE],
   COMMISSION_MACHINE: ["machineActionForm", NodeActions.COMMISSION],
   DELETE_MACHINE: ["machineActionForm", NodeActions.DELETE],

src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.test.tsx

@@ -118,9 +118,7 @@ describe("MachineHeader", () => {
         { store, route: "/machine/abc123" }
       );
 
-      await userEvent.click(
-        screen.getByRole("button", { name: /take action:/i })
-      );
+      await userEvent.click(screen.getByRole("button", { name: /Power/i }));
       await userEvent.click(
         screen.getByRole("button", { name: /check power/i })
       );

src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.tsx

@@ -1,4 +1,4 @@
-import { useRef, useState } from "react";
+import { useState } from "react";
 
 import { useDispatch, useSelector } from "react-redux";
 import { useLocation } from "react-router-dom";
@@ -11,7 +11,6 @@ import NodeActionMenuGroup from "app/base/components/NodeActionMenuGroup";
 import PowerIcon from "app/base/components/PowerIcon";
 import ScriptStatus from "app/base/components/ScriptStatus";
 import SectionHeader from "app/base/components/SectionHeader";
-import TableMenu from "app/base/components/TableMenu";
 import TooltipButton from "app/base/components/TooltipButton";
 import { useSendAnalytics } from "app/base/hooks";
 import { MachineSidePanelViews } from "app/machines/constants";
@@ -53,7 +52,6 @@ const MachineHeader = ({
     useSelectedMachinesActionsDispatch({
       selectedMachines: { items: [systemId] },
     });
-  const powerMenuRef = useRef<HTMLSpanElement>(null);
   const isDetails = isMachineDetails(machine);
   useFetchMachine(systemId);
 
@@ -69,6 +67,8 @@ const MachineHeader = ({
 
     if (isImmediateAction) {
       dispatchForSelectedMachines(machineActions[action]);
+    } else if (action === NodeActions.CHECK_POWER) {
+      dispatch(machineActions.checkPower(systemId));
     } else {
       const view = Object.values(MachineSidePanelViews).find(
         ([, actionName]) => actionName === action
@@ -113,21 +113,8 @@ const MachineHeader = ({
                   ? "Checking power"
                   : `Power ${machine.power_state}`}
               </PowerIcon>
-              <TableMenu
-                className="u-nudge-right--small p-table-menu u-nudge-left--large"
-                links={[
-                  {
-                    children: "Check power",
-                    onClick: () => {
-                      dispatch(machineActions.checkPower(systemId));
-                    },
-                  },
-                ]}
-                positionNode={powerMenuRef?.current}
-                title="Take action:"
-              />
             </div>
-            <div className="u-hide--medium u-hide--small">
+            <div className="u-hide--medium u-hide--small u-nudge-right">
               <NodeActionMenuGroup
                 alwaysShowLifecycle
                 excludeActions={[NodeActions.IMPORT_IMAGES]}
@@ -140,7 +127,7 @@ const MachineHeader = ({
                 singleNode
               />
             </div>
-            <div className="u-hide--large">
+            <div className="u-hide--large u-nudge-right">
               <NodeActionMenu
                 alwaysShowLifecycle
                 className="u-hide--large"

src/app/machines/views/MachineList/MachineListControls/MachineActionMenu/MachineActionMenu.tsx

@@ -42,59 +42,41 @@ const MachineActionMenu = ({
     [tagsSeen]
   );
 
+  const commonProps = {
+    alwaysShowLifecycle: true,
+    excludeActions: [NodeActions.IMPORT_IMAGES, NodeActions.CHECK_POWER],
+    getTitle,
+    hasSelection,
+    nodeDisplay: "machine",
+    onActionClick: (action: NodeActions) => {
+      if (action === NodeActions.TAG && !tagsSeen) {
+        setTagsSeen(true);
+      }
+      const view = Object.values(MachineSidePanelViews).find(
+        ([, actionName]) => actionName === action
+      );
+      if (view) {
+        setSidePanelContent({ view });
+      }
+      sendAnalytics(
+        "Machine list action form",
+        getNodeActionTitle(action),
+        "Open"
+      );
+    },
+  };
+
   return (
     <>
       <div className="u-hide--medium u-hide--small">
-        <NodeActionMenuGroup
-          alwaysShowLifecycle
-          excludeActions={[NodeActions.IMPORT_IMAGES]}
-          getTitle={getTitle}
-          hasSelection={hasSelection}
-          nodeDisplay="machine"
-          onActionClick={(action) => {
-            if (action === NodeActions.TAG && !tagsSeen) {
-              setTagsSeen(true);
-            }
-            const view = Object.values(MachineSidePanelViews).find(
-              ([, actionName]) => actionName === action
-            );
-            if (view) {
-              setSidePanelContent({ view });
-            }
-            sendAnalytics(
-              "Machine list action form",
-              getNodeActionTitle(action),
-              "Open"
-            );
-          }}
-        />
+        <NodeActionMenuGroup {...commonProps} />
       </div>
       <div className="u-hide--large">
         <NodeActionMenu
-          alwaysShowLifecycle
+          {...commonProps}
           className="is-maas-select"
           constrainPanelWidth
-          excludeActions={[NodeActions.IMPORT_IMAGES]}
-          getTitle={getTitle}
-          hasSelection={hasSelection}
           menuPosition="left"
-          nodeDisplay="machine"
-          onActionClick={(action) => {
-            if (action === NodeActions.TAG && !tagsSeen) {
-              setTagsSeen(true);
-            }
-            const view = Object.values(MachineSidePanelViews).find(
-              ([, actionName]) => actionName === action
-            );
-            if (view) {
-              setSidePanelContent({ view });
-            }
-            sendAnalytics(
-              "Machine list action form",
-              getNodeActionTitle(action),
-              "Open"
-            );
-          }}
           toggleAppearance=""
           toggleClassName="p-action-menu"
           toggleLabel="Menu"

src/app/machines/views/MachineList/MachineListControls/MachineListControls.test.tsx

@@ -95,7 +95,7 @@ describe("MachineListControls", () => {
       screen.queryByRole("button", { name: "Actions" })
     ).not.toBeInTheDocument();
     expect(
-      screen.queryByRole("button", { name: "Power cycle" })
+      screen.queryByRole("button", { name: "Power" })
     ).not.toBeInTheDocument();
     expect(
       screen.queryByRole("button", { name: "Troubleshoot" })
@@ -130,9 +130,7 @@ describe("MachineListControls", () => {
     );
 
     expect(screen.getByRole("button", { name: "Actions" })).toBeInTheDocument();
-    expect(
-      screen.getByRole("button", { name: "Power cycle" })
-    ).toBeInTheDocument();
+    expect(screen.getByRole("button", { name: "Power" })).toBeInTheDocument();
     expect(
       screen.getByRole("button", { name: "Troubleshoot" })
     ).toBeInTheDocument();

src/app/store/types/node.ts

@@ -158,6 +158,7 @@ export enum FetchNodeStatus {
 export enum NodeActions {
   ABORT = "abort",
   ACQUIRE = "acquire",
+  CHECK_POWER = "check-power",
   CLONE = "clone",
   COMMISSION = "commission",
   DELETE = "delete",

src/app/store/utils/node/base.ts

@@ -52,6 +52,8 @@ export const getNodeActionTitle = (actionName: NodeActions): string => {
       return "Abort";
     case NodeActions.ACQUIRE:
       return "Allocate";
+    case NodeActions.CHECK_POWER:
+      return "Check power";
     case NodeActions.CLONE:
       return "Clone from";
     case NodeActions.COMMISSION:
@@ -109,6 +111,10 @@ export const getNodeActionLabel = (
       } actions for ${modelString}`;
     case NodeActions.ACQUIRE:
       return `${isProcessing ? "Allocating" : "Allocate"} ${modelString}`;
+    case NodeActions.CHECK_POWER:
+      return `${
+        isProcessing ? "Checking power" : "Check power"
+      } for ${modelString}`;
     case NodeActions.CLONE:
       return isProcessing ? "Cloning in progress" : `Clone to ${modelString}`;
     case NodeActions.COMMISSION:
@@ -267,5 +273,10 @@ export const canOpenActionForm = (
     // machines can only be in a subset of statuses.
     return [NodeStatus.READY, NodeStatus.FAILED_TESTING].includes(node.status);
   }
+
+  if (nodeIsMachine(node) && actionName === NodeActions.CHECK_POWER) {
+    // "Check power" is always shown for machines, even though it's not listed in node.actions.
+    return true;
+  }
   return node.actions.some((nodeAction) => nodeAction === actionName);
 };