From e2002509d9f4f7ad2025acca2a4666531187863d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 07:42:16 +0000 Subject: [PATCH 01/39] web: Add component for adding visual prominence --- web/src/assets/styles/blocks.scss | 21 ++++++++++++++ web/src/assets/styles/variables.scss | 4 +++ web/src/components/core/Tag.jsx | 41 ++++++++++++++++++++++++++++ web/src/components/core/Tag.test.jsx | 33 ++++++++++++++++++++++ web/src/components/core/index.js | 1 + 5 files changed, 100 insertions(+) create mode 100644 web/src/components/core/Tag.jsx create mode 100644 web/src/components/core/Tag.test.jsx diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index e568ae24db..07060b3806 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -403,6 +403,27 @@ ul[data-type="agama/list"][role="grid"] { } } +[data-type="agama/tag"] { + font-size: var(--fs-small); + + &[data-variant="teal"] { + color: var(--color-teal); + } + + &[data-variant="orange"] { + color: var(--color-orange); + } + + &[data-variant="gray-highlight"] { + padding: var(--spacer-smaller); + color: var(--color-gray-darkest); + background: var(--color-gray); + border: 1px solid var(--color-gray-dark); + border-radius: 5px; + margin-inline-start: var(--spacer-smaller); + } +} + // compact lists in popover .pf-v5-c-popover li + li { margin: 0; diff --git a/web/src/assets/styles/variables.scss b/web/src/assets/styles/variables.scss index b25bfcbad3..490429fe54 100644 --- a/web/src/assets/styles/variables.scss +++ b/web/src/assets/styles/variables.scss @@ -51,8 +51,12 @@ --color-gray: #f2f2f2; --color-gray-dark: #efefef; // Fog --color-gray-darker: #999; + --color-gray-darkest: #333; --color-gray-dimmed: #888; --color-gray-dimmest: #666; + --color-teal: #279c9c; + --color-blue: #0d4ea6; + --color-orange: #e86427; --color-link: #0c322c; --color-link-hover: #30ba78; diff --git a/web/src/components/core/Tag.jsx b/web/src/components/core/Tag.jsx new file mode 100644 index 0000000000..c910d07295 --- /dev/null +++ b/web/src/components/core/Tag.jsx @@ -0,0 +1,41 @@ +/* + * 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"; + +/** + * Simple component that helps wrapped content stand out visually. The variant + * prop determines what kind of enhancement is applied. + * @component + * + * @param {object} props + * @param {("simple"|"teal"|"orange"|"gray-highlight")} [props.variant="simple"] + * @param {React.ReactNode} props.children + */ +export default function Tag ({ variant = "simple", children }) { + return ( + + {children} + + ); +} diff --git a/web/src/components/core/Tag.test.jsx b/web/src/components/core/Tag.test.jsx new file mode 100644 index 0000000000..e4a9739abf --- /dev/null +++ b/web/src/components/core/Tag.test.jsx @@ -0,0 +1,33 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { Tag } from "~/components/core"; + +describe("Tag", () => { + it("renders its children in a node with data-type='agama/tag' attribute", () => { + plainRender(New); + const node = screen.getByText("New"); + expect(node).toHaveAttribute("data-type", "agama/tag"); + }); +}); diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index 6bdac86248..55d5e5e645 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -58,3 +58,4 @@ export { default as DevelopmentInfo } from "./DevelopmentInfo"; export { default as Selector } from "./Selector"; export { default as OptionsPicker } from "./OptionsPicker"; export { default as Reminder } from "./Reminder"; +export { default as Tag } from "./Tag"; From 25a6445c50a37efce47fc71b2853b151c19677f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 08:11:44 +0000 Subject: [PATCH 02/39] web: Add core/TreeTable component To make it easier mount PF/Tabe#isTreeTable component. Test still pending. --- web/src/assets/styles/blocks.scss | 14 +++ web/src/components/core/TreeTable.jsx | 134 +++++++++++++++++++++ web/src/components/core/TreeTable.test.jsx | 24 ++++ web/src/components/core/index.js | 1 + 4 files changed, 173 insertions(+) create mode 100644 web/src/components/core/TreeTable.jsx create mode 100644 web/src/components/core/TreeTable.test.jsx diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 07060b3806..645cdc84c1 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -424,6 +424,20 @@ ul[data-type="agama/list"][role="grid"] { } } +table[data-type="agama/tree-table"] { + th:first-child { + width: 1%; + padding-inline-end: var(--spacer-normal); + } + + @media (width <= 768px) { + td:empty, + td div:empty { + display: none; + } + } +} + // compact lists in popover .pf-v5-c-popover li + li { margin: 0; diff --git a/web/src/components/core/TreeTable.jsx b/web/src/components/core/TreeTable.jsx new file mode 100644 index 0000000000..1e09dd68ce --- /dev/null +++ b/web/src/components/core/TreeTable.jsx @@ -0,0 +1,134 @@ +/* + * 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 { Table, Thead, Tr, Th, Tbody, Td, TreeRowWrapper } from '@patternfly/react-table'; + +/** + * @typedef {import("@patternfly/react-table").TableProps} TableProps + */ + +/** + * @typedef {object} TreeTableColumn + * @property {string} title + * @property {(any) => React.ReactNode} content + * @property {string} [classNames] + */ + +/** + * @typedef {object} TreeTableBaseProps + * @property {TreeTableColumn[]} columns=[] + * @property {object[]} items=[] + * @property {(any) => array} [itemChildren] + * @property {(any) => string} [rowClassNames] + */ + +/** + * Table built on top of PF/Table + * @component + * + * FIXME: omitting `ref` here to avoid a TypeScript error but keep component as + * typed as possible. Further investigation is needed. + * + * @typedef {TreeTableBaseProps & Omit} TreeTableProps + * + * @param {TreeTableProps} props + */ +export default function TreeTable({ + columns = [], + items = [], + itemChildren = () => [], + rowClassNames = () => "", + ...tableProps +}) { + const renderColumns = (item, treeRow) => { + return columns.map((c, cIdx) => { + const props = { + dataLabel: c.title, + className: c.classNames + }; + + if (cIdx === 0) props.treeRow = treeRow; + + return ( + // FIXME: using an array below because for some reason React is + // complaining about + // Objects are not valid as a React child (found: object with keys {title}). If you meant to render a collection of children, use an array instead. + // when rendering the first column using the treeRow prop + // Checking the PF/Table code might help to understand what is going on + // there + {[c.content(item)]} + ); + }); + }; + + const renderRows = (items, level) => { + if (items?.length <= 0) return; + + return ( + items.map((item, itemIdx) => { + const children = itemChildren(item); + + const treeRow = { + props: { + isExpanded: true, + isDetailsExpanded: true, + "aria-level": level, + "aria-posinset": itemIdx + 1, + "aria-setsize": children?.length || 0 + } + }; + + const rowProps = { + row: { props: treeRow.props }, + className: rowClassNames(item) + }; + + return ( + + {renderColumns(item, treeRow)} + { renderRows(children, level + 1)} + + ); + }) + ); + }; + + return ( + + + + { columns.map((c, i) => ) } + + + + { renderRows(items, 1) } + +
{c.title}
+ ); +} diff --git a/web/src/components/core/TreeTable.test.jsx b/web/src/components/core/TreeTable.test.jsx new file mode 100644 index 0000000000..c1c858da16 --- /dev/null +++ b/web/src/components/core/TreeTable.test.jsx @@ -0,0 +1,24 @@ +/* + * 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. + */ + +describe("TreeTable", () => { + it.todo("add examples for testing core/TreeTable component"); +}); diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index 55d5e5e645..f6448c03d3 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -59,3 +59,4 @@ export { default as Selector } from "./Selector"; export { default as OptionsPicker } from "./OptionsPicker"; export { default as Reminder } from "./Reminder"; export { default as Tag } from "./Tag"; +export { default as TreeTable } from "./TreeTable"; From 232096bd92429eb92692234f91e52e4b0062460a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 09:41:54 +0000 Subject: [PATCH 03/39] web: Add a Result section to Storage/ProposalPage Some tests were skipped. They are going to be adapted ASAP. --- web/src/assets/styles/blocks.scss | 38 +++ web/src/components/storage/ProposalPage.jsx | 61 +++-- .../components/storage/ProposalPage.test.jsx | 25 +- .../storage/ProposalResultSection.jsx | 224 ++++++++++++++++++ .../storage/ProposalResultSection.test.jsx | 29 +++ web/src/components/storage/index.js | 1 + 6 files changed, 335 insertions(+), 43 deletions(-) create mode 100644 web/src/components/storage/ProposalResultSection.jsx create mode 100644 web/src/components/storage/ProposalResultSection.test.jsx diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 645cdc84c1..2dc067cb41 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -438,6 +438,44 @@ table[data-type="agama/tree-table"] { } } +table.proposal-result { + tr.dimmed-row { + background-color: #fff; + opacity: 0.8; + background: repeating-linear-gradient( -45deg, #fcfcff, #fcfcff 3px, #fff 3px, #fff 10px ); + + td { + color: var(--color-gray-dimmed); + padding-block: 0; + } + } + + @media (width > 768px) { + th.details-column { + padding-inline-start: calc(60px + var(--spacer-smaller) * 2); + } + + td.details-column { + display: grid; + gap: var(--spacer-smaller); + grid-template-columns: 60px 1fr; + + :first-child { + text-align: end; + } + } + + th.sizes-column, + td.sizes-column { + text-align: end; + + div.split { + justify-content: flex-end; + } + } + } +} + // compact lists in popover .pf-v5-c-popover li + li { margin: 0; diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index c3d4753623..46a2c9b8ab 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -26,11 +26,11 @@ import { useInstallerClient } from "~/context/installer"; import { toValidationError, useCancellablePromise } from "~/utils"; import { Page } from "~/components/core"; import { - ProposalActionsSection, ProposalPageMenu, - ProposalSettingsSection, ProposalDeviceSection, - ProposalTransactionalInfo + ProposalTransactionalInfo, + ProposalSettingsSection, + ProposalResultSection } from "~/components/storage"; import { IDLE } from "~/client/status"; @@ -216,40 +216,33 @@ export default function ProposalPage() { calculate(newSettings).catch(console.error); }; - const PageContent = () => { - return ( - <> - - - - - - ); - }; - return ( - // TRANSLATORS: page title + // TRANSLATORS: Storage page title - + + + + ); } diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index e47906c18b..2822f6b75a 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -76,6 +76,7 @@ const vdb = { size: 1e+6 }; +// FIXME: needed to be reviewed/adapted after latest changes const storageMock = { probe: jest.fn().mockResolvedValue(0), proposal: { @@ -84,7 +85,13 @@ const storageMock = { getProductMountPoints: jest.fn().mockResolvedValue([]), getResult: jest.fn().mockResolvedValue(undefined), defaultVolume: jest.fn(mountPath => Promise.resolve({ mountPath })), - calculate: jest.fn().mockResolvedValue(0) + calculate: jest.fn().mockResolvedValue(0), + }, + system: { + getDevices: jest.fn().mockResolvedValue([vda, vdb]), + }, + staging: { + getDevices: jest.fn().mockResolvedValue([vda, vdb]), }, system: { getDevices: jest.fn().mockResolvedValue([vda, vdb]) @@ -105,18 +112,18 @@ beforeEach(() => { createClient.mockImplementation(() => ({ storage })); }); -it("probes storage if the storage devices are deprecated", async () => { +it.skip("probes storage if the storage devices are deprecated", async () => { storage.isDeprecated = jest.fn().mockResolvedValue(true); installerRender(); await waitFor(() => expect(storage.probe).toHaveBeenCalled()); }); -it("does not probe storage if the storage devices are not deprecated", async () => { +it.skip("does not probe storage if the storage devices are not deprecated", async () => { installerRender(); await waitFor(() => expect(storage.probe).not.toHaveBeenCalled()); }); -it("loads the proposal data", async () => { +it.skip("loads the proposal data", async () => { storage.proposal.getResult = jest.fn().mockResolvedValue( { settings: { bootDevice: vda.name } } ); @@ -129,15 +136,15 @@ it("loads the proposal data", async () => { await screen.findByText(/\/dev\/vda/); }); -it("renders the device, settings and actions sections", async () => { +it.skip("renders the device, settings, find space and result sections", async () => { installerRender(); await screen.findByText(/Device/); await screen.findByText(/Settings/); - await screen.findByText(/Planned Actions/); + await screen.findByText(/Result/); }); -describe("when the storage devices become deprecated", () => { +describe.skip("when the storage devices become deprecated", () => { it("probes storage", async () => { const [mockFunction, callbacks] = createCallbackMock(); storage.onDeprecate = mockFunction; @@ -171,7 +178,7 @@ describe("when the storage devices become deprecated", () => { }); }); -describe("when there is no proposal yet", () => { +describe.skip("when there is no proposal yet", () => { beforeEach(() => { storage.proposal.getResult = jest.fn().mockResolvedValue(undefined); }); @@ -201,7 +208,7 @@ describe("when there is no proposal yet", () => { }); }); -describe("when there is a proposal", () => { +describe.skip("when there is a proposal", () => { beforeEach(() => { storage.proposal.getResult = jest.fn().mockResolvedValue( { settings: { bootDevice: vda.name } } diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx new file mode 100644 index 0000000000..d33ab99cec --- /dev/null +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -0,0 +1,224 @@ +/* + * 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 { Alert, Skeleton } from "@patternfly/react-core"; +import { sprintf } from "sprintf-js"; +import { _ } from "~/i18n"; +import { deviceSize } from "~/components/storage/utils"; +import { If, Section, Tag, TreeTable } from "~/components/core"; + +/** + * @todo Create a component for rendering a customized skeleton + */ +const ActionsSkeleton = () => { + return ( + <> + + + + + + + ); +}; + +/** + * Returns the delete actions from given list + * + * @param {object[]} actions + * @returns {object[]} + */ +const deleteActions = (actions) => actions.filter(a => a.delete); + +/** + * Renders a warning alert if there are delete actions + * + * @param {object} props + * @param {object} props.deleteActions + */ +const Warning = ({ deleteActions }) => { + const count = deleteActions.length; + + if (count === 0) return; + + const title = sprintf(_("%s delete actions will be performed"), count); + + return ( + +
    + { deleteActions.map(a =>
  • {a.text}
    • )} +
+ + ); +}; + +/** + * Renders a TreeTable rendering the devices proposal result + * + * FIXME: add expected types + * + * @param {object} props + * @param {object} props.settings + * @param {object} props.devices + */ +const DevicesTreeTable = ({ settings, devices }) => { + const { system: systemDevices = [], staging: stagingDevices = [] } = devices; + + const sids = settings.installationDevices.map(d => d.sid); + const installationDevices = stagingDevices.filter(d => sids.includes(d.sid)); + const lvmVgs = stagingDevices.filter(d => d.logicalVolumes?.find(l => l.filesystem?.mountPath !== undefined)); + + const items = installationDevices.concat(lvmVgs); + + // FIXME: Move to a utils + const childrenFromPartitionTable = ({ partitionTable }) => { + const { partitions, unusedSlots } = partitionTable; + const children = partitions.concat(unusedSlots); + + return children.sort((a, b) => a.start < b.start ? -1 : 1); + }; + + // FIXME: Move to a utils + const childrenFromLvmVg = (vg) => { + return vg.logicalVolumes.sort((a, b) => a.name < b.name ? -1 : 1); + }; + + // FIXME: Move to a utils + const childrenFor = (device) => { + if (device.partitionTable) return childrenFromPartitionTable(device); + if (device.type === "lvmVg") return childrenFromLvmVg(device); + return []; + }; + + const renderNewLabel = (device) => { + if (!device.sid) return; + if (systemDevices.find(d => d.sid === device.sid)) return; + + return ( + {_("New")} + ); + }; + + // FIXME: add the logic to render it conditionally + const renderShrankLabel = (item) => { + if (item) return {_("Shrank")}; + }; + + const renderDeviceName = (item) => { + return ( +
+ {item.sid && item.name} +
+ ); + }; + + const renderFilesystemLabel = (item) => { + const label = item.filesystem?.label; + if (label) return {label}; + }; + + const renderDetails = (item) => { + const description = item.sid ? item.description : _("Unused space"); + + return ( + <> +
{ renderNewLabel(item) }
+
{description} {renderFilesystemLabel(item)}
+ + ); + }; + + const renderSize = (item) => { + return ( +
+ { renderShrankLabel(item) } + { deviceSize(item.size) } +
+ ); + }; + + const renderMountPoint = (item) => item.sid && {item.filesystem?.mountPath}; + + return ( + { + if (!item.sid) return "dimmed-row"; + }} + className="proposal-result" + /> + ); +}; + +/** + * Section holding the proposal result and actions to perform in the system + * @component + * + * FIXME: add expected types + * + * @param {object} props + * @param {object[]} [props.actions=[]] + * @param {object[]} [props.settings=[]] + * @param {object[]} [props.devices=[]] + * @param {import("~/client/mixins").ValidationError[]} props.errors - Validation errors + * @param {boolean} [props.isLoading=false] - Whether the section content should be rendered as loading + */ +export default function ProposalResultSection({ + actions, + settings, + devices, + errors = [], + isLoading = false +}) { + if (isLoading) errors = []; + + return ( +
+ } + else={ + <> + + + + } + /> +
+ ); +} diff --git a/web/src/components/storage/ProposalResultSection.test.jsx b/web/src/components/storage/ProposalResultSection.test.jsx new file mode 100644 index 0000000000..546c7dbad4 --- /dev/null +++ b/web/src/components/storage/ProposalResultSection.test.jsx @@ -0,0 +1,29 @@ +/* + * 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. + */ + +describe("ProposalResultSection", () => { + it.todo("check that it shows a warning when there are deletion actions"); + it.todo("check that it shows a tree table with the proposal result"); + it.todo("check that 'New' label is included when proceed"); + it.todo("check that 'Shrank' label is included when proceed"); + it.todo("check that filesystem label is shown if present"); + it.todo("check that there is a working link for checking all actions"); +}); diff --git a/web/src/components/storage/index.js b/web/src/components/storage/index.js index 8a99b0180d..ca43995771 100644 --- a/web/src/components/storage/index.js +++ b/web/src/components/storage/index.js @@ -26,6 +26,7 @@ export { default as ProposalSpacePolicyField } from "./ProposalSpacePolicyField" export { default as ProposalDeviceSection } from "./ProposalDeviceSection"; export { default as ProposalActionsSection } from "./ProposalActionsSection"; export { default as ProposalTransactionalInfo } from "./ProposalTransactionalInfo"; +export { default as ProposalResultSection } from "./ProposalResultSection"; export { default as ProposalVolumes } from "./ProposalVolumes"; export { default as DASDPage } from "./DASDPage"; export { default as DASDTable } from "./DASDTable"; From f7b92fac6f04f51148a76a3ac2d904ad45583c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 12:59:51 +0000 Subject: [PATCH 04/39] web: Move storage proposal actions to a dialog With some skipped tests. --- ...sSection.jsx => ProposalActionsDialog.jsx} | 99 ++++--------- .../storage/ProposalActionsDialog.test.jsx | 127 ++++++++++++++++ .../storage/ProposalActionsSection.test.jsx | 135 ------------------ .../storage/ProposalResultSection.jsx | 27 +++- web/src/components/storage/index.js | 2 +- 5 files changed, 184 insertions(+), 206 deletions(-) rename web/src/components/storage/{ProposalActionsSection.jsx => ProposalActionsDialog.jsx} (54%) create mode 100644 web/src/components/storage/ProposalActionsDialog.test.jsx delete mode 100644 web/src/components/storage/ProposalActionsSection.test.jsx diff --git a/web/src/components/storage/ProposalActionsSection.jsx b/web/src/components/storage/ProposalActionsDialog.jsx similarity index 54% rename from web/src/components/storage/ProposalActionsSection.jsx rename to web/src/components/storage/ProposalActionsDialog.jsx index 8d35563afd..053f9d852c 100644 --- a/web/src/components/storage/ProposalActionsSection.jsx +++ b/web/src/components/storage/ProposalActionsDialog.jsx @@ -20,21 +20,12 @@ */ import React, { useState } from "react"; -import { - List, - ListItem, - ExpandableSection, - Skeleton, -} from "@patternfly/react-core"; +import { List, ListItem, ExpandableSection, } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; - import { _, n_ } from "~/i18n"; -import { If, Section } from "~/components/core"; import { partition } from "~/utils"; +import { If, Popup } from "~/components/core"; -// TODO: would be nice adding an aria-description to these lists, but aria-description still in -// draft yet and aria-describedby should be used... which id not ideal right now -// https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-description const ActionsList = ({ actions }) => { // Some actions (e.g., deleting a LV) are reported as several actions joined by a line break const actionItems = (action, id) => { @@ -53,15 +44,21 @@ const ActionsList = ({ actions }) => { }; /** - * Renders the list of actions to perform in the system + * Renders a dialog with the given list of actions * @component * * @param {object} props - * @param {object[]} [props.actions=[]] + * @param {object[]} [props.actions=[]] - The actions to perform in the system. + * @param {boolean} [props.isOpen=false] - Whether the dialog is visible or not. + * @param {function} props.onClose - Whether the dialog is visible or not. */ -const ProposalActions = ({ actions = [] }) => { +export default function ProposalActionsDialog({ actions = [], isOpen = false, onClose }) { const [isExpanded, setIsExpanded] = useState(false); + if (typeof onClose !== 'function') { + console.error("Missing ProposalActionsDialog#onClose callback"); + } + if (actions.length === 0) return null; const [generalActions, subvolActions] = partition(actions, a => !a.subvol); @@ -72,66 +69,32 @@ const ProposalActions = ({ actions = [] }) => { : sprintf(n_("Show %d subvolume action", "Show %d subvolume actions", subvolActions.length), subvolActions.length); return ( - <> - - {subvolActions.length > 0 && ( - setIsExpanded(!isExpanded)} - toggleText={toggleText} - className="expandable-actions" - > - - - )} - - ); -}; - -/** - * @todo Create a component for rendering a customized skeleton - */ -const ActionsSkeleton = () => { - return ( - <> - - - - - - - ); -}; - -/** - * Section with the actions to perform in the system - * @component - * - * @param {object} props - * @param {object[]} [props.actions=[]] - * @param {string[]} [props.errors=[]] - * @param {boolean} [props.isLoading=false] - Whether the section content should be rendered as loading - */ -export default function ProposalActionsSection({ actions = [], errors = [], isLoading = false }) { - if (isLoading) errors = []; - - return ( -
+ } - else={} + condition={subvolActions.length > 0} + then={ + setIsExpanded(!isExpanded)} + toggleText={toggleText} + className="expandable-actions" + > + + + } /> -
+ + {_("Close")} + + ); } diff --git a/web/src/components/storage/ProposalActionsDialog.test.jsx b/web/src/components/storage/ProposalActionsDialog.test.jsx new file mode 100644 index 0000000000..41276c83ea --- /dev/null +++ b/web/src/components/storage/ProposalActionsDialog.test.jsx @@ -0,0 +1,127 @@ +/* + * Copyright (c) [2022-2023] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen, within, waitForElementToBeRemoved } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { ProposalActionsDialog } from "~/components/storage"; + +const actions = [ + { text: 'Create GPT on /dev/vdc', subvol: false, delete: false }, + { text: 'Create partition /dev/vdc1 (8.00 MiB) as BIOS Boot Partition', subvol: false, delete: false }, + { text: 'Create encrypted partition /dev/vdc2 (29.99 GiB) as LVM physical volume', subvol: false, delete: false }, + { text: 'Create volume group system0 (29.98 GiB) with /dev/mapper/cr_vdc2 (29.99 GiB)', subvol: false, delete: false }, + { text: 'Create LVM logical volume /dev/system0/root (20.00 GiB) on volume group system0 for / with btrfs', subvol: false, delete: false }, +]; + +const subvolumeActions = [ + { text: 'Create subvolume @ on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/var on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/usr/local on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/srv on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/root on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/opt on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/home on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/boot/writable on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/boot/grub2/x86_64-efi on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, + { text: 'Create subvolume @/boot/grub2/i386-pc on /dev/system0/root (20.00 GiB)', subvol: true, delete: false } +]; + +const destructiveAction = { text: 'Delete ext4 on /dev/vdc', subvol: false, delete: true }; + +it("renders nothing by default", () => { + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); +}); + +it("renders nothing when isOpen=false", () => { + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); +}); + +describe("when isOpen", () => { + it("renders nothing if there are no actions", () => { + plainRender(); + + expect(screen.queryAllByText(/Delete/)).toEqual([]); + expect(screen.queryAllByText(/Create/)).toEqual([]); + expect(screen.queryAllByText(/Show/)).toEqual([]); + }); + + describe.skip("and there are actions", () => { + it("renders a dialog with the list of actions", () => { + plainRender(); + + const dialog = screen.getByRole("dialog", { name: "Proposal Actions"}); + const actionsList = screen.getByRole("list"); + const actionsListItems = within(actionsList).getAllByRole("listitem"); + expect(actionsListItems.map(i => i.textContent)).toEqual(actions.map(a => a.text)); + }); + + describe("when there is a destructive action", () => { + it("emphasizes the action", () => { + plainRender(); + + // https://stackoverflow.com/a/63080940 + const actionItems = screen.getAllByRole("listitem"); + const destructiveActionItem = actionItems.find(item => item.textContent === destructiveAction.text); + + expect(destructiveActionItem).toHaveClass("proposal-action--delete"); + }); + }); + + describe("when there are subvolume actions", () => { + it("does not render the subvolume actions", () => { + plainRender(); + + // For now, we know that there are two lists and the subvolume list is the second one. + // The test could be simplified once we have aria-descriptions for the lists. + const [genericList, subvolList] = screen.getAllByRole("list", { hidden: true }); + expect(genericList).not.toBeNull(); + expect(subvolList).not.toBeNull(); + const subvolItems = within(subvolList).queryAllByRole("listitem"); + expect(subvolItems).toEqual([]); + }); + + it("renders the subvolume actions after clicking on 'show subvolumes'", async () => { + const { user } = plainRender( + + ); + + const link = screen.getByText(/Show.*subvolume actions/); + + expect(screen.getAllByRole("list").length).toEqual(1); + + await user.click(link); + + waitForElementToBeRemoved(link); + screen.getByText(/Hide.*subvolume actions/); + + // For now, we know that there are two lists and the subvolume list is the second one. + // The test could be simplified once we have aria-descriptions for the lists. + const [, subvolList] = screen.getAllByRole("list"); + const subvolItems = within(subvolList).getAllByRole("listitem"); + + expect(subvolItems.map(i => i.textContent)).toEqual(subvolumeActions.map(a => a.text)); + }); + }); + }); +}); diff --git a/web/src/components/storage/ProposalActionsSection.test.jsx b/web/src/components/storage/ProposalActionsSection.test.jsx deleted file mode 100644 index 9864391d0d..0000000000 --- a/web/src/components/storage/ProposalActionsSection.test.jsx +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright (c) [2022-2023] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -import React from "react"; -import { screen, within, waitForElementToBeRemoved } from "@testing-library/react"; -import { plainRender } from "~/test-utils"; -import { ProposalActionsSection } from "~/components/storage"; - -jest.mock("@patternfly/react-core", () => { - const original = jest.requireActual("@patternfly/react-core"); - - return { - ...original, - Skeleton: () =>
PFSkeleton
- }; -}); - -const actions = [ - { text: 'Create GPT on /dev/vdc', subvol: false, delete: false }, - { text: 'Create partition /dev/vdc1 (8.00 MiB) as BIOS Boot Partition', subvol: false, delete: false }, - { text: 'Create encrypted partition /dev/vdc2 (29.99 GiB) as LVM physical volume', subvol: false, delete: false }, - { text: 'Create volume group system0 (29.98 GiB) with /dev/mapper/cr_vdc2 (29.99 GiB)', subvol: false, delete: false }, - { text: 'Create LVM logical volume /dev/system0/root (20.00 GiB) on volume group system0 for / with btrfs', subvol: false, delete: false }, -]; - -const subvolumeActions = [ - { text: 'Create subvolume @ on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/var on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/usr/local on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/srv on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/root on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/opt on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/home on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/boot/writable on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/boot/grub2/x86_64-efi on /dev/system0/root (20.00 GiB)', subvol: true, delete: false }, - { text: 'Create subvolume @/boot/grub2/i386-pc on /dev/system0/root (20.00 GiB)', subvol: true, delete: false } -]; - -const destructiveAction = { text: 'Delete ext4 on /dev/vdc', subvol: false, delete: true }; - -it("renders skeleton while loading", () => { - plainRender(); - - screen.getAllByText(/PFSkeleton/); -}); - -it("renders nothing when there is no actions", () => { - plainRender(); - - expect(screen.queryAllByText(/Delete/)).toEqual([]); - expect(screen.queryAllByText(/Create/)).toEqual([]); - expect(screen.queryAllByText(/Show/)).toEqual([]); -}); - -describe("when there are actions", () => { - it("renders an explanatory text", () => { - plainRender(); - - screen.getByText(/Actions to create/); - }); - - it("renders the list of actions", () => { - plainRender(); - - const actionsList = screen.getByRole("list"); - const actionsListItems = within(actionsList).getAllByRole("listitem"); - expect(actionsListItems.map(i => i.textContent)).toEqual(actions.map(a => a.text)); - }); - - describe("when there is a destructive action", () => { - it("emphasizes the action", () => { - plainRender(); - - // https://stackoverflow.com/a/63080940 - const actionItems = screen.getAllByRole("listitem"); - const destructiveActionItem = actionItems.find(item => item.textContent === destructiveAction.text); - - expect(destructiveActionItem).toHaveClass("proposal-action--delete"); - }); - }); - - describe("when there are subvolume actions", () => { - it("does not render the subvolume actions", () => { - plainRender(); - - // For now, we know that there are two lists and the subvolume list is the second one. - // The test could be simplified once we have aria-descriptions for the lists. - const [genericList, subvolList] = screen.getAllByRole("list", { hidden: true }); - expect(genericList).not.toBeNull(); - expect(subvolList).not.toBeNull(); - const subvolItems = within(subvolList).queryAllByRole("listitem"); - expect(subvolItems).toEqual([]); - }); - - it("renders the subvolume actions after clicking on 'show subvolumes'", async () => { - const { user } = plainRender( - - ); - - const link = screen.getByText(/Show.*subvolume actions/); - - expect(screen.getAllByRole("list").length).toEqual(1); - - await user.click(link); - - waitForElementToBeRemoved(link); - screen.getByText(/Hide.*subvolume actions/); - - // For now, we know that there are two lists and the subvolume list is the second one. - // The test could be simplified once we have aria-descriptions for the lists. - const [, subvolList] = screen.getAllByRole("list"); - const subvolItems = within(subvolList).getAllByRole("listitem"); - - expect(subvolItems.map(i => i.textContent)).toEqual(subvolumeActions.map(a => a.text)); - }); - }); -}); diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index d33ab99cec..5ca7970f48 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -21,12 +21,13 @@ // @ts-check -import React from "react"; -import { Alert, Skeleton } from "@patternfly/react-core"; +import React, { useState } from "react"; +import { Alert, Button, Skeleton } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; import { deviceSize } from "~/components/storage/utils"; import { If, Section, Tag, TreeTable } from "~/components/core"; +import { ProposalActionsDialog } from "~/components/storage"; /** * @todo Create a component for rendering a customized skeleton @@ -198,8 +199,29 @@ export default function ProposalResultSection({ errors = [], isLoading = false }) { + const [showActions, setShowActions] = useState(false); + if (isLoading) errors = []; + const openActions = () => setShowActions(true); + const closeActions = () => setShowActions(false); + + const ActionsInfo = () => { + return ( + <> +

+ + {_("to create these file systems and to ensure the new system boots.")} +

+ + + ); + }; + return (
+ } /> diff --git a/web/src/components/storage/index.js b/web/src/components/storage/index.js index ca43995771..063d513dab 100644 --- a/web/src/components/storage/index.js +++ b/web/src/components/storage/index.js @@ -24,8 +24,8 @@ export { default as ProposalPageMenu } from "./ProposalPageMenu"; export { default as ProposalSettingsSection } from "./ProposalSettingsSection"; export { default as ProposalSpacePolicyField } from "./ProposalSpacePolicyField"; export { default as ProposalDeviceSection } from "./ProposalDeviceSection"; -export { default as ProposalActionsSection } from "./ProposalActionsSection"; export { default as ProposalTransactionalInfo } from "./ProposalTransactionalInfo"; +export { default as ProposalActionsDialog } from "./ProposalActionsDialog"; export { default as ProposalResultSection } from "./ProposalResultSection"; export { default as ProposalVolumes } from "./ProposalVolumes"; export { default as DASDPage } from "./DASDPage"; From 62b099aeaf4af9d1715614c0aab7ea5304342856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 13:23:11 +0000 Subject: [PATCH 05/39] web: adapt storage/ProposalActionsDialog tests --- .../storage/ProposalActionsDialog.test.jsx | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/web/src/components/storage/ProposalActionsDialog.test.jsx b/web/src/components/storage/ProposalActionsDialog.test.jsx index 41276c83ea..93dfa68271 100644 --- a/web/src/components/storage/ProposalActionsDialog.test.jsx +++ b/web/src/components/storage/ProposalActionsDialog.test.jsx @@ -47,38 +47,53 @@ const subvolumeActions = [ const destructiveAction = { text: 'Delete ext4 on /dev/vdc', subvol: false, delete: true }; +const onCloseFn = jest.fn(); + it("renders nothing by default", () => { - const { container } = plainRender(); + const { container } = plainRender(); expect(container).toBeEmptyDOMElement(); }); it("renders nothing when isOpen=false", () => { - const { container } = plainRender(); + const { container } = plainRender( + + ); expect(container).toBeEmptyDOMElement(); }); describe("when isOpen", () => { it("renders nothing if there are no actions", () => { - plainRender(); + plainRender(); expect(screen.queryAllByText(/Delete/)).toEqual([]); expect(screen.queryAllByText(/Create/)).toEqual([]); expect(screen.queryAllByText(/Show/)).toEqual([]); }); - describe.skip("and there are actions", () => { + describe("and there are actions", () => { it("renders a dialog with the list of actions", () => { - plainRender(); + plainRender(); - const dialog = screen.getByRole("dialog", { name: "Proposal Actions"}); + const dialog = screen.getByRole("dialog", { name: "Planned Actions"}); const actionsList = screen.getByRole("list"); const actionsListItems = within(actionsList).getAllByRole("listitem"); expect(actionsListItems.map(i => i.textContent)).toEqual(actions.map(a => a.text)); }); + it("triggers the onClose callback when user clicks the Close button", async () => { + const { user } = plainRender(); + const closeButton = screen.getByRole("button", { name: "Close" }); + + await user.click(closeButton); + + expect(onCloseFn).toHaveBeenCalled(); + }); + describe("when there is a destructive action", () => { it("emphasizes the action", () => { - plainRender(); + plainRender( + + ); // https://stackoverflow.com/a/63080940 const actionItems = screen.getAllByRole("listitem"); @@ -90,7 +105,9 @@ describe("when isOpen", () => { describe("when there are subvolume actions", () => { it("does not render the subvolume actions", () => { - plainRender(); + plainRender( + + ); // For now, we know that there are two lists and the subvolume list is the second one. // The test could be simplified once we have aria-descriptions for the lists. @@ -103,7 +120,7 @@ describe("when isOpen", () => { it("renders the subvolume actions after clicking on 'show subvolumes'", async () => { const { user } = plainRender( - + ); const link = screen.getByText(/Show.*subvolume actions/); From 5fe6044a986d7052c359a1c64e96939f1ee3b70c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 13:31:53 +0000 Subject: [PATCH 06/39] web: Undo undesired changes from a git rebase --- .../components/storage/ProposalPage.test.jsx | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 2822f6b75a..3ab0ea09ec 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -87,12 +87,6 @@ const storageMock = { defaultVolume: jest.fn(mountPath => Promise.resolve({ mountPath })), calculate: jest.fn().mockResolvedValue(0), }, - system: { - getDevices: jest.fn().mockResolvedValue([vda, vdb]), - }, - staging: { - getDevices: jest.fn().mockResolvedValue([vda, vdb]), - }, system: { getDevices: jest.fn().mockResolvedValue([vda, vdb]) }, @@ -112,18 +106,18 @@ beforeEach(() => { createClient.mockImplementation(() => ({ storage })); }); -it.skip("probes storage if the storage devices are deprecated", async () => { +it("probes storage if the storage devices are deprecated", async () => { storage.isDeprecated = jest.fn().mockResolvedValue(true); installerRender(); await waitFor(() => expect(storage.probe).toHaveBeenCalled()); }); -it.skip("does not probe storage if the storage devices are not deprecated", async () => { +it("does not probe storage if the storage devices are not deprecated", async () => { installerRender(); await waitFor(() => expect(storage.probe).not.toHaveBeenCalled()); }); -it.skip("loads the proposal data", async () => { +it("loads the proposal data", async () => { storage.proposal.getResult = jest.fn().mockResolvedValue( { settings: { bootDevice: vda.name } } ); @@ -136,7 +130,7 @@ it.skip("loads the proposal data", async () => { await screen.findByText(/\/dev\/vda/); }); -it.skip("renders the device, settings, find space and result sections", async () => { +it("renders the device, settings, find space and result sections", async () => { installerRender(); await screen.findByText(/Device/); @@ -144,7 +138,7 @@ it.skip("renders the device, settings, find space and result sections", async () await screen.findByText(/Result/); }); -describe.skip("when the storage devices become deprecated", () => { +describe("when the storage devices become deprecated", () => { it("probes storage", async () => { const [mockFunction, callbacks] = createCallbackMock(); storage.onDeprecate = mockFunction; @@ -178,7 +172,7 @@ describe.skip("when the storage devices become deprecated", () => { }); }); -describe.skip("when there is no proposal yet", () => { +describe("when there is no proposal yet", () => { beforeEach(() => { storage.proposal.getResult = jest.fn().mockResolvedValue(undefined); }); @@ -208,7 +202,7 @@ describe.skip("when there is no proposal yet", () => { }); }); -describe.skip("when there is a proposal", () => { +describe("when there is a proposal", () => { beforeEach(() => { storage.proposal.getResult = jest.fn().mockResolvedValue( { settings: { bootDevice: vda.name } } From b3462d8d3e41fbf241d19b5ace3acee3712ccb47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 14:03:58 +0000 Subject: [PATCH 07/39] web: Fix storage/ProposalResultSection To display nothing but error message when the proposal couldn't be calculated. --- .../storage/ProposalResultSection.jsx | 58 ++++++++++--------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 5ca7970f48..20497a0c59 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -32,14 +32,12 @@ import { ProposalActionsDialog } from "~/components/storage"; /** * @todo Create a component for rendering a customized skeleton */ -const ActionsSkeleton = () => { +const ResultSkeleton = () => { return ( <> - - ); }; @@ -74,6 +72,29 @@ const Warning = ({ deleteActions }) => { ); }; +/** + * Renders needed UI elements to allow users check all planned actions + * + * @param {object} props + * @param {object[]} props.actions + */ +const ActionsInfo = ({ actions }) => { + const [showActions, setShowActions] = useState(false); + + const onOpen = () => setShowActions(true); + const onClose = () => setShowActions(false); + + return ( + <> +

+ + {_("to create these file systems and to ensure the new system boots.")} +

+ + + ); +}; + /** * Renders a TreeTable rendering the devices proposal result * @@ -199,25 +220,16 @@ export default function ProposalResultSection({ errors = [], isLoading = false }) { - const [showActions, setShowActions] = useState(false); - if (isLoading) errors = []; - const openActions = () => setShowActions(true); - const closeActions = () => setShowActions(false); + const SectionContent = () => { + if (errors.length) return; - const ActionsInfo = () => { return ( <> -

- - {_("to create these file systems and to ensure the new system boots.")} -

- + + + ); }; @@ -231,17 +243,7 @@ export default function ProposalResultSection({ id="storage-result" errors={errors} > - } - else={ - <> - - - - - } - /> + } else={} />
); } From d9ad7f2efe91cfd37c5eb7b7438b15c39b81579a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 14:04:50 +0000 Subject: [PATCH 08/39] web: remove not needed hack The complaint is no longer reproducible. --- web/src/components/core/TreeTable.jsx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/web/src/components/core/TreeTable.jsx b/web/src/components/core/TreeTable.jsx index 1e09dd68ce..5f7612adb8 100644 --- a/web/src/components/core/TreeTable.jsx +++ b/web/src/components/core/TreeTable.jsx @@ -71,13 +71,7 @@ export default function TreeTable({ if (cIdx === 0) props.treeRow = treeRow; return ( - // FIXME: using an array below because for some reason React is - // complaining about - // Objects are not valid as a React child (found: object with keys {title}). If you meant to render a collection of children, use an array instead. - // when rendering the first column using the treeRow prop - // Checking the PF/Table code might help to understand what is going on - // there - {[c.content(item)]} + {c.content(item)} ); }); }; From 82e6773affe545474642410b379fabe8377d34cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Mar 2024 14:05:44 +0000 Subject: [PATCH 09/39] web: please ESlint --- web/src/components/storage/ProposalActionsDialog.test.jsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/src/components/storage/ProposalActionsDialog.test.jsx b/web/src/components/storage/ProposalActionsDialog.test.jsx index 93dfa68271..cde3a95c51 100644 --- a/web/src/components/storage/ProposalActionsDialog.test.jsx +++ b/web/src/components/storage/ProposalActionsDialog.test.jsx @@ -56,7 +56,7 @@ it("renders nothing by default", () => { it("renders nothing when isOpen=false", () => { const { container } = plainRender( - + ); expect(container).toBeEmptyDOMElement(); }); @@ -74,8 +74,8 @@ describe("when isOpen", () => { it("renders a dialog with the list of actions", () => { plainRender(); - const dialog = screen.getByRole("dialog", { name: "Planned Actions"}); - const actionsList = screen.getByRole("list"); + const dialog = screen.getByRole("dialog", { name: "Planned Actions" }); + const actionsList = within(dialog).getByRole("list"); const actionsListItems = within(actionsList).getAllByRole("listitem"); expect(actionsListItems.map(i => i.textContent)).toEqual(actions.map(a => a.text)); }); @@ -92,7 +92,7 @@ describe("when isOpen", () => { describe("when there is a destructive action", () => { it("emphasizes the action", () => { plainRender( - + ); // https://stackoverflow.com/a/63080940 From 66ad402707f2e28e6b27e3a279e055f255bbafac 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, 11 Mar 2024 17:06:34 +0000 Subject: [PATCH 10/39] web: Fix state initialization --- .../components/storage/ProposalDeviceSection.jsx | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/web/src/components/storage/ProposalDeviceSection.jsx b/web/src/components/storage/ProposalDeviceSection.jsx index df37eaf304..a39f4e333a 100644 --- a/web/src/components/storage/ProposalDeviceSection.jsx +++ b/web/src/components/storage/ProposalDeviceSection.jsx @@ -19,7 +19,7 @@ * find current contact information at www.suse.com. */ -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { Button, Form, @@ -101,7 +101,7 @@ const InstallationDeviceField = ({ isLoading = false, onChange = noop }) => { - const [device, setDevice] = useState(devices.find(d => d.name === current)); + const [device, setDevice] = useState(); const [isFormOpen, setIsFormOpen] = useState(false); const openForm = () => setIsFormOpen(true); @@ -114,6 +114,10 @@ const InstallationDeviceField = ({ onChange(selectedDevice); }; + useEffect(() => { + setDevice(devices.find(d => d.name === current)); + }, [current, devices, setDevice]); + /** * Renders a button that allows changing selected device * @@ -292,7 +296,7 @@ const LVMField = ({ isLoading = false, onChange: onChangeProp = noop }) => { - const [isChecked, setIsChecked] = useState(isCheckedProp); + const [isChecked, setIsChecked] = useState(); const [isFormOpen, setIsFormOpen] = useState(false); const [isFormValid, setIsFormValid] = useState(true); @@ -312,6 +316,10 @@ const LVMField = ({ onChangeProp({ vgDevices }); }; + useEffect(() => { + setIsChecked(isCheckedProp); + }, [isCheckedProp, setIsChecked]); + const description = _("Configuration of the system volume group. All the file systems will be \ created in a logical volume of the system volume group."); From fb7d75288903537e5747dd8f182bfe48bd398c0c 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, 11 Mar 2024 17:07:26 +0000 Subject: [PATCH 11/39] web: Improve content of result section --- .../storage/ProposalResultSection.jsx | 74 ++++++++++++++----- 1 file changed, 57 insertions(+), 17 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 20497a0c59..494530af1e 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -46,18 +46,30 @@ const ResultSkeleton = () => { * Returns the delete actions from given list * * @param {object[]} actions - * @returns {object[]} + * @param {object[]} devices + * @returns {string[]} */ -const deleteActions = (actions) => actions.filter(a => a.delete); +const deleteActions = (actions, devices) => { + const actionText = (action) => { + const device = devices.find(d => d.sid === action.device); + + if (device && device.systems.length > 0) + return sprintf(_("%s which contains %s"), action.text, device.systems.join(", ")); + + return action.text; + }; + + return actions.filter(a => a.delete).map(a => actionText(a)); +}; /** * Renders a warning alert if there are delete actions * * @param {object} props - * @param {object} props.deleteActions + * @param {string[]} props.content */ -const Warning = ({ deleteActions }) => { - const count = deleteActions.length; +const Warning = ({ content }) => { + const count = content.length; if (count === 0) return; @@ -66,7 +78,7 @@ const Warning = ({ deleteActions }) => { return (
    - { deleteActions.map(a =>
  • {a.text}
    • )} + { content.map((action, i) =>
  • ) }
); @@ -135,22 +147,37 @@ const DevicesTreeTable = ({ settings, devices }) => { const renderNewLabel = (device) => { if (!device.sid) return; - if (systemDevices.find(d => d.sid === device.sid)) return; - return ( - {_("New")} - ); + const systemDevice = systemDevices.find(d => d.sid === device.sid); + + if (!systemDevice || systemDevice.fileystem?.sid !== device.filesystem?.sid) + return {_("New")}; }; // FIXME: add the logic to render it conditionally - const renderShrankLabel = (item) => { - if (item) return {_("Shrank")}; + const renderResizedLabel = (item) => { + const systemDevice = systemDevices.find(d => d.sid === item.sid); + const stagingDevice = stagingDevices.find(d => d.sid === item.sid); + + if (!systemDevice || !stagingDevice) return; + + const amount = systemDevice.size - stagingDevice.size; + + if (amount > 0) + return {sprintf(_("Resized %s"), deviceSize(amount))}; }; 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 (
- {item.sid && item.name} + {name}
); }; @@ -160,13 +187,26 @@ const DevicesTreeTable = ({ settings, devices }) => { if (label) return {label}; }; + const renderPTableType = (item) => { + // TODO: Create a map for partition table types and use an here. + const pType = item.partitionTable?.type; + if (pType) return {pType.toUpperCase()}; + }; + const renderDetails = (item) => { - const description = item.sid ? item.description : _("Unused space"); + const deviceDetails = (device) => { + if (!item.sid) + return _("Unused space"); + if (!device.partitionTable && device.systems?.length > 0) + return device.systems.join(", "); + + return device.description; + }; return ( <>
{ renderNewLabel(item) }
-
{description} {renderFilesystemLabel(item)}
+
{deviceDetails(item)} {renderFilesystemLabel(item)} {renderPTableType(item)}
); }; @@ -174,7 +214,7 @@ const DevicesTreeTable = ({ settings, devices }) => { const renderSize = (item) => { return (
- { renderShrankLabel(item) } + { renderResizedLabel(item) } { deviceSize(item.size) }
); @@ -227,7 +267,7 @@ export default function ProposalResultSection({ return ( <> - + From 630cd2cc93e7258b5ab9f8471a48ba7905003e64 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, 12 Mar 2024 14:14:31 +0000 Subject: [PATCH 12/39] web: Add DevicesManager - Adapt ProposalResultSection to use DevicesManager. - Unit tests still pending. --- web/src/components/storage/DevicesManager.js | 181 +++++++++++++++++ .../storage/ProposalResultSection.jsx | 192 +++++++++--------- 2 files changed, 275 insertions(+), 98 deletions(-) create mode 100644 web/src/components/storage/DevicesManager.js diff --git a/web/src/components/storage/DevicesManager.js b/web/src/components/storage/DevicesManager.js new file mode 100644 index 0000000000..a773fcf9f8 --- /dev/null +++ b/web/src/components/storage/DevicesManager.js @@ -0,0 +1,181 @@ +/* + * 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. + */ + +/** + * @typedef {import ("~/clients/storage").StorageDevice} StorageDevice + * @typedef {import ("~/clients/storage").PartitionSlot} PartitionSlot + */ + +/** + * Class for managing storage devices. + */ +export default class DevicesManager { + /** + * @param {StorageDevice[]} system - Devices representing the current state of the system. + * @param {StorageDevice[]} staging - Devices representing the target state of the system. + */ + constructor(system, staging) { + this.system = system; + this.staging = staging; + } + + /** + * System device with the given SID. + * + * @param {Number} sid + * @returns {StorageDevice|undefined} + */ + systemDevice(sid) { + return this.#device(sid, this.system); + } + + /** + * Staging device with the given SID. + * + * @param {Number} sid + * @returns {StorageDevice|undefined} + */ + stagingDevice(sid) { + return this.#device(sid, this.staging); + } + + /** + * Whether the given device exists in system. + * + * @param {StorageDevice} device + * @returns {Boolean} + */ + existInSystem(device) { + return this.#exist(device, this.system); + } + + /** + * Whether the given device exists in staging. + * + * @param {StorageDevice} device + * @returns {Boolean} + */ + existInStaging(device) { + return this.#exist(device, this.staging); + } + + /** + * LVM volume groups that are going to be created. + * + * @returns {StorageDevice[]} + */ + newLvmVgs() { + return this.#stagingLvmVgs() + .filter(v => !this.existInSystem(v)); + } + + /** + * LVM volume groups that are going to be reused. + * + * @returns {StorageDevice[]} + */ + reusedLvmVgs() { + return this.#stagingLvmVgs() + .filter(v => this.existInSystem(v)) + .filter(v => v.logicalVolumes.find(l => this.hasNewFilesystem(l))); + } + + /** + * Whether the given device is going to be formatted. + * + * @param {StorageDevice} device + * @returns {Boolean} + */ + hasNewFilesystem(device) { + const systemDevice = this.systemDevice(device.sid); + const stagingDevice = this.stagingDevice(device.sid); + + if (!systemDevice || !stagingDevice) return false; + + const systemFilesystemSID = systemDevice.filesystem?.sid; + const stagingFilesystemSID = stagingDevice.filesystem?.sid; + + return systemFilesystemSID !== stagingFilesystemSID; + } + + /** + * Sorted list of children devices (i.e., partitions and unused slots or logical volumes). + * + * @param {StorageDevice} device + * @returns {(StorageDevice|PartitionSlot)[]} + */ + children(device) { + if (device.partitionTable) return this.#partitionTableChildren(device.partitionTable); + if (device.type === "lvmVg") return this.#lvmVgChildren(device); + return []; + } + + /** + * Whether the given device is going to be shrunk. + * + * @param {StorageDevice} device + * @returns {Boolean} + */ + isResized(device) { + return this.resizeSize(device) > 0; + } + + /** + * Amount of bytes the given device is going to be shrunk. + * + * @param {StorageDevice} device + * @returns {Number} + */ + resizeSize(device) { + const systemDevice = this.systemDevice(device.sid); + const stagingDevice = this.stagingDevice(device.sid); + + if (!systemDevice || !stagingDevice) return 0; + + return systemDevice.size - stagingDevice.size; + } + + #device(sid, source) { + return source.find(d => d.sid === sid); + } + + #exist(device, source) { + return this.#device(device.sid, source) !== undefined; + } + + #stagingLvmVgs() { + return this.#lvmVgs(this.staging); + } + + #lvmVgs(source) { + return source.filter(d => d.type === "lvmVg"); + } + + #partitionTableChildren(partitionTable) { + const { partitions, unusedSlots } = partitionTable; + const children = partitions.concat(unusedSlots); + return children.sort((a, b) => a.start < b.start ? -1 : 1); + } + + #lvmVgChildren(lvmVg) { + return lvmVg.logicalVolumes.sort((a, b) => a.name < b.name ? -1 : 1); + } +} diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 494530af1e..efbd5d03e7 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -26,32 +26,20 @@ import { Alert, Button, Skeleton } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; import { deviceSize } from "~/components/storage/utils"; +import DevicesManager from "~/components/storage/DevicesManager"; import { If, Section, Tag, TreeTable } from "~/components/core"; import { ProposalActionsDialog } from "~/components/storage"; -/** - * @todo Create a component for rendering a customized skeleton - */ -const ResultSkeleton = () => { - return ( - <> - - - - - ); -}; - /** * Returns the delete actions from given list * * @param {object[]} actions - * @param {object[]} devices + * @param {object} devicesManager * @returns {string[]} */ -const deleteActions = (actions, devices) => { +const deleteActions = (actions, devicesManager) => { const actionText = (action) => { - const device = devices.find(d => d.sid === action.device); + const device = devicesManager.systemDevice(action.device); if (device && device.systems.length > 0) return sprintf(_("%s which contains %s"), action.text, device.systems.join(", ")); @@ -59,7 +47,7 @@ const deleteActions = (actions, devices) => { return action.text; }; - return actions.filter(a => a.delete).map(a => actionText(a)); + return actions.filter(a => a.delete).map(actionText); }; /** @@ -114,62 +102,25 @@ const ActionsInfo = ({ actions }) => { * * @param {object} props * @param {object} props.settings - * @param {object} props.devices + * @param {object} props.devicesManager */ -const DevicesTreeTable = ({ settings, devices }) => { - const { system: systemDevices = [], staging: stagingDevices = [] } = devices; - - const sids = settings.installationDevices.map(d => d.sid); - const installationDevices = stagingDevices.filter(d => sids.includes(d.sid)); - const lvmVgs = stagingDevices.filter(d => d.logicalVolumes?.find(l => l.filesystem?.mountPath !== undefined)); - - const items = installationDevices.concat(lvmVgs); - - // FIXME: Move to a utils - const childrenFromPartitionTable = ({ partitionTable }) => { - const { partitions, unusedSlots } = partitionTable; - const children = partitions.concat(unusedSlots); - - return children.sort((a, b) => a.start < b.start ? -1 : 1); - }; - - // FIXME: Move to a utils - const childrenFromLvmVg = (vg) => { - return vg.logicalVolumes.sort((a, b) => a.name < b.name ? -1 : 1); - }; - - // FIXME: Move to a utils - const childrenFor = (device) => { - if (device.partitionTable) return childrenFromPartitionTable(device); - if (device.type === "lvmVg") return childrenFromLvmVg(device); - return []; - }; - - const renderNewLabel = (device) => { - if (!device.sid) return; - - const systemDevice = systemDevices.find(d => d.sid === device.sid); - - if (!systemDevice || systemDevice.fileystem?.sid !== device.filesystem?.sid) - return {_("New")}; - }; - - // FIXME: add the logic to render it conditionally - const renderResizedLabel = (item) => { - const systemDevice = systemDevices.find(d => d.sid === item.sid); - const stagingDevice = stagingDevices.find(d => d.sid === item.sid); - - if (!systemDevice || !stagingDevice) return; - - const amount = systemDevice.size - stagingDevice.size; +const DevicesTreeTable = ({ settings, devicesManager }) => { + const usedDevices = () => { + const selectedDiskDevices = () => { + return settings.installationDevices + .map(d => devicesManager.stagingDevice(d.sid)) + .filter(Boolean); + }; - if (amount > 0) - return {sprintf(_("Resized %s"), deviceSize(amount))}; + return selectedDiskDevices() + .concat(devicesManager.newLvmVgs()) + .concat(devicesManager.reusedLvmVgs()); }; 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. + // 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)) @@ -182,6 +133,23 @@ const DevicesTreeTable = ({ settings, devices }) => { ); }; + 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 renderFilesystemLabel = (item) => { const label = item.filesystem?.label; if (label) return {label}; @@ -189,33 +157,34 @@ const DevicesTreeTable = ({ settings, devices }) => { const renderPTableType = (item) => { // TODO: Create a map for partition table types and use an here. - const pType = item.partitionTable?.type; - if (pType) return {pType.toUpperCase()}; + const type = item.partitionTable?.type; + if (type) return {type.toUpperCase()}; }; const renderDetails = (item) => { - const deviceDetails = (device) => { - if (!item.sid) - return _("Unused space"); - if (!device.partitionTable && device.systems?.length > 0) - return device.systems.join(", "); - - return device.description; - }; - return ( <> -
{ renderNewLabel(item) }
-
{deviceDetails(item)} {renderFilesystemLabel(item)} {renderPTableType(item)}
+
{renderNewLabel(item)}
+
{renderContent(item)} {renderFilesystemLabel(item)} {renderPTableType(item)}
); }; + const renderResizedLabel = (item) => { + if (!item.sid || !devicesManager.isResized(item)) return; + + return ( + + {sprintf(_("Resized %s"), deviceSize(devicesManager.resizeSize(item)))} + + ); + }; + const renderSize = (item) => { return (
- { renderResizedLabel(item) } - { deviceSize(item.size) } + {renderResizedLabel(item)} + {deviceSize(item.size)}
); }; @@ -230,8 +199,8 @@ const DevicesTreeTable = ({ settings, devices }) => { { title: _("Details"), content: renderDetails, classNames: "details-column" }, { title: _("Size"), content: renderSize, classNames: "sizes-column" } ]} - items={items} - itemChildren={childrenFor} + items={usedDevices()} + itemChildren={d => devicesManager.children(d)} rowClassNames={(item) => { if (!item.sid) return "dimmed-row"; }} @@ -240,6 +209,34 @@ const DevicesTreeTable = ({ settings, devices }) => { ); }; +/** + * @todo Create a component for rendering a customized skeleton + */ +const ResultSkeleton = () => { + return ( + <> + + + + + ); +}; + +const SectionContent = ({ actions, settings, devices, errors }) => { + if (errors.length) return; + + const { system = [], staging = [] } = devices; + const devicesManager = new DevicesManager(system, staging); + + return ( + <> + + + + + ); +}; + /** * Section holding the proposal result and actions to perform in the system * @component @@ -249,7 +246,7 @@ const DevicesTreeTable = ({ settings, devices }) => { * @param {object} props * @param {object[]} [props.actions=[]] * @param {object[]} [props.settings=[]] - * @param {object[]} [props.devices=[]] + * @param {object} [props.devices={}] * @param {import("~/client/mixins").ValidationError[]} props.errors - Validation errors * @param {boolean} [props.isLoading=false] - Whether the section content should be rendered as loading */ @@ -262,18 +259,6 @@ export default function ProposalResultSection({ }) { if (isLoading) errors = []; - const SectionContent = () => { - if (errors.length) return; - - return ( - <> - - - - - ); - }; - return (
- } else={} /> + } + else={ + + } + />
); } From fa89d9d982e85ce938ebd8b0222cbb0d76573857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 12 Mar 2024 15:29:41 +0000 Subject: [PATCH 13/39] web: add tests for storage proposal result section --- web/.eslintignore | 1 + web/cspell.json | 3 +- .../storage/ProposalResultSection.test.jsx | 100 +- .../storage/test-data/full-result-example.js | 1449 +++++++++++++++++ 4 files changed, 1546 insertions(+), 7 deletions(-) create mode 100644 web/src/components/storage/test-data/full-result-example.js diff --git a/web/.eslintignore b/web/.eslintignore index 8faa0e3fd2..fb9357ef5e 100644 --- a/web/.eslintignore +++ b/web/.eslintignore @@ -1,2 +1,3 @@ node_modules/* src/lib/* +src/**/test-data/* diff --git a/web/cspell.json b/web/cspell.json index 9c7619f4a9..3a2944fcda 100644 --- a/web/cspell.json +++ b/web/cspell.json @@ -5,7 +5,8 @@ "ignorePaths": [ "src/lib/cockpit.js", "src/lib/cockpit-po-plugin.js", - "src/manifest.json" + "src/manifest.json", + "src/**/test-data/*" ], "import": [ "@cspell/dict-css/cspell-ext.json", diff --git a/web/src/components/storage/ProposalResultSection.test.jsx b/web/src/components/storage/ProposalResultSection.test.jsx index 546c7dbad4..663e178634 100644 --- a/web/src/components/storage/ProposalResultSection.test.jsx +++ b/web/src/components/storage/ProposalResultSection.test.jsx @@ -19,11 +19,99 @@ * find current contact information at www.suse.com. */ +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { ProposalResultSection } from "~/components/storage"; +import { settings, devices, actions } from "./test-data/full-result-example"; + +const errorMessage = "Something went wrong, proposal not possible"; +const errors = [{ severity: 0, message: errorMessage }]; +const defaultProps = { settings, actions, devices }; + describe("ProposalResultSection", () => { - it.todo("check that it shows a warning when there are deletion actions"); - it.todo("check that it shows a tree table with the proposal result"); - it.todo("check that 'New' label is included when proceed"); - it.todo("check that 'Shrank' label is included when proceed"); - it.todo("check that filesystem label is shown if present"); - it.todo("check that there is a working link for checking all actions"); + describe("when there are errors (proposal was not possible)", () => { + it("renders given errors", () => { + plainRender(); + expect(screen.queryByText(errorMessage)).toBeInTheDocument(); + }); + + it("does not render an warning for delete actions", () => { + plainRender(); + expect(screen.queryByText(/Warning alert:/)).toBeNull(); + }); + + it("does not render a treegrid node", () => { + plainRender(); + expect(screen.queryByRole("treegrid")).toBeNull(); + }); + + it("does not render the link for opening the planned actions dialog", () => { + plainRender(); + expect(screen.queryByRole("button", { name: /planned actions/ })).toBeNull(); + }); + }); + + describe("when there are no errors (proposal was possible)", () => { + it("does not render a warning when there are not delete actions", () => { + const props = { + ...defaultProps, + actions: defaultProps.actions.filter(a => !a.delete) + }; + + plainRender(); + expect(screen.queryByText(/Warning alert:/)).toBeNull(); + }); + + it("renders a warning when when there are delete", () => { + plainRender(); + screen.getByText(/Warning alert:/); + }); + + it("renders a treegrid including all relevant information about final result", () => { + plainRender(); + const treegrid = screen.getByRole("treegrid"); + /** + * Expected rows for full-result-example + * -------------------------------------------------- + * "/dev/vdc Disk GPT 30 GiB" + * "vdc1 New BIOS Boot Partition 8 MiB" + * "vdc3 swap New Swap Partition 1.5 GiB" + * "Unused space 3.49 GiB" + * "vdc2 openSUSE Leap 15.2, Fedora 10.30 5 GiB" + * "Unused space 1 GiB" + * "vdc4 Linux Resized 514 MiB 1.5 GiB" + * "vdc5 / New Btrfs Partition 17.5 GiB" + * + * Device Mount point Details Size + * ------------------------------------------------------------------------- + * /dev/vdc Disk GPT 30 GiB + * vdc1 New BIOS Boot Partition 8 MiB + * vdc3 swap New Swap Partition 1.5 GiB + * Unused space 3.49 GiB + * vdc2 openSUSE Leap 15.2, Fedora 10.30 5 GiB + * Unused space 1 GiB + * vdc4 Linux Resized 514 MiB 1.5 GiB + * vdc5 / New Btrfs Partition 17.5 GiB + * ------------------------------------------------------------------------- + */ + within(treegrid).getByRole("row", { name: "/dev/vdc Disk GPT 30 GiB" }); + within(treegrid).getByRole("row", { name: "vdc1 New BIOS Boot Partition 8 MiB" }); + within(treegrid).getByRole("row", { name: "vdc3 swap New Swap Partition 1.5 GiB" }); + within(treegrid).getByRole("row", { name: "Unused space 3.49 GiB" }); + within(treegrid).getByRole("row", { name: "vdc2 openSUSE Leap 15.2, Fedora 10.30 5 GiB" }); + within(treegrid).getByRole("row", { name: "Unused space 1 GiB" }); + within(treegrid).getByRole("row", { name: "vdc4 Linux Resized 514 MiB 1.5 GiB" }); + within(treegrid).getByRole("row", { name: "vdc5 / New Btrfs Partition 17.5 GiB" }); + }); + + it("renders a button for opening the planned actions dialog", async () => { + const { user } = plainRender(); + const button = screen.getByRole("button", { name: /planned actions/ }); + + await user.click(button); + + screen.getByRole("dialog", { name: "Planned Actions" }); + }); + }); }); diff --git a/web/src/components/storage/test-data/full-result-example.js b/web/src/components/storage/test-data/full-result-example.js new file mode 100644 index 0000000000..bcb8932d18 --- /dev/null +++ b/web/src/components/storage/test-data/full-result-example.js @@ -0,0 +1,1449 @@ +// eslint:ignore +// cspell:ignore + +export const settings = { + "bootDevice": "/dev/vdc", + "lvm": false, + "spacePolicy": "custom", + "spaceActions": [ + { + "device": "/dev/vdc3", + "action": "force_delete" + }, + { + "device": "/dev/vdc4", + "action": "resize" + }, + { + "device": "/dev/vdc1", + "action": "force_delete" + } + ], + "systemVGDevices": [], + "encryptionPassword": "", + "encryptionMethod": "luks2", + "volumes": [ + { + "mountPath": "/", + "fsType": "Btrfs", + "minSize": 18790481920, + "autoSize": true, + "snapshots": true, + "transactional": false, + "outline": { + "required": true, + "fsTypes": [ + "Btrfs", + "Ext2", + "Ext3", + "Ext4", + "XFS" + ], + "supportAutoSize": true, + "snapshotsConfigurable": true, + "snapshotsAffectSizes": true, + "sizeRelevantVolumes": [ + "/home" + ] + } + }, + { + "mountPath": "swap", + "fsType": "Swap", + "minSize": 1610612736, + "maxSize": 1610612736, + "autoSize": false, + "snapshots": false, + "transactional": false, + "outline": { + "required": false, + "fsTypes": [ + "Swap" + ], + "supportAutoSize": false, + "snapshotsConfigurable": false, + "snapshotsAffectSizes": false, + "sizeRelevantVolumes": [] + } + } + ], + "installationDevices": [ + { + "sid": 70, + "name": "/dev/vdc", + "description": "Disk", + "isDrive": true, + "type": "disk", + "vendor": "", + "model": "Disk", + "driver": [ + "virtio-pci", + "virtio_blk" + ], + "bus": "None", + "busId": "", + "transport": "unknown", + "sdCard": false, + "dellBOSS": false, + "active": true, + "encrypted": false, + "start": 0, + "size": 32212254720, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0" + ], + "partitionTable": { + "type": "gpt", + "partitions": [ + { + "sid": 78, + "name": "/dev/vdc1", + "description": "Part of md0", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 5368709120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part1" + ], + "isEFI": false, + "component": { + "type": "md_device", + "deviceNames": [ + "/dev/md0" + ] + } + }, + { + "sid": 79, + "name": "/dev/vdc2", + "description": "Part of md0", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 10487808, + "size": 5368709120, + "recoverableSize": 0, + "systems": [ + "openSUSE Leap 15.2", + "Fedora 10.30" + ], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "md_device", + "deviceNames": [ + "/dev/md0" + ] + } + }, + { + "sid": 80, + "name": "/dev/vdc3", + "description": "XFS Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 20973568, + "size": 1073741824, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part3" + ], + "isEFI": false, + "filesystem": { + "sid": 92, + "type": "xfs" + } + }, + { + "sid": 81, + "name": "/dev/vdc4", + "description": "Linux", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 23070720, + "size": 2147483648, + "recoverableSize": 2147483136, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part4" + ], + "isEFI": false + } + ], + "unpartitionedSize": 18253611008, + "unusedSlots": [ + { + "start": 27265024, + "size": 18252545536 + } + ] + } + } + ] +}; + +export const devices = { + "system": [ + { + "sid": 71, + "name": "/dev/vda", + "description": "Disk", + "isDrive": true, + "type": "disk", + "vendor": "", + "model": "Disk", + "driver": [ + "virtio-pci", + "virtio_blk" + ], + "bus": "None", + "busId": "", + "transport": "unknown", + "sdCard": false, + "dellBOSS": false, + "active": true, + "encrypted": false, + "start": 0, + "size": 53687091200, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0" + ], + "partitionTable": { + "type": "gpt", + "partitions": [ + { + "sid": 83, + "name": "/dev/vda1", + "description": "BIOS Boot Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 8388608, + "recoverableSize": 8388096, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part1" + ], + "isEFI": false + }, + { + "sid": 84, + "name": "/dev/vda2", + "description": "PV of system", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 18432, + "size": 53677637120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "physical_volume", + "deviceNames": [ + "/dev/system" + ] + } + } + ], + "unpartitionedSize": 1065472, + "unusedSlots": [] + } + }, + { + "sid": 69, + "name": "/dev/vdb", + "description": "Ext4 Disk", + "isDrive": true, + "type": "disk", + "vendor": "", + "model": "Disk", + "driver": [ + "virtio-pci", + "virtio_blk" + ], + "bus": "None", + "busId": "", + "transport": "unknown", + "sdCard": false, + "dellBOSS": false, + "active": true, + "encrypted": false, + "start": 0, + "size": 5368709120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:08:00.0" + ], + "filesystem": { + "sid": 87, + "type": "ext4" + } + }, + { + "sid": 70, + "name": "/dev/vdc", + "description": "Disk", + "isDrive": true, + "type": "disk", + "vendor": "", + "model": "Disk", + "driver": [ + "virtio-pci", + "virtio_blk" + ], + "bus": "None", + "busId": "", + "transport": "unknown", + "sdCard": false, + "dellBOSS": false, + "active": true, + "encrypted": false, + "start": 0, + "size": 32212254720, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0" + ], + "partitionTable": { + "type": "gpt", + "partitions": [ + { + "sid": 78, + "name": "/dev/vdc1", + "description": "Part of md0", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 5368709120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part1" + ], + "isEFI": false, + "component": { + "type": "md_device", + "deviceNames": [ + "/dev/md0" + ] + } + }, + { + "sid": 79, + "name": "/dev/vdc2", + "description": "Part of md0", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 10487808, + "size": 5368709120, + "recoverableSize": 0, + "systems": [ + "openSUSE Leap 15.2", + "Fedora 10.30" + ], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "md_device", + "deviceNames": [ + "/dev/md0" + ] + } + }, + { + "sid": 80, + "name": "/dev/vdc3", + "description": "XFS Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 20973568, + "size": 1073741824, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part3" + ], + "isEFI": false, + "filesystem": { + "sid": 92, + "type": "xfs" + } + }, + { + "sid": 81, + "name": "/dev/vdc4", + "description": "Linux", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 23070720, + "size": 2147483648, + "recoverableSize": 2147483136, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part4" + ], + "isEFI": false + } + ], + "unpartitionedSize": 18253611008, + "unusedSlots": [ + { + "start": 27265024, + "size": 18252545536 + } + ] + } + }, + { + "sid": 72, + "name": "/dev/md0", + "description": "Disk", + "isDrive": false, + "type": "md", + "level": "raid0", + "uuid": "644aeee1:5f5b946a:4da99758:3f85b3ea", + "devices": [ + { + "sid": 78, + "name": "/dev/vdc1", + "description": "Part of md0", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 5368709120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part1" + ], + "isEFI": false, + "component": { + "type": "md_device", + "deviceNames": [ + "/dev/md0" + ] + } + }, + { + "sid": 79, + "name": "/dev/vdc2", + "description": "Part of md0", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 10487808, + "size": 5368709120, + "recoverableSize": 0, + "systems": [ + "openSUSE Leap 15.2", + "Fedora 10.30" + ], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "md_device", + "deviceNames": [ + "/dev/md0" + ] + } + } + ], + "active": true, + "encrypted": false, + "start": 0, + "size": 10737287168, + "recoverableSize": 0, + "systems": [], + "udevIds": [ + "md-uuid-644aeee1:5f5b946a:4da99758:3f85b3ea" + ], + "udevPaths": [], + "partitionTable": { + "type": "gpt", + "partitions": [ + { + "sid": 86, + "name": "/dev/md0p1", + "description": "Ext4 Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 2147483648, + "recoverableSize": 2040147968, + "systems": [], + "udevIds": [ + "md-uuid-644aeee1:5f5b946a:4da99758:3f85b3ea-part1" + ], + "udevPaths": [], + "isEFI": false, + "filesystem": { + "sid": 93, + "type": "ext4" + } + } + ], + "unpartitionedSize": 8589803520, + "unusedSlots": [ + { + "start": 4196352, + "size": 8588738048 + } + ] + } + }, + { + "sid": 73, + "name": "/dev/system", + "description": "LVM", + "isDrive": false, + "type": "lvmVg", + "size": 53674508288, + "physicalVolumes": [ + { + "sid": 84, + "name": "/dev/vda2", + "description": "PV of system", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 18432, + "size": 53677637120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "physical_volume", + "deviceNames": [ + "/dev/system" + ] + } + } + ], + "logicalVolumes": [ + { + "sid": 75, + "name": "/dev/system/root", + "description": "Ext4 LV", + "isDrive": false, + "type": "lvmLv", + "active": true, + "encrypted": false, + "start": 0, + "size": 51527024640, + "recoverableSize": 30647779328, + "systems": [], + "udevIds": [], + "udevPaths": [], + "filesystem": { + "sid": 88, + "type": "ext4", + "mountPath": "/" + } + }, + { + "sid": 76, + "name": "/dev/system/swap", + "description": "Swap LV", + "isDrive": false, + "type": "lvmLv", + "active": true, + "encrypted": false, + "start": 0, + "size": 2147483648, + "recoverableSize": 2143289344, + "systems": [], + "udevIds": [], + "udevPaths": [], + "filesystem": { + "sid": 90, + "type": "swap", + "mountPath": "swap" + } + } + ] + }, + { + "sid": 75, + "name": "/dev/system/root", + "description": "Ext4 LV", + "isDrive": false, + "type": "lvmLv", + "active": true, + "encrypted": false, + "start": 0, + "size": 51527024640, + "recoverableSize": 30647779328, + "systems": [], + "udevIds": [], + "udevPaths": [], + "filesystem": { + "sid": 88, + "type": "ext4", + "mountPath": "/" + } + }, + { + "sid": 76, + "name": "/dev/system/swap", + "description": "Swap LV", + "isDrive": false, + "type": "lvmLv", + "active": true, + "encrypted": false, + "start": 0, + "size": 2147483648, + "recoverableSize": 2143289344, + "systems": [], + "udevIds": [], + "udevPaths": [], + "filesystem": { + "sid": 90, + "type": "swap", + "mountPath": "swap" + } + }, + { + "sid": 83, + "name": "/dev/vda1", + "description": "BIOS Boot Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 8388608, + "recoverableSize": 8388096, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part1" + ], + "isEFI": false + }, + { + "sid": 84, + "name": "/dev/vda2", + "description": "PV of system", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 18432, + "size": 53677637120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "physical_volume", + "deviceNames": [ + "/dev/system" + ] + } + }, + { + "sid": 78, + "name": "/dev/vdc1", + "description": "Part of md0", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 5368709120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part1" + ], + "isEFI": false, + "component": { + "type": "md_device", + "deviceNames": [ + "/dev/md0" + ] + } + }, + { + "sid": 79, + "name": "/dev/vdc2", + "description": "Part of md0", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 10487808, + "size": 5368709120, + "recoverableSize": 0, + "systems": [ + "openSUSE Leap 15.2", + "Fedora 10.30" + ], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "md_device", + "deviceNames": [ + "/dev/md0" + ] + } + }, + { + "sid": 80, + "name": "/dev/vdc3", + "description": "XFS Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 20973568, + "size": 1073741824, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part3" + ], + "isEFI": false, + "filesystem": { + "sid": 92, + "type": "xfs" + } + }, + { + "sid": 81, + "name": "/dev/vdc4", + "description": "Linux", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 23070720, + "size": 2147483648, + "recoverableSize": 2147483136, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part4" + ], + "isEFI": false + }, + { + "sid": 86, + "name": "/dev/md0p1", + "description": "Ext4 Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 2147483648, + "recoverableSize": 2040147968, + "systems": [], + "udevIds": [ + "md-uuid-644aeee1:5f5b946a:4da99758:3f85b3ea-part1" + ], + "udevPaths": [], + "isEFI": false, + "filesystem": { + "sid": 93, + "type": "ext4" + } + } + ], + "staging": [ + { + "sid": 71, + "name": "/dev/vda", + "description": "Disk", + "isDrive": true, + "type": "disk", + "vendor": "", + "model": "Disk", + "driver": [ + "virtio-pci", + "virtio_blk" + ], + "bus": "None", + "busId": "", + "transport": "unknown", + "sdCard": false, + "dellBOSS": false, + "active": true, + "encrypted": false, + "start": 0, + "size": 53687091200, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0" + ], + "partitionTable": { + "type": "gpt", + "partitions": [ + { + "sid": 83, + "name": "/dev/vda1", + "description": "BIOS Boot Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 8388608, + "recoverableSize": 8388096, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part1" + ], + "isEFI": false + }, + { + "sid": 84, + "name": "/dev/vda2", + "description": "PV of system", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 18432, + "size": 53677637120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "physical_volume", + "deviceNames": [ + "/dev/system" + ] + } + } + ], + "unpartitionedSize": 1065472, + "unusedSlots": [] + } + }, + { + "sid": 69, + "name": "/dev/vdb", + "description": "Ext4 Disk", + "isDrive": true, + "type": "disk", + "vendor": "", + "model": "Disk", + "driver": [ + "virtio-pci", + "virtio_blk" + ], + "bus": "None", + "busId": "", + "transport": "unknown", + "sdCard": false, + "dellBOSS": false, + "active": true, + "encrypted": false, + "start": 0, + "size": 5368709120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:08:00.0" + ], + "filesystem": { + "sid": 87, + "type": "ext4" + } + }, + { + "sid": 70, + "name": "/dev/vdc", + "description": "Disk", + "isDrive": true, + "type": "disk", + "vendor": "", + "model": "Disk", + "driver": [ + "virtio-pci", + "virtio_blk" + ], + "bus": "None", + "busId": "", + "transport": "unknown", + "sdCard": false, + "dellBOSS": false, + "active": true, + "encrypted": false, + "start": 0, + "size": 32212254720, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0" + ], + "partitionTable": { + "type": "gpt", + "partitions": [ + { + "sid": 79, + "name": "/dev/vdc2", + "description": "Linux RAID", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 10487808, + "size": 5368709120, + "recoverableSize": 5368708608, + "systems": [ + "openSUSE Leap 15.2", + "Fedora 10.30" + ], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part2" + ], + "isEFI": false + }, + { + "sid": 81, + "name": "/dev/vdc4", + "description": "Linux", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 23070720, + "size": 1608515584, + "recoverableSize": 1608515072, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part4" + ], + "isEFI": false + }, + { + "sid": 459, + "name": "/dev/vdc1", + "description": "BIOS Boot Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 8388608, + "recoverableSize": 8388096, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part1" + ], + "isEFI": false + }, + { + "sid": 460, + "name": "/dev/vdc3", + "description": "Swap Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 18432, + "size": 1610612736, + "recoverableSize": 1610571776, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part3" + ], + "isEFI": false, + "filesystem": { + "sid": 461, + "type": "swap", + "mountPath": "swap" + } + }, + { + "sid": 463, + "name": "/dev/vdc5", + "description": "Btrfs Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 26212352, + "size": 18791513600, + "recoverableSize": 18523078144, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part5" + ], + "isEFI": false, + "filesystem": { + "sid": 464, + "type": "btrfs", + "mountPath": "/" + } + } + ], + "unpartitionedSize": 4824515072, + "unusedSlots": [ + { + "start": 3164160, + "size": 3749707776 + }, + { + "start": 20973568, + "size": 1073741824 + } + ] + } + }, + { + "sid": 73, + "name": "/dev/system", + "description": "LVM", + "isDrive": false, + "type": "lvmVg", + "size": 53674508288, + "physicalVolumes": [ + { + "sid": 84, + "name": "/dev/vda2", + "description": "PV of system", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 18432, + "size": 53677637120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "physical_volume", + "deviceNames": [ + "/dev/system" + ] + } + } + ], + "logicalVolumes": [ + { + "sid": 75, + "name": "/dev/system/root", + "description": "Ext4 LV", + "isDrive": false, + "type": "lvmLv", + "active": true, + "encrypted": false, + "start": 0, + "size": 51527024640, + "recoverableSize": 30647779328, + "systems": [], + "udevIds": [], + "udevPaths": [], + "filesystem": { + "sid": 88, + "type": "ext4", + "mountPath": "/" + } + }, + { + "sid": 76, + "name": "/dev/system/swap", + "description": "Swap LV", + "isDrive": false, + "type": "lvmLv", + "active": true, + "encrypted": false, + "start": 0, + "size": 2147483648, + "recoverableSize": 2143289344, + "systems": [], + "udevIds": [], + "udevPaths": [], + "filesystem": { + "sid": 90, + "type": "swap", + "mountPath": "swap" + } + } + ] + }, + { + "sid": 75, + "name": "/dev/system/root", + "description": "Ext4 LV", + "isDrive": false, + "type": "lvmLv", + "active": true, + "encrypted": false, + "start": 0, + "size": 51527024640, + "recoverableSize": 30647779328, + "systems": [], + "udevIds": [], + "udevPaths": [], + "filesystem": { + "sid": 88, + "type": "ext4", + "mountPath": "/" + } + }, + { + "sid": 76, + "name": "/dev/system/swap", + "description": "Swap LV", + "isDrive": false, + "type": "lvmLv", + "active": true, + "encrypted": false, + "start": 0, + "size": 2147483648, + "recoverableSize": 2143289344, + "systems": [], + "udevIds": [], + "udevPaths": [], + "filesystem": { + "sid": 90, + "type": "swap", + "mountPath": "swap" + } + }, + { + "sid": 83, + "name": "/dev/vda1", + "description": "BIOS Boot Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 8388608, + "recoverableSize": 8388096, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part1" + ], + "isEFI": false + }, + { + "sid": 84, + "name": "/dev/vda2", + "description": "PV of system", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 18432, + "size": 53677637120, + "recoverableSize": 0, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:04:00.0-part2" + ], + "isEFI": false, + "component": { + "type": "physical_volume", + "deviceNames": [ + "/dev/system" + ] + } + }, + { + "sid": 79, + "name": "/dev/vdc2", + "description": "Linux RAID", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 10487808, + "size": 5368709120, + "recoverableSize": 5368708608, + "systems": [ + "openSUSE Leap 15.2", + "Fedora 10.30" + ], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part2" + ], + "isEFI": false + }, + { + "sid": 81, + "name": "/dev/vdc4", + "description": "Linux", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 23070720, + "size": 1608515584, + "recoverableSize": 1608515072, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part4" + ], + "isEFI": false + }, + { + "sid": 459, + "name": "/dev/vdc1", + "description": "BIOS Boot Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 2048, + "size": 8388608, + "recoverableSize": 8388096, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part1" + ], + "isEFI": false + }, + { + "sid": 460, + "name": "/dev/vdc3", + "description": "Swap Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 18432, + "size": 1610612736, + "recoverableSize": 1610571776, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part3" + ], + "isEFI": false, + "filesystem": { + "sid": 461, + "type": "swap", + "mountPath": "swap" + } + }, + { + "sid": 463, + "name": "/dev/vdc5", + "description": "Btrfs Partition", + "isDrive": false, + "type": "partition", + "active": true, + "encrypted": false, + "start": 26212352, + "size": 18791513600, + "recoverableSize": 18523078144, + "systems": [], + "udevIds": [], + "udevPaths": [ + "pci-0000:09:00.0-part5" + ], + "isEFI": false, + "filesystem": { + "sid": 464, + "type": "btrfs", + "mountPath": "/" + } + } + ] +}; + +export const actions = [ + { + "device": 86, + "text": "Delete partition /dev/md0p1 (2.00 GiB)", + "subvol": false, + "delete": true + }, + { + "device": 72, + "text": "Delete RAID0 /dev/md0 (10.00 GiB)", + "subvol": false, + "delete": true + }, + { + "device": 80, + "text": "Delete partition /dev/vdc3 (1.00 GiB)", + "subvol": false, + "delete": true + }, + { + "device": 78, + "text": "Delete partition /dev/vdc1 (5.00 GiB)", + "subvol": false, + "delete": true + }, + { + "device": 81, + "text": "Shrink partition /dev/vdc4 from 2.00 GiB to 1.50 GiB", + "subvol": false, + "delete": false + }, + { + "device": 459, + "text": "Create partition /dev/vdc1 (8.00 MiB) as BIOS Boot Partition", + "subvol": false, + "delete": false + }, + { + "device": 460, + "text": "Create partition /dev/vdc3 (1.50 GiB) for swap", + "subvol": false, + "delete": false + }, + { + "device": 463, + "text": "Create partition /dev/vdc5 (17.50 GiB) for / with btrfs", + "subvol": false, + "delete": false + }, + { + "device": 467, + "text": "Create subvolume @ on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + }, + { + "device": 482, + "text": "Create subvolume @/boot/grub2/x86_64-efi on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + }, + { + "device": 480, + "text": "Create subvolume @/boot/grub2/i386-pc on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + }, + { + "device": 478, + "text": "Create subvolume @/var on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + }, + { + "device": 476, + "text": "Create subvolume @/usr/local on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + }, + { + "device": 474, + "text": "Create subvolume @/srv on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + }, + { + "device": 472, + "text": "Create subvolume @/root on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + }, + { + "device": 470, + "text": "Create subvolume @/opt on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + }, + { + "device": 468, + "text": "Create subvolume @/home on /dev/vdc5 (17.50 GiB)", + "subvol": true, + "delete": false + } +]; From 6e9afbccbf44cbe697838431584a4309ac3b33e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 28 Feb 2024 15:36:14 +0000 Subject: [PATCH 14/39] web: Add #uniq and #compact utils --- web/src/utils.js | 26 +++++++++++++++++++++++++- web/src/utils.test.js | 18 +++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/web/src/utils.js b/web/src/utils.js index 5969268cf1..f9645ab238 100644 --- a/web/src/utils.js +++ b/web/src/utils.js @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022-2023] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -66,6 +66,28 @@ const partition = (collection, filter) => { return [pass, fail]; }; +/** + * Generates a new array without null and undefined values. + * @function + * + * @param {Array} collection + * @returns {Array} + */ +function compact(collection) { + return collection.filter(e => e !== null && e !== undefined); +} + +/** + * Generates a new array without duplicates. + * @function + * + * @param {Array} collection + * @returns {Array} + */ +function uniq(collection) { + return [...new Set(collection)]; +} + /** * Simple utility function to help building className conditionally * @@ -355,6 +377,8 @@ export { noop, isObject, partition, + compact, + uniq, classNames, useCancellablePromise, useLocalStorage, diff --git a/web/src/utils.test.js b/web/src/utils.test.js index 8be48355cb..5d340828f8 100644 --- a/web/src/utils.test.js +++ b/web/src/utils.test.js @@ -20,7 +20,7 @@ */ import { - classNames, partition, noop, toValidationError, + classNames, partition, compact, uniq, noop, toValidationError, localConnection, remoteConnection, isObject } from "./utils"; @@ -41,6 +41,22 @@ describe("partition", () => { }); }); +describe("compact", () => { + it("removes null and undefined values", () => { + expect(compact([])).toEqual([]); + expect(compact([undefined, null, "", 0, 1, NaN, false, true])) + .toEqual(["", 0, 1, NaN, false, true]); + }); +}); + +describe("uniq", () => { + it("removes duplicated values", () => { + expect(uniq([])).toEqual([]); + expect(uniq([undefined, null, null, 0, 1, NaN, false, true, false, "test"])) + .toEqual([undefined, null, 0, 1, NaN, false, true, "test"]); + }); +}); + describe("classNames", () => { it("join given arguments, ignoring falsy values", () => { expect(classNames( From 77fb147431e8ae6f24ec17759751782b1af31b07 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, 12 Mar 2024 16:38:23 +0000 Subject: [PATCH 15/39] web: Change detection of used devices - Now it is based on the actions --- web/src/components/storage/DevicesManager.js | 40 ++----------------- .../storage/ProposalResultSection.jsx | 36 +++++++++-------- 2 files changed, 23 insertions(+), 53 deletions(-) diff --git a/web/src/components/storage/DevicesManager.js b/web/src/components/storage/DevicesManager.js index a773fcf9f8..73dde56696 100644 --- a/web/src/components/storage/DevicesManager.js +++ b/web/src/components/storage/DevicesManager.js @@ -77,27 +77,6 @@ export default class DevicesManager { return this.#exist(device, this.staging); } - /** - * LVM volume groups that are going to be created. - * - * @returns {StorageDevice[]} - */ - newLvmVgs() { - return this.#stagingLvmVgs() - .filter(v => !this.existInSystem(v)); - } - - /** - * LVM volume groups that are going to be reused. - * - * @returns {StorageDevice[]} - */ - reusedLvmVgs() { - return this.#stagingLvmVgs() - .filter(v => this.existInSystem(v)) - .filter(v => v.logicalVolumes.find(l => this.hasNewFilesystem(l))); - } - /** * Whether the given device is going to be formatted. * @@ -105,15 +84,12 @@ export default class DevicesManager { * @returns {Boolean} */ hasNewFilesystem(device) { - const systemDevice = this.systemDevice(device.sid); - const stagingDevice = this.stagingDevice(device.sid); + if (!device.filesystem) return false; - if (!systemDevice || !stagingDevice) return false; - - const systemFilesystemSID = systemDevice.filesystem?.sid; - const stagingFilesystemSID = stagingDevice.filesystem?.sid; + const systemDevice = this.systemDevice(device.sid); + const systemFilesystemSID = systemDevice?.filesystem?.sid; - return systemFilesystemSID !== stagingFilesystemSID; + return device.filesystem.sid !== systemFilesystemSID; } /** @@ -161,14 +137,6 @@ export default class DevicesManager { return this.#device(device.sid, source) !== undefined; } - #stagingLvmVgs() { - return this.#lvmVgs(this.staging); - } - - #lvmVgs(source) { - return source.filter(d => d.type === "lvmVg"); - } - #partitionTableChildren(partitionTable) { const { partitions, unusedSlots } = partitionTable; const children = partitions.concat(unusedSlots); diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index efbd5d03e7..dae9703ad1 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -25,6 +25,7 @@ import React, { useState } from "react"; import { Alert, Button, Skeleton } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; +import { compact, uniq } from "~/utils"; import { deviceSize } from "~/components/storage/utils"; import DevicesManager from "~/components/storage/DevicesManager"; import { If, Section, Tag, TreeTable } from "~/components/core"; @@ -101,22 +102,10 @@ const ActionsInfo = ({ actions }) => { * FIXME: add expected types * * @param {object} props - * @param {object} props.settings + * @param {object[]} props.devices * @param {object} props.devicesManager */ -const DevicesTreeTable = ({ settings, devicesManager }) => { - const usedDevices = () => { - const selectedDiskDevices = () => { - return settings.installationDevices - .map(d => devicesManager.stagingDevice(d.sid)) - .filter(Boolean); - }; - - return selectedDiskDevices() - .concat(devicesManager.newLvmVgs()) - .concat(devicesManager.reusedLvmVgs()); - }; - +const DevicesTreeTable = ({ devices, 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 + @@ -199,7 +188,7 @@ const DevicesTreeTable = ({ settings, devicesManager }) => { { title: _("Details"), content: renderDetails, classNames: "details-column" }, { title: _("Size"), content: renderSize, classNames: "sizes-column" } ]} - items={usedDevices()} + items={devices} itemChildren={d => devicesManager.children(d)} rowClassNames={(item) => { if (!item.sid) return "dimmed-row"; @@ -222,16 +211,29 @@ const ResultSkeleton = () => { ); }; -const SectionContent = ({ actions, settings, devices, errors }) => { +const SectionContent = ({ actions, devices, errors }) => { if (errors.length) return; const { system = [], staging = [] } = devices; const devicesManager = new DevicesManager(system, staging); + const isUsed = (device) => { + const actionDevices = uniq(compact(actions.map(a => a.device))); + + return actionDevices.includes(device.sid) || + devicesManager.children(device).find(c => isUsed(c)); + }; + + const usedDevices = () => { + return staging + .filter(d => d.isDrive || d.type === "lvmVg") + .filter(d => isUsed(d) || isUsed(devicesManager.systemDevice(d.sid))); + }; + return ( <> - + ); From ad1e4d2449922c689fd43d090e78da684f1d2fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 12 Mar 2024 17:23:00 +0000 Subject: [PATCH 16/39] web: Move warning about deletions It is now below the result table and it includes the link to check all the actions. What is shown in the warning has been changed too: now it is just a summary about type of devices and systems deleted. Still work in progress. --- .../storage/ProposalResultSection.jsx | 88 +++++++++---------- 1 file changed, 40 insertions(+), 48 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index dae9703ad1..daee2ae95b 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -32,60 +32,19 @@ import { If, Section, Tag, TreeTable } from "~/components/core"; import { ProposalActionsDialog } from "~/components/storage"; /** - * Returns the delete actions from given list - * - * @param {object[]} actions - * @param {object} devicesManager - * @returns {string[]} - */ -const deleteActions = (actions, devicesManager) => { - const actionText = (action) => { - const device = devicesManager.systemDevice(action.device); - - if (device && device.systems.length > 0) - return sprintf(_("%s which contains %s"), action.text, device.systems.join(", ")); - - return action.text; - }; - - return actions.filter(a => a.delete).map(actionText); -}; - -/** - * Renders a warning alert if there are delete actions - * - * @param {object} props - * @param {string[]} props.content - */ -const Warning = ({ content }) => { - const count = content.length; - - if (count === 0) return; - - const title = sprintf(_("%s delete actions will be performed"), count); - - return ( - -
    - { content.map((action, i) =>
  • ) } -
- - ); -}; - -/** - * Renders needed UI elements to allow users check all planned actions + * Renders information about planned actions, allowing to check all of them and + * warning with a summary about the deletion ones, if any. * * @param {object} props * @param {object[]} props.actions + * @param {object} props.devicesManager */ -const ActionsInfo = ({ actions }) => { +const ActionsInfo = ({ actions, devicesManager }) => { const [showActions, setShowActions] = useState(false); - const onOpen = () => setShowActions(true); const onClose = () => setShowActions(false); - return ( + const GeneralActionsInfo = () => ( <>

@@ -94,6 +53,40 @@ const ActionsInfo = ({ actions }) => { ); + + const deleteActions = actions.filter(a => a.delete); + + // Borrowed from https://www.30secondsofcode.org/js/s/count-grouped-elements/ + const frequencies = arr => + arr.reduce((a, v) => { + a[v] = (a[v] ?? 0) + 1; + return a; + }, {}); + + if (deleteActions.length === 0) return ; + + const deletedSids = deleteActions.map(a => a.device); + const deletedDevices = deletedSids.map(sid => devicesManager.systemDevice(sid)); + const deletedTypes = frequencies(deletedDevices.map(d => d.type)); + const deletedSystems = deletedDevices.map(d => d.systems).flat(); + const deletionTypeTexts = Object.entries(deletedTypes).map(([type, amount]) => `${amount} ${type}`) + .join(", "); + + const warningTitle = sprintf(_("%s delete actions will be performed"), deleteActions.length); + const warningTexts = [deletionTypeTexts]; + + if (deletedSystems.length > 0) { + warningTexts.push(_("including")); + warningTexts.push(deletedSystems.join(", ")); + warningTexts.push("systems"); + } + + return ( + +

{warningTexts.join(" ")}

+ + + ); }; /** @@ -232,9 +225,8 @@ const SectionContent = ({ actions, devices, errors }) => { return ( <> - - + ); }; From b16a477668be62e274b98a2fe1b85268737a143c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 12 Mar 2024 17:24:39 +0000 Subject: [PATCH 17/39] web: Drop no needed comments --- web/src/components/storage/test-data/full-result-example.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/web/src/components/storage/test-data/full-result-example.js b/web/src/components/storage/test-data/full-result-example.js index bcb8932d18..9013994320 100644 --- a/web/src/components/storage/test-data/full-result-example.js +++ b/web/src/components/storage/test-data/full-result-example.js @@ -1,6 +1,3 @@ -// eslint:ignore -// cspell:ignore - export const settings = { "bootDevice": "/dev/vdc", "lvm": false, From 4551ef3d3be2727b0b30a0dce3b3a826f7060b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 12:54:19 +0000 Subject: [PATCH 18/39] web: Extend DevicesManager --- web/src/components/storage/DevicesManager.js | 65 +-- .../components/storage/DevicesManager.test.js | 369 ++++++++++++++++++ 2 files changed, 409 insertions(+), 25 deletions(-) create mode 100644 web/src/components/storage/DevicesManager.test.js diff --git a/web/src/components/storage/DevicesManager.js b/web/src/components/storage/DevicesManager.js index 73dde56696..4ffa09c1e1 100644 --- a/web/src/components/storage/DevicesManager.js +++ b/web/src/components/storage/DevicesManager.js @@ -19,9 +19,11 @@ * find current contact information at www.suse.com. */ +import { compact, uniq } from "~/utils"; + /** * @typedef {import ("~/clients/storage").StorageDevice} StorageDevice - * @typedef {import ("~/clients/storage").PartitionSlot} PartitionSlot + * @typedef {import ("~/clients/storage").Action} Action */ /** @@ -31,10 +33,12 @@ export default class DevicesManager { /** * @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. */ - constructor(system, staging) { + constructor(system, staging, actions) { this.system = system; this.staging = staging; + this.actions = actions; } /** @@ -92,26 +96,14 @@ export default class DevicesManager { return device.filesystem.sid !== systemFilesystemSID; } - /** - * Sorted list of children devices (i.e., partitions and unused slots or logical volumes). - * - * @param {StorageDevice} device - * @returns {(StorageDevice|PartitionSlot)[]} - */ - children(device) { - if (device.partitionTable) return this.#partitionTableChildren(device.partitionTable); - if (device.type === "lvmVg") return this.#lvmVgChildren(device); - return []; - } - /** * Whether the given device is going to be shrunk. * * @param {StorageDevice} device * @returns {Boolean} */ - isResized(device) { - return this.resizeSize(device) > 0; + isShrunk(device) { + return this.shrinkSize(device) > 0; } /** @@ -120,13 +112,35 @@ export default class DevicesManager { * @param {StorageDevice} device * @returns {Number} */ - resizeSize(device) { + shrinkSize(device) { const systemDevice = this.systemDevice(device.sid); const stagingDevice = this.stagingDevice(device.sid); if (!systemDevice || !stagingDevice) return 0; - return systemDevice.size - stagingDevice.size; + const amount = systemDevice.size - stagingDevice.size; + return amount > 0 ? amount : 0; + } + + /** + * Disk devices and LVM volume groups used for the installation. + * + * @note The used devices are extracted from the actions. + * + * @returns {StorageDevice[]} + */ + usedDevices() { + const isTarget = (device) => device.isDrive || device.type === "lvmVg"; + + // Check in system devices to detect removals. + const targetSystem = this.system.filter(isTarget); + const targetStaging = this.staging.filter(isTarget); + + const sids = targetSystem.concat(targetStaging) + .filter(d => this.#isUsed(d)) + .map(d => d.sid); + + return compact(uniq(sids).map(sid => this.stagingDevice(sid))); } #device(sid, source) { @@ -137,13 +151,14 @@ export default class DevicesManager { return this.#device(device.sid, source) !== undefined; } - #partitionTableChildren(partitionTable) { - const { partitions, unusedSlots } = partitionTable; - const children = partitions.concat(unusedSlots); - return children.sort((a, b) => a.start < b.start ? -1 : 1); - } + #isUsed(device) { + const sids = uniq(compact(this.actions.map(a => a.device))); + + const partitions = device.partitionTable?.partitions || []; + const lvmLvs = device.logicalVolumes || []; - #lvmVgChildren(lvmVg) { - return lvmVg.logicalVolumes.sort((a, b) => a.name < b.name ? -1 : 1); + return sids.includes(device.sid) || + partitions.find(p => this.#isUsed(p)) !== undefined || + lvmLvs.find(l => this.#isUsed(l)) !== undefined; } } diff --git a/web/src/components/storage/DevicesManager.test.js b/web/src/components/storage/DevicesManager.test.js new file mode 100644 index 0000000000..a343241ad7 --- /dev/null +++ b/web/src/components/storage/DevicesManager.test.js @@ -0,0 +1,369 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import DevicesManager from "./DevicesManager"; + +let system; +let staging; +let actions; + +beforeEach(() => { + system = []; + staging = []; + actions = []; +}); + +describe("systemDevice", () => { + beforeEach(() => { + staging = [{ sid: 60, name: "/dev/sda" }]; + }); + + describe("if there is no system device with the given SID", () => { + it("returns undefined", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.systemDevice(60)).toBeUndefined(); + }); + }); + + describe("if there is a system device with the given SID", () => { + beforeEach(() => { + system = [{ sid: 60, name: "/dev/sdb" }]; + }); + + it("returns the system device", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.systemDevice(60).name).toEqual("/dev/sdb"); + }); + }); +}); + +describe("stagingDevice", () => { + beforeEach(() => { + system = [{ sid: 60, name: "/dev/sda" }]; + }); + + describe("if there is no staging device with the given SID", () => { + it("returns undefined", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.stagingDevice(60)).toBeUndefined(); + }); + }); + + describe("if there is a staging device with the given SID", () => { + beforeEach(() => { + staging = [{ sid: 60, name: "/dev/sdb" }]; + }); + + it("returns the staging device", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.stagingDevice(60).name).toEqual("/dev/sdb"); + }); + }); +}); + +describe("existInSystem", () => { + beforeEach(() => { + system = [{ sid: 61, name: "/dev/sda2" }]; + staging = [{ sid: 60, name: "/dev/sda1" }]; + }); + + describe("if the given device does not exist in system", () => { + it("returns false", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.existInSystem({ sid: 60 })).toEqual(false); + }); + }); + + describe("if the given device exists in system", () => { + it("returns true", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.existInSystem({ sid: 61 })).toEqual(true); + }); + }); +}); + +describe("existInStaging", () => { + beforeEach(() => { + system = [{ sid: 61, name: "/dev/sda2" }]; + staging = [{ sid: 60, name: "/dev/sda1" }]; + }); + + describe("if the given device does not exist in staging", () => { + it("returns false", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.existInStaging({ sid: 61 })).toEqual(false); + }); + }); + + describe("if the given device exists in staging", () => { + it("returns true", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.existInStaging({ sid: 60 })).toEqual(true); + }); + }); +}); + +describe("hasNewFilesystem", () => { + describe("if the given device has no file system", () => { + beforeEach(() => { + system = [{ sid: 60, name: "/dev/sda1", fileystem: { sid: 61 } }]; + staging = [{ sid: 60, name: "/dev/sda1" }]; + }); + + it("returns false", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.hasNewFilesystem(device)).toEqual(false); + }); + }); + + describe("if the given device has no new file system", () => { + beforeEach(() => { + system = [{ sid: 60, name: "/dev/sda1", filesystem: { sid: 61 } }]; + staging = [{ sid: 60, name: "/dev/sda1", filesystem: { sid: 61 } }]; + }); + + it("returns false", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.hasNewFilesystem(device)).toEqual(false); + }); + }); + + describe("if the given device has a new file system", () => { + beforeEach(() => { + system = [{ sid: 60, name: "/dev/sda1", fileystem: { sid: 61 } }]; + staging = [{ sid: 60, name: "/dev/sda1", filesystem: { sid: 62 } }]; + }); + + it("returns true", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.hasNewFilesystem(device)).toEqual(true); + }); + }); +}); + +describe("isShrunk", () => { + describe("if the device is new", () => { + beforeEach(() => { + system = []; + staging = [{ sid: 60, size: 2048 }]; + }); + + it("returns false", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.isShrunk(device)).toEqual(false); + }); + }); + + describe("if the device does not exist anymore", () => { + beforeEach(() => { + system = [{ sid: 60, size: 2048 }]; + staging = []; + }); + + it("returns false", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.systemDevice(60); + expect(manager.isShrunk(device)).toEqual(false); + }); + }); + + describe("if the size is kept", () => { + beforeEach(() => { + system = [{ sid: 60, size: 1024 }]; + staging = [{ sid: 60, size: 1024 }]; + }); + + it("returns false", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.isShrunk(device)).toEqual(false); + }); + }); + + describe("if the size is more than initially", () => { + beforeEach(() => { + system = [{ sid: 60, size: 1024 }]; + staging = [{ sid: 60, size: 2048 }]; + }); + + it("returns false", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.isShrunk(device)).toEqual(false); + }); + }); + + describe("if the size is less than initially", () => { + beforeEach(() => { + system = [{ sid: 60, size: 1024 }]; + staging = [{ sid: 60, size: 512 }]; + }); + + it("returns true", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.isShrunk(device)).toEqual(true); + }); + }); +}); + +describe("shrinkSize", () => { + describe("if the device is new", () => { + beforeEach(() => { + system = []; + staging = [{ sid: 60, size: 2048 }]; + }); + + it("returns 0", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.shrinkSize(device)).toEqual(0); + }); + }); + + describe("if the device does not exist anymore", () => { + beforeEach(() => { + system = [{ sid: 60, size: 2048 }]; + staging = []; + }); + + it("returns 0", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.systemDevice(60); + expect(manager.shrinkSize(device)).toEqual(0); + }); + }); + + describe("if the size is kept", () => { + beforeEach(() => { + system = [{ sid: 60, size: 1024 }]; + staging = [{ sid: 60, size: 1024 }]; + }); + + it("returns 0", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.shrinkSize(device)).toEqual(0); + }); + }); + + describe("if the size is more than initially", () => { + beforeEach(() => { + system = [{ sid: 60, size: 1024 }]; + staging = [{ sid: 60, size: 2048 }]; + }); + + it("returns 0", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.shrinkSize(device)).toEqual(0); + }); + }); + + describe("if the size is less than initially", () => { + beforeEach(() => { + system = [{ sid: 60, size: 1024 }]; + staging = [{ sid: 60, size: 512 }]; + }); + + it("returns the shrink amount", () => { + const manager = new DevicesManager(system, staging, actions); + const device = manager.stagingDevice(60); + expect(manager.shrinkSize(device)).toEqual(512); + }); + }); +}); + +describe("usedDevices", () => { + beforeEach(() => { + system = [ + { sid: 60, isDrive: false }, + { sid: 61, isDrive: true }, + { sid: 62, isDrive: true, partitionTable: { partitions: [{ sid: 67 }] } }, + { sid: 63, isDrive: true, partitionTable: { partitions: [] } }, + { sid: 64, isDrive: false, type: "lvmVg", logicalVolumes: [] }, + { sid: 65, isDrive: false, type: "lvmVg", logicalVolumes: [] }, + { sid: 66, isDrive: false, type: "lvmVg", logicalVolumes: [{ sid: 68 }] } + ]; + staging = [ + { sid: 60, isDrive: false }, + // Partition removed + { sid: 62, isDrive: true, partitionTable: { partitions: [] } }, + // Partition added + { sid: 63, isDrive: true, partitionTable: { partitions: [{ sid: 69 }] } }, + { sid: 64, isDrive: false, type: "lvmVg", logicalVolumes: [] }, + // Logical volume added + { sid: 65, isDrive: false, type: "lvmVg", logicalVolumes: [{ sid: 70 }, { sid: 71 }] }, + // Logical volume removed + { sid: 66, isDrive: false, type: "lvmVg", logicalVolumes: [] } + ]; + }); + + describe("if there are no actions", () => { + beforeEach(() => { + actions = []; + }); + + it("returns an empty list", () => { + const manager = new DevicesManager(system, staging, actions); + expect(manager.usedDevices()).toEqual([]); + }); + }); + + describe("if there are actions", () => { + beforeEach(() => { + actions = [ + // This device is ignored because it is neither a drive nor a LVM VG. + { device: 60 }, + // This device was removed. + { device: 61 }, + // This partition was removed (belongs to device 62). + { device: 67 }, + // This logical volume was removed (belongs to device 66). + { device: 68 }, + // This partition was added (belongs to device 63). + { device: 69 }, + // This logical volume was added (belongs to device 65). + { device: 70 }, + // This logical volume was added (belongs to device 65). + { device: 71 } + ]; + }); + + it("does not include removed disk devices or LVM volume groups", () => { + const manager = new DevicesManager(system, staging, actions); + const sids = manager.usedDevices().map(d => d.sid) + .sort(); + expect(sids).not.toContain(61); + }); + + it("includes all disk devices and LVM volume groups affected by the actions", () => { + const manager = new DevicesManager(system, staging, actions); + const sids = manager.usedDevices().map(d => d.sid) + .sort(); + expect(sids).toEqual([62, 63, 65, 66]); + }); + }); +}); From 64a4234c934251b775188bcd17f19ac78f284328 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 12:55:02 +0000 Subject: [PATCH 19/39] web: Move method to get device children --- web/src/components/storage/utils.js | 29 +++++++++++ web/src/components/storage/utils.test.js | 61 ++++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/web/src/components/storage/utils.js b/web/src/components/storage/utils.js index 082756b08c..ee32b6c7db 100644 --- a/web/src/components/storage/utils.js +++ b/web/src/components/storage/utils.js @@ -28,6 +28,7 @@ import { N_ } from "~/i18n"; /** * @typedef {import ("~/client/storage").Volume} Volume * @typedef {import ("~/clients/storage").StorageDevice} StorageDevice + * @typedef {import ("~/clients/storage").PartitionSlot} PartitionSlot */ /** @@ -140,6 +141,33 @@ const deviceLabel = (device) => { return size ? `${name}, ${deviceSize(size)}` : name; }; +/** + * Sorted list of children devices (i.e., partitions and unused slots or logical volumes). + * @function + * + * @note This method could be directly provided by the device object. For now, the method is kept + * here because the elements considered as children (e.g., partitions + unused slots) is not a + * semantic storage concept but a helper for UI components. + * + * @param {StorageDevice} device + * @returns {(StorageDevice|PartitionSlot)[]} + */ +const deviceChildren = (device) => { + const partitionTableChildren = (partitionTable) => { + const { partitions, unusedSlots } = partitionTable; + const children = partitions.concat(unusedSlots); + return children.sort((a, b) => a.start < b.start ? -1 : 1); + }; + + const lvmVgChildren = (lvmVg) => { + return lvmVg.logicalVolumes.sort((a, b) => a.name < b.name ? -1 : 1); + }; + + if (device.partitionTable) return partitionTableChildren(device.partitionTable); + if (device.type === "lvmVg") return lvmVgChildren(device); + return []; +}; + /** * Checks if volume uses given fs. This method works same as in backend * case insensitive. @@ -193,6 +221,7 @@ export { SIZE_METHODS, SIZE_UNITS, deviceLabel, + deviceChildren, deviceSize, parseToBytes, splitSize, diff --git a/web/src/components/storage/utils.test.js b/web/src/components/storage/utils.test.js index e5eb3cf0aa..c144b123a0 100644 --- a/web/src/components/storage/utils.test.js +++ b/web/src/components/storage/utils.test.js @@ -22,6 +22,7 @@ import { deviceSize, deviceLabel, + deviceChildren, parseToBytes, splitSize, hasFS, @@ -49,6 +50,66 @@ describe("deviceLabel", () => { }); }); +describe("deviceChildren", () => { + let device; + + describe("if the device has partition table", () => { + beforeEach(() => { + device = { + sid: 60, + partitionTable: { + partitions: [ + { sid: 61 }, + { sid: 62 }, + ], + unusedSlots: [ + { start: 1, size: 1024 }, + { start: 2345, size: 512 } + ] + } + }; + }); + + it("returns the partitions and unused slots", () => { + const children = deviceChildren(device); + expect(children.length).toEqual(4); + device.partitionTable.partitions.forEach(p => expect(children).toContainEqual(p)); + device.partitionTable.unusedSlots.forEach(s => expect(children).toContainEqual(s)); + }); + }); + + describe("if the device is a LVM volume group", () => { + beforeEach(() => { + device = { + sid: 60, + type: "lvmVg", + logicalVolumes: [ + { sid: 61 }, + { sid: 62 }, + { sid: 63 } + ] + }; + }); + + it("returns the logical volumes", () => { + const children = deviceChildren(device); + expect(children.length).toEqual(3); + device.logicalVolumes.forEach(l => expect(children).toContainEqual(l)); + }); + }); + + describe("if the device has neither partition table nor logical volumes", () => { + beforeEach(() => { + device = { sid: 60 }; + }); + + it("returns an empty list", () => { + const children = deviceChildren(device); + expect(children.length).toEqual(0); + }); + }); +}); + describe("parseToBytes", () => { it("returns bytes from given input", () => { expect(parseToBytes(1024)).toEqual(1024); From 2281b7909677d8bcb6fff1d784d74b94dc2f5e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 12:55:47 +0000 Subject: [PATCH 20/39] web: Adapt components --- web/src/components/storage/ProposalPage.jsx | 1 - .../storage/ProposalResultSection.jsx | 40 +++++-------------- 2 files changed, 11 insertions(+), 30 deletions(-) diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index 46a2c9b8ab..92ddc2e5ab 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -238,7 +238,6 @@ export default function ProposalPage() { /> { * FIXME: add expected types * * @param {object} props - * @param {object[]} props.devices * @param {object} props.devicesManager */ -const DevicesTreeTable = ({ devices, 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 + @@ -153,11 +151,11 @@ const DevicesTreeTable = ({ devices, devicesManager }) => { }; const renderResizedLabel = (item) => { - if (!item.sid || !devicesManager.isResized(item)) return; + if (!item.sid || !devicesManager.isShrunk(item)) return; return ( - {sprintf(_("Resized %s"), deviceSize(devicesManager.resizeSize(item)))} + {sprintf(_("Resized %s"), deviceSize(devicesManager.shrinkSize(item)))} ); }; @@ -181,8 +179,8 @@ const DevicesTreeTable = ({ devices, devicesManager }) => { { title: _("Details"), content: renderDetails, classNames: "details-column" }, { title: _("Size"), content: renderSize, classNames: "sizes-column" } ]} - items={devices} - itemChildren={d => devicesManager.children(d)} + items={devicesManager.usedDevices()} + itemChildren={d => deviceChildren(d)} rowClassNames={(item) => { if (!item.sid) return "dimmed-row"; }} @@ -204,28 +202,14 @@ const ResultSkeleton = () => { ); }; -const SectionContent = ({ actions, devices, errors }) => { +const SectionContent = ({ system, staging, actions, errors }) => { if (errors.length) return; - const { system = [], staging = [] } = devices; - const devicesManager = new DevicesManager(system, staging); - - const isUsed = (device) => { - const actionDevices = uniq(compact(actions.map(a => a.device))); - - return actionDevices.includes(device.sid) || - devicesManager.children(device).find(c => isUsed(c)); - }; - - const usedDevices = () => { - return staging - .filter(d => d.isDrive || d.type === "lvmVg") - .filter(d => isUsed(d) || isUsed(devicesManager.systemDevice(d.sid))); - }; + const devicesManager = new DevicesManager(system, staging, actions); return ( <> - + ); @@ -239,14 +223,12 @@ const SectionContent = ({ actions, devices, errors }) => { * * @param {object} props * @param {object[]} [props.actions=[]] - * @param {object[]} [props.settings=[]] * @param {object} [props.devices={}] * @param {import("~/client/mixins").ValidationError[]} props.errors - Validation errors * @param {boolean} [props.isLoading=false] - Whether the section content should be rendered as loading */ export default function ProposalResultSection({ actions, - settings, devices, errors = [], isLoading = false @@ -267,9 +249,9 @@ export default function ProposalResultSection({ then={} else={ } From 64638a984f5de040ca406c63ca17f4ed3c9ae957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 13:09:00 +0000 Subject: [PATCH 21/39] web: Fix typo in tests --- web/src/components/storage/DevicesManager.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/components/storage/DevicesManager.test.js b/web/src/components/storage/DevicesManager.test.js index a343241ad7..a14f6caaca 100644 --- a/web/src/components/storage/DevicesManager.test.js +++ b/web/src/components/storage/DevicesManager.test.js @@ -124,7 +124,7 @@ describe("existInStaging", () => { describe("hasNewFilesystem", () => { describe("if the given device has no file system", () => { beforeEach(() => { - system = [{ sid: 60, name: "/dev/sda1", fileystem: { sid: 61 } }]; + system = [{ sid: 60, name: "/dev/sda1", filesystem: { sid: 61 } }]; staging = [{ sid: 60, name: "/dev/sda1" }]; }); @@ -150,7 +150,7 @@ describe("hasNewFilesystem", () => { describe("if the given device has a new file system", () => { beforeEach(() => { - system = [{ sid: 60, name: "/dev/sda1", fileystem: { sid: 61 } }]; + system = [{ sid: 60, name: "/dev/sda1", filesystem: { sid: 61 } }]; staging = [{ sid: 60, name: "/dev/sda1", filesystem: { sid: 62 } }]; }); From 655aca379bf7a25a66e4ffa33e75bacc0254413b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 13:09:24 +0000 Subject: [PATCH 22/39] web: Do not consider subvol for delete actions --- web/src/components/storage/ProposalResultSection.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 160953637b..84010532e6 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -53,7 +53,7 @@ const ActionsInfo = ({ actions, devicesManager }) => { ); - const deleteActions = actions.filter(a => a.delete); + const deleteActions = actions.filter(a => a.delete && !a.subvol); // Borrowed from https://www.30secondsofcode.org/js/s/count-grouped-elements/ const frequencies = arr => From bbdd6e580a98c5dd2c119cbeb248f074498c849c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 13:13:53 +0000 Subject: [PATCH 23/39] service: Export SID of the file system to D-Bus - It is useful to dectect whether the file system is new. --- .../dbus/storage/interfaces/device/filesystem.rb | 12 +++++++++++- .../storage/interfaces/device/filesystem_examples.rb | 6 ++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/service/lib/agama/dbus/storage/interfaces/device/filesystem.rb b/service/lib/agama/dbus/storage/interfaces/device/filesystem.rb index ad066f2022..545a179813 100644 --- a/service/lib/agama/dbus/storage/interfaces/device/filesystem.rb +++ b/service/lib/agama/dbus/storage/interfaces/device/filesystem.rb @@ -45,6 +45,15 @@ def self.apply?(storage_device) FILESYSTEM_INTERFACE = "org.opensuse.Agama.Storage1.Filesystem" private_constant :FILESYSTEM_INTERFACE + # SID of the file system. + # + # It is useful to detect whether a file system is new. + # + # @return [Integer] + def filesystem_sid + storage_device.filesystem.sid + end + # File system type. # # @return [String] e.g., "ext4" @@ -68,7 +77,8 @@ def filesystem_label def self.included(base) base.class_eval do - dbus_interface FILESYSTEM_INTERFACE do + dbus_interface FILESYSTEM_INTERFACE do + dbus_reader :filesystem_sid, "u", dbus_name: "SID" dbus_reader :filesystem_type, "s", dbus_name: "Type" dbus_reader :filesystem_mount_path, "s", dbus_name: "MountPath" dbus_reader :filesystem_label, "s", dbus_name: "Label" 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 84b8da4a10..71694a3ab7 100644 --- a/service/test/agama/dbus/storage/interfaces/device/filesystem_examples.rb +++ b/service/test/agama/dbus/storage/interfaces/device/filesystem_examples.rb @@ -28,6 +28,12 @@ let(:device) { devicegraph.find_by_name("/dev/mapper/0QEMU_QEMU_HARDDISK_mpath1") } + describe "#filesystem_sid" do + it "returns the file system SID" do + expect(subject.filesystem_sid).to eq(45) + end + end + describe "#filesystem_type" do it "returns the file system type" do expect(subject.filesystem_type).to eq("ext4") From 49a71f25ac78ad1d058dfec4d650e9dd4d8636b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 13:15:01 +0000 Subject: [PATCH 24/39] web: Adapt storage client --- web/src/client/storage.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index b7d6135f4b..cf75d81e61 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -112,7 +112,7 @@ class DevicesManager { * @property {string} name - Device name * @property {string} description - Device description * @property {boolean} isDrive - Whether the device is a drive - * @property {string} type - Type of device ("disk", "raid", "multipath", "dasd", "md") + * @property {string} type - Type of device (e.g., "disk", "raid", "multipath", "dasd", "md") * @property {string} [vendor] * @property {string} [model] * @property {string[]} [driver] @@ -236,6 +236,7 @@ class DevicesManager { const buildMountPath = path => path.length > 0 ? path : undefined; const buildLabel = label => label.length > 0 ? label : undefined; device.filesystem = { + sid: filesystemProperties.SID.v, type: filesystemProperties.Type.v, mountPath: buildMountPath(filesystemProperties.MountPath.v), label: buildLabel(filesystemProperties.Label.v) @@ -427,6 +428,7 @@ class ProposalManager { * @property {Action[]} actions * * @typedef {object} Action + * @property {number} device * @property {string} text * @property {boolean} subvol * @property {boolean} delete From 57dcc3753b67b328dc54479b6bddd92e4572e4a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 13:56:06 +0000 Subject: [PATCH 25/39] web: Fix types --- web/src/client/storage.js | 194 +++++++++--------- web/src/components/storage/DevicesManager.js | 6 +- web/src/components/storage/ProposalPage.jsx | 3 +- .../storage/ProposalResultSection.jsx | 50 +++-- web/src/components/storage/utils.js | 7 +- 5 files changed, 140 insertions(+), 120 deletions(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index cf75d81e61..4c2b2153bd 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -47,6 +47,103 @@ const ZFCP_CONTROLLER_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Controller"; const ZFCP_DISKS_NAMESPACE = "/org/opensuse/Agama/Storage1/zfcp_disks"; const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk"; +/** + * @typedef {object} StorageDevice + * @property {number} sid - Storage ID + * @property {string} name - Device name + * @property {string} description - Device description + * @property {boolean} isDrive - Whether the device is a drive + * @property {string} type - Type of device (e.g., "disk", "raid", "multipath", "dasd", "md") + * @property {string} [vendor] + * @property {string} [model] + * @property {string[]} [driver] + * @property {string} [bus] + * @property {string} [busId] - DASD Bus ID (only for "dasd" type) + * @property {string} [transport] + * @property {boolean} [sdCard] + * @property {boolean} [dellBOOS] + * @property {string[]} [devices] - RAID devices (only for "raid" and "md" types) + * @property {string[]} [wires] - Multipath wires (only for "multipath" type) + * @property {string} [level] - MD RAID level (only for "md" type) + * @property {string} [uuid] + * @property {number} [start] - First block of the region (only for block devices) + * @property {boolean} [active] + * @property {boolean} [encrypted] - Whether the device is encrypted (only for block devices) + * @property {boolean} [isEFI] - Whether the device is an EFI partition (only for partition) + * @property {number} [size] + * @property {number} [recoverableSize] + * @property {string[]} [systems] - Name of the installed systems + * @property {string[]} [udevIds] + * @property {string[]} [udevPaths] + * @property {PartitionTable} [partitionTable] + * @property {Filesystem} [filesystem] + * @property {Component} [component] - When it is used as component of other devices + * @property {StorageDevice[]} [physicalVolumes] - Only for LVM VGs + * @property {StorageDevice[]} [logicalVolumes] - Only for LVM VGs + * + * @typedef {object} PartitionTable + * @property {string} type + * @property {StorageDevice[]} partitions + * @property {PartitionSlot[]} unusedSlots + * @property {number} unpartitionedSize - Total size not assigned to any partition + * + * @typedef {object} PartitionSlot + * @property {number} start + * @property {number} size + * + * @typedef {object} Component + * @property {string} type + * @property {string[]} deviceNames + * + * @typedef {object} Filesystem + * @property {number} sid + * @property {string} type + * @property {string} [mountPath] + * + * @typedef {object} ProposalResult + * @property {ProposalSettings} settings + * @property {Action[]} actions + * + * @typedef {object} Action + * @property {number} device + * @property {string} text + * @property {boolean} subvol + * @property {boolean} delete + * + * @typedef {object} ProposalSettings + * @property {string} bootDevice + * @property {string} encryptionPassword + * @property {string} encryptionMethod + * @property {boolean} lvm + * @property {string} spacePolicy + * @property {SpaceAction[]} spaceActions + * @property {string[]} systemVGDevices + * @property {Volume[]} volumes + * @property {StorageDevice[]} installationDevices + * + * @typedef {object} SpaceAction + * @property {string} device + * @property {string} action + * + * @typedef {object} Volume + * @property {string} mountPath + * @property {string} fsType + * @property {number} minSize + * @property {number} [maxSize] + * @property {boolean} autoSize + * @property {boolean} snapshots + * @property {boolean} transactional + * @property {VolumeOutline} outline + * + * @typedef {object} VolumeOutline + * @property {boolean} required + * @property {string[]} fsTypes + * @property {boolean} supportAutoSize + * @property {boolean} snapshotsConfigurable + * @property {boolean} snapshotsAffectSizes + * @property {string[]} sizeRelevantVolumes + */ + /** * Enum for the encryption method values * @@ -106,57 +203,6 @@ class DevicesManager { * Gets all the exported devices * * @returns {Promise} - * - * @typedef {object} StorageDevice - * @property {string} sid - Storage ID - * @property {string} name - Device name - * @property {string} description - Device description - * @property {boolean} isDrive - Whether the device is a drive - * @property {string} type - Type of device (e.g., "disk", "raid", "multipath", "dasd", "md") - * @property {string} [vendor] - * @property {string} [model] - * @property {string[]} [driver] - * @property {string} [bus] - * @property {string} [busId] - DASD Bus ID (only for "dasd" type) - * @property {string} [transport] - * @property {boolean} [sdCard] - * @property {boolean} [dellBOOS] - * @property {string[]} [devices] - RAID devices (only for "raid" and "md" types) - * @property {string[]} [wires] - Multipath wires (only for "multipath" type) - * @property {string} [level] - MD RAID level (only for "md" type) - * @property {string} [uuid] - * @property {number} [start] - First block of the region (only for block devices) - * @property {boolean} [active] - * @property {boolean} [encrypted] - Whether the device is encrypted (only for block devices) - * @property {boolean} [isEFI] - Whether the device is an EFI partition (only for partition) - * @property {number} [size] - * @property {number} [recoverableSize] - * @property {string[]} [systems] - Name of the installed systems - * @property {string[]} [udevIds] - * @property {string[]} [udevPaths] - * @property {PartitionTable} [partitionTable] - * @property {Filesystem} [filesystem] - * @property {Component} [component] - When it is used as component of other devices - * @property {StorageDevice[]} [physicalVolumes] - Only for LVM VGs - * @property {StorageDevice[]} [logicalVolumes] - Only for LVM VGs - * - * @typedef {object} PartitionTable - * @property {string} type - * @property {StorageDevice[]} partitions - * @property {PartitionSlot[]} unusedSlots - * @property {number} unpartitionedSize - Total size not assigned to any partition - * - * @typedef {object} PartitionSlot - * @property {number} start - * @property {number} size - * - * @typedef {object} Component - * @property {string} type - * @property {string[]} deviceNames - * - * @typedef {object} Filesystem - * @property {string} type - * @property {string} [mountPath] */ async getDevices() { const buildDevice = (path, dbusDevices) => { @@ -330,42 +376,6 @@ class ProposalManager { }; } - /** - * @typedef {object} ProposalSettings - * @property {string} bootDevice - * @property {string} encryptionPassword - * @property {string} encryptionMethod - * @property {boolean} lvm - * @property {string} spacePolicy - * @property {SpaceAction[]} spaceActions - * @property {string[]} systemVGDevices - * @property {Volume[]} volumes - * @property {StorageDevice[]} installationDevices - * - * @typedef {object} SpaceAction - * @property {string} device - * @property {string} action - * - * @typedef {object} Volume - * @property {string} mountPath - * @property {string} fsType - * @property {number} minSize - * @property {number} [maxSize] - * @property {boolean} autoSize - * @property {boolean} snapshots - * @property {boolean} transactional - * @property {VolumeOutline} outline - * - * @typedef {object} VolumeOutline - * @property {boolean} required - * @property {string[]} fsTypes - * @property {boolean} supportAutoSize - * @property {boolean} adjustByRam - * @property {boolean} snapshotsConfigurable - * @property {boolean} snapshotsAffectSizes - * @property {string[]} sizeRelevantVolumes - */ - /** * Gets the list of available devices * @@ -422,16 +432,6 @@ class ProposalManager { * Gets the values of the current proposal * * @return {Promise} - * - * @typedef {object} ProposalResult - * @property {ProposalSettings} settings - * @property {Action[]} actions - * - * @typedef {object} Action - * @property {number} device - * @property {string} text - * @property {boolean} subvol - * @property {boolean} delete */ async getResult() { const proxy = await this.proposalProxy(); diff --git a/web/src/components/storage/DevicesManager.js b/web/src/components/storage/DevicesManager.js index 4ffa09c1e1..9848ce5bfd 100644 --- a/web/src/components/storage/DevicesManager.js +++ b/web/src/components/storage/DevicesManager.js @@ -19,11 +19,13 @@ * find current contact information at www.suse.com. */ +// @ts-check + import { compact, uniq } from "~/utils"; /** - * @typedef {import ("~/clients/storage").StorageDevice} StorageDevice - * @typedef {import ("~/clients/storage").Action} Action + * @typedef {import ("~/client/storage").Action} Action + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice */ /** diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index 92ddc2e5ab..f17568c08b 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -237,8 +237,9 @@ export default function ProposalPage() { isLoading={state.loading} /> diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 84010532e6..abe534e7a2 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -31,12 +31,19 @@ import { If, Section, Tag, TreeTable } from "~/components/core"; import { ProposalActionsDialog } from "~/components/storage"; /** - * Renders information about planned actions, allowing to check all of them and - * warning with a summary about the deletion ones, if any. + * @typedef {import ("~/client/storage").Action} Action + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import("~/client/mixins").ValidationError} ValidationError + */ + +/** + * Renders information about planned actions, allowing to check all of them and warning with a + * summary about the deletion ones, if any. + * @component * * @param {object} props - * @param {object[]} props.actions - * @param {object} props.devicesManager + * @param {Action[]} props.actions + * @param {DevicesManager} props.devicesManager */ const ActionsInfo = ({ actions, devicesManager }) => { const [showActions, setShowActions] = useState(false); @@ -89,12 +96,11 @@ const ActionsInfo = ({ actions, devicesManager }) => { }; /** - * Renders a TreeTable rendering the devices proposal result - * - * FIXME: add expected types + * Renders a TreeTable rendering the devices proposal result. + * @component * * @param {object} props - * @param {object} props.devicesManager + * @param {DevicesManager} props.devicesManager */ const DevicesTreeTable = ({ devicesManager }) => { const renderDeviceName = (item) => { @@ -202,6 +208,16 @@ const ResultSkeleton = () => { ); }; +/** + * Content of the section. + * @component + * + * @param {object} props + * @param {StorageDevice[]} props.system + * @param {StorageDevice[]} props.staging + * @param {Action[]} props.actions + * @param {ValidationError[]} props.errors + */ const SectionContent = ({ system, staging, actions, errors }) => { if (errors.length) return; @@ -219,17 +235,17 @@ const SectionContent = ({ system, staging, actions, errors }) => { * Section holding the proposal result and actions to perform in the system * @component * - * FIXME: add expected types - * * @param {object} props - * @param {object[]} [props.actions=[]] - * @param {object} [props.devices={}] - * @param {import("~/client/mixins").ValidationError[]} props.errors - Validation errors + * @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 */ export default function ProposalResultSection({ - actions, - devices, + system = [], + staging = [], + actions = [], errors = [], isLoading = false }) { @@ -249,8 +265,8 @@ export default function ProposalResultSection({ then={} else={ diff --git a/web/src/components/storage/utils.js b/web/src/components/storage/utils.js index ee32b6c7db..7817acd4e1 100644 --- a/web/src/components/storage/utils.js +++ b/web/src/components/storage/utils.js @@ -19,6 +19,7 @@ * find current contact information at www.suse.com. */ +// @ts-check // cspell:ignore xbytes import xbytes from "xbytes"; @@ -27,8 +28,8 @@ import { N_ } from "~/i18n"; /** * @typedef {import ("~/client/storage").Volume} Volume - * @typedef {import ("~/clients/storage").StorageDevice} StorageDevice - * @typedef {import ("~/clients/storage").PartitionSlot} PartitionSlot + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * @typedef {import ("~/client/storage").PartitionSlot} PartitionSlot */ /** @@ -122,7 +123,7 @@ const deviceSize = (size) => { const parseToBytes = (size) => { if (!size || size === undefined || size === "") return 0; - const value = xbytes.parseSize(size, { iec: true }) || parseInt(size); + const value = xbytes.parseSize(size.toString(), { iec: true }) || parseInt(size.toString()); // Avoid decimals resulting from the conversion. D-Bus iface only accepts integer return Math.trunc(value); From f11b86e934c5d7c13b12758bec244405391d76d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 13:24:09 +0000 Subject: [PATCH 26/39] web: change warning content in storage result It has been decided to only show affected systems by now. --- .../storage/ProposalResultSection.jsx | 45 +++++++++---------- .../storage/ProposalResultSection.test.jsx | 26 ++++++++--- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index abe534e7a2..abd85c2b67 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -24,7 +24,7 @@ import React, { useState } from "react"; import { Alert, Button, Skeleton } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; -import { _ } from "~/i18n"; +import { _, n_ } from "~/i18n"; import { deviceChildren, deviceSize } from "~/components/storage/utils"; import DevicesManager from "~/components/storage/DevicesManager"; import { If, Section, Tag, TreeTable } from "~/components/core"; @@ -61,35 +61,32 @@ const ActionsInfo = ({ actions, devicesManager }) => { ); const deleteActions = actions.filter(a => a.delete && !a.subvol); + const deleteActionsSize = deleteActions.length; - // Borrowed from https://www.30secondsofcode.org/js/s/count-grouped-elements/ - const frequencies = arr => - arr.reduce((a, v) => { - a[v] = (a[v] ?? 0) + 1; - return a; - }, {}); - - if (deleteActions.length === 0) return ; + if (deleteActionsSize === 0) return ; const deletedSids = deleteActions.map(a => a.device); const deletedDevices = deletedSids.map(sid => devicesManager.systemDevice(sid)); - const deletedTypes = frequencies(deletedDevices.map(d => d.type)); - const deletedSystems = deletedDevices.map(d => d.systems).flat(); - const deletionTypeTexts = Object.entries(deletedTypes).map(([type, amount]) => `${amount} ${type}`) - .join(", "); - - const warningTitle = sprintf(_("%s delete actions will be performed"), deleteActions.length); - const warningTexts = [deletionTypeTexts]; - - if (deletedSystems.length > 0) { - warningTexts.push(_("including")); - warningTexts.push(deletedSystems.join(", ")); - warningTexts.push("systems"); - } - + const deletedSystems = deletedDevices.map(d => d?.systems).flat(); + const warningTitle = sprintf(n_( + "That proposal will perform %d destructive action", + "That proposal will perform %d destructive actions", + deleteActionsSize + ), deleteActionsSize); + + // FIXME: Use the Intl.ListFormat instead of the `join(", ")` used below. + // Most probably, a `listFormat` or similar wrapper should live in src/i18n.js or so. + // Read https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/ListFormat return ( -

{warningTexts.join(" ")}

+ 0} + then={ +

+ {_("Including the deletion of")} {deletedSystems.join(", ")} +

+ } + /> ); diff --git a/web/src/components/storage/ProposalResultSection.test.jsx b/web/src/components/storage/ProposalResultSection.test.jsx index 663e178634..cec16e2b6e 100644 --- a/web/src/components/storage/ProposalResultSection.test.jsx +++ b/web/src/components/storage/ProposalResultSection.test.jsx @@ -23,11 +23,11 @@ import React from "react"; import { screen, within } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { ProposalResultSection } from "~/components/storage"; -import { settings, devices, actions } from "./test-data/full-result-example"; +import { devices, actions } from "./test-data/full-result-example"; const errorMessage = "Something went wrong, proposal not possible"; const errors = [{ severity: 0, message: errorMessage }]; -const defaultProps = { settings, actions, devices }; +const defaultProps = { system: devices.system, staging: devices.staging, actions }; describe("ProposalResultSection", () => { describe("when there are errors (proposal was not possible)", () => { @@ -36,7 +36,7 @@ describe("ProposalResultSection", () => { expect(screen.queryByText(errorMessage)).toBeInTheDocument(); }); - it("does not render an warning for delete actions", () => { + it("does not render a warning for delete actions", () => { plainRender(); expect(screen.queryByText(/Warning alert:/)).toBeNull(); }); @@ -63,9 +63,25 @@ describe("ProposalResultSection", () => { expect(screen.queryByText(/Warning alert:/)).toBeNull(); }); - it("renders a warning when when there are delete", () => { + it("renders a warning when there are delete actions", () => { plainRender(); - screen.getByText(/Warning alert:/); + const warning = screen.getByText(/Warning alert:/).parentNode; + within(warning).getByText(/4 destructive/); + }); + + it("renders the affected systems in the deletion warning, if any", () => { + // NOTE: simulate the deletion of vdc2 (sid: 79) for checking that + // affected systems are rendered in the warning summary + const props = { + ...defaultProps, + actions: [{ device: 79, delete: true }] + }; + + plainRender(); + // FIXME: below line reveals that warning wrapper deserves a role or + // something + const warning = screen.getByText(/Warning alert:/).parentNode.parentNode; + within(warning).getByText(/openSUSE/); }); it("renders a treegrid including all relevant information about final result", () => { From db04816e557758b089f4200961cd354c31036214 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 16:35:48 +0000 Subject: [PATCH 27/39] web: Fix tests --- web/src/client/storage.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index d78dc64263..1604c04449 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -205,7 +205,7 @@ const md0 = { systems : ["openSUSE Leap 15.2"], udevIds: [], udevPaths: [], - filesystem: { type: "ext4", mountPath: "/test", label: "system" } + filesystem: { sid: 100, type: "ext4", mountPath: "/test", label: "system" } }; const raid = { @@ -866,6 +866,7 @@ const contexts = { UdevPaths: { t: "as", v: [] } }, "org.opensuse.Agama.Storage1.Filesystem": { + SID: { t: "u", v: 100 }, Type: { t: "s", v: "ext4" }, MountPath: { t: "s", v: "/test" }, Label: { t: "s", v: "system" } From 8f986634472946eea3b064b4255a93ee7a7d5fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 16:36:18 +0000 Subject: [PATCH 28/39] web: Add methods to DevicesManager --- web/src/components/storage/DevicesManager.js | 41 +++++++++++++++ .../components/storage/DevicesManager.test.js | 51 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/web/src/components/storage/DevicesManager.js b/web/src/components/storage/DevicesManager.js index 9848ce5bfd..2f836fe86c 100644 --- a/web/src/components/storage/DevicesManager.js +++ b/web/src/components/storage/DevicesManager.js @@ -145,14 +145,55 @@ export default class DevicesManager { return compact(uniq(sids).map(sid => this.stagingDevice(sid))); } + /** + * Devices deleted. + * + * @note The devices are extracted from the actions. + * + * @returns {StorageDevice[]} + */ + deletedDevices() { + const sids = this.actions + .filter(a => a.delete) + .map(a => a.device); + const devices = sids.map(sid => this.systemDevice(sid)); + return compact(devices); + } + + /** + * Systems deleted. + * + * @returns {string[]} + */ + deletedSystems() { + const systems = this.deletedDevices() + .map(d => d.systems) + .flat(); + return compact(systems); + } + + /** + * @param {number} sid + * @param {StorageDevice[]} source + * @returns {StorageDevice|undefined} + */ #device(sid, source) { return source.find(d => d.sid === sid); } + /** + * @param {StorageDevice} device + * @param {StorageDevice[]} source + * @returns {boolean} + */ #exist(device, source) { return this.#device(device.sid, source) !== undefined; } + /** + * @param {StorageDevice} device + * @returns {boolean} + */ #isUsed(device) { const sids = uniq(compact(this.actions.map(a => a.device))); diff --git a/web/src/components/storage/DevicesManager.test.js b/web/src/components/storage/DevicesManager.test.js index a14f6caaca..53ff712927 100644 --- a/web/src/components/storage/DevicesManager.test.js +++ b/web/src/components/storage/DevicesManager.test.js @@ -367,3 +367,54 @@ describe("usedDevices", () => { }); }); }); + +describe("deletedDevices", () => { + beforeEach(() => { + system = [ + { sid: 60 }, + { sid: 62 }, + { sid: 63 }, + { sid: 64 } + ]; + actions = [ + { device: 60, delete: true }, + // This device does not exist in system. + { device: 61, delete: true }, + { device: 62, delete: false }, + { device: 63, delete: true } + ]; + }); + + it("includes all deleted devices", () => { + const manager = new DevicesManager(system, staging, actions); + const sids = manager.deletedDevices().map(d => d.sid) + .sort(); + expect(sids).toEqual([60, 63]); + }); +}); + +describe("deletedSystems", () => { + beforeEach(() => { + system = [ + { sid: 60, systems: ["Windows XP"] }, + { sid: 62, systems: ["Ubuntu"] }, + { sid: 63, systems: ["openSUSE Leap"] }, + { sid: 64 } + ]; + actions = [ + { device: 60, delete: true }, + // This device does not exist in system. + { device: 61, delete: true }, + { device: 62, delete: false }, + { device: 63, delete: true } + ]; + }); + + it("includes all deleted systems", () => { + const manager = new DevicesManager(system, staging, actions); + const systems = manager.deletedSystems(); + expect(systems.length).toEqual(2); + expect(systems).toContain("Windows XP"); + expect(systems).toContain("openSUSE Leap"); + }); +}); From 042a96b32f5d70493adf91913c9a100f15266f3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 16:46:52 +0000 Subject: [PATCH 29/39] web: Add a new core/Reminder variant --- web/src/assets/styles/blocks.scss | 19 ++++++++++++++++++- web/src/components/core/Reminder.jsx | 6 ++++-- web/src/components/core/Reminder.test.jsx | 6 ++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 2dc067cb41..e29ef42181 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -559,7 +559,24 @@ table.proposal-result { h4 { color: var(--accent-color); - margin-block-end: var(--spacer-smaller); + } + + h4 ~ * { + margin-block-start: var(--spacer-small); + } +} + +section [data-type="agama/reminder"] { + margin-inline: 0; +} + +[data-type="agama/reminder"][data-variant="subtle"] { + --accent-color: var(--color-primary); + padding-block: 0; + border-inline-start-width: 1px; + + h4 { + font-size: var(--fs-normal); } } diff --git a/web/src/components/core/Reminder.jsx b/web/src/components/core/Reminder.jsx index 997e870a91..cd4e0943ad 100644 --- a/web/src/components/core/Reminder.jsx +++ b/web/src/components/core/Reminder.jsx @@ -62,7 +62,8 @@ const ReminderTitle = ({ children }) => { * @param {object} props * @param {string} [props.icon] - The name of desired icon. * @param {JSX.Element|string} [props.title] - The content for the title. - * @param {string} [props.role="status"] - The reminder's role, "status" by + * @param {string} [props.role="status"] - The reminder's role, "status" by default. + * @param {("subtle")} [props.variant] - The reminder's variant, none by default. * default. See {@link https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/status_role} * @param {JSX.Element} [props.children] - The content for the description. */ @@ -70,10 +71,11 @@ export default function Reminder ({ icon, title, role = "status", + variant, children }) { return ( -
+
{title} diff --git a/web/src/components/core/Reminder.test.jsx b/web/src/components/core/Reminder.test.jsx index 24527cf2d0..41ebfaf300 100644 --- a/web/src/components/core/Reminder.test.jsx +++ b/web/src/components/core/Reminder.test.jsx @@ -37,6 +37,12 @@ describe("Reminder", () => { within(reminder).getByText("Example"); }); + it("renders a region with given data-variant, if any", () => { + plainRender(Example); + const reminder = screen.getByRole("alert"); + expect(reminder).toHaveAttribute("data-variant", "subtle"); + }); + it("renders given title", () => { plainRender( Kindly reminder}> From 50e4a35a44b9b75a55867af0ba4ed729d3618fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 16:52:36 +0000 Subject: [PATCH 30/39] web: Render deletion actions summary in a reminder Instead of using an intrusive warning since it is too early to do so. A subtle reminder is enough in the storage proposal page, postponing the warning to the moment right before start the installation if finally it is needed. --- .../storage/ProposalResultSection.jsx | 54 ++++++++++--------- .../storage/ProposalResultSection.test.jsx | 12 ++--- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index abd85c2b67..f14d14938e 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -22,12 +22,12 @@ // @ts-check import React, { useState } from "react"; -import { Alert, Button, Skeleton } from "@patternfly/react-core"; +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, Tag, TreeTable } from "~/components/core"; +import { If, Section, Reminder, Tag, TreeTable } from "~/components/core"; import { ProposalActionsDialog } from "~/components/storage"; /** @@ -45,32 +45,18 @@ import { ProposalActionsDialog } from "~/components/storage"; * @param {Action[]} props.actions * @param {DevicesManager} props.devicesManager */ -const ActionsInfo = ({ actions, devicesManager }) => { - const [showActions, setShowActions] = useState(false); - const onOpen = () => setShowActions(true); - const onClose = () => setShowActions(false); - - const GeneralActionsInfo = () => ( - <> -

- - {_("to create these file systems and to ensure the new system boots.")} -

- - - ); - +const DeletionsInfo = ({ actions, devicesManager }) => { const deleteActions = actions.filter(a => a.delete && !a.subvol); const deleteActionsSize = deleteActions.length; - if (deleteActionsSize === 0) return ; + if (deleteActionsSize === 0) return; const deletedSids = deleteActions.map(a => a.device); const deletedDevices = deletedSids.map(sid => devicesManager.systemDevice(sid)); const deletedSystems = deletedDevices.map(d => d?.systems).flat(); const warningTitle = sprintf(n_( - "That proposal will perform %d destructive action", - "That proposal will perform %d destructive actions", + "There are %d destructive planned action", + "There are %d destructive planned actions", deleteActionsSize ), deleteActionsSize); @@ -78,7 +64,7 @@ const ActionsInfo = ({ actions, devicesManager }) => { // Most probably, a `listFormat` or similar wrapper should live in src/i18n.js or so. // Read https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/ListFormat return ( - + 0} then={ @@ -87,8 +73,27 @@ const ActionsInfo = ({ actions, devicesManager }) => {

} /> - - + + ); +}; + +/** + * Renders needed UI elements to allow user check the proposal planned actions + * @component + * + * @param {object} props + * @param {Action[]} props.actions + */ +const ActionsInfo = ({ actions }) => { + const [showActions, setShowActions] = useState(false); + const onOpen = () => setShowActions(true); + const onClose = () => setShowActions(false); + + return ( + <> + + + ); }; @@ -223,7 +228,8 @@ const SectionContent = ({ system, staging, actions, errors }) => { return ( <> - + + ); }; diff --git a/web/src/components/storage/ProposalResultSection.test.jsx b/web/src/components/storage/ProposalResultSection.test.jsx index cec16e2b6e..7e96795516 100644 --- a/web/src/components/storage/ProposalResultSection.test.jsx +++ b/web/src/components/storage/ProposalResultSection.test.jsx @@ -63,13 +63,13 @@ describe("ProposalResultSection", () => { expect(screen.queryByText(/Warning alert:/)).toBeNull(); }); - it("renders a warning when there are delete actions", () => { + it("renders a reminder when there are delete actions", () => { plainRender(); - const warning = screen.getByText(/Warning alert:/).parentNode; - within(warning).getByText(/4 destructive/); + const reminder = screen.getByRole("status"); + within(reminder).getByText(/4 destructive/); }); - it("renders the affected systems in the deletion warning, if any", () => { + it("renders the affected systems in the deletion reminder, if any", () => { // NOTE: simulate the deletion of vdc2 (sid: 79) for checking that // affected systems are rendered in the warning summary const props = { @@ -80,8 +80,8 @@ describe("ProposalResultSection", () => { plainRender(); // FIXME: below line reveals that warning wrapper deserves a role or // something - const warning = screen.getByText(/Warning alert:/).parentNode.parentNode; - within(warning).getByText(/openSUSE/); + const reminder = screen.getByRole("status"); + within(reminder).getByText(/openSUSE/); }); it("renders a treegrid including all relevant information about final result", () => { From 47fd870c087d6fea77bbe9177fbcd0d1a0b39f0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 13 Mar 2024 17:08:55 +0000 Subject: [PATCH 31/39] web: adapt code to changes made in DevicesManager See c26d540e559aa585505bcf9cb4b40903867266ca --- .../storage/ProposalResultSection.jsx | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index f14d14938e..1465fa8026 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -42,23 +42,19 @@ import { ProposalActionsDialog } from "~/components/storage"; * @component * * @param {object} props - * @param {Action[]} props.actions - * @param {DevicesManager} props.devicesManager + * @param {object[]} props.actions + * @param {string[]} props.systems */ -const DeletionsInfo = ({ actions, devicesManager }) => { - const deleteActions = actions.filter(a => a.delete && !a.subvol); - const deleteActionsSize = deleteActions.length; +const DeletionsInfo = ({ actions, systems }) => { + const total = actions.length; - if (deleteActionsSize === 0) return; + if (total === 0) return; - const deletedSids = deleteActions.map(a => a.device); - const deletedDevices = deletedSids.map(sid => devicesManager.systemDevice(sid)); - const deletedSystems = deletedDevices.map(d => d?.systems).flat(); const warningTitle = sprintf(n_( "There are %d destructive planned action", "There are %d destructive planned actions", - deleteActionsSize - ), deleteActionsSize); + total + ), total); // FIXME: Use the Intl.ListFormat instead of the `join(", ")` used below. // Most probably, a `listFormat` or similar wrapper should live in src/i18n.js or so. @@ -66,10 +62,10 @@ const DeletionsInfo = ({ actions, devicesManager }) => { return ( 0} + condition={systems.length > 0} then={

- {_("Including the deletion of")} {deletedSystems.join(", ")} + {_("Including the deletion of")} {systems.join(", ")}

} /> @@ -228,7 +224,10 @@ const SectionContent = ({ system, staging, actions, errors }) => { return ( <> - + a.delete && !a.subvol)} + systems={devicesManager.deletedSystems()} + /> ); From 5ad2b50b54146838468869e9090464d0dded8877 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, 14 Mar 2024 09:29:42 +0000 Subject: [PATCH 32/39] web: Fixt type --- web/src/components/storage/ProposalResultSection.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 1465fa8026..19050936d9 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -42,7 +42,7 @@ import { ProposalActionsDialog } from "~/components/storage"; * @component * * @param {object} props - * @param {object[]} props.actions + * @param {Action[]} props.actions * @param {string[]} props.systems */ const DeletionsInfo = ({ actions, systems }) => { From 6b6fe0e9686ff646cc1592dd33b3beb768bebfec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 14 Mar 2024 09:44:29 +0000 Subject: [PATCH 33/39] web: Reorder Result section content And improve a bit the warning. Now the actions summary is placed at the top of the section, near to the description which already hints about the amount actions when there is a valid proposal. --- .../components/storage/ProposalResultSection.jsx | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 19050936d9..17fe5c3044 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -50,9 +50,10 @@ const DeletionsInfo = ({ actions, systems }) => { if (total === 0) return; + // TRANSLATORS: %d will be replaced by the amount of destructive actions const warningTitle = sprintf(n_( - "There are %d destructive planned action", - "There are %d destructive planned actions", + "There is %d destructive action planned", + "There are %d destructive actions planned", total ), total); @@ -223,12 +224,12 @@ const SectionContent = ({ system, staging, actions, errors }) => { return ( <> - a.delete && !a.subvol)} systems={devicesManager.deletedSystems()} /> + ); }; @@ -253,12 +254,18 @@ export default function ProposalResultSection({ }) { if (isLoading) errors = []; + const description = sprintf( + // TRANSLATORS: %d will be replaced by the amount actions + _("During installation, %d actions will be performed to configure the system as displayed below"), + actions.length + ); + return (
From cacaec27cd8034f1eb4c1811cf33b487fc641974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 14 Mar 2024 10:02:19 +0000 Subject: [PATCH 34/39] web: Display section description only if not loading --- web/src/components/storage/ProposalResultSection.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 17fe5c3044..fd07fc41eb 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -265,7 +265,7 @@ export default function ProposalResultSection({ // TRANSLATORS: The storage "Result" section's title title={_("Result")} // TRANSLATORS: The storage "Result" section's description - description={errors.length === 0 && description} + description={!isLoading && errors.length === 0 && description} id="storage-result" errors={errors} > From ed6f01caaec7298dbd1f8ebe9381c810e3879a75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 14 Mar 2024 17:22:36 +0000 Subject: [PATCH 35/39] web: CSS hacks and adjustments --- web/src/assets/styles/blocks.scss | 51 ++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index e29ef42181..effe852e42 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -439,7 +439,7 @@ table[data-type="agama/tree-table"] { } table.proposal-result { - tr.dimmed-row { + tr.dimmed-row { background-color: #fff; opacity: 0.8; background: repeating-linear-gradient( -45deg, #fcfcff, #fcfcff 3px, #fff 3px, #fff 10px ); @@ -448,33 +448,48 @@ table.proposal-result { color: var(--color-gray-dimmed); padding-block: 0; } + } - @media (width > 768px) { - th.details-column { - padding-inline-start: calc(60px + var(--spacer-smaller) * 2); - } + /** Temporary hack because the collapse/expand callback was not given to the + * tree table **/ +th button { + background: red; + display: none; +} - td.details-column { - display: grid; - gap: var(--spacer-smaller); - grid-template-columns: 60px 1fr; +tbody th .pf-v5-c-table__tree-view-main { + padding-inline-start: var(--spacer-normal); + flex-direction: row-reverse; + cursor: auto; +} +/** End of temporary hack */ - :first-child { - text-align: end; - } - } +@media (width > 768px) { + th.details-column { + padding-inline-start: calc(60px + var(--spacer-smaller) * 2); + } - th.sizes-column, - td.sizes-column { + td.details-column { + display: grid; + gap: var(--spacer-smaller); + grid-template-columns: 60px 1fr; + + :first-child { text-align: end; + } + } - div.split { - justify-content: flex-end; - } + th.sizes-column, + td.sizes-column { + text-align: end; + + div.split { + justify-content: flex-end; } } } +} // compact lists in popover .pf-v5-c-popover li + li { From 436ea2129c368c552577a4e7876bb2f80ac00bd9 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, 15 Mar 2024 13:03:37 +0000 Subject: [PATCH 36/39] service: Fix tests - See https://github.com/yast/yast-storage-ng/pull/1375 --- service/test/fixtures/trivial_lvm.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/test/fixtures/trivial_lvm.yml b/service/test/fixtures/trivial_lvm.yml index 86f3fc904e..ff7aec2699 100644 --- a/service/test/fixtures/trivial_lvm.yml +++ b/service/test/fixtures/trivial_lvm.yml @@ -6,7 +6,7 @@ partitions: - partition: - size: unlimited + size: 100 GiB name: /dev/sda1 id: lvm @@ -18,7 +18,7 @@ lvm_lvs: - lvm_lv: - size: unlimited + size: 100 GiB lv_name: lv1 file_system: btrfs mount_point: / From e0731fdc577f8907a661211d7f2058ab7b6b03ec 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, 15 Mar 2024 13:37:42 +0000 Subject: [PATCH 37/39] web: Improve detection of deleted devices and systems --- web/src/components/storage/DevicesManager.js | 20 ++++++++++----- .../components/storage/DevicesManager.test.js | 25 ++++++++++++++----- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/web/src/components/storage/DevicesManager.js b/web/src/components/storage/DevicesManager.js index 2f836fe86c..5f70318564 100644 --- a/web/src/components/storage/DevicesManager.js +++ b/web/src/components/storage/DevicesManager.js @@ -153,11 +153,7 @@ export default class DevicesManager { * @returns {StorageDevice[]} */ deletedDevices() { - const sids = this.actions - .filter(a => a.delete) - .map(a => a.device); - const devices = sids.map(sid => this.systemDevice(sid)); - return compact(devices); + return this.#deleteActionsDevice().filter(d => !d.isDrive); } /** @@ -166,7 +162,8 @@ export default class DevicesManager { * @returns {string[]} */ deletedSystems() { - const systems = this.deletedDevices() + const systems = this.#deleteActionsDevice() + .filter(d => !d.partitionTable) .map(d => d.systems) .flat(); return compact(systems); @@ -204,4 +201,15 @@ export default class DevicesManager { partitions.find(p => this.#isUsed(p)) !== undefined || lvmLvs.find(l => this.#isUsed(l)) !== undefined; } + + /** + * @returns {StorageDevice[]} + */ + #deleteActionsDevice() { + const sids = this.actions + .filter(a => a.delete) + .map(a => a.device); + const devices = sids.map(sid => this.systemDevice(sid)); + return compact(devices); + } } diff --git a/web/src/components/storage/DevicesManager.test.js b/web/src/components/storage/DevicesManager.test.js index 53ff712927..23636fc106 100644 --- a/web/src/components/storage/DevicesManager.test.js +++ b/web/src/components/storage/DevicesManager.test.js @@ -374,14 +374,16 @@ describe("deletedDevices", () => { { sid: 60 }, { sid: 62 }, { sid: 63 }, - { sid: 64 } + { sid: 64 }, + { sid: 65, isDrive: true } ]; actions = [ { device: 60, delete: true }, // This device does not exist in system. { device: 61, delete: true }, { device: 62, delete: false }, - { device: 63, delete: true } + { device: 63, delete: true }, + { device: 65, delete: true } ]; }); @@ -398,23 +400,34 @@ describe("deletedSystems", () => { system = [ { sid: 60, systems: ["Windows XP"] }, { sid: 62, systems: ["Ubuntu"] }, - { sid: 63, systems: ["openSUSE Leap"] }, - { sid: 64 } + { + sid: 63, + systems: ["openSUSE Leap", "openSUSE Tumbleweed"], + partitionTable: { + partitions: [{ sid: 65 }, { sid: 66 }] + } + }, + { sid: 64 }, + { sid: 65, systems: ["openSUSE Leap"] }, + { sid: 66, systems: ["openSUSE Tumbleweed"] } ]; actions = [ { device: 60, delete: true }, // This device does not exist in system. { device: 61, delete: true }, { device: 62, delete: false }, - { device: 63, delete: true } + { device: 63, delete: true }, + { device: 65, delete: true }, + { device: 66, delete: true } ]; }); it("includes all deleted systems", () => { const manager = new DevicesManager(system, staging, actions); const systems = manager.deletedSystems(); - expect(systems.length).toEqual(2); + expect(systems.length).toEqual(3); expect(systems).toContain("Windows XP"); expect(systems).toContain("openSUSE Leap"); + expect(systems).toContain("openSUSE Tumbleweed"); }); }); From 195e962b1b99f3ab9e3ee0a3ed1223e40f9c8ca9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 15 Mar 2024 13:55:20 +0000 Subject: [PATCH 38/39] web: Fixes and adjustments --- web/src/assets/styles/blocks.scss | 111 +++++++++++++----- .../components/storage/ProposalPage.test.jsx | 1 - .../storage/ProposalResultSection.jsx | 24 ++-- 3 files changed, 95 insertions(+), 41 deletions(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index effe852e42..43b336bf31 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -426,15 +426,58 @@ ul[data-type="agama/list"][role="grid"] { table[data-type="agama/tree-table"] { th:first-child { - width: 1%; + block-size: fit-content; padding-inline-end: var(--spacer-normal); } + th.fit-content { + block-size: fit-content; + overflow: visible; + text-overflow: unset; + } + + /** + * Temporary PF/Table overrides for small devices + **/ @media (width <= 768px) { - td:empty, - td div:empty { + &.pf-m-tree-view-grid-md.pf-v5-c-table tr[aria-level="1"] td { + padding-inline-start: var(--spacer-medium); + } + + &.pf-m-tree-view-grid-md.pf-v5-c-table tr[aria-level="2"] th { + padding-inline-start: calc(var(--spacer-large) * 1.1); + } + + &.pf-m-tree-view-grid-md.pf-v5-c-table tr[aria-level="2"] td { + padding-inline-start: calc(var(--spacer-large) * 1.4); + } + + &.pf-m-tree-view-grid-md.pf-v5-c-table tr:where(.pf-v5-c-table__tr).pf-m-tree-view-details-expanded { + padding-block-end: var(--spacer-smaller); + } + + &.pf-m-tree-view-grid-md.pf-v5-c-table tr:where(.pf-v5-c-table__tr) td:empty, + &.pf-m-tree-view-grid-md.pf-v5-c-table tr:where(.pf-v5-c-table__tr) td *:empty, + &.pf-m-tree-view-grid-md.pf-v5-c-table tr:where(.pf-v5-c-table__tr) td:has(> *:empty) { display: none; } + + &.pf-m-tree-view-grid-md.pf-v5-c-table tr:where(.pf-v5-c-table__tr) td:has(> *:not(:empty)) { + display: inherit; + } + + &.pf-m-tree-view-grid-md.pf-v5-c-table tbody:where(.pf-v5-c-table__tbody) tr:where(.pf-v5-c-table__tr)::before { + inset-inline-start: 0; + } + + &.pf-v5-c-table.pf-m-compact tr:where(.pf-v5-c-table__tr):not(.pf-v5-c-table__expandable-row) > *:last-child { + padding-inline-end: 8px; + } + + tbody th:first-child { + font-size: var(--fs-large); + padding-block-start: var(--spacer-small); + } } } @@ -451,45 +494,51 @@ table.proposal-result { } - /** Temporary hack because the collapse/expand callback was not given to the - * tree table **/ -th button { - background: red; - display: none; -} + /** + * Temporary hack because the collapse/expand callback was not given to the + * tree table + **/ + th button { + display: none; + } -tbody th .pf-v5-c-table__tree-view-main { - padding-inline-start: var(--spacer-normal); - flex-direction: row-reverse; - cursor: auto; -} -/** End of temporary hack */ + tbody th .pf-v5-c-table__tree-view-main { + padding-inline-start: var(--pf-v5-c-table--m-compact--cell--first-last-child--PaddingLeft); + cursor: auto; + } -@media (width > 768px) { - th.details-column { - padding-inline-start: calc(60px + var(--spacer-smaller) * 2); + tbody tr[aria-level="2"] th .pf-v5-c-table__tree-view-main { + padding-inline-start: calc( + var(--pf-v5-c-table--m-compact--cell--first-last-child--PaddingLeft) + var(--spacer-large) + ); } + /** End of temporary hack */ - td.details-column { - display: grid; - gap: var(--spacer-smaller); - grid-template-columns: 60px 1fr; + @media (width > 768px) { + th.details-column { + padding-inline-start: calc(60px + var(--spacer-smaller) * 2); + } - :first-child { - text-align: end; + td.details-column { + display: grid; + gap: var(--spacer-smaller); + grid-template-columns: 60px 1fr; + + :first-child { + text-align: end; + } } - } - th.sizes-column, - td.sizes-column { - text-align: end; + th.sizes-column, + td.sizes-column { + text-align: end; - div.split { - justify-content: flex-end; + div.split { + justify-content: flex-end; + } } } } -} // compact lists in popover .pf-v5-c-popover li + li { diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 3ab0ea09ec..7bd4f44a31 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -76,7 +76,6 @@ const vdb = { size: 1e+6 }; -// FIXME: needed to be reviewed/adapted after latest changes const storageMock = { probe: jest.fn().mockResolvedValue(0), proposal: { diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index fd07fc41eb..5def2aecb9 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -66,7 +66,7 @@ const DeletionsInfo = ({ actions, systems }) => { condition={systems.length > 0} then={

- {_("Including the deletion of")} {systems.join(", ")} + {_("Affecting %s")} {systems.join(", ")}

} /> @@ -160,7 +160,11 @@ const DevicesTreeTable = ({ devicesManager }) => { return ( - {sprintf(_("Resized %s"), deviceSize(devicesManager.shrinkSize(item)))} + { + // TRANSLATORS: a label to show how much a device was resized. %s will be + // replaced with such a size, including the unit. E.g., 508 MiB + sprintf(_("Resized %s"), deviceSize(devicesManager.shrinkSize(item))) + } ); }; @@ -180,7 +184,7 @@ const DevicesTreeTable = ({ devicesManager }) => { Date: Mon, 18 Mar 2024 08:59:09 +0000 Subject: [PATCH 39/39] web: improve comments for translators And also delete a leftover interpolation. --- web/src/components/storage/ProposalResultSection.jsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/web/src/components/storage/ProposalResultSection.jsx b/web/src/components/storage/ProposalResultSection.jsx index 5def2aecb9..3759c25740 100644 --- a/web/src/components/storage/ProposalResultSection.jsx +++ b/web/src/components/storage/ProposalResultSection.jsx @@ -66,7 +66,11 @@ const DeletionsInfo = ({ actions, systems }) => { condition={systems.length > 0} then={

- {_("Affecting %s")} {systems.join(", ")} + { + // TRANSLATORS: This is part of a sentence to hint the user about affected systems. + // Eg. "Affecting Windows 11, openSUSE Leap 15, and Ubuntu 22.04" + } + {_("Affecting")} {systems.join(", ")}

} />