From e5a510f92a3c9eacd99eb98f5fa854354808de64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 18 Dec 2024 10:33:18 +0000 Subject: [PATCH] feat(web): allow changing boot options --- .../components/storage/BootConfigField.tsx | 2 +- .../components/storage/BootSelection.test.tsx | 149 ++++++++---------- web/src/components/storage/BootSelection.tsx | 64 ++++---- .../components/storage/ConfigEditorMenu.tsx | 7 +- web/src/routes/paths.ts | 2 +- web/src/routes/storage.tsx | 2 +- 6 files changed, 110 insertions(+), 116 deletions(-) diff --git a/web/src/components/storage/BootConfigField.tsx b/web/src/components/storage/BootConfigField.tsx index e9398a9379..b275592d55 100644 --- a/web/src/components/storage/BootConfigField.tsx +++ b/web/src/components/storage/BootConfigField.tsx @@ -39,7 +39,7 @@ import { STORAGE as PATHS } from "~/routes/paths"; const Link = ({ isBold = false }: { isBold?: boolean }) => { const text = _("Change boot options"); - return {isBold ? {text} : text}; + return {isBold ? {text} : text}; }; export type BootConfig = { diff --git a/web/src/components/storage/BootSelection.test.tsx b/web/src/components/storage/BootSelection.test.tsx index 354c4571a1..f2d8450770 100644 --- a/web/src/components/storage/BootSelection.test.tsx +++ b/web/src/components/storage/BootSelection.test.tsx @@ -24,9 +24,10 @@ import React from "react"; import { screen, within } from "@testing-library/react"; -import { plainRender } from "~/test-utils"; +import { mockNavigateFn, plainRender } from "~/test-utils"; import BootSelection from "./BootSelection"; import { StorageDevice } from "~/types/storage"; +import { BootHook } from "~/queries/storage/config-model"; const sda: StorageDevice = { sid: 59, @@ -94,50 +95,62 @@ const sdc: StorageDevice = { udevPaths: ["pci-0000:00-19"], }; -let props; - -describe.skip("BootSelection", () => { - beforeEach(() => { - props = { - isOpen: true, - configureBoot: false, - availableDevices: [sda, sdb, sdc], - bootDevice: undefined, - defaultBootDevice: undefined, - onCancel: jest.fn(), - onAccept: jest.fn(), - }; - }); +const mockAvailableDevices = [sda, sdb, sdc]; + +const mockBoot: BootHook = { + configure: false, + isDefault: false, + deviceName: undefined, + setDevice: jest.fn(), + setDefault: jest.fn(), + disable: jest.fn(), +}; + +jest.mock("react-router-dom", () => ({ + ...jest.requireActual("react-router-dom"), + useNavigate: () => mockNavigateFn, +})); + +jest.mock("~/queries/storage", () => ({ + ...jest.requireActual("~/queries/storage"), + useAvailableDevices: () => mockAvailableDevices, +})); +jest.mock("~/queries/storage/config-model", () => ({ + ...jest.requireActual("~/queries/storage/config-model"), + useBoot: () => mockBoot, +})); + +describe("BootSelection", () => { const automaticOption = () => screen.queryByRole("radio", { name: "Automatic" }); const selectDiskOption = () => screen.queryByRole("radio", { name: "Select a disk" }); const notConfigureOption = () => screen.queryByRole("radio", { name: "Do not configure" }); const diskSelector = () => screen.queryByRole("combobox", { name: /choose a disk/i }); it("offers an option to configure boot in the installation disk", () => { - plainRender(); + plainRender(); expect(automaticOption()).toBeInTheDocument(); }); it("offers an option to configure boot in a selected disk", () => { - plainRender(); + plainRender(); expect(selectDiskOption()).toBeInTheDocument(); expect(diskSelector()).toBeInTheDocument(); }); it("offers an option to not configure boot", () => { - plainRender(); + plainRender(); expect(notConfigureOption()).toBeInTheDocument(); }); describe("if the current value is set to boot from the installation disk", () => { beforeEach(() => { - props.configureBoot = true; - props.bootDevice = undefined; + mockBoot.configure = true; + mockBoot.isDefault = true; }); it("selects 'Automatic' option by default", () => { - plainRender(); + plainRender(); expect(automaticOption()).toBeChecked(); expect(selectDiskOption()).not.toBeChecked(); expect(diskSelector()).toBeDisabled(); @@ -147,12 +160,13 @@ describe.skip("BootSelection", () => { describe("if the current value is set to boot from a selected disk", () => { beforeEach(() => { - props.configureBoot = true; - props.bootDevice = sdb; + mockBoot.configure = true; + mockBoot.isDefault = false; + mockBoot.deviceName = sda.name; }); it("selects 'Select a disk' option by default", () => { - plainRender(); + plainRender(); expect(automaticOption()).not.toBeChecked(); expect(selectDiskOption()).toBeChecked(); expect(diskSelector()).toBeEnabled(); @@ -162,12 +176,11 @@ describe.skip("BootSelection", () => { describe("if the current value is set to not configure boot", () => { beforeEach(() => { - props.configureBoot = false; - props.bootDevice = sdb; + mockBoot.configure = false; }); it("selects 'Do not configure' option by default", () => { - plainRender(); + plainRender(); expect(automaticOption()).not.toBeChecked(); expect(selectDiskOption()).not.toBeChecked(); expect(diskSelector()).toBeDisabled(); @@ -175,78 +188,48 @@ describe.skip("BootSelection", () => { }); }); - it("does not call onAccept on cancel", async () => { - const { user } = plainRender(); + it("does not change the boot options on cancel", async () => { + const { user } = plainRender(); const cancel = screen.getByRole("button", { name: "Cancel" }); await user.click(cancel); - expect(props.onAccept).not.toHaveBeenCalled(); + expect(mockBoot.setDevice).not.toHaveBeenCalled(); + expect(mockBoot.setDefault).not.toHaveBeenCalled(); + expect(mockBoot.disable).not.toHaveBeenCalled(); }); - describe("if the 'Automatic' option is selected", () => { - beforeEach(() => { - props.configureBoot = false; - props.bootDevice = undefined; - }); - - it("calls onAccept with the selected options on accept", async () => { - const { user } = plainRender(); - - await user.click(automaticOption()); + it("applies the expected boot options when 'Automatic' is selected", async () => { + const { user } = plainRender(); + await user.click(automaticOption()); - const accept = screen.getByRole("button", { name: "Confirm" }); - await user.click(accept); + const accept = screen.getByRole("button", { name: "Accept" }); + await user.click(accept); - expect(props.onAccept).toHaveBeenCalledWith({ - configureBoot: true, - bootDevice: undefined, - }); - }); + expect(mockBoot.setDefault).toHaveBeenCalled(); }); - describe("if the 'Select a disk' option is selected", () => { - beforeEach(() => { - props.configureBoot = false; - props.bootDevice = undefined; - }); + it("applies the expected boot options when a disk is selected", async () => { + const { user } = plainRender(); - it("calls onAccept with the selected options on accept", async () => { - const { user } = plainRender(); + await user.click(selectDiskOption()); + const selector = diskSelector(); + const sdbOption = within(selector).getByRole("option", { name: /sdb/ }); + await user.selectOptions(selector, sdbOption); - await user.click(selectDiskOption()); - const selector = diskSelector(); - const sdbOption = within(selector).getByRole("option", { name: /sdb/ }); - await user.selectOptions(selector, sdbOption); + const accept = screen.getByRole("button", { name: "Accept" }); + await user.click(accept); - const accept = screen.getByRole("button", { name: "Confirm" }); - await user.click(accept); - - expect(props.onAccept).toHaveBeenCalledWith({ - configureBoot: true, - bootDevice: sdb, - }); - }); + expect(mockBoot.setDevice).toHaveBeenCalledWith(sdb.name); }); - describe("if the 'Do not configure' option is selected", () => { - beforeEach(() => { - props.configureBoot = true; - props.bootDevice = sda; - }); - - it("calls onAccept with the selected options on accept", async () => { - const { user } = plainRender(); + it("applies the expected boot options when 'No configure' is selected", async () => { + const { user } = plainRender(); + await user.click(notConfigureOption()); - await user.click(notConfigureOption()); + const accept = screen.getByRole("button", { name: "Accept" }); + await user.click(accept); - const accept = screen.getByRole("button", { name: "Confirm" }); - await user.click(accept); - - expect(props.onAccept).toHaveBeenCalledWith({ - configureBoot: false, - bootDevice: undefined, - }); - }); + expect(mockBoot.disable).toHaveBeenCalled(); }); }); diff --git a/web/src/components/storage/BootSelection.tsx b/web/src/components/storage/BootSelection.tsx index 27fc49fa29..5f4b56a8d6 100644 --- a/web/src/components/storage/BootSelection.tsx +++ b/web/src/components/storage/BootSelection.tsx @@ -27,10 +27,11 @@ import { DevicesFormSelect } from "~/components/storage"; import { Page } from "~/components/core"; import { deviceLabel } from "~/components/storage/utils"; import { StorageDevice } from "~/types/storage"; -import { useAvailableDevices, useProposalMutation, useProposalResult } from "~/queries/storage"; +import { useAvailableDevices } from "~/queries/storage"; import textStyles from "@patternfly/react-styles/css/utilities/Text/text"; import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; +import { useBoot } from "~/queries/storage/config-model"; // FIXME: improve classNames // FIXME: improve and rename to BootSelectionDialog @@ -39,50 +40,49 @@ const BOOT_AUTO_ID = "boot-auto"; const BOOT_MANUAL_ID = "boot-manual"; const BOOT_DISABLED_ID = "boot-disabled"; +type BootSelectionState = { + load: boolean; + selectedOption?: string; + configureBoot?: boolean; + bootDevice?: StorageDevice; + defaultBootDevice?: StorageDevice; + availableDevices?: StorageDevice[]; +}; + /** * Allows the user to select the boot configuration. */ export default function BootSelectionDialog() { - type BootSelectionState = { - load: boolean; - selectedOption?: string; - configureBoot?: boolean; - bootDevice?: StorageDevice; - defaultBootDevice?: StorageDevice; - availableDevices?: StorageDevice[]; - }; - const [state, setState] = useState({ load: false }); - const { settings } = useProposalResult(); const availableDevices = useAvailableDevices(); - const updateProposal = useProposalMutation(); const navigate = useNavigate(); + const boot = useBoot(); useEffect(() => { if (state.load) return; let selectedOption: string; - const { bootDevice, configureBoot, defaultBootDevice } = settings; - if (!configureBoot) { + if (!boot.configure) { selectedOption = BOOT_DISABLED_ID; - } else if (configureBoot && bootDevice === "") { + } else if (boot.isDefault) { selectedOption = BOOT_AUTO_ID; } else { selectedOption = BOOT_MANUAL_ID; } - const findDevice = (name: string) => availableDevices.find((d) => d.name === name); + const bootDevice = availableDevices.find((d) => d.name === boot.deviceName); + const defaultBootDevice = boot.isDefault ? bootDevice : undefined; setState({ load: true, - bootDevice: findDevice(bootDevice) || findDevice(defaultBootDevice) || availableDevices[0], - configureBoot, - defaultBootDevice: findDevice(defaultBootDevice), + bootDevice: bootDevice || availableDevices[0], + configureBoot: boot.configure, + defaultBootDevice, availableDevices, selectedOption, }); - }, [availableDevices, settings, state.load]); + }, [availableDevices, boot, state.load]); if (!state.load) return; @@ -92,12 +92,18 @@ export default function BootSelectionDialog() { // const formData = new FormData(e.target); // const mode = formData.get("bootMode"); // const device = formData.get("bootDevice"); - const newSettings = { - configureBoot: state.selectedOption !== BOOT_DISABLED_ID, - bootDevice: state.selectedOption === BOOT_MANUAL_ID ? state.bootDevice.name : undefined, - }; - await updateProposal.mutateAsync({ ...settings, ...newSettings }); + switch (state.selectedOption) { + case BOOT_DISABLED_ID: + boot.disable(); + break; + case BOOT_AUTO_ID: + boot.setDefault(); + break; + default: + boot.setDevice(state.bootDevice?.name); + } + navigate(".."); }; @@ -126,20 +132,20 @@ partitions in the appropriate disk.", setState({ ...state, selectedOption: e.target.value }); }; - const setBootDevice = (v) => { + const changeBootDevice = (v) => { setState({ ...state, bootDevice: v }); }; return ( -

{_("Select booting partition")}

+

{_("Boot options")}

{description}
- + diff --git a/web/src/components/storage/ConfigEditorMenu.tsx b/web/src/components/storage/ConfigEditorMenu.tsx index 4c9e95bd12..b8b5714f45 100644 --- a/web/src/components/storage/ConfigEditorMenu.tsx +++ b/web/src/components/storage/ConfigEditorMenu.tsx @@ -21,6 +21,7 @@ */ import React, { useState } from "react"; +import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { useHref } from "react-router-dom"; import { @@ -31,8 +32,10 @@ import { DropdownItem, Divider, } from "@patternfly/react-core"; +import { STORAGE as PATHS } from "~/routes/paths"; export default function ConfigEditorMenu() { + const navigate = useNavigate(); const [isOpen, setIsOpen] = useState(false); const toggle = () => setIsOpen(!isOpen); @@ -58,7 +61,9 @@ export default function ConfigEditorMenu() { {_("Add LVM volume group")} {_("Add MD RAID")} - {_("Change boot options")} + navigate(PATHS.bootDevice)}> + {_("Change boot options")} + {_("Reinstall an existing system")} diff --git a/web/src/routes/paths.ts b/web/src/routes/paths.ts index de6c97b825..395bc4d4b4 100644 --- a/web/src/routes/paths.ts +++ b/web/src/routes/paths.ts @@ -65,7 +65,7 @@ const SOFTWARE = { const STORAGE = { root: "/storage", targetDevice: "/storage/target-device", - bootingPartition: "/storage/booting-partition", + bootDevice: "/storage/boot-device", spacePolicy: "/storage/space-policy/:id", iscsi: "/storage/iscsi", dasd: "/storage/dasd", diff --git a/web/src/routes/storage.tsx b/web/src/routes/storage.tsx index 975c3b8d86..7a94b22553 100644 --- a/web/src/routes/storage.tsx +++ b/web/src/routes/storage.tsx @@ -47,7 +47,7 @@ const routes = (): Route => ({ element: , }, { - path: PATHS.bootingPartition, + path: PATHS.bootDevice, element: , }, {