diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 3b8b0b9783..32a47769c2 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Thu Apr 25 15:04:05 UTC 2024 - José Iván López González + +- Allow adding arbitrary volumes (gh#openSUSE/agama#1154). + ------------------------------------------------------------------- Thu Apr 25 13:40:06 UTC 2024 - Ancor Gonzalez Sosa diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 1e0af12921..55623d8c36 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -151,6 +151,7 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk"; * * @typedef {object} VolumeOutline * @property {boolean} required + * @property {boolean} productDefined * @property {string[]} fsTypes * @property {boolean} adjustByRam * @property {boolean} supportAutoSize @@ -479,7 +480,8 @@ class ProposalManager { async defaultVolume(mountPath) { const proxy = await this.proxies.proposalCalculator; const systemDevices = await this.system.getDevices(); - return this.buildVolume(await proxy.DefaultVolume(mountPath), systemDevices); + const productMountPoints = await this.getProductMountPoints(); + return this.buildVolume(await proxy.DefaultVolume(mountPath), systemDevices, productMountPoints); } /** @@ -493,6 +495,7 @@ class ProposalManager { if (!proxy) return undefined; const systemDevices = await this.system.getDevices(); + const productMountPoints = await this.getProductMountPoints(); const buildResult = (proxy) => { const buildSpaceAction = dbusSpaceAction => { @@ -547,10 +550,11 @@ class ProposalManager { const values = [ dbusSettings.TargetDevice?.v, buildTargetPVDevices(dbusSettings.TargetPVDevices), - dbusSettings.BootDevice.v, dbusSettings.Volumes.v.map(vol => vol.v.TargetDevice.v) ].flat(); + if (dbusSettings.ConfigureBoot.v) values.push(dbusSettings.BootDevice.v); + const names = uniq(compact(values)).filter(d => d.length > 0); // #findDevice returns undefined if no device is found with the given name. @@ -571,7 +575,9 @@ class ProposalManager { spaceActions: dbusSettings.SpaceActions.v.map(a => buildSpaceAction(a.v)), encryptionPassword: dbusSettings.EncryptionPassword.v, encryptionMethod: dbusSettings.EncryptionMethod.v, - volumes: dbusSettings.Volumes.v.map(vol => this.buildVolume(vol.v, systemDevices)), + volumes: dbusSettings.Volumes.v.map(vol => ( + this.buildVolume(vol.v, systemDevices, productMountPoints)) + ), // NOTE: strictly speaking, installation devices does not belong to the settings. It // should be a separate method instead of an attribute in the settings object. // Nevertheless, it was added here for simplicity and to avoid passing more props in some @@ -654,6 +660,8 @@ class ProposalManager { * Builds a volume from the D-Bus data * * @param {DBusVolume} dbusVolume + * @param {StorageDevice[]} devices + * @param {string[]} productMountPoints * * @typedef {Object} DBusVolume * @property {CockpitString} Target @@ -699,7 +707,7 @@ class ProposalManager { * * @returns {Volume} */ - buildVolume(dbusVolume, devices) { + buildVolume(dbusVolume, devices, productMountPoints) { /** * Builds a volume target from a D-Bus value. * @@ -719,11 +727,11 @@ class ProposalManager { } }; + /** @returns {VolumeOutline} */ const buildOutline = (dbusOutline) => { - if (dbusOutline === undefined) return null; - return { required: dbusOutline.Required.v, + productDefined: false, fsTypes: dbusOutline.FsTypes.v.map(val => val.v), supportAutoSize: dbusOutline.SupportAutoSize.v, adjustByRam: dbusOutline.AdjustByRam.v, @@ -733,7 +741,7 @@ class ProposalManager { }; }; - return { + const volume = { target: buildTarget(dbusVolume.Target.v), targetDevice: devices.find(d => d.name === dbusVolume.TargetDevice?.v), mountPath: dbusVolume.MountPath.v, @@ -745,6 +753,12 @@ class ProposalManager { transactional: dbusVolume.Transactional.v, outline: buildOutline(dbusVolume.Outline.v) }; + + // Indicate whether a volume is defined by the product. + if (productMountPoints.includes(volume.mountPath)) + volume.outline.productDefined = true; + + return volume; } /** diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index 4a99593dad..5e0bc13e76 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -1430,6 +1430,7 @@ describe("#proposal", () => { describe("#defaultVolume", () => { beforeEach(() => { + cockpitProxies.proposalCalculator.ProductMountPoints = ["/", "swap", "/home"]; cockpitProxies.proposalCalculator.DefaultVolume = jest.fn(mountPath => { switch (mountPath) { case "/home": return { @@ -1504,7 +1505,8 @@ describe("#proposal", () => { snapshotsConfigurable: false, snapshotsAffectSizes: false, adjustByRam: false, - sizeRelevantVolumes: [] + sizeRelevantVolumes: [], + productDefined: true } }); @@ -1527,7 +1529,8 @@ describe("#proposal", () => { snapshotsConfigurable: false, snapshotsAffectSizes: false, adjustByRam: false, - sizeRelevantVolumes: [] + sizeRelevantVolumes: [], + productDefined: false } }); }); @@ -1550,10 +1553,12 @@ describe("#proposal", () => { beforeEach(() => { contexts.withSystemDevices(); contexts.withProposal(); - client = new StorageClient(); + cockpitProxies.proposalCalculator.ProductMountPoints = ["/", "swap"]; }); it("returns the proposal settings and actions", async () => { + client = new StorageClient(); + const { settings, actions } = await client.proposal.getResult(); expect(settings).toMatchObject({ @@ -1585,7 +1590,8 @@ describe("#proposal", () => { supportAutoSize: true, snapshotsConfigurable: true, snapshotsAffectSizes: true, - sizeRelevantVolumes: ["/home"] + sizeRelevantVolumes: ["/home"], + productDefined: true } }, { @@ -1604,7 +1610,8 @@ describe("#proposal", () => { supportAutoSize: false, snapshotsConfigurable: false, snapshotsAffectSizes: false, - sizeRelevantVolumes: [] + sizeRelevantVolumes: [], + productDefined: false } } ] @@ -1618,6 +1625,19 @@ describe("#proposal", () => { { device: 2, text: "Mount /dev/sdb1 as root", subvol: false, delete: false } ]); }); + + describe("if boot is not configured", () => { + beforeEach(() => { + cockpitProxies.proposal.Settings.ConfigureBoot = { t: "b", v: false }; + cockpitProxies.proposal.Settings.BootDevice = { t: "s", v: "/dev/sdc" }; + }); + + it("does not include the boot device as installation device", async () => { + client = new StorageClient(); + const { settings } = await client.proposal.getResult(); + expect(settings.installationDevices).toEqual([sda, sdb]); + }); + }); }); }); diff --git a/web/src/components/core/FormReadOnlyField.jsx b/web/src/components/core/FormReadOnlyField.jsx new file mode 100644 index 0000000000..7e98b80fb0 --- /dev/null +++ b/web/src/components/core/FormReadOnlyField.jsx @@ -0,0 +1,60 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * 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. + */ + +// @ts-check +// cspell:ignore labelable + +import React from "react"; +import styles from '@patternfly/react-styles/css/components/Form/form'; + +/** + * Renders a read-only form value with a label that visually looks identically + * that a a label of an editable form value, without using the `label` HTML tag. + * + * Basically, this "mimicking component" is needed for two reasons: + * + * - The HTML specification limits the use of labels to "labelable elements". + * + * > Some elements, not all of them form-associated, are categorized as labelable + * > elements. These are elements that can be associated with a label element. + * > => button, input (if the type attribute is not in the Hidden state), meter, + * > output, progress, select, textarea, form-associated custom elements + * > + * > https://html.spec.whatwg.org/multipage/forms.html#categories + * + * - Agama does not use disabled form controls for rendering a value that users + * have no chance to change by any means, but a raw text instead. + * + * Based on PatternFly Form styles to maintain consistency. + * + * @typedef {import("react").ComponentProps<"div">} HTMLDivProps + * @param {HTMLDivProps & { label: string }} props + */ +export default function FormReadOnlyField({ label, children, className = "", ...props }) { + return ( +
+
+ {label} +
+ {children} +
+ ); +} diff --git a/web/src/components/core/FormReadOnlyField.test.jsx b/web/src/components/core/FormReadOnlyField.test.jsx new file mode 100644 index 0000000000..f4ebd86d4b --- /dev/null +++ b/web/src/components/core/FormReadOnlyField.test.jsx @@ -0,0 +1,33 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * 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 } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { FormReadOnlyField } from "~/components/core"; + +it("renders label and content wrapped in div nodes using expected PatternFly styles", () => { + plainRender(Agama); + const field = screen.getByText("Agama"); + const label = screen.getByText("Product"); + expect(field.classList.contains("pf-v5-c-form__group")).toBe(true); + expect(label.classList.contains("pf-v5-c-form__label-text")).toBe(true); +}); diff --git a/web/src/components/core/FormValidationError.jsx b/web/src/components/core/FormValidationError.jsx index bb81dea922..bf86d5572d 100644 --- a/web/src/components/core/FormValidationError.jsx +++ b/web/src/components/core/FormValidationError.jsx @@ -19,14 +19,18 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; import { FormHelperText, HelperText, HelperTextItem } from "@patternfly/react-core"; /** * Helper component for displaying error messages in a PF/FormGroup * + * @todo: allow it to accept children instead of message prop + * * @param {object} props - component props - * @param {string} [props.message] - text to be shown as error + * @param {React.ReactNode} [props.message] - text to be shown as error */ export default function FormValidationError({ message }) { if (!message) return; @@ -35,7 +39,7 @@ export default function FormValidationError({ message }) { - { message } + {message} diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index 7834183e6f..c111b5364c 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -27,6 +27,7 @@ export { default as Disclosure } from "./Disclosure"; export { default as Sidebar } from "./Sidebar"; export { default as Section } from "./Section"; export { default as FormLabel } from "./FormLabel"; +export { default as FormReadOnlyField } from "./FormReadOnlyField"; export { default as FormValidationError } from "./FormValidationError"; export { default as Fieldset } from "./Fieldset"; export { default as Em } from "./Em"; diff --git a/web/src/components/storage/BootSelectionDialog.jsx b/web/src/components/storage/BootSelectionDialog.jsx index ea043eb29e..006cb54d9e 100644 --- a/web/src/components/storage/BootSelectionDialog.jsx +++ b/web/src/components/storage/BootSelectionDialog.jsx @@ -104,7 +104,7 @@ export default function BootSelectionDialog({ const onSubmit = (e) => { e.preventDefault(); - const device = isBootAuto ? undefined : bootDevice; + const device = ((configureBoot && !isBootAuto) ? bootDevice : undefined); onAccept({ configureBoot, bootDevice: device }); }; diff --git a/web/src/components/storage/BootSelectionDialog.test.jsx b/web/src/components/storage/BootSelectionDialog.test.jsx index 395c0e4ebb..4b3a80fe39 100644 --- a/web/src/components/storage/BootSelectionDialog.test.jsx +++ b/web/src/components/storage/BootSelectionDialog.test.jsx @@ -225,7 +225,7 @@ describe("BootSelectionDialog", () => { describe("if the 'Do not configure' option is selected", () => { beforeEach(() => { props.configureBoot = true; - props.bootDevice = undefined; + props.bootDevice = sda; }); it("calls onAccept with the selected options on accept", async () => { diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 6c8aeafc4d..dc971ff49c 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -22,15 +22,19 @@ // @ts-check import React, { useState } from "react"; -import { Button, List, ListItem, Skeleton } from '@patternfly/react-core'; +import { + Button, Dropdown, DropdownList, DropdownItem, List, ListItem, MenuToggle, Skeleton +} from '@patternfly/react-core'; import { Table, Thead, Tr, Th, Tbody, Td } from '@patternfly/react-table'; import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; -import { If, ExpandableField, Popup, RowActions, Tip } from '~/components/core'; -import { VolumeForm } from '~/components/storage'; +import { If, ExpandableField, RowActions, Tip } from '~/components/core'; +import VolumeDialog from '~/components/storage/VolumeDialog'; import VolumeLocationDialog from '~/components/storage/VolumeLocationDialog'; -import { deviceSize, hasSnapshots, isTransactionalRoot } from '~/components/storage/utils'; +import { + deviceSize, hasSnapshots, isTransactionalRoot, isTransactionalSystem +} from '~/components/storage/utils'; import SnapshotsField from "~/components/storage/SnapshotsField"; import BootConfigField from "~/components/storage/BootConfigField"; import { noop } from "~/utils"; @@ -38,6 +42,8 @@ import { noop } from "~/utils"; /** * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import("~/components/storage/SnapshotsField").SnapshotsConfig} SnapshotsConfig + * * @typedef {import ("~/client/storage").Volume} Volume */ @@ -202,57 +208,6 @@ const AutoCalculatedHint = ({ volume }) => { ); }; -/** - * Button with general actions for the file systems - * @component - * - * @param {object} props - * @param {object[]} props.templates - Volume templates - * @param {onAddFn} props.onAdd - Function to use for adding a new volume - * @param {onResetFn} props.onReset - Function to use for resetting to the default subvolumes - * - * @callback onAddFn - * @param {object} volume - * @return {void} - * - * @callback onResetFn - * @return {void} - */ -const GeneralActions = ({ templates, onAdd, onReset }) => { - const [isFormOpen, setIsFormOpen] = useState(false); - - const openForm = () => setIsFormOpen(true); - - const closeForm = () => setIsFormOpen(false); - - const acceptForm = (volume) => { - closeForm(); - onAdd(volume); - }; - - return ( -
- - - - - - {_("Accept")} - - - -
- ); -}; - /** * @component * @@ -290,6 +245,8 @@ const BootLabel = ({ bootDevice, configureBoot }) => { * @param {object} props * @param {object} [props.columns] - Column specs * @param {Volume} [props.volume] - Volume to show + * @param {Volume[]} [props.volumes] - List of current volumes + * @param {Volume[]} [props.templates] - List of available templates * @param {StorageDevice[]} [props.devices=[]] - Devices available for installation * @param {ProposalTarget} [props.target] - Installation target * @param {StorageDevice} [props.targetDevice] - Device selected for installation, if target is a disk @@ -300,6 +257,8 @@ const BootLabel = ({ bootDevice, configureBoot }) => { const VolumeRow = ({ columns, volume, + volumes, + templates, devices, target, targetDevice, @@ -440,20 +399,19 @@ const VolumeRow = ({ /> - - - - - {_("Accept")} - - - - + + } + /> void} props.onVolumesChange - Function to submit changes in volumes */ -const VolumesTable = ({ volumes, devices, target, targetDevice, isLoading, onVolumesChange }) => { +const VolumesTable = ({ + volumes, + templates, + devices, + target, + targetDevice, + isLoading, + onVolumesChange +}) => { const columns = { mountPath: _("Mount point"), details: _("Details"), @@ -494,6 +461,7 @@ const VolumesTable = ({ volumes, devices, target, targetDevice, isLoading, onVol actions: _("Actions") }; + /** @type {(volume: Volume) => void} */ const editVolume = (volume) => { const index = volumes.findIndex(v => v.mountPath === volume.mountPath); const newVolumes = [...volumes]; @@ -501,11 +469,13 @@ const VolumesTable = ({ volumes, devices, target, targetDevice, isLoading, onVol onVolumesChange(newVolumes); }; + /** @type {(volume: Volume) => void} */ const deleteVolume = (volume) => { const newVolumes = volumes.filter(v => v.mountPath !== volume.mountPath); onVolumesChange(newVolumes); }; + /** @type {() => React.ReactElement[]|React.ReactElement} */ const renderVolumes = () => { if (volumes.length === 0 && isLoading) return ; @@ -515,6 +485,8 @@ const VolumesTable = ({ volumes, devices, target, targetDevice, isLoading, onVol key={index} columns={columns} volume={volume} + volumes={volumes} + templates={templates} devices={devices} target={target} targetDevice={targetDevice} @@ -573,6 +545,68 @@ const Basic = ({ volumes, configureBoot, bootDevice, target, isLoading }) => { ); }; +/** + * Button for adding a new volume. It renders either a menu or a button depending on the number + * of options. + * @component + * + * @param {object} props + * @param {string[]} props.options - Possible mount points to add. An empty string represent an + * arbitrary mount point. + * @param {(option: string) => void} props.onClick + */ +const AddVolumeButton = ({ options, onClick }) => { + const [isOpen, setIsOpen] = React.useState(false); + + /** @type {() => void} */ + const onToggleClick = () => setIsOpen(!isOpen); + + /** @type {(_: any, value: string) => void} */ + const onSelect = (_, value) => { + setIsOpen(false); + onClick(value); + }; + + // Shows a button if the only option is to add an arbitrary volume. + if (options.length === 1 && options[0] === "") { + return ( + + ); + } + + const isDisabled = !options.length; + + return ( + ( + + {_("Add file system")} + + )} + shouldFocusToggleOnSelect + > + + {options.map((option, index) => { + return ( + + {option === "" ? _("Other") : option} + + ); + })} + + + ); +}; + /** * Content to show when the field is expanded. * @component @@ -603,15 +637,69 @@ const Advanced = ({ onBootChange, isLoading }) => { - const rootVolume = (volumes || []).find((i) => i.mountPath === "/"); + const [isVolumeDialogOpen, setIsVolumeDialogOpen] = useState(false); + /** @type {[Volume|undefined, (volume: Volume) => void]} */ + const [template, setTemplate] = useState(); + + const openVolumeDialog = () => setIsVolumeDialogOpen(true); + + const closeVolumeDialog = () => setIsVolumeDialogOpen(false); + + /** @type {(volume: Volume) => void} */ + const onAcceptVolumeDialog = (volume) => { + closeVolumeDialog(); - const addVolume = (volume) => onVolumesChange([...volumes, volume]); + const index = volumes.findIndex(v => v.mountPath === volume.mountPath); + + if (index !== -1) { + const newVolumes = [...volumes]; + newVolumes[index] = volume; + onVolumesChange(newVolumes); + } else { + onVolumesChange([...volumes, volume]); + } + }; const resetVolumes = () => onVolumesChange([]); - const changeBtrfsSnapshots = ({ active }) => { - // const rootVolume = volumes.find((i) => i.mountPath === "/"); + /** @type {(mountPath: string) => void} */ + const addVolume = (mountPath) => { + const template = templates.find(t => t.mountPath === mountPath); + setTemplate(template); + openVolumeDialog(); + }; + + /** + * Possible mount paths to add. + * @type {() => string[]} + */ + const mountPathOptions = () => { + const mountPaths = volumes.map(v => v.mountPath); + const isTransactional = isTransactionalSystem(templates); + + return templates + .map(t => t.mountPath) + .filter(p => !mountPaths.includes(p)) + .filter(p => !isTransactional || p.length); + }; + + /** + * Whether to show the button for adding a volume. + * @type {() => boolean} + */ + const showAddVolume = () => { + const hasOptionalVolumes = () => { + return templates.find(t => t.mountPath.length && !t.outline.required) !== undefined; + }; + + return !isTransactionalSystem(templates) || hasOptionalVolumes(); + }; + + /** @type {Volume} */ + const rootVolume = volumes.find(v => v.mountPath === "/"); + /** @type {(config: SnapshotsConfig) => void} */ + const changeBtrfsSnapshots = ({ active }) => { if (active) { rootVolume.fsType = "Btrfs"; rootVolume.snapshots = true; @@ -630,16 +718,32 @@ const Advanced = ({ /> - + } + /> + + + + } />
{ }; }); -/** @todo Add tests for collapsed field. */ - it("allows to reset the file systems", async () => { const { user } = await expandField(); const button = screen.getByRole("button", { name: "Reset to defaults" }); @@ -179,15 +202,61 @@ it("allows to reset the file systems", async () => { expect(props.onVolumesChange).toHaveBeenCalledWith([]); }); +it("renders a button for adding a file system when only arbitrary volumes can be added", async () => { + props.templates = [arbitraryVolume]; + const { user } = await expandField(); + const button = screen.getByRole("button", { name: "Add file system" }); + expect(button).not.toHaveAttribute("aria-expanded"); + await user.click(button); + screen.getByRole("dialog", { name: "Add file system" }); +}); + +it("renders a menu for adding a file system when predefined and arbitrary volume can be added", async () => { + props.templates = [homeVolume, arbitraryVolume]; + const { user } = await expandField(); + + const button = screen.getByRole("button", { name: "Add file system" }); + expect(button).toHaveAttribute("aria-expanded", "false"); + await user.click(button); + + expect(button).toHaveAttribute("aria-expanded", "true"); + const homeOption = screen.getByRole("menuitem", { name: "/home" }); + await user.click(homeOption); + + screen.getByRole("dialog", { name: "Add /home file system" }); +}); + +it("renders the control for adding a file system when using transactional system with optional templates", async () => { + props.templates = [{ ...rootVolume, transactional: true }, homeVolume]; + await expandField(); + screen.queryByRole("button", { name: "Add file system" }); +}); + +it("does not render the control for adding a file system when using transactional system with no optional templates", async () => { + props.templates = [{ ...rootVolume, transactional: true }]; + await expandField(); + expect(screen.queryByRole("button", { name: "Add file system" })).toBeNull(); +}); + +it("renders the control as disabled when there are no more left predefined volumes to add and arbitrary volumes are not allowed", async () => { + props.templates = [rootVolume, homeVolume]; + props.volumes = [rootVolume, homeVolume]; + await expandField(); + const button = screen.getByRole("button", { name: "Add file system" }); + expect(button).toBeDisabled(); +}); + it("allows to add a file system", async () => { props.templates = [homeVolume]; const { user } = await expandField(); const button = screen.getByRole("button", { name: "Add file system" }); await user.click(button); + const homeOption = screen.getByRole("menuitem", { name: "/home" }); + await user.click(homeOption); - const popup = await screen.findByRole("dialog"); - const accept = within(popup).getByRole("button", { name: "Accept" }); + const dialog = await screen.findByRole("dialog"); + const accept = within(dialog).getByRole("button", { name: "Accept" }); await user.click(accept); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); @@ -195,7 +264,7 @@ it("allows to add a file system", async () => { }); it("allows to cancel adding a file system", async () => { - props.templates = [homeVolume]; + props.templates = [arbitraryVolume]; const { user } = await expandField(); const button = screen.getByRole("button", { name: "Add file system" }); @@ -262,7 +331,7 @@ describe("if there are volumes", () => { await user.click(editAction); const popup = await screen.findByRole("dialog"); - within(popup).getByText("Edit file system"); + within(popup).getByRole("heading", { name: "Edit /home file system" }); }); it("allows changing the location of the volume", async () => { diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 00649c0bf0..661223c847 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -115,7 +115,8 @@ const volume = (mountPath) => { snapshotsConfigurable: false, snapshotsAffectSizes: false, sizeRelevantVolumes: [], - adjustByRam: false + adjustByRam: false, + productDefined: false } } ); diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 04948c5df7..37bf0669ac 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -112,18 +112,6 @@ export default function ProposalSettingsSection({ const defaultBootDevice = findDevice(settings.defaultBootDevice); const spacePolicy = SPACE_POLICIES.find(p => p.id === settings.spacePolicy); - /** - * Templates for already existing mount points are filtered out. - * - * @returns {Volume[]} - */ - const usefulTemplates = () => { - const mountPaths = volumes.map(v => v.mountPath); - return volumeTemplates.filter(t => ( - t.mountPath.length > 0 && !mountPaths.includes(t.mountPath) - )); - }; - return ( <>
@@ -144,7 +132,7 @@ export default function ProposalSettingsSection({ /> void} onChange + * @property {(config: SnapshotsConfig) => void} onChange + * + * @typedef {object} SnapshotsConfig + * @property {boolean} active * * @param {SnapshotsFieldProps} props */ diff --git a/web/src/components/storage/SnapshotsField.test.jsx b/web/src/components/storage/SnapshotsField.test.jsx index 4b2bb43ae5..9516843a77 100644 --- a/web/src/components/storage/SnapshotsField.test.jsx +++ b/web/src/components/storage/SnapshotsField.test.jsx @@ -47,7 +47,8 @@ const rootVolume = { snapshotsConfigurable: false, snapshotsAffectSizes: true, adjustByRam: false, - sizeRelevantVolumes: ["/home"] + sizeRelevantVolumes: ["/home"], + productDefined: true } }; diff --git a/web/src/components/storage/VolumeDialog.jsx b/web/src/components/storage/VolumeDialog.jsx new file mode 100644 index 0000000000..67072d96bc --- /dev/null +++ b/web/src/components/storage/VolumeDialog.jsx @@ -0,0 +1,756 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * 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. + */ + +// @ts-check + +import React, { useReducer } from "react"; +import { Button, Form } from "@patternfly/react-core"; +import { sprintf } from "sprintf-js"; + +import { _ } from "~/i18n"; +import { compact, useDebounce } from "~/utils"; +import { + DEFAULT_SIZE_UNIT, SIZE_METHODS, parseToBytes, splitSize +} from '~/components/storage/utils'; +import { FsField, MountPathField, SizeOptionsField } from "~/components/storage/VolumeFields"; +import { Popup } from '~/components/core'; + +/** + * @typedef {import ("~/client/storage").Volume} Volume + * @typedef {import("~/components/storage/utils").SizeMethod} SizeMethod + * + * @typedef {object} VolumeFormState + * @property {Volume} volume + * @property {VolumeFormData} formData + * @property {VolumeFormErrors} errors + * + * @typedef {object} VolumeFormData + * @property {number|string} [size] + * @property {string} [sizeUnit] + * @property {number|string} [minSize] + * @property {string} [minSizeUnit] + * @property {number|string} [maxSize] + * @property {string} [maxSizeUnit] + * @property {SizeMethod} sizeMethod + * @property {string} mountPath + * @property {string} fsType + * @property {boolean} snapshots + * + * @typedef {object} VolumeFormErrors + * @property {string|null} missingMountPath + * @property {string|null} invalidMountPath + * @property {React.ReactElement|null} existingVolume + * @property {React.ReactElement|null} existingTemplate + * @property {string|null} missingSize + * @property {string|null} missingMinSize + * @property {string|null} invalidMaxSize + * + * @typedef {object} VolumeDialogProps + * @property {Volume} volume + * @property {Volume[]} volumes + * @property {Volume[]} templates + * @property {boolean} [isOpen=false] + * @property {() => void} onCancel + * @property {(volume: Volume) => void} onAccept + */ + +/** + * Renders the title for the dialog. + * @function + * + * @param {Volume} volume + * @param {Volume[]} volumes + * @returns {string} + */ +const renderTitle = (volume, volumes) => { + const isNewVolume = !volumes.includes(volume); + const isProductDefined = volume.outline.productDefined; + const mountPath = volume.mountPath === "/" ? "root" : volume.mountPath; + + if (isNewVolume && isProductDefined) return sprintf(_("Add %s file system"), mountPath); + if (!isNewVolume && isProductDefined) return sprintf(_("Edit %s file system"), mountPath); + + return isNewVolume ? _("Add file system") : _("Edit file system"); +}; + +/** @fixme Redesign *Error classes. + * + * Having different *Error classes does not seem to be a good design. Note these classes do not + * represent an error but a helper to check and render an error. It would be a better approach to + * have something like a volume checker which generates errors: + * + * For example: + * + * const checker = new VolumeChecker(volume, volumes, templates); + * const error = checker.existingMountPathError(); + * const message = error?.render(onClick); +*/ + +class MissingMountPathError { + /** + * @constructor + * @param {string} mountPath + */ + constructor(mountPath) { + this.mountPath = mountPath; + } + + /** + * @method + * @returns {boolean} + */ + check() { + return this.mountPath.length === 0; + } + + /** + * @method + * @returns {String} + */ + render() { + return _("A mount point is required"); + } +} + +class InvalidMountPathError { + /** + * @constructor + * @param {string} mountPath + */ + constructor(mountPath) { + this.mountPath = mountPath; + } + + /** + * @method + * @returns {boolean} + */ + check() { + const regex = /^swap$|^\/$|^(\/[^/\s]+([^/]*[^/\s])*)+$/; + return !regex.test(this.mountPath); + } + + /** + * @method + * @returns {string} + */ + render() { + return _("The mount point is invalid"); + } +} + +class MissingSizeError { + /** + * @constructor + * @param {SizeMethod} sizeMethod + * @param {string|number} size + */ + constructor (sizeMethod, size) { + this.sizeMethod = sizeMethod; + this.size = size; + } + + /** + * @method + * @returns {boolean} + */ + check() { + return this.sizeMethod === SIZE_METHODS.MANUAL && !this.size; + } + + /** + * @method + * @returns {string} + */ + render() { + return _("A size value is required"); + } +} + +class MissingMinSizeError { + /** + * @constructor + * @param {SizeMethod} sizeMethod + * @param {string|number} minSize + */ + constructor (sizeMethod, minSize) { + this.sizeMethod = sizeMethod; + this.minSize = minSize; + } + + /** + * @method + * @returns {boolean} + */ + check() { + return this.sizeMethod === SIZE_METHODS.RANGE && !this.minSize; + } + + /** + * @method + * @returns {string} + */ + render() { + return _("Minimum size is required"); + } +} + +class InvalidMaxSizeError { + /** + * @constructor + * @param {SizeMethod} sizeMethod + * @param {string|number} minSize + * @param {string|number} maxSize + */ + constructor (sizeMethod, minSize, maxSize) { + this.sizeMethod = sizeMethod; + this.minSize = minSize; + this.maxSize = maxSize; + } + + /** + * @method + * @returns {boolean} + */ + check() { + return this.sizeMethod === SIZE_METHODS.RANGE && + this.maxSize !== -1 && + this.maxSize <= this.minSize; + } + + /** + * @method + * @returns {string} + */ + render() { + return _("Maximum must be greater than minimum"); + } +} + +class ExistingVolumeError { + /** + * @constructor + * @param {string} mountPath + * @param {Volume[]} volumes + */ + constructor(mountPath, volumes) { + this.mountPath = mountPath; + this.volumes = volumes; + } + + /** + * @method + * @returns {Volume|undefined} + */ + findVolume() { + return this.volumes.find(t => t.mountPath === this.mountPath); + } + + /** + * @method + * @returns {boolean} + */ + check() { + return this.mountPath.length && this.findVolume() !== undefined; + } + + /** + * @method + * @param {(volume: Volume) => void} onClick + * @returns {React.ReactElement} + */ + render(onClick) { + const volume = this.findVolume(); + const path = this.mountPath === "/" ? "root" : this.mountPath; + + return ( +
+ {sprintf(_("There is already a file system for %s."), path)} + +
+ ); + } +} + +class ExistingTemplateError { + /** + * @constructor + * @param {string} mountPath + * @param {Volume[]} templates + */ + constructor(mountPath, templates) { + this.mountPath = mountPath; + this.templates = templates; + } + + /** + * @method + * @returns {Volume|undefined} + */ + findTemplate() { + return this.templates.find(t => t.mountPath === this.mountPath); + } + + /** + * @method + * @returns {boolean} + */ + check() { + return this.mountPath.length && this.findTemplate() !== undefined; + } + + /** + * @method + * @param {(template: Volume) => void} onClick + * @returns {React.ReactElement} + */ + render(onClick) { + const template = this.findTemplate(); + const path = this.mountPath === "/" ? "root" : this.mountPath; + + return ( +
+ {sprintf(_("There is a predefined file system for %s."), path)} + +
+ ); + } +} + +/** + * Error if the mount path is missing. + * @function + * + * @param {string} mountPath + * @returns {string|null} + */ +const missingMountPathError = (mountPath) => { + const error = new MissingMountPathError(mountPath); + return error.check() ? error.render() : null; +}; + +/** + * Error if the mount path is not valid. + * @function + * + * @param {string} mountPath + * @returns {string|null} + */ +const invalidMountPathError = (mountPath) => { + const error = new InvalidMountPathError(mountPath); + return error.check() ? error.render() : null; +}; + +/** + * Error if the size is missing. + * @function + * + * @param {SizeMethod} sizeMethod + * @param {string|number} size + * @returns {string|null} + */ +const missingSizeError = (sizeMethod, size) => { + const error = new MissingSizeError(sizeMethod, size); + return error.check() ? error.render() : null; +}; + +/** + * Error if the min size is missing. + * @function + * + * @param {SizeMethod} sizeMethod + * @param {string|number} minSize + * @returns {string|null} + */ +const missingMinSizeError = (sizeMethod, minSize) => { + const error = new MissingMinSizeError(sizeMethod, minSize); + return error.check() ? error.render() : null; +}; + +/** + * Error if the max size is not valid. + * @function + * + * @param {SizeMethod} sizeMethod + * @param {string|number} minSize + * @param {string|number} maxSize + * @returns {string|null} + */ +const invalidMaxSizeError = (sizeMethod, minSize, maxSize) => { + const error = new InvalidMaxSizeError(sizeMethod, minSize, maxSize); + return error.check() ? error.render() : null; +}; + +/** + * Error if the given mount path exists in the list of volumes. + * @function + * + * @param {string} mountPath + * @param {Volume[]} volumes + * @param {(volume: Volume) => void} onClick + * @returns {React.ReactElement|null} + */ +const existingVolumeError = (mountPath, volumes, onClick) => { + const error = new ExistingVolumeError(mountPath, volumes); + return error.check() ? error.render(onClick) : null; +}; + +/** + * Error if the given mount path exists in the list of templates. + * @function + * + * @param {string} mountPath + * @param {Volume[]} templates + * @param {(template: Volume) => void} onClick + * @returns {React.ReactElement|null} + */ +const existingTemplateError = (mountPath, templates, onClick) => { + const error = new ExistingTemplateError(mountPath, templates); + return error.check() ? error.render(onClick) : null; +}; + +/** + * Checks whether there is any error. + * @function + * + * @param {VolumeFormErrors} errors + * @returns {boolean} + */ +const anyError = (errors) => { + return compact(Object.values(errors)).length > 0; +}; + +/** + * Remove leftover trailing slash. + * @function + * + * @param {string} mountPath + * @returns {string} + */ +const sanitizeMountPath = (mountPath) => { + if (mountPath === "/") return mountPath; + + return mountPath.replace(/\/$/, ""); +}; + +/** + * Creates a new storage volume object based on given params. + * @function + * + * @param {Volume} volume + * @param {VolumeFormData} formData + * @returns {Volume} + */ +const createUpdatedVolume = (volume, formData) => { + let sizeAttrs = {}; + const size = parseToBytes(`${formData.size} ${formData.sizeUnit}`); + const minSize = parseToBytes(`${formData.minSize} ${formData.minSizeUnit}`); + const maxSize = parseToBytes(`${formData.maxSize} ${formData.maxSizeUnit}`); + + switch (formData.sizeMethod) { + case SIZE_METHODS.AUTO: + sizeAttrs = { minSize: undefined, maxSize: undefined, autoSize: true }; + break; + case SIZE_METHODS.MANUAL: + sizeAttrs = { minSize: size, maxSize: size, autoSize: false }; + break; + case SIZE_METHODS.RANGE: + sizeAttrs = { minSize, maxSize: formData.maxSize ? maxSize : undefined, autoSize: false }; + break; + } + + const { fsType, snapshots } = formData; + const mountPath = sanitizeMountPath(formData.mountPath); + + return { ...volume, mountPath, ...sizeAttrs, fsType, snapshots }; +}; + +/** + * Form-related helper for guessing the size method for given volume + * @function + * + * @param {Volume} volume - a storage volume + * @return {SizeMethod} corresponding size method + */ +const sizeMethodFor = (volume) => { + const { autoSize, minSize, maxSize } = volume; + + if (autoSize) { + return SIZE_METHODS.AUTO; + } else if (minSize !== maxSize) { + return SIZE_METHODS.RANGE; + } else { + return SIZE_METHODS.MANUAL; + } +}; + +/** + * Form-related helper for preparing data based on given volume + * @function + * + * @param {Volume} volume - a storage volume object + * @return {VolumeFormData} an object ready to be used as a "form state" + */ +const prepareFormData = (volume) => { + const { size: minSize = "", unit: minSizeUnit = DEFAULT_SIZE_UNIT } = splitSize(volume.minSize); + const { size: maxSize = "", unit: maxSizeUnit = minSizeUnit || DEFAULT_SIZE_UNIT } = splitSize(volume.maxSize); + + return { + size: minSize, + sizeUnit: minSizeUnit, + minSize, + minSizeUnit, + maxSize, + maxSizeUnit, + sizeMethod: sizeMethodFor(volume), + mountPath: volume.mountPath, + fsType: volume.fsType, + snapshots: volume.snapshots + }; +}; + +/** + * Possible errors from the form data. + * @function + * + * @returns {VolumeFormErrors} + */ +const prepareErrors = () => { + return { + missingMountPath: null, + invalidMountPath: null, + existingVolume: null, + existingTemplate: null, + missingSize: null, + missingMinSize: null, + invalidMaxSize: null + }; +}; + +/** + * Initializer function for the React#useReducer used in the {@link VolumesForm} + * @function + * + * @param {Volume} volume - a storage volume object + * @returns {VolumeFormState} + */ +const createInitialState = (volume) => { + const formData = prepareFormData(volume); + const errors = prepareErrors(); + + return { volume, formData, errors }; +}; + +/** + * The VolumeForm reducer. + * @function + * + * @param {VolumeFormState} state + * @param {object} action + */ +const reducer = (state, action) => { + const { type, payload } = action; + + switch (type) { + case "CHANGE_VOLUME": { + return createInitialState(payload.volume); + } + + case "UPDATE_DATA": { + return { + ...state, + formData: { + ...state.formData, + ...payload + } + }; + } + + case "SET_ERRORS": { + const errors = { ...state.errors, ...payload }; + return { ...state, errors }; + } + + default: { + return state; + } + } +}; + +/** + * Renders a dialog that allows the user to add or edit a file system. + * @component + * + * @param {VolumeDialogProps} props + */ +export default function VolumeDialog({ + volume: currentVolume, + volumes, + templates, + isOpen, + onCancel, + onAccept +}) { + /** @type {[VolumeFormState, (action: object) => void]} */ + const [state, dispatch] = useReducer(reducer, currentVolume, createInitialState); + + /** @type {Function} */ + const delayed = useDebounce(f => f(), 1000); + + /** @type {(volume: Volume) => void} */ + const changeVolume = (volume) => { + dispatch({ type: "CHANGE_VOLUME", payload: { volume } }); + }; + + /** @type {(data: object) => void} */ + const updateData = (data) => dispatch({ type: "UPDATE_DATA", payload: data }); + + /** @type {(errors: object) => void} */ + const updateErrors = (errors) => dispatch({ type: "SET_ERRORS", payload: errors }); + + /** @type {() => string|React.ReactElement} */ + const mountPathError = () => { + const { missingMountPath, invalidMountPath, existingVolume, existingTemplate } = state.errors; + return missingMountPath || invalidMountPath || existingVolume || existingTemplate; + }; + + /** @type {() => object} */ + const sizeErrors = () => { + return { + size: state.errors.missingSize, + minSize: state.errors.missingMinSize, + maxSize: state.errors.invalidMaxSize + }; + }; + + /** @type {() => boolean} */ + const disableWidgets = () => { + const { existingVolume, existingTemplate } = state.errors; + return existingVolume !== null || existingTemplate !== null; + }; + + /** @type {() => boolean} */ + const isMountPathEditable = () => { + const isNewVolume = !volumes.includes(state.volume); + const isPredefined = state.volume.outline.productDefined; + return isNewVolume && !isPredefined; + }; + + /** @type {(mountPath: string) => void} */ + const changeMountPath = (mountPath) => { + // Reset current errors. + const errors = { + missingMountPath: null, + invalidMountPath: null, + existingVolume: null, + existingTemplate: null + }; + updateErrors(errors); + + delayed(() => { + // Reevaluate in a delayed way. + const errors = { + existingVolume: existingVolumeError(mountPath, volumes, changeVolume), + existingTemplate: existingTemplateError(mountPath, templates, changeVolume) + }; + updateErrors(errors); + }); + + updateData({ mountPath }); + }; + + /** @type {(data: object) => void} */ + const changeSizeOptions = (data) => { + // Reset errors. + const errors = { + missingSize: null, + missingMinSize: null, + invalidMaxSize: null + }; + updateErrors(errors); + updateData(data); + }; + + /** @type {(e: import("react").FormEvent) => void} */ + const submitForm = (e) => { + e.preventDefault(); + const { volume: originalVolume, formData } = state; + const volume = createUpdatedVolume(originalVolume, formData); + + const checkMountPath = isMountPathEditable(); + + const errors = { + missingMountPath: checkMountPath ? missingMountPathError(volume.mountPath) : null, + invalidMountPath: checkMountPath ? invalidMountPathError(volume.mountPath) : null, + existingVolume: checkMountPath ? existingVolumeError(volume.mountPath, volumes, changeVolume) : null, + existingTemplate: checkMountPath ? existingTemplateError(volume.mountPath, templates, changeVolume) : null, + missingSize: missingSizeError(formData.sizeMethod, volume.minSize), + missingMinSize: missingMinSizeError(formData.sizeMethod, volume.minSize), + invalidMaxSize: invalidMaxSizeError(formData.sizeMethod, volume.minSize, volume.maxSize) + }; + + anyError(errors) ? updateErrors(errors) : onAccept(volume); + }; + + const title = renderTitle(state.volume, volumes); + const { fsType, mountPath } = state.formData; + const isDisabled = disableWidgets(); + + return ( + /** @fixme blockSize medium is too big and small is too small. */ + +
+ + + + + + + {_("Accept")} + + + +
+ ); +} diff --git a/web/src/components/storage/VolumeDialog.test.jsx b/web/src/components/storage/VolumeDialog.test.jsx new file mode 100644 index 0000000000..3f88e74f6d --- /dev/null +++ b/web/src/components/storage/VolumeDialog.test.jsx @@ -0,0 +1,418 @@ +/* + * Copyright (c) [2004] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * 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. + */ + +// @ts-check + +import React from "react"; +import { screen, waitFor, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { parseToBytes } from "~/components/storage/utils"; +import VolumeDialog from "~/components/storage/VolumeDialog"; + +/** + * @typedef {import ("~/client/storage").Volume} Volume + * @typedef {import ("~/components/storage/VolumeDialog").VolumeDialogProps} VolumeDialogProps + */ + +/** @type {Volume} */ +const rootVolume = { + mountPath: "/", + target: "DEFAULT", + fsType: "Btrfs", + minSize: 1024, + maxSize: 2048, + autoSize: false, + snapshots: false, + transactional: false, + outline: { + required: true, + fsTypes: ["Btrfs", "Ext4"], + supportAutoSize: true, + snapshotsConfigurable: true, + snapshotsAffectSizes: true, + sizeRelevantVolumes: [], + adjustByRam: false, + productDefined: true + } +}; + +/** @type {Volume} */ +const swapVolume = { + mountPath: "swap", + target: "DEFAULT", + fsType: "Swap", + minSize: 1024, + maxSize: 1024, + autoSize: false, + snapshots: false, + transactional: false, + outline: { + required: false, + fsTypes: ["Swap"], + supportAutoSize: false, + snapshotsConfigurable: false, + snapshotsAffectSizes: false, + adjustByRam: false, + sizeRelevantVolumes: [], + productDefined: true + } +}; + +/** @type {Volume} */ +const homeVolume = { + mountPath: "/home", + target: "DEFAULT", + fsType: "XFS", + minSize: 1024, + maxSize: 4096, + autoSize: false, + snapshots: false, + transactional: false, + outline: { + required: false, + fsTypes: ["Ext4", "XFS"], + supportAutoSize: false, + snapshotsConfigurable: false, + snapshotsAffectSizes: false, + adjustByRam: false, + sizeRelevantVolumes: [], + productDefined: true + } +}; + +/** @type {Volume} */ +const arbitraryVolume = { + mountPath: "", + target: "DEFAULT", + fsType: "XFS", + minSize: 1024, + maxSize: 4096, + autoSize: false, + snapshots: false, + transactional: false, + outline: { + required: false, + fsTypes: ["Ext4", "XFS"], + supportAutoSize: false, + snapshotsConfigurable: false, + snapshotsAffectSizes: false, + adjustByRam: false, + sizeRelevantVolumes: [], + productDefined: false + } +}; + +/** @type {VolumeDialogProps} */ +let props; + +describe("VolumeDialog", () => { + beforeEach(() => { + props = { + volume: undefined, + volumes: [], + templates: [], + isOpen: true, + onCancel: jest.fn(), + onAccept: jest.fn() + }; + }); + + describe("when adding a new volume", () => { + describe("predefined by the product", () => { + it("does not allow settings the mount point", () => { + plainRender(); + expect(screen.queryByRole("textbox", { name: "Mount point" })).toBeNull(); + }); + }); + + describe("not predefined by the product", () => { + it("allows setting the mount point", async () => { + const { user } = plainRender(); + const mountPointInput = screen.getByRole("textbox", { name: "Mount point" }); + const submit = screen.getByRole("button", { name: "Accept" }); + await user.type(mountPointInput, "/var/log"); + await user.click(submit); + expect(props.onAccept).toHaveBeenCalledWith(expect.objectContaining({ mountPath: "/var/log" })); + }); + }); + + it("renders a file system picker if it allows more than one", async () => { + const { user } = plainRender(); + const fsTypeButton = screen.getByRole("button", { name: "File system type" }); + await user.click(fsTypeButton); + screen.getByRole("option", { name: "XFS" }); + screen.getByRole("option", { name: "Ext4" }); + }); + + it("does not render a file system picker when it accepts only one", async () => { + plainRender(); + await waitFor(() => ( + expect(screen.queryByRole("button", { name: "File system type" })).not.toBeInTheDocument()) + ); + }); + + it("renders 'Auto', 'Fixed', and 'Range' size options when volume supports auto size", () => { + plainRender(); + screen.getByRole("radio", { name: "Auto" }); + screen.getByRole("radio", { name: "Fixed" }); + screen.getByRole("radio", { name: "Range" }); + }); + + it("renders only 'Fixed' and 'Range' size options if volume does not support auto size", () => { + plainRender(); + expect(screen.queryByRole("radio", { name: "Auto" })).toBeNull(); + screen.getByRole("radio", { name: "Fixed" }); + screen.getByRole("radio", { name: "Range" }); + }); + + it("uses the min size unit as max size unit when it is missing", () => { + plainRender(); + const maxSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the maximum size" }); + expect(maxSizeUnitSelector).toHaveValue("TiB"); + }); + }); + + describe("when editing a volume", () => { + beforeEach(() => { + props = { ...props, volumes: [rootVolume, homeVolume, swapVolume, arbitraryVolume] }; + }); + + it("does not allow changing the mount point", () => { + const { rerender } = plainRender(); + expect(screen.queryByRole("textbox", { name: "Mount point" })).toBeNull(); + rerender(); + expect(screen.queryByRole("textbox", { name: "Mount point" })).toBeNull(); + }); + + it("renders a file system picker if it allows more than one", async () => { + const { user } = plainRender(); + const fsTypeButton = screen.getByRole("button", { name: "File system type" }); + await user.click(fsTypeButton); + screen.getByRole("option", { name: "XFS" }); + screen.getByRole("option", { name: "Ext4" }); + }); + + it("does not render a file system picker when it accepts only one", async () => { + plainRender(); + await waitFor(() => ( + expect(screen.queryByRole("button", { name: "File system type" })).not.toBeInTheDocument()) + ); + }); + + it("renders 'Auto', 'Fixed', and 'Range' size options when volume supports auto size", () => { + plainRender(); + screen.getByRole("radio", { name: "Auto" }); + screen.getByRole("radio", { name: "Fixed" }); + screen.getByRole("radio", { name: "Range" }); + }); + + it("renders only 'Fixed' and 'Range' size options if volume does not support auto size", () => { + plainRender(); + expect(screen.queryByRole("radio", { name: "Auto" })).toBeNull(); + screen.getByRole("radio", { name: "Fixed" }); + screen.getByRole("radio", { name: "Range" }); + }); + + it("uses the min size unit as max size unit when it is missing", () => { + plainRender(); + const maxSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the maximum size" }); + expect(maxSizeUnitSelector).toHaveValue("TiB"); + }); + }); + + it("calls the onAccept callback with resulting volume when the form is submitted", async () => { + const { user } = plainRender(); + const submit = screen.getByRole("button", { name: "Accept" }); + const rangeSize = screen.getByRole("radio", { name: "Range" }); + + await user.click(rangeSize); + + const minSizeInput = screen.getByRole("textbox", { name: "Minimum desired size" }); + const minSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the minimum size" }); + const minSizeGiBUnit = within(minSizeUnitSelector).getByRole("option", { name: "GiB" }); + const maxSizeInput = screen.getByRole("textbox", { name: "Maximum desired size" }); + const maxSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the maximum size" }); + const maxSizeGiBUnit = within(maxSizeUnitSelector).getByRole("option", { name: "GiB" }); + + await user.clear(minSizeInput); + await user.type(minSizeInput, "10"); + await user.selectOptions(minSizeUnitSelector, minSizeGiBUnit); + await user.clear(maxSizeInput); + await user.type(maxSizeInput, "25"); + await user.selectOptions(maxSizeUnitSelector, maxSizeGiBUnit); + + await user.click(submit); + + expect(props.onAccept).toHaveBeenCalledWith({ + ...rootVolume, + autoSize: false, + minSize: parseToBytes("10 GiB"), + maxSize: parseToBytes("25 GiB") + }); + }); + + it("does not call the onAccept callback when the form is not submitted", async () => { + const { user } = plainRender(); + const cancelButton = screen.getByRole("button", { name: "Cancel" }); + await user.click(cancelButton); + expect(props.onAccept).not.toHaveBeenCalled(); + expect(props.onCancel).toHaveBeenCalled(); + }); + + describe("mount point validations", () => { + it("warns and helps user when entered mount path included in a not existing but predefined volume", async () => { + const { user } = plainRender(); + const mountPointInput = screen.getByRole("textbox", { name: "Mount point" }); + await user.type(mountPointInput, "/home"); + await screen.findByText("There is a predefined file system for /home."); + const addButton = screen.getByRole("button", { name: "Do you want to add it?" }); + await user.click(addButton); + screen.getByRole("heading", { name: "Add /home file system" }); + expect(screen.queryByRole("textbox", { name: "Mount point" })).toBeNull(); + screen.getByText("/home"); + }); + + it("warns and helps user when entered mount path including in an existing volume", async () => { + const { user } = plainRender(); + const mountPointInput = screen.getByRole("textbox", { name: "Mount point" }); + await user.type(mountPointInput, "swap"); + await screen.findByText("There is already a file system for swap."); + const editButton = screen.getByRole("button", { name: "Do you want to edit it?" }); + await user.click(editButton); + screen.getByRole("heading", { name: "Edit swap file system" }); + expect(screen.queryByRole("textbox", { name: "Mount point" })).toBeNull(); + screen.getByText("swap"); + }); + + it("renders an error if a not valid path was given", async () => { + const { user } = plainRender(); + const mountPointInput = screen.getByRole("textbox", { name: "Mount point" }); + const submitButton = screen.getByRole("button", { name: "Accept" }); + + // No mount point given + await user.click(submitButton); + screen.getByText("A mount point is required"); + + // Without starting backslash + await user.clear(mountPointInput); + await user.type(mountPointInput, "var/log"); + await user.click(submitButton); + screen.getByText("The mount point is invalid"); + + // With more than one leading backslash + await user.clear(mountPointInput); + await user.type(mountPointInput, "//var/log/"); + await user.click(submitButton); + screen.getByText("The mount point is invalid"); + + // With more than one trailing backslash + await user.clear(mountPointInput); + await user.type(mountPointInput, "/var/log//"); + await user.click(submitButton); + screen.getByText("The mount point is invalid"); + }); + }); + + describe("size validations", () => { + describe("when 'Fixed' size is selected", () => { + it("renders an error when size is not given", async () => { + const { user } = plainRender(); + const submitButton = screen.getByRole("button", { name: "Accept" }); + const fixedSizeOption = screen.getByRole("radio", { name: "Fixed" }); + await user.click(fixedSizeOption); + const sizeInput = screen.getByRole("textbox", { name: "Exact size" }); + await user.clear(sizeInput); + await user.click(submitButton); + screen.getByText("A size value is required"); + await user.type(sizeInput, "10"); + await user.click(submitButton); + expect(screen.queryByText("A size value is required")).toBeNull(); + }); + }); + + describe("when 'Range' size is selected", () => { + it("renders an error when min size is not given", async () => { + const { user } = plainRender(); + + const submitButton = screen.getByRole("button", { name: "Accept" }); + const rangeSize = screen.getByRole("radio", { name: "Range" }); + await user.click(rangeSize); + + const minSizeInput = screen.getByRole("textbox", { name: "Minimum desired size" }); + + await user.clear(minSizeInput); + await user.click(submitButton); + screen.getByText("Minimum size is required"); + + await user.type(minSizeInput, "10"); + await user.click(submitButton); + expect(screen.queryByText("Minimum size is required")).toBeNull(); + }); + + it("renders an error when max size is smaller than or equal to min size", async () => { + const volume = { ...homeVolume, minSize: undefined, maxSize: undefined }; + const { user } = plainRender(); + const submitButton = screen.getByRole("button", { name: "Accept" }); + const rangeSize = screen.getByRole("radio", { name: "Range" }); + await user.click(rangeSize); + + const minSizeInput = screen.getByRole("textbox", { name: "Minimum desired size" }); + const minSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the minimum size" }); + const minSizeMiBUnit = within(minSizeUnitSelector).getByRole("option", { name: "MiB" }); + const maxSizeInput = screen.getByRole("textbox", { name: "Maximum desired size" }); + const maxSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the maximum size" }); + const maxSizeGiBUnit = within(maxSizeUnitSelector).getByRole("option", { name: "GiB" }); + const maxSizeMiBUnit = within(maxSizeUnitSelector).getByRole("option", { name: "MiB" }); + + // Max (11 GiB) > Min (10 GiB) BEFORE changing any unit size + await user.clear(minSizeInput); + await user.type(minSizeInput, "10"); + await user.clear(maxSizeInput); + await user.type(maxSizeInput, "11"); + await user.click(submitButton); + expect(screen.queryByText("Maximum must be greater than minimum")).toBeNull(); + + // Max (10 GiB) === Min (10 GiB) + await user.clear(maxSizeInput); + await user.type(maxSizeInput, "10"); + await user.click(submitButton); + screen.getByText("Maximum must be greater than minimum"); + + // Max (10 MiB) < Min (10 GiB) because choosing a lower size unit + await user.selectOptions(maxSizeUnitSelector, maxSizeMiBUnit); + await user.click(submitButton); + screen.getByText("Maximum must be greater than minimum"); + + // Max (9 MiB) < Min (10 MiB) because choosing a lower size + await user.selectOptions(minSizeUnitSelector, minSizeMiBUnit); + await user.clear(maxSizeInput); + await user.type(maxSizeInput, "9"); + await user.click(submitButton); + screen.getByText("Maximum must be greater than minimum"); + + // Max (11 MiB) > Min (10 MiB) + await user.clear(maxSizeInput); + await user.type(maxSizeInput, "11"); + await user.selectOptions(maxSizeUnitSelector, maxSizeGiBUnit); + }); + }); + }); +}); diff --git a/web/src/components/storage/VolumeForm.jsx b/web/src/components/storage/VolumeFields.jsx similarity index 51% rename from web/src/components/storage/VolumeForm.jsx rename to web/src/components/storage/VolumeFields.jsx index dbf4382a0c..66ce0b230e 100644 --- a/web/src/components/storage/VolumeForm.jsx +++ b/web/src/components/storage/VolumeFields.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2023-2024] SUSE LLC + * Copyright (c) [2024] SUSE LLC * * All Rights Reserved. * @@ -21,29 +21,57 @@ // @ts-check -import React, { useReducer, useState } from "react"; +import React, { useState } from "react"; import { - InputGroup, InputGroupItem, Form, FormGroup, FormSelect, FormSelectOption, MenuToggle, - Popover, Radio, Select, SelectOption, SelectList + InputGroup, InputGroupItem, FormGroup, FormSelect, FormSelectOption, MenuToggle, Popover, Radio, + Select, SelectOption, SelectList, TextInput } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; import { _, N_ } from "~/i18n"; -import { FormValidationError, If, NumericTextInput } from '~/components/core'; -import { DEFAULT_SIZE_UNIT, SIZE_METHODS, SIZE_UNITS, parseToBytes, splitSize } from '~/components/storage/utils'; +import { FormValidationError, FormReadOnlyField, If, NumericTextInput } from '~/components/core'; import { Icon } from "~/components/layout"; +import { SIZE_METHODS, SIZE_UNITS } from '~/components/storage/utils'; /** * @typedef {import ("~/client/storage").Volume} Volume */ /** - * Callback function for notifying a form input change + * Field for the mount path of a volume. + * @component + * + * @typedef {object} MountPathFieldProps + * @property {string} [value=""] + * @property {boolean} [isReadOnly=false] + * @property {(mountPath: string) => void} onChange + * @property {React.ReactNode} [error] * - * @callback onChangeFn - * @param {object} an object with the changed input and its new value - * @return {void} + * @param {MountPathFieldProps} props */ +const MountPathField = ({ value = "", onChange, isReadOnly = false, error }) => { + const label = _("Mount point"); + /** @type {(_: any, mountPath: string) => void} */ + const changeMountPath = (_, mountPath) => onChange(mountPath); + + if (isReadOnly) { + return {value}; + } + + return ( + + + + + ); +}; /** * Form control for selecting a size unit @@ -57,72 +85,14 @@ import { Icon } from "~/components/layout"; */ const SizeUnitFormSelect = ({ units, ...formSelectProps }) => { return ( - + {/* the unit values are marked for translation in the utils.js file */} {/* eslint-disable-next-line agama-i18n/string-literals */} - { units.map(unit => ) } + {units.map(unit => )} ); }; -/** - * Form control for selecting a mount point - * @component - * - * Based on {@link PF/FormSelect https://www.patternfly.org/components/forms/form-select} - * - * @param {object} props - * @param {string} props.value - mountPath of current selected volume - * @param {Array} props.volumes - a collection of storage volumes - * @param {onChangeFn} props.onChange - callback for notifying input changes - * @param {import("@patternfly/react-core").SelectProps} [props.selectProps] -*/ - -const MountPointFormSelect = ({ value, volumes, onChange, ...selectProps }) => { - const [isOpen, setIsOpen] = useState(false); - - const onSelect = (_, mountPath) => { - setIsOpen(false); - onChange(mountPath); - }; - - const onToggleClick = () => { - setIsOpen(!isOpen); - }; - - const toggle = toggleRef => { - return ( - - {value || _("Select a value")} - - ); - }; - return ( - - ); -}; - /** * Possible file system type options for a volume. * @function @@ -157,9 +127,10 @@ const FsSelectOption = ({ fsOption }) => { * @param {string} props.id - Widget id. * @param {string} props.value - Currently selected file system. * @param {Volume} props.volume - The selected storage volume. - * @param {onChangeFn} props.onChange - Callback for notifying input changes. + * @param {boolean} props.isDisabled + * @param {(data: object) => void} props.onChange - Callback for notifying input changes. */ -const FsSelect = ({ id, value, volume, onChange }) => { +const FsSelect = ({ id, value, volume, isDisabled, onChange }) => { const [isOpen, setIsOpen] = useState(false); const options = fsOptions(volume); @@ -171,7 +142,7 @@ const FsSelect = ({ id, value, volume, onChange }) => { const onSelect = (_event, option) => { setIsOpen(false); - onChange({ fsType: option, snapshots: false }); + onChange({ fsType: option }); }; const toggle = toggleRef => { @@ -182,6 +153,7 @@ const FsSelect = ({ id, value, volume, onChange }) => { onClick={onToggleClick} isExpanded={isOpen} className="full-width" + isDisabled={isDisabled} > {selected} @@ -213,12 +185,15 @@ const FsSelect = ({ id, value, volume, onChange }) => { * text with the unique option. * @component * - * @param {object} props - * @param {string} props.value - Currently selected file system. - * @param {Volume} props.volume - The selected storage volume. - * @param {onChangeFn} props.onChange - Callback for notifying input changes. + * @typedef {object} FsFieldProps + * @property {string} value - Currently selected file system. + * @property {Volume} volume - The selected storage volume. + * @property {boolean} [isDisabled=false] - Whether the field is disabled or not. + * @property {(data: object) => void} onChange - Callback for notifying input changes. + * + * @param {FsFieldProps} props */ -const FsField = ({ value, volume, onChange }) => { +const FsField = ({ value, volume, isDisabled = false, onChange }) => { const isSingleFs = () => { // check for btrfs with snapshots if (volume.fsType === "Btrfs" && volume.snapshots) { @@ -250,20 +225,20 @@ const FsField = ({ value, volume, onChange }) => { // TRANSLATORS: label for the file system selector. const label = _("File system type"); + if (isSingleFs()) { + return {value}; + } + return ( - -

{value}

- - } - else={ - } fieldId="fsType"> - - - } - /> + } fieldId="fsType"> + + ); }; @@ -312,9 +287,10 @@ const SizeAuto = ({ volume }) => { * @param {object} props * @param {object} props.errors - the form errors * @param {object} props.formData - the form data - * @param {onChangeFn} props.onChange - callback for notifying input changes + * @param {boolean} props.isDisabled + * @param {(v: object) => void} props.onChange - callback for notifying input changes */ -const SizeManual = ({ errors, formData, onChange }) => { +const SizeManual = ({ errors, formData, isDisabled, onChange }) => { return (

@@ -340,6 +316,7 @@ const SizeManual = ({ errors, formData, onChange }) => { value={formData.size} onChange={(size) => onChange({ size })} validated={errors.size && 'error'} + isDisabled={isDisabled} /> @@ -349,8 +326,9 @@ const SizeManual = ({ errors, formData, onChange }) => { // TRANSLATORS: units selector (like KiB, MiB, GiB...) aria-label={_("Size unit")} units={Object.values(SIZE_UNITS)} - value={formData.sizeUnit } + value={formData.sizeUnit} onChange={(_, sizeUnit) => onChange({ sizeUnit })} + isDisabled={isDisabled} /> @@ -367,9 +345,10 @@ const SizeManual = ({ errors, formData, onChange }) => { * @param {object} props * @param {object} props.errors - the form errors * @param {object} props.formData - the form data - * @param {onChangeFn} props.onChange - callback for notifying input changes + * @param {boolean} props.isDisabled + * @param {(v: object) => void} props.onChange - callback for notifying input changes */ -const SizeRange = ({ errors, formData, onChange }) => { +const SizeRange = ({ errors, formData, isDisabled, onChange }) => { return (

@@ -395,6 +374,7 @@ and maximum. If no maximum is given then the file system will be as big as possi value={formData.minSize} onChange={(minSize) => onChange({ minSize })} validated={errors.minSize && 'error'} + isDisabled={isDisabled} /> @@ -403,8 +383,9 @@ and maximum. If no maximum is given then the file system will be as big as possi id="minSizeUnit" aria-label={_("Unit for the minimum size")} units={Object.values(SIZE_UNITS)} - value={formData.minSizeUnit } + value={formData.minSizeUnit} onChange={(_, minSizeUnit) => onChange({ minSizeUnit })} + isDisabled={isDisabled} /> @@ -427,6 +408,7 @@ and maximum. If no maximum is given then the file system will be as big as possi aria-label={_("Maximum desired size")} value={formData.maxSize} onChange={(maxSize) => onChange({ maxSize })} + isDisabled={isDisabled} /> @@ -435,8 +417,9 @@ and maximum. If no maximum is given then the file system will be as big as possi id="maxSizeUnit" aria-label={_("Unit for the maximum size")} units={Object.values(SIZE_UNITS)} - value={formData.maxSizeUnit || formData.minSizeUnit } + value={formData.maxSizeUnit || formData.minSizeUnit} onChange={(_, maxSizeUnit) => onChange({ maxSizeUnit })} + isDisabled={isDisabled} /> @@ -461,15 +444,18 @@ const SIZE_OPTION_LABELS = Object.freeze({ * Widget for rendering the volume size options * @component * - * @param {object} props - * @param {object} props.errors - the form errors - * @param {object} props.formData - the form data - * @param {Volume} props.volume - the selected storage volume - * @param {onChangeFn} props.onChange - callback for notifying input changes + * @typedef {object} SizeOptionsFieldProps + * @property {Volume} volume - the selected storage volume + * @property {object} formData - the form data + * @property {object} [errors={}] - the form errors + * @property {boolean} [isDisabled=false] - Whether the field options are disabled or not. + * @property {(v: object) => void} onChange - callback for notifying input changes + * + * @param {SizeOptionsFieldProps} props */ -const SizeOptions = ({ errors, formData, volume, onChange }) => { +const SizeOptionsField = ({ volume, formData, isDisabled = false, errors = {}, onChange }) => { const { sizeMethod } = formData; - const sizeWidgetProps = { errors, formData, volume, onChange }; + const sizeWidgetProps = { errors, formData, volume, isDisabled, onChange }; /** @type {string[]} */ const sizeOptions = [SIZE_METHODS.MANUAL, SIZE_METHODS.RANGE]; @@ -477,255 +463,37 @@ const SizeOptions = ({ errors, formData, volume, onChange }) => { if (volume.outline.supportAutoSize) sizeOptions.push(SIZE_METHODS.AUTO); return ( -

-
- { sizeOptions.map((value) => { - const isSelected = sizeMethod === value; - - return ( - onChange({ sizeMethod: value })} - /> - ); - })} -
- -
- } /> - } /> - } /> + +
+
+ {sizeOptions.map((value) => { + const isSelected = sizeMethod === value; + + return ( + onChange({ sizeMethod: value })} + isDisabled={isDisabled} + /> + ); + })} +
+ +
+ } /> + } /> + } /> +
-
+ ); }; -/** - * Creates a new storage volume object based on given params - * - * @param {Volume} volume - a storage volume - * @param {object} formData - data used to calculate the volume updates - * @returns {object} storage volume object - */ -const createUpdatedVolume = (volume, formData) => { - let sizeAttrs = {}; - const size = parseToBytes(`${formData.size} ${formData.sizeUnit}`); - const minSize = parseToBytes(`${formData.minSize} ${formData.minSizeUnit}`); - const maxSize = parseToBytes(`${formData.maxSize} ${formData.maxSizeUnit}`); - - switch (formData.sizeMethod) { - case SIZE_METHODS.AUTO: - sizeAttrs = { minSize: undefined, maxSize: undefined, autoSize: true }; - break; - case SIZE_METHODS.MANUAL: - sizeAttrs = { minSize: size, maxSize: size, autoSize: false }; - break; - case SIZE_METHODS.RANGE: - sizeAttrs = { minSize, maxSize: formData.maxSize ? maxSize : undefined, autoSize: false }; - break; - } - - const { fsType, snapshots } = formData; - - return { ...volume, ...sizeAttrs, fsType, snapshots }; -}; - -/** - * Form-related helper for guessing the size method for given volume - * - * @param {Volume} volume - a storage volume - * @return {string} corresponding size method - */ -const sizeMethodFor = (volume) => { - const { autoSize, minSize, maxSize } = volume; - - if (autoSize) { - return SIZE_METHODS.AUTO; - } else if (minSize !== maxSize) { - return SIZE_METHODS.RANGE; - } else { - return SIZE_METHODS.MANUAL; - } -}; - -/** - * Form-related helper for preparing data based on given volume - * - * @param {Volume} volume - a storage volume object - * @return {object} an object ready to be used as a "form state" - */ -const prepareFormData = (volume) => { - const { size: minSize = "", unit: minSizeUnit = DEFAULT_SIZE_UNIT } = splitSize(volume.minSize); - const { size: maxSize = "", unit: maxSizeUnit = minSizeUnit || DEFAULT_SIZE_UNIT } = splitSize(volume.maxSize); - - return { - size: minSize, - sizeUnit: minSizeUnit, - minSize, - minSizeUnit, - maxSize, - maxSizeUnit, - sizeMethod: sizeMethodFor(volume), - mountPoint: volume.mountPath, - fsType: volume.fsType, - snapshots: volume.snapshots - }; -}; - -/** - * Initializer function for the React#useReducer used in the {@link VolumesForm} - * - * @param {Volume} volume - a storage volume object - * @returns {object} a ready to use initial state - */ -const createInitialState = (volume) => { - return { - volume, - formData: prepareFormData(volume), - errors: {} - }; -}; - -/** - * The VolumeForm reducer - */ -const reducer = (state, action) => { - const { type, payload } = action; - - switch (type) { - case "CHANGE_VOLUME": { - return createInitialState(payload.volume); - } - - case "UPDATE_DATA": { - return { - ...state, - formData: { - ...state.formData, - ...payload - } - }; - } - - case "SET_ERRORS": { - return { ...state, errors: payload }; - } - - default: { - return state; - } - } -}; - -/** - * Form used for adding a new file system from a list of templates - * @component - * - * @note VolumeForm does not provide a submit button. It is the consumer's - * responsibility to provide both: the button for triggering the submission by - * using the form id and the callback function used to perform the submission - * once the form has been validated. - * - * @param {object} props - * @param {string} props.id - Form ID - * @param {Volume} [props.volume] - Volume if editing - * @param {Volume[]} props.templates - * @param {onSubmitFn} props.onSubmit - Function to use for submitting a new volume - * - * @callback onSubmitFn - * @param {Volume} volume - a storage volume object - * @return {void} - */ -export default function VolumeForm({ id, volume: currentVolume, templates = [], onSubmit }) { - /** @type {[object, (action: object) => void]} */ - const [state, dispatch] = useReducer(reducer, currentVolume || templates[0], createInitialState); - - const changeVolume = (mountPath) => { - const volume = templates.find(t => t.mountPath === mountPath); - dispatch({ type: "CHANGE_VOLUME", payload: { volume } }); - }; - - const updateData = (data) => dispatch({ type: "UPDATE_DATA", payload: data }); - - const validateVolumeSize = (sizeMethod, volume) => { - const errors = {}; - const { minSize, maxSize } = volume; - - switch (sizeMethod) { - case SIZE_METHODS.AUTO: - break; - case SIZE_METHODS.MANUAL: - if (!minSize) { - errors.size = _("A size value is required"); - } - break; - case SIZE_METHODS.RANGE: - if (!minSize) { - errors.minSize = _("Minimum size is required"); - } - - if (maxSize !== -1 && maxSize <= minSize) { - errors.maxSize = _("Maximum must be greater than minimum"); - } - break; - } - - return errors; - }; - - const submitForm = (e) => { - e.preventDefault(); - const { volume: originalVolume, formData } = state; - const volume = createUpdatedVolume(originalVolume, formData); - const errors = validateVolumeSize(formData.sizeMethod, volume); - - dispatch({ type: "SET_ERRORS", payload: errors }); - - if (!Object.keys(errors).length) onSubmit(volume); - }; - - const { fsType } = state.formData; - - const ShowMountPointSelector = () => ( - - ); - - const ShowMountPoint = () =>

{state.formData.mountPoint}

; - - return ( -
- - - - } - else={ - - - - } - /> - - - - - - ); -} +export { FsField, MountPathField, SizeOptionsField }; diff --git a/web/src/components/storage/VolumeFields.test.jsx b/web/src/components/storage/VolumeFields.test.jsx new file mode 100644 index 0000000000..84e30153e8 --- /dev/null +++ b/web/src/components/storage/VolumeFields.test.jsx @@ -0,0 +1,195 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * 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. + */ + +// @ts-check + +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { SIZE_METHODS } from '~/components/storage/utils'; +import { FsField, MountPathField, SizeOptionsField } from "~/components/storage/VolumeFields"; + +/** + * @typedef {import ("~/client/storage").Volume} Volume + */ + +/** @type {Volume} */ +const volume = { + mountPath: "/home", + target: "DEFAULT", + fsType: "XFS", + minSize: 1024, + maxSize: 4096, + autoSize: false, + snapshots: false, + transactional: false, + outline: { + required: false, + fsTypes: ["Ext4", "XFS"], + supportAutoSize: false, + snapshotsConfigurable: false, + snapshotsAffectSizes: false, + adjustByRam: false, + sizeRelevantVolumes: [], + productDefined: true + } +}; + +const callbackFn = jest.fn(); + +describe("MountPathField", () => { + it("renders a text input with given value", () => { + const { rerender } = plainRender(); + const input = screen.getByRole("textbox", { name: "Mount point" }); + expect(input).toHaveValue("/home"); + rerender(); + expect(input).toHaveValue("/var"); + }); + + it("renders given error", () => { + plainRender(); + screen.getByText("Something went wrong"); + }); + + it("calls given callback when user changes its content", async () => { + const { user } = plainRender(); + const input = screen.getByRole("textbox", { name: "Mount point" }); + // NOTE: MountPathField is a controlled component. That makes a bit more + // difficult to write more sensible test here by typing "/var" and expecting that + // callback is called multiple times with "/", "/v", "/va", and "/var". It + // will not work because it's actually triggering the onChange with "/", + // "v", "a", and "r", but having a different "value" each time it's + // re-rendered. Anyway, checking that callback is called is enough. + await user.type(input, "/"); + expect(callbackFn).toHaveBeenCalledWith("/"); + }); + + it("renders only the value if mount as read-only (no input)", () => { + plainRender(); + expect(screen.queryByRole("textbox", { name: "Mount point" })).toBeNull(); + screen.getByText("/home"); + }); +}); + +describe("SizeOptionsField", () => { + it("renders radio group with sizing options", () => { + plainRender(); + screen.getByRole("radiogroup", { name: "Size" }); + }); + + it("renders 'Fixed' and 'Range' options always but 'Auto' only if volume accepts auto size", () => { + const { rerender } = plainRender( + + ); + const group = screen.getByRole("radiogroup", { name: "Size" }); + within(group).getByRole("radio", { name: "Fixed" }); + within(group).getByRole("radio", { name: "Range" }); + expect(within(group).queryByRole("radio", { name: "Auto" })).toBeNull(); + + rerender( + + ); + within(group).getByRole("radio", { name: "Auto" }); + within(group).getByRole("radio", { name: "Fixed" }); + within(group).getByRole("radio", { name: "Range" }); + }); + + it("renders options as disabled when isDisabled props is given", () => { + plainRender( + + ); + const group = screen.getByRole("radiogroup", { name: "Size" }); + within(group).getAllByRole("radio") + .forEach(option => expect(option).toBeDisabled()); + }); + + it("calls given callback when user changes selected option", async () => { + const { user } = plainRender( + + ); + const group = screen.getByRole("radiogroup", { name: "Size" }); + const rangeOption = within(group).getByRole("radio", { name: "Range" }); + await user.click(rangeOption); + expect(callbackFn).toHaveBeenCalledWith({ sizeMethod: SIZE_METHODS.RANGE }); + }); + + it.todo("test SizeAuto internal component"); + it.todo("test SizeManual internal component"); + it.todo("test SizeRange internal component"); +}); + +describe("FsField", () => { + it("renders control for selecting a file system, with the given value as initial selection", async () => { + const { user, rerender } = plainRender( + + ); + let button = screen.getByRole("button", { name: "File system type" }); + await user.click(button); + const xfsOption = screen.getByRole("option", { name: "XFS" }); + let ext4Option = screen.getByRole("option", { name: "Ext4" }); + expect(xfsOption).toHaveAttribute("aria-selected", "true"); + expect(ext4Option).toHaveAttribute("aria-selected", "false"); + expect(screen.queryByRole("option", { name: "Btrfs" })).toBeNull(); + + rerender( + + ); + button = screen.getByRole("button", { name: "File system type" }); + await user.click(button); + ext4Option = screen.getByRole("option", { name: "Ext4" }); + const btrfsOption = screen.getByRole("option", { name: "Btrfs" }); + expect(ext4Option).toHaveAttribute("aria-selected", "true"); + expect(btrfsOption).toHaveAttribute("aria-selected", "false"); + expect(screen.queryByRole("option", { name: "XFS" })).toBeNull(); + }); + + it("renders control as disabled when isDisabled is given", () => { + plainRender( + + ); + const button = screen.getByRole("button", { name: "File system type" }); + expect(button).toBeDisabled(); + }); + + it("calls given callback when user clicks on an option", async () => { + const { user } = plainRender( + + ); + const button = screen.getByRole("button", { name: "File system type" }); + await user.click(button); + const ext4Option = screen.getByRole("option", { name: "Ext4" }); + await user.click(ext4Option); + expect(callbackFn).toHaveBeenCalledWith(expect.objectContaining({ fsType: "Ext4" })); + }); +}); diff --git a/web/src/components/storage/VolumeForm.test.jsx b/web/src/components/storage/VolumeForm.test.jsx deleted file mode 100644 index a3ca147f48..0000000000 --- a/web/src/components/storage/VolumeForm.test.jsx +++ /dev/null @@ -1,339 +0,0 @@ -/* - * Copyright (c) [2022-2023] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * 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, waitFor, within } from "@testing-library/react"; -import { plainRender } from "~/test-utils"; -import { DEFAULT_SIZE_UNIT, parseToBytes } from "~/components/storage/utils"; -import { VolumeForm } from "~/components/storage"; - -const volumes = { - root: { - mountPath: "/", - fsType: "Btrfs", - minSize: 1024, - maxSize: 2048, - autoSize: false, - snapshots: true, - outline: { - required: true, - fsTypes: ["Btrfs", "Ext4"], - supportAutoSize: true, - snapshotsConfigurable: true, - snapshotsAffectSizes: true, - sizeRelevantVolumes: [] - } - }, - swap: { - mountPath: "swap", - fsType: "Swap", - minSize: 1024, - maxSize: 1024, - autoSize: false, - snapshots: false, - outline: { - required: false, - fsTypes: ["Swap"], - supportAutoSize: false, - snapshotsConfigurable: false, - snapshotsAffectSizes: false, - sizeRelevantVolumes: [] - } - }, - home: { - mountPath: "/home", - fsType: "XFS", - minSize: 1024, - autoSize: false, - snapshots: false, - outline: { - required: false, - fsTypes: ["Ext4", "XFS"], - supportAutoSize: false, - snapshotsConfigurable: false, - snapshotsAffectSizes: false, - sizeRelevantVolumes: [] - } - } -}; - -const onSubmitFn = jest.fn(); - -// TL;DR, the form does not provide a submit button by itself. -// Refers the VolumeForm documentation. -const VolumeFormWrapper = ({ volume, onSubmit }) => { - return ( - <> - - - - ); -}; - -let props; - -beforeEach(() => { - props = { templates: [volumes.root, volumes.home, volumes.swap] }; -}); - -it("renders a control for displaying/selecting the file system type", async () => { - // use home which is not using snapshots - const { user } = plainRender(); - - const fsTypeButton = screen.getByRole("button", { name: "File system type" }); - await user.click(fsTypeButton); - screen.getByRole("option", { name: "XFS" }); - screen.getByRole("option", { name: "Ext4" }); -}); - -it("does not render the file system control if there is only one option", async () => { - const { user } = plainRender(); - - const mountPointButton = screen.getByRole("button", { name: "Mount point" }); - await user.click(mountPointButton); - const swap = screen.getByRole("option", { name: "swap" }); - await user.click(swap); - await screen.findByText("Swap"); - await waitFor(() => ( - expect(screen.queryByRole("button", { name: "File system type" })).not.toBeInTheDocument()) - ); -}); - -it("renders the file system control for root mount point without snapshots", async () => { - const { user } = plainRender(); - - const fsTypeButton = screen.getByRole("button", { name: "File system type" }); - await user.click(fsTypeButton); - screen.getByRole("option", { name: "Btrfs" }); - screen.getByRole("option", { name: "Ext4" }); -}); - -it("does not render the file system control for root mount point with btrfs with snapshots", async () => { - plainRender(); - - await screen.findByText("Btrfs"); - await waitFor(() => ( - expect(screen.queryByRole("button", { name: "File system type" })).not.toBeInTheDocument()) - ); -}); - -it("renders controls for setting the desired size", () => { - plainRender(); - - screen.getByRole("radio", { name: "Auto" }); - screen.getByRole("radio", { name: "Fixed" }); - screen.getByRole("radio", { name: "Range" }); -}); - -it("uses the default size unit when min size unit is missing", () => { - plainRender(); - - const maxSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the maximum size" }); - expect(maxSizeUnitSelector).toHaveValue(DEFAULT_SIZE_UNIT); -}); - -it("uses the min size unit as max size unit when it is missing", () => { - plainRender(); - - const maxSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the maximum size" }); - expect(maxSizeUnitSelector).toHaveValue("TiB"); -}); - -it("renders the 'Auto' size option only when a volume with 'adaptive sizes' is selected", async () => { - const { user } = plainRender(); - - // We know that first volume (root in this example) is selected. And we know - // that it's configured for allowing adaptive sizes too. - screen.getByRole("radio", { name: "Auto" }); - - const mountPointButton = screen.getByRole("button", { name: "Mount point" }); - await user.click(mountPointButton); - - // And we know that /home volume is not set to allow adaptive sizes. Thus, - // let's select it. - const homeVolumeOption = screen.getByRole("option", { name: "/home" }); - await user.click(homeVolumeOption); - - const autoSizeOption = screen.queryByRole("radio", { name: "Auto" }); - expect(autoSizeOption).toBeNull(); -}); - -it("calls the onSubmit callback with resulting volume when the form is submitted", async () => { - const { user } = plainRender(); - const submitForm = screen.getByRole("button", { name: "Submit VolumeForm" }); - const rangeSize = screen.getByRole("radio", { name: "Range" }); - - await user.click(rangeSize); - - const minSizeInput = screen.getByRole("textbox", { name: "Minimum desired size" }); - const minSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the minimum size" }); - const minSizeGiBUnit = within(minSizeUnitSelector).getByRole("option", { name: "GiB" }); - const maxSizeInput = screen.getByRole("textbox", { name: "Maximum desired size" }); - const maxSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the maximum size" }); - const maxSizeGiBUnit = within(maxSizeUnitSelector).getByRole("option", { name: "GiB" }); - - await user.clear(minSizeInput); - await user.type(minSizeInput, "10"); - await user.selectOptions(minSizeUnitSelector, minSizeGiBUnit); - await user.clear(maxSizeInput); - await user.type(maxSizeInput, "25"); - await user.selectOptions(maxSizeUnitSelector, maxSizeGiBUnit); - - // root is with btrfs and snapshots, so it is not possible to select it - await screen.findByText("Btrfs"); - await waitFor(() => ( - expect(screen.queryByRole("button", { name: "File system type" })).not.toBeInTheDocument()) - ); - - await user.click(submitForm); - - expect(onSubmitFn).toHaveBeenCalledWith({ - ...volumes.root, - minSize: parseToBytes("10 GiB"), - maxSize: parseToBytes("25 GiB") - }); -}); - -describe("size validations", () => { - describe("when 'Fixed' size is selected", () => { - beforeEach(() => { props.volume = volumes.home }); - - it("renders an error when size is not given", async () => { - const { user } = plainRender(); - - const submitForm = screen.getByRole("button", { name: "Submit VolumeForm" }); - const manualSize = screen.getByRole("radio", { name: "Fixed" }); - await user.click(manualSize); - - const sizeInput = screen.getByRole("textbox", { name: "Exact size" }); - await user.clear(sizeInput); - await user.click(submitForm); - screen.getByText("A size value is required"); - - await user.type(sizeInput, "10"); - await user.click(submitForm); - expect(screen.queryByText("A size value is required")).toBeNull(); - }); - }); - - describe("when 'Range' size is selected", () => { - beforeEach(() => { props.volume = volumes.home }); - - it("renders an error when min size is not given", async () => { - const { user } = plainRender(); - - const submitForm = screen.getByRole("button", { name: "Submit VolumeForm" }); - const rangeSize = screen.getByRole("radio", { name: "Range" }); - await user.click(rangeSize); - - const minSizeInput = screen.getByRole("textbox", { name: "Minimum desired size" }); - - await user.clear(minSizeInput); - await user.click(submitForm); - screen.getByText("Minimum size is required"); - - await user.type(minSizeInput, "10"); - await user.click(submitForm); - expect(screen.queryByText("Minimum size is required")).toBeNull(); - }); - - it("renders an error when max size is smaller than or equal to min size", async () => { - // Let's start the test without predefined sizes - const volume = { ...volumes.home, minSize: "", maxSize: "" }; - const { user } = plainRender(); - - const submitForm = screen.getByRole("button", { name: "Submit VolumeForm" }); - const rangeSize = screen.getByRole("radio", { name: "Range" }); - await user.click(rangeSize); - - const minSizeInput = screen.getByRole("textbox", { name: "Minimum desired size" }); - const minSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the minimum size" }); - const minSizeMiBUnit = within(minSizeUnitSelector).getByRole("option", { name: "MiB" }); - const maxSizeInput = screen.getByRole("textbox", { name: "Maximum desired size" }); - const maxSizeUnitSelector = screen.getByRole("combobox", { name: "Unit for the maximum size" }); - const maxSizeGiBUnit = within(maxSizeUnitSelector).getByRole("option", { name: "GiB" }); - const maxSizeMiBUnit = within(maxSizeUnitSelector).getByRole("option", { name: "MiB" }); - - // Max (11 GiB) > Min (10 GiB) BEFORE changing any unit size - await user.clear(minSizeInput); - await user.type(minSizeInput, "10"); - await user.clear(maxSizeInput); - await user.type(maxSizeInput, "11"); - await user.click(submitForm); - expect(screen.queryByText("Maximum must be greater than minimum")).toBeNull(); - - // Max (10 GiB) === Min (10 GiB) - await user.clear(maxSizeInput); - await user.type(maxSizeInput, "10"); - await user.click(submitForm); - screen.getByText("Maximum must be greater than minimum"); - - // Max (10 MiB) < Min (10 GiB) because choosing a lower size unit - await user.selectOptions(maxSizeUnitSelector, maxSizeMiBUnit); - await user.click(submitForm); - screen.getByText("Maximum must be greater than minimum"); - - // Max (9 MiB) < Min (10 MiB) because choosing a lower size - await user.selectOptions(minSizeUnitSelector, minSizeMiBUnit); - await user.clear(maxSizeInput); - await user.type(maxSizeInput, "9"); - await user.click(submitForm); - screen.getByText("Maximum must be greater than minimum"); - - // Max (11 MiB) > Min (10 MiB) - await user.clear(maxSizeInput); - await user.type(maxSizeInput, "11"); - await user.selectOptions(maxSizeUnitSelector, maxSizeGiBUnit); - }); - }); -}); - -describe("when editing a new volume", () => { - beforeEach(() => { props.volume = volumes.root }); - - it("renders the mount point", () => { - plainRender(); - - screen.getByText("/"); - }); - - it("does not render a control for changing the mount point", async () => { - plainRender(); - await waitFor(() => ( - expect(screen.queryByRole("button", { name: "Mount point" })).not.toBeInTheDocument() - )); - }); -}); - -describe("when adding a new volume", () => { - it("renders a control for setting the mount point", async () => { - const { user } = plainRender(); - - let mountPointButton = screen.getByRole("button", { name: "Mount point" }); - await user.click(mountPointButton); - screen.getByRole("option", { name: "/" }); - screen.getByRole("option", { name: "swap" }); - const homeMountPoint = screen.getByRole("option", { name: "/home" }); - await user.click(homeMountPoint); - mountPointButton = screen.getByRole("button", { name: "Mount point" }); - await within(mountPointButton).findByText("/home"); - }); -}); diff --git a/web/src/components/storage/VolumeLocationDialog.test.jsx b/web/src/components/storage/VolumeLocationDialog.test.jsx index e89367209e..81c4cc823d 100644 --- a/web/src/components/storage/VolumeLocationDialog.test.jsx +++ b/web/src/components/storage/VolumeLocationDialog.test.jsx @@ -118,7 +118,8 @@ const volume = { snapshotsConfigurable: true, snapshotsAffectSizes: true, sizeRelevantVolumes: [], - adjustByRam: false + adjustByRam: false, + productDefined: true } }; diff --git a/web/src/components/storage/index.js b/web/src/components/storage/index.js index 37821821ec..763840a3b7 100644 --- a/web/src/components/storage/index.js +++ b/web/src/components/storage/index.js @@ -32,7 +32,6 @@ export { default as ZFCPPage } from "./ZFCPPage"; export { default as ZFCPDiskForm } from "./ZFCPDiskForm"; export { default as ISCSIPage } from "./ISCSIPage"; export { DeviceContentInfo, DeviceExtendedInfo, FilesystemLabel } from "./device-utils"; -export { default as VolumeForm } from "./VolumeForm"; export { default as BootSelectionDialog } from "./BootSelectionDialog"; export { default as DeviceSelectionDialog } from "./DeviceSelectionDialog"; export { default as DeviceSelectorTable } from "./DeviceSelectorTable"; diff --git a/web/src/components/storage/utils.js b/web/src/components/storage/utils.js index 92282c9e5b..b47c9460c4 100644 --- a/web/src/components/storage/utils.js +++ b/web/src/components/storage/utils.js @@ -33,21 +33,18 @@ import { N_ } from "~/i18n"; */ /** - * @typedef {Object} SizeObject - * * @note undefined for either property means unknown - * + * @typedef {object} SizeObject * @property {number|undefined} size - The "amount" of size (10, 128, ...) * @property {string|undefined} unit - The size unit (MiB, GiB, ...) - */ - -/** - * @typedef SpacePolicy - * @type {object} + * + * @typedef {object} SpacePolicy * @property {string} id * @property {string} label * @property {string} description * @property {string[]} summaryLabels + * + * @typedef {"auto"|"fixed"|"range"} SizeMethod */ const SIZE_METHODS = Object.freeze({ diff --git a/web/src/components/storage/utils.test.js b/web/src/components/storage/utils.test.js index 4219a0c030..3798ea2c4c 100644 --- a/web/src/components/storage/utils.test.js +++ b/web/src/components/storage/utils.test.js @@ -63,7 +63,8 @@ const volume = (properties = {}) => { snapshotsConfigurable: false, snapshotsAffectSizes: false, sizeRelevantVolumes: [], - adjustByRam: false + adjustByRam: false, + productDefined: false } }; diff --git a/web/src/utils.js b/web/src/utils.js index f9645ab238..7513fad6a1 100644 --- a/web/src/utils.js +++ b/web/src/utils.js @@ -97,8 +97,8 @@ function uniq(collection) { * * @todo Use https://github.com/JedWatson/classnames instead? * - * @param {...*} CSS classes to join - * @returns {String} CSS classes joined together after ignoring falsy values + * @param {...*} classes - CSS classes to join + * @returns {String} - CSS classes joined together after ignoring falsy values */ function classNames(...classes) { return classes.filter((item) => !!item).join(' '); @@ -209,9 +209,9 @@ const useLocalStorage = (storageKey, fallbackState) => { * * Source {@link https://designtechworld.medium.com/create-a-custom-debounce-hook-in-react-114f3f245260} * - * @param {function} callback - Function to be called after some delay. + * @param {Function} callback - Function to be called after some delay. * @param {number} delay - Delay in milliseconds. - * @returns {function} + * @returns {Function} * * @example * @@ -255,7 +255,7 @@ const hex = (value) => { * * @todo This conversion will not be needed after adapting Section to directly work with issues. * - * @param {import("~/client/mixins").Issue} issues + * @param {import("~/client/mixins").Issue} issue * @returns {import("~/client/mixins").ValidationError} */ const toValidationError = (issue) => ({ message: issue.description });