From 5cfbd32745991c0fb0dde37eb975f47ed9a6c810 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 29 Oct 2024 12:19:57 +0000 Subject: [PATCH 1/4] rust: Change source of the list of actions --- rust/agama-lib/src/storage/client.rs | 10 ++++++++-- rust/agama-lib/src/storage/proxies.rs | 13 ++++++++++--- rust/agama-server/src/storage/web.rs | 4 ++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/rust/agama-lib/src/storage/client.rs b/rust/agama-lib/src/storage/client.rs index 7675d538f9..4125188233 100644 --- a/rust/agama-lib/src/storage/client.rs +++ b/rust/agama-lib/src/storage/client.rs @@ -25,7 +25,7 @@ use super::model::{ Md, Multipath, Partition, PartitionTable, ProposalSettings, ProposalSettingsPatch, Raid, Volume, }; -use super::proxies::{ProposalCalculatorProxy, ProposalProxy, Storage1Proxy}; +use super::proxies::{DevicesProxy, ProposalCalculatorProxy, ProposalProxy, Storage1Proxy}; use super::StorageSettings; use crate::dbus::get_property; use crate::error::ServiceError; @@ -50,6 +50,7 @@ pub struct StorageClient<'a> { calculator_proxy: ProposalCalculatorProxy<'a>, storage_proxy: Storage1Proxy<'a>, object_manager_proxy: ObjectManagerProxy<'a>, + devices_proxy: DevicesProxy<'a>, proposal_proxy: ProposalProxy<'a>, } @@ -69,6 +70,11 @@ impl<'a> StorageClient<'a> { .cache_properties(zbus::CacheProperties::No) .build() .await?, + // Same than above, actions are reexported with every call to recalculate + devices_proxy: DevicesProxy::builder(&connection) + .cache_properties(zbus::CacheProperties::No) + .build() + .await?, connection, }) } @@ -80,7 +86,7 @@ impl<'a> StorageClient<'a> { /// Actions to perform in the storage devices. pub async fn actions(&self) -> Result, ServiceError> { - let actions = self.proposal_proxy.actions().await?; + let actions = self.devices_proxy.actions().await?; let mut result: Vec = Vec::with_capacity(actions.len()); for i in actions { diff --git a/rust/agama-lib/src/storage/proxies.rs b/rust/agama-lib/src/storage/proxies.rs index 922ecf31d4..f83ee7af09 100644 --- a/rust/agama-lib/src/storage/proxies.rs +++ b/rust/agama-lib/src/storage/proxies.rs @@ -88,17 +88,24 @@ trait ProposalCalculator { } #[dbus_proxy( - interface = "org.opensuse.Agama.Storage1.Proposal", + interface = "org.opensuse.Agama.Storage1.Devices", default_service = "org.opensuse.Agama.Storage1", - default_path = "/org/opensuse/Agama/Storage1/Proposal" + default_path = "/org/opensuse/Agama/Storage1" )] -trait Proposal { +trait Devices { /// Actions property #[dbus_proxy(property)] fn actions( &self, ) -> zbus::Result>>; +} +#[dbus_proxy( + interface = "org.opensuse.Agama.Storage1.Proposal", + default_service = "org.opensuse.Agama.Storage1", + default_path = "/org/opensuse/Agama/Storage1/Proposal" +)] +trait Proposal { /// Settings property #[dbus_proxy(property)] fn settings( diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index 5049923ab8..bfb909e055 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -120,7 +120,7 @@ pub async fn storage_service(dbus: zbus::Connection) -> Result), From 69c6439e73e31aee90945be18dd7e0be671a77be Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 29 Oct 2024 12:20:34 +0000 Subject: [PATCH 2/4] web: Change source of the list of actions --- web/src/api/storage/proposal.ts | 2 +- web/src/queries/storage.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/api/storage/proposal.ts b/web/src/api/storage/proposal.ts index 3a5335beb6..727da24e77 100644 --- a/web/src/api/storage/proposal.ts +++ b/web/src/api/storage/proposal.ts @@ -40,7 +40,7 @@ const fetchDefaultVolume = (mountPath: string): Promise => { const fetchSettings = (): Promise => get("/api/storage/proposal/settings"); -const fetchActions = (): Promise => get("/api/storage/proposal/actions"); +const fetchActions = (): Promise => get("/api/storage/devices/actions"); const calculate = (settings: ProposalSettingsPatch) => put("/api/storage/proposal/settings", settings); diff --git a/web/src/queries/storage.ts b/web/src/queries/storage.ts index ee58f1c77c..39b3cf92d0 100644 --- a/web/src/queries/storage.ts +++ b/web/src/queries/storage.ts @@ -206,7 +206,7 @@ const proposalSettingsQuery = { }; const proposalActionsQuery = { - queryKey: ["storage", "proposal", "actions"], + queryKey: ["storage", "devices", "actions"], queryFn: fetchActions, }; From 144a2d5c9f4cb8709f849dad17fb874b8df73352 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 29 Oct 2024 14:57:21 +0000 Subject: [PATCH 3/4] web: Temporarily remove storage overview --- web/src/components/overview/OverviewPage.tsx | 2 -- web/src/components/overview/index.js | 1 - 2 files changed, 3 deletions(-) diff --git a/web/src/components/overview/OverviewPage.tsx b/web/src/components/overview/OverviewPage.tsx index 7e992ff7a3..c056d9cf58 100644 --- a/web/src/components/overview/OverviewPage.tsx +++ b/web/src/components/overview/OverviewPage.tsx @@ -38,7 +38,6 @@ import { Link } from "react-router-dom"; import { Center } from "~/components/layout"; import { EmptyState, InstallButton, Page } from "~/components/core"; import L10nSection from "./L10nSection"; -import StorageSection from "./StorageSection"; import SoftwareSection from "./SoftwareSection"; import { _ } from "~/i18n"; import { useAllIssues } from "~/queries/issues"; @@ -116,7 +115,6 @@ const OverviewSection = () => ( > - diff --git a/web/src/components/overview/index.js b/web/src/components/overview/index.js index 1ffe2bad18..a97896e9c6 100644 --- a/web/src/components/overview/index.js +++ b/web/src/components/overview/index.js @@ -23,4 +23,3 @@ export { default as OverviewPage } from "./OverviewPage"; export { default as L10nSection } from "./L10nSection"; export { default as SoftwareSection } from "./SoftwareSection"; -export { default as StorageSection } from "./StorageSection"; From 0d418d8180c90e56a891e25212b3a0acda9cc5f9 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 29 Oct 2024 15:00:57 +0000 Subject: [PATCH 4/4] web: Temporarily simplify storage page --- .../storage/ProposalActionsSummary.tsx | 96 +++---------------- web/src/components/storage/ProposalPage.tsx | 45 +-------- web/src/queries/storage.ts | 75 +-------------- 3 files changed, 18 insertions(+), 198 deletions(-) diff --git a/web/src/components/storage/ProposalActionsSummary.tsx b/web/src/components/storage/ProposalActionsSummary.tsx index 4e343f8735..0736938563 100644 --- a/web/src/components/storage/ProposalActionsSummary.tsx +++ b/web/src/components/storage/ProposalActionsSummary.tsx @@ -22,40 +22,25 @@ import React from "react"; import { Button, Skeleton, Stack, List, ListItem } from "@patternfly/react-core"; -import { Link, Page } from "~/components/core"; +import { Page } from "~/components/core"; import DevicesManager from "~/components/storage/DevicesManager"; import { _, n_ } from "~/i18n"; import { sprintf } from "sprintf-js"; import textStyles from "@patternfly/react-styles/css/utilities/Text/text"; -import { PATHS } from "~/routes/storage"; -import { Action, SpaceAction, StorageDevice } from "~/types/storage"; -import { SpacePolicy } from "./utils"; +import { Action, StorageDevice } from "~/types/storage"; import { ValidationError } from "~/types/issues"; /** * Renders information about delete actions */ -const DeletionsInfo = ({ - policy, - manager, - spaceActions, -}: { - policy: SpacePolicy | undefined; - manager: DevicesManager; - spaceActions: SpaceAction[]; -}) => { +const DeletionsInfo = ({ manager }: { manager: DevicesManager }) => { let label: React.ReactNode; let systemsLabel: React.ReactNode; const systems = manager.deletedSystems(); const deleteActions = manager.actions.filter((a) => a.delete && !a.subvol).length; - const isDeletePolicy = policy?.id === "delete"; const hasDeleteActions = deleteActions !== 0; - if (!isDeletePolicy && spaceActions.length === 0) { - label = _("Destructive actions are not allowed"); - } else if ((isDeletePolicy || spaceActions.length > 0) && !hasDeleteActions) { - label = _("Destructive actions are allowed"); - } else if (hasDeleteActions) { + if (hasDeleteActions) { // TRANSLATORS: %d will be replaced by the amount of destructive actions label = ( @@ -69,6 +54,8 @@ const DeletionsInfo = ({ )} ); + } else { + label = _("No destructive actions are planned"); } if (systems.length) { @@ -92,37 +79,20 @@ const DeletionsInfo = ({ /** * Renders information about resize actions */ -const ResizesInfo = ({ - policy, - manager, - validProposal, - spaceActions, -}: { - policy: SpacePolicy | undefined; - manager: DevicesManager; - validProposal: boolean; - spaceActions: SpaceAction[]; -}) => { +const ResizesInfo = ({ manager }: { manager: DevicesManager }) => { let label: React.ReactNode; let systemsLabel: React.ReactNode; const systems = manager.resizedSystems(); const resizeActions = manager.actions.filter((a) => a.resize).length; - const isResizePolicy = policy?.id === "resize"; const hasResizeActions = resizeActions !== 0; - if (!isResizePolicy && spaceActions.length === 0) { - label = _("Shrinking partitions is not allowed"); - } - - if (!validProposal && (isResizePolicy || spaceActions.length > 0)) { - label = _("Shrinking partitions is allowed"); - } else if (validProposal && (isResizePolicy || spaceActions.length > 0) && !hasResizeActions) { - label = _("Shrinking some partitions is allowed but not needed"); - } else if (hasResizeActions) { + if (hasResizeActions) { label = sprintf( n_("%d partition will be shrunk", "%d partitions will be shrunk", resizeActions), resizeActions, ); + } else { + label = _("No partitions wil be shrunk"); } if (systems.length) { @@ -194,11 +164,9 @@ const ActionsSkeleton = () => ( export type ProposalActionsSummaryProps = { isLoading: boolean; errors: ValidationError[]; - policy: SpacePolicy | undefined; system: StorageDevice[]; staging: StorageDevice[]; actions: Action[]; - spaceActions: SpaceAction[]; devices: StorageDevice[]; onActionsClick: () => void | undefined; }; @@ -212,59 +180,21 @@ export type ProposalActionsSummaryProps = { export default function ProposalActionsSummary({ isLoading, errors = [], - policy, system = [], staging = [], actions = [], - spaceActions = [], - devices, onActionsClick, }: ProposalActionsSummaryProps) { - let value: React.ReactNode; - - if (isLoading || !policy) { - value = ; - } else if (policy.summaryLabels.length === 1) { - // eslint-disable-next-line agama-i18n/string-literals - value = _(policy.summaryLabels[0]); - } else { - value = sprintf( - // eslint-disable-next-line agama-i18n/string-literals - n_(policy.summaryLabels[0], policy.summaryLabels[1], devices.length), - devices.length, - ); - } - const devicesManager = new DevicesManager(system, staging, actions); return ( - - ) : ( - {_("Change")} - ) - } - pfCardProps={{ isFullHeight: false }} - > + {isLoading ? ( ) : ( - a.action === "force_delete")} - /> - a.action === "resize")} - /> + + s.severity === IssueSeverity.Error) .map(toValidationError); - const changeSettings = async (changing, updated: object) => { - const newSettings = { ...settings, ...updated }; - updateProposal.mutateAsync(newSettings).catch(console.error); - }; - - const spacePolicy = SPACE_POLICIES.find((p) => p.id === settings.spacePolicy); - return ( @@ -111,20 +87,6 @@ export default function ProposalPage() { - - - - - - {_("Planned Actions")}} @@ -132,13 +94,10 @@ export default function ProposalPage() { > { return [...availableDevices, ...mds, ...vgs]; }; -const proposalSettingsQuery = { - queryKey: ["storage", "proposal", "settings"], - queryFn: fetchSettings, -}; - const proposalActionsQuery = { queryKey: ["storage", "devices", "actions"], queryFn: fetchActions, @@ -214,64 +200,9 @@ const proposalActionsQuery = { * Hook that returns the current proposal (settings and actions). */ const useProposalResult = (): ProposalResult | undefined => { - const buildTarget = (value: APIProposalTarget): ProposalTarget => { - // FIXME: handle the case where they do not match - const target = value as ProposalTarget; - return target; - }; - - /** @todo Read installation devices from D-Bus. */ - const buildInstallationDevices = (settings: APIProposalSettings, devices: StorageDevice[]) => { - const findDevice = (name: string) => { - const device = devices.find((d) => d.name === name); - - if (device === undefined) console.error("Device object not found: ", name); - - return device; - }; - - // Only consider the device assigned to a volume as installation device if it is needed - // to find space in that device. For example, devices directly formatted or mounted are not - // considered as installation devices. - const volumes = settings.volumes.filter((vol) => { - const target = vol.target as VolumeTarget; - return [VolumeTarget.NEW_PARTITION, VolumeTarget.NEW_VG].includes(target); - }); + const { data: actions } = useSuspenseQuery(proposalActionsQuery); - const values = [ - settings.targetDevice, - settings.targetPVDevices, - volumes.map((v) => v.targetDevice), - ].flat(); - - if (settings.configureBoot) values.push(settings.bootDevice); - - 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.sort().map(findDevice)); - }; - - const [{ data: settings }, { data: actions }] = useSuspenseQueries({ - queries: [proposalSettingsQuery, proposalActionsQuery], - }); - const systemDevices = useDevices("system", { suspense: true }); - const { mountPoints: productMountPoints } = useProductParams({ suspense: true }); - - return { - settings: { - ...settings, - targetPVDevices: settings.targetPVDevices || [], - target: buildTarget(settings.target), - volumes: settings.volumes.map((v) => buildVolume(v, systemDevices, productMountPoints)), - // NOTE: strictly speaking, installation devices does not belong to the settings. It - // should be a separate method instead of an attribute in the settings object. - // Nevertheless, it was added here for simplicity and to avoid passing more props in some - // react components. Please, do not use settings as a jumble. - installationDevices: buildInstallationDevices(settings, systemDevices), - }, - actions, - }; + return { actions }; }; const useProposalMutation = () => {