Added DASDs format confirmation dialog (agama-project#1618)

**Trello:** https://trello.com/c/9TprKm3i As formatting a device is a dangerous action and could irreversibly destroy data on the target disk it would be nice to have a confirmation before continue. In the current PR we added that confirmation dialog.
jcronenberg · Oct 1, 2024 · 7b02af1 · 7b02af1
2 parents 710060d + 8afe8cb
commit 7b02af1
Show file tree

Hide file tree

Showing 3 changed files with 166 additions and 46 deletions.
diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes
@@ -14,6 +14,12 @@ Fri Sep 27 13:00:05 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>
 
 - Properly translate the storage interface when switching
   the language of the UI (gh#agama-project/agama#1629).
+
+-------------------------------------------------------------------
+Mon Sep 23 09:04:56 UTC 2024 - Knut Anderssen <[email protected]>
+
+- Added confirmation dialog when formatting DASD devices as it is
+  considered a dangerous action (gh#openSUSE/agama#1618).
 
 -------------------------------------------------------------------
 Fri Sep 20 11:42:25 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

diff --git a/web/src/components/storage/dasd/DASDTable.test.tsx b/web/src/components/storage/dasd/DASDTable.test.tsx
@@ -39,8 +39,20 @@ describe("DASDTable", () => {
     beforeEach(() => {
       mockDASDDevices = [
         {
-          id: "0.0.0200",
+          id: "0.0.0160",
           enabled: false,
+          deviceName: "",
+          deviceType: "",
+          formatted: false,
+          diag: false,
+          status: "offline",
+          accessType: "",
+          partitionInfo: "",
+          hexId: 0x160,
+        },
+        {
+          id: "0.0.0200",
+          enabled: true,
           deviceName: "dasda",
           deviceType: "eckd",
           formatted: false,
@@ -57,5 +69,51 @@ describe("DASDTable", () => {
       installerRender(<DASDTable />);
       screen.getByText("active");
     });
+
+    it("does not allow to perform any action if not selected any device", () => {
+      installerRender(<DASDTable />);
+      const button = screen.getByRole("button", { name: "Perform an action" });
+      expect(button).toHaveAttribute("disabled");
+    });
+
+    describe("when there are some DASD selected", () => {
+      it("allows to perform a set of actions over them", async () => {
+        const { user } = installerRender(<DASDTable />);
+        const selection = screen.getByRole("checkbox", { name: "Select row 0" });
+        await user.click(selection);
+        const button = screen.getByRole("button", { name: "Perform an action" });
+        expect(button).not.toHaveAttribute("disabled");
+        await user.click(button);
+        screen.getByRole("menuitem", { name: "Format" });
+      });
+
+      describe("and the user click on format", () => {
+        it("shows a confirmation dialog if all the devices are online", async () => {
+          const { user } = installerRender(<DASDTable />);
+          const selection = screen.getByRole("checkbox", { name: "Select row 1" });
+          await user.click(selection);
+          const button = screen.getByRole("button", { name: "Perform an action" });
+          expect(button).not.toHaveAttribute("disabled");
+          await user.click(button);
+          const format = screen.getByRole("menuitem", { name: "Format" });
+          await user.click(format);
+          screen.getByRole("dialog", { name: "Format selected devices?" });
+        });
+
+        it("shows a warning dialog if some device is offline", async () => {
+          const { user } = installerRender(<DASDTable />);
+          let selection = screen.getByRole("checkbox", { name: "Select row 0" });
+          await user.click(selection);
+          selection = screen.getByRole("checkbox", { name: "Select row 1" });
+          await user.click(selection);
+          const button = screen.getByRole("button", { name: "Perform an action" });
+          expect(button).not.toHaveAttribute("disabled");
+          await user.click(button);
+          const format = screen.getByRole("menuitem", { name: "Format" });
+          await user.click(format);
+          screen.getByRole("dialog", { name: "Cannot format all selected devices" });
+        });
+      });
+    });
   });
 });
diff --git a/web/src/components/storage/dasd/DASDTable.tsx b/web/src/components/storage/dasd/DASDTable.tsx
@@ -27,7 +27,11 @@ import {
   Dropdown,
   DropdownItem,
   DropdownList,
+  List,
+  ListItem,
   MenuToggle,
+  Stack,
+  Text,
   TextInputGroup,
   TextInputGroupMain,
   TextInputGroupUtilities,
@@ -36,6 +40,7 @@ import {
   ToolbarGroup,
   ToolbarItem,
 } from "@patternfly/react-core";
+import { Popup } from "~/components/core";
 import { Table, Thead, Tr, Th, Tbody, Td } from "@patternfly/react-table";
 import { Page } from "~/components/core";
 import { Icon } from "~/components/layout";
@@ -56,7 +61,7 @@ const columnData = (device: DASDDevice, column: { id: string; sortId?: string; l
       if (!device.enabled) data = "";
       break;
     case "partitionInfo":
-      data = data.split(",").map((d) => <div key={d}>{d}</div>);
+      data = data.split(",").map((d: string) => <div key={d}>{d}</div>);
       break;
   }
 
@@ -79,29 +84,64 @@ const columns = [
   { id: "formatted", label: _("Formatted") },
   { id: "partitionInfo", label: _("Partition Info") },
 ];
+const DevicesList = ({ devices }) => (
+  <List>
+    {devices.map((d: DASDDevice) => (
+      <ListItem key={d.id}>{d.id}</ListItem>
+    ))}
+  </List>
+);
+const FormatNotPossible = ({ devices, onAccept }) => (
+  <Popup isOpen title={_("Cannot format all selected devices")}>
+    <Stack hasGutter>
+      <Text>
+        {_(
+          "Offline devices must be activated before formatting them. Please, unselect or activate the devices listed below and try it again",
+        )}
+      </Text>
+      <DevicesList devices={devices} />
+    </Stack>
+    <Popup.Actions>
+      <Popup.Confirm onClick={onAccept}>{_("Accept")}</Popup.Confirm>
+    </Popup.Actions>
+  </Popup>
+);
+
+const FormatConfirmation = ({ devices, onCancel, onConfirm }) => (
+  <Popup isOpen title={_("Format selected devices?")}>
+    <Stack hasGutter>
+      <Text>
+        {_(
+          "This action could destroy any data stored on the devices listed below. Please, confirm that you really want to continue.",
+        )}
+      </Text>
+      <DevicesList devices={devices} />
+    </Stack>
+    <Popup.Actions>
+      <Popup.Confirm onClick={onConfirm} />
+      <Popup.Cancel onClick={onCancel} autoFocus />
+    </Popup.Actions>
+  </Popup>
+);
 
 const Actions = ({ devices, isDisabled }: { devices: DASDDevice[]; isDisabled: boolean }) => {
   const { mutate: updateDASD } = useDASDMutation();
   const { mutate: formatDASD } = useFormatDASDMutation();
   const [isOpen, setIsOpen] = useState(false);
+  const [requestFormat, setRequestFormat] = useState(false);
 
   const onToggle = () => setIsOpen(!isOpen);
   const onSelect = () => setIsOpen(false);
+  const cancelFormatRequest = () => setRequestFormat(false);
 
   const deviceIds = devices.map((d) => d.id);
+  const offlineDevices = devices.filter((d) => !d.enabled);
+  const offlineDevicesSelected = offlineDevices.length > 0;
   const activate = () => updateDASD({ action: "enable", devices: deviceIds });
   const deactivate = () => updateDASD({ action: "disable", devices: deviceIds });
   const setDiagOn = () => updateDASD({ action: "diagOn", devices: deviceIds });
   const setDiagOff = () => updateDASD({ action: "diagOff", devices: deviceIds });
-  const format = () => {
-    const offline = devices.filter((d) => !d.enabled);
-
-    if (offline.length > 0) {
-      return false;
-    }
-
-    return formatDASD(devices.map((d) => d.id));
-  };
+  const format = () => formatDASD(devices.map((d) => d.id));
 
   const Action = ({ children, ...props }) => (
     <DropdownItem component="button" {...props}>
@@ -110,41 +150,57 @@ const Actions = ({ devices, isDisabled }: { devices: DASDDevice[]; isDisabled: b
   );
 
   return (
-    <Dropdown
-      isOpen={isOpen}
-      onSelect={onSelect}
-      toggle={(toggleRef) => (
-        <MenuToggle ref={toggleRef} variant="primary" isDisabled={isDisabled} onClick={onToggle}>
-          {/* TRANSLATORS: drop down menu label */}
-          {_("Perform an action")}
-        </MenuToggle>
+    <>
+      {requestFormat && offlineDevicesSelected && (
+        <FormatNotPossible devices={offlineDevices} onAccept={cancelFormatRequest} />
+      )}
+
+      {requestFormat && !offlineDevicesSelected && (
+        <FormatConfirmation
+          devices={devices}
+          onCancel={cancelFormatRequest}
+          onConfirm={() => {
+            cancelFormatRequest();
+            format();
+          }}
+        />
       )}
-    >
-      <DropdownList>
-        {/** TRANSLATORS: drop down menu action, activate the device */}
-        <Action key="activate" onClick={activate}>
-          {_("Activate")}
-        </Action>
-        {/** TRANSLATORS: drop down menu action, deactivate the device */}
-        <Action key="deactivate" onClick={deactivate}>
-          {_("Deactivate")}
-        </Action>
-        <Divider key="first-separator" />
-        {/** TRANSLATORS: drop down menu action, enable DIAG access method */}
-        <Action key="set_diag_on" onClick={setDiagOn}>
-          {_("Set DIAG On")}
-        </Action>
-        {/** TRANSLATORS: drop down menu action, disable DIAG access method */}
-        <Action key="set_diag_off" onClick={setDiagOff}>
-          {_("Set DIAG Off")}
-        </Action>
-        <Divider key="second-separator" />
-        {/** TRANSLATORS: drop down menu action, format the disk */}
-        <Action key="format" onClick={format}>
-          {_("Format")}
-        </Action>
-      </DropdownList>
-    </Dropdown>
+      <Dropdown
+        isOpen={isOpen}
+        onSelect={onSelect}
+        toggle={(toggleRef) => (
+          <MenuToggle ref={toggleRef} variant="primary" isDisabled={isDisabled} onClick={onToggle}>
+            {/* TRANSLATORS: drop down menu label */}
+            {_("Perform an action")}
+          </MenuToggle>
+        )}
+      >
+        <DropdownList>
+          {/** TRANSLATORS: drop down menu action, activate the device */}
+          <Action key="activate" onClick={activate}>
+            {_("Activate")}
+          </Action>
+          {/** TRANSLATORS: drop down menu action, deactivate the device */}
+          <Action key="deactivate" onClick={deactivate}>
+            {_("Deactivate")}
+          </Action>
+          <Divider key="first-separator" />
+          {/** TRANSLATORS: drop down menu action, enable DIAG access method */}
+          <Action key="set_diag_on" onClick={setDiagOn}>
+            {_("Set DIAG On")}
+          </Action>
+          {/** TRANSLATORS: drop down menu action, disable DIAG access method */}
+          <Action key="set_diag_off" onClick={setDiagOff}>
+            {_("Set DIAG Off")}
+          </Action>
+          <Divider key="second-separator" />
+          {/** TRANSLATORS: drop down menu action, format the disk */}
+          <Action key="format" onClick={() => setRequestFormat(true)}>
+            {_("Format")}
+          </Action>
+        </DropdownList>
+      </Dropdown>
+    </>
   );
 };
 
@@ -326,7 +382,7 @@ export default function DASDTable() {
         </ToolbarContent>
       </Toolbar>
 
-      <Page.Section>
+      <Page.Section aria-label={_("DASDs table section")}>
         <Content />
       </Page.Section>
     </>