Skip to content

Commit

Permalink
[web] Show space actions form only for partitions
Browse files Browse the repository at this point in the history
  • Loading branch information
ancorgs committed Apr 24, 2024
1 parent 35a43fe commit 5332a39
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 37 deletions.
45 changes: 35 additions & 10 deletions web/src/components/storage/SpaceActionsTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { FormSelect, FormSelectOption } from "@patternfly/react-core";
import { _ } from "~/i18n";
import { FilesystemLabel } from "~/components/storage";
import { deviceChildren, deviceSize } from '~/components/storage/utils';
import { If, Tag, TreeTable } from "~/components/core";
import { Tag, TreeTable } from "~/components/core";
import { sprintf } from "sprintf-js";

/**
Expand Down Expand Up @@ -97,7 +97,7 @@ const DeviceSizeDetails = ({ device }) => {
};

/**
* Column content with the space action for a device.
* Form to configure the space action for a device (a partition).
* @component
*
* @param {object} props
Expand All @@ -106,9 +106,7 @@ const DeviceSizeDetails = ({ device }) => {
* @param {boolean} [props.isDisabled=false]
* @param {(action: SpaceAction) => void} [props.onChange]
*/
const DeviceAction = ({ device, action, isDisabled = false, onChange }) => {
if (!device.sid || device.partitionTable) return null;

const DeviceActionForm = ({ device, action, isDisabled = false, onChange }) => {
const changeAction = (_, action) => onChange({ device: device.name, action });

return (
Expand All @@ -122,16 +120,43 @@ const DeviceAction = ({ device, action, isDisabled = false, onChange }) => {
}
>
<FormSelectOption value="force_delete" label={_("Delete")} />
{/* Resize action does not make sense for drives, so it is filtered out. */}
<If
condition={!device.isDrive}
then={<FormSelectOption value="resize" label={_("Allow resize")} />}
/>
<FormSelectOption value="resize" label={_("Allow resize")} />
<FormSelectOption value="keep" label={_("Do not modify")} />
</FormSelect>
);
};

/**
* Column content with the space action (a form or a description) for a device
* @component
*
* @param {object} props
* @param {StorageDevice} props.device
* @param {string} props.action - Possible values: "force_delete", "resize" or "keep".
* @param {boolean} [props.isDisabled=false]
* @param {(action: SpaceAction) => void} [props.onChange]
*/
const DeviceAction = ({ device, action, isDisabled = false, onChange }) => {
if (device.type === "partition") {
return (
<DeviceActionForm
device={device}
action={action}
isDisabled={isDisabled}
onChange={onChange}
/>
);
}

if (device.filesystem || device.component)
return _("The content may be deleted");

if (!device.partitionTable || device.partitionTable.partitions.length === 0)
return _("No content found");

return null;
};

/**
* Table for selecting the space actions of the given devices.
* @component
Expand Down
13 changes: 3 additions & 10 deletions web/src/components/storage/SpacePolicyDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,19 +114,12 @@ export default function SpacePolicyDialog({

// Generates the action value according to the policy.
const deviceAction = (device) => {
let action;

if (policy.id === "custom") {
action = actions.find(a => a.device === device.name)?.action || "keep";
} else {
const policyAction = { delete: "force_delete", resize: "resize", keep: "keep" };
action = policyAction[policy.id];
return actions.find(a => a.device === device.name)?.action || "keep";
}

// For a drive device (e.g., Disk, RAID) it does not make sense to offer the resize action.
// At this moment, the Agama backend generates a resize action for drives if the policy is set
// to 'resize'. In that cases, the action is converted here to 'keep'.
return ((device.isDrive && action === "resize") ? "keep" : action);
const policyAction = { delete: "force_delete", resize: "resize", keep: "keep" };
return policyAction[policy.id];
};

const changeActions = (spaceAction) => {
Expand Down
27 changes: 10 additions & 17 deletions web/src/components/storage/SpacePolicyDialog.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const sda = {
const sda1 = {
sid: 60,
isDrive: false,
type: "",
type: "partition",
active: true,
name: "/dev/sda1",
size: 512,
Expand All @@ -62,7 +62,7 @@ const sda1 = {
const sda2 = {
sid: 61,
isDrive: false,
type: "",
type: "partition",
active: true,
name: "/dev/sda2",
size: 512,
Expand Down Expand Up @@ -182,7 +182,7 @@ describe("SpacePolicyDialog", () => {
plainRender(<SpacePolicyDialog { ...props } />);
const picker = screen.getByRole("listbox");
within(picker).getByRole("option", { name: nameRegexp, selected: true });
expect(within(picker).getAllByRole("option", { selected: false }).length).toEqual(3);
expect(within(picker).getAllByRole("option", { selected: false }).length).toEqual(2);
});

it("does not allow to modify the space actions", async () => {
Expand All @@ -194,7 +194,7 @@ describe("SpacePolicyDialog", () => {
// TODO: use a more inclusive way to disable the actions.
// https://css-tricks.com/making-disabled-buttons-more-inclusive/
const spaceActions = screen.getAllByRole("combobox", { name: /Space action selector/, hidden: true });
expect(spaceActions.length).toEqual(3);
expect(spaceActions.length).toEqual(2);
});
});

Expand All @@ -207,13 +207,13 @@ describe("SpacePolicyDialog", () => {
plainRender(<SpacePolicyDialog { ...props } />);
const picker = screen.getByRole("listbox");
within(picker).getByRole("option", { name: /custom/i, selected: true });
expect(within(picker).getAllByRole("option", { selected: false }).length).toEqual(3);
expect(within(picker).getAllByRole("option", { selected: false }).length).toEqual(2);
});

it("allows to modify the space actions", async () => {
plainRender(<SpacePolicyDialog { ...props } />);
const spaceActions = screen.getAllByRole("combobox", { name: /Space action selector/ });
expect(spaceActions.length).toEqual(3);
expect(spaceActions.length).toEqual(2);
});
});

Expand All @@ -222,7 +222,7 @@ describe("SpacePolicyDialog", () => {
props.policy = customPolicy;
});

it("renders the space actions selector for devices without partition table", async () => {
it("renders the space actions selector for partitions", async () => {
plainRender(<SpacePolicyDialog { ...props } />);
// sda has partition table, the selector shouldn't be found
const sdaRow = screen.getByRole("row", { name: /sda gpt/i });
Expand All @@ -232,17 +232,10 @@ describe("SpacePolicyDialog", () => {
const unusedRow = screen.getByRole("row", { name: /unused space/i });
const unusedActionsSelector = within(unusedRow).queryByRole("combobox");
expect(unusedActionsSelector).toBeNull();
// sdb does not have partition table, selector should be there
// sdb is a disk, selector shouldn't be there
const sdbRow = screen.getByRole("row", { name: /sdb/ });
within(sdbRow).getByRole("combobox");
});

it("does not renders the 'resize' option for drives", async () => {
plainRender(<SpacePolicyDialog { ...props } />);
const sdbRow = screen.getByRole("row", { name: /sdb/ });
const spaceActionsSelector = within(sdbRow).getByRole("combobox");
const resizeOption = within(spaceActionsSelector).queryByRole("option", { name: /resize/ });
expect(resizeOption).toBeNull();
const sdbActionsSelector = within(sdbRow).queryByRole("combobox");
expect(sdbActionsSelector).toBeNull();
});

it("renders the 'resize' option for devices other than drives", async () => {
Expand Down

0 comments on commit 5332a39

Please sign in to comment.