Storage UI: Delete Partition (in proposal; trash-can icon) (#1915)

## Problem Making the trash-can icon work - https://trello.com/c/vPPaRAIg ![deleteme_C3_A7](https://github.com/user-attachments/assets/4b46ab58-231a-480e-8430-4c47756ab196) ## Solution - [x] delete the item from the list (via configModel.Config) - [ ] ~ask for confirmation~ - [ ] ~check that the partition is not mandatory~ ... these two are postponed to a followup PBI ## Testing - Tested manually - Added a unit test for the UI part (thanks David!) ## Screenshots No. The deleted partition is not visible 🤣 and a video would be overkill.
agama-project · Jan 23, 2025 · 8d0d533 · 8d0d533
2 parents 31635d4 + 50f677d
commit 8d0d533
Show file tree

Hide file tree

Showing 4 changed files with 207 additions and 35 deletions.
diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes
@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Thu Jan 23 15:09:09 UTC 2025 - Martin Vidner <[email protected]>
+
+- Make the trash can icon (Delete) of a proposed partition work
+  gh#agama-project/agama#1915)
+
 -------------------------------------------------------------------
 Mon Jan 20 16:45:18 UTC 2025 - Ladislav Slezák <[email protected]>
 

diff --git a/web/src/components/storage/DriveEditor.test.tsx b/web/src/components/storage/DriveEditor.test.tsx
@@ -0,0 +1,93 @@
+/*
+ * Copyright (c) [2025] SUSE LLC
+ *
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, contact SUSE LLC.
+ *
+ * To contact SUSE LLC about this file by physical or electronic mail, you may
+ * find current contact information at www.suse.com.
+ */
+
+import React from "react";
+import { screen, within } from "@testing-library/react";
+import { plainRender } from "~/test-utils";
+import DriveEditor, { DriveEditorProps } from "~/components/storage/DriveEditor";
+import * as ConfigModel from "~/api/storage/types/config-model";
+import { StorageDevice } from "~/types/storage";
+
+const sda: StorageDevice = {
+  sid: 59,
+  isDrive: true,
+  type: "disk",
+  vendor: "Micron",
+  model: "Micron 1100 SATA",
+  driver: ["ahci", "mmcblk"],
+  bus: "IDE",
+  busId: "",
+  transport: "usb",
+  dellBOSS: false,
+  sdCard: true,
+  active: true,
+  name: "/dev/sda",
+  size: 1024,
+  shrinking: { unsupported: ["Resizing is not supported"] },
+  systems: [],
+  udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"],
+  udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"],
+  description: "",
+};
+
+const mockDrive: ConfigModel.Drive = {
+  name: "/dev/sda",
+  spacePolicy: "delete",
+  partitions: [
+    {
+      mountPath: "swap",
+      size: {
+        min: 2_000_000_000,
+        default: false, // false: user provided, true: calculated
+      },
+    },
+  ],
+};
+
+const mockDeletePartition = jest.fn();
+// TODO: why does "~/queries/storage" work elsewhere??
+jest.mock("~/queries/storage/config-model", () => ({
+  ...jest.requireActual("~/queries/storage/config-model"),
+  useConfigModel: () => ({ drives: [mockDrive] }),
+  useDrive: () => mockDrive,
+  usePartition: () => ({ delete: mockDeletePartition }),
+}));
+
+const props: DriveEditorProps = {
+  drive: mockDrive,
+  driveDevice: sda,
+};
+
+describe("PartitionMenuItem", () => {
+  it("allows users delete a the partition", async () => {
+    const { user } = plainRender(<DriveEditor {...props} />);
+
+    const partitionsButton = screen.getByRole("button", { name: "Partitions" });
+    await user.click(partitionsButton);
+    const partitionsMenu = screen.getByRole("menu");
+    const deleteSwapButton = within(partitionsMenu).getByRole("menuitem", {
+      name: "Delete swap",
+    });
+    await user.click(deleteSwapButton);
+    expect(mockDeletePartition).toHaveBeenCalled();
+  });
+});
diff --git a/web/src/components/storage/DriveEditor.tsx b/web/src/components/storage/DriveEditor.tsx
@@ -20,7 +20,7 @@
  * find current contact information at www.suse.com.
  */
 
-import React, { useRef, useState } from "react";
+import React, { useId, useRef, useState } from "react";
 import { useNavigate, generatePath } from "react-router-dom";
 import { _, formatList } from "~/i18n";
 import { sprintf } from "sprintf-js";
@@ -29,7 +29,7 @@ import { useAvailableDevices } from "~/queries/storage";
 import { configModel } from "~/api/storage/types";
 import { StorageDevice } from "~/types/storage";
 import { STORAGE as PATHS } from "~/routes/paths";
-import { useDrive } from "~/queries/storage/config-model";
+import { useDrive, usePartition } from "~/queries/storage/config-model";
 import * as driveUtils from "~/components/storage/utils/drive";
 import { typeDescription, contentDescription } from "~/components/storage/utils/device";
 import { Icon } from "../layout";
@@ -58,7 +58,7 @@ import {
 
 import spacingStyles from "@patternfly/react-styles/css/utilities/Spacing/spacing";
 
-type DriveEditorProps = { drive: configModel.Drive; driveDevice: StorageDevice };
+export type DriveEditorProps = { drive: configModel.Drive; driveDevice: StorageDevice };
 
 export const InlineMenuToggle = React.forwardRef(
   (props: MenuToggleProps, ref: React.Ref<MenuToggleElement>) => (
@@ -506,7 +506,8 @@ const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => {
   );
 };
 
-const PartitionsNoContentSelector = () => {
+const PartitionsNoContentSelector = ({ toggleAriaLabel }) => {
+  const menuId = useId();
   const menuRef = useRef();
   const toggleMenuRef = useRef();
   const [isOpen, setIsOpen] = useState(false);
@@ -518,19 +519,26 @@ const PartitionsNoContentSelector = () => {
       onOpenChange={setIsOpen}
       toggleRef={toggleMenuRef}
       toggle={
-        <InlineMenuToggle ref={toggleMenuRef} onClick={onToggle} isExpanded={isOpen}>
-          <span>{_("No additional partitions will be created")}</span>
+        <InlineMenuToggle
+          ref={toggleMenuRef}
+          onClick={onToggle}
+          isExpanded={isOpen}
+          aria-label={toggleAriaLabel}
+          aria-controls={menuId}
+        >
+          <span aria-hidden>{_("No additional partitions will be created")}</span>
         </InlineMenuToggle>
       }
       menuRef={menuRef}
       menu={
-        <Menu ref={menuRef}>
+        <Menu ref={menuRef} role="menu" id={menuId}>
           <MenuContent>
             <MenuList>
               <MenuItem
                 key="add-partition"
                 itemId="add-partition"
                 description={_("Add another partition or mount an existing one")}
+                role="menuitem"
               >
                 <Flex component="span" justifyContent={{ default: "justifyContentSpaceBetween" }}>
                   <span>{_("Add or use partition")}</span>
@@ -544,7 +552,39 @@ const PartitionsNoContentSelector = () => {
   );
 };
 
-const PartitionsWithContentSelector = ({ drive }) => {
+const PartitionMenuItem = ({ driveName, mountPath }) => {
+  const { delete: deletePartition } = usePartition(driveName, mountPath);
+
+  return (
+    <MenuItem
+      itemId={mountPath}
+      description="Btrfs with snapshots"
+      role="menuitem"
+      actions={
+        <>
+          <MenuItemAction
+            style={{ paddingInline: "4px", alignSelf: "center" }}
+            icon={<Icon name="edit_square" size="xs" aria-label={"Edit"} />}
+            actionId={`edit-${mountPath}`}
+            aria-label={`Edit ${mountPath}`}
+          />
+          <MenuItemAction
+            style={{ paddingInline: "4px", alignSelf: "center" }}
+            icon={<Icon name="delete" size="xs" aria-label={"Delete"} />}
+            actionId={`delete-${mountPath}`}
+            aria-label={`Delete ${mountPath}`}
+            onClick={deletePartition}
+          />
+        </>
+      }
+    >
+      {mountPath}
+    </MenuItem>
+  );
+};
+
+const PartitionsWithContentSelector = ({ drive, toggleAriaLabel }) => {
+  const menuId = useId();
   const menuRef = useRef();
   const toggleMenuRef = useRef();
   const [isOpen, setIsOpen] = useState(false);
@@ -556,42 +596,30 @@ const PartitionsWithContentSelector = ({ drive }) => {
       onOpenChange={setIsOpen}
       toggleRef={toggleMenuRef}
       toggle={
-        <InlineMenuToggle ref={toggleMenuRef} onClick={onToggle} isExpanded={isOpen}>
-          <span>{driveUtils.contentDescription(drive)}</span>
+        <InlineMenuToggle
+          ref={toggleMenuRef}
+          onClick={onToggle}
+          isExpanded={isOpen}
+          aria-label={toggleAriaLabel}
+          aria-controls={menuId}
+        >
+          <span aria-hidden>{driveUtils.contentDescription(drive)}</span>
         </InlineMenuToggle>
       }
       menuRef={menuRef}
       menu={
-        <Menu ref={menuRef}>
+        <Menu ref={menuRef} id={menuId} role="menu">
           <MenuContent>
             <MenuList>
               {drive.partitions
                 .filter((p) => p.mountPath)
                 .map((partition) => {
                   return (
-                    <MenuItem
+                    <PartitionMenuItem
                       key={partition.mountPath}
-                      itemId={partition.mountPath}
-                      description="Btrfs with snapshots"
-                      actions={
-                        <>
-                          <MenuItemAction
-                            style={{ paddingInline: "4px", alignSelf: "center" }}
-                            icon={<Icon name="edit_square" size="xs" aria-label={"Edit"} />}
-                            actionId={`edit-${partition.mountPath}`}
-                            aria-label={`Edit ${partition.mountPath}`}
-                          />
-                          <MenuItemAction
-                            style={{ paddingInline: "4px", alignSelf: "center" }}
-                            icon={<Icon name="delete" size="xs" aria-label={"Edit"} />}
-                            actionId={`delete-${partition.mountPath}`}
-                            aria-label={`Delete ${partition.mountPath}`}
-                          />
-                        </>
-                      }
-                    >
-                      {partition.mountPath}
-                    </MenuItem>
+                      driveName={drive.name}
+                      mountPath={partition.mountPath}
+                    />
                   );
                 })}
               <Divider component="li" />
@@ -614,10 +642,10 @@ const PartitionsWithContentSelector = ({ drive }) => {
 
 const PartitionsSelector = ({ drive }) => {
   if (drive.partitions.some((p) => p.mountPath)) {
-    return <PartitionsWithContentSelector drive={drive} />;
+    return <PartitionsWithContentSelector drive={drive} toggleAriaLabel={_("Partitions")} />;
   }
 
-  return <PartitionsNoContentSelector />;
+  return <PartitionsNoContentSelector toggleAriaLabel={_("Partitions")} />;
 };
 
 export default function DriveEditor({ drive, driveDevice }: DriveEditorProps) {

diff --git a/web/src/queries/storage/config-model.ts b/web/src/queries/storage/config-model.ts
@@ -62,6 +62,18 @@ function isUsedDrive(model: configModel.Config, driveName: string) {
   return drive.partitions?.some((p) => isNewPartition(p) || isReusedPartition(p));
 }
 
+function findPartition(
+  model: configModel.Config,
+  driveName: string,
+  mountPath: string,
+): configModel.Partition | undefined {
+  const drive = findDrive(model, driveName);
+  if (drive === undefined) return undefined;
+
+  const partitions = drive.partitions || [];
+  return partitions.find((p) => p.mountPath === mountPath);
+}
+
 function isBoot(model: configModel.Config, driveName: string): boolean {
   return model.boot?.configure && driveName === model.boot?.device?.name;
 }
@@ -104,6 +116,20 @@ function disableBoot(originalModel: configModel.Config): configModel.Config {
   return setBoot(originalModel, { configure: false });
 }
 
+function deletePartition(
+  originalModel: configModel.Config,
+  driveName: string,
+  mountPath: string,
+): configModel.Config {
+  const model = copyModel(originalModel);
+  const drive = findDrive(model, driveName);
+  if (drive === undefined) return;
+
+  const partitions = (drive.partitions || []).filter((p) => p.mountPath !== mountPath);
+  drive.partitions = partitions;
+  return model;
+}
+
 function switchDrive(
   originalModel: configModel.Config,
   driveName: string,
@@ -252,6 +278,25 @@ export function useBoot(): BootHook {
   };
 }
 
+export type PartitionHook = {
+  delete: () => void;
+};
+
+/**
+ * @param driveName like "/dev/sda"
+ * @param mountPath like "/" or "swap"
+ */
+export function usePartition(driveName: string, mountPath: string): PartitionHook | undefined {
+  const model = useConfigModel();
+  const { mutate } = useConfigModelMutation();
+
+  if (findPartition(model, driveName, mountPath) === undefined) return;
+
+  return {
+    delete: () => mutate(deletePartition(model, driveName, mountPath)),
+  };
+}
+
 export type DriveHook = {
   isBoot: boolean;
   isExplicitBoot: boolean;