Skip to content

web: Adapt the storage overview to stop using ProposalResult #1757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ancorgs merged 3 commits into from
Nov 14, 2024
Merged

web: Adapt the storage overview to stop using ProposalResult #1757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dbe8070
web: Adapt the storage overview to stop using ProposalResult
ancorgs Nov 13, 2024
89db30e
web: Improvements from code review
ancorgs Nov 14, 2024
7a9aede
Fix typo
ancorgs Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions web/src/components/overview/OverviewPage.tsx
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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";