diff --git a/service/lib/agama/dbus/storage/interfaces/device/filesystem.rb b/service/lib/agama/dbus/storage/interfaces/device/filesystem.rb index 545a179813..b16cb24d5a 100644 --- a/service/lib/agama/dbus/storage/interfaces/device/filesystem.rb +++ b/service/lib/agama/dbus/storage/interfaces/device/filesystem.rb @@ -58,7 +58,7 @@ def filesystem_sid # # @return [String] e.g., "ext4" def filesystem_type - storage_device.filesystem.type.to_s + storage_device.filesystem.type.to_human_string end # Mount path of the file system. diff --git a/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb b/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb index b029b45b1f..eacee97742 100644 --- a/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb +++ b/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb @@ -169,7 +169,9 @@ def missing_volumes def volume_device_conversion(target) return unless settings.device.is_a?(DeviceSettings::Disk) - target.volumes.select(&:proposed?).each { |v| v.device ||= settings.device.name } + target.volumes + .select { |v| v.proposed? && !v.reuse_name } + .each { |v| v.device ||= settings.device.name } end # @param target [Y2Storage::ProposalSettings] diff --git a/service/lib/agama/storage/volume_conversion/from_y2storage.rb b/service/lib/agama/storage/volume_conversion/from_y2storage.rb index d7f1b93226..01415a4135 100644 --- a/service/lib/agama/storage/volume_conversion/from_y2storage.rb +++ b/service/lib/agama/storage/volume_conversion/from_y2storage.rb @@ -59,8 +59,8 @@ def sizes_conversion(target) planned = planned_device_for(target.mount_path) return unless planned - target.min_size = planned.min - target.max_size = planned.max + target.min_size = planned.min if planned.respond_to?(:min) + target.max_size = planned.max if planned.respond_to?(:max) end # Planned device for the given mount path. diff --git a/service/test/agama/dbus/storage/interfaces/device/filesystem_examples.rb b/service/test/agama/dbus/storage/interfaces/device/filesystem_examples.rb index 71694a3ab7..7d1eb3ae76 100644 --- a/service/test/agama/dbus/storage/interfaces/device/filesystem_examples.rb +++ b/service/test/agama/dbus/storage/interfaces/device/filesystem_examples.rb @@ -36,7 +36,7 @@ describe "#filesystem_type" do it "returns the file system type" do - expect(subject.filesystem_type).to eq("ext4") + expect(subject.filesystem_type).to eq("Ext4") end end diff --git a/service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb b/service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb index 6cd5be7191..48918aa4bb 100644 --- a/service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb +++ b/service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb @@ -135,6 +135,25 @@ expect(y2storage_settings.candidate_devices).to contain_exactly("/dev/sda") end end + + context "and a volume is reusing a device" do + before do + settings.volumes = [ + Agama::Storage::Volume.new("/test1").tap do |volume| + volume.location.target = :device + volume.location.device = "/dev/sdb" + end + ] + end + + it "does not set the target device as device for the volume with missing device" do + y2storage_settings = subject.convert + + expect(y2storage_settings.volumes).to contain_exactly( + an_object_having_attributes(mount_point: "/test1", device: nil) + ) + end + end end context "when the device settings is set to create a new LVM volume group" do diff --git a/web/cspell.json b/web/cspell.json index 5a6b767489..57a61da4fa 100644 --- a/web/cspell.json +++ b/web/cspell.json @@ -71,6 +71,7 @@ "subvolume", "subvolumes", "svgrrc", + "scrollbox", "teleporter", "testfile", "testsuite", diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 8cc66aeef9..77b9b3ee55 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Fri May 3 09:45:36 UTC 2024 - José Iván López González + +- Allow reusing an existing device or file system + (gh#openSUSE/agama#1165). + ------------------------------------------------------------------- Thu May 02 11:07:23 UTC 2024 - Balsa Asanovic diff --git a/web/src/assets/styles/composition.scss b/web/src/assets/styles/composition.scss index 07c7994aa7..7055502e3f 100644 --- a/web/src/assets/styles/composition.scss +++ b/web/src/assets/styles/composition.scss @@ -24,6 +24,40 @@ flex-wrap: wrap; } +// TODO: make it less specific. +.location-layout > div { + display: flex; + + form { + display: flex; + flex-direction: column; + flex: 1 1 0; + gap: 0 + } + + form > div:nth-child(2) { + overflow-y: auto; + min-block-size: 120px; + margin-block-end: var(--spacer-medium); + table { background: transparent; } + } + + form > div:last-child { + min-block-size: min-content; + } +} + +// FIXME: Temporary solution to hide expand button. The button should be removed instead. +.location-layout table button[id^="expand-toggle"] { + display: none; +} + +.location-layout table tbody tr:not(:first-child) { + td:nth-child(3) { + padding-inline-start: 5ch; + } +} + [data-state="reversed"] { flex-direction: row-reverse; } diff --git a/web/src/assets/styles/patternfly-overrides.scss b/web/src/assets/styles/patternfly-overrides.scss index 71040a7797..71361c6f53 100644 --- a/web/src/assets/styles/patternfly-overrides.scss +++ b/web/src/assets/styles/patternfly-overrides.scss @@ -268,6 +268,14 @@ ul { vertical-align: middle; } +.pf-v5-c-radio { + align-items: center; +} + +.pf-v5-c-radio__label { + font-size: var(--pf-v5-c-form__label--FontSize); +} + @media screen and (width <= 768px) { .pf-m-grid-md.pf-v5-c-table tr:where(.pf-v5-c-table__tr):not(.pf-v5-c-table__expandable-row) { padding-inline: 0; diff --git a/web/src/assets/styles/utilities.scss b/web/src/assets/styles/utilities.scss index 31c96d36fc..16ff87dc16 100644 --- a/web/src/assets/styles/utilities.scss +++ b/web/src/assets/styles/utilities.scss @@ -223,6 +223,18 @@ max-inline-size: calc(var(--ui-max-inline-size) + var(--spacer-large)) } +.scrollbox { + background: + linear-gradient(#fff 33%, rgb(255 255 255 / 0%)), + linear-gradient(rgb(255 255 255 / 0%), #fff 66%) 0 100%, + radial-gradient(farthest-side at 50% 0, rgb(102 102 102 / 50%), rgb(0 0 0 / 0%)), + radial-gradient(farthest-side at 50% 100%, rgba(102 102 102 / 50%), rgb(0 0 0 / 0%)) 0 100%; + background-color: #fff; + background-repeat: no-repeat; + background-attachment: local, local, scroll, scroll; + background-size: 100% 48px, 100% 48px, 100% 16px, 100% 16px; +} + .height-75 { height: 75dvh; } diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 55623d8c36..182b1166bc 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -451,6 +451,40 @@ class ProposalManager { return proxy.AvailableDevices.map(path => findDevice(systemDevices, path)).filter(d => d); } + /** + * Gets the devices that can be selected as target for a volume. + * + * A device can be selected as target for a volume if either it is an available device for + * installation or it is a device built over the available devices for installation. For example, + * a MD RAID is a possible target only if all its members are available devices or children of the + * available devices. + * + * @returns {Promise} + */ + async getVolumeDevices() { + /** @type {StorageDevice[]} */ + const availableDevices = await this.getAvailableDevices(); + + /** @type {(device: StorageDevice) => boolean} */ + const isAvailable = (device) => { + const isChildren = (device, parentDevice) => { + const partitions = parentDevice.partitionTable?.partitions || []; + return !!partitions.find(d => d.name === device.name); + }; + + return !!availableDevices.find(d => d.name === device.name || isChildren(device, d)); + }; + + /** @type {(device: StorageDevice[]) => boolean} */ + const allAvailable = (devices) => devices.every(isAvailable); + + const system = await this.system.getDevices(); + const mds = system.filter(d => d.type === "md" && allAvailable(d.devices)); + const vgs = system.filter(d => d.type === "lvmVg" && allAvailable(d.physicalVolumes)); + + return [...availableDevices, ...mds, ...vgs]; + } + /** * Gets the list of meaningful mount points for the selected product * @@ -537,7 +571,7 @@ class ProposalManager { return dbusTargetPVDevices.v.map(d => d.v); }; - // TODO: Read installation devices from D-Bus. + /** @todo Read installation devices from D-Bus. */ const buildInstallationDevices = (dbusSettings, devices) => { const findDevice = (name) => { const device = devices.find(d => d.name === name); @@ -547,10 +581,17 @@ class ProposalManager { return device; }; + // Only consider the device assigned to a volume as installation device if it is needed + // to find space in that device. For example, devices directly formatted or mounted are not + // considered as installation devices. + const volumes = dbusSettings.Volumes.v.filter(vol => ( + [VolumeTargets.NEW_PARTITION, VolumeTargets.NEW_VG].includes(vol.v.Target.v)) + ); + const values = [ dbusSettings.TargetDevice?.v, buildTargetPVDevices(dbusSettings.TargetPVDevices), - dbusSettings.Volumes.v.map(vol => vol.v.TargetDevice.v) + volumes.map(vol => vol.v.TargetDevice.v) ].flat(); if (dbusSettings.ConfigureBoot.v) values.push(dbusSettings.BootDevice.v); diff --git a/web/src/components/core/ExpandableSelector.jsx b/web/src/components/core/ExpandableSelector.jsx index 7499f37f1d..af98528c71 100644 --- a/web/src/components/core/ExpandableSelector.jsx +++ b/web/src/components/core/ExpandableSelector.jsx @@ -24,6 +24,11 @@ import React, { useState } from "react"; import { Table, Thead, Tr, Th, Tbody, Td, ExpandableRowContent, RowSelectVariant } from "@patternfly/react-table"; +/** + * @typedef {import("@patternfly/react-table").TableProps} TableProps + * @typedef {import("react").RefAttributes} HTMLTableProps + */ + /** * An object for sharing data across nested maps * @@ -93,23 +98,26 @@ const sanitizeSelection = (selection, allowMultiple) => { }; /** - * Build a expandable table with selectable items + * Build a expandable table with selectable items. * @component * * @note It only accepts one nesting level. * - * @param {object} props - * @param {ExpandableSelectorColumn[]} props.columns - Collection of objects defining columns. - * @param {boolean} [props.isMultiple=false] - Whether multiple selection is allowed. - * @param {object[]} props.items - Collection of items to be rendered. - * @param {string} [props.itemIdKey="id"] - The key for retrieving the item id. - * @param {(item: object) => Array} [props.itemChildren=() => []] - Lookup method to retrieve children from given item. - * @param {(item: object) => boolean} [props.itemSelectable=() => true] - Whether an item will be selectable or not. - * @param {(item: object) => (string|undefined)} [props.itemClassNames=() => ""] - Callback that allows adding additional CSS class names to item row. - * @param {object[]} [props.itemsSelected=[]] - Collection of selected items. - * @param {string[]} [props.initialExpandedKeys=[]] - Ids of initially expanded items. - * @param {(selection: Array) => void} [props.onSelectionChange=noop] - Callback to be triggered when selection changes. - * @param {object} [props.tableProps] - Props for {@link https://www.patternfly.org/components/table/#table PF/Table}. + * @typedef {object} ExpandableSelectorBaseProps + * @property {ExpandableSelectorColumn[]} [columns=[]] - Collection of objects defining columns. + * @property {boolean} [isMultiple=false] - Whether multiple selection is allowed. + * @property {object[]} [items=[]] - Collection of items to be rendered. + * @property {string} [itemIdKey="id"] - The key for retrieving the item id. + * @property {(item: object) => Array} [itemChildren=() => []] - Lookup method to retrieve children from given item. + * @property {(item: object) => boolean} [itemSelectable=() => true] - Whether an item will be selectable or not. + * @property {(item: object) => (string|undefined)} [itemClassNames=() => ""] - Callback that allows adding additional CSS class names to item row. + * @property {object[]} [itemsSelected=[]] - Collection of selected items. + * @property {any[]} [initialExpandedKeys=[]] - Ids of initially expanded items. + * @property {(selection: Array) => void} [onSelectionChange=noop] - Callback to be triggered when selection changes. + * + * @typedef {ExpandableSelectorBaseProps & TableProps & HTMLTableProps} ExpandableSelectorProps + * + * @param {ExpandableSelectorProps} props */ export default function ExpandableSelector({ columns = [], @@ -126,7 +134,14 @@ export default function ExpandableSelector({ }) { const [expandedItemsKeys, setExpandedItemsKeys] = useState(initialExpandedKeys); const selection = sanitizeSelection(itemsSelected, isMultiple); - const isItemSelected = (item) => selection.includes(item); + const isItemSelected = (item) => { + const selected = selection.find((selectionItem) => { + return Object.hasOwn(selectionItem, itemIdKey) && + selectionItem[itemIdKey] === item[itemIdKey]; + }); + + return selected !== undefined || selection.includes(item); + }; const isItemExpanded = (key) => expandedItemsKeys.includes(key); const toggleExpanded = (key) => { if (isItemExpanded(key)) { diff --git a/web/src/components/core/TreeTable.jsx b/web/src/components/core/TreeTable.jsx index b7f12d1ad9..2f258a13f9 100644 --- a/web/src/components/core/TreeTable.jsx +++ b/web/src/components/core/TreeTable.jsx @@ -30,8 +30,8 @@ import { Table, Thead, Tr, Th, Tbody, Td, TreeRowWrapper } from '@patternfly/rea /** * @typedef {object} TreeTableColumn - * @property {string} title - * @property {(any) => React.ReactNode} content + * @property {string} name + * @property {(object) => React.ReactNode} value * @property {string} [classNames] */ @@ -82,14 +82,14 @@ export default function TreeTable({ const renderColumns = (item, treeRow) => { return columns.map((c, cIdx) => { const props = { - dataLabel: c.title, + dataLabel: c.name, className: c.classNames }; if (cIdx === 0) props.treeRow = treeRow; return ( - {c.content(item)} + {c.value(item)} ); }); }; @@ -138,7 +138,7 @@ export default function TreeTable({ > - { columns.map((c, i) => {c.title}) } + { columns.map((c, i) => {c.name}) } diff --git a/web/src/components/storage/BootConfigField.jsx b/web/src/components/storage/BootConfigField.jsx index 34f5b23422..c1b57571f3 100644 --- a/web/src/components/storage/BootConfigField.jsx +++ b/web/src/components/storage/BootConfigField.jsx @@ -56,23 +56,25 @@ const Button = ({ isBold = false, onClick }) => { * Allows to select the boot config. * @component * - * @param {object} props - * @param {boolean} props.configureBoot - * @param {StorageDevice|undefined} props.bootDevice - * @param {StorageDevice|undefined} props.defaultBootDevice - * @param {StorageDevice[]} props.devices - * @param {boolean} props.isLoading - * @param {(boot: BootConfig) => void} props.onChange + * @typedef {object} BootConfigFieldProps + * @property {boolean} configureBoot + * @property {StorageDevice|undefined} bootDevice + * @property {StorageDevice|undefined} defaultBootDevice + * @property {StorageDevice[]} availableDevices + * @property {boolean} isLoading + * @property {(boot: BootConfig) => void} onChange * * @typedef {object} BootConfig * @property {boolean} configureBoot * @property {StorageDevice} bootDevice + * + * @param {BootConfigFieldProps} props */ export default function BootConfigField({ configureBoot, bootDevice, defaultBootDevice, - devices, + availableDevices, isLoading, onChange }) { @@ -113,7 +115,7 @@ export default function BootConfigField({ configureBoot={configureBoot} bootDevice={bootDevice} defaultBootDevice={defaultBootDevice} - devices={devices} + availableDevices={availableDevices} onAccept={onAccept} onCancel={closeDialog} /> diff --git a/web/src/components/storage/BootConfigField.test.jsx b/web/src/components/storage/BootConfigField.test.jsx index 036db095ab..ac6636aaa5 100644 --- a/web/src/components/storage/BootConfigField.test.jsx +++ b/web/src/components/storage/BootConfigField.test.jsx @@ -26,6 +26,12 @@ import { screen, within } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import BootConfigField from "~/components/storage/BootConfigField"; +/** + * @typedef {import("~/components/storage/BootConfigField").BootConfigFieldProps} BootConfigFieldProps + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + */ + +/** @type {StorageDevice} */ const sda = { sid: 59, description: "A fake disk for testing", @@ -48,6 +54,7 @@ const sda = { udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], }; +/** @type {BootConfigFieldProps} */ let props; beforeEach(() => { @@ -55,7 +62,7 @@ beforeEach(() => { configureBoot: false, bootDevice: undefined, defaultBootDevice: undefined, - devices: [sda], + availableDevices: [sda], isLoading: false, onChange: jest.fn() }; diff --git a/web/src/components/storage/BootSelectionDialog.jsx b/web/src/components/storage/BootSelectionDialog.jsx index 006cb54d9e..dfcf502d9a 100644 --- a/web/src/components/storage/BootSelectionDialog.jsx +++ b/web/src/components/storage/BootSelectionDialog.jsx @@ -19,17 +19,16 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React, { useState } from "react"; import { Form } from "@patternfly/react-core"; import { _ } from "~/i18n"; import { DevicesFormSelect } from "~/components/storage"; -import { noop } from "~/utils"; import { Popup } from "~/components/core"; import { deviceLabel } from "~/components/storage/utils"; import { sprintf } from "sprintf-js"; -// @ts-check - /** * @typedef {import ("~/client/storage").StorageDevice} StorageDevice */ @@ -58,27 +57,29 @@ const RadioOption = ({ id, onChange, defaultChecked, children }) => { * Renders a dialog that allows the user to select the boot configuration. * @component * - * @typedef {object} Boot + * @typedef {object} BootSelectionDialogProps + * @property {boolean} configureBoot - Whether the boot is configurable + * @property {StorageDevice|undefined} bootDevice - Currently selected booting device. + * @property {StorageDevice|undefined} defaultBootDevice - Default booting device. + * @property {StorageDevice[]} availableDevices - Devices that user can select to boot from. + * @property {boolean} [isOpen=false] - Whether the dialog is visible or not. + * @property {() => void} onCancel + * @property {(boot: BootConfig) => void} onAccept + * + * @typedef {object} BootConfig * @property {boolean} configureBoot * @property {StorageDevice|undefined} bootDevice * - * @param {object} props - * @param {boolean} props.configureBoot - Whether the boot is configurable - * @param {StorageDevice|undefined} props.bootDevice - Currently selected booting device. - * @param {StorageDevice|undefined} props.defaultBootDevice - Default booting device. - * @param {StorageDevice[]} props.devices - Devices that user can select to boot from. - * @param {boolean} [props.isOpen=false] - Whether the dialog is visible or not. - * @param {function} [props.onCancel=noop] - * @param {(boot: Boot) => void} [props.onAccept=noop] + * @param {BootSelectionDialogProps} props */ export default function BootSelectionDialog({ configureBoot: configureBootProp, bootDevice: bootDeviceProp, defaultBootDevice, - devices, + availableDevices, isOpen, - onCancel = noop, - onAccept = noop, + onCancel, + onAccept, ...props }) { const [configureBoot, setConfigureBoot] = useState(configureBootProp); @@ -161,7 +162,7 @@ partitions in the appropriate disk." { @@ -96,7 +108,9 @@ describe("BootSelectionDialog", () => { props = { isOpen: true, configureBoot: false, - devices: [sda, sdb, sdc], + availableDevices: [sda, sdb, sdc], + bootDevice: undefined, + defaultBootDevice: undefined, onCancel: jest.fn(), onAccept: jest.fn() }; diff --git a/web/src/components/storage/DeviceSelectionDialog.jsx b/web/src/components/storage/DeviceSelectionDialog.jsx index 403f0a88e7..921d92ec30 100644 --- a/web/src/components/storage/DeviceSelectionDialog.jsx +++ b/web/src/components/storage/DeviceSelectionDialog.jsx @@ -28,7 +28,7 @@ import { _ } from "~/i18n"; import { deviceChildren } from "~/components/storage/utils"; import { ControlledPanels as Panels, Popup } from "~/components/core"; import { DeviceSelectorTable } from "~/components/storage"; -import { noop } from "~/utils"; +import { compact, noop } from "~/utils"; /** * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget @@ -46,21 +46,22 @@ const OPTIONS_NAME = "selection-mode"; * Renders a dialog that allows the user to select a target device for installation. * @component * - * @param {object} props - * @param {ProposalTarget} props.target - * @param {StorageDevice|undefined} props.targetDevice - * @param {StorageDevice[]} props.targetPVDevices - * @param {StorageDevice[]} props.devices - The actions to perform in the system. - * @param {boolean} [props.isOpen=false] - Whether the dialog is visible or not. - * @param {boolean} [props.isLoading=false] - Whether loading the data is in progress - * @param {() => void} [props.onCancel=noop] - * @param {(target: Target) => void} [props.onAccept=noop] + * @typedef {object} DeviceSelectionDialogProps + * @property {ProposalTarget} target + * @property {StorageDevice|undefined} targetDevice + * @property {StorageDevice[]} targetPVDevices + * @property {StorageDevice[]} devices - The actions to perform in the system. + * @property {boolean} [isOpen=false] - Whether the dialog is visible or not. + * @property {boolean} [isLoading=false] - Whether loading the data is in progress + * @property {() => void} [onCancel=noop] + * @property {(target: TargetConfig) => void} [onAccept=noop] * - * @typedef {object} Target + * @typedef {object} TargetConfig * @property {string} target * @property {StorageDevice|undefined} targetDevice * @property {StorageDevice[]} targetPVDevices - + * + * @param {DeviceSelectionDialogProps} props */ export default function DeviceSelectionDialog({ target: defaultTarget, @@ -157,7 +158,7 @@ devices.").split(/[[\]]/); { @@ -124,8 +265,9 @@ describe("DeviceSelectionDialog", () => { props = { isOpen: true, target: "DISK", + targetDevice: undefined, targetPVDevices: [], - devices: [sda, sdb, sdc], + devices: [vda, vdb, vdc, md0, raid, multipath, dasd], onCancel: jest.fn(), onAccept: jest.fn() }; @@ -144,7 +286,7 @@ describe("DeviceSelectionDialog", () => { describe("if the target is a disk", () => { beforeEach(() => { props.target = "DISK"; - props.targetDevice = sda; + props.targetDevice = vda; }); it("selects the disk option by default", () => { @@ -166,9 +308,9 @@ describe("DeviceSelectionDialog", () => { it("shows the target disk as selected", () => { plainRender(); const selector = screen.getByRole("grid", { name: /selector for target disk/i }); - expectSelector(selector).toHaveCheckedOption(/sda/); - expectSelector(selector).not.toHaveCheckedOption(/sdb/); - expectSelector(selector).not.toHaveCheckedOption(/sdc/); + expectSelector(selector).toHaveCheckedOption(/\/dev\/vda/); + expectSelector(selector).not.toHaveCheckedOption(/\/dev\/vdb/); + expectSelector(selector).not.toHaveCheckedOption(/\/dev\/vdc/); }); it("allows to switch to new LVM", async () => { @@ -191,7 +333,7 @@ describe("DeviceSelectionDialog", () => { describe("if the target is a new LVM volume group", () => { beforeEach(() => { props.target = "NEW_LVM_VG"; - props.targetPVDevices = [sda, sdc]; + props.targetPVDevices = [vda, vdc]; }); it("selects the LVM option by default", () => { @@ -213,9 +355,9 @@ describe("DeviceSelectionDialog", () => { it("shows the current candidate devices as selected", () => { plainRender(); const selector = screen.getByRole("grid", { name: /selector for new lvm/i }); - expectSelector(selector).toHaveCheckedOption(/sda/); - expectSelector(selector).not.toHaveCheckedOption(/sdb/); - expectSelector(selector).toHaveCheckedOption(/sdc/); + expectSelector(selector).toHaveCheckedOption(/\/dev\/vda/); + expectSelector(selector).not.toHaveCheckedOption(/\/dev\/vdb/); + expectSelector(selector).toHaveCheckedOption(/\/dev\/vdc/); }); it("allows to switch to disk", async () => { @@ -247,7 +389,7 @@ describe("DeviceSelectionDialog", () => { describe("if the option to select a disk as target device is selected", () => { beforeEach(() => { props.target = "NEW_LVM_VG"; - props.targetDevice = sda; + props.targetDevice = vda; }); it("calls onAccept with the selected target and disk on accept", async () => { @@ -257,14 +399,14 @@ describe("DeviceSelectionDialog", () => { await user.click(diskOption); const selector = screen.getByRole("grid", { name: /selector for target disk/i }); - await clickSelectorOption(user, selector, /sdb/); + await clickSelectorOption(user, selector, /\/dev\/vdb/); const accept = screen.getByRole("button", { name: "Confirm" }); await user.click(accept); expect(props.onAccept).toHaveBeenCalledWith({ target: "DISK", - targetDevice: sdb, + targetDevice: vdb, targetPVDevices: [] }); }); @@ -273,7 +415,7 @@ describe("DeviceSelectionDialog", () => { describe("if the option to create a new LVM volume group is selected", () => { beforeEach(() => { props.target = "DISK"; - props.targetDevice = sdb; + props.targetDevice = vdb; }); it("calls onAccept with the selected target and the candidate devices on accept", async () => { @@ -283,17 +425,94 @@ describe("DeviceSelectionDialog", () => { await user.click(lvmOption); const selector = screen.getByRole("grid", { name: /selector for new lvm/i }); - await clickSelectorOption(user, selector, /sda/); - await clickSelectorOption(user, selector, /sdc/); + await clickSelectorOption(user, selector, /\/dev\/vda/); + await clickSelectorOption(user, selector, /\/dev\/vdb/); const accept = screen.getByRole("button", { name: "Confirm" }); await user.click(accept); expect(props.onAccept).toHaveBeenCalledWith({ target: "NEW_LVM_VG", - targetDevice: sdb, - targetPVDevices: [sda, sdc] + targetDevice: vdb, + targetPVDevices: [vda, vdb] + }); + }); + }); + + describe("content", () => { + const row = (name) => { + const selector = screen.getByRole("grid", { name: /selector for target disk/i }); + return within(selector).getByRole("row", { name }); + }; + + it("renders the device model", () => { + plainRender(); + within(row(/\/dev\/vda/)).getByText("Micron 1100 SATA"); + }); + + describe("when there is a SDCard", () => { + it("renders 'SD Card'", () => { + plainRender(); + within(row(/\/dev\/vda/)).getByText("SD Card"); + }); + }); + + describe("when there is a software RAID", () => { + it("renders its level", () => { + plainRender(); + within(row(/\/dev\/md0/)).getByText("Software RAID0"); + }); + + it("renders its members", () => { + plainRender(); + const mdRow = row(/\/dev\/md0/); + within(mdRow).getByText(/Members/); + within(mdRow).getByText(/vdb/); }); }); + + describe("when device is RAID", () => { + it("renders its devices", () => { + plainRender(); + const raidRow = row(/\/dev\/mapper\/isw_ddgdcbibhd_244/); + within(raidRow).getByText(/Devices/); + within(raidRow).getByText(/vda/); + within(raidRow).getByText(/vdb/); + }); + }); + + describe("when device is a multipath", () => { + it("renders 'Multipath'", () => { + plainRender(); + within(row(/\/dev\/mapper\/36005076305ffc73a00000000000013b4/)).getByText("Multipath"); + }); + + it("renders its wires", () => { + plainRender(); + const multipathRow = row(/\/dev\/mapper\/36005076305ffc73a00000000000013b4/); + within(multipathRow).getByText(/Wires/); + within(multipathRow).getByText(/vda/); + within(multipathRow).getByText(/vdb/); + }); + }); + + describe("when device is DASD", () => { + it("renders its bus id", () => { + plainRender(); + within(row(/\/dev\/dasda/)).getByText("DASD 0.0.0150"); + }); + }); + + it("renders the partition table info", () => { + plainRender(); + within(row(/\/dev\/vda/)).getByText("GPT with 2 partitions"); + }); + + it("renders systems info", () => { + plainRender(); + const vdaRow = row(/\/dev\/vda/); + within(vdaRow).getByText("Windows 11"); + within(vdaRow).getByText("openSUSE Leap 15.2"); + }); }); }); diff --git a/web/src/components/storage/DeviceSelectorTable.jsx b/web/src/components/storage/DeviceSelectorTable.jsx index 0653019f70..839ad265fd 100644 --- a/web/src/components/storage/DeviceSelectorTable.jsx +++ b/web/src/components/storage/DeviceSelectorTable.jsx @@ -19,33 +19,192 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; +import { sprintf } from "sprintf-js"; + import { _ } from "~/i18n"; -import { deviceSize } from '~/components/storage/utils'; -import { DeviceExtendedInfo, DeviceContentInfo } from "~/components/storage"; -import { ExpandableSelector } from "~/components/core"; +import { deviceBaseName } from "~/components/storage/utils"; +import { + DeviceName, DeviceDetails, DeviceSize, FilesystemLabel, toStorageDevice +} from "~/components/storage/device-utils"; +import { ExpandableSelector, If } from "~/components/core"; +import { Icon } from "~/components/layout"; /** - * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings + * @typedef {import("../core/ExpandableSelector").ExpandableSelectorColumn} ExpandableSelectorColumn + * @typedef {import("../core/ExpandableSelector").ExpandableSelectorProps} ExpandableSelectorProps + * @typedef {import("~/client/storage").PartitionSlot} PartitionSlot * @typedef {import ("~/client/storage").StorageDevice} StorageDevice */ -const DeviceInfo = ({ device }) => { - if (!device.sid) return _("Unused space"); +/** + * @component + * + * @param {object} props + * @param {PartitionSlot|StorageDevice} props.item + */ +const DeviceInfo = ({ item }) => { + const device = toStorageDevice(item); + if (!device) return null; + + const DeviceType = () => { + let type; + + switch (device.type) { + case "multipath": { + // TRANSLATORS: multipath device type + type = _("Multipath"); + break; + } + case "dasd": { + // TRANSLATORS: %s is replaced by the device bus ID + type = sprintf(_("DASD %s"), device.busId); + break; + } + case "md": { + // TRANSLATORS: software RAID device, %s is replaced by the RAID level, e.g. RAID-1 + type = sprintf(_("Software %s"), device.level.toUpperCase()); + break; + } + case "disk": { + if (device.sdCard) { + type = _("SD Card"); + } else { + const technology = device.transport || device.bus; + type = technology + // TRANSLATORS: %s is substituted by the type of disk like "iSCSI" or "SATA" + ? sprintf(_("%s disk"), technology) + : _("Disk"); + } + } + } + + return {type}} />; + }; + + const DeviceModel = () => { + if (!device.model || device.model === "") return null; + + return
{device.model}
; + }; + + const MDInfo = () => { + if (device.type !== "md" || !device.devices) return null; + + const members = device.devices.map(deviceBaseName); + + // TRANSLATORS: RAID details, %s is replaced by list of devices used by the array + return
{sprintf(_("Members: %s"), members.sort().join(", "))}
; + }; + + const RAIDInfo = () => { + if (device.type !== "raid") return null; + + const devices = device.devices.map(deviceBaseName); + + // TRANSLATORS: RAID details, %s is replaced by list of devices used by the array + return
{sprintf(_("Devices: %s"), devices.sort().join(", "))}
; + }; + + const MultipathInfo = () => { + if (device.type !== "multipath") return null; + + const wires = device.wires.map(deviceBaseName); + + // TRANSLATORS: multipath details, %s is replaced by list of connections used by the device + return
{sprintf(_("Wires: %s"), wires.sort().join(", "))}
; + }; + + return ( +
+ + + + + + +
+ ); +}; + +/** + * @component + * + * @param {object} props + * @param {PartitionSlot|StorageDevice} props.item + */ +const DeviceExtendedDetails = ({ item }) => { + const device = toStorageDevice(item); + + if (!device || ["partition", "lvmLv"].includes(device.type)) + return ; + + // TODO: there is a lot of room for improvement here, but first we would need + // device.description (comes from YaST) to be way more granular + const Description = () => { + if (device.partitionTable) { + const type = device.partitionTable.type.toUpperCase(); + const numPartitions = device.partitionTable.partitions.length; + + // TRANSLATORS: disk partition info, %s is replaced by partition table + // type (MS-DOS or GPT), %d is the number of the partitions + return sprintf(_("%s with %d partitions"), type, numPartitions); + } + + if (!!device.model && device.model === device.description) { + // TRANSLATORS: status message, no existing content was found on the disk, + // i.e. the disk is completely empty + return _("No content found"); + } + + return
{device.description}
; + }; - return ; + const Systems = () => { + if (!device.systems || device.systems.length === 0) return null; + + const System = ({ system }) => { + const logo = /windows/i.test(system) ? "windows_logo" : "linux_logo"; + + return
{system}
; + }; + + return device.systems.map((s, i) => ); + }; + + return ( +
+ + +
+ ); }; -const deviceColumns = [ - { name: _("Device"), value: (device) => }, - { name: _("Content"), value: (device) => }, - { name: _("Size"), value: (device) => deviceSize(device.size), classNames: "sizes-column" } +/** @type {ExpandableSelectorColumn[]} */ +const columns = [ + { name: _("Device"), value: (item) => }, + { name: _("Details"), value: (item) => }, + { name: _("Size"), value: (item) => , classNames: "sizes-column" } ]; -export default function DeviceSelectorTable({ devices, selected, ...props }) { +/** + * Table for selecting the installation device. + * @component + * + * @typedef {object} DeviceSelectorTableBaseProps + * @property {StorageDevice[]} devices + * @property {StorageDevice[]} selectedDevices + * + * @typedef {DeviceSelectorTableBaseProps & ExpandableSelectorProps} DeviceSelectorTableProps + * + * @param {DeviceSelectorTableProps} props + */ +export default function DeviceSelectorTable({ devices, selectedDevices, ...props }) { return ( { @@ -53,7 +212,7 @@ export default function DeviceSelectorTable({ devices, selected, ...props }) { return "dimmed-row"; } }} - itemsSelected={selected} + itemsSelected={selectedDevices} className="devices-table" {...props} /> diff --git a/web/src/components/storage/DevicesManager.js b/web/src/components/storage/DevicesManager.js index 5f70318564..bd645307a4 100644 --- a/web/src/components/storage/DevicesManager.js +++ b/web/src/components/storage/DevicesManager.js @@ -33,6 +33,8 @@ import { compact, uniq } from "~/utils"; */ export default class DevicesManager { /** + * @constructor + * * @param {StorageDevice[]} system - Devices representing the current state of the system. * @param {StorageDevice[]} staging - Devices representing the target state of the system. * @param {Action[]} actions - Actions to perform from system to staging. @@ -45,6 +47,7 @@ export default class DevicesManager { /** * System device with the given SID. + * @method * * @param {Number} sid * @returns {StorageDevice|undefined} @@ -55,6 +58,7 @@ export default class DevicesManager { /** * Staging device with the given SID. + * @method * * @param {Number} sid * @returns {StorageDevice|undefined} @@ -65,6 +69,7 @@ export default class DevicesManager { /** * Whether the given device exists in system. + * @method * * @param {StorageDevice} device * @returns {Boolean} @@ -75,6 +80,7 @@ export default class DevicesManager { /** * Whether the given device exists in staging. + * @method * * @param {StorageDevice} device * @returns {Boolean} @@ -85,6 +91,7 @@ export default class DevicesManager { /** * Whether the given device is going to be formatted. + * @method * * @param {StorageDevice} device * @returns {Boolean} @@ -100,6 +107,7 @@ export default class DevicesManager { /** * Whether the given device is going to be shrunk. + * @method * * @param {StorageDevice} device * @returns {Boolean} @@ -110,6 +118,7 @@ export default class DevicesManager { /** * Amount of bytes the given device is going to be shrunk. + * @method * * @param {StorageDevice} device * @returns {Number} @@ -126,13 +135,14 @@ export default class DevicesManager { /** * Disk devices and LVM volume groups used for the installation. + * @method * * @note The used devices are extracted from the actions. * * @returns {StorageDevice[]} */ usedDevices() { - const isTarget = (device) => device.isDrive || device.type === "lvmVg"; + const isTarget = (device) => device.isDrive || ["md", "lvmVg"].includes(device.type); // Check in system devices to detect removals. const targetSystem = this.system.filter(isTarget); @@ -147,6 +157,7 @@ export default class DevicesManager { /** * Devices deleted. + * @method * * @note The devices are extracted from the actions. * @@ -158,6 +169,7 @@ export default class DevicesManager { /** * Systems deleted. + * @method * * @returns {string[]} */ diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index dc971ff49c..892f17ffa4 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -29,21 +29,20 @@ import { Table, Thead, Tr, Th, Tbody, Td } from '@patternfly/react-table'; import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; -import { If, ExpandableField, RowActions, Tip } from '~/components/core'; -import VolumeDialog from '~/components/storage/VolumeDialog'; -import VolumeLocationDialog from '~/components/storage/VolumeLocationDialog'; +import BootConfigField from "~/components/storage/BootConfigField"; import { - deviceSize, hasSnapshots, isTransactionalRoot, isTransactionalSystem + deviceSize, hasSnapshots, isTransactionalRoot, isTransactionalSystem, reuseDevice } from '~/components/storage/utils'; -import SnapshotsField from "~/components/storage/SnapshotsField"; -import BootConfigField from "~/components/storage/BootConfigField"; +import { If, ExpandableField, RowActions, Tip } from '~/components/core'; import { noop } from "~/utils"; +import SnapshotsField from "~/components/storage/SnapshotsField"; +import VolumeDialog from '~/components/storage/VolumeDialog'; +import VolumeLocationDialog from '~/components/storage/VolumeLocationDialog'; /** * @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 */ @@ -55,7 +54,7 @@ import { noop } from "~/utils"; */ const SizeText = ({ volume }) => { let targetSize; - if (volume.target === "FILESYSTEM" || volume.target === "DEVICE") + if (reuseDevice(volume)) targetSize = volume.targetDevice.size; const minSize = deviceSize(targetSize || volume.minSize); @@ -238,6 +237,90 @@ const BootLabel = ({ bootDevice, configureBoot }) => { ); }; +// TODO: Extract VolumesTable or at least VolumeRow and all related internal +// components to a new file. + +/** + * @component + * @param {object} props + * @param {Volume} props.volume + */ +const VolumeSizeLimits = ({ volume }) => { + const isAuto = volume.autoSize; + + return ( +
+ {SizeText({ volume })} + {/* TRANSLATORS: device flag, the partition size is automatically computed */} + {_("auto")}} + /> +
+ ); +}; + +/** + * @component + * @param {object} props + * @param {Volume} props.volume + */ +const VolumeDetails = ({ volume }) => { + const snapshots = hasSnapshots(volume); + const transactional = isTransactionalRoot(volume); + + 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) + return _("Transactional Btrfs"); + if (snapshots) + return _("Btrfs with snapshots"); + + return volume.fsType; +}; + +/** + * @component + * @param {object} props + * @param {Volume} props.volume + * @param {ProposalTarget} props.target + */ +const VolumeLocation = ({ 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") + // 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") + return volume.targetDevice?.name || ""; + if (target === "NEW_LVM_VG") + return _("Logical volume at system LVM"); + + return _("Partition at installation disk"); +}; + +/** + * @component + * @param {object} props + * @param {Volume} props.volume + * @param {() => void} props.onEdit + * @param {() => void} props.onResetLocation + * @param {() => void} props.onLocation + * @param {() => void} props.onDelete + */ +const VolumeActions = ({ volume, onEdit, onResetLocation, onLocation, onDelete }) => { + const actions = [ + { title: _("Edit"), onClick: onEdit }, + volume.target !== "DEFAULT" && { title: _("Reset location"), onClick: onResetLocation }, + { title: _("Change location"), onClick: onLocation }, + !volume.outline.required && { title: _("Delete"), onClick: onDelete, isDanger: true } + ]; + + return ; +}; + /** * Renders a table row with the information and actions for a volume * @component @@ -247,21 +330,21 @@ const BootLabel = ({ bootDevice, configureBoot }) => { * @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 + * @param {StorageDevice[]} [props.volumeDevices=[]] - Devices available for installation + * @param {ProposalTarget} [props.target] + * @param {StorageDevice[]} [props.targetDevices] - Device selected for installation, if target is a disk * @param {boolean} props.isLoading - Whether to show the row as loading * @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 + * @param {() => void} [props.onDelete=noop] - Function to use for deleting the volume */ const VolumeRow = ({ columns, volume, volumes, templates, - devices, + volumeDevices, target, - targetDevice, + targetDevices, isLoading, onEdit = noop, onDelete = noop @@ -275,6 +358,10 @@ const VolumeRow = ({ const closeDialog = () => setDialog(undefined); + const onResetLocationClick = () => { + onEdit({ ...volume, target: "DEFAULT", targetDevice: undefined }); + }; + const acceptForm = (volume) => { closeDialog(); onEdit(volume); @@ -283,98 +370,6 @@ const VolumeRow = ({ const isEditDialogOpen = dialog === "edit"; const isLocationDialogOpen = dialog === "location"; - /** - * @component - * @param {object} props - * @param {Volume} props.volume - */ - const SizeLimits = ({ volume }) => { - const isAuto = volume.autoSize; - - return ( -
- {SizeText({ volume })} - {/* TRANSLATORS: device flag, the partition size is automatically computed */} - {_("auto")}} /> -
- ); - }; - - /** - * @component - * @param {object} props - * @param {Volume} props.volume - */ - const Details = ({ volume }) => { - const snapshots = hasSnapshots(volume); - const transactional = isTransactionalRoot(volume); - - 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) - return _("Transactional Btrfs"); - if (snapshots) - return _("Btrfs with snapshots"); - - return volume.fsType; - }; - - /** - * @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") - // 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") - return volume.targetDevice?.name || ""; - if (target === "NEW_LVM_VG") - return _("Logical volume at system LVM"); - - return _("Partition at installation disk"); - }; - - /** - * @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: () => onDeleteClick(volume), - isDanger: true - }, - edit: { - title: _("Edit"), - onClick: onEditClick - }, - location: { - title: _("Change location"), - onClick: onLocationClick - } - }; - - if (!volume.outline.required) return Object.values(actions); - - return [actions.edit, actions.location]; - }; - - return ; - }; - if (isLoading) { return ( @@ -387,15 +382,16 @@ const VolumeRow = ({ <> {volume.mountPath} -
- - + + + @@ -418,9 +414,9 @@ const VolumeRow = ({ @@ -437,18 +433,18 @@ const VolumeRow = ({ * @param {object} props * @param {Volume[]} props.volumes - Volumes to show * @param {Volume[]} props.templates - List of available templates - * @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 {StorageDevice[]} props.volumeDevices + * @param {ProposalTarget} props.target + * @param {StorageDevice[]} props.targetDevices * @param {boolean} props.isLoading - Whether to show the table as loading * @param {(volumes: Volume[]) => void} props.onVolumesChange - Function to submit changes in volumes */ const VolumesTable = ({ volumes, templates, - devices, + volumeDevices, target, - targetDevice, + targetDevices, isLoading, onVolumesChange }) => { @@ -487,12 +483,12 @@ const VolumesTable = ({ volume={volume} volumes={volumes} templates={templates} - devices={devices} + volumeDevices={volumeDevices} target={target} - targetDevice={targetDevice} + targetDevices={targetDevices} isLoading={isLoading} onEdit={editVolume} - onDelete={deleteVolume} + onDelete={() => deleteVolume(volume)} /> ); }); @@ -614,9 +610,10 @@ const AddVolumeButton = ({ options, onClick }) => { * @param {object} props * @param {Volume[]} props.volumes * @param {Volume[]} props.templates - * @param {StorageDevice[]} props.devices + * @param {StorageDevice[]} props.availableDevices + * @param {StorageDevice[]} props.volumeDevices * @param {ProposalTarget} props.target - * @param {StorageDevice|undefined} props.targetDevice + * @param {StorageDevice[]} props.targetDevices * @param {boolean} props.configureBoot * @param {StorageDevice|undefined} props.bootDevice * @param {StorageDevice|undefined} props.defaultBootDevice @@ -627,9 +624,10 @@ const AddVolumeButton = ({ options, onClick }) => { const Advanced = ({ volumes, templates, - devices, + availableDevices, + volumeDevices, target, - targetDevice, + targetDevices, configureBoot, bootDevice, defaultBootDevice, @@ -719,9 +717,9 @@ const Advanced = ({ @@ -750,7 +748,7 @@ const Advanced = ({ configureBoot={configureBoot} bootDevice={bootDevice} defaultBootDevice={defaultBootDevice} - devices={devices} + availableDevices={availableDevices} isLoading={isLoading} onChange={onBootChange} /> @@ -768,9 +766,10 @@ const Advanced = ({ * @typedef {object} PartitionsFieldProps * @property {Volume[]} volumes - Volumes to show * @property {Volume[]} templates - Templates to use for new volumes - * @property {StorageDevice[]} devices - Devices available for installation + * @property {StorageDevice[]} availableDevices - Devices available for installation + * @property {StorageDevice[]} volumeDevices - Devices that can be selected as target for a volume * @property {ProposalTarget} target - Installation target - * @property {StorageDevice|undefined} targetDevice - Device selected for installation, if target is a disk + * @property {StorageDevice[]} targetDevices * @property {boolean} configureBoot - Whether to configure boot partitions. * @property {StorageDevice|undefined} bootDevice - Device to use for creating boot partitions. * @property {StorageDevice|undefined} defaultBootDevice - Default device for boot partitions if no device has been indicated yet. @@ -787,9 +786,10 @@ const Advanced = ({ export default function PartitionsField({ volumes, templates, - devices, + availableDevices, + volumeDevices, target, - targetDevice, + targetDevices, configureBoot, bootDevice, defaultBootDevice, @@ -813,9 +813,10 @@ export default function PartitionsField({ { props = { volumes: [rootVolume, swapVolume], templates: [], - devices: [], + availableDevices: [], + volumeDevices: [sda], target: "DISK", - targetDevice: undefined, + targetDevices: [], configureBoot: false, bootDevice: undefined, defaultBootDevice: undefined, @@ -348,6 +349,44 @@ describe("if there are volumes", () => { within(popup).getByText("Location for /home file system"); }); + // FIXME: improve at least the test description + it("does not allow resetting the volume location when already using the default location", async () => { + const { user } = await expandField(); + + 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); + expect(within(row).queryByRole("menuitem", { name: "Reset location" })).toBeNull(); + }); + + describe("and a volume has a non default location", () => { + beforeEach(() => { + props.volumes = [{ ...homeVolume, target: "NEW_PARTITION", targetDevice: sda }]; + }); + + it("allows resetting the volume location", async () => { + const { user } = await expandField(); + + const [, body] = await screen.findAllByRole("rowgroup"); + const row = within(body).getByRole("row", { name: "/home XFS at least 1 KiB Partition at /dev/sda" }); + const actions = within(row).getByRole("button", { name: "Actions" }); + await user.click(actions); + const resetLocationAction = within(row).queryByRole("menuitem", { name: "Reset location" }); + await user.click(resetLocationAction); + expect(props.onVolumesChange).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ mountPath: "/home", target: "DEFAULT", targetDevice: undefined }) + ]) + ); + + // NOTE: sadly we cannot perform the below check because the component is + // always receiving the same mocked props and will still having a /home as + // "Partition at /dev/sda" + // await within(body).findByRole("row", { name: "/home XFS at least 1 KiB Partition at installation device" }); + }); + }); + describe("and there is a transactional Btrfs root volume", () => { beforeEach(() => { props.volumes = [{ ...rootVolume, snapshots: true, transactional: true }]; diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index 16262c5fa6..32fb7b751e 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -38,6 +38,7 @@ const initialState = { // which UI item is being changed by user changing: undefined, availableDevices: [], + volumeDevices: [], volumeTemplates: [], encryptionMethods: [], settings: {}, @@ -63,6 +64,11 @@ const reducer = (state, action) => { return { ...state, availableDevices }; } + case "UPDATE_VOLUME_DEVICES": { + const { volumeDevices } = action.payload; + return { ...state, volumeDevices }; + } + case "UPDATE_ENCRYPTION_METHODS": { const { encryptionMethods } = action.payload; return { ...state, encryptionMethods }; @@ -132,6 +138,10 @@ export default function ProposalPage() { return await cancellablePromise(client.proposal.getAvailableDevices()); }, [client, cancellablePromise]); + const loadVolumeDevices = useCallback(async () => { + return await cancellablePromise(client.proposal.getVolumeDevices()); + }, [client, cancellablePromise]); + const loadEncryptionMethods = useCallback(async () => { return await cancellablePromise(client.proposal.getEncryptionMethods()); }, [client, cancellablePromise]); @@ -180,6 +190,9 @@ export default function ProposalPage() { const availableDevices = await loadAvailableDevices(); dispatch({ type: "UPDATE_AVAILABLE_DEVICES", payload: { availableDevices } }); + const volumeDevices = await loadVolumeDevices(); + dispatch({ type: "UPDATE_VOLUME_DEVICES", payload: { volumeDevices } }); + const encryptionMethods = await loadEncryptionMethods(); dispatch({ type: "UPDATE_ENCRYPTION_METHODS", payload: { encryptionMethods } }); @@ -196,7 +209,7 @@ export default function ProposalPage() { dispatch({ type: "UPDATE_ERRORS", payload: { errors } }); if (result !== undefined) dispatch({ type: "STOP_LOADING" }); - }, [calculateProposal, cancellablePromise, client, loadAvailableDevices, loadDevices, loadEncryptionMethods, loadErrors, loadProposalResult, loadVolumeTemplates]); + }, [calculateProposal, cancellablePromise, client, loadAvailableDevices, loadVolumeDevices, loadDevices, loadEncryptionMethods, loadErrors, loadProposalResult, loadVolumeTemplates]); const calculate = useCallback(async (settings) => { dispatch({ type: "START_LOADING" }); @@ -258,6 +271,7 @@ export default function ProposalPage() { /> { // @ts-expect-error Some methods have to be private to avoid type complaint. proposal: { getAvailableDevices: jest.fn().mockResolvedValue([vda, vdb]), + getVolumeDevices: jest.fn().mockResolvedValue([vda, vdb]), getEncryptionMethods: jest.fn().mockResolvedValue([]), getProductMountPoints: jest.fn().mockResolvedValue([]), getResult: jest.fn().mockResolvedValue(proposalResult), diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 913d2cdbc7..e0ad5822a4 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -25,10 +25,10 @@ import React, { useState } from "react"; import { Button, Skeleton } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; import { _, n_ } from "~/i18n"; -import { deviceChildren, deviceSize } from "~/components/storage/utils"; import DevicesManager from "~/components/storage/DevicesManager"; -import { If, Section, Reminder, Tag, TreeTable } from "~/components/core"; -import { ProposalActionsDialog, FilesystemLabel } from "~/components/storage"; +import { If, Section, Reminder } from "~/components/core"; +import { ProposalActionsDialog } from "~/components/storage"; +import ProposalResultTable from "~/components/storage/ProposalResultTable"; /** * @typedef {import ("~/client/storage").Action} Action @@ -98,109 +98,6 @@ const ActionsInfo = ({ actions }) => { ); }; -/** - * Renders a TreeTable rendering the devices proposal result. - * @component - * - * @param {object} props - * @param {DevicesManager} props.devicesManager - */ -const DevicesTreeTable = ({ devicesManager }) => { - const renderDeviceName = (item) => { - let name = item.sid && item.name; - // NOTE: returning a fragment here to avoid a weird React complaint when using a PF/Table + - // treeRow props. - if (!name) return <>; - - if (["partition", "lvmLv"].includes(item.type)) - name = name.split("/").pop(); - - return ( -
- {name} -
- ); - }; - - const renderNewLabel = (item) => { - if (!item.sid) return; - - // FIXME New PVs over a disk is not detected as new. - if (!devicesManager.existInSystem(item) || devicesManager.hasNewFilesystem(item)) - return {_("New")}; - }; - - const renderContent = (item) => { - if (!item.sid) - return _("Unused space"); - if (!item.partitionTable && item.systems?.length > 0) - return item.systems.join(", "); - - return item.description; - }; - - const renderPTableType = (item) => { - // TODO: Create a map for partition table types. - const type = item.partitionTable?.type; - if (type) return {type.toUpperCase()}; - }; - - const renderDetails = (item) => { - return ( - <> -
{renderNewLabel(item)}
-
{renderContent(item)} {renderPTableType(item)}
- - ); - }; - - const renderResizedLabel = (item) => { - if (!item.sid || !devicesManager.isShrunk(item)) return; - - const sizeBefore = devicesManager.systemDevice(item.sid).size; - - return ( - - { - // TRANSLATORS: Label to indicate the device size before resizing, where %s is replaced by - // the original size (e.g., 3.00 GiB). - sprintf(_("Before %s"), deviceSize(sizeBefore)) - } - - ); - }; - - const renderSize = (item) => { - return ( -
- {renderResizedLabel(item)} - {deviceSize(item.size)} -
- ); - }; - - const renderMountPoint = (item) => item.sid && {item.filesystem?.mountPath}; - const devices = devicesManager.usedDevices(); - - return ( - deviceChildren(d)} - rowClassNames={(item) => { - if (!item.sid) return "dimmed-row"; - }} - className="proposal-result" - /> - ); -}; - /** * @todo Create a component for rendering a customized skeleton */ @@ -236,7 +133,7 @@ const SectionContent = ({ system, staging, actions, errors }) => { systems={devicesManager.deletedSystems()} /> - + ); }; @@ -245,13 +142,14 @@ const SectionContent = ({ system, staging, actions, errors }) => { * Section holding the proposal result and actions to perform in the system * @component * - * @param {object} props - * @param {StorageDevice[]} [props.system=[]] - * @param {StorageDevice[]} [props.staging=[]] - * @param {Action[]} [props.actions=[]] - * @param {ValidationError[]} [props.errors=[]] - Validation errors - * @param {boolean} [props.isLoading=false] - Whether the section content should be rendered as loading - * @param {symbol} [props.changing=undefined] - Which part of the configuration is being changed by user + * @typedef {object} ProposalResultSectionProps + * @property {StorageDevice[]} [system=[]] + * @property {StorageDevice[]} [staging=[]] + * @property {Action[]} [actions=[]] + * @property {ValidationError[]} [errors=[]] - Validation errors + * @property {boolean} [isLoading=false] - Whether the section content should be rendered as loading + * + * @param {ProposalResultSectionProps} props */ export default function ProposalResultSection({ system = [], diff --git a/web/src/components/storage/ProposalResultSection.test.jsx b/web/src/components/storage/ProposalResultSection.test.jsx index 95f098f10e..37b1b338c7 100644 --- a/web/src/components/storage/ProposalResultSection.test.jsx +++ b/web/src/components/storage/ProposalResultSection.test.jsx @@ -19,14 +19,21 @@ * 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 { ProposalResultSection } from "~/components/storage"; import { devices, actions } from "./test-data/full-result-example"; +/** + * @typedef {import("./ProposalResultSection").ProposalResultSectionProps} ProposalResultSectionProps + */ + const errorMessage = "Something went wrong, proposal not possible"; const errors = [{ severity: 0, message: errorMessage }]; +/** @type {ProposalResultSectionProps} */ const defaultProps = { system: devices.system, staging: devices.staging, actions }; describe("ProposalResultSection", () => { @@ -74,7 +81,7 @@ describe("ProposalResultSection", () => { // affected systems are rendered in the warning summary const props = { ...defaultProps, - actions: [{ device: 79, delete: true }] + actions: [{ device: 79, subvol: false, delete: true, text: "" }] }; plainRender(); diff --git a/web/src/components/storage/ProposalResultTable.jsx b/web/src/components/storage/ProposalResultTable.jsx new file mode 100644 index 0000000000..d4350f4016 --- /dev/null +++ b/web/src/components/storage/ProposalResultTable.jsx @@ -0,0 +1,156 @@ +/* + * 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 { sprintf } from "sprintf-js"; +import { _ } from "~/i18n"; +import { + DeviceName, DeviceDetails, DeviceSize, toStorageDevice +} from "~/components/storage/device-utils"; +import { deviceChildren, deviceSize } from "~/components/storage/utils"; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +import DevicesManager from "~/components/storage/DevicesManager"; +import { If, Tag, TreeTable } from "~/components/core"; + +/** + * @typedef {import("~/client/storage").PartitionSlot} PartitionSlot + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import("../core/TreeTable").TreeTableColumn} TreeTableColumn + * @typedef {StorageDevice | PartitionSlot} TableItem + */ + +/** + * @component + * @param {object} props + * @param {TableItem} props.item + */ +const MountPoint = ({ item }) => { + const device = toStorageDevice(item); + + if (!(device && device.filesystem?.mountPath)) return null; + + return {device.filesystem.mountPath}; +}; + +/** + * @component + * @param {object} props + * @param {TableItem} props.item + * @param {DevicesManager} props.devicesManager + */ +const DeviceCustomDetails = ({ item, devicesManager }) => { + const isNew = () => { + const device = toStorageDevice(item); + if (!device) return false; + + // FIXME New PVs over a disk is not detected as new. + return !devicesManager.existInSystem(device) || devicesManager.hasNewFilesystem(device); + }; + + return ( + <> +
+ {_("New")}} /> +
+ + + ); +}; + +/** + * @component + * @param {object} props + * @param {TableItem} props.item + * @param {DevicesManager} props.devicesManager + */ +const DeviceCustomSize = ({ item, devicesManager }) => { + const device = toStorageDevice(item); + const isResized = device && devicesManager.isShrunk(device); + const sizeBefore = isResized ? devicesManager.systemDevice(device.sid).size : item.size; + + return ( +
+ + { + // TRANSLATORS: Label to indicate the device size before resizing, where %s is + // replaced by the original size (e.g., 3.00 GiB). + sprintf(_("Before %s"), deviceSize(sizeBefore)) + } + + } + /> + +
+ ); +}; + +/** @type {(devicesManager: DevicesManager) => TreeTableColumn[] } */ +const columns = (devicesManager) => { + /** @type {(item: TableItem) => React.ReactNode} */ + const renderDevice = (item) => ; + + /** @type {(item: TableItem) => React.ReactNode} */ + const renderMountPoint = (item) => ; + + /** @type {(item: TableItem) => React.ReactNode} */ + const renderDetails = (item) => ; + + /** @type {(item: TableItem) => React.ReactNode} */ + const renderSize = (item) => ; + + return [ + { name: _("Device"), value: renderDevice }, + { name: _("Mount Point"), value: renderMountPoint }, + { name: _("Details"), value: renderDetails, classNames: "details-column" }, + { name: _("Size"), value: renderSize, classNames: "sizes-column" } + ]; +}; + +/** + * Renders the proposal result. + * @component + * + * @typedef {object} ProposalResultTableProps + * @property {DevicesManager} devicesManager + * + * @param {ProposalResultTableProps} props + */ +export default function ProposalResultTable({ devicesManager }) { + const devices = devicesManager.usedDevices(); + + return ( + { + if (!item.sid) return "dimmed-row"; + }} + className="proposal-result" + /> + ); +} diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 6e79c68633..c7e8ca9f8d 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -60,6 +60,7 @@ const showSkeleton = (loading, component, changing) => { * @typedef {object} ProposalSettingsSectionProps * @property {ProposalSettings} settings * @property {StorageDevice[]} availableDevices + * @property {StorageDevice[]} volumeDevices * @property {String[]} encryptionMethods * @property {Volume[]} volumeTemplates * @property {boolean} [isLoading=false] @@ -71,6 +72,7 @@ const showSkeleton = (loading, component, changing) => { export default function ProposalSettingsSection({ settings, availableDevices, + volumeDevices, encryptionMethods, volumeTemplates, isLoading = false, @@ -136,6 +138,8 @@ export default function ProposalSettingsSection({ const defaultBootDevice = findDevice(settings.defaultBootDevice); const spacePolicy = SPACE_POLICIES.find(p => p.id === settings.spacePolicy); + const targetDevices = compact([targetDevice, ...targetPVDevices]); + return ( <>
@@ -157,9 +161,10 @@ export default function ProposalSettingsSection({ { installationDevices: [sda, sdb] }, availableDevices: [], + volumeDevices: [], encryptionMethods: [], volumeTemplates: [], onChange: jest.fn() diff --git a/web/src/components/storage/SpaceActionsTable.jsx b/web/src/components/storage/SpaceActionsTable.jsx index ce7f615fa2..fb3741a535 100644 --- a/web/src/components/storage/SpaceActionsTable.jsx +++ b/web/src/components/storage/SpaceActionsTable.jsx @@ -23,75 +23,31 @@ import React from "react"; import { FormSelect, FormSelectOption } from "@patternfly/react-core"; +import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; -import { FilesystemLabel } from "~/components/storage"; import { deviceChildren, deviceSize } from '~/components/storage/utils'; -import { Tag, TreeTable } from "~/components/core"; -import { sprintf } from "sprintf-js"; +import { + DeviceName, DeviceDetails, DeviceSize, toStorageDevice +} from "~/components/storage/device-utils"; +import { TreeTable } from "~/components/core"; /** + * @typedef {import("~/client/storage").PartitionSlot} PartitionSlot * @typedef {import ("~/client/storage").SpaceAction} SpaceAction * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import("../core/TreeTable").TreeTableColumn} TreeTableColumn */ /** - * Column content. - * @component - * - * @param {object} props - * @param {StorageDevice} props.device - */ -const DeviceName = ({ device }) => { - let name = device.sid && device.name; - // NOTE: returning a fragment here to avoid a weird React complaint when using a PF/Table + - // treeRow props. - if (!name) return <>; - - if (["partition"].includes(device.type)) - name = name.split("/").pop(); - - return ( - {name} - ); -}; - -/** - * Column content. - * @component - * - * @param {object} props - * @param {StorageDevice} props.device - */ -const DeviceDetails = ({ device }) => { - if (!device.sid) return _("Unused space"); - - const renderContent = (device) => { - if (!device.partitionTable && device.systems?.length > 0) - return device.systems.join(", "); - - return device.description; - }; - - const renderPTableType = (device) => { - const type = device.partitionTable?.type; - if (type) return {type.toUpperCase()}; - }; - - return ( -
{renderContent(device)} {renderPTableType(device)}
- ); -}; - -/** - * Column content. * @component * * @param {object} props - * @param {StorageDevice} props.device + * @param {PartitionSlot|StorageDevice} props.item */ -const DeviceSizeDetails = ({ device }) => { - if (!device.sid || device.isDrive || device.recoverableSize === 0) return null; +const DeviceSizeDetails = ({ item }) => { + const device = toStorageDevice(item); + if (!device || device.isDrive || device.recoverableSize === 0) return null; return deviceSize(device.recoverableSize); }; @@ -131,13 +87,14 @@ const DeviceActionForm = ({ device, action, isDisabled = false, onChange }) => { * @component * * @param {object} props - * @param {StorageDevice} props.device + * @param {PartitionSlot|StorageDevice} props.item * @param {string} props.action - Possible values: "force_delete", "resize" or "keep". * @param {boolean} [props.isDisabled=false] * @param {(action: SpaceAction) => void} [props.onChange] */ -const DeviceAction = ({ device, action, isDisabled = false, onChange }) => { - if (!device.sid) return null; +const DeviceAction = ({ item, action, isDisabled = false, onChange }) => { + const device = toStorageDevice(item); + if (!device) return null; if (device.type === "partition") { return ( @@ -163,12 +120,14 @@ const DeviceAction = ({ device, action, isDisabled = false, onChange }) => { * Table for selecting the space actions of the given devices. * @component * - * @param {object} props - * @param {StorageDevice[]} props.devices - * @param {StorageDevice[]} [props.expandedDevices=[]] - Initially expanded devices. - * @param {boolean} [props.isActionDisabled=false] - Whether the action selector is disabled. - * @param {(device: StorageDevice) => string} props.deviceAction - Gets the action for a device. - * @param {(action: SpaceAction) => void} props.onActionChange + * @typedef {object} SpaceActionsTableProps + * @property {StorageDevice[]} devices + * @property {StorageDevice[]} [expandedDevices=[]] - Initially expanded devices. + * @property {boolean} [isActionDisabled=false] - Whether the action selector is disabled. + * @property {(item: PartitionSlot|StorageDevice) => string} deviceAction - Gets the action for a device. + * @property {(action: SpaceAction) => void} onActionChange + * + * @param {SpaceActionsTableProps} props */ export default function SpaceActionsTable({ devices, @@ -177,17 +136,18 @@ export default function SpaceActionsTable({ deviceAction, onActionChange, }) { + /** @type {TreeTableColumn[]} */ const columns = [ - { title: _("Device"), content: (device) => }, - { title: _("Details"), content: (device) => }, - { title: _("Size"), content: (device) => deviceSize(device.size) }, - { title: _("Shrinkable"), content: (device) => }, + { name: _("Device"), value: (item) => }, + { name: _("Details"), value: (item) => }, + { name: _("Size"), value: (item) => }, + { name: _("Shrinkable"), value: (item) => }, { - title: _("Action"), - content: (device) => ( + name: _("Action"), + value: (item) => ( @@ -201,7 +161,7 @@ export default function SpaceActionsTable({ items={devices} aria-label={_("Actions to find space")} expandedItems={expandedDevices} - itemChildren={d => deviceChildren(d)} + itemChildren={deviceChildren} rowClassNames={(item) => { if (!item.sid) return "dimmed-row"; }} diff --git a/web/src/components/storage/SpacePolicyDialog.jsx b/web/src/components/storage/SpacePolicyDialog.jsx index 77c1753214..795003fb22 100644 --- a/web/src/components/storage/SpacePolicyDialog.jsx +++ b/web/src/components/storage/SpacePolicyDialog.jsx @@ -68,18 +68,20 @@ const SpacePolicyPicker = ({ currentPolicy, onChange = noop }) => { * Renders a dialog that allows the user to select the space policy and actions. * @component * - * @param {object} props - * @param {SpacePolicy} props.policy - * @param {SpaceAction[]} props.actions - * @param {StorageDevice[]} props.devices - * @param {boolean} [props.isOpen=false] - * @param {boolean} [props.isLoading] - * @param {() => void} [props.onCancel=noop] - * @param {(spaceConfig: SpaceConfig) => void} [props.onAccept=noop] + * @typedef {object} SpacePolicyDialogProps + * @property {SpacePolicy} policy + * @property {SpaceAction[]} actions + * @property {StorageDevice[]} devices + * @property {boolean} [isOpen=false] + * @property {boolean} [isLoading] + * @property {() => void} [onCancel=noop] + * @property {(spaceConfig: SpaceConfig) => void} [onAccept=noop] * * @typedef {object} SpaceConfig * @property {SpacePolicy} spacePolicy * @property {SpaceAction[]} spaceActions + * + * @param {SpacePolicyDialogProps} props */ export default function SpacePolicyDialog({ policy: defaultPolicy, diff --git a/web/src/components/storage/SpacePolicyDialog.test.jsx b/web/src/components/storage/SpacePolicyDialog.test.jsx index a28ed8ac19..c9e44f0161 100644 --- a/web/src/components/storage/SpacePolicyDialog.test.jsx +++ b/web/src/components/storage/SpacePolicyDialog.test.jsx @@ -19,12 +19,20 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; import { screen, within } from "@testing-library/react"; import { plainRender, resetLocalStorage } from "~/test-utils"; import { SPACE_POLICIES } from "~/components/storage/utils"; import { SpacePolicyDialog } from "~/components/storage"; +/** + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import("./SpacePolicyDialog").SpacePolicyDialogProps} SpacePolicyDialogProps + */ + +/** @type {StorageDevice} */ const sda = { sid: 59, isDrive: true, @@ -39,6 +47,7 @@ const sda = { sdCard: true, active: true, name: "/dev/sda", + description: "", size: 1024, recoverableSize: 0, systems : [], @@ -46,12 +55,14 @@ const sda = { udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], }; +/** @type {StorageDevice} */ const sda1 = { sid: 60, isDrive: false, type: "partition", active: true, name: "/dev/sda1", + description: "", size: 512, recoverableSize: 128, systems : [], @@ -59,12 +70,14 @@ const sda1 = { udevPaths: [] }; +/** @type {StorageDevice} */ const sda2 = { sid: 61, isDrive: false, type: "partition", active: true, name: "/dev/sda2", + description: "", size: 512, recoverableSize: 0, systems : [], @@ -79,6 +92,7 @@ sda.partitionTable = { unpartitionedSize: 512 }; +/** @type {StorageDevice} */ const sdb = { sid: 62, isDrive: true, @@ -93,6 +107,7 @@ const sdb = { sdCard: false, active: true, name: "/dev/sdb", + description: "", size: 2048, recoverableSize: 0, systems : [], @@ -105,6 +120,7 @@ const resizePolicy = SPACE_POLICIES.find(p => p.id === "resize"); const keepPolicy = SPACE_POLICIES.find(p => p.id === "keep"); const customPolicy = SPACE_POLICIES.find(p => p.id === "custom"); +/** @type {SpacePolicyDialogProps} */ let props; const expandRow = async (user, name) => { diff --git a/web/src/components/storage/VolumeDialog.jsx b/web/src/components/storage/VolumeDialog.jsx index 67072d96bc..40ac5337cf 100644 --- a/web/src/components/storage/VolumeDialog.jsx +++ b/web/src/components/storage/VolumeDialog.jsx @@ -22,13 +22,14 @@ // @ts-check import React, { useReducer } from "react"; -import { Button, Form } from "@patternfly/react-core"; +import { Alert, 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 + DEFAULT_SIZE_UNIT, SIZE_METHODS, mountFilesystem, parseToBytes, reuseDevice, splitSize, + volumeLabel } from '~/components/storage/utils'; import { FsField, MountPathField, SizeOptionsField } from "~/components/storage/VolumeFields"; import { Popup } from '~/components/core'; @@ -62,14 +63,6 @@ import { Popup } from '~/components/core'; * @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 */ /** @@ -83,14 +76,47 @@ import { Popup } from '~/components/core'; const renderTitle = (volume, volumes) => { const isNewVolume = !volumes.includes(volume); const isProductDefined = volume.outline.productDefined; - const mountPath = volume.mountPath === "/" ? "root" : volume.mountPath; + const label = volumeLabel(volume); - if (isNewVolume && isProductDefined) return sprintf(_("Add %s file system"), mountPath); - if (!isNewVolume && isProductDefined) return sprintf(_("Edit %s file system"), mountPath); + if (isNewVolume && isProductDefined) return sprintf(_("Add %s file system"), label); + if (!isNewVolume && isProductDefined) return sprintf(_("Edit %s file system"), label); return isNewVolume ? _("Add file system") : _("Edit file system"); }; +/** + * @component + * + * @param {object} props + * @param {Volume} props.volume + */ +const VolumeAlert = ({ volume }) => { + let alert; + + if (mountFilesystem(volume)) { + alert = { + // TRANSLATORS: Warning when editing a file system. + title: _("The type and size of the file system cannot be edited."), + // TRANSLATORS: Description of a warning. The first %s is replaced by a device name (e.g., + // /dev/vda) and the second %s is replaced by a mount path (e.g., /home). + text: sprintf(_("The current file system on %s is selected to be mounted at %s."), + volume.targetDevice.name, volume.mountPath) + }; + } else if (reuseDevice(volume)) { + alert = { + // TRANSLATORS: Warning when editing a file system. + title: _("The size of the file system cannot be edited"), + // TRANSLATORS: Description of a warning. %s is replaced by a device name (e.g., /dev/vda). + text: sprintf(_("The file system is allocated at the device %s."), + volume.targetDevice.name) + }; + } + + if (!alert) return null; + + return {alert.text}; +}; + /** @fixme Redesign *Error classes. * * Having different *Error classes does not seem to be a good design. Note these classes do not @@ -603,6 +629,14 @@ const reducer = (state, action) => { * Renders a dialog that allows the user to add or edit a file system. * @component * + * @typedef {object} VolumeDialogProps + * @property {Volume} volume + * @property {Volume[]} volumes + * @property {Volume[]} templates + * @property {boolean} [isOpen=false] + * @property {() => void} onCancel + * @property {(volume: Volume) => void} onAccept + * * @param {VolumeDialogProps} props */ export default function VolumeDialog({ @@ -717,11 +751,14 @@ export default function VolumeDialog({ const title = renderTitle(state.volume, volumes); const { fsType, mountPath } = state.formData; const isDisabled = disableWidgets(); + const isFsFieldDisabled = isDisabled || mountFilesystem(state.volume); + const isSizeFieldDisabled = isDisabled || reuseDevice(state.volume); return ( /** @fixme blockSize medium is too big and small is too small. */
+ diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index 9261919d01..b94350c0e4 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -22,57 +22,52 @@ // @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 { Radio, Form, FormGroup } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; +import { _ } from "~/i18n"; +import { deviceChildren, volumeLabel } from "~/components/storage/utils"; +import { FormReadOnlyField, Popup } from "~/components/core"; +import VolumeLocationSelectorTable from "~/components/storage/VolumeLocationSelectorTable"; + /** * @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"; +// TRANSLATORS: Description of the dialog for changing the location of a file system. +const DIALOG_DESCRIPTION = _("The file systems are allocated at the installation device by \ +default. Indicate a custom location to create the file system at a specific device."); -/** - * 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"; +/** @type {(device: StorageDevice|undefined) => VolumeTarget} */ +const defaultTarget = (device) => { + if (["partition", "lvmLv", "md"].includes(device?.type)) return "DEVICE"; + + return "NEW_PARTITION"; +}; + +/** @type {(volume: Volume, device: StorageDevice|undefined) => VolumeTarget[]} */ +const availableTargets = (volume, device) => { + /** @type {VolumeTarget[]} */ + const targets = ["DEVICE"]; + + if (device?.isDrive) { + targets.push("NEW_PARTITION"); + targets.push("NEW_VG"); } + + if (device?.filesystem && volume.outline.fsTypes.includes(device.filesystem.type)) + targets.push("FILESYSTEM"); + + return targets; }; -/** - * Internal component for building the options. - * @component - * - * @param {React.PropsWithChildren>} props - */ -const RadioOption = ({ id, onChange, defaultChecked, children }) => { - return ( - <> - - - - ); +/** @type {(volume: Volume, device: StorageDevice|undefined) => VolumeTarget} */ +const sanitizeTarget = (volume, device) => { + const targets = availableTargets(volume, device); + return targets.includes(volume.target) ? volume.target : defaultTarget(device); }; /** @@ -80,10 +75,10 @@ const RadioOption = ({ id, onChange, defaultChecked, children }) => { * @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 {Volume} volume + * @property {Volume[]} volumes + * @property {StorageDevice[]} volumeDevices + * @property {StorageDevice[]} targetDevices * @property {boolean} [isOpen=false] - Whether the dialog is visible or not. * @property {() => void} onCancel * @property {(volume: Volume) => void} onAccept @@ -92,108 +87,127 @@ const RadioOption = ({ id, onChange, defaultChecked, children }) => { */ export default function VolumeLocationDialog({ volume, - devices, - target, - targetDevice: defaultTargetDevice, + volumes, + volumeDevices, + targetDevices, 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); + /** @type {StorageDevice|undefined} */ + const initialDevice = volume.targetDevice || targetDevices[0] || volumeDevices[0]; + /** @type {VolumeTarget} */ + const initialTarget = sanitizeTarget(volume, initialDevice); - const isLocationAuto = locationOption === "auto"; - const isLocationDevice = locationOption === "device"; - - const onSubmit = (e) => { - e.preventDefault(); - const newVolume = { ...volume }; + const [target, setTarget] = useState(initialTarget); + const [targetDevice, setTargetDevice] = useState(initialDevice); - if (isLocationAuto) { - newVolume.target = "DEFAULT"; - newVolume.targetDevice = undefined; - } + /** @type {(devices: StorageDevice[]) => void} */ + const changeTargetDevice = (devices) => { + const newTargetDevice = devices[0]; - if (isLocationDevice) { - newVolume.target = isDedicatedVG ? "NEW_VG" : "NEW_PARTITION"; - newVolume.targetDevice = targetDevice; + if (newTargetDevice.name !== targetDevice.name) { + setTarget(defaultTarget(newTargetDevice)); + setTargetDevice(newTargetDevice); } + }; + /** @type {(e: import("react").FormEvent) => void} */ + const onSubmit = (e) => { + e.preventDefault(); + const newVolume = { ...volume, target, targetDevice }; onAccept(newVolume); }; - const isAcceptDisabled = () => { - return isLocationDevice && targetDevice === undefined; + /** @type {(device: StorageDevice) => boolean} */ + const isDeviceSelectable = (device) => { + return device.isDrive || ["md", "partition", "lvmLv"].includes(device.type); }; - 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."); + const targets = availableTargets(volume, targetDevice); - return _("The file system will be allocated as a logical volume at the system LVM."); - }; + if (!targetDevice) return null; return (
-
- - - {_("Automatic")} - - -
- {autoText()} -
-
- -
- - - {_("Select a disk")} - - - + { /** FIXME: Rename FormReadOnlyField */} + +
+ d.sid)} + variant="compact" + /> +
+
-
- {_("The file system will be allocated as a new partition at the selected disk.")} -
- setTarget("NEW_PARTITION")} /> - setTarget("NEW_VG")} + /> + setTarget("DEVICE")} + /> + setTarget("FILESYSTEM")} />
-
+
- + diff --git a/web/src/components/storage/VolumeLocationDialog.test.jsx b/web/src/components/storage/VolumeLocationDialog.test.jsx index 81c4cc823d..0f261fdb83 100644 --- a/web/src/components/storage/VolumeLocationDialog.test.jsx +++ b/web/src/components/storage/VolumeLocationDialog.test.jsx @@ -56,31 +56,43 @@ const sda = { }; /** @type {StorageDevice} */ -const sdb = { - sid: 62, - isDrive: true, - type: "disk", +const sda1 = { + sid: 69, + name: "/dev/sda1", 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"] + isDrive: false, + type: "partition", + size: 256, + filesystem: { + sid: 169, + type: "Swap" + } +}; + +/** @type {StorageDevice} */ +const sda2 = { + sid: 79, + name: "/dev/sda2", + description: "", + isDrive: false, + type: "partition", + size: 512, + filesystem: { + sid: 179, + type: "Ext4" + } +}; + +sda.partitionTable = { + type: "gpt", + partitions: [sda1, sda2], + unpartitionedSize: 0, + unusedSlots: [] }; /** @type {StorageDevice} */ -const sdc = { - sid: 63, +const sdb = { + sid: 62, isDrive: true, type: "disk", description: "", @@ -93,7 +105,7 @@ const sdc = { dellBOSS: false, sdCard: false, active: true, - name: "/dev/sdc", + name: "/dev/sdb", size: 2048, recoverableSize: 0, systems : [], @@ -131,146 +143,105 @@ describe("VolumeLocationDialog", () => { props = { isOpen: true, volume, - devices: [sda, sdb, sdc], - target: "DISK", - targetDevice: sda, + volumes: [], + volumeDevices: [sda, sdb], + targetDevices: [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 create a new partition", () => { + plainRender(); + screen.getByRole("radio", { name: "Create a new partition" }); + }); - it("offers an option to use the installation disk", () => { + it("offers an option to create a dedicated VG", () => { plainRender(); - expect(automaticOption()).toBeInTheDocument(); + screen.getByRole("radio", { name: /Create a dedicated LVM/ }); }); - it("offers an option to selected a disk", () => { + it("offers an option to format the device", () => { plainRender(); - expect(selectDiskOption()).toBeInTheDocument(); - expect(diskSelector()).toBeInTheDocument(); - expect(lvmSelector()).toBeInTheDocument(); + screen.getByRole("radio", { name: "Format the device" }); }); - describe("if the current value is set to use the installation disk", () => { - beforeEach(() => { - props.volume.target = "DEFAULT"; - props.targetDevice = sda; - }); + it("offers an option to mount the file system", () => { + plainRender(); + screen.getByRole("radio", { name: "Mount the file system" }); + }); - it("selects 'Automatic' option by default", () => { - plainRender(); - expect(automaticOption()).toBeChecked(); - expect(selectDiskOption()).not.toBeChecked(); - expect(diskSelector()).toBeDisabled(); - expect(lvmSelector()).toBeDisabled(); + describe("if the selected device cannot be partitioned", () => { + beforeEach(async () => { + const { user } = plainRender(); + const sda1Row = screen.getByRole("row", { name: /sda1/ }); + const sda1Radio = within(sda1Row).getByRole("radio"); + await user.click(sda1Radio); }); - }); - describe("if the current value is set to use a selected disk", () => { - beforeEach(() => { - props.volume.target = "NEW_PARTITION"; - props.targetDevice = sda; + it("disables the option for creating a new partition", () => { + const option = screen.getByRole("radio", { name: "Create a new partition" }); + expect(option).toBeDisabled(); }); - 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(); + it("disables the option for creating a dedicated VG", () => { + const option = screen.getByRole("radio", { name: /Create a dedicated LVM/ }); + expect(option).toBeDisabled(); }); }); - 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; + describe("if the selected device has not a compatible file system", () => { + beforeEach(async () => { + const { user } = plainRender(); + const sda1Row = screen.getByRole("row", { name: /sda1/ }); + const sda1Radio = within(sda1Row).getByRole("radio"); + await user.click(sda1Radio); }); - 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("disables the option for mounting the file system", () => { + const option = screen.getByRole("radio", { name: "Mount the file system" }); + expect(option).toBeDisabled(); }); }); - 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 () => { + describe("if the selected device has a compatible file system", () => { + beforeEach(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 } - )); + const sda2Row = screen.getByRole("row", { name: /sda2/ }); + const sda2Radio = within(sda2Row).getByRole("radio"); + await user.click(sda2Radio); }); - }); - describe("if the 'Select a disk' option is selected", () => { - beforeEach(() => { - props.volume.target = "DEFAULT"; - props.volume.targetDevice = undefined; + it("enables the option for mounting the file system", () => { + const option = screen.getByRole("radio", { name: "Mount the file system" }); + expect(option).toBeEnabled(); }); + }); - it("calls onAccept with the selected options on accept", async () => { - const { user } = plainRender(); + it("calls onAccept with the selected options on accept", async () => { + const { user } = plainRender(); - await user.click(selectDiskOption()); - const selector = diskSelector(); - const sdbOption = within(selector).getByRole("option", { name: /sdb/ }); - await user.selectOptions(selector, sdbOption); + const sdbRow = screen.getByRole("row", { name: /sdb/ }); + const sdbRadio = within(sdbRow).getByRole("radio"); + await user.click(sdbRadio); - const accept = screen.getByRole("button", { name: "Confirm" }); - await user.click(accept); + const formatRadio = screen.getByRole("radio", { name: /format the device/i }); + await user.click(formatRadio); - expect(props.onAccept).toHaveBeenCalledWith(expect.objectContaining( - { target: "NEW_PARTITION", targetDevice: sdb } - )); - }); + const accept = screen.getByRole("button", { name: "Confirm" }); + await user.click(accept); - describe("and dedicated LVM is checked", () => { - it("calls onAccept with the selected options on accept", async () => { - const { user } = plainRender(); + expect(props.onAccept).toHaveBeenCalledWith(expect.objectContaining( + { target: "DEVICE", targetDevice: sdb } + )); + }); - await user.click(selectDiskOption()); - const selector = diskSelector(); - const sdbOption = within(selector).getByRole("option", { name: /sdb/ }); - await user.selectOptions(selector, sdbOption); - await user.click(lvmSelector()); + it("does not call onAccept on cancel", async () => { + const { user } = plainRender(); + const cancel = screen.getByRole("button", { name: "Cancel" }); - const accept = screen.getByRole("button", { name: "Confirm" }); - await user.click(accept); + await user.click(cancel); - expect(props.onAccept).toHaveBeenCalledWith(expect.objectContaining( - { target: "NEW_VG", targetDevice: sdb } - )); - }); - }); + expect(props.onAccept).not.toHaveBeenCalled(); }); }); diff --git a/web/src/components/storage/VolumeLocationSelectorTable.jsx b/web/src/components/storage/VolumeLocationSelectorTable.jsx new file mode 100644 index 0000000000..de8be15618 --- /dev/null +++ b/web/src/components/storage/VolumeLocationSelectorTable.jsx @@ -0,0 +1,121 @@ +/* + * 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 { Chip } from '@patternfly/react-core'; + +import { _ } from "~/i18n"; +import { + DeviceName, DeviceDetails, DeviceSize, toStorageDevice +} from "~/components/storage/device-utils"; +import { ExpandableSelector } from "~/components/core"; + +/** + * @typedef {import ("~/components/core/ExpandableSelector").ExpandableSelectorColumn} ExpandableSelectorColumn + * @typedef {import ("~/components/core/ExpandableSelector").ExpandableSelectorProps} ExpandableSelectorProps + * @typedef {import ("~/client/storage").PartitionSlot} PartitionSlot + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").Volume} Volume + */ + +/** + * Returns what (volumes, installation device) is using a device. + * @function + * + * @param {PartitionSlot|StorageDevice} item + * @param {StorageDevice[]} targetDevices + * @param {Volume[]} volumes + * @returns {string[]} + */ +const deviceUsers = (item, targetDevices, volumes) => { + const device = toStorageDevice(item); + if (!device) return []; + + const isTargetDevice = !!targetDevices.find(d => d.name === device.name); + const volumeUsers = volumes.filter(v => v.targetDevice?.name === device.name); + + const users = []; + if (isTargetDevice) users.push(_("Installation device")); + + return users.concat(volumeUsers.map(v => v.mountPath)); +}; + +/** + * @component + * + * @param {object} props + * @param {string[]} props.users + */ +const DeviceUsage = ({ users }) => { + return ( +
+ {users.map((user, index) => {user})} +
+ ); +}; + +/** + * Table for selecting the location for a volume. + * @component + * + * @typedef {object} VolumeLocationSelectorTableBaseProps + * @property {StorageDevice[]} devices + * @property {StorageDevice[]} selectedDevices + * @property {StorageDevice[]} targetDevices + * @property {Volume[]} volumes + * + * @typedef {VolumeLocationSelectorTableBaseProps & ExpandableSelectorProps} VolumeLocationSelectorTableProps + * + * @param {VolumeLocationSelectorTableProps} props + */ +export default function VolumeLocationSelectorTable({ + devices, + selectedDevices, + targetDevices, + volumes, + ...props +}) { + /** @type {ExpandableSelectorColumn[]} */ + const columns = [ + { name: _("Device"), value: (item) => }, + { name: _("Details"), value: (item) => }, + { name: _("Usage"), value: (item) => }, + { name: _("Size"), value: (item) => , classNames: "sizes-column" } + ]; + + return ( + { + if (!device.sid) { + return "dimmed-row"; + } + }} + itemsSelected={selectedDevices} + className="devices-table" + {...props} + /> + ); +} diff --git a/web/src/components/storage/device-utils.jsx b/web/src/components/storage/device-utils.jsx index c902fe89a9..6d1a4666c9 100644 --- a/web/src/components/storage/device-utils.jsx +++ b/web/src/components/storage/device-utils.jsx @@ -22,118 +22,85 @@ // @ts-check import React from "react"; -import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; -import { Icon } from "~/components/layout"; -import { If, Tag } from "~/components/core"; -import { deviceBaseName } from "~/components/storage/utils"; +import { Tag } from "~/components/core"; +import { deviceBaseName, deviceSize } from "~/components/storage/utils"; /** + * @typedef {import ("~/client/storage").PartitionSlot} PartitionSlot * @typedef {import ("~/client/storage").StorageDevice} StorageDevice */ /** - * @component + * Ensures the given item is a StorageDevice. + * @function * - * @param {object} props - * @param {StorageDevice} props.device + * @param {PartitionSlot|StorageDevice} item + * @returns {StorageDevice|undefined} */ -const FilesystemLabel = ({ device }) => { - const label = device.filesystem?.label; - if (label) return {label}; +const toStorageDevice = (item) => { + const { sid } = /** @type {object} */ (item); + if (!sid) return undefined; + + return /** @type {StorageDevice} */ (item); }; /** * @component * * @param {object} props - * @param {StorageDevice} props.device + * @param {PartitionSlot|StorageDevice} props.item */ -const DeviceExtendedInfo = ({ device }) => { - const DeviceName = () => { - if (device.name === undefined) return null; - - return
{device.name}
; - }; - - const DeviceType = () => { - let type; - - switch (device.type) { - case "multipath": { - // TRANSLATORS: multipath device type - type = _("Multipath"); - break; - } - case "dasd": { - // TRANSLATORS: %s is replaced by the device bus ID - type = sprintf(_("DASD %s"), device.busId); - break; - } - case "md": { - // TRANSLATORS: software RAID device, %s is replaced by the RAID level, e.g. RAID-1 - type = sprintf(_("Software %s"), device.level.toUpperCase()); - break; - } - case "disk": { - if (device.sdCard) { - type = _("SD Card"); - } else { - const technology = device.transport || device.bus; - type = technology - // TRANSLATORS: %s is substituted by the type of disk like "iSCSI" or "SATA" - ? sprintf(_("%s disk"), technology) - : _("Disk"); - } - } - } - - return {type}} />; - }; +const FilesystemLabel = ({ item }) => { + const device = toStorageDevice(item); + if (!device) return null; - const DeviceModel = () => { - if (!device.model || device.model === "") return null; + const label = device.filesystem?.label; + if (!label) return null; - return
{device.model}
; - }; + return {label}; +}; - const MDInfo = () => { - if (device.type !== "md" || !device.devices) return null; +/** + * @component + * + * @param {object} props + * @param {PartitionSlot|StorageDevice} props.item + */ +const DeviceName = ({ item }) => { + const device = toStorageDevice(item); + if (!device) return null; - const members = device.devices.map(deviceBaseName); + if (["partition", "lvmLv"].includes(device.type)) return deviceBaseName(device); - // TRANSLATORS: RAID details, %s is replaced by list of devices used by the array - return
{sprintf(_("Members: %s"), members.sort().join(", "))}
; - }; + return device.name; +}; - const RAIDInfo = () => { - if (device.type !== "raid") return null; +/** + * @component + * + * @param {object} props + * @param {PartitionSlot|StorageDevice} props.item + */ +const DeviceDetails = ({ item }) => { + const device = toStorageDevice(item); + if (!device) return _("Unused space"); - const devices = device.devices.map(deviceBaseName); + const renderContent = (device) => { + if (!device.partitionTable && device.systems?.length > 0) + return device.systems.join(", "); - // TRANSLATORS: RAID details, %s is replaced by list of devices used by the array - return
{sprintf(_("Devices: %s"), devices.sort().join(", "))}
; + return device.description; }; - const MultipathInfo = () => { - if (device.type !== "multipath") return null; - - const wires = device.wires.map(deviceBaseName); - - // TRANSLATORS: multipath details, %s is replaced by list of connections used by the device - return
{sprintf(_("Wires: %s"), wires.sort().join(", "))}
; + const renderPTableType = (device) => { + const type = device.partitionTable?.type; + if (type) return {type.toUpperCase()}; }; return ( -
- - - - - - -
+
{renderContent(device)} {renderPTableType(device)}
); }; @@ -141,59 +108,10 @@ const DeviceExtendedInfo = ({ device }) => { * @component * * @param {object} props - * @param {StorageDevice} props.device + * @param {PartitionSlot|StorageDevice} props.item */ -const DeviceContentInfo = ({ device }) => { - const PTable = () => { - if (device.partitionTable === undefined) return null; - - const type = device.partitionTable.type.toUpperCase(); - const numPartitions = device.partitionTable.partitions.length; - - // TRANSLATORS: disk partition info, %s is replaced by partition table - // type (MS-DOS or GPT), %d is the number of the partitions - const text = sprintf(_("%s with %d partitions"), type, numPartitions); - - return ( -
- {text} -
- ); - }; - - const Systems = () => { - if (!device.systems || device.systems.length === 0) return null; - - const System = ({ system }) => { - const logo = /windows/i.test(system) ? "windows_logo" : "linux_logo"; - - return
{system}
; - }; - - return device.systems.map((s, i) => ); - }; - - // TODO: there is a lot of room for improvement here, but first we would need - // device.description (comes from YaST) to be way more granular - const Description = () => { - if (device.partitionTable) return null; - - if (!device.sid || (!!device.model && device.model === device.description)) { - // TRANSLATORS: status message, no existing content was found on the disk, - // i.e. the disk is completely empty - return
{_("No content found")}
; - } - - return
{device.description}
; - }; - - return ( -
- - - -
- ); +const DeviceSize = ({ item }) => { + return deviceSize(item.size); }; -export { DeviceContentInfo, DeviceExtendedInfo, FilesystemLabel }; +export { toStorageDevice, DeviceName, DeviceDetails, DeviceSize, FilesystemLabel }; diff --git a/web/src/components/storage/device-utils.test.jsx b/web/src/components/storage/device-utils.test.jsx index 770d1fda03..1fdc0f040e 100644 --- a/web/src/components/storage/device-utils.test.jsx +++ b/web/src/components/storage/device-utils.test.jsx @@ -20,17 +20,22 @@ */ // @ts-check -// cspell:ignore dasda ddgdcbibhd import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; -import { DeviceContentInfo, DeviceExtendedInfo } from "~/components/storage/device-utils"; +import { + DeviceDetails, DeviceName, DeviceSize, FilesystemLabel, toStorageDevice +} from "~/components/storage/device-utils"; /** + * @typedef {import("~/client/storage").PartitionSlot} PartitionSlot * @typedef {import("~/client/storage").StorageDevice} StorageDevice */ +/** @type {PartitionSlot} */ +const slot = { start: 1234, size: 256 }; + /** @type {StorageDevice} */ const vda = { sid: 59, @@ -49,7 +54,7 @@ const vda = { size: 1024, 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"], + udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"] }; /** @type {StorageDevice} */ @@ -67,51 +72,19 @@ const vda1 = { systems : [], udevIds: [], udevPaths: [], - isEFI: false + isEFI: false, + filesystem: { sid: 100, type: "ext4", mountPath: "/test", label: "system" } }; -/** @type {StorageDevice} */ -const vda2 = { - sid: 61, +/** @type {StorageDevice} */ +const lvmLv1 = { + sid: 73, isDrive: false, - type: "partition", - active: true, - name: "/dev/vda2", - description: "", - size: 256, - start: 1789, - encrypted: false, - recoverableSize: 0, - systems : [], - udevIds: [], - udevPaths: [], - isEFI: false -}; - -vda.partitionTable = { - type: "gpt", - partitions: [vda1, vda2], - unpartitionedSize: 0, - unusedSlots: [] -}; - -/** @type {StorageDevice} */ -const vdb = { - sid: 62, - isDrive: true, - type: "disk", - vendor: "Disk", - model: "", - driver: [], - bus: "IDE", - busId: "", - transport: "", - dellBOSS: false, - sdCard: false, + type: "lvmLv", active: true, - name: "/dev/vdb", + name: "/dev/vg0/lv1", description: "", - size: 2048, + size: 512, start: 0, encrypted: false, recoverableSize: 0, @@ -120,163 +93,117 @@ const vdb = { udevPaths: [] }; -/** @type {StorageDevice} */ -const md0 = { - sid: 63, - isDrive: false, - type: "md", - level: "raid0", - uuid: "12345:abcde", - devices: [vdb], - active: true, - name: "/dev/md0", - description: "", - size: 2048, - systems : [], - udevIds: [], - udevPaths: [] -}; - -/** @type {StorageDevice} */ -const raid = { - sid: 64, - isDrive: true, - type: "raid", - devices: [vda, vdb], - vendor: "Dell", - model: "Dell BOSS-N1 Modular", - driver: [], - bus: "", - busId: "", - transport: "", - dellBOSS: true, - sdCard: false, - active: true, - name: "/dev/mapper/isw_ddgdcbibhd_244", - description: "", - size: 2048, - systems : [], - udevIds: [], - udevPaths: [] -}; - -/** @type {StorageDevice} */ -const multipath = { - sid: 65, - isDrive: true, - type: "multipath", - wires: [vda, vdb], - vendor: "", - model: "", - driver: [], - bus: "", - busId: "", - transport: "", - dellBOSS: false, - sdCard: false, - active: true, - name: "/dev/mapper/36005076305ffc73a00000000000013b4", - description: "", - size: 2048, - systems : [], - udevIds: [], - udevPaths: [] -}; - -/** @type {StorageDevice} */ -const dasd = { - sid: 66, - isDrive: true, - type: "dasd", - vendor: "IBM", - model: "IBM", - driver: [], - bus: "", - busId: "0.0.0150", - transport: "", - dellBOSS: false, - sdCard: false, - active: true, - name: "/dev/dasda", - description: "", - size: 2048, - systems : [], - udevIds: [], - udevPaths: [] -}; +describe("FilesystemLabel", () => { + it("renders the label of the file system", () => { + plainRender(); + screen.getByText("system"); + }); +}); -describe("DeviceExtendedInfo", () => { - it("renders the device name", () => { - plainRender(); - screen.getByText("/dev/vda"); +describe("DeviceName", () => { + it("renders the base name if the device is a partition", () => { + plainRender(); + screen.getByText(/^vda1/); }); - it("renders the device model", () => { - plainRender(); - screen.getByText("Micron 1100 SATA"); + it("renders the base name if the device is a logical volume", () => { + plainRender(); + screen.getByText(/^lv1/); }); - describe("when device is a SDCard", () => { - it("renders 'SD Card'", () => { - const sdCard = { ...vda, sdCard: true }; - plainRender(); - screen.getByText("SD Card"); - }); + it("renders the full name for other devices", () => { + plainRender(); + screen.getByText(/\/dev\/vda/); }); +}); - describe("when device is software RAID", () => { - it("renders its level", () => { - plainRender(); - screen.getByText("Software RAID0"); +describe("DeviceDetails", () => { + /** @type {PartitionSlot|StorageDevice} */ + let item; + + describe("if the item is a partition slot", () => { + beforeEach(() => { + item = slot; }); - it("renders its members", () => { - plainRender(); - screen.getByText(/Members/); - screen.getByText(/vdb/); + it("renders 'Unused space'", () => { + plainRender(); + screen.getByText("Unused space"); }); }); - describe("when device is RAID", () => { - it("renders its devices", () => { - plainRender(); - screen.getByText(/Devices/); - screen.getByText(/vda/); - screen.getByText(/vdb/); + describe("if the item is a storage device", () => { + beforeEach(() => { + item = { ...vda }; }); - }); - describe("when device is a multipath", () => { - it("renders 'Multipath'", () => { - plainRender(); - screen.getByText("Multipath"); + describe("and it has a file system", () => { + beforeEach(() => { + item = toStorageDevice(item); + item.filesystem = { sid: 100, type: "ext4", mountPath: "/test", label: "data" }; + }); + + it("renders the file system label", () => { + plainRender(); + screen.getByText("data"); + }); }); - it("renders its wires", () => { - plainRender(); - screen.getByText(/Wires/); - screen.getByText(/vda/); - screen.getByText(/vdb/); + describe("and it has a partition table", () => { + beforeEach(() => { + item = toStorageDevice(item); + item.partitionTable = { + type: "gpt", + partitions: [], + unpartitionedSize: 0, + unusedSlots: [] + }; + }); + + it("renders the partition table type", () => { + plainRender(); + screen.getByText("GPT"); + }); }); - }); - describe("when device is DASD", () => { - it("renders its bus id", () => { - plainRender(); - screen.getByText("DASD 0.0.0150"); + describe("and it has no partition table", () => { + beforeEach(() => { + item = toStorageDevice(item); + item.partitionTable = undefined; + item.description = "Ext4 disk"; + }); + + describe("and it has systems", () => { + beforeEach(() => { + item = toStorageDevice(item); + item.systems = ["Tumbleweed", "Leap"]; + }); + + it("renders the systems", () => { + plainRender(); + screen.getByText(/Tumbleweed/); + screen.getByText(/Leap/); + }); + }); + + describe("and it has no systems", () => { + beforeEach(() => { + item = toStorageDevice(item); + item.systems = []; + }); + + it("renders the description", () => { + plainRender(); + screen.getByText("Ext4 disk"); + }); + }); }); }); }); -describe("DeviceContentInfo", () => { - it("renders the partition table info", () => { - plainRender(); - screen.getByText("GPT with 2 partitions"); - }); - - it("renders systems info", () => { - plainRender(); - screen.getByText("Windows 11"); - screen.getByText("openSUSE Leap 15.2"); +describe("DeviceSize", () => { + it("renders the device size", () => { + plainRender(); + screen.getByText("1 KiB"); }); }); diff --git a/web/src/components/storage/index.js b/web/src/components/storage/index.js index 763840a3b7..6432494fa2 100644 --- a/web/src/components/storage/index.js +++ b/web/src/components/storage/index.js @@ -31,7 +31,6 @@ export { default as DASDFormatProgress } from "./DASDFormatProgress"; 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 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 b47c9460c4..9998449271 100644 --- a/web/src/components/storage/utils.js +++ b/web/src/components/storage/utils.js @@ -22,6 +22,13 @@ // @ts-check // cspell:ignore xbytes +/** + * @fixme This file implements utils for the storage components and it also offers several functions + * to get information from a Volume (e.g., #hasSnapshots, #isTransactionalRoot, etc). It would be + * better to use another approach to encapsulate the volume information. For example, by creating + * a Volume class or by providing a kind of interface for volumes. + */ + import xbytes from "xbytes"; import { N_ } from "~/i18n"; @@ -285,6 +292,33 @@ const isTransactionalSystem = (volumes = []) => { return volumes.find(v => isTransactionalRoot(v)) !== undefined; }; +/** + * Checks whether the given volume is configured to mount an existing file system. + * @function + * + * @param {Volume} volume + * @returns {boolean} + */ +const mountFilesystem = (volume) => volume.target === "FILESYSTEM"; + +/** + * Checks whether the given volume is configured to reuse a device (format or mount a file system). + * @function + * + * @param {Volume} volume + * @returns {boolean} + */ +const reuseDevice = (volume) => volume.target === "FILESYSTEM" || volume.target === "DEVICE"; + +/** + * Generates a label for the given volume. + * @function + * + * @param {Volume} volume + * @returns {string} + */ +const volumeLabel = (volume) => volume.mountPath === "/" ? "root" : volume.mountPath; + export { DEFAULT_SIZE_UNIT, SIZE_METHODS, @@ -299,5 +333,8 @@ export { hasFS, hasSnapshots, isTransactionalRoot, - isTransactionalSystem + isTransactionalSystem, + mountFilesystem, + reuseDevice, + volumeLabel };