From 4c082bf3b25df45a6332b434efa243fd564a7053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 26 Apr 2024 13:23:46 +0100 Subject: [PATCH 01/31] web: Put "Delete" volume option at the end --- web/src/components/storage/PartitionsField.jsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index dc971ff49c..5e133de1c3 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -352,11 +352,6 @@ const VolumeRow = ({ const VolumeActions = ({ volume, onEditClick, onLocationClick, onDeleteClick }) => { const actions = () => { const actions = { - delete: { - title: _("Delete"), - onClick: () => onDeleteClick(volume), - isDanger: true - }, edit: { title: _("Edit"), onClick: onEditClick @@ -364,7 +359,12 @@ const VolumeRow = ({ location: { title: _("Change location"), onClick: onLocationClick - } + }, + delete: { + title: _("Delete"), + onClick: () => onDeleteClick(volume), + isDanger: true + }, }; if (!volume.outline.required) return Object.values(actions); From 1a107bd8df9737a8d71626d98572b470393be446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 26 Apr 2024 14:10:54 +0100 Subject: [PATCH 02/31] web: Add the "Reset location" volume action In a temporary wrong placement. Incoming commit will fix it by refactoring the code for building the volume actions. --- .../components/storage/PartitionsField.jsx | 15 ++++++- .../storage/PartitionsField.test.jsx | 39 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 5e133de1c3..7b598b31b8 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -275,6 +275,10 @@ const VolumeRow = ({ const closeDialog = () => setDialog(undefined); + const onResetLocationClick = () => { + onEdit({ ...volume, target: "DEFAULT", targetDevice: undefined }); + }; + const acceptForm = (volume) => { closeDialog(); onEdit(volume); @@ -346,10 +350,11 @@ const VolumeRow = ({ * @param {object} props * @param {Volume} props.volume * @param {() => void} props.onEditClick + * @param {() => void} props.onResetLocationClick, * @param {() => void} props.onLocationClick * @param {(volume: Volume) => void} props.onDeleteClick */ - const VolumeActions = ({ volume, onEditClick, onLocationClick, onDeleteClick }) => { + const VolumeActions = ({ volume, onEditClick, onResetLocationClick, onLocationClick, onDeleteClick }) => { const actions = () => { const actions = { edit: { @@ -367,6 +372,13 @@ const VolumeRow = ({ }, }; + if (volume.target !== "DEFAULT") { + actions.reset_location = { + title: _("Reset location"), + onClick: onResetLocationClick + }; + } + if (!volume.outline.required) return Object.values(actions); return [actions.edit, actions.location]; @@ -394,6 +406,7 @@ const VolumeRow = ({ diff --git a/web/src/components/storage/PartitionsField.test.jsx b/web/src/components/storage/PartitionsField.test.jsx index e4822c356c..4e9db0b091 100644 --- a/web/src/components/storage/PartitionsField.test.jsx +++ b/web/src/components/storage/PartitionsField.test.jsx @@ -184,6 +184,7 @@ beforeEach(() => { volumes: [rootVolume, swapVolume], templates: [], devices: [], + system: [sda], target: "DISK", targetDevice: undefined, configureBoot: false, @@ -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 reseting 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 defautl location", () => { + beforeEach(() => { + props.volumes = [{ ...homeVolume, target: "NEW_PARTITION", targetDevice: sda }]; + }); + + it("allows reseting 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 peform 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 }]; From 0a18ab9a440caffcc421d5a3075301cda03673f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 26 Apr 2024 14:36:29 +0100 Subject: [PATCH 03/31] web: Refactor VolumeActions internal component To make it a bit simpler and keep actions in the desired order. --- .../components/storage/PartitionsField.jsx | 58 ++++++------------- 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 7b598b31b8..e3ae23ec5a 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -349,42 +349,20 @@ const VolumeRow = ({ * @component * @param {object} props * @param {Volume} props.volume - * @param {() => void} props.onEditClick - * @param {() => void} props.onResetLocationClick, - * @param {() => void} props.onLocationClick - * @param {(volume: Volume) => void} props.onDeleteClick + * @param {() => void} props.onEdit + * @param {() => void} props.onResetLocation + * @param {() => void} props.onLocation + * @param {() => void} props.onDelete */ - const VolumeActions = ({ volume, onEditClick, onResetLocationClick, onLocationClick, onDeleteClick }) => { - const actions = () => { - const actions = { - edit: { - title: _("Edit"), - onClick: onEditClick - }, - location: { - title: _("Change location"), - onClick: onLocationClick - }, - delete: { - title: _("Delete"), - onClick: () => onDeleteClick(volume), - isDanger: true - }, - }; - - if (volume.target !== "DEFAULT") { - actions.reset_location = { - title: _("Reset location"), - onClick: onResetLocationClick - }; - } - - if (!volume.outline.required) return Object.values(actions); - - return [actions.edit, actions.location]; - }; - - return ; + 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 ; }; if (isLoading) { @@ -405,10 +383,10 @@ const VolumeRow = ({ @@ -505,7 +483,7 @@ const VolumesTable = ({ targetDevice={targetDevice} isLoading={isLoading} onEdit={editVolume} - onDelete={deleteVolume} + onDelete={() => deleteVolume(volume)} /> ); }); From dd8ce25cb8cede88a04bfde3101d5b78b23af7ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 26 Apr 2024 14:42:54 +0100 Subject: [PATCH 04/31] web: Extract few components from VolumeRow Although these components are just convenient internal components of VolumeRow, they can be defined outside of it for helping to reduce the components redefinition each time the parent component gets rendered. --- .../components/storage/PartitionsField.jsx | 165 +++++++++--------- 1 file changed, 84 insertions(+), 81 deletions(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index e3ae23ec5a..6d9ed1033f 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -238,6 +238,87 @@ const BootLabel = ({ bootDevice, configureBoot }) => { ); }; +// TODO: Extract VolumesTable or at least VolumeRow and all related internal +// comonents 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 @@ -287,84 +368,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.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 ; - }; - if (isLoading) { return ( @@ -377,9 +380,9 @@ const VolumeRow = ({ <> {volume.mountPath} -
- - + + + Date: Fri, 26 Apr 2024 15:15:21 +0100 Subject: [PATCH 05/31] web: Improvements in tables - Unify columns. - Activate type checking. - Refactoring code. --- .../components/core/ExpandableSelector.jsx | 43 ++-- web/src/components/core/TreeTable.jsx | 10 +- .../storage/DeviceSelectionDialog.jsx | 27 +-- .../storage/DeviceSelectionDialog.test.jsx | 13 ++ .../storage/DeviceSelectorTable.jsx | 187 ++++++++++++++++-- web/src/components/storage/DevicesManager.js | 12 ++ .../storage/ProposalResultSection.jsx | 125 ++---------- .../storage/ProposalResultSection.test.jsx | 9 +- .../storage/ProposalResultTable.jsx | 164 +++++++++++++++ .../components/storage/SpaceActionsTable.jsx | 106 ++++------ .../components/storage/SpacePolicyDialog.jsx | 16 +- .../storage/SpacePolicyDialog.test.jsx | 16 ++ web/src/components/storage/device-utils.jsx | 184 +++++------------ web/src/components/storage/index.js | 1 - 14 files changed, 538 insertions(+), 375 deletions(-) create mode 100644 web/src/components/storage/ProposalResultTable.jsx diff --git a/web/src/components/core/ExpandableSelector.jsx b/web/src/components/core/ExpandableSelector.jsx index 7499f37f1d..f81df3f271 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 {string[]} [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/DeviceSelectionDialog.jsx b/web/src/components/storage/DeviceSelectionDialog.jsx index 5efdeeb019..26c66e40fa 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,20 +46,21 @@ 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 {() => 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 {() => 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, @@ -149,7 +150,7 @@ devices.").split(/[[\]]/); { @@ -124,6 +136,7 @@ describe("DeviceSelectionDialog", () => { props = { isOpen: true, target: "DISK", + targetDevice: undefined, targetPVDevices: [], devices: [sda, sdb, sdc], onCancel: jest.fn(), diff --git a/web/src/components/storage/DeviceSelectorTable.jsx b/web/src/components/storage/DeviceSelectorTable.jsx index 0653019f70..6874a181e4 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..78ada2d882 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,6 +135,7 @@ export default class DevicesManager { /** * Disk devices and LVM volume groups used for the installation. + * @method * * @note The used devices are extracted from the actions. * @@ -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/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 741a5c6c4a..1103892dc4 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,12 +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 + * @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..e581092bf5 --- /dev/null +++ b/web/src/components/storage/ProposalResultTable.jsx @@ -0,0 +1,164 @@ +/* + * 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 ExtendedDeviceDetails = ({ 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 ExtendedDeviceSize = ({ 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 deviceRender = () => { + return (item) => ; + }; + + /** @type {() => (item: TableItem) => React.ReactNode} */ + const mountPointRender = () => { + return (item) => ; + }; + + /** @type {() => (item: TableItem) => React.ReactNode} */ + const detailsRender = () => { + return (item) => ; + }; + + /** @type {() => (item: TableItem) => React.ReactNode} */ + const sizeRender = () => { + return (item) => ; + }; + + return [ + { name: _("Device"), value: deviceRender() }, + { name: _("Mount Point"), value: mountPointRender() }, + { name: _("Details"), value: detailsRender(), classNames: "details-column" }, + { name: _("Size"), value: sizeRender(), classNames: "sizes-column" } + ]; +}; + +/** + * Renders a TreeTable rendering the devices 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/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 e0f5162d04..74bc708b55 100644 --- a/web/src/components/storage/SpacePolicyDialog.jsx +++ b/web/src/components/storage/SpacePolicyDialog.jsx @@ -68,17 +68,19 @@ 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 {() => 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 {() => 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/device-utils.jsx b/web/src/components/storage/device-utils.jsx index c902fe89a9..0c783b4b39 100644 --- a/web/src/components/storage/device-utils.jsx +++ b/web/src/components/storage/device-utils.jsx @@ -22,24 +22,40 @@ // @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 */ +/** + * Conversion to StorageDevice. + * @function + * + * @param {PartitionSlot|StorageDevice} item + * @returns {StorageDevice|undefined} + */ +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 FilesystemLabel = ({ device }) => { +const FilesystemLabel = ({ item }) => { + const device = toStorageDevice(item); + if (!device) return null; + const label = device.filesystem?.label; if (label) return {label}; }; @@ -48,92 +64,41 @@ const FilesystemLabel = ({ device }) => { * @component * * @param {object} props - * @param {StorageDevice} props.device + * @param {PartitionSlot|StorageDevice} props.item */ -const DeviceExtendedInfo = ({ device }) => { - const DeviceName = () => { - if (device.name === undefined) return null; +const DeviceName = ({ item }) => { + const device = toStorageDevice(item); + if (!device) 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}} />; - }; + if (["partition", "lvmLv"].includes(device.type)) return deviceBaseName(device); - 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(", "))}
; - }; + 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 +106,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/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"; From d88d1f00afa455ddbf6115f07cdca67c3c0bf93e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 26 Apr 2024 15:37:21 +0100 Subject: [PATCH 06/31] web: Fix a bunch of typos --- web/src/components/storage/PartitionsField.jsx | 2 +- web/src/components/storage/PartitionsField.test.jsx | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 6d9ed1033f..ced0f31d57 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -239,7 +239,7 @@ const BootLabel = ({ bootDevice, configureBoot }) => { }; // TODO: Extract VolumesTable or at least VolumeRow and all related internal -// comonents to a new file. +// components to a new file. /** * @component diff --git a/web/src/components/storage/PartitionsField.test.jsx b/web/src/components/storage/PartitionsField.test.jsx index 4e9db0b091..aa6ff7e62b 100644 --- a/web/src/components/storage/PartitionsField.test.jsx +++ b/web/src/components/storage/PartitionsField.test.jsx @@ -350,7 +350,7 @@ describe("if there are volumes", () => { }); // FIXME: improve at least the test description - it("does not allow reseting the volume location when already using the default location", async () => { + 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"); @@ -360,12 +360,12 @@ describe("if there are volumes", () => { expect(within(row).queryByRole("menuitem", { name: "Reset location" })).toBeNull(); }); - describe("and a volume has a non defautl location", () => { + describe("and a volume has a non default location", () => { beforeEach(() => { props.volumes = [{ ...homeVolume, target: "NEW_PARTITION", targetDevice: sda }]; }); - it("allows reseting the volume location", async () => { + it("allows resetting the volume location", async () => { const { user } = await expandField(); const [, body] = await screen.findAllByRole("rowgroup"); @@ -380,7 +380,7 @@ describe("if there are volumes", () => { ]) ); - // NOTE: sadly we cannot peform the below check because the component is + // 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" }); From 7fb343b3051829f229f5209ca907ce3cf71ec047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 29 Apr 2024 16:47:56 +0100 Subject: [PATCH 07/31] web: WIP first version of the new location dialog --- web/src/client/storage.js | 29 +++ .../components/core/ExpandableSelector.jsx | 2 +- .../components/storage/BootConfigField.jsx | 6 +- .../storage/BootSelectionDialog.jsx | 6 +- web/src/components/storage/DevicesManager.js | 2 +- .../components/storage/PartitionsField.jsx | 63 ++--- web/src/components/storage/ProposalPage.jsx | 16 +- .../storage/ProposalResultTable.jsx | 8 +- .../storage/ProposalSettingsSection.jsx | 9 +- .../storage/VolumeLocationDialog.jsx | 216 +++++++++--------- .../storage/VolumeLocationSelectorTable.jsx | 101 ++++++++ 11 files changed, 305 insertions(+), 153 deletions(-) create mode 100644 web/src/components/storage/VolumeLocationSelectorTable.jsx diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 55623d8c36..8649ba9395 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -451,6 +451,35 @@ 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. + * + * @returns {Promise} + */ + async getVolumeDevices() { + const availableDevices = await this.getAvailableDevices(); + + 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) || + !!availableDevices.find(d => isChildren(device, d)) + ); + }; + + 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 * diff --git a/web/src/components/core/ExpandableSelector.jsx b/web/src/components/core/ExpandableSelector.jsx index f81df3f271..af98528c71 100644 --- a/web/src/components/core/ExpandableSelector.jsx +++ b/web/src/components/core/ExpandableSelector.jsx @@ -112,7 +112,7 @@ const sanitizeSelection = (selection, allowMultiple) => { * @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 {string[]} [initialExpandedKeys=[]] - Ids of initially expanded 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 diff --git a/web/src/components/storage/BootConfigField.jsx b/web/src/components/storage/BootConfigField.jsx index b62ccf14fb..3742f1971d 100644 --- a/web/src/components/storage/BootConfigField.jsx +++ b/web/src/components/storage/BootConfigField.jsx @@ -60,7 +60,7 @@ const Button = ({ isBold = false, onClick }) => { * @param {boolean} props.configureBoot * @param {StorageDevice|undefined} props.bootDevice * @param {StorageDevice|undefined} props.defaultBootDevice - * @param {StorageDevice[]} props.devices + * @param {StorageDevice[]} props.availableDevices * @param {boolean} props.isLoading * @param {(boot: BootConfig) => void} props.onChange * @@ -72,7 +72,7 @@ export default function BootConfigField({ configureBoot, bootDevice, defaultBootDevice, - devices, + availableDevices, isLoading, onChange }) { @@ -113,7 +113,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/BootSelectionDialog.jsx b/web/src/components/storage/BootSelectionDialog.jsx index 006cb54d9e..565ddfa2f8 100644 --- a/web/src/components/storage/BootSelectionDialog.jsx +++ b/web/src/components/storage/BootSelectionDialog.jsx @@ -66,7 +66,7 @@ const RadioOption = ({ id, onChange, defaultChecked, children }) => { * @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 {StorageDevice[]} props.availableDevices - 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] @@ -75,7 +75,7 @@ export default function BootSelectionDialog({ configureBoot: configureBootProp, bootDevice: bootDeviceProp, defaultBootDevice, - devices, + availableDevices, isOpen, onCancel = noop, onAccept = noop, @@ -161,7 +161,7 @@ partitions in the appropriate disk." 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); diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index ced0f31d57..3967f4bb0b 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -328,21 +328,21 @@ const VolumeActions = ({ volume, onEdit, onResetLocation, onLocation, onDelete } * @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 @@ -412,9 +412,9 @@ const VolumeRow = ({ @@ -431,18 +431,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 }) => { @@ -481,9 +481,9 @@ const VolumesTable = ({ volume={volume} volumes={volumes} templates={templates} - devices={devices} + volumeDevices={volumeDevices} target={target} - targetDevice={targetDevice} + targetDevices={targetDevices} isLoading={isLoading} onEdit={editVolume} onDelete={() => deleteVolume(volume)} @@ -608,9 +608,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 @@ -621,9 +622,10 @@ const AddVolumeButton = ({ options, onClick }) => { const Advanced = ({ volumes, templates, - devices, + availableDevices, + volumeDevices, target, - targetDevice, + targetDevices, configureBoot, bootDevice, defaultBootDevice, @@ -713,9 +715,9 @@ const Advanced = ({ @@ -744,7 +746,7 @@ const Advanced = ({ configureBoot={configureBoot} bootDevice={bootDevice} defaultBootDevice={defaultBootDevice} - devices={devices} + availableDevices={availableDevices} isLoading={isLoading} onChange={onBootChange} /> @@ -762,9 +764,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 * @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. @@ -781,9 +784,10 @@ const Advanced = ({ export default function PartitionsField({ volumes, templates, - devices, + availableDevices, + volumeDevices, target, - targetDevice, + targetDevices, configureBoot, bootDevice, defaultBootDevice, @@ -807,9 +811,10 @@ export default function PartitionsField({ { 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 }; @@ -105,6 +111,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]); @@ -153,6 +163,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 } }); @@ -169,7 +182,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" }); @@ -231,6 +244,7 @@ export default function ProposalPage() { /> { * @param {TableItem} props.item * @param {DevicesManager} props.devicesManager */ -const ExtendedDeviceDetails = ({ item, devicesManager }) => { +const DeviceCustomDetails = ({ item, devicesManager }) => { const isNew = () => { const device = toStorageDevice(item); if (!device) return false; @@ -83,7 +83,7 @@ const ExtendedDeviceDetails = ({ item, devicesManager }) => { * @param {TableItem} props.item * @param {DevicesManager} props.devicesManager */ -const ExtendedDeviceSize = ({ item, 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; @@ -121,12 +121,12 @@ const columns = (devicesManager) => { /** @type {() => (item: TableItem) => React.ReactNode} */ const detailsRender = () => { - return (item) => ; + return (item) => ; }; /** @type {() => (item: TableItem) => React.ReactNode} */ const sizeRender = () => { - return (item) => ; + return (item) => ; }; return [ diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 37bf0669ac..822e14efa2 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -47,6 +47,7 @@ import SpacePolicyField from "~/components/storage/SpacePolicyField"; * @typedef {object} ProposalSettingsSectionProps * @property {ProposalSettings} settings * @property {StorageDevice[]} availableDevices + * @property {StorageDevice[]} volumeDevices * @property {String[]} encryptionMethods * @property {Volume[]} volumeTemplates * @property {boolean} [isLoading=false] @@ -57,6 +58,7 @@ import SpacePolicyField from "~/components/storage/SpacePolicyField"; export default function ProposalSettingsSection({ settings, availableDevices, + volumeDevices, encryptionMethods, volumeTemplates, isLoading = false, @@ -112,6 +114,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 ( <>
@@ -133,9 +137,10 @@ export default function ProposalSettingsSection({ VolumeTarget} */ +const defaultTarget = (device) => { + if (["partition", "lvmLv", "md"].includes(device.type)) return "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"; + return "NEW_PARTITION"; +}; + +/** @type {(volume: Volume, device: StorageDevice) => VolumeTarget[]} */ +const availableTargets = (volume, device) => { + /** @type {VolumeTarget[]} */ + const targets = ["DEVICE"]; + + if (device.isDrive) { + targets.push("NEW_PARTITION"); + targets.push("NEW_VG"); } + + /** @fixme define type for possible fstypes */ + const fsTypes = volume.outline.fsTypes.map(f => f.toLowerCase()); + if (device.filesystem && 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) => VolumeTarget} */ +const sanitizeTarget = (volume, device) => { + const targets = availableTargets(volume, device); + return targets.includes(volume.target) ? volume.target : defaultTarget(device); }; /** @@ -80,10 +73,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,105 +85,110 @@ 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); - - const isLocationAuto = locationOption === "auto"; - const isLocationDevice = locationOption === "device"; + const initialDevice = volume.targetDevice || targetDevices[0] || volumeDevices[0]; + const initialTarget = sanitizeTarget(volume, initialDevice); - 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; - } + 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); } + }; + const onSubmit = (e) => { + e.preventDefault(); + const newVolume = { ...volume, target, targetDevice }; onAccept(newVolume); }; const isAcceptDisabled = () => { - return isLocationDevice && targetDevice === undefined; + return false; }; - const autoText = () => { - if (target === "DISK" && defaultTargetDevice) - // TRANSLATORS: %s is replaced by a device label (e.g., "/dev/vda, 50 GiB"). - return sprintf(_("The filesystem will be allocated as a new partition at the installation \ -disk (%s)."), deviceLabel(defaultTargetDevice)); - - if (target === "DISK") - return _("The filesystem will be allocated as a new partition at the installation disk."); - - return _("The file system will be allocated as a logical volume at the system LVM."); + /** @type {(device: StorageDevice) => boolean} */ + const isDeviceSelectable = (device) => { + return device.isDrive || ["md", "partition", "lvmLv"].includes(device.type); }; + const targets = availableTargets(volume, targetDevice); + return (
-
- - - {_("Automatic")} - - -
- {autoText()} -
-
- -
- - - {_("Select a disk")} - - - -
-
- {_("The file system will be allocated as a new partition at the selected disk.")} -
- + d.sid)} + variant="compact" + /> + + setTarget("NEW_PARTITION")} /> - setTarget("NEW_VG")} + /> + setTarget("DEVICE")} + /> + setTarget("FILESYSTEM")} /> -
-
+ +
diff --git a/web/src/components/storage/VolumeLocationSelectorTable.jsx b/web/src/components/storage/VolumeLocationSelectorTable.jsx new file mode 100644 index 0000000000..c9a8db2604 --- /dev/null +++ b/web/src/components/storage/VolumeLocationSelectorTable.jsx @@ -0,0 +1,101 @@ +/* + * 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("../core/ExpandableSelector").ExpandableSelectorColumn} ExpandableSelectorColumn + * @typedef {import("../core/ExpandableSelector").ExpandableSelectorProps} ExpandableSelectorProps + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").Volume} Volume + */ + +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)); +}; + +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} VolumeLocationSelectorTable + * + * @param {VolumeLocationSelectorTable} 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} + /> + ); +} From 66b9b007654b6949cafa27067ee4365c768be58c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 30 Apr 2024 12:37:53 +0100 Subject: [PATCH 08/31] service: Fix bug converting settings from Y2Storage --- service/lib/agama/storage/volume_conversion/from_y2storage.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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. From 4bcef6bdc365afc2b82064f582b69061dfc8a347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 30 Apr 2024 13:29:32 +0100 Subject: [PATCH 09/31] web: Improve wording --- .../storage/VolumeLocationDialog.jsx | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index c1d3b66a10..c25cccd66d 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -27,7 +27,7 @@ import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; import { deviceChildren } from "~/components/storage/utils"; -import { Popup } from "~/components/core"; +import { FormReadOnlyField, Popup } from "~/components/core"; import VolumeLocationSelectorTable from "~/components/storage/VolumeLocationSelectorTable"; /** @@ -37,6 +37,9 @@ import VolumeLocationSelectorTable from "~/components/storage/VolumeLocationSele * @typedef {import ("~/client/storage").VolumeTarget} VolumeTarget */ +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."); + /** @type {(device: StorageDevice) => VolumeTarget} */ const defaultTarget = (device) => { if (["partition", "lvmLv", "md"].includes(device.type)) return "DEVICE"; @@ -128,31 +131,33 @@ export default function VolumeLocationDialog({ return (
- d.sid)} - variant="compact" - /> - + + d.sid)} + variant="compact" + /> + + setTarget("DEVICE")} @@ -181,8 +185,8 @@ mounted at %s."), volume.fsType, volume.mountPath)} setTarget("FILESYSTEM")} From 7a4f45760fe969d8ae305ca60922763400cf435e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 30 Apr 2024 13:49:37 +0100 Subject: [PATCH 10/31] service: Always export the file system human string - Volumes were using the human string of the file system type, but the devices were using a different represetation for the file system type. --- service/lib/agama/dbus/storage/interfaces/device/filesystem.rb | 2 +- .../agama/dbus/storage/interfaces/device/filesystem_examples.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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/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 From dc6c80f7d70064060967b60dcfba41352938e14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 30 Apr 2024 13:53:23 +0100 Subject: [PATCH 11/31] web: Remove unnecessary conversion for file system type --- web/src/components/storage/VolumeLocationDialog.jsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index c25cccd66d..a498cfe47a 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -57,9 +57,7 @@ const availableTargets = (volume, device) => { targets.push("NEW_VG"); } - /** @fixme define type for possible fstypes */ - const fsTypes = volume.outline.fsTypes.map(f => f.toLowerCase()); - if (device.filesystem && fsTypes.includes(device.filesystem.type)) + if (device.filesystem && volume.outline.fsTypes.includes(device.filesystem.type)) targets.push("FILESYSTEM"); return targets; From 07f850db9213608f4206ec93ac71517ce4b8f96a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 30 Apr 2024 14:54:49 +0100 Subject: [PATCH 12/31] web: Do not consider reused devices as installation devices --- web/src/client/storage.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 8649ba9395..7734bde311 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -566,7 +566,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); @@ -576,10 +576,14 @@ class ProposalManager { return device; }; + 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); From bded1d0b3739830e4b88e9f7897aefcd61eab27c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 30 Apr 2024 16:44:24 +0100 Subject: [PATCH 13/31] wer: Add alert when editing volume --- .../components/storage/PartitionsField.jsx | 20 +++++----- web/src/components/storage/VolumeDialog.jsx | 40 +++++++++++++++---- .../storage/VolumeLocationDialog.jsx | 4 +- web/src/components/storage/utils.js | 11 ++++- 4 files changed, 56 insertions(+), 19 deletions(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 3967f4bb0b..4ad32c49f6 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); @@ -253,7 +252,10 @@ const VolumeSizeLimits = ({ volume }) => {
{SizeText({ volume })} {/* TRANSLATORS: device flag, the partition size is automatically computed */} - {_("auto")}} /> + {_("auto")}} + />
); }; diff --git a/web/src/components/storage/VolumeDialog.jsx b/web/src/components/storage/VolumeDialog.jsx index 67072d96bc..535852c746 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'; @@ -83,14 +84,36 @@ 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"); }; +const VolumeAlert = ({ volume }) => { + let alert; + + if (mountFilesystem(volume)) { + alert = { + title: _("The type and size of the file system cannot be edited."), + 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 = { + title: _("The size of the file system cannot be edited"), + 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 @@ -717,10 +740,13 @@ 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 a498cfe47a..de88ef0643 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -26,7 +26,7 @@ import { Radio, Form, FormGroup } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; -import { deviceChildren } from "~/components/storage/utils"; +import { deviceChildren, volumeLabel } from "~/components/storage/utils"; import { FormReadOnlyField, Popup } from "~/components/core"; import VolumeLocationSelectorTable from "~/components/storage/VolumeLocationSelectorTable"; @@ -128,7 +128,7 @@ export default function VolumeLocationDialog({ return ( { return volumes.find(v => isTransactionalRoot(v)) !== undefined; }; +const mountFilesystem = (volume) => volume.target === "FILESYSTEM"; + +const reuseDevice = (volume) => volume.target === "FILESYSTEM" || volume.target === "DEVICE"; + +const volumeLabel = (volume) => volume.mountPath === "/" ? "root" : volume.mountPath; + export { DEFAULT_SIZE_UNIT, SIZE_METHODS, @@ -299,5 +305,8 @@ export { hasFS, hasSnapshots, isTransactionalRoot, - isTransactionalSystem + isTransactionalSystem, + mountFilesystem, + reuseDevice, + volumeLabel }; From a9b8d0d45e7e70d9ccc2b69e52fd3f5bffde466b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 09:55:27 +0100 Subject: [PATCH 14/31] web: Several improvements - Documentation - Types --- web/src/client/storage.js | 11 ++++++ .../storage/ProposalResultTable.jsx | 34 +++++++------------ web/src/components/storage/VolumeDialog.jsx | 27 ++++++++++----- .../storage/VolumeLocationDialog.jsx | 22 +++++++----- .../storage/VolumeLocationSelectorTable.jsx | 20 +++++++++-- web/src/components/storage/device-utils.jsx | 2 +- web/src/components/storage/utils.js | 28 +++++++++++++++ 7 files changed, 103 insertions(+), 41 deletions(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 7734bde311..2303485570 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -454,11 +454,18 @@ class ProposalManager { /** * 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 || []; @@ -471,6 +478,7 @@ class ProposalManager { ); }; + /** @type {(device: StorageDevice[]) => boolean} */ const allAvailable = (devices) => devices.every(isAvailable); const system = await this.system.getDevices(); @@ -576,6 +584,9 @@ 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)) ); diff --git a/web/src/components/storage/ProposalResultTable.jsx b/web/src/components/storage/ProposalResultTable.jsx index bc3436c433..d4350f4016 100644 --- a/web/src/components/storage/ProposalResultTable.jsx +++ b/web/src/components/storage/ProposalResultTable.jsx @@ -109,36 +109,28 @@ const DeviceCustomSize = ({ item, devicesManager }) => { /** @type {(devicesManager: DevicesManager) => TreeTableColumn[] } */ const columns = (devicesManager) => { - /** @type {() => (item: TableItem) => React.ReactNode} */ - const deviceRender = () => { - return (item) => ; - }; + /** @type {(item: TableItem) => React.ReactNode} */ + const renderDevice = (item) => ; - /** @type {() => (item: TableItem) => React.ReactNode} */ - const mountPointRender = () => { - return (item) => ; - }; + /** @type {(item: TableItem) => React.ReactNode} */ + const renderMountPoint = (item) => ; - /** @type {() => (item: TableItem) => React.ReactNode} */ - const detailsRender = () => { - return (item) => ; - }; + /** @type {(item: TableItem) => React.ReactNode} */ + const renderDetails = (item) => ; - /** @type {() => (item: TableItem) => React.ReactNode} */ - const sizeRender = () => { - return (item) => ; - }; + /** @type {(item: TableItem) => React.ReactNode} */ + const renderSize = (item) => ; return [ - { name: _("Device"), value: deviceRender() }, - { name: _("Mount Point"), value: mountPointRender() }, - { name: _("Details"), value: detailsRender(), classNames: "details-column" }, - { name: _("Size"), value: sizeRender(), classNames: "sizes-column" } + { name: _("Device"), value: renderDevice }, + { name: _("Mount Point"), value: renderMountPoint }, + { name: _("Details"), value: renderDetails, classNames: "details-column" }, + { name: _("Size"), value: renderSize, classNames: "sizes-column" } ]; }; /** - * Renders a TreeTable rendering the devices proposal result. + * Renders the proposal result. * @component * * @typedef {object} ProposalResultTableProps diff --git a/web/src/components/storage/VolumeDialog.jsx b/web/src/components/storage/VolumeDialog.jsx index 535852c746..b116447580 100644 --- a/web/src/components/storage/VolumeDialog.jsx +++ b/web/src/components/storage/VolumeDialog.jsx @@ -63,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 */ /** @@ -92,18 +84,29 @@ const renderTitle = (volume, volumes) => { 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) }; @@ -626,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({ diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index de88ef0643..d926e358db 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -37,6 +37,7 @@ import VolumeLocationSelectorTable from "~/components/storage/VolumeLocationSele * @typedef {import ("~/client/storage").VolumeTarget} VolumeTarget */ +// 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."); @@ -115,10 +116,6 @@ export default function VolumeLocationDialog({ onAccept(newVolume); }; - const isAcceptDisabled = () => { - return false; - }; - /** @type {(device: StorageDevice) => boolean} */ const isDeviceSelectable = (device) => { return device.isDrive || ["md", "partition", "lvmLv"].includes(device.type); @@ -128,6 +125,8 @@ export default function VolumeLocationDialog({ return ( setTarget("NEW_PARTITION")} @@ -175,7 +174,11 @@ file system will be created as a logical volume.")} id="format" name="target" label={_("Format the device")} - description={sprintf(_("The selected device will be formatted as %s file system."), volume.fsType)} + description={ + // TRANSLATORS: %s is replaced by a file system type (e.g., Ext4). + sprintf(_("The selected device will be formatted as %s file system."), + volume.fsType) + } isChecked={target === "DEVICE"} isDisabled={!targets.includes("DEVICE")} onChange={() => setTarget("DEVICE")} @@ -184,7 +187,8 @@ file system will be created as a logical volume.")} id="mount" name="target" label={_("Mount the file system")} - description={_("The current file system on the selected device will be mounted without formatting the device.")} + description={_("The current file system on the selected device will be mounted \ +without formatting the device.")} isChecked={target === "FILESYSTEM"} isDisabled={!targets.includes("FILESYSTEM")} onChange={() => setTarget("FILESYSTEM")} @@ -193,7 +197,7 @@ file system will be created as a logical volume.")}
- +
diff --git a/web/src/components/storage/VolumeLocationSelectorTable.jsx b/web/src/components/storage/VolumeLocationSelectorTable.jsx index c9a8db2604..0392bd1248 100644 --- a/web/src/components/storage/VolumeLocationSelectorTable.jsx +++ b/web/src/components/storage/VolumeLocationSelectorTable.jsx @@ -33,10 +33,20 @@ import { ExpandableSelector } from "~/components/core"; /** * @typedef {import("../core/ExpandableSelector").ExpandableSelectorColumn} ExpandableSelectorColumn * @typedef {import("../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 []; @@ -50,6 +60,12 @@ const deviceUsers = (item, targetDevices, volumes) => { 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}); }; @@ -64,9 +80,9 @@ const DeviceUsage = ({ users }) => { * @property {StorageDevice[]} targetDevices * @property {Volume[]} volumes * - * @typedef {VolumeLocationSelectorTableBaseProps & ExpandableSelectorProps} VolumeLocationSelectorTable + * @typedef {VolumeLocationSelectorTableBaseProps & ExpandableSelectorProps} VolumeLocationSelectorTableProps * - * @param {VolumeLocationSelectorTable} props + * @param {VolumeLocationSelectorTableProps} props */ export default function VolumeLocationSelectorTable({ devices, diff --git a/web/src/components/storage/device-utils.jsx b/web/src/components/storage/device-utils.jsx index 0c783b4b39..80a90cbf4e 100644 --- a/web/src/components/storage/device-utils.jsx +++ b/web/src/components/storage/device-utils.jsx @@ -33,7 +33,7 @@ import { deviceBaseName, deviceSize } from "~/components/storage/utils"; */ /** - * Conversion to StorageDevice. + * Ensures the given item is a StorageDevice. * @function * * @param {PartitionSlot|StorageDevice} item diff --git a/web/src/components/storage/utils.js b/web/src/components/storage/utils.js index 47d80cbdb4..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,10 +292,31 @@ 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 { From 3d479a8ddbcb8e94efb142c9eb6117c585d2b444 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 08:42:22 +0100 Subject: [PATCH 15/31] web: Force two rows layout in VolumeLocationDialog To be precise, changes try to force the content to fit in the available space by adding scroll to the devices table if needed in an attempt to keep allocation options always visible. This is temporary and should be changed/improved. --- web/src/assets/styles/composition.scss | 19 +++ .../storage/VolumeLocationDialog.jsx | 115 +++++++++--------- 2 files changed, 76 insertions(+), 58 deletions(-) diff --git a/web/src/assets/styles/composition.scss b/web/src/assets/styles/composition.scss index 07c7994aa7..a415ef2fbc 100644 --- a/web/src/assets/styles/composition.scss +++ b/web/src/assets/styles/composition.scss @@ -24,6 +24,25 @@ flex-wrap: wrap; } +// TODO: make it less specific. +.location-layout > div { + display: flex; + + form { + display: flex; + flex-direction: column; + flex: 1 1 0; + } + + form > div:first-child { + overflow-y: auto; + } + + form > div:last-child { + min-block-size: min-content; + } +} + [data-state="reversed"] { flex-direction: row-reverse; } diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index d926e358db..9439b50413 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -131,70 +131,69 @@ export default function VolumeLocationDialog({ description={DIALOG_DESCRIPTION} inlineSize="large" isOpen={isOpen} + className="location-layout" {...props} >
-
- - d.sid)} - variant="compact" - /> - - - + d.sid)} + variant="compact" + /> + + + setTarget("NEW_PARTITION")} - /> - setTarget("NEW_PARTITION")} + /> + setTarget("NEW_VG")} - /> - setTarget("DEVICE")} - /> - setTarget("NEW_VG")} + /> + setTarget("DEVICE")} + /> + setTarget("FILESYSTEM")} - /> - -
+ isChecked={target === "FILESYSTEM"} + isDisabled={!targets.includes("FILESYSTEM")} + onChange={() => setTarget("FILESYSTEM")} + /> +
From 78a57ae6502f998565c4c33cc1a35fed3d7db5af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 08:53:04 +0100 Subject: [PATCH 16/31] web: Increase a bit the size of radio labels To create more difference between them and the radio descriptions. As previous changes, this aims to be a temporary solution and should be re-evaluated. --- web/src/assets/styles/patternfly-overrides.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/src/assets/styles/patternfly-overrides.scss b/web/src/assets/styles/patternfly-overrides.scss index 71040a7797..9570e53e4f 100644 --- a/web/src/assets/styles/patternfly-overrides.scss +++ b/web/src/assets/styles/patternfly-overrides.scss @@ -268,6 +268,10 @@ ul { vertical-align: middle; } +.pf-v5-c-radio__label { + font-size: var(--fs-large); +} + @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; From d45800004a513e92a2912c6ff91cb5f19235faa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 09:52:54 +0100 Subject: [PATCH 17/31] web: Add spaces between VolumeLocationDialog elements --- web/src/assets/styles/composition.scss | 4 + .../storage/VolumeLocationDialog.jsx | 88 ++++++++++--------- .../storage/VolumeLocationSelectorTable.jsx | 6 +- 3 files changed, 54 insertions(+), 44 deletions(-) diff --git a/web/src/assets/styles/composition.scss b/web/src/assets/styles/composition.scss index a415ef2fbc..67a3c1a29f 100644 --- a/web/src/assets/styles/composition.scss +++ b/web/src/assets/styles/composition.scss @@ -3,6 +3,10 @@ margin-block-end: var(--stack-gutter); } +.stack.small { + --stack-gutter: var(--spacer-smaller); +} + .flex-stack { display: flex; flex-direction: column; diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index 9439b50413..87965c0917 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -150,49 +150,51 @@ export default function VolumeLocationDialog({ /> - setTarget("NEW_PARTITION")} - /> - setTarget("NEW_VG")} - /> - setTarget("DEVICE")} - /> - setTarget("FILESYSTEM")} - /> +
+ setTarget("NEW_PARTITION")} + /> + setTarget("NEW_VG")} + /> + setTarget("DEVICE")} + /> + setTarget("FILESYSTEM")} + /> +
diff --git a/web/src/components/storage/VolumeLocationSelectorTable.jsx b/web/src/components/storage/VolumeLocationSelectorTable.jsx index 0392bd1248..b3ad6c8cf6 100644 --- a/web/src/components/storage/VolumeLocationSelectorTable.jsx +++ b/web/src/components/storage/VolumeLocationSelectorTable.jsx @@ -67,7 +67,11 @@ const deviceUsers = (item, targetDevices, volumes) => { * @param {string[]} props.users */ const DeviceUsage = ({ users }) => { - return users.map((user, index) => {user}); + return ( +
+ {users.map((user, index) => {user})} +
+ ); }; /** From 9bfedf764c2f22b6fedfdd1d20e0eab39d1b7291 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 10:06:10 +0100 Subject: [PATCH 18/31] web: CSS tweaks for VolumeLocationDialog table * Hide the expand button * Indent children devices --- web/src/assets/styles/composition.scss | 14 ++++++++++++++ .../components/storage/VolumeLocationDialog.jsx | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/web/src/assets/styles/composition.scss b/web/src/assets/styles/composition.scss index 67a3c1a29f..22c346f366 100644 --- a/web/src/assets/styles/composition.scss +++ b/web/src/assets/styles/composition.scss @@ -47,6 +47,20 @@ } } +.location-layout table button[id^="expand-toggle"] { + display: none; +} + +.location-layout table tbody tr:not(:first-child) { + td:nth-child(2) { + padding-inline-start: 2ch; + } + + td:nth-child(3) { + padding-inline-start: 5ch; + } +} + [data-state="reversed"] { flex-direction: row-reverse; } diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index 87965c0917..f50b6e36fe 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -178,7 +178,7 @@ export default function VolumeLocationDialog({ description={ // TRANSLATORS: %s is replaced by a file system type (e.g., Ext4). sprintf(_("The selected device will be formatted as %s file system."), - volume.fsType) + volume.fsType) } isChecked={target === "DEVICE"} isDisabled={!targets.includes("DEVICE")} From 98a46ac9deb1ef94db1a41af81ecd9a3863c70a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 13:50:43 +0100 Subject: [PATCH 19/31] service: Do not set device if reusing a device --- .../to_y2storage.rb | 4 +++- .../to_y2storage_test.rb | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) 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/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 From b2d35126a69c9fa0e9d2299bf2d9696eb07e1513 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 13:51:47 +0100 Subject: [PATCH 20/31] web: Small fixes in styles --- web/src/assets/styles/composition.scss | 10 ++-------- web/src/assets/styles/patternfly-overrides.scss | 6 +++++- web/src/components/storage/VolumeDialog.jsx | 2 +- web/src/components/storage/VolumeLocationDialog.jsx | 3 ++- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/web/src/assets/styles/composition.scss b/web/src/assets/styles/composition.scss index 22c346f366..306975066f 100644 --- a/web/src/assets/styles/composition.scss +++ b/web/src/assets/styles/composition.scss @@ -3,10 +3,6 @@ margin-block-end: var(--stack-gutter); } -.stack.small { - --stack-gutter: var(--spacer-smaller); -} - .flex-stack { display: flex; flex-direction: column; @@ -39,6 +35,7 @@ } form > div:first-child { + min-block-size: 120px; overflow-y: auto; } @@ -47,15 +44,12 @@ } } +// FIXME: Teporary 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(2) { - padding-inline-start: 2ch; - } - td:nth-child(3) { padding-inline-start: 5ch; } diff --git a/web/src/assets/styles/patternfly-overrides.scss b/web/src/assets/styles/patternfly-overrides.scss index 9570e53e4f..71361c6f53 100644 --- a/web/src/assets/styles/patternfly-overrides.scss +++ b/web/src/assets/styles/patternfly-overrides.scss @@ -268,8 +268,12 @@ ul { vertical-align: middle; } +.pf-v5-c-radio { + align-items: center; +} + .pf-v5-c-radio__label { - font-size: var(--fs-large); + font-size: var(--pf-v5-c-form__label--FontSize); } @media screen and (width <= 768px) { diff --git a/web/src/components/storage/VolumeDialog.jsx b/web/src/components/storage/VolumeDialog.jsx index b116447580..40ac5337cf 100644 --- a/web/src/components/storage/VolumeDialog.jsx +++ b/web/src/components/storage/VolumeDialog.jsx @@ -757,8 +757,8 @@ export default function VolumeDialog({ return ( /** @fixme blockSize medium is too big and small is too small. */ -
+ -
+
Date: Thu, 2 May 2024 16:03:40 +0100 Subject: [PATCH 21/31] web: Small visual fix at DeviceSelectorTable --- web/src/components/storage/DeviceSelectorTable.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/storage/DeviceSelectorTable.jsx b/web/src/components/storage/DeviceSelectorTable.jsx index 6874a181e4..839ad265fd 100644 --- a/web/src/components/storage/DeviceSelectorTable.jsx +++ b/web/src/components/storage/DeviceSelectorTable.jsx @@ -168,7 +168,7 @@ const DeviceExtendedDetails = ({ item }) => { const System = ({ system }) => { const logo = /windows/i.test(system) ? "windows_logo" : "linux_logo"; - return <> {system}; + return
{system}
; }; return device.systems.map((s, i) => ); From 82f7be5f515648656a04cb4b3d6674a3cedaa62a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 16:24:48 +0100 Subject: [PATCH 22/31] web: Add tests for VolumeLocationDialog --- .../storage/VolumeLocationDialog.test.jsx | 223 ++++++++---------- 1 file changed, 97 insertions(+), 126 deletions(-) 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(); }); }); From b397231e56e1ae57185831b6a106bcb238578ab4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 16:25:47 +0100 Subject: [PATCH 23/31] web: Small fixes --- web/src/components/storage/PartitionsField.jsx | 2 +- .../storage/VolumeLocationDialog.jsx | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 4ad32c49f6..892f17ffa4 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -767,7 +767,7 @@ const Advanced = ({ * @property {Volume[]} volumes - Volumes to show * @property {Volume[]} templates - Templates to use for new volumes * @property {StorageDevice[]} availableDevices - Devices available for installation - * @property {StorageDevice[]} volumeDevices + * @property {StorageDevice[]} volumeDevices - Devices that can be selected as target for a volume * @property {ProposalTarget} target - Installation target * @property {StorageDevice[]} targetDevices * @property {boolean} configureBoot - Whether to configure boot partitions. diff --git a/web/src/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index 2d7c8e8c45..946d05259a 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -41,30 +41,30 @@ import VolumeLocationSelectorTable from "~/components/storage/VolumeLocationSele 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."); -/** @type {(device: StorageDevice) => VolumeTarget} */ +/** @type {(device: StorageDevice|undefined) => VolumeTarget} */ const defaultTarget = (device) => { - if (["partition", "lvmLv", "md"].includes(device.type)) return "DEVICE"; + if (["partition", "lvmLv", "md"].includes(device?.type)) return "DEVICE"; return "NEW_PARTITION"; }; -/** @type {(volume: Volume, device: StorageDevice) => VolumeTarget[]} */ +/** @type {(volume: Volume, device: StorageDevice|undefined) => VolumeTarget[]} */ const availableTargets = (volume, device) => { /** @type {VolumeTarget[]} */ const targets = ["DEVICE"]; - if (device.isDrive) { + if (device?.isDrive) { targets.push("NEW_PARTITION"); targets.push("NEW_VG"); } - if (device.filesystem && volume.outline.fsTypes.includes(device.filesystem.type)) + if (device?.filesystem && volume.outline.fsTypes.includes(device.filesystem.type)) targets.push("FILESYSTEM"); return targets; }; -/** @type {(volume: Volume, device: StorageDevice) => VolumeTarget} */ +/** @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); @@ -95,12 +95,15 @@ export default function VolumeLocationDialog({ onAccept, ...props }) { + /** @type {StorageDevice|undefined} */ const initialDevice = volume.targetDevice || targetDevices[0] || volumeDevices[0]; + /** @type {VolumeTarget} */ const initialTarget = sanitizeTarget(volume, initialDevice); const [target, setTarget] = useState(initialTarget); const [targetDevice, setTargetDevice] = useState(initialDevice); + /** @type {(devices: StorageDevice[]) => void} */ const changeTargetDevice = (devices) => { const newTargetDevice = devices[0]; @@ -110,6 +113,7 @@ export default function VolumeLocationDialog({ } }; + /** @type {(e: import("react").FormEvent) => void} */ const onSubmit = (e) => { e.preventDefault(); const newVolume = { ...volume, target, targetDevice }; @@ -123,6 +127,8 @@ export default function VolumeLocationDialog({ const targets = availableTargets(volume, targetDevice); + if (!targetDevice) return null; + return ( Date: Thu, 2 May 2024 16:26:04 +0100 Subject: [PATCH 24/31] web: Fix some tests --- web/src/components/storage/PartitionsField.test.jsx | 6 +++--- web/src/components/storage/ProposalSettingsSection.test.jsx | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/web/src/components/storage/PartitionsField.test.jsx b/web/src/components/storage/PartitionsField.test.jsx index aa6ff7e62b..e7f20052cc 100644 --- a/web/src/components/storage/PartitionsField.test.jsx +++ b/web/src/components/storage/PartitionsField.test.jsx @@ -183,10 +183,10 @@ beforeEach(() => { props = { volumes: [rootVolume, swapVolume], templates: [], - devices: [], - system: [sda], + availableDevices: [], + volumeDevices: [sda], target: "DISK", - targetDevice: undefined, + targetDevices: [], configureBoot: false, bootDevice: undefined, defaultBootDevice: undefined, diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx index 7071a83780..35f9f9b934 100644 --- a/web/src/components/storage/ProposalSettingsSection.test.jsx +++ b/web/src/components/storage/ProposalSettingsSection.test.jsx @@ -106,6 +106,7 @@ beforeEach(() => { installationDevices: [sda, sdb] }, availableDevices: [], + volumeDevices: [], encryptionMethods: [], volumeTemplates: [], onChange: jest.fn() From e77b83a12360a3469b555e97a3f014560353a617 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 17:41:56 +0100 Subject: [PATCH 25/31] web: Fix and adapt some tests --- .../components/storage/BootConfigField.jsx | 16 +- .../storage/BootConfigField.test.jsx | 9 +- .../storage/DeviceSelectionDialog.test.jsx | 265 ++++++++++++++++-- 3 files changed, 252 insertions(+), 38 deletions(-) diff --git a/web/src/components/storage/BootConfigField.jsx b/web/src/components/storage/BootConfigField.jsx index 4b6cca3eb1..c1b57571f3 100644 --- a/web/src/components/storage/BootConfigField.jsx +++ b/web/src/components/storage/BootConfigField.jsx @@ -56,17 +56,19 @@ 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.availableDevices - * @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, 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/DeviceSelectionDialog.test.jsx b/web/src/components/storage/DeviceSelectionDialog.test.jsx index 762ae120be..74dd349b1b 100644 --- a/web/src/components/storage/DeviceSelectionDialog.test.jsx +++ b/web/src/components/storage/DeviceSelectionDialog.test.jsx @@ -32,7 +32,7 @@ import { DeviceSelectionDialog } from "~/components/storage"; */ /** @type {StorageDevice} */ -const sda = { +const vda = { sid: 59, isDrive: true, type: "disk", @@ -40,45 +40,88 @@ const sda = { model: "Micron 1100 SATA", driver: ["ahci", "mmcblk"], bus: "IDE", - busId: "", transport: "usb", dellBOSS: false, sdCard: true, active: true, - name: "/dev/sda", + name: "/dev/vda", description: "", size: 1024, - recoverableSize: 0, - systems : [], + 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"], }; /** @type {StorageDevice} */ -const sdb = { +const vda1 = { + sid: 60, + isDrive: false, + type: "partition", + active: true, + name: "/dev/vda1", + description: "", + size: 512, + start: 123, + encrypted: false, + recoverableSize: 128, + systems : [], + udevIds: [], + udevPaths: [], + isEFI: false +}; + +/** @type {StorageDevice} */ +const vda2 = { + sid: 61, + 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: "Samsung", - model: "Samsung Evo 8 Pro", - driver: ["ahci"], + vendor: "Disk", + model: "", + driver: [], bus: "IDE", busId: "", transport: "", dellBOSS: false, sdCard: false, active: true, - name: "/dev/sdb", + name: "/dev/vdb", description: "", size: 2048, + start: 0, + encrypted: false, recoverableSize: 0, systems : [], udevIds: [], - udevPaths: ["pci-0000:00-19"] + udevPaths: [] }; /** @type {StorageDevice} */ -const sdc = { +const vdc = { sid: 63, isDrive: true, type: "disk", @@ -91,7 +134,7 @@ const sdc = { dellBOSS: false, sdCard: false, active: true, - name: "/dev/sdc", + name: "/dev/vdc", description: "", size: 2048, recoverableSize: 0, @@ -100,6 +143,91 @@ const sdc = { udevPaths: ["pci-0000:00-19"] }; +/** @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: [] +}; + /** @type {DeviceSelectionDialogProps} */ let props; @@ -138,7 +266,7 @@ describe("DeviceSelectionDialog", () => { target: "DISK", targetDevice: undefined, targetPVDevices: [], - devices: [sda, sdb, sdc], + devices: [vda, vdb, vdc, md0, raid, multipath, dasd], onCancel: jest.fn(), onAccept: jest.fn() }; @@ -157,7 +285,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", () => { @@ -179,9 +307,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 () => { @@ -204,7 +332,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", () => { @@ -226,9 +354,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 () => { @@ -260,7 +388,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 () => { @@ -270,14 +398,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: [] }); }); @@ -286,7 +414,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 () => { @@ -296,17 +424,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"); + }); + }); }); From e2ae65d36b07b1d64d1fd61442e5f52da37d8531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 17:48:34 +0100 Subject: [PATCH 26/31] web: Make scrollable table more discoverable Relying in the Lea Verou technique described at https://lea.verou.me/blog/2012/04/background-attachment-local. It makes the scrollable table a bit more evident, but still feeling as a workaround. --- web/src/assets/styles/composition.scss | 7 +++++-- web/src/assets/styles/utilities.scss | 12 ++++++++++++ web/src/components/storage/VolumeLocationDialog.jsx | 6 ++++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/web/src/assets/styles/composition.scss b/web/src/assets/styles/composition.scss index 306975066f..57f98a5bac 100644 --- a/web/src/assets/styles/composition.scss +++ b/web/src/assets/styles/composition.scss @@ -32,11 +32,14 @@ display: flex; flex-direction: column; flex: 1 1 0; + gap: 0 } - form > div:first-child { - min-block-size: 120px; + 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 { 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/components/storage/VolumeLocationDialog.jsx b/web/src/components/storage/VolumeLocationDialog.jsx index 946d05259a..b94350c0e4 100644 --- a/web/src/components/storage/VolumeLocationDialog.jsx +++ b/web/src/components/storage/VolumeLocationDialog.jsx @@ -142,7 +142,9 @@ export default function VolumeLocationDialog({ {...props} > - + { /** FIXME: Rename FormReadOnlyField */} + +
d.sid)} variant="compact" /> - +
Date: Thu, 2 May 2024 18:11:19 +0100 Subject: [PATCH 27/31] web: Please CSpell --- web/cspell.json | 1 + web/src/assets/styles/composition.scss | 2 +- .../storage/DeviceSelectionDialog.test.jsx | 19 ++++++++++--------- 3 files changed, 12 insertions(+), 10 deletions(-) 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/src/assets/styles/composition.scss b/web/src/assets/styles/composition.scss index 57f98a5bac..7055502e3f 100644 --- a/web/src/assets/styles/composition.scss +++ b/web/src/assets/styles/composition.scss @@ -47,7 +47,7 @@ } } -// FIXME: Teporary solution to hide expand button. The button should be removed instead. +// FIXME: Temporary solution to hide expand button. The button should be removed instead. .location-layout table button[id^="expand-toggle"] { display: none; } diff --git a/web/src/components/storage/DeviceSelectionDialog.test.jsx b/web/src/components/storage/DeviceSelectionDialog.test.jsx index 74dd349b1b..5b9d4591c2 100644 --- a/web/src/components/storage/DeviceSelectionDialog.test.jsx +++ b/web/src/components/storage/DeviceSelectionDialog.test.jsx @@ -20,6 +20,7 @@ */ // @ts-check +// cspell:ignore dasda ddgdcbibhd import React from "react"; import { screen, within } from "@testing-library/react"; @@ -47,7 +48,7 @@ const vda = { name: "/dev/vda", description: "", size: 1024, - systems : ["Windows 11", "openSUSE Leap 15.2"], + 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"], }; @@ -64,7 +65,7 @@ const vda1 = { start: 123, encrypted: false, recoverableSize: 128, - systems : [], + systems: [], udevIds: [], udevPaths: [], isEFI: false @@ -82,7 +83,7 @@ const vda2 = { start: 1789, encrypted: false, recoverableSize: 0, - systems : [], + systems: [], udevIds: [], udevPaths: [], isEFI: false @@ -115,7 +116,7 @@ const vdb = { start: 0, encrypted: false, recoverableSize: 0, - systems : [], + systems: [], udevIds: [], udevPaths: [] }; @@ -138,7 +139,7 @@ const vdc = { description: "", size: 2048, recoverableSize: 0, - systems : [], + systems: [], udevIds: [], udevPaths: ["pci-0000:00-19"] }; @@ -155,7 +156,7 @@ const md0 = { name: "/dev/md0", description: "", size: 2048, - systems : [], + systems: [], udevIds: [], udevPaths: [] }; @@ -178,7 +179,7 @@ const raid = { name: "/dev/mapper/isw_ddgdcbibhd_244", description: "", size: 2048, - systems : [], + systems: [], udevIds: [], udevPaths: [] }; @@ -201,7 +202,7 @@ const multipath = { name: "/dev/mapper/36005076305ffc73a00000000000013b4", description: "", size: 2048, - systems : [], + systems: [], udevIds: [], udevPaths: [] }; @@ -223,7 +224,7 @@ const dasd = { name: "/dev/dasda", description: "", size: 2048, - systems : [], + systems: [], udevIds: [], udevPaths: [] }; From 068e7b35bd8150ecd5a4e279c4c10ec6109969a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 3 May 2024 07:41:54 +0100 Subject: [PATCH 28/31] web: Add and adapt tests --- .../storage/BootSelectionDialog.jsx | 29 +- .../storage/BootSelectionDialog.test.jsx | 16 +- .../components/storage/ProposalPage.test.jsx | 1 + .../components/storage/device-utils.test.jsx | 284 +++++++----------- 4 files changed, 137 insertions(+), 193 deletions(-) diff --git a/web/src/components/storage/BootSelectionDialog.jsx b/web/src/components/storage/BootSelectionDialog.jsx index 565ddfa2f8..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,18 +57,20 @@ 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.availableDevices - 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, @@ -77,8 +78,8 @@ export default function BootSelectionDialog({ defaultBootDevice, availableDevices, isOpen, - onCancel = noop, - onAccept = noop, + onCancel, + onAccept, ...props }) { const [configureBoot, setConfigureBoot] = useState(configureBootProp); diff --git a/web/src/components/storage/BootSelectionDialog.test.jsx b/web/src/components/storage/BootSelectionDialog.test.jsx index 4b3a80fe39..f03b8bee88 100644 --- a/web/src/components/storage/BootSelectionDialog.test.jsx +++ b/web/src/components/storage/BootSelectionDialog.test.jsx @@ -26,6 +26,12 @@ import { screen, within } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { BootSelectionDialog } from "~/components/storage"; +/** + * @typedef {import("./BootSelectionDialog").BootSelectionDialogProps} BootSelectionDialogProps + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + */ + +/** @type {StorageDevice} */ const sda = { sid: 59, isDrive: true, @@ -40,6 +46,7 @@ const sda = { sdCard: true, active: true, name: "/dev/sda", + description: "", size: 1024, recoverableSize: 0, systems : [], @@ -47,6 +54,7 @@ const sda = { udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], }; +/** @type {StorageDevice} */ const sdb = { sid: 62, isDrive: true, @@ -61,6 +69,7 @@ const sdb = { sdCard: false, active: true, name: "/dev/sdb", + description: "", size: 2048, recoverableSize: 0, systems : [], @@ -68,6 +77,7 @@ const sdb = { udevPaths: ["pci-0000:00-19"] }; +/** @type {StorageDevice} */ const sdc = { sid: 63, isDrive: true, @@ -82,6 +92,7 @@ const sdc = { sdCard: false, active: true, name: "/dev/sdc", + description: "", size: 2048, recoverableSize: 0, systems : [], @@ -89,6 +100,7 @@ const sdc = { udevPaths: ["pci-0000:00-19"] }; +/** @type {BootSelectionDialogProps} */ let props; describe("BootSelectionDialog", () => { @@ -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/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 661223c847..0e2b3a4f83 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -151,6 +151,7 @@ beforeEach(() => { // @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/device-utils.test.jsx b/web/src/components/storage/device-utils.test.jsx index 770d1fda03..ecae766a66 100644 --- a/web/src/components/storage/device-utils.test.jsx +++ b/web/src/components/storage/device-utils.test.jsx @@ -25,12 +25,18 @@ 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 +55,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 +73,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 +94,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"); }); }); From f4ce92b6d20a3ee6a3d74a4b77968b41ee1246d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 3 May 2024 07:42:15 +0100 Subject: [PATCH 29/31] web: Minor fixes --- web/src/components/storage/device-utils.jsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/src/components/storage/device-utils.jsx b/web/src/components/storage/device-utils.jsx index 80a90cbf4e..6d1a4666c9 100644 --- a/web/src/components/storage/device-utils.jsx +++ b/web/src/components/storage/device-utils.jsx @@ -57,7 +57,9 @@ const FilesystemLabel = ({ item }) => { if (!device) return null; const label = device.filesystem?.label; - if (label) return {label}; + if (!label) return null; + + return {label}; }; /** From 2dddefa87188f94a0683c909adbc21dc74bb69f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 3 May 2024 10:46:47 +0100 Subject: [PATCH 30/31] web: Changelog --- web/package/cockpit-agama.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 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 From ca7e244fbe3458149ea41dd21c3084bbd22daf05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 3 May 2024 12:15:56 +0100 Subject: [PATCH 31/31] web: Improvements from review --- web/src/client/storage.js | 5 +---- web/src/components/storage/VolumeLocationSelectorTable.jsx | 4 ++-- web/src/components/storage/device-utils.test.jsx | 1 - 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 2303485570..182b1166bc 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -472,10 +472,7 @@ class ProposalManager { return !!partitions.find(d => d.name === device.name); }; - return ( - !!availableDevices.find(d => d.name === device.name) || - !!availableDevices.find(d => isChildren(device, d)) - ); + return !!availableDevices.find(d => d.name === device.name || isChildren(device, d)); }; /** @type {(device: StorageDevice[]) => boolean} */ diff --git a/web/src/components/storage/VolumeLocationSelectorTable.jsx b/web/src/components/storage/VolumeLocationSelectorTable.jsx index b3ad6c8cf6..de8be15618 100644 --- a/web/src/components/storage/VolumeLocationSelectorTable.jsx +++ b/web/src/components/storage/VolumeLocationSelectorTable.jsx @@ -31,8 +31,8 @@ import { import { ExpandableSelector } from "~/components/core"; /** - * @typedef {import("../core/ExpandableSelector").ExpandableSelectorColumn} ExpandableSelectorColumn - * @typedef {import("../core/ExpandableSelector").ExpandableSelectorProps} ExpandableSelectorProps + * @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 diff --git a/web/src/components/storage/device-utils.test.jsx b/web/src/components/storage/device-utils.test.jsx index ecae766a66..1fdc0f040e 100644 --- a/web/src/components/storage/device-utils.test.jsx +++ b/web/src/components/storage/device-utils.test.jsx @@ -20,7 +20,6 @@ */ // @ts-check -// cspell:ignore dasda ddgdcbibhd import React from "react"; import { screen } from "@testing-library/react";