From 1938f2eda1bd445db636f34c73a13168a2638e5e Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Thu, 26 Oct 2023 08:38:47 +0100 Subject: [PATCH] fix(machines): move "check power" to power action dropdown MAASENG-2125 (#5196) --- .../NodeActionMenu/NodeActionMenu.tsx | 7 +- .../NodeActionMenuGroup.test.tsx | 4 +- .../NodeActionMenuGroup.tsx | 11 ++- src/app/base/hooks/node.ts | 1 + .../MachineActionFormWrapper.tsx | 5 ++ src/app/machines/constants.ts | 1 + .../MachineHeader/MachineHeader.test.tsx | 4 +- .../MachineHeader/MachineHeader.tsx | 23 ++---- .../MachineActionMenu/MachineActionMenu.tsx | 70 +++++++------------ .../MachineListControls.test.tsx | 6 +- src/app/store/types/node.ts | 1 + src/app/store/utils/node/base.ts | 11 +++ 12 files changed, 69 insertions(+), 75 deletions(-) diff --git a/src/app/base/components/NodeActionMenu/NodeActionMenu.tsx b/src/app/base/components/NodeActionMenu/NodeActionMenu.tsx index ddf511ed28..165a335725 100644 --- a/src/app/base/components/NodeActionMenu/NodeActionMenu.tsx +++ b/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", diff --git a/src/app/base/components/NodeActionMenuGroup/NodeActionMenuGroup.test.tsx b/src/app/base/components/NodeActionMenuGroup/NodeActionMenuGroup.test.tsx index b8fefbd47d..5291188eb2 100644 --- a/src/app/base/components/NodeActionMenuGroup/NodeActionMenuGroup.test.tsx +++ b/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(); diff --git a/src/app/base/components/NodeActionMenuGroup/NodeActionMenuGroup.tsx b/src/app/base/components/NodeActionMenuGroup/NodeActionMenuGroup.tsx index a9837c5bdf..d3147d319d 100644 --- a/src/app/base/components/NodeActionMenuGroup/NodeActionMenuGroup.tsx +++ b/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", diff --git a/src/app/base/hooks/node.ts b/src/app/base/hooks/node.ts index bce97a7c96..416f44fb4f 100644 --- a/src/app/base/hooks/node.ts +++ b/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 diff --git a/src/app/machines/components/MachineForms/MachineActionFormWrapper/MachineActionFormWrapper.tsx b/src/app/machines/components/MachineForms/MachineActionFormWrapper/MachineActionFormWrapper.tsx index c31f34f2ee..af1f655b80 100644 --- a/src/app/machines/components/MachineForms/MachineActionFormWrapper/MachineActionFormWrapper.tsx +++ b/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; } }; diff --git a/src/app/machines/constants.ts b/src/app/machines/constants.ts index 334b232b3b..2382cdd1a7 100644 --- a/src/app/machines/constants.ts +++ b/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], diff --git a/src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.test.tsx b/src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.test.tsx index b7805f7eac..d5c5d232e1 100644 --- a/src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.test.tsx +++ b/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 }) ); diff --git a/src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.tsx b/src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.tsx index e1bc25eb68..ec540baa68 100644 --- a/src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.tsx +++ b/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(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}`} - { - dispatch(machineActions.checkPower(systemId)); - }, - }, - ]} - positionNode={powerMenuRef?.current} - title="Take 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" + ); + }, + }; + return ( <>
- { - 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" - ); - }} - /> +
{ - 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" diff --git a/src/app/machines/views/MachineList/MachineListControls/MachineListControls.test.tsx b/src/app/machines/views/MachineList/MachineListControls/MachineListControls.test.tsx index 7dcb882f13..8283099839 100644 --- a/src/app/machines/views/MachineList/MachineListControls/MachineListControls.test.tsx +++ b/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(); diff --git a/src/app/store/types/node.ts b/src/app/store/types/node.ts index a287497fe6..2d4e048637 100644 --- a/src/app/store/types/node.ts +++ b/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", diff --git a/src/app/store/utils/node/base.ts b/src/app/store/utils/node/base.ts index 7a594081bf..5d9d93d1e5 100644 --- a/src/app/store/utils/node/base.ts +++ b/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); };