Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions web/src/components/storage/DevicesManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,15 @@ export default class DevicesManager {
* Disk devices and LVM volume groups used for the installation.
* @method
*
* @note The used devices are extracted from the actions.
* @note The used devices are extracted from the actions, but the optional argument
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: DeviceManager is one of a few files that still pending to be migrated to TypeScript. Just saying 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look! It's a three-headed monkey!

* can be used to expand the list if some devices must be included despite not
* being affected by the actions.
*
* @param {string[]} knownNames - names of devices already known to be used, even if
* there are no actions on them
* @returns {StorageDevice[]}
*/
usedDevices() {
usedDevices(knownNames = []) {
const isTarget = (device) => device.isDrive || ["md", "lvmVg"].includes(device.type);

// Check in system devices to detect removals.
Expand All @@ -151,7 +155,7 @@ export default class DevicesManager {

const sids = targetSystem
.concat(targetStaging)
.filter((d) => this.#isUsed(d))
.filter((d) => this.#isUsed(d) || knownNames.includes(d.name))
.map((d) => d.sid);

return compact(uniq(sids).map((sid) => this.stagingDevice(sid)));
Expand Down
36 changes: 30 additions & 6 deletions web/src/components/storage/DevicesManager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,24 +301,27 @@ 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: 61, isDrive: true, name: "/dev/sda" },
{ sid: 62, isDrive: true, name: "/dev/sdb", partitionTable: { partitions: [{ sid: 67 }] } },
{ sid: 63, isDrive: true, name: "/dev/sdc", 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 }] },
{ sid: 72, isDrive: true, name: "/dev/sdd" },
];
staging = [
{ sid: 60, isDrive: false },
// Partition removed
{ sid: 62, isDrive: true, partitionTable: { partitions: [] } },
{ sid: 62, isDrive: true, name: "/dev/sdb", partitionTable: { partitions: [] } },
// Partition added
{ sid: 63, isDrive: true, partitionTable: { partitions: [{ sid: 69 }] } },
{ sid: 63, isDrive: true, name: "/dev/sdc", 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: [] },
// Not modified
{ sid: 72, isDrive: true, name: "/dev/sdd" },
];
});

Expand All @@ -327,10 +330,22 @@ describe("usedDevices", () => {
actions = [];
});

it("returns an empty list", () => {
it("returns an empty list if no argument is passed", () => {
const manager = new DevicesManager(system, staging, actions);
expect(manager.usedDevices()).toEqual([]);
});

it("returns an empty list if non-existent names are passed", () => {
const manager = new DevicesManager(system, staging, actions);
expect(manager.usedDevices(["/dev/nodisk"])).toEqual([]);
});

it("returns a list including only the disks passed by argument", () => {
const manager = new DevicesManager(system, staging, actions);
const devs = manager.usedDevices(["/dev/sdb", "/dev/sdb"]);
expect(devs.length).toEqual(1);
expect(devs[0].sid).toEqual(62);
});
});

describe("if there are actions", () => {
Expand Down Expand Up @@ -370,6 +385,15 @@ describe("usedDevices", () => {
.sort();
expect(sids).toEqual([62, 63, 65, 66]);
});

it("also includes extra disks passed as argument without repeating redundant ones", () => {
const manager = new DevicesManager(system, staging, actions);
const sids = manager
.usedDevices(["/dev/sdb", "/dev/sdd"])
.map((d) => d.sid)
.sort();
expect(sids).toEqual([62, 63, 65, 66, 72]);
});
});
});

Expand Down
19 changes: 2 additions & 17 deletions web/src/components/storage/ProposalPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,7 @@ import ConfigEditorMenu from "./ConfigEditorMenu";
import { toValidationError } from "~/utils";
import { useIssues } from "~/queries/issues";
import { IssueSeverity } from "~/types/issues";
import {
useDevices,
useDeprecated,
useDeprecatedChanges,
useActions,
useReprobeMutation,
} from "~/queries/storage";
import { useDeprecated, useDeprecatedChanges, useReprobeMutation } from "~/queries/storage";
import { _ } from "~/i18n";

