diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index a197659bd0..ebc4711a3b 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon May 5 14:50:48 UTC 2025 - David Diaz + +- Improve storage proposal warning to mention mount paths when + logical volume space allocation fails (gh#agama-project/agama#2323). + ------------------------------------------------------------------- Tue Apr 22 14:14:53 UTC 2025 - Imobach Gonzalez Sosa diff --git a/web/src/components/storage/ProposalFailedInfo.test.tsx b/web/src/components/storage/ProposalFailedInfo.test.tsx new file mode 100644 index 0000000000..80e648210b --- /dev/null +++ b/web/src/components/storage/ProposalFailedInfo.test.tsx @@ -0,0 +1,216 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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 { installerRender } from "~/test-utils"; +import ProposalFailedInfo from "./ProposalFailedInfo"; +import { LogicalVolume } from "~/types/storage/data"; +import { Issue, IssueSeverity, IssueSource } from "~/types/issues"; +import { apiModel } from "~/api/storage/types"; + +const mockUseConfigErrorsFn = jest.fn(); +let mockUseIssues = []; + +const configError: Issue = { + description: "Config error", + kind: "storage", + details: "", + source: 2, + severity: 1, +}; + +const storageIssue: Issue = { + description: "Fake Storage Issue", + details: "", + kind: "storage_issue", + source: IssueSource.Unknown, + severity: IssueSeverity.Error, +}; + +const mockApiModel: apiModel.Config = { + boot: { + configure: true, + device: { + default: true, + name: "/dev/vdb", + }, + }, + drives: [ + { + name: "/dev/vdb", + spacePolicy: "delete", + partitions: [ + { + name: "/dev/vdb1", + size: { + default: true, + min: 6430916608, + max: 6430916608, + }, + delete: true, + deleteIfNeeded: false, + resize: false, + resizeIfNeeded: false, + }, + { + name: "/dev/vdb2", + size: { + default: true, + min: 4305436160, + max: 4305436160, + }, + delete: true, + deleteIfNeeded: false, + resize: false, + resizeIfNeeded: false, + }, + ], + }, + { + name: "/dev/vdc", + spacePolicy: "delete", + partitions: [ + { + mountPath: "/documents", + filesystem: { + reuse: false, + default: false, + type: "xfs", + label: "", + }, + size: { + default: false, + min: 136365211648, + }, + delete: false, + deleteIfNeeded: false, + resize: false, + resizeIfNeeded: false, + }, + ], + }, + ], + volumeGroups: [ + { + vgName: "system", + targetDevices: ["/dev/vdb"], + logicalVolumes: [ + { + lvName: "root", + mountPath: "/", + filesystem: { + reuse: false, + default: true, + type: "btrfs", + snapshots: true, + }, + size: { + default: true, + min: 13421772800, + }, + }, + { + lvName: "swap", + mountPath: "swap", + filesystem: { + reuse: false, + default: true, + type: "swap", + }, + size: { + default: true, + min: 1073741824, + max: 2147483648, + }, + }, + ], + }, + ], +}; + +jest.mock("~/hooks/storage/api-model", () => ({ + ...jest.requireActual("~/hooks/storage/api-model"), + useApiModel: () => mockApiModel, +})); + +jest.mock("~/queries/issues", () => ({ + ...jest.requireActual("~/queries/issues"), + useConfigErrors: () => mockUseConfigErrorsFn(), + useIssues: () => mockUseIssues, +})); + +// eslint-disable-next-line +const fakeLogicalVolume: LogicalVolume = { + // @ts-expect-error: The #name property is used to distinguish new "devices" + // in the API model, but it is not yet exposed for logical volumes since they + // are currently not reusable. This directive exists to ensure developers + // don't overlook updating the ProposalFailedInfo component in the future, + // when logical volumes become reusable and the #name property is exposed. See + // the FIXME in the ProposalFailedInfo component for more context. + name: "Reusable LV", + lvName: "helpful", +}; + +describe("ProposalFailedInfo", () => { + beforeEach(() => { + mockUseIssues = []; + mockUseConfigErrorsFn.mockReturnValue([]); + }); + + describe("when proposal can't be created due to configuration errors", () => { + beforeEach(() => { + mockUseConfigErrorsFn.mockReturnValue([configError]); + }); + + it("renders nothing", () => { + const { container } = installerRender(); + expect(container).toBeEmptyDOMElement(); + }); + }); + + describe("when proposal is valid", () => { + describe("and has no errors", () => { + beforeEach(() => { + mockUseIssues = []; + }); + + it("renders nothing", () => { + const { container } = installerRender(); + expect(container).toBeEmptyDOMElement(); + }); + }); + + describe("but has errors", () => { + beforeEach(() => { + mockUseIssues = [storageIssue]; + }); + + it("renders a warning alert with hints about the failure", () => { + installerRender(); + screen.getByText("Warning alert:"); + screen.getByText("Failed to calculate a storage layout"); + screen.getByText(/It is not possible to allocate space for/); + }); + }); + }); +}); diff --git a/web/src/components/storage/ProposalFailedInfo.tsx b/web/src/components/storage/ProposalFailedInfo.tsx index 41b8c1e37e..8620d65752 100644 --- a/web/src/components/storage/ProposalFailedInfo.tsx +++ b/web/src/components/storage/ProposalFailedInfo.tsx @@ -22,17 +22,32 @@ import React from "react"; import { Alert, Content } from "@patternfly/react-core"; -import { _, n_, formatList } from "~/i18n"; -import { useIssues, useConfigErrors } from "~/queries/issues"; -import { useConfigModel } from "~/queries/storage/config-model"; import { IssueSeverity } from "~/types/issues"; +import { useApiModel } from "~/hooks/storage/api-model"; +import { useIssues, useConfigErrors } from "~/queries/issues"; import * as partitionUtils from "~/components/storage/utils/partition"; +import { _, formatList } from "~/i18n"; import { sprintf } from "sprintf-js"; -function Description({ partitions, booting }) { +const Description = () => { + const model = useApiModel({ suspense: true }); + const partitions = model.drives.flatMap((d) => d.partitions || []); + const logicalVolumes = model.volumeGroups.flatMap((vg) => vg.logicalVolumes || []); + const newPartitions = partitions.filter((p) => !p.name); - if (!newPartitions.length) { + // FIXME: Currently, it's not possible to reuse a logical volume, so all + // volumes are treated as new. This code cannot be made future-proof due to an + // internal decision not to expose unused properties, even though "#name" is + // used to infer whether a "device" is new or not. + // const newLogicalVolumes = logicalVolumes.filter((lv) => !lv.name); + + const isBootConfigured = !!model.boot?.configure; + const mountPaths = [newPartitions, logicalVolumes] + .flat() + .map((d) => partitionUtils.pathWithSize(d)); + + if (mountPaths.length === 0) { return ( {_( @@ -42,59 +57,46 @@ function Description({ partitions, booting }) { ); } - const mountPaths = newPartitions.map((p) => partitionUtils.pathWithSize(p)); - const msg1 = booting - ? sprintf( - // TRANSLATORS: %s is a list of formatted mount points with a partition size like - // '"/" (at least 10 GiB), "/var" (20 GiB) and "swap" (2 GiB)' - // (or a single mount point in the singular case). - n_( - "It is not possible to allocate the requested partitions for booting and for %s.", - "It is not possible to allocate the requested partitions for booting, %s.", - mountPaths.length, - ), - formatList(mountPaths), - ) - : sprintf( - // TRANSLATORS: %s is a list of formatted mount points with a partition size like - // '"/" (at least 10 GiB), "/var" (20 GiB) and "swap" (2 GiB)' - // (or a single mount point in the singular case). - n_( - "It is not possible to allocate the requested partition for %s.", - "It is not possible to allocate the requested partitions for %s.", - mountPaths.length, - ), - formatList(mountPaths), - ); - return ( <> - {msg1} + + {sprintf( + isBootConfigured + ? // TRANSLATORS: %s is a list of formatted mount points with a partition size like + // '"/" (at least 10 GiB), "/var" (20 GiB) and "swap" (2 GiB)' + _("It is not possible to allocate space for the boot partition and for %s.") + : // TRANSLATORS: %s is a list of formatted mount points with a partition size like + // '"/" (at least 10 GiB), "/var" (20 GiB) and "swap" (2 GiB)' + _("It is not possible to allocate space for %s."), + formatList(mountPaths), + )} + {_("Adjust the settings below to make the new system fit into the available space.")} ); -} +}; /** - * Information about a failed storage proposal + * Displays information to help users understand why a storage proposal + * could not be generated with the current configuration. * + * Renders nothing if: + * - The proposal could not be generated at all (known by the presence of + * configuration errors in the storage scope) + * - The generated proposal contains no errors. */ export default function ProposalFailedInfo() { const configErrors = useConfigErrors("storage"); const errors = useIssues("storage").filter((s) => s.severity === IssueSeverity.Error); - const model = useConfigModel({ suspense: true }); - - if (configErrors.length) return; - if (!errors.length) return; - const modelPartitions = model.drives.flatMap((d) => d.partitions || []); - const booting = !!model.boot?.configure; + if (configErrors.length !== 0) return; + if (errors.length === 0) return; return ( - + ); }