Skip to content
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.

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

Adapt to recent changes on Y2Storage::GuidedProposal #1164

Merged
merged 4 commits into from
Apr 25, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -207,20 +207,9 @@ def boot_device
#
# @see ProposalSettings#installation_devices
#
# If a device is partitioned, then its partitions are included instead of the device.
#
# @return [Array<String>]
def all_devices
settings.installation_devices
.map { |d| device_or_partitions(d) }
.flatten
end

# @param device [String]
# @return [String, Array<String>]
def device_or_partitions(device)
partitions = partitions(device)
partitions.empty? ? device : partitions
settings.installation_devices.flat_map { |d| partitions(d) }
end

# @param device [String]
Expand Down
2 changes: 1 addition & 1 deletion service/package/gem2rpm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
Requires: yast2-iscsi-client >= 4.5.7
Requires: yast2-network
Requires: yast2-proxy
Requires: yast2-storage-ng >= 5.0.12
Requires: yast2-storage-ng >= 5.0.13
Requires: yast2-users
%ifarch s390 s390x
Requires: yast2-s390 >= 4.6.4
Expand Down
6 changes: 6 additions & 0 deletions service/package/rubygem-agama-yast.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Thu Apr 25 13:40:06 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>

- Adapted to recent changes on Y2Storage::GuidedProposal
(gh#yast/yast-storage-ng#1382)

-------------------------------------------------------------------
Thu Apr 18 08:46:06 UTC 2024 - Ladislav Slezák <[email protected]>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,17 +262,15 @@
settings.space.policy = :delete
end

it "generates delete actions for all used devices" do
it "generates delete actions for all the partitions at the used devices" do
y2storage_settings = subject.convert

expect(y2storage_settings.space_settings).to have_attributes(
strategy: :bigger_resize,
actions: {
"/dev/sda1" => :force_delete,
"/dev/sda2" => :force_delete,
"/dev/sda3" => :force_delete,
"/dev/sdb" => :force_delete,
"/dev/sdc" => :force_delete
"/dev/sda3" => :force_delete
}
)
end
Expand All @@ -283,17 +281,15 @@
settings.space.policy = :resize
end

it "generates resize actions for all used devices" do
it "generates resize actions for all the partitions at the used devices" do
y2storage_settings = subject.convert

expect(y2storage_settings.space_settings).to have_attributes(
strategy: :bigger_resize,
actions: {
"/dev/sda1" => :resize,
"/dev/sda2" => :resize,
"/dev/sda3" => :resize,
"/dev/sdb" => :resize,
"/dev/sdc" => :resize
"/dev/sda3" => :resize
}
)
end
Expand Down
6 changes: 6 additions & 0 deletions web/package/cockpit-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Thu Apr 25 13:40:06 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>

- Adapted to recent changes on Y2Storage::GuidedProposal
(gh#yast/yast-storage-ng#1382)

-------------------------------------------------------------------
Tue Apr 16 11:15:13 UTC 2024 - David Diaz <[email protected]>

Expand Down
2 changes: 1 addition & 1 deletion web/src/client/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ class ProposalManager {
const names = uniq(compact(values)).filter(d => d.length > 0);

// #findDevice returns undefined if no device is found with the given name.
return compact(names.map(findDevice));
return compact(names.sort().map(findDevice));
};

const dbusSettings = proxy.Settings;
Expand Down
47 changes: 37 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,45 @@ 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.sid) return null;

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");
Copy link
Contributor

Choose a reason for hiding this comment

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

NP: I still thinking that "No content found" is a bad content for an "Action" column. But I do not have a good proposal at the time of writing.


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
23 changes: 8 additions & 15 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 @@ -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 @@ -213,7 +213,7 @@ describe("SpacePolicyDialog", () => {
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
Loading