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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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;