From e1cf3a0e06115104f5fd08cf5344c7e980777f6d Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 22 Jan 2025 17:03:25 +0000 Subject: [PATCH 1/2] web: Tiny reorganization in the usage of queries --- web/src/components/storage/ProposalPage.tsx | 19 +--- .../storage/ProposalResultSection.test.tsx | 92 ++++++++++--------- .../storage/ProposalResultSection.tsx | 11 +-- 3 files changed, 56 insertions(+), 66 deletions(-) diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index 8271a37681..adc6af67a4 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/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() { - + diff --git a/web/src/components/storage/ProposalResultSection.test.tsx b/web/src/components/storage/ProposalResultSection.test.tsx index 660030a0cf..63787b0b63 100644 --- a/web/src/components/storage/ProposalResultSection.test.tsx +++ b/web/src/components/storage/ProposalResultSection.test.tsx @@ -31,13 +31,20 @@ 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(); + +jest.mock("~/queries/storage", () => ({ + ...jest.requireActual("~/queries/storage"), + 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(); @@ -61,35 +68,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(); - 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(); + expect(screen.queryByText(/Warning alert:/)).toBeNull(); + }); }); - it("renders a reminder when there are delete actions", () => { - plainRender(); - 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(); + 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(); - // 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(); + 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 +107,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 +118,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 +143,7 @@ describe.skip("ProposalResultSection", () => { await user.click(button); - screen.getByRole("dialog", { name: "Planned Actions" }); + screen.getByRole("button", { name: "Collapse the list of planned actions" }); }); }); }); diff --git a/web/src/components/storage/ProposalResultSection.tsx b/web/src/components/storage/ProposalResultSection.tsx index 4b6c0470da..ac4f0a48a4 100644 --- a/web/src/components/storage/ProposalResultSection.tsx +++ b/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 ( From 682cb105dcedc32a18c0201096964f8da36dcf53 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 22 Jan 2025 17:05:47 +0000 Subject: [PATCH 2/2] web: Include all configured disks at the result --- web/src/components/storage/DevicesManager.js | 10 ++++-- .../components/storage/DevicesManager.test.js | 36 +++++++++++++++---- .../storage/ProposalResultSection.test.tsx | 2 ++ .../storage/ProposalResultTable.tsx | 4 ++- 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/web/src/components/storage/DevicesManager.js b/web/src/components/storage/DevicesManager.js index cd900962c2..d5e9624ce0 100644 --- a/web/src/components/storage/DevicesManager.js +++ b/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))); diff --git a/web/src/components/storage/DevicesManager.test.js b/web/src/components/storage/DevicesManager.test.js index 378e88b1f9..095de0b14a 100644 --- a/web/src/components/storage/DevicesManager.test.js +++ b/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]); + }); }); }); diff --git a/web/src/components/storage/ProposalResultSection.test.tsx b/web/src/components/storage/ProposalResultSection.test.tsx index 63787b0b63..672ab63fdb 100644 --- a/web/src/components/storage/ProposalResultSection.test.tsx +++ b/web/src/components/storage/ProposalResultSection.test.tsx @@ -33,9 +33,11 @@ const errorMessage = "Something went wrong, proposal not possible"; const errors = [{ severity: 0, message: errorMessage }]; 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(), })); diff --git a/web/src/components/storage/ProposalResultTable.tsx b/web/src/components/storage/ProposalResultTable.tsx index 71a95c9ee2..2604284a0b 100644 --- a/web/src/components/storage/ProposalResultTable.tsx +++ b/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 (