Display all affected devices at the table of results (#1936)

## Problem

The result box at the Storage section displays all the devices affected
by the installation proposal. But for doing that, it relies on the
actions to be done on the disks. That means that a disk is not displayed
there if is not going to be modified. In principle, that's fine but...

There is a corner case if a disk is chosen only for booting and no
action is needed for that (eg. legacy boot with a disk that already
includes the bios_boot partition). It's certainly not going to be
affected, but the disk is displayed at the configuration section and
omitted at the result. That's confusing.

## Solution

Display at the result all disks that are mentioned at the configuration,
even if no actions are needed to prepare the disks.

## Testing

- Tested manually.
- Extended unit tests to cover the new cases.
- Bonus: resurrected tests for `ProposalResultSection` (no longer
skipped).

web/src/components/storage/DevicesManager.js

@@ -138,11 +138,15 @@ export default class DevicesManager {
    * Disk devices and LVM volume groups used for the installation.
    * @method
    *
-   * @note The used devices are extracted from the actions.
+   * @note The used devices are extracted from the actions, but the optional argument
+   * can be used to expand the list if some devices must be included despite not
+   * being affected by the actions.
    *
+   * @param {string[]} knownNames - names of devices already known to be used, even if
+   *    there are no actions on them
    * @returns {StorageDevice[]}
    */
-  usedDevices() {
+  usedDevices(knownNames = []) {
     const isTarget = (device) => device.isDrive || ["md", "lvmVg"].includes(device.type);
 
     // Check in system devices to detect removals.
@@ -151,7 +155,7 @@ export default class DevicesManager {
 
     const sids = targetSystem
       .concat(targetStaging)
-      .filter((d) => this.#isUsed(d))
+      .filter((d) => this.#isUsed(d) || knownNames.includes(d.name))
       .map((d) => d.sid);
 
     return compact(uniq(sids).map((sid) => this.stagingDevice(sid)));

web/src/components/storage/DevicesManager.test.js

@@ -301,24 +301,27 @@ describe("usedDevices", () => {
   beforeEach(() => {
     system = [
       { sid: 60, isDrive: false },
-      { sid: 61, isDrive: true },
-      { sid: 62, isDrive: true, partitionTable: { partitions: [{ sid: 67 }] } },
-      { sid: 63, isDrive: true, partitionTable: { partitions: [] } },
+      { sid: 61, isDrive: true, name: "/dev/sda" },
+      { sid: 62, isDrive: true, name: "/dev/sdb", partitionTable: { partitions: [{ sid: 67 }] } },
+      { sid: 63, isDrive: true, name: "/dev/sdc", partitionTable: { partitions: [] } },
       { sid: 64, isDrive: false, type: "lvmVg", logicalVolumes: [] },
       { sid: 65, isDrive: false, type: "lvmVg", logicalVolumes: [] },
       { sid: 66, isDrive: false, type: "lvmVg", logicalVolumes: [{ sid: 68 }] },
+      { sid: 72, isDrive: true, name: "/dev/sdd" },
     ];
     staging = [
       { sid: 60, isDrive: false },
       // Partition removed
-      { sid: 62, isDrive: true, partitionTable: { partitions: [] } },
+      { sid: 62, isDrive: true, name: "/dev/sdb", partitionTable: { partitions: [] } },
       // Partition added
-      { sid: 63, isDrive: true, partitionTable: { partitions: [{ sid: 69 }] } },
+      { sid: 63, isDrive: true, name: "/dev/sdc", partitionTable: { partitions: [{ sid: 69 }] } },
       { sid: 64, isDrive: false, type: "lvmVg", logicalVolumes: [] },
       // Logical volume added
       { sid: 65, isDrive: false, type: "lvmVg", logicalVolumes: [{ sid: 70 }, { sid: 71 }] },
       // Logical volume removed
       { sid: 66, isDrive: false, type: "lvmVg", logicalVolumes: [] },
+      // Not modified
+      { sid: 72, isDrive: true, name: "/dev/sdd" },
     ];
   });
 
@@ -327,10 +330,22 @@ describe("usedDevices", () => {
       actions = [];
     });
 
-    it("returns an empty list", () => {
+    it("returns an empty list if no argument is passed", () => {
       const manager = new DevicesManager(system, staging, actions);
       expect(manager.usedDevices()).toEqual([]);
     });
+
+    it("returns an empty list if non-existent names are passed", () => {
+      const manager = new DevicesManager(system, staging, actions);
+      expect(manager.usedDevices(["/dev/nodisk"])).toEqual([]);
+    });
+
+    it("returns a list including only the disks passed by argument", () => {
+      const manager = new DevicesManager(system, staging, actions);
+      const devs = manager.usedDevices(["/dev/sdb", "/dev/sdb"]);
+      expect(devs.length).toEqual(1);
+      expect(devs[0].sid).toEqual(62);
+    });
   });
 
   describe("if there are actions", () => {
@@ -370,6 +385,15 @@ describe("usedDevices", () => {
         .sort();
       expect(sids).toEqual([62, 63, 65, 66]);
     });
+
+    it("also includes extra disks passed as argument without repeating redundant ones", () => {
+      const manager = new DevicesManager(system, staging, actions);
+      const sids = manager
+        .usedDevices(["/dev/sdb", "/dev/sdd"])
+        .map((d) => d.sid)
+        .sort();
+      expect(sids).toEqual([62, 63, 65, 66, 72]);
+    });
   });
 });
 

web/src/components/storage/ProposalPage.tsx

@@ -32,13 +32,7 @@ import ConfigEditorMenu from "./ConfigEditorMenu";
 import { toValidationError } from "~/utils";
 import { useIssues } from "~/queries/issues";
 import { IssueSeverity } from "~/types/issues";
-import {
-  useDevices,
-  useDeprecated,
-  useDeprecatedChanges,
-  useActions,
-  useReprobeMutation,
-} from "~/queries/storage";
+import { useDeprecated, useDeprecatedChanges, useReprobeMutation } from "~/queries/storage";
 import { _ } from "~/i18n";
 
 /**
@@ -65,11 +59,8 @@ export const NOT_AFFECTED = {
 };
 
 export default function ProposalPage() {
-  const systemDevices = useDevices("system");
-  const stagingDevices = useDevices("result");
   const isDeprecated = useDeprecated();
   const { mutateAsync: reprobe } = useReprobeMutation();
-  const actions = useActions();
 
   useDeprecatedChanges();
 
@@ -128,13 +119,7 @@ export default function ProposalPage() {
             <EncryptionField password={""} isLoading={false} />
           </GridItem>
           <GridItem sm={12}>
-            <ProposalResultSection
-              system={systemDevices}
-              staging={stagingDevices}
-              actions={actions}
-              errors={errors}
-              isLoading={false}
-            />
+            <ProposalResultSection errors={errors} isLoading={false} />
           </GridItem>
         </Grid>
       </Page.Content>

web/src/components/storage/ProposalResultSection.test.tsx

@@ -31,13 +31,22 @@ import { ProposalResultSectionProps } from "./ProposalResultSection";
 
 const errorMessage = "Something went wrong, proposal not possible";
 const errors = [{ severity: 0, message: errorMessage }];
-const defaultProps: ProposalResultSectionProps = {
-  system: devices.system,
-  staging: devices.staging,
-  actions,
-};
+const defaultProps: ProposalResultSectionProps = {};
+const mockUseActionsFn = jest.fn();
+const mockConfig = { drives: [] };
+
+jest.mock("~/queries/storage", () => ({
+  ...jest.requireActual("~/queries/storage"),
+  useConfigModel: () => mockConfig,
+  useDevices: () => devices.staging,
+  useActions: () => mockUseActionsFn(),
+}));
+
+describe("ProposalResultSection", () => {
+  beforeEach(() => {
+    mockUseActionsFn.mockReturnValue(actions);
+  });
 
-describe.skip("ProposalResultSection", () => {
   describe("when there are errors (proposal was not possible)", () => {
     it("renders given errors", () => {
       plainRender(<ProposalResultSection {...defaultProps} errors={errors} />);
@@ -61,35 +70,36 @@ describe.skip("ProposalResultSection", () => {
   });
 
   describe("when there are no errors (proposal was possible)", () => {
-    it("does not render a warning when there are not delete actions", () => {
-      const props = {
-        ...defaultProps,
-        actions: defaultProps.actions.filter((a) => !a.delete),
-      };
-
-      plainRender(<ProposalResultSection {...props} />);
-      expect(screen.queryByText(/Warning alert:/)).toBeNull();
+    describe("and there are no delete actions", () => {
+      beforeEach(() => {
+        mockUseActionsFn.mockReturnValue(actions.filter((a) => !a.delete));
+      });
+
+      it("does not render a warning when there are not delete actions", () => {
+        plainRender(<ProposalResultSection {...defaultProps} />);
+        expect(screen.queryByText(/Warning alert:/)).toBeNull();
+      });
     });
 
-    it("renders a reminder when there are delete actions", () => {
-      plainRender(<ProposalResultSection {...defaultProps} />);
-      const reminder = screen.getByRole("status");
-      within(reminder).getByText(/4 destructive/);
+    describe("and there are delete actions affecting a previous system", () => {
+      beforeEach(() => {
+        // NOTE: simulate the deletion of vdc2 (sid: 79) for checking that
+        // affected systems are rendered in the warning summary
+        mockUseActionsFn.mockReturnValue([
+          { device: 79, subvol: false, delete: true, resize: false, text: "" },
+        ]);
+      });
+
+      it("renders the affected systems in the deletion reminder, if any", () => {
+        plainRender(<ProposalResultSection {...defaultProps} />);
+        expect(screen.queryByText(/affecting openSUSE/)).toBeInTheDocument();
+      });
     });
 
-    it("renders the affected systems in the deletion reminder, if any", () => {
-      // NOTE: simulate the deletion of vdc2 (sid: 79) for checking that
-      // affected systems are rendered in the warning summary
-      const props = {
-        ...defaultProps,
-        actions: [{ device: 79, subvol: false, delete: true, resize: false, text: "" }],
-      };
-
-      plainRender(<ProposalResultSection {...props} />);
-      // FIXME: below line reveals that warning wrapper deserves a role or
-      // something
-      const reminder = screen.getByRole("status");
-      within(reminder).getByText(/openSUSE/);
+    it("renders a reminder about the delete actions", () => {
+      plainRender(<ProposalResultSection {...defaultProps} />);
+      expect(screen.queryByText(/Warning alert:/)).toBeInTheDocument();
+      expect(screen.queryByText(/4 destructive/)).toBeInTheDocument();
     });
 
     it("renders a treegrid including all relevant information about final result", () => {
@@ -99,8 +109,8 @@ describe.skip("ProposalResultSection", () => {
        * Expected rows for full-result-example
        * --------------------------------------------------
        * "/dev/vdc Disk GPT 30 GiB"
-       * "vdc1 New BIOS Boot Partition 8 MiB"
-       * "vdc3 swap New Swap Partition 1.5 GiB"
+       * "vdc1 BIOS Boot Partition 8 MiB"
+       * "vdc3 swap Swap Partition 1.5 GiB"
        * "Unused space 3.49 GiB"
        * "vdc2 openSUSE Leap 15.2, Fedora 10.30 5 GiB"
        * "Unused space 1 GiB"
@@ -110,23 +120,23 @@ describe.skip("ProposalResultSection", () => {
        * Device      Mount point      Details                                 Size
        * -------------------------------------------------------------------------
        * /dev/vdc                     Disk GPT                              30 GiB
-       *     vdc1                 New BIOS Boot Partition                    8 MiB
-       *     vdc3    swap         New Swap Partition                       1.5 GiB
+       *     vdc1                     BIOS Boot Partition                    8 MiB
+       *     vdc3    swap             Swap Partition                       1.5 GiB
        *                              Unused space                        3.49 GiB
        *     vdc2                     openSUSE Leap 15.2, Fedora 10.30       5 GiB
        *                              Unused space                           1 GiB
-       *     vdc4                     Linux                   Before 2 GiB 1.5 GiB
-       *     vdc5    /            New Btrfs Partition                     17.5 GiB
+       *     vdc4                     Linux                                1.5 GiB
+       *     vdc5    /                Btrfs Partition                     17.5 GiB
        * -------------------------------------------------------------------------
        */
       within(treegrid).getByRole("row", { name: "/dev/vdc Disk GPT 30 GiB" });
-      within(treegrid).getByRole("row", { name: "vdc1 New BIOS Boot Partition 8 MiB" });
-      within(treegrid).getByRole("row", { name: "vdc3 swap New Swap Partition 1.5 GiB" });
+      within(treegrid).getByRole("row", { name: "vdc1 BIOS Boot Partition 8 MiB" });
+      within(treegrid).getByRole("row", { name: "vdc3 swap Swap Partition 1.5 GiB" });
       within(treegrid).getByRole("row", { name: "Unused space 3.49 GiB" });
       within(treegrid).getByRole("row", { name: "vdc2 openSUSE Leap 15.2, Fedora 10.30 5 GiB" });
       within(treegrid).getByRole("row", { name: "Unused space 1 GiB" });
-      within(treegrid).getByRole("row", { name: "vdc4 Linux Before 2 GiB 1.5 GiB" });
-      within(treegrid).getByRole("row", { name: "vdc5 / New Btrfs Partition 17.5 GiB" });
+      within(treegrid).getByRole("row", { name: "vdc4 Linux 1.5 GiB" });
+      within(treegrid).getByRole("row", { name: "vdc5 / Btrfs Partition 17.5 GiB" });
     });
 
     it("renders a button for opening the planned actions dialog", async () => {
@@ -135,7 +145,7 @@ describe.skip("ProposalResultSection", () => {
 
       await user.click(button);
 
-      screen.getByRole("dialog", { name: "Planned Actions" });
+      screen.getByRole("button", { name: "Collapse the list of planned actions" });
     });
   });
 });

web/src/components/storage/ProposalResultSection.tsx

@@ -27,8 +27,8 @@ import DevicesManager from "~/components/storage/DevicesManager";
 import ProposalResultTable from "~/components/storage/ProposalResultTable";
 import { ProposalActionsDialog } from "~/components/storage";
 import { _, n_, formatList } from "~/i18n";
-import { Action, StorageDevice } from "~/types/storage";
 import { ValidationError } from "~/types/issues";
+import { useActions, useDevices } from "~/queries/storage";
 import { sprintf } from "sprintf-js";
 
 /**
@@ -111,20 +111,17 @@ function ActionsList({ manager }: ActionsListProps) {
 }
 
 export type ProposalResultSectionProps = {
-  system?: StorageDevice[];
-  staging?: StorageDevice[];
-  actions?: Action[];
   errors?: ValidationError[];
   isLoading?: boolean;
 };
 
 export default function ProposalResultSection({
-  system = [],
-  staging = [],
-  actions = [],
   errors = [],
   isLoading = false,
 }: ProposalResultSectionProps) {
+  const system = useDevices("system", { suspense: true });
+  const staging = useDevices("result", { suspense: true });
+  const actions = useActions();
   const devicesManager = new DevicesManager(system, staging, actions);
 
   return (

web/src/components/storage/ProposalResultTable.tsx

@@ -36,6 +36,7 @@ import { deviceChildren, deviceSize } from "~/components/storage/utils";
 import { PartitionSlot, StorageDevice } from "~/types/storage";
 import { TreeTableColumn } from "~/components/core/TreeTable";
 import { DeviceInfo } from "~/api/storage/types";
+import { useConfigModel } from "~/queries/storage";
 
 type TableItem = StorageDevice | PartitionSlot;
 
@@ -144,7 +145,8 @@ type ProposalResultTableProps = {
  * @component
  */
 export default function ProposalResultTable({ devicesManager }: ProposalResultTableProps) {
-  const devices = devicesManager.usedDevices();
+  const model = useConfigModel({ suspense: true });
+  const devices = devicesManager.usedDevices(model.drives.map((d) => d.name));
 
   return (
     <TreeTable