feat(web): improve storage/ProposalFailedInfo

web/package/agama-web-ui.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Mon May  5 14:50:48 UTC 2025 - David Diaz <dgonzalez@suse.com>
+
+- 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 <igonzalezsosa@suse.com>
 

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(<ProposalFailedInfo />);
+      expect(container).toBeEmptyDOMElement();
+    });
+  });
+
+  describe("when proposal is valid", () => {
+    describe("and has no errors", () => {
+      beforeEach(() => {
+        mockUseIssues = [];
+      });
+
+      it("renders nothing", () => {
+        const { container } = installerRender(<ProposalFailedInfo />);
+        expect(container).toBeEmptyDOMElement();
+      });
+    });
+
+    describe("but has errors", () => {
+      beforeEach(() => {
+        mockUseIssues = [storageIssue];
+      });
+
+      it("renders a warning alert with hints about the failure", () => {
+        installerRender(<ProposalFailedInfo />);
+        screen.getByText("Warning alert:");
+        screen.getByText("Failed to calculate a storage layout");
+        screen.getByText(/It is not possible to allocate space for/);
+      });
+    });
+  });
+});

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 (
       <Content component="p">
         {_(
@@ -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 (
     <>
-      <Content component="p">{msg1}</Content>
+      <Content component="p">
+        {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),
+        )}
+      </Content>
       <Content component="p">
         {_("Adjust the settings below to make the new system fit into the available space.")}
       </Content>
     </>
   );
-}
+};
 
 /**
- * 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 (
     <Alert variant="warning" title={_("Failed to calculate a storage layout")}>
-      <Description partitions={modelPartitions} booting={booting} />
+      <Description />
     </Alert>
   );
 }