/**
Expand All @@ -65,11 +59,8 @@ export const NOT_AFFECTED = {
};

export default function ProposalPage() {
const systemDevices = useDevices("system");
const stagingDevices = useDevices("result");
const isDeprecated = useDeprecated();
const { mutateAsync: reprobe } = useReprobeMutation();
const actions = useActions();

useDeprecatedChanges();

Expand Down Expand Up @@ -128,13 +119,7 @@ export default function ProposalPage() {
<EncryptionField password={""} isLoading={false} />
</GridItem>
<GridItem sm={12}>
<ProposalResultSection
system={systemDevices}
staging={stagingDevices}
actions={actions}
errors={errors}
isLoading={false}
/>
<ProposalResultSection errors={errors} isLoading={false} />
</GridItem>
</Grid>
</Page.Content>
Expand Down
94 changes: 52 additions & 42 deletions web/src/components/storage/ProposalResultSection.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,22 @@ import { ProposalResultSectionProps } from "./ProposalResultSection";

const errorMessage = "Something went wrong, proposal not possible";
const errors = [{ severity: 0, message: errorMessage }];
const defaultProps: ProposalResultSectionProps = {
system: devices.system,
staging: devices.staging,
actions,
};
const defaultProps: ProposalResultSectionProps = {};
const mockUseActionsFn = jest.fn();
const mockConfig = { drives: [] };

jest.mock("~/queries/storage", () => ({
...jest.requireActual("~/queries/storage"),
useConfigModel: () => mockConfig,
useDevices: () => devices.staging,
useActions: () => mockUseActionsFn(),
}));

describe("ProposalResultSection", () => {
beforeEach(() => {
mockUseActionsFn.mockReturnValue(actions);
});

describe.skip("ProposalResultSection", () => {
describe("when there are errors (proposal was not possible)", () => {
it("renders given errors", () => {
plainRender(<ProposalResultSection {...defaultProps} errors={errors} />);
Expand All @@ -61,35 +70,36 @@ describe.skip("ProposalResultSection", () => {
});

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(<ProposalResultSection {...props} />);
expect(screen.queryByText(/Warning alert:/)).toBeNull();
describe("and there are no delete actions", () => {
beforeEach(() => {
mockUseActionsFn.mockReturnValue(actions.filter((a) => !a.delete));
});

it("does not render a warning when there are not delete actions", () => {
plainRender(<ProposalResultSection {...defaultProps} />);
expect(screen.queryByText(/Warning alert:/)).toBeNull();
});
});

it("renders a reminder when there are delete actions", () => {
plainRender(<ProposalResultSection {...defaultProps} />);
const reminder = screen.getByRole("status");
within(reminder).getByText(/4 destructive/);
describe("and there are delete actions affecting a previous system", () => {
beforeEach(() => {
// NOTE: simulate the deletion of vdc2 (sid: 79) for checking that
// affected systems are rendered in the warning summary
mockUseActionsFn.mockReturnValue([
{ device: 79, subvol: false, delete: true, resize: false, text: "" },
]);
});

it("renders the affected systems in the deletion reminder, if any", () => {
plainRender(<ProposalResultSection {...defaultProps} />);
expect(screen.queryByText(/affecting openSUSE/)).toBeInTheDocument();
});
});

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 = {
...defaultProps,
actions: [{ device: 79, subvol: false, delete: true, resize: false, text: "" }],
};

plainRender(<ProposalResultSection {...props} />);
// FIXME: below line reveals that warning wrapper deserves a role or
// something
const reminder = screen.getByRole("status");
within(reminder).getByText(/openSUSE/);
it("renders a reminder about the delete actions", () => {
plainRender(<ProposalResultSection {...defaultProps} />);
expect(screen.queryByText(/Warning alert:/)).toBeInTheDocument();
expect(screen.queryByText(/4 destructive/)).toBeInTheDocument();
});

it("renders a treegrid including all relevant information about final result", () => {
Expand All @@ -99,8 +109,8 @@ describe.skip("ProposalResultSection", () => {
* 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"
* "vdc1 BIOS Boot Partition 8 MiB"
* "vdc3 swap Swap Partition 1.5 GiB"
* "Unused space 3.49 GiB"
* "vdc2 openSUSE Leap 15.2, Fedora 10.30 5 GiB"
* "Unused space 1 GiB"
Expand All @@ -110,23 +120,23 @@ describe.skip("ProposalResultSection", () => {
* 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
* vdc1 BIOS Boot Partition 8 MiB
* vdc3 swap 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 Before 2 GiB 1.5 GiB
* vdc5 / New Btrfs Partition 17.5 GiB
* vdc4 Linux 1.5 GiB
* vdc5 / 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: "vdc1 BIOS Boot Partition 8 MiB" });
within(treegrid).getByRole("row", { name: "vdc3 swap 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 Before 2 GiB 1.5 GiB" });
within(treegrid).getByRole("row", { name: "vdc5 / New Btrfs Partition 17.5 GiB" });
within(treegrid).getByRole("row", { name: "vdc4 Linux 1.5 GiB" });
within(treegrid).getByRole("row", { name: "vdc5 / Btrfs Partition 17.5 GiB" });
});

it("renders a button for opening the planned actions dialog", async () => {
Expand All @@ -135,7 +145,7 @@ describe.skip("ProposalResultSection", () => {

await user.click(button);

screen.getByRole("dialog", { name: "Planned Actions" });
screen.getByRole("button", { name: "Collapse the list of planned actions" });
});
});
});
11 changes: 4 additions & 7 deletions web/src/components/storage/ProposalResultSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import DevicesManager from "~/components/storage/DevicesManager";
import ProposalResultTable from "~/components/storage/ProposalResultTable";
import { ProposalActionsDialog } from "~/components/storage";
import { _, n_, formatList } from "~/i18n";
import { Action, StorageDevice } from "~/types/storage";
import { ValidationError } from "~/types/issues";
import { useActions, useDevices } from "~/queries/storage";
import { sprintf } from "sprintf-js";

/**
Expand Down Expand Up @@ -111,20 +111,17 @@ function ActionsList({ manager }: ActionsListProps) {
}

export type ProposalResultSectionProps = {
system?: StorageDevice[];
staging?: StorageDevice[];
actions?: Action[];
errors?: ValidationError[];
isLoading?: boolean;
};

export default function ProposalResultSection({
system = [],
staging = [],
actions = [],
errors = [],
isLoading = false,
}: ProposalResultSectionProps) {
const system = useDevices("system", { suspense: true });
const staging = useDevices("result", { suspense: true });
const actions = useActions();
const devicesManager = new DevicesManager(system, staging, actions);

return (
Expand Down
4 changes: 3 additions & 1 deletion web/src/components/storage/ProposalResultTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { deviceChildren, deviceSize } from "~/components/storage/utils";
import { PartitionSlot, StorageDevice } from "~/types/storage";
import { TreeTableColumn } from "~/components/core/TreeTable";
import { DeviceInfo } from "~/api/storage/types";
import { useConfigModel } from "~/queries/storage";

type TableItem = StorageDevice | PartitionSlot;

Expand Down Expand Up @@ -144,7 +145,8 @@ type ProposalResultTableProps = {
* @component
*/
export default function ProposalResultTable({ devicesManager }: ProposalResultTableProps) {
const devices = devicesManager.usedDevices();
const model = useConfigModel({ suspense: true });
const devices = devicesManager.usedDevices(model.drives.map((d) => d.name));

return (
<TreeTable
Expand Down