From 1ecad832482186d9ba4704116ed267cb05e1a310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 11 Apr 2024 15:52:21 +0100 Subject: [PATCH 1/6] web: Add enums for possible targets --- web/src/client/storage.js | 83 +++++++++++++++++-- web/src/client/storage.test.js | 12 +-- .../components/overview/StorageSection.jsx | 2 +- .../storage/DeviceSelectionDialog.jsx | 10 +-- .../storage/DeviceSelectionDialog.test.jsx | 14 ++-- .../storage/ProposalDeviceSection.jsx | 15 ++-- .../storage/ProposalDeviceSection.test.jsx | 8 +- .../components/storage/ProposalVolumes.jsx | 10 +-- 8 files changed, 111 insertions(+), 43 deletions(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index e76959ffb3..2e7dca9072 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -111,7 +111,7 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk"; * @property {boolean} delete * * @typedef {object} ProposalSettings - * @property {string} target + * @property {ProposalTarget} target * @property {string} [targetDevice] * @property {string[]} targetPVDevices * @property {boolean} configureBoot @@ -124,13 +124,15 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk"; * @property {Volume[]} volumes * @property {StorageDevice[]} installationDevices * + * @typedef {keyof ProposalTargets} ProposalTarget + * * @typedef {object} SpaceAction * @property {string} device * @property {string} action * * @typedef {object} Volume * @property {string} mountPath - * @property {string} target + * @property {VolumeTarget} target * @property {StorageDevice} [targetDevice] * @property {string} fsType * @property {number} minSize @@ -140,15 +142,44 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk"; * @property {boolean} transactional * @property {VolumeOutline} outline * + * @typedef {keyof VolumeTargets} VolumeTarget + * + * @todo Define an enum for file system types. + * * @typedef {object} VolumeOutline * @property {boolean} required * @property {string[]} fsTypes + * @property {boolean} adjustByRam * @property {boolean} supportAutoSize * @property {boolean} snapshotsConfigurable * @property {boolean} snapshotsAffectSizes * @property {string[]} sizeRelevantVolumes */ +/** + * Enum for the possible proposal targets. + * + * @readonly + */ +const ProposalTargets = Object.freeze({ + DISK: "disk", + NEW_LVM_VG: "newLvmVg", + REUSED_LVM_VG: "reusedLvmVg" +}); + +/** + * Enum for the possible volume targets. + * + * @readonly + */ +const VolumeTargets = Object.freeze({ + DEFAULT: "default", + NEW_PARTITION: "new_partition", + NEW_VG: "new_vg", + DEVICE: "device", + FILESYSTEM: "filesystem" +}); + /** * Enum for the encryption method values * @@ -463,6 +494,23 @@ class ProposalManager { }; }; + /** + * Builds the proposal target from a D-Bus value. + * + * @param {string} dbusTarget + * @returns {ProposalTarget} + */ + const buildTarget = (dbusTarget) => { + switch (dbusTarget) { + case "disk": return "DISK"; + case "newLvmVg": return "NEW_LVM_VG"; + case "reusedLvmVg": return "REUSED_LVM_VG"; + default: + console.info(`Unknown proposal target "${dbusTarget}", using "disk".`); + return "DISK"; + } + }; + const buildTargetPVDevices = dbusTargetPVDevices => { if (!dbusTargetPVDevices) return []; @@ -474,7 +522,7 @@ class ProposalManager { const findDevice = (name) => { const device = devices.find(d => d.name === name); - if (device === undefined) console.log("D-Bus object not found: ", name); + if (device === undefined) console.error("D-Bus object not found: ", name); return device; }; @@ -489,14 +537,14 @@ class ProposalManager { const names = uniq(compact(values)).filter(d => d.length > 0); // #findDevice returns undefined if no device is found with the given name. - return compact(names.map(n => findDevice(n))); + return compact(names.map(findDevice)); }; const dbusSettings = proxy.Settings; return { settings: { - target: dbusSettings.Target.v, + target: buildTarget(dbusSettings.Target.v), targetDevice: dbusSettings.TargetDevice?.v, targetPVDevices: buildTargetPVDevices(dbusSettings.TargetPVDevices), configureBoot: dbusSettings.ConfigureBoot.v, @@ -560,7 +608,7 @@ class ProposalManager { MinSize: { t: "t", v: volume.minSize }, MaxSize: { t: "t", v: volume.maxSize }, AutoSize: { t: "b", v: volume.autoSize }, - Target: { t: "s", v: volume.target }, + Target: { t: "s", v: VolumeTargets[volume.target] }, TargetDevice: { t: "s", v: volume.targetDevice?.name }, Snapshots: { t: "b", v: volume.snapshots }, Transactional: { t: "b", v: volume.transactional }, @@ -568,7 +616,7 @@ class ProposalManager { }; const dbusSettings = removeUndefinedCockpitProperties({ - Target: { t: "s", v: target }, + Target: { t: "s", v: ProposalTargets[target] }, TargetDevice: { t: "s", v: targetDevice }, TargetPVDevices: { t: "as", v: targetPVDevices }, ConfigureBoot: { t: "b", v: configureBoot }, @@ -635,6 +683,25 @@ class ProposalManager { * @returns {Volume} */ buildVolume(dbusVolume, devices) { + /** + * Builds a volume target from a D-Bus value. + * + * @param {string} dbusTarget + * @returns {VolumeTarget} + */ + const buildTarget = (dbusTarget) => { + switch (dbusTarget) { + case "default": return "DEFAULT"; + case "new_partition": return "NEW_PARTITION"; + case "new_vg": return "NEW_VG"; + case "device": return "DEVICE"; + case "filesystem": return "FILESYSTEM"; + default: + console.info(`Unknown volume target "${dbusTarget}", using "default".`); + return "DEFAULT"; + } + }; + const buildOutline = (dbusOutline) => { if (dbusOutline === undefined) return null; @@ -650,7 +717,7 @@ class ProposalManager { }; return { - target: dbusVolume.Target.v, + target: buildTarget(dbusVolume.Target.v), targetDevice: devices.find(d => d.name === dbusVolume.TargetDevice?.v), mountPath: dbusVolume.MountPath.v, fsType: dbusVolume.FsType.v, diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index 756af42f46..c7bd15499c 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -1467,7 +1467,7 @@ describe("#proposal", () => { expect(home).toStrictEqual({ mountPath: "/home", - target: "default", + target: "DEFAULT", targetDevice: undefined, fsType: "XFS", minSize: 2048, @@ -1490,7 +1490,7 @@ describe("#proposal", () => { expect(generic).toStrictEqual({ mountPath: "", - target: "default", + target: "DEFAULT", targetDevice: undefined, fsType: "Ext4", minSize: 1024, @@ -1535,7 +1535,7 @@ describe("#proposal", () => { const { settings, actions } = await client.proposal.getResult(); expect(settings).toMatchObject({ - target: "newLvmVg", + target: "NEW_LVM_VG", targetPVDevices: ["/dev/sda", "/dev/sdb"], configureBoot: true, bootDevice: "/dev/sda", @@ -1549,7 +1549,7 @@ describe("#proposal", () => { volumes: [ { mountPath: "/", - target: "default", + target: "DEFAULT", targetDevice: undefined, fsType: "Btrfs", minSize: 1024, @@ -1568,7 +1568,7 @@ describe("#proposal", () => { }, { mountPath: "/home", - target: "default", + target: "DEFAULT", targetDevice: undefined, fsType: "XFS", minSize: 2048, @@ -1615,7 +1615,7 @@ describe("#proposal", () => { it("calculates a proposal with the given settings", async () => { await client.proposal.calculate({ - target: "disk", + target: "DISK", targetDevice: "/dev/vdc", configureBoot: true, bootDevice: "/dev/vdb", diff --git a/web/src/components/overview/StorageSection.jsx b/web/src/components/overview/StorageSection.jsx index 733b2cbe6c..078323193d 100644 --- a/web/src/components/overview/StorageSection.jsx +++ b/web/src/components/overview/StorageSection.jsx @@ -44,7 +44,7 @@ const ProposalSummary = ({ proposal }) => { return device ? deviceLabel(device) : deviceName; }; - if (result.settings?.target === "newLvmVg") { + if (result.settings.target === "NEW_LVM_VG") { // TRANSLATORS: Part of the message describing where the system will be installed. // Do not translate 'abbr' and 'title', they are part of the HTML markup. const vg = _("LVM volume group"); diff --git a/web/src/components/storage/DeviceSelectionDialog.jsx b/web/src/components/storage/DeviceSelectionDialog.jsx index 6f88fb8240..cdce4b18af 100644 --- a/web/src/components/storage/DeviceSelectionDialog.jsx +++ b/web/src/components/storage/DeviceSelectionDialog.jsx @@ -50,7 +50,7 @@ const Html = ({ children, ...props }) => ( * @component * * @param {object} props - * @param {string} props.target + * @param {ProposalTarget} props.target * @param {StorageDevice|undefined} props.targetDevice * @param {StorageDevice[]} props.targetPVDevices * @param {StorageDevice[]} props.devices - The actions to perform in the system. @@ -78,11 +78,11 @@ export default function DeviceSelectionDialog({ const [targetDevice, setTargetDevice] = useState(defaultTargetDevice); const [targetPVDevices, setTargetPVDevices] = useState(defaultPVDevices); - const isTargetDisk = target === "disk"; - const isTargetNewLvmVg = target === "newLvmVg"; + const isTargetDisk = target === "DISK"; + const isTargetNewLvmVg = target === "NEW_LVM_VG"; - const selectTargetDisk = () => setTarget("disk"); - const selectTargetNewLvmVG = () => setTarget("newLvmVg"); + const selectTargetDisk = () => setTarget("DISK"); + const selectTargetNewLvmVG = () => setTarget("NEW_LVM_VG"); const selectTargetDevice = (devices) => setTargetDevice(devices[0]); diff --git a/web/src/components/storage/DeviceSelectionDialog.test.jsx b/web/src/components/storage/DeviceSelectionDialog.test.jsx index bb450ccf8c..8171def342 100644 --- a/web/src/components/storage/DeviceSelectionDialog.test.jsx +++ b/web/src/components/storage/DeviceSelectionDialog.test.jsx @@ -121,7 +121,7 @@ describe("DeviceSelectionDialog", () => { beforeEach(() => { props = { isOpen: true, - target: "disk", + target: "DISK", targetPVDevices: [], devices: [sda, sdb, sdc], onCancel: jest.fn(), @@ -141,7 +141,7 @@ describe("DeviceSelectionDialog", () => { describe("if the target is a disk", () => { beforeEach(() => { - props.target = "disk"; + props.target = "DISK"; props.targetDevice = sda; }); @@ -188,7 +188,7 @@ describe("DeviceSelectionDialog", () => { describe("if the target is a new LVM volume group", () => { beforeEach(() => { - props.target = "newLvmVg"; + props.target = "NEW_LVM_VG"; props.targetPVDevices = [sda, sdc]; }); @@ -244,7 +244,7 @@ describe("DeviceSelectionDialog", () => { describe("if the option to select a disk as target device is selected", () => { beforeEach(() => { - props.target = "newLvmVg"; + props.target = "NEW_LVM_VG"; props.targetDevice = sda; }); @@ -261,7 +261,7 @@ describe("DeviceSelectionDialog", () => { await user.click(accept); expect(props.onAccept).toHaveBeenCalledWith({ - target: "disk", + target: "DISK", targetDevice: sdb, targetPVDevices: [] }); @@ -270,7 +270,7 @@ describe("DeviceSelectionDialog", () => { describe("if the option to create a new LVM volume group is selected", () => { beforeEach(() => { - props.target = "disk"; + props.target = "DISK"; props.targetDevice = sdb; }); @@ -288,7 +288,7 @@ describe("DeviceSelectionDialog", () => { await user.click(accept); expect(props.onAccept).toHaveBeenCalledWith({ - target: "newLvmVg", + target: "NEW_LVM_VG", targetDevice: sdb, targetPVDevices: [sda, sdc] }); diff --git a/web/src/components/storage/ProposalDeviceSection.jsx b/web/src/components/storage/ProposalDeviceSection.jsx index 0b065a4c0f..a076529db2 100644 --- a/web/src/components/storage/ProposalDeviceSection.jsx +++ b/web/src/components/storage/ProposalDeviceSection.jsx @@ -35,6 +35,7 @@ import { sprintf } from "sprintf-js"; import { compact, noop } from "~/utils"; /** + * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings * @typedef {import ("~/client/storage").StorageDevice} StorageDevice */ @@ -43,15 +44,15 @@ import { compact, noop } from "~/utils"; * Renders a button that allows changing the target device for installation. * * @param {object} props - * @param {string} props.target + * @param {ProposalTarget} props.target * @param {StorageDevice|undefined} props.targetDevice * @param {StorageDevice[]} props.targetPVDevices * @param {import("react").MouseEventHandler} [props.onClick=noop] */ const TargetDeviceButton = ({ target, targetDevice, targetPVDevices, onClick = noop }) => { const label = () => { - if (target === "disk" && targetDevice) return deviceLabel(targetDevice); - if (target === "newLvmVg" && targetPVDevices.length > 0) { + if (target === "DISK" && targetDevice) return deviceLabel(targetDevice); + if (target === "NEW_LVM_VG" && targetPVDevices.length > 0) { if (targetPVDevices.length > 1) return _("new LVM volume group"); if (targetPVDevices.length === 1) { @@ -75,15 +76,15 @@ const TargetDeviceButton = ({ target, targetDevice, targetPVDevices, onClick = n * @component * * @param {object} props - * @param {string} props.target - Installation target ("disk", "newLvmVg", "reusedLvmVg"). - * @param {StorageDevice|undefined} props.targetDevice - Target device (for target "disk"). - * @param {StorageDevice[]} props.targetPVDevices - Target devices for the LVM volume group (target "newLvmVg"). + * @param {ProposalTarget} props.target - Installation target + * @param {StorageDevice|undefined} props.targetDevice - Target device (for target "DISK"). + * @param {StorageDevice[]} props.targetPVDevices - Target devices for the LVM volume group (target "NEW_LVM_VG"). * @param {StorageDevice[]} props.devices - Available devices for installation. * @param {boolean} props.isLoading * @param {(target: Target) => void} props.onChange * * @typedef {object} Target - * @property {string} target + * @property {ProposalTarget} target * @property {StorageDevice|undefined} targetDevice * @property {StorageDevice[]} targetPVDevices */ diff --git a/web/src/components/storage/ProposalDeviceSection.test.jsx b/web/src/components/storage/ProposalDeviceSection.test.jsx index 3a85b1224f..9e3b3666db 100644 --- a/web/src/components/storage/ProposalDeviceSection.test.jsx +++ b/web/src/components/storage/ProposalDeviceSection.test.jsx @@ -72,7 +72,7 @@ describe("ProposalDeviceSection", () => { beforeEach(() => { props = { settings: { - target: "disk", + target: "DISK", targetDevice: "/dev/sda", }, availableDevices: [sda, sdb], @@ -101,7 +101,7 @@ describe("ProposalDeviceSection", () => { describe("when the target is a disk", () => { beforeEach(() => { - props.settings.target = "disk"; + props.settings.target = "DISK"; }); describe("and installation device is not selected yet", () => { @@ -137,7 +137,7 @@ describe("ProposalDeviceSection", () => { describe("when the target is a new LVM volume group", () => { beforeEach(() => { - props.settings.target = "newLvmVg"; + props.settings.target = "NEW_LVM_VG"; }); describe("and the target devices are not selected yet", () => { @@ -203,7 +203,7 @@ describe("ProposalDeviceSection", () => { expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); expect(props.onChange).toHaveBeenCalledWith({ - target: "disk", + target: "DISK", targetDevice: sdb.name, targetPVDevices: [] }); diff --git a/web/src/components/storage/ProposalVolumes.jsx b/web/src/components/storage/ProposalVolumes.jsx index d8bfb4f60a..5230fa83d9 100644 --- a/web/src/components/storage/ProposalVolumes.jsx +++ b/web/src/components/storage/ProposalVolumes.jsx @@ -185,7 +185,7 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => const SizeLimits = ({ volume }) => { let targetSize; - if (volume.target === "filesystem" || volume.target === "device") + if (volume.target === "FILESYSTEM" || volume.target === "DEVICE") targetSize = volume.targetDevice.size; const minSize = deviceSize(targetSize || volume.minSize); @@ -210,7 +210,7 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => const snapshots = hasSnapshots(volume); const transactional = isTransactionalRoot(volume); - if (volume.target === "filesystem") + if (volume.target === "FILESYSTEM") // TRANSLATORS: %s will be replaced by a file-system type like "Btrfs" or "Ext4" return sprintf(_("Reused %s"), volume.targetDevice?.filesystem?.type || ""); if (transactional) @@ -225,12 +225,12 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => if (volume.target === "new_partition") // TRANSLATORS: %s will be replaced by a disk name (eg. "/dev/sda") return sprintf(_("Partition at %s"), volume.targetDevice?.name || ""); - if (volume.target === "new_vg") + if (volume.target === "NEW_VG") // TRANSLATORS: %s will be replaced by a disk name (eg. "/dev/sda") return sprintf(_("Separate LVM at %s"), volume.targetDevice?.name || ""); - if (volume.target === "device" || volume.target === "filesystem") + if (volume.target === "DEVICE" || volume.target === "FILESYSTEM") return volume.targetDevice?.name || ""; - if (options.lvm) + if (target === "NEW_LVM_VG") return _("Logical volume at system LVM"); return _("Partition at installation disk"); From 0d195537baf6c8ac7f1768acc1a729e5e5ec550e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 11 Apr 2024 16:00:06 +0100 Subject: [PATCH 2/6] web: Allow changing the location of a volume --- .../storage/ProposalSettingsSection.jsx | 10 +- .../components/storage/ProposalVolumes.jsx | 225 ++++++++------ .../storage/ProposalVolumes.test.jsx | 179 +++++++----- .../storage/VolumeLocationDialog.jsx | 201 +++++++++++++ .../storage/VolumeLocationDialog.test.jsx | 275 ++++++++++++++++++ 5 files changed, 730 insertions(+), 160 deletions(-) create mode 100644 web/src/components/storage/VolumeLocationDialog.jsx create mode 100644 web/src/components/storage/VolumeLocationDialog.test.jsx diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 3ae852af7b..c01266bb2c 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -458,8 +458,8 @@ export default function ProposalSettingsSection({ }); }; - const lvm = settings.target === "newLvmVg" || settings.target === "reusedLvmVg"; - const encryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0; + const targetDevice = availableDevices.find(d => d.name === settings.targetDevice); + const useEncryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0; const { volumes = [], installationDevices = [], spaceActions = [] } = settings; const bootDevice = availableDevices.find(d => d.name === settings.bootDevice); const defaultBootDevice = availableDevices.find(d => d.name === settings.defaultBootDevice); @@ -484,14 +484,16 @@ export default function ProposalSettingsSection({ password={settings.encryptionPassword || ""} method={settings.encryptionMethod} methods={encryptionMethods} - isChecked={encryption} + isChecked={useEncryption} isLoading={settings.encryptionPassword === undefined} onChange={changeEncryption} /> diff --git a/web/src/components/storage/ProposalVolumes.jsx b/web/src/components/storage/ProposalVolumes.jsx index 5230fa83d9..52b8205541 100644 --- a/web/src/components/storage/ProposalVolumes.jsx +++ b/web/src/components/storage/ProposalVolumes.jsx @@ -19,6 +19,8 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React, { useState } from "react"; import { Dropdown, DropdownItem, DropdownList, @@ -33,16 +35,23 @@ import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; import { If, Popup, RowActions, Tip } from '~/components/core'; import { VolumeForm } from '~/components/storage'; +import VolumeLocationDialog from '~/components/storage/VolumeLocationDialog'; import { deviceSize, hasSnapshots, isTransactionalRoot } from '~/components/storage/utils'; import { noop } from "~/utils"; +/** + * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").Volume} Volume + */ + /** * Generates an hint describing which attributes affect the auto-calculated limits. * If the limits are not affected then it returns `null`. * @function * * @param {object} volume - storage volume object - * @returns {(ReactComponent|null)} component to display (can be `null`) + * @returns {(React.ReactElement|null)} component to display (can be `null`) */ const AutoCalculatedHint = (volume) => { const { snapshotsAffectSizes = false, sizeRelevantVolumes = [], adjustByRam } = volume.outline; @@ -161,28 +170,47 @@ const GeneralActions = ({ templates, onAdd, onReset }) => { * @component * * @param {object} props - * @param {object[]} props.columns - Column specs - * @param {object} props.volume - Volume to show - * @param {ProposalOptions} props.options - General proposal options + * @param {object} [props.columns] - Column specs + * @param {Volume} [props.volume] - Volume to show + * @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 * @param {boolean} props.isLoading - Whether to show the row as loading - * @param {onDeleteFn} props.onDelete - Function to use for deleting the volume - * - * @callback onDeleteFn - * @param {object} volume - * @return {void} + * @param {(volume: Volume) => void} [props.onEdit=noop] - Function to use for editing the volume + * @param {(volume: Volume) => void} [props.onDelete=noop] - Function to use for deleting the volume */ -const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => { - const [isFormOpen, setIsFormOpen] = useState(false); +const VolumeRow = ({ + columns, + volume, + devices, + target, + targetDevice, + isLoading, + onEdit = noop, + onDelete = noop +}) => { + /** @type {[string, (dialog: string) => void]} */ + const [dialog, setDialog] = useState(); - const openForm = () => setIsFormOpen(true); + const openEditDialog = () => setDialog("edit"); - const closeForm = () => setIsFormOpen(false); + const openLocationDialog = () => setDialog("location"); + + const closeDialog = () => setDialog(undefined); const acceptForm = (volume) => { - closeForm(); + closeDialog(); onEdit(volume); }; + const isEditDialogOpen = dialog === "edit"; + const isLocationDialogOpen = dialog === "location"; + + /** + * @component + * @param {object} props + * @param {Volume} props.volume + */ const SizeLimits = ({ volume }) => { let targetSize; if (volume.target === "FILESYSTEM" || volume.target === "DEVICE") @@ -206,6 +234,11 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => ); }; + /** + * @component + * @param {object} props + * @param {Volume} props.volume + */ const Details = ({ volume }) => { const snapshots = hasSnapshots(volume); const transactional = isTransactionalRoot(volume); @@ -221,8 +254,14 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => return volume.fsType; }; - const Location = ({ volume, options }) => { - if (volume.target === "new_partition") + /** + * @component + * @param {object} props + * @param {Volume} props.volume + * @param {ProposalTarget} props.target + */ + const Location = ({ volume, target }) => { + if (volume.target === "NEW_PARTITION") // TRANSLATORS: %s will be replaced by a disk name (eg. "/dev/sda") return sprintf(_("Partition at %s"), volume.targetDevice?.name || ""); if (volume.target === "NEW_VG") @@ -236,31 +275,38 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => return _("Partition at installation disk"); }; - const VolumeActions = ({ volume, onEdit, onDelete }) => { + /** + * @component + * @param {object} props + * @param {Volume} props.volume + * @param {() => void} props.onEditClick + * @param {() => void} props.onLocationClick + * @param {(volume: Volume) => void} props.onDeleteClick + */ + const VolumeActions = ({ volume, onEditClick, onLocationClick, onDeleteClick }) => { const actions = () => { const actions = { delete: { title: _("Delete"), - onClick: () => onDelete(volume), + onClick: () => onDeleteClick(volume), isDanger: true }, edit: { title: _("Edit"), - onClick: () => onEdit(volume) + onClick: onEditClick + }, + location: { + title: _("Change location"), + onClick: onLocationClick } }; - if (volume.outline.required) - return [actions.edit]; - else - return [actions.edit, actions.delete]; - }; + if (!volume.outline.required) return Object.values(actions); - const currentActions = actions(); - - if (currentActions.length === 0) return null; + return [actions.edit, actions.location]; + }; - return ; + return ; }; if (isLoading) { @@ -277,17 +323,18 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => {volume.mountPath}
- + - + /> {_("Accept")} - + + + + } + /> ); }; @@ -309,15 +371,13 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) => * * @param {object} props * @param {object[]} props.volumes - Volumes to show - * @param {ProposalOptions} props.options - General proposal options + * @param {StorageDevice[]} props.devices - Devices available for installation + * @param {ProposalTarget} props.target - Installation target + * @param {StorageDevice|undefined} props.targetDevice - Device selected for installation, if target is a disk * @param {boolean} props.isLoading - Whether to show the table as loading - * @param {onVolumesChangeFn} props.onVolumesChange - Function to submit changes in volumes - * - * @callback onVolumesChangeFn - * @param {object[]} volumes - * @return {void} + * @param {(volumes: Volume[]) => void} props.onVolumesChange - Function to submit changes in volumes */ -const VolumesTable = ({ volumes, options, isLoading, onVolumesChange }) => { +const VolumesTable = ({ volumes, devices, target, targetDevice, isLoading, onVolumesChange }) => { const columns = { mountPath: _("Mount point"), details: _("Details"), @@ -327,29 +387,30 @@ const VolumesTable = ({ volumes, options, isLoading, onVolumesChange }) => { actions: _("Actions") }; - const VolumesContent = ({ volumes, options, isLoading, onVolumesChange }) => { - const editVolume = (volume) => { - const index = volumes.findIndex(v => v.mountPath === volume.mountPath); - const newVolumes = [...volumes]; - newVolumes[index] = volume; - onVolumesChange(newVolumes); - }; + const editVolume = (volume) => { + const index = volumes.findIndex(v => v.mountPath === volume.mountPath); + const newVolumes = [...volumes]; + newVolumes[index] = volume; + onVolumesChange(newVolumes); + }; - const deleteVolume = (volume) => { - const newVolumes = volumes.filter(v => v.mountPath !== volume.mountPath); - onVolumesChange(newVolumes); - }; + const deleteVolume = (volume) => { + const newVolumes = volumes.filter(v => v.mountPath !== volume.mountPath); + onVolumesChange(newVolumes); + }; + const renderVolumes = () => { if (volumes.length === 0 && isLoading) return ; return volumes.map((volume, index) => { return ( { - + {renderVolumes()} ); }; /** + * @todo This component should be restructured to use the same approach as other newer components: + * * Create dialog components for the popup forms (e.g., EditVolumeDialog). + * * Use a TreeTable, specially if we need to represent subvolumes. + * * Renders information of the volumes and actions to modify them * @component * - * @param {object} props - * @param {object[]} [props.volumes=[]] - Volumes to show - * @param {object[]} [props.templates=[]] - Templates to use for new volumes - * @param {ProposalOptions} [props.options={}] - General proposal options - * @param {boolean} [props.isLoading=false] - Whether to show the content as loading - * @param {onChangeFn} [props.onChange=noop] - Function to use for changing the volumes + * @typedef {object} ProposalVolumesProps + * @property {Volume[]} volumes - Volumes to show + * @property {Volume[]} templates - Templates to use for new volumes + * @property {StorageDevice[]} devices - Devices available for installation + * @property {ProposalTarget} target - Installation target + * @property {StorageDevice|undefined} targetDevice - Device selected for installation, if target is a disk + * @property {boolean} [isLoading=false] - Whether to show the content as loading + * @property {(volumes: Volume[]) => void} onChange - Function to use for changing the volumes * - * @typedef {object} ProposalOptions - * @property {boolean} [lvm] - * @property {boolean} [encryption] - * - * @callback onChangeFn - * @param {object[]} volumes - * @return {void} + * @param {ProposalVolumesProps} props */ export default function ProposalVolumes({ - volumes = [], - templates = [], - options = {}, + volumes, + templates, + devices, + target, + targetDevice, isLoading = false, onChange = noop }) { - const addVolume = (volume) => { - if (onChange === noop) return; - const newVolumes = [...volumes, volume]; - onChange(newVolumes); - }; + const addVolume = (volume) => onChange([...volumes, volume]); - const resetVolumes = () => { - if (onChange === noop) return; - onChange([]); - }; + const resetVolumes = () => onChange([]); return ( <> @@ -436,7 +487,9 @@ export default function ProposalVolumes({ diff --git a/web/src/components/storage/ProposalVolumes.test.jsx b/web/src/components/storage/ProposalVolumes.test.jsx index bed4b1dcd9..34557c521e 100644 --- a/web/src/components/storage/ProposalVolumes.test.jsx +++ b/web/src/components/storage/ProposalVolumes.test.jsx @@ -19,11 +19,19 @@ * 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 { ProposalVolumes } from "~/components/storage"; +/** + * @typedef {import ("~/components/storage/ProposalVolumes").ProposalVolumesProps} ProposalVolumesProps + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").Volume} Volume + */ + jest.mock("@patternfly/react-core", () => { const original = jest.requireActual("@patternfly/react-core"); @@ -34,60 +42,73 @@ jest.mock("@patternfly/react-core", () => { }; }); -const volumes = { - root: { - mountPath: "/", - 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: [] - } - }, - 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: [] - } +/** @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 + } +}; + +/** @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, + sizeRelevantVolumes: [], + adjustByRam: false } }; +/** @type {Volume} */ +const homeVolume = { + mountPath: "/home", + target: "DEFAULT", + fsType: "XFS", + minSize: 1024, + autoSize: false, + snapshots: false, + transactional: false, + outline: { + required: false, + fsTypes: ["Ext4", "XFS"], + supportAutoSize: false, + snapshotsConfigurable: false, + snapshotsAffectSizes: false, + sizeRelevantVolumes: [], + adjustByRam: false + } +}; + +/** @type {StorageDevice} */ const sda = { sid: 59, name: "/dev/sda", + description: "", isDrive: true, type: "disk", vendor: "Micron", @@ -96,9 +117,11 @@ const sda = { size: 1024 }; +/** @type {StorageDevice} */ const sda1 = { sid: 69, name: "/dev/sda1", + description: "", isDrive: false, type: "partition", size: 256, @@ -108,9 +131,11 @@ const sda1 = { } }; +/** @type {StorageDevice} */ const sda2 = { sid: 79, name: "/dev/sda2", + description: "", isDrive: false, type: "partition", size: 512, @@ -120,10 +145,18 @@ const sda2 = { } }; +/** @type {ProposalVolumesProps} */ let props; beforeEach(() => { - props = {}; + props = { + volumes: [], + templates: [], + devices: [], + target: "DISK", + targetDevice: undefined, + onChange: jest.fn() + }; }); it("renders a button for the generic actions", async () => { @@ -138,8 +171,6 @@ it("renders a button for the generic actions", async () => { }); it("changes the volumes if reset action is used", async () => { - props.onChange = jest.fn(); - const { user } = plainRender(); const button = screen.getByRole("button", { name: "Actions" }); @@ -152,8 +183,7 @@ it("changes the volumes if reset action is used", async () => { }); it("allows to add a volume if add action is used", async () => { - props.templates = [volumes.home]; - props.onChange = jest.fn(); + props.templates = [homeVolume]; const { user } = plainRender(); @@ -172,8 +202,7 @@ it("allows to add a volume if add action is used", async () => { }); it("allows to cancel if add action is used", async () => { - props.templates = [volumes.home]; - props.onChange = jest.fn(); + props.templates = [homeVolume]; const { user } = plainRender(); @@ -193,7 +222,7 @@ it("allows to cancel if add action is used", async () => { describe("if there are volumes", () => { beforeEach(() => { - props.volumes = [volumes.root, volumes.home, volumes.swap]; + props.volumes = [rootVolume, homeVolume, swapVolume]; }); it("renders skeleton for each volume if loading", async () => { @@ -222,8 +251,6 @@ describe("if there are volumes", () => { }); it("allows deleting the volume", async () => { - props.onChange = jest.fn(); - const { user } = plainRender(); const [, body] = await screen.findAllByRole("rowgroup"); @@ -233,12 +260,10 @@ describe("if there are volumes", () => { const deleteAction = within(row).queryByRole("menuitem", { name: "Delete" }); await user.click(deleteAction); - expect(props.onChange).toHaveBeenCalledWith(expect.not.arrayContaining([volumes.home])); + expect(props.onChange).toHaveBeenCalledWith(expect.not.arrayContaining([homeVolume])); }); it("allows editing the volume", async () => { - props.onChange = jest.fn(); - const { user } = plainRender(); const [, body] = await screen.findAllByRole("rowgroup"); @@ -252,9 +277,23 @@ describe("if there are volumes", () => { within(popup).getByText("Edit file system"); }); - describe("and there is transactional Btrfs root volume", () => { + it("allows changing the location of the volume", async () => { + const { user } = plainRender(); + + const [, body] = await screen.findAllByRole("rowgroup"); + const row = within(body).getByRole("row", { name: "/home XFS At least 1 KiB Partition at installation disk" }); + const actions = within(row).getByRole("button", { name: "Actions" }); + await user.click(actions); + const locationAction = within(row).queryByRole("menuitem", { name: "Change location" }); + await user.click(locationAction); + + const popup = await screen.findByRole("dialog"); + within(popup).getByText("Location for /home file system"); + }); + + describe("and there is a transactional Btrfs root volume", () => { beforeEach(() => { - props.volumes = [{ ...volumes.root, snapshots: true, transactional: true }]; + props.volumes = [{ ...rootVolume, snapshots: true, transactional: true }]; }); it("renders 'transactional' legend as part of its information", async () => { @@ -268,7 +307,7 @@ describe("if there are volumes", () => { describe("and there is Btrfs volume using snapshots", () => { beforeEach(() => { - props.volumes = [{ ...volumes.root, snapshots: true, transactional: false }]; + props.volumes = [{ ...rootVolume, snapshots: true, transactional: false }]; }); it("renders 'with snapshots' legend as part of its information", async () => { @@ -283,9 +322,9 @@ describe("if there are volumes", () => { describe("and some volumes are allocated at separate disks", () => { beforeEach(() => { props.volumes = [ - volumes.root, - { ...volumes.swap, target: "new_partition", targetDevice: sda }, - { ...volumes.home, target: "new_vg", targetDevice: sda } + rootVolume, + { ...swapVolume, target: "NEW_PARTITION", targetDevice: sda }, + { ...homeVolume, target: "NEW_VG", targetDevice: sda } ]; }); @@ -302,9 +341,9 @@ describe("if there are volumes", () => { describe("and some volumes are reusing existing block devices", () => { beforeEach(() => { props.volumes = [ - volumes.root, - { ...volumes.swap, target: "filesystem", targetDevice: sda1 }, - { ...volumes.home, target: "device", targetDevice: sda2 } + rootVolume, + { ...swapVolume, target: "FILESYSTEM", targetDevice: sda1 }, + { ...homeVolume, target: "DEVICE", targetDevice: sda2 } ]; }); diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx new file mode 100644 index 0000000000..9261919d01 --- /dev/null +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -0,0 +1,201 @@ +/* + * 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, { useState } from "react"; +import { Checkbox, Form } from "@patternfly/react-core"; +import { _ } from "~/i18n"; +import { DevicesFormSelect } from "~/components/storage"; +import { Popup } from "~/components/core"; +import { deviceLabel } from "~/components/storage/utils"; +import { sprintf } from "sprintf-js"; + +/** + * @typedef {"auto"|"device"|"reuse"} LocationOption + * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").Volume} Volume + * @typedef {import ("~/client/storage").VolumeTarget} VolumeTarget + */ + +const LOCATION_AUTO_ID = "location-auto"; +const LOCATION_MANUAL_ID = "location-manual"; + +/** + * Generates a location option value from the given target. + * @function + * + * @param {VolumeTarget} target + * @returns {LocationOption} + */ +const targetToOption = (target) => { + switch (target) { + case "DEFAULT": + return "auto"; + case "NEW_PARTITION": + case "NEW_VG": + return "device"; + case "DEVICE": + case "FILESYSTEM": + return "reuse"; + } +}; + +/** + * Internal component for building the options. + * @component + * + * @param {React.PropsWithChildren>} props + */ +const RadioOption = ({ id, onChange, defaultChecked, children }) => { + return ( + <> + + + + ); +}; + +/** + * Renders a dialog that allows the user to change the location of a volume. + * @component + * + * @typedef {object} VolumeLocationDialogProps + * @property {Volume} volume - Volume to edit. + * @property {StorageDevice[]} devices - Devices available for installation. + * @property {ProposalTarget} target - Installation target. + * @property {StorageDevice|undefined} targetDevice - Device selected for installation, if target is a disk. + * @property {boolean} [isOpen=false] - Whether the dialog is visible or not. + * @property {() => void} onCancel + * @property {(volume: Volume) => void} onAccept + * + * @param {VolumeLocationDialogProps} props + */ +export default function VolumeLocationDialog({ + volume, + devices, + target, + targetDevice: defaultTargetDevice, + isOpen, + onCancel, + onAccept, + ...props +}) { + const [locationOption, setLocationOption] = useState(targetToOption(volume.target)); + const [targetDevice, setTargetDevice] = useState(volume.targetDevice || defaultTargetDevice || devices[0]); + const [isDedicatedVG, setIsDedicatedVG] = useState(volume.target === "NEW_VG"); + + const selectAutoOption = () => setLocationOption("auto"); + const selectDeviceOption = () => setLocationOption("device"); + const toggleDedicatedVG = (_, value) => setIsDedicatedVG(value); + + const isLocationAuto = locationOption === "auto"; + const isLocationDevice = locationOption === "device"; + + const onSubmit = (e) => { + e.preventDefault(); + const newVolume = { ...volume }; + + if (isLocationAuto) { + newVolume.target = "DEFAULT"; + newVolume.targetDevice = undefined; + } + + if (isLocationDevice) { + newVolume.target = isDedicatedVG ? "NEW_VG" : "NEW_PARTITION"; + newVolume.targetDevice = targetDevice; + } + + onAccept(newVolume); + }; + + const isAcceptDisabled = () => { + return isLocationDevice && targetDevice === undefined; + }; + + const autoText = () => { + if (target === "DISK" && defaultTargetDevice) + // TRANSLATORS: %s is replaced by a device label (e.g., "/dev/vda, 50 GiB"). + return sprintf(_("The filesystem will be allocated as a new partition at the installation \ +disk (%s)."), deviceLabel(defaultTargetDevice)); + + if (target === "DISK") + return _("The filesystem will be allocated as a new partition at the installation disk."); + + return _("The file system will be allocated as a logical volume at the system LVM."); + }; + + return ( + +
+
+ + + {_("Automatic")} + + +
+ {autoText()} +
+
+ +
+ + + {_("Select a disk")} + + + +
+
+ {_("The file system will be allocated as a new partition at the selected disk.")} +
+ + +
+
+
+ + + + + + ); +} diff --git a/web/src/components/storage/VolumeLocationDialog.test.jsx b/web/src/components/storage/VolumeLocationDialog.test.jsx new file mode 100644 index 0000000000..e89367209e --- /dev/null +++ b/web/src/components/storage/VolumeLocationDialog.test.jsx @@ -0,0 +1,275 @@ +/* + * 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 VolumeLocationDialog from "~/components/storage/VolumeLocationDialog"; + +/** + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").Volume} Volume + * @typedef {import ("~/components/storage/VolumeLocationDialog").VolumeLocationDialogProps} VolumeLocationDialogProps + */ + +/** @type {StorageDevice} */ +const sda = { + sid: 59, + isDrive: true, + type: "disk", + description: "", + vendor: "Micron", + model: "Micron 1100 SATA", + driver: ["ahci", "mmcblk"], + bus: "IDE", + busId: "", + transport: "usb", + dellBOSS: false, + sdCard: true, + active: true, + name: "/dev/sda", + size: 1024, + recoverableSize: 0, + systems : [], + udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"], + udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], +}; + +/** @type {StorageDevice} */ +const sdb = { + sid: 62, + isDrive: true, + type: "disk", + description: "", + vendor: "Samsung", + model: "Samsung Evo 8 Pro", + driver: ["ahci"], + bus: "IDE", + busId: "", + transport: "", + dellBOSS: false, + sdCard: false, + active: true, + name: "/dev/sdb", + size: 2048, + recoverableSize: 0, + systems : [], + udevIds: [], + udevPaths: ["pci-0000:00-19"] +}; + +/** @type {StorageDevice} */ +const sdc = { + sid: 63, + isDrive: true, + type: "disk", + description: "", + vendor: "Samsung", + model: "Samsung Evo 8 Pro", + driver: ["ahci"], + bus: "IDE", + busId: "", + transport: "", + dellBOSS: false, + sdCard: false, + active: true, + name: "/dev/sdc", + size: 2048, + recoverableSize: 0, + systems : [], + udevIds: [], + udevPaths: ["pci-0000:00-19"] +}; + +/** @type {Volume} */ +const volume = { + 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 + } +}; + +/** @type {VolumeLocationDialogProps} */ +let props; + +describe("VolumeLocationDialog", () => { + beforeEach(() => { + props = { + isOpen: true, + volume, + devices: [sda, sdb, sdc], + target: "DISK", + targetDevice: sda, + onCancel: jest.fn(), + onAccept: jest.fn() + }; + }); + + const automaticOption = () => screen.queryByRole("radio", { name: "Automatic" }); + const selectDiskOption = () => screen.queryByRole("radio", { name: "Select a disk" }); + const diskSelector = () => screen.queryByRole("combobox", { name: /choose a disk/i }); + const lvmSelector = () => screen.queryByRole("checkbox", { name: /dedicated lvm/i }); + + it("offers an option to use the installation disk", () => { + plainRender(); + expect(automaticOption()).toBeInTheDocument(); + }); + + it("offers an option to selected a disk", () => { + plainRender(); + expect(selectDiskOption()).toBeInTheDocument(); + expect(diskSelector()).toBeInTheDocument(); + expect(lvmSelector()).toBeInTheDocument(); + }); + + describe("if the current value is set to use the installation disk", () => { + beforeEach(() => { + props.volume.target = "DEFAULT"; + props.targetDevice = sda; + }); + + it("selects 'Automatic' option by default", () => { + plainRender(); + expect(automaticOption()).toBeChecked(); + expect(selectDiskOption()).not.toBeChecked(); + expect(diskSelector()).toBeDisabled(); + expect(lvmSelector()).toBeDisabled(); + }); + }); + + describe("if the current value is set to use a selected disk", () => { + beforeEach(() => { + props.volume.target = "NEW_PARTITION"; + props.targetDevice = sda; + }); + + it("selects 'Select a disk' option by default", () => { + plainRender(); + expect(automaticOption()).not.toBeChecked(); + expect(selectDiskOption()).toBeChecked(); + expect(diskSelector()).toBeEnabled(); + expect(lvmSelector()).toBeEnabled(); + expect(lvmSelector()).not.toBeChecked(); + }); + }); + + describe("if the current value is set to use a selected disk for a dedicated LVM", () => { + beforeEach(() => { + props.volume.target = "NEW_VG"; + props.targetDevice = sda; + }); + + it("selects 'Select a disk' option and check LVM by default", () => { + plainRender(); + expect(automaticOption()).not.toBeChecked(); + expect(selectDiskOption()).toBeChecked(); + expect(diskSelector()).toBeEnabled(); + expect(lvmSelector()).toBeEnabled(); + expect(lvmSelector()).toBeChecked(); + }); + }); + + it("does not call onAccept on cancel", async () => { + const { user } = plainRender(); + const cancel = screen.getByRole("button", { name: "Cancel" }); + + await user.click(cancel); + + expect(props.onAccept).not.toHaveBeenCalled(); + }); + + describe("if the 'Automatic' option is selected", () => { + beforeEach(() => { + props.volume.target = "NEW_PARTITION"; + props.volume.targetDevice = sda; + }); + + it("calls onAccept with the selected options on accept", async () => { + const { user } = plainRender(); + + await user.click(automaticOption()); + + const accept = screen.getByRole("button", { name: "Confirm" }); + await user.click(accept); + + expect(props.onAccept).toHaveBeenCalledWith(expect.objectContaining( + { target: "DEFAULT", targetDevice: undefined } + )); + }); + }); + + describe("if the 'Select a disk' option is selected", () => { + beforeEach(() => { + props.volume.target = "DEFAULT"; + props.volume.targetDevice = undefined; + }); + + it("calls onAccept with the selected options on accept", async () => { + const { user } = plainRender(); + + await user.click(selectDiskOption()); + const selector = diskSelector(); + const sdbOption = within(selector).getByRole("option", { name: /sdb/ }); + await user.selectOptions(selector, sdbOption); + + const accept = screen.getByRole("button", { name: "Confirm" }); + await user.click(accept); + + expect(props.onAccept).toHaveBeenCalledWith(expect.objectContaining( + { target: "NEW_PARTITION", targetDevice: sdb } + )); + }); + + describe("and dedicated LVM is checked", () => { + it("calls onAccept with the selected options on accept", async () => { + const { user } = plainRender(); + + await user.click(selectDiskOption()); + const selector = diskSelector(); + const sdbOption = within(selector).getByRole("option", { name: /sdb/ }); + await user.selectOptions(selector, sdbOption); + await user.click(lvmSelector()); + + const accept = screen.getByRole("button", { name: "Confirm" }); + await user.click(accept); + + expect(props.onAccept).toHaveBeenCalledWith(expect.objectContaining( + { target: "NEW_VG", targetDevice: sdb } + )); + }); + }); + }); +}); From 4e0784093798487969388ef7ce82754e6db89547 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 11 Apr 2024 16:03:14 +0100 Subject: [PATCH 3/6] web: Add and fix type checking --- web/src/client/mixins.js | 2 +- web/src/client/storage.js | 4 +- web/src/components/core/Description.jsx | 8 ++- web/src/components/core/NumericTextInput.jsx | 6 +- web/src/components/core/RowActions.jsx | 2 - web/src/components/core/Tip.jsx | 7 +- .../components/overview/StorageSection.jsx | 27 ++++++-- .../storage/DeviceSelectionDialog.jsx | 1 + web/src/components/storage/ProposalPage.jsx | 7 ++ .../storage/ProposalSettingsSection.jsx | 24 +++---- web/src/components/storage/VolumeForm.jsx | 65 +++++++------------ web/src/test-utils.js | 2 +- 12 files changed, 84 insertions(+), 71 deletions(-) diff --git a/web/src/client/mixins.js b/web/src/client/mixins.js index 30ab4e95bf..c334ad1670 100644 --- a/web/src/client/mixins.js +++ b/web/src/client/mixins.js @@ -155,7 +155,7 @@ const WithStatus = (superclass, object_path) => class extends superclass { * Register a callback to run when the "CurrentInstallationPhase" changes * * @param {function} handler - callback function - * @return {function} function to disable the callback + * @return {() => void} function to disable the callback */ onStatusChange(handler) { return this.client.onObjectChanged(object_path, STATUS_IFACE, (changes) => { diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 2e7dca9072..190600f862 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -61,7 +61,7 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk"; * @property {string} [busId] - DASD Bus ID (only for "dasd" type) * @property {string} [transport] * @property {boolean} [sdCard] - * @property {boolean} [dellBOOS] + * @property {boolean} [dellBOSS] * @property {string[]} [devices] - RAID devices (only for "raid" and "md" types) * @property {string[]} [wires] - Multipath wires (only for "multipath" type) * @property {string} [level] - MD RAID level (only for "md" type) @@ -110,6 +110,8 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk"; * @property {boolean} subvol * @property {boolean} delete * + * @todo Define an enum for space policies. + * * @typedef {object} ProposalSettings * @property {ProposalTarget} target * @property {string} [targetDevice] diff --git a/web/src/components/core/Description.jsx b/web/src/components/core/Description.jsx index 849334be31..0d5e271f1a 100644 --- a/web/src/components/core/Description.jsx +++ b/web/src/components/core/Description.jsx @@ -19,6 +19,8 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; import { Popover, Button } from "@patternfly/react-core"; @@ -26,8 +28,10 @@ import { Popover, Button } from "@patternfly/react-core"; * Displays details popup after clicking the children elements * @component * - * @param {(JSX.Element|null)} description content displayed in a popup - * @param {JSX.Element} children the wrapped content + * @param {object} props + * @param {React.ReactElement} props.description - Content displayed in a popup. + * @param {React.ReactNode} props.children - The wrapped content. + * @param {import("@patternfly/react-core").PopoverProps} [props.otherProps] */ export default function Description ({ description, children, ...otherProps }) { if (description) { diff --git a/web/src/components/core/NumericTextInput.jsx b/web/src/components/core/NumericTextInput.jsx index cdce9f7787..a8a095ba6e 100644 --- a/web/src/components/core/NumericTextInput.jsx +++ b/web/src/components/core/NumericTextInput.jsx @@ -19,6 +19,8 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; import { TextInput } from "@patternfly/react-core"; import { noop } from "~/utils"; @@ -42,9 +44,7 @@ import { noop } from "~/utils"; * @param {object} props * @param {string|number} props.value - the input value * @param {onChangeFn} props.onChange - the callback to be called when the entered value match the input pattern - * @param {object} props.textInputProps - @see {@link https://www.patternfly.org/components/forms/text-input#props} - * - * @returns {ReactComponent} + * @param {import("@patternfly/react-core").TextInputProps} props.textInputProps */ export default function NumericTextInput({ value = "", onChange = noop, ...textInputProps }) { // NOTE: Using \d* instead of \d+ at the beginning to allow empty diff --git a/web/src/components/core/RowActions.jsx b/web/src/components/core/RowActions.jsx index 598832cd75..d72b847887 100644 --- a/web/src/components/core/RowActions.jsx +++ b/web/src/components/core/RowActions.jsx @@ -52,8 +52,6 @@ import { _ } from "~/i18n"; * @param {object} [props.rest] * * @typedef {import("@patternfly/react-table").IAction} Action - * - * @return {React.ActionsColumn} */ export default function RowActions({ id, actions, "aria-label": toggleAriaLabel, ...rest }) { const actionsToggle = (props) => ( diff --git a/web/src/components/core/Tip.jsx b/web/src/components/core/Tip.jsx index de68d9257a..8a72e22f3b 100644 --- a/web/src/components/core/Tip.jsx +++ b/web/src/components/core/Tip.jsx @@ -19,6 +19,8 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; import { Label } from "@patternfly/react-core"; @@ -31,8 +33,9 @@ import { Icon } from "~/components/layout"; * If the label is not defined or is empty it behaves like a plain label. * @component * - * @param {JSX.Element} description details displayed after clicking the label - * @param {JSX.Element} children the content of the label + * @param {object} props + * @param {React.ReactElement} props.description - Details displayed after clicking the label. + * @param {React.ReactNode} props.children - The content of the label. */ export default function Tip ({ description, children }) { if (description) { diff --git a/web/src/components/overview/StorageSection.jsx b/web/src/components/overview/StorageSection.jsx index 078323193d..272a8eeac3 100644 --- a/web/src/components/overview/StorageSection.jsx +++ b/web/src/components/overview/StorageSection.jsx @@ -19,6 +19,8 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React, { useReducer, useEffect } from "react"; import { Text } from "@patternfly/react-core"; @@ -30,14 +32,26 @@ import { Em, ProgressText, Section } from "~/components/core"; import { sprintf } from "sprintf-js"; import { _, n_ } from "~/i18n"; +/** + * @typedef {import ("~/client/storage").ProposalResult} ProposalResult + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * + * @typedef {object} Proposal + * @property {StorageDevice[]} availableDevices + * @property {ProposalResult} result + */ + /** * Text explaining the storage proposal * * FIXME: this needs to be basically rewritten. See * https://github.com/openSUSE/agama/discussions/778#discussioncomment-7715244 + * + * @param {object} props + * @param {Proposal} props.proposal */ const ProposalSummary = ({ proposal }) => { - const { availableDevices = [], result = {} } = proposal; + const { availableDevices, result } = proposal; const label = (deviceName) => { const device = availableDevices.find(d => d.name === deviceName); @@ -48,7 +62,7 @@ const ProposalSummary = ({ proposal }) => { // TRANSLATORS: Part of the message describing where the system will be installed. // Do not translate 'abbr' and 'title', they are part of the HTML markup. const vg = _("LVM volume group"); - const pvDevices = result.settings?.targetPVDevices; + const pvDevices = result.settings.targetPVDevices; const fullMsg = (policy, num_pvs) => { switch (policy) { case "resize": @@ -86,7 +100,7 @@ const ProposalSummary = ({ proposal }) => { } }; - const msg = sprintf(fullMsg(result.settings?.spacePolicy, pvDevices.length), vg, "%dev%"); + const msg = sprintf(fullMsg(result.settings.spacePolicy, pvDevices.length), vg, "%dev%"); if (pvDevices.length > 1) { return (); @@ -103,7 +117,7 @@ const ProposalSummary = ({ proposal }) => { } } - const targetDevice = result.settings?.targetDevice; + const targetDevice = result.settings.targetDevice; if (!targetDevice) return {_("No device selected yet")}; const fullMsg = (policy) => { @@ -127,7 +141,7 @@ const ProposalSummary = ({ proposal }) => { return _("Install using device %s with a custom strategy to find the needed space"); }; - const [msg1, msg2] = fullMsg(result.settings?.spacePolicy).split("%s"); + const [msg1, msg2] = fullMsg(result.settings.spacePolicy).split("%s"); return ( @@ -183,6 +197,7 @@ const reducer = (state, action) => { export default function StorageSection({ showErrors = false }) { const { storage: client } = useInstallerClient(); const { cancellablePromise } = useCancellablePromise(); + /** @type {[object, (action: object) => void]} */ const [state, dispatch] = useReducer(reducer, initialState); useEffect(() => { @@ -258,7 +273,7 @@ export default function StorageSection({ showErrors = false }) { } return ( - + ); }; diff --git a/web/src/components/storage/DeviceSelectionDialog.jsx b/web/src/components/storage/DeviceSelectionDialog.jsx index cdce4b18af..1095b63963 100644 --- a/web/src/components/storage/DeviceSelectionDialog.jsx +++ b/web/src/components/storage/DeviceSelectionDialog.jsx @@ -31,6 +31,7 @@ import { DeviceSelectorTable } from "~/components/storage"; import { noop } from "~/utils"; /** + * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings * @typedef {import ("~/client/storage").StorageDevice} StorageDevice */ diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index 5099a8fca4..6123835fae 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -216,6 +216,13 @@ export default function ProposalPage() { calculate(newSettings).catch(console.error); }; + /** + * @todo Enable type checking and ensure the components are called with the correct props. + * + * @note The default value for `settings` should be `undefined` instead of an empty object, and + * the settings prop of the components should accept both a ProposalSettings object or undefined. + */ + return ( // TRANSLATORS: Storage page title diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index c01266bb2c..8d9fc4f253 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -407,21 +407,23 @@ const SpacePolicyField = ({ * Section for editing the proposal settings * @component * - * @param {object} props - * @param {ProposalSettings} props.settings - * @param {StorageDevice[]} [props.availableDevices=[]] - * @param {String[]} [props.encryptionMethods=[]] - * @param {Volume[]} [props.volumeTemplates=[]] - * @param {boolean} [props.isLoading=false] - * @param {(settings: object) => void} [props.onChange=noop] + * @typedef {object} ProposalSettingsSectionProps + * @property {ProposalSettings} settings + * @property {StorageDevice[]} availableDevices + * @property {String[]} encryptionMethods + * @property {Volume[]} volumeTemplates + * @property {boolean} [isLoading=false] + * @property {(settings: object) => void} onChange + * + * @param {ProposalSettingsSectionProps} props */ export default function ProposalSettingsSection({ settings, - availableDevices = [], - encryptionMethods = [], - volumeTemplates = [], + availableDevices, + encryptionMethods, + volumeTemplates, isLoading = false, - onChange = noop + onChange }) { const changeEncryption = ({ password, method }) => { onChange({ encryptionPassword: password, encryptionMethod: method }); diff --git a/web/src/components/storage/VolumeForm.jsx b/web/src/components/storage/VolumeForm.jsx index b106d9925d..dbf4382a0c 100644 --- a/web/src/components/storage/VolumeForm.jsx +++ b/web/src/components/storage/VolumeForm.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2023] SUSE LLC + * Copyright (c) [2023-2024] SUSE LLC * * All Rights Reserved. * @@ -19,6 +19,8 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React, { useReducer, useState } from "react"; import { InputGroup, InputGroupItem, Form, FormGroup, FormSelect, FormSelectOption, MenuToggle, @@ -32,7 +34,7 @@ import { DEFAULT_SIZE_UNIT, SIZE_METHODS, SIZE_UNITS, parseToBytes, splitSize } import { Icon } from "~/components/layout"; /** - * @typedef {import ("~/client/storage").ProposalManager.Volume} Volume + * @typedef {import ("~/client/storage").Volume} Volume */ /** @@ -51,8 +53,7 @@ import { Icon } from "~/components/layout"; * * @param {object} props * @param {Array} props.units - a collection of size units - * @param {object} props.formSelectProps - @see {@link https://www.patternfly.org/components/forms/form-select#props} - * @returns {ReactComponent} + * @param {import("@patternfly/react-core").FormSelectProps} props.formSelectProps */ const SizeUnitFormSelect = ({ units, ...formSelectProps }) => { return ( @@ -74,8 +75,7 @@ const SizeUnitFormSelect = ({ units, ...formSelectProps }) => { * @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 {object} props.selectProps - other props sent to {@link https://www.patternfly.org/components/menus/select#props PF/Select} - * @returns {ReactComponent} + * @param {import("@patternfly/react-core").SelectProps} [props.selectProps] */ const MountPointFormSelect = ({ value, volumes, onChange, ...selectProps }) => { @@ -134,25 +134,12 @@ const fsOptions = (volume) => { return volume.outline.fsTypes; }; -/** - * File system properties from a file system type option. - * @function - * - * @param {string} fsOption - * @returns {FsValue} - */ -const fsValue = (fsOption) => { - return { fsType: fsOption, snapshots: false }; -}; - /** * Option for selecting a file system type. * @component * * @param {object} props * @param {string} props.fsOption - File system type option. - * - * @returns {ReactComponent} */ const FsSelectOption = ({ fsOption }) => { return ( @@ -168,11 +155,9 @@ const FsSelectOption = ({ fsOption }) => { * * @param {object} props * @param {string} props.id - Widget id. - * @param {FsValue} props.value - Currently selected file system properties. - * @param {Volume} volume - The selected storage volume. + * @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. - * - * @returns {ReactComponent} */ const FsSelect = ({ id, value, volume, onChange }) => { const [isOpen, setIsOpen] = useState(false); @@ -186,7 +171,7 @@ const FsSelect = ({ id, value, volume, onChange }) => { const onSelect = (_event, option) => { setIsOpen(false); - onChange(fsValue(option)); + onChange({ fsType: option, snapshots: false }); }; const toggle = toggleRef => { @@ -229,15 +214,9 @@ const FsSelect = ({ id, value, volume, onChange }) => { * @component * * @param {object} props - * @param {FsValue} props.value - Currently selected file system properties. - * @param {Volume} volume - The selected storage volume. + * @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} FsValue - * @property {string} fsType - * @property {boolean} snapshots - * - * @returns {ReactComponent} */ const FsField = ({ value, volume, onChange }) => { const isSingleFs = () => { @@ -293,8 +272,7 @@ const FsField = ({ value, volume, onChange }) => { * @component * * @param {object} props - * @param {Volume} volume - a storage volume object - * @returns {ReactComponent} + * @param {Volume} props.volume - a storage volume object */ const SizeAuto = ({ volume }) => { const conditions = []; @@ -335,8 +313,6 @@ const SizeAuto = ({ volume }) => { * @param {object} props.errors - the form errors * @param {object} props.formData - the form data * @param {onChangeFn} props.onChange - callback for notifying input changes - * - * @returns {ReactComponent} */ const SizeManual = ({ errors, formData, onChange }) => { return ( @@ -351,6 +327,7 @@ const SizeManual = ({ errors, formData, onChange }) => { { { * @param {object} props.errors - the form errors * @param {object} props.formData - the form data * @param {onChangeFn} props.onChange - callback for notifying input changes - * - * @returns {ReactComponent} */ const SizeRange = ({ errors, formData, onChange }) => { return ( @@ -411,6 +387,7 @@ and maximum. If no maximum is given then the file system will be as big as possi { const { sizeMethod } = formData; const sizeWidgetProps = { errors, formData, volume, onChange }; + /** @type {string[]} */ const sizeOptions = [SIZE_METHODS.MANUAL, SIZE_METHODS.RANGE]; if (volume.outline.supportAutoSize) sizeOptions.push(SIZE_METHODS.AUTO); @@ -656,7 +635,8 @@ const reducer = (state, action) => { * * @param {object} props * @param {string} props.id - Form ID - * @param {Array} props.volumes - a collection of storage volumes + * @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 @@ -664,6 +644,7 @@ const reducer = (state, action) => { * @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) => { diff --git a/web/src/test-utils.js b/web/src/test-utils.js index 4c843703fa..00a6aaf4ef 100644 --- a/web/src/test-utils.js +++ b/web/src/test-utils.js @@ -149,7 +149,7 @@ const plainRender = (ui, options = {}) => { * It can be useful to mock functions that might receive a callback that you can * execute on-demand during the test. * - * @return a tuple with the mocked function and the list of callbacks. + * @return {[() => () => void, Array<(any) => void>]} a tuple with the mocked function and the list of callbacks. */ const createCallbackMock = () => { const callbacks = []; From b80ba46bb42b825b1567003ae0f5181c16c8caea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 11 Apr 2024 16:03:48 +0100 Subject: [PATCH 4/6] web: Add type checking and fix tests --- .../storage/BootSelectionDialog.test.jsx | 2 + .../storage/DeviceSelectionDialog.test.jsx | 2 + .../storage/ProposalDeviceSection.test.jsx | 6 +- .../components/storage/ProposalPage.test.jsx | 148 +++++++++++++----- .../storage/ProposalSettingsSection.test.jsx | 81 +++++++--- 5 files changed, 178 insertions(+), 61 deletions(-) diff --git a/web/src/components/storage/BootSelectionDialog.test.jsx b/web/src/components/storage/BootSelectionDialog.test.jsx index 7e759a1115..395c0e4ebb 100644 --- a/web/src/components/storage/BootSelectionDialog.test.jsx +++ b/web/src/components/storage/BootSelectionDialog.test.jsx @@ -19,6 +19,8 @@ * 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"; diff --git a/web/src/components/storage/DeviceSelectionDialog.test.jsx b/web/src/components/storage/DeviceSelectionDialog.test.jsx index 8171def342..8dc9a217e1 100644 --- a/web/src/components/storage/DeviceSelectionDialog.test.jsx +++ b/web/src/components/storage/DeviceSelectionDialog.test.jsx @@ -19,6 +19,8 @@ * 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"; diff --git a/web/src/components/storage/ProposalDeviceSection.test.jsx b/web/src/components/storage/ProposalDeviceSection.test.jsx index 9e3b3666db..bdbab62055 100644 --- a/web/src/components/storage/ProposalDeviceSection.test.jsx +++ b/web/src/components/storage/ProposalDeviceSection.test.jsx @@ -19,13 +19,15 @@ * 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 { ProposalDeviceSection } from "~/components/storage"; const sda = { - sid: "59", + sid: 59, isDrive: true, type: "disk", vendor: "Micron", @@ -46,7 +48,7 @@ const sda = { }; const sdb = { - sid: "62", + sid: 62, isDrive: true, type: "disk", vendor: "Samsung", diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 516fff030f..4a21a31bd0 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -19,13 +19,23 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; import { act, screen, waitFor } from "@testing-library/react"; import { createCallbackMock, installerRender } from "~/test-utils"; import { createClient } from "~/client"; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +import { StorageClient } from "~/client/storage"; import { IDLE } from "~/client/status"; import { ProposalPage } from "~/components/storage"; +/** + * @typedef {import ("~/client/storage").ProposalResult} ProposalResult + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").Volume} Volume + */ + jest.mock("~/client"); jest.mock("@patternfly/react-core", () => { const original = jest.requireActual("@patternfly/react-core"); @@ -46,9 +56,12 @@ jest.mock("~/context/product", () => ({ }) })); +/** @type {StorageDevice} */ const vda = { - sid: "59", + sid: 59, type: "disk", + isDrive: true, + description: "", vendor: "Micron", model: "Micron 1100 SATA", driver: ["ahci", "mmcblk"], @@ -62,12 +75,14 @@ const vda = { systems : ["Windows 11", "openSUSE Leap 15.2"], udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"], udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], - partitionTable: { type: "gpt", partitions: ["/dev/vda1", "/dev/vda2"] } }; +/** @type {StorageDevice} */ const vdb = { - sid: "60", + sid: 60, type: "disk", + isDrive: true, + description: "", vendor: "Seagate", model: "Unknown", driver: ["ahci", "mmcblk"], @@ -76,32 +91,84 @@ const vdb = { size: 1e+6 }; -const storageMock = { - probe: jest.fn().mockResolvedValue(0), - proposal: { - getAvailableDevices: jest.fn().mockResolvedValue([vda, vdb]), - getEncryptionMethods: jest.fn().mockResolvedValue([]), - getProductMountPoints: jest.fn().mockResolvedValue([]), - getResult: jest.fn().mockResolvedValue(undefined), - defaultVolume: jest.fn(mountPath => Promise.resolve({ mountPath })), - calculate: jest.fn().mockResolvedValue(0), - }, - system: { - getDevices: jest.fn().mockResolvedValue([vda, vdb]) - }, - staging: { - getDevices: jest.fn().mockResolvedValue([vda]) - }, - getErrors: jest.fn().mockResolvedValue([]), - isDeprecated: jest.fn().mockResolvedValue(false), - onDeprecate: jest.fn(), - onStatusChange: jest.fn() +/** + * @param {string} mountPath + * @returns {Volume} + */ +const volume = (mountPath) => { + return ( + { + mountPath, + target: "DEFAULT", + fsType: "Btrfs", + minSize: 1024, + maxSize: 1024, + autoSize: false, + snapshots: false, + transactional: false, + outline: { + required: false, + fsTypes: ["Btrfs"], + supportAutoSize: false, + snapshotsConfigurable: false, + snapshotsAffectSizes: false, + sizeRelevantVolumes: [], + adjustByRam: false + } + } + ); }; +/** @type {StorageClient} */ let storage; +/** @type {ProposalResult} */ +let proposalResult; + beforeEach(() => { - storage = { ...storageMock, proposal: { ...storageMock.proposal } }; + proposalResult = { + settings: { + target: "DISK", + targetPVDevices: [], + configureBoot: false, + bootDevice: "", + defaultBootDevice: "", + encryptionPassword: "", + encryptionMethod: "", + spacePolicy: "", + spaceActions: [], + volumes: [], + installationDevices: [] + }, + actions: [] + }; + + storage = { + probe: jest.fn().mockResolvedValue(0), + // @ts-expect-error Some methods have to be private to avoid type complaint. + proposal: { + getAvailableDevices: jest.fn().mockResolvedValue([vda, vdb]), + getEncryptionMethods: jest.fn().mockResolvedValue([]), + getProductMountPoints: jest.fn().mockResolvedValue([]), + getResult: jest.fn().mockResolvedValue(proposalResult), + defaultVolume: jest.fn(mountPath => Promise.resolve(volume(mountPath))), + calculate: jest.fn().mockResolvedValue(0), + }, + // @ts-expect-error Some methods have to be private to avoid type complaint. + system: { + getDevices: jest.fn().mockResolvedValue([vda, vdb]) + }, + // @ts-expect-error Some methods have to be private to avoid type complaint. + staging: { + getDevices: jest.fn().mockResolvedValue([vda]) + }, + getErrors: jest.fn().mockResolvedValue([]), + isDeprecated: jest.fn().mockResolvedValue(false), + onDeprecate: jest.fn(), + onStatusChange: jest.fn() + }; + + // @ts-expect-error createClient.mockImplementation(() => ({ storage })); }); @@ -117,9 +184,8 @@ it("does not probe storage if the storage devices are not deprecated", async () }); it("loads the proposal data", async () => { - storage.proposal.getResult = jest.fn().mockResolvedValue( - { settings: { target: "disk", targetDevice: vda.name } } - ); + proposalResult.settings.target = "DISK"; + proposalResult.settings.targetDevice = vda.name; installerRender(); @@ -152,8 +218,8 @@ describe("when the storage devices become deprecated", () => { }); it("loads the proposal data", async () => { - const result = { settings: { target: "disk", targetDevice: vda.name } }; - storage.proposal.getResult = jest.fn().mockResolvedValue(result); + proposalResult.settings.target = "DISK"; + proposalResult.settings.targetDevice = vda.name; const [mockFunction, callbacks] = createCallbackMock(); storage.onDeprecate = mockFunction; @@ -162,7 +228,7 @@ describe("when the storage devices become deprecated", () => { await screen.findByText(/\/dev\/vda/); - result.settings.targetDevice = vdb.name; + proposalResult.settings.targetDevice = vdb.name; const [onDeprecateCb] = callbacks; await act(() => onDeprecateCb()); @@ -172,11 +238,9 @@ describe("when the storage devices become deprecated", () => { }); describe("when there is no proposal yet", () => { - beforeEach(() => { - storage.proposal.getResult = jest.fn().mockResolvedValue(undefined); - }); - it("shows the page as loading", async () => { + proposalResult = undefined; + installerRender(); screen.getAllByText(/PFSkeleton/); @@ -184,6 +248,9 @@ describe("when there is no proposal yet", () => { }); it("loads the proposal when the service finishes to calculate", async () => { + const defaultResult = proposalResult; + proposalResult = undefined; + const [mockFunction, callbacks] = createCallbackMock(); storage.onStatusChange = mockFunction; @@ -191,9 +258,9 @@ describe("when there is no proposal yet", () => { screen.getAllByText(/PFSkeleton/); - storage.proposal.getResult = jest.fn().mockResolvedValue( - { settings: { target: "disk", targetDevice: vda.name } } - ); + proposalResult = defaultResult; + proposalResult.settings.target = "DISK"; + proposalResult.settings.targetDevice = vda.name; const [onStatusChangeCb] = callbacks; await act(() => onStatusChangeCb(IDLE)); @@ -203,14 +270,13 @@ describe("when there is no proposal yet", () => { describe("when there is a proposal", () => { beforeEach(() => { - storage.proposal.getResult = jest.fn().mockResolvedValue( - { settings: { target: "disk", targetDevice: vda.name } } - ); + proposalResult.settings.target = "DISK"; + proposalResult.settings.targetDevice = vda.name; }); it("does not load the proposal when the service finishes to calculate", async () => { const [mockFunction, callbacks] = createCallbackMock(); - storage.proposal.onStatusChange = mockFunction; + storage.onStatusChange = mockFunction; installerRender(); diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx index 2b15463f95..03e05df65b 100644 --- a/web/src/components/storage/ProposalSettingsSection.test.jsx +++ b/web/src/components/storage/ProposalSettingsSection.test.jsx @@ -19,11 +19,19 @@ * 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 { ProposalSettingsSection } from "~/components/storage"; +/** + * @typedef {import ("~/components/storage/ProposalSettingsSection").ProposalSettingsSectionProps} ProposalSettingsSectionProps + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").Volume} Volume + */ + jest.mock("@patternfly/react-core", () => { const original = jest.requireActual("@patternfly/react-core"); @@ -33,20 +41,57 @@ jest.mock("@patternfly/react-core", () => { }; }); +/** @type {Volume} */ +let volume; + +/** @type {ProposalSettingsSectionProps} */ let props; beforeEach(() => { + volume = { + 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 + } + }; + props = { - settings: {}, + settings: { + target: "DISK", + targetPVDevices: [], + configureBoot: false, + bootDevice: "", + defaultBootDevice: "", + encryptionPassword: "", + encryptionMethod: "", + spacePolicy: "", + spaceActions: [], + volumes: [], + installationDevices: [] + }, + availableDevices: [], + encryptionMethods: [], + volumeTemplates: [], onChange: jest.fn() }; }); -const rootVolume = { mountPath: "/", fsType: "Btrfs", outline: { snapshotsConfigurable: true } }; - describe("if snapshots are configurable", () => { beforeEach(() => { - props.settings = { volumes: [rootVolume] }; + props.settings.volumes = [volume]; }); it("renders the snapshots switch", () => { @@ -58,7 +103,7 @@ describe("if snapshots are configurable", () => { describe("if snapshots are not configurable", () => { beforeEach(() => { - props.settings = { volumes: [{ ...rootVolume, outline: { ...rootVolume.outline, snapshotsConfigurable: false } }] }; + volume.outline.snapshotsConfigurable = false; }); it("does not render the snapshots switch", () => { @@ -91,9 +136,10 @@ it("requests a volume change when onChange callback is triggered", async () => { }); describe("Encryption field", () => { - describe("if encryption password setting is not set yet", () => { + describe.skip("if encryption password setting is not set yet", () => { beforeEach(() => { - props.settings = {}; + // Currently settings cannot be undefined. + props.settings = undefined; }); it("does not render the encryption switch", () => { @@ -105,7 +151,7 @@ describe("Encryption field", () => { describe("if encryption password setting is set", () => { beforeEach(() => { - props.settings = { encryptionPassword: "" }; + props.settings.encryptionPassword = ""; }); it("renders the encryption switch", () => { @@ -117,8 +163,7 @@ describe("Encryption field", () => { describe("if encryption password is not empty", () => { beforeEach(() => { - props.settings = { encryptionPassword: "1234" }; - props.onChange = jest.fn(); + props.settings.encryptionPassword = "1234"; }); it("renders the encryption switch as selected", () => { @@ -179,8 +224,7 @@ describe("Encryption field", () => { describe("if encryption password is empty", () => { beforeEach(() => { - props.settings = { encryptionPassword: "" }; - props.onChange = jest.fn(); + props.settings.encryptionPassword = ""; }); it("renders the encryption switch as not selected", () => { @@ -239,9 +283,10 @@ describe("Encryption field", () => { }); describe("Space policy field", () => { - describe("if there is no space policy", () => { + describe.skip("if there is no space policy", () => { beforeEach(() => { - props.settings = {}; + // Currently settings cannot be undefined. + props.settings = undefined; }); it("does not render the space policy field", () => { @@ -253,15 +298,15 @@ describe("Space policy field", () => { describe("if there is a space policy", () => { beforeEach(() => { - props.settings = { - spacePolicy: "delete" - }; + props.settings.spacePolicy = "delete"; }); it("renders the button with a text according to given policy", () => { const { rerender } = plainRender(); screen.getByRole("button", { name: /deleting/ }); - rerender(); + + props.settings.spacePolicy = "resize"; + rerender(); screen.getByRole("button", { name: /shrinking/ }); }); From 86f85f57937a875fe8fa5ad13e6c44616e063cdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 11 Apr 2024 16:18:07 +0100 Subject: [PATCH 5/6] web: Changelog --- web/package/cockpit-agama.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 4605122b18..6a5e0f302a 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu Apr 11 15:16:42 UTC 2024 - José Iván López González + +- Allow changing the location of a file system + (gh#openSUSE/agama#1141). + ------------------------------------------------------------------- Mon Apr 8 14:17:45 UTC 2024 - José Iván López González From 3b4ec557a1c2c95fe2dcd372c02547ca9aedd3d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 08:59:03 +0100 Subject: [PATCH 6/6] web: Add hint about expected error --- web/src/components/storage/ProposalPage.test.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 4a21a31bd0..96021a8a24 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -168,7 +168,7 @@ beforeEach(() => { onStatusChange: jest.fn() }; - // @ts-expect-error + // @ts-expect-error Mocking method does not exist fo InstallerClient type. createClient.mockImplementation(() => ({ storage })); });