Skip to content

Commit

Permalink
web: Adapt the storage overview to stop using ProposalResult (#1757)
Browse files Browse the repository at this point in the history 
## Problem

As a first step to start using `AgamaProposal` instead of
`MinGuidedProposal` at the web UI, we disabled the storage summary at
the overview page (commenting out the usage of the component
`Overview/StorageSection`.

## Solution

This enables the component back, but now reading from the correct source
of information.

## Screenshots

The "storage" entry is back home. See following screenshot.


![storage_section](https://github.com/user-attachments/assets/70440551-25c8-4b52-b213-5aeaec45cc2f)
  • Loading branch information
@ancorgs
ancorgs authored Nov 14, 2024
2 parents 2fcd9ab + 7a9aede commit 42db91c
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 141 deletions.
2 changes: 2 additions & 0 deletions web/src/components/overview/OverviewPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
import { Link } from "react-router-dom";
import { Page } from "~/components/core";
import L10nSection from "./L10nSection";
import StorageSection from "./StorageSection";
import SoftwareSection from "./SoftwareSection";
import { _ } from "~/i18n";
import { useAllIssues } from "~/queries/issues";
Expand Down Expand Up @@ -100,6 +101,7 @@ const OverviewSection = () => (
>
<Stack hasGutter>
<L10nSection />
<StorageSection />
<SoftwareSection />
</Stack>
</Page.Section>
Expand Down
56 changes: 39 additions & 17 deletions web/src/components/overview/StorageSection.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,36 @@ import React from "react";
import { screen } from "@testing-library/react";
import { plainRender } from "~/test-utils";
import { StorageSection } from "~/components/overview";
import * as ConfigModel from "~/storage/model/config";

const mockAvailableDevices = [
const mockDevices = [
{ name: "/dev/sda", size: 536870912000 },
{ name: "/dev/sdb", size: 697932185600 },
];

const mockResultSettings = { target: "disk", targetDevice: "/dev/sda", spacePolicy: "delete" };
const mockConfig = { devices: [] as ConfigModel.Device[] };

jest.mock("~/queries/storage", () => ({
...jest.requireActual("~/queries/storage"),
useAvailableDevices: () => mockAvailableDevices,
useProposalResult: () => ({
settings: mockResultSettings,
actions: [],
}),
useDevices: () => mockDevices,
useConfigDevices: () => mockConfig.devices,
}));

describe("when there is a proposal", () => {
describe("when the configuration does not include any device", () => {
beforeEach(() => {
mockResultSettings.target = "disk";
mockResultSettings.spacePolicy = "delete";
mockConfig.devices = [];
});

it("indicates that a device is not selected", async () => {
plainRender(<StorageSection />);

await screen.findByText(/No device selected/);
});
});

describe("when the configuration contains one drive", () => {
beforeEach(() => {
mockConfig.devices = [{ name: "/dev/sda", spacePolicy: "delete" }];
});

it("renders the proposal summary", async () => {
Expand All @@ -57,9 +66,7 @@ describe("when there is a proposal", () => {

describe("and the space policy is set to 'resize'", () => {
beforeEach(() => {
// const result = { settings: { spacePolicy: "resize", targetDevice: "/dev/sda" } };
mockResultSettings.spacePolicy = "resize";
mockResultSettings.targetDevice = "/dev/sda";
mockConfig.devices[0].spacePolicy = "resize";
});

it("indicates that partitions may be shrunk", async () => {
Expand All @@ -71,8 +78,7 @@ describe("when there is a proposal", () => {

describe("and the space policy is set to 'keep'", () => {
beforeEach(() => {
mockResultSettings.spacePolicy = "keep";
mockResultSettings.targetDevice = "/dev/sda";
mockConfig.devices[0].spacePolicy = "keep";
});

it("indicates that partitions will be kept", async () => {
Expand All @@ -82,9 +88,9 @@ describe("when there is a proposal", () => {
});
});

describe("and there is no target device", () => {
describe("and the drive matches no disk", () => {
beforeEach(() => {
mockResultSettings.targetDevice = "";
mockConfig.devices[0].name = null;
});

it("indicates that a device is not selected", async () => {
Expand All @@ -94,3 +100,19 @@ describe("when there is a proposal", () => {
});
});
});

describe("when the configuration contains several drives", () => {
beforeEach(() => {
mockConfig.devices = [
{ name: "/dev/sda", spacePolicy: "delete" },
{ name: "/dev/sdb", spacePolicy: "delete" },
];
});

it("renders the proposal summary", async () => {
plainRender(<StorageSection />);

await screen.findByText(/Install using several devices/);
await screen.findByText(/and deleting all its content/);
});
});
175 changes: 51 additions & 124 deletions web/src/components/overview/StorageSection.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,75 +25,8 @@ import { Text, TextContent, TextVariants } from "@patternfly/react-core";
import { deviceLabel } from "~/components/storage/utils";
import { Em } from "~/components/core";
import { _ } from "~/i18n";
import { useAvailableDevices, useProposalResult } from "~/queries/storage";
import { ProposalTarget } from "~/types/storage";

/**
* Build a translated summary string for installing on an LVM with multiple
* physical partitions/disks
* @param policy - Find space policy
* @returns Translated description
*/
const msgLvmMultipleDisks = (policy: string): string => {
switch (policy) {
case "resize":
// TRANSLATORS: installing on an LVM with multiple physical partitions/disks
return _(
"Install in a new Logical Volume Manager (LVM) volume group shrinking existing partitions at the underlying devices as needed",
);
case "keep":
// TRANSLATORS: installing on an LVM with multiple physical partitions/disks
return _(
"Install in a new Logical Volume Manager (LVM) volume group without modifying the partitions at the underlying devices",
);
case "delete":
// TRANSLATORS: installing on an LVM with multiple physical partitions/disks
return _(
"Install in a new Logical Volume Manager (LVM) volume group deleting all the content of the underlying devices",
);
case "custom":
// TRANSLATORS: installing on an LVM with multiple physical partitions/disks
return _(
"Install in a new Logical Volume Manager (LVM) volume group using a custom strategy to find the needed space at the underlying devices",
);
}
};

/**
* Build a translated summary string for installing on an LVM with a single
* physical partition/disk
* @param policy - Find space policy
* @returns Translated description with %s placeholder for the device
* name
*/
const msgLvmSingleDisk = (policy: string): string => {
switch (policy) {
case "resize":
// TRANSLATORS: installing on an LVM with a single physical partition/disk,
// %s will be replaced by a device name and its size (eg. "/dev/sda, 20 GiB")
return _(
"Install in a new Logical Volume Manager (LVM) volume group on %s shrinking existing partitions as needed",
);
case "keep":
// TRANSLATORS: installing on an LVM with a single physical partition/disk,
// %s will be replaced by a device name and its size (eg. "/dev/sda, 20 GiB")
return _(
"Install in a new Logical Volume Manager (LVM) volume group on %s without modifying existing partitions",
);
case "delete":
// TRANSLATORS: installing on an LVM with a single physical partition/disk,
// %s will be replaced by a device name and its size (eg. "/dev/sda, 20 GiB")
return _(
"Install in a new Logical Volume Manager (LVM) volume group on %s deleting all its content",
);
case "custom":
// TRANSLATORS: installing on an LVM with a single physical partition/disk,
// %s will be replaced by a device name and its size (eg. "/dev/sda, 20 GiB")
return _(
"Install in a new Logical Volume Manager (LVM) volume group on %s using a custom strategy to find the needed space",
);
}
};
import { useDevices, useConfigDevices } from "~/queries/storage";
import * as ConfigModel from "~/storage/model/config";

const Content = ({ children }) => (
<TextContent>
Expand All @@ -105,80 +38,74 @@ const Content = ({ children }) => (
/**
* Text explaining the storage proposal
*
* FIXME: this needs to be basically rewritten. See
* https://github.com/openSUSE/agama/discussions/778#discussioncomment-7715244
*
* @param {object} props
* @param {Proposal} props.proposal
* TODO: The current implementation assumes there are only drives and no other kind of devices like
* LVM volume groups or MD raids. Support for more cases (like LVM installation) will be added as
* the rest of the interface is also adapted.
*/
export default function StorageSection() {
const availableDevices = useAvailableDevices();
const result = useProposalResult();
const drives = useConfigDevices();
const devices = useDevices("system", { suspense: true });

if (result === undefined) return;

const label = (deviceName) => {
const device = availableDevices.find((d) => d.name === deviceName);
return device ? deviceLabel(device) : deviceName;
const label = (drive) => {
const device = devices.find((d) => d.name === drive.name);
return device ? deviceLabel(device) : drive.name;
};

if (result.settings.target === ProposalTarget.NEW_LVM_VG) {
const pvDevices = result.settings.targetPVDevices;

if (pvDevices.length > 1) {
return (
<Content>
<span>{msgLvmMultipleDisks(result.settings.spacePolicy)}</span>
</Content>
);
} else {
const [msg1, msg2] = msgLvmSingleDisk(result.settings.spacePolicy).split("%s");

return (
<Content>
<Text>
<span>{msg1}</span>
<Em>{label(pvDevices[0])}</Em>
<span>{msg2}</span>
</Text>
</Content>
);
}
}

const targetDevice = result.settings.targetDevice;
if (!targetDevice) return <Text>{_("No device selected yet")}</Text>;

const fullMsg = (policy: string): string => {
switch (policy) {
const msgSingleDisk = (drive: ConfigModel.Device): string => {
switch (drive.spacePolicy) {
case "resize":
// TRANSLATORS: %s will be replaced by the device name and its size,
// example: "/dev/sda, 20 GiB"
return _("Install using device %s shrinking existing partitions as needed");
return _("Install using device %s shrinking existing partitions as needed.");
case "keep":
// TRANSLATORS: %s will be replaced by the device name and its size,
// example: "/dev/sda, 20 GiB"
return _("Install using device %s without modifying existing partitions");
return _("Install using device %s without modifying existing partitions.");
case "delete":
// TRANSLATORS: %s will be replaced by the device name and its size,
// example: "/dev/sda, 20 GiB"
return _("Install using device %s and deleting all its content");
return _("Install using device %s and deleting all its content.");
}

// TRANSLATORS: %s will be replaced by the device name and its size,
// example: "/dev/sda, 20 GiB"
return _("Install using device %s with a custom strategy to find the needed space");
return _("Install using device %s with a custom strategy to find the needed space.");
};

const [msg1, msg2] = fullMsg(result.settings.spacePolicy).split("%s");
const msgMultipleDisks = (drives: ConfigModel.Device[]): string => {
if (drives.every((d) => d.spacePolicy === drives[0].spacePolicy)) {
switch (drives[0].spacePolicy) {
case "resize":
return _("Install using several devices shrinking existing partitions as needed.");
case "keep":
return _("Install using several devices without modifying existing partitions.");
case "delete":
return _("Install using several devices and deleting all its content.");
}
}

return (
<Content>
<Text>
{msg1}
<Em>{label(targetDevice)}</Em>
{msg2}
</Text>
</Content>
);
return _("Install using several devices with a custom strategy to find the needed space.");
};

if (drives.length === 0) return <Text>{_("No device selected yet")}</Text>;

if (drives.length > 1) {
return (
<Content>
<span>{msgMultipleDisks(drives)}</span>
</Content>
);
} else {
const [msg1, msg2] = msgSingleDisk(drives[0]).split("%s");

return (
<Content>
<Text>
<span>{msg1}</span>
<Em>{label(drives[0])}</Em>
<span>{msg2}</span>
</Text>
</Content>
);
}
}
1 change: 1 addition & 0 deletions web/src/components/overview/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@

export { default as OverviewPage } from "./OverviewPage";
export { default as L10nSection } from "./L10nSection";
export { default as StorageSection } from "./StorageSection";
export { default as SoftwareSection } from "./SoftwareSection";

0 comments on commit 42db91c

Please sign in to comment.