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.

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

Temporary fixes to UI #1712

Merged
merged 4 commits into from
Oct 30, 2024
Merged

Temporary fixes to UI #1712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5cfbd32
rust: Change source of the list of actions
ancorgs Oct 29, 2024
69c6439
web: Change source of the list of actions
ancorgs Oct 29, 2024
144a2d5
web: Temporarily remove storage overview
ancorgs Oct 29, 2024
0d418d8
web: Temporarily simplify storage page
ancorgs Oct 29, 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
10 changes: 8 additions & 2 deletions rust/agama-lib/src/storage/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>,
}

Expand All @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

The Storage1.Devices interface is not reexported. This should be similar to the storage_proxy and calculator_proxy. I would say caching it is ok here. Anyway, it is not that important because we are caching at client level.

Copy link
Contributor

@joseivanlopez joseivanlopez Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

At some point I would like to reevalute the Devices interface. I think it would be better to add a new method Storage1#devices instead of having a separate interface. That method would report a JSON according to a schema, for example:

{
  system: {},
  target: {},
  actions: {}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Definitely all the proxies need to be reconsidered... afterwards.

.build()
.await?,
connection,
})
}
Expand All @@ -80,7 +86,7 @@ impl<'a> StorageClient<'a> {

/// Actions to perform in the storage devices.
pub async fn actions(&self) -> Result<Vec<Action>, ServiceError> {
let actions = self.proposal_proxy.actions().await?;
let actions = self.devices_proxy.actions().await?;
let mut result: Vec<Action> = Vec::with_capacity(actions.len());

for i in actions {
Expand Down
13 changes: 10 additions & 3 deletions rust/agama-lib/src/storage/proxies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<std::collections::HashMap<String, zbus::zvariant::OwnedValue>>>;
}

#[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(
Expand Down
4 changes: 2 additions & 2 deletions rust/agama-server/src/storage/web.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub async fn storage_service(dbus: zbus::Connection) -> Result<Router, ServiceEr
.route("/devices/result", get(staging_devices))
.route("/product/volume_for", get(volume_for))
.route("/product/params", get(product_params))
.route("/proposal/actions", get(actions))
.route("/devices/actions", get(actions))
.route("/proposal/usable_devices", get(usable_devices))
.route(
"/proposal/settings",
Expand Down Expand Up @@ -331,7 +331,7 @@ pub struct ProductParams {
/// Gets the actions to perform in the storage devices.
#[utoipa::path(
get,
path = "/proposal/actions",
path = "/devices/actions",
context_path = "/api/storage",
responses(
(status = 200, description = "List of actions", body = Vec<Action>),
Expand Down
2 changes: 1 addition & 1 deletion web/src/api/storage/proposal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const fetchDefaultVolume = (mountPath: string): Promise<Volume | undefined> => {

const fetchSettings = (): Promise<ProposalSettings> => get("/api/storage/proposal/settings");

const fetchActions = (): Promise<Action[]> => get("/api/storage/proposal/actions");
const fetchActions = (): Promise<Action[]> => get("/api/storage/devices/actions");

const calculate = (settings: ProposalSettingsPatch) =>
put("/api/storage/proposal/settings", settings);
Expand Down
2 changes: 0 additions & 2 deletions web/src/components/overview/OverviewPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -116,7 +115,6 @@ const OverviewSection = () => (
>
<Stack hasGutter>
<L10nSection />
<StorageSection />
<SoftwareSection />
</Stack>
</Page.Section>
Expand Down
1 change: 0 additions & 1 deletion web/src/components/overview/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
96 changes: 13 additions & 83 deletions web/src/components/storage/ProposalActionsSummary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
<strong className={textStyles.warningColor_200}>
Expand All @@ -69,6 +54,8 @@ const DeletionsInfo = ({
)}
</strong>
);
} else {
label = _("No destructive actions are planned");
}

if (systems.length) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
};
Expand All @@ -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 = <Skeleton fontSize="sm" width="65%" />;
} 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 (
<Page.Section
title={_("Actions")}
value={value}
actions={
isLoading ? (
<Skeleton fontSize="sm" width="100px" />
) : (
<Link to={PATHS.spacePolicy}>{_("Change")}</Link>
)
}
pfCardProps={{ isFullHeight: false }}
>
<Page.Section title={_("Actions")} pfCardProps={{ isFullHeight: false }}>
{isLoading ? (
<ActionsSkeleton />
) : (
<List>
<DeletionsInfo
policy={policy}
manager={devicesManager}
spaceActions={spaceActions.filter((a) => a.action === "force_delete")}
/>
<ResizesInfo
policy={policy}
manager={devicesManager}
validProposal={errors.length === 0}
spaceActions={spaceActions.filter((a) => a.action === "resize")}
/>
<DeletionsInfo manager={devicesManager} />
<ResizesInfo manager={devicesManager} />
<ActionsInfo
actions={actions}
validProposal={errors.length === 0}
Expand Down
45 changes: 2 additions & 43 deletions web/src/components/storage/ProposalPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,14 @@
import React, { useRef } from "react";
import { Grid, GridItem, Stack } from "@patternfly/react-core";
import { Page, Drawer } from "~/components/core/";
import ProposalTransactionalInfo from "./ProposalTransactionalInfo";
import ProposalSettingsSection from "./ProposalSettingsSection";
import ProposalResultSection from "./ProposalResultSection";
import ProposalActionsSummary from "~/components/storage/ProposalActionsSummary";
import { ProposalActionsDialog } from "~/components/storage";
import { _ } from "~/i18n";
import { SPACE_POLICIES } from "~/components/storage/utils";
import { toValidationError } from "~/utils";
import { useIssues } from "~/queries/issues";
import { IssueSeverity } from "~/types/issues";
import {
useAvailableDevices,
useDeprecated,
useDevices,
useProductParams,
useProposalMutation,
useProposalResult,
useVolumeDevices,
useVolumeTemplates,
} from "~/queries/storage";
import { useDeprecated, useDevices, useProposalResult } from "~/queries/storage";
import { useQueryClient } from "@tanstack/react-query";
import { refresh } from "~/api/storage";

Expand Down Expand Up @@ -74,12 +62,7 @@ export default function ProposalPage() {
const drawerRef = useRef();
const systemDevices = useDevices("system");
const stagingDevices = useDevices("result");
const availableDevices = useAvailableDevices();
const volumeDevices = useVolumeDevices();
const volumeTemplates = useVolumeTemplates();
const { encryptionMethods } = useProductParams({ suspense: true });
const { actions, settings } = useProposalResult();
const updateProposal = useProposalMutation();
const { actions } = useProposalResult();
const deprecated = useDeprecated();
const queryClient = useQueryClient();

Expand All @@ -95,13 +78,6 @@ export default function ProposalPage() {
.filter((s) => 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 (
<Page>
<Page.Header>
Expand All @@ -111,34 +87,17 @@ export default function ProposalPage() {
<Page.Content>
<Grid hasGutter>
<GridItem sm={12}>
<ProposalTransactionalInfo settings={settings} />
</GridItem>
<GridItem sm={12} xl={6}>
<ProposalSettingsSection
availableDevices={availableDevices}
volumeDevices={volumeDevices}
encryptionMethods={encryptionMethods}
volumeTemplates={volumeTemplates}
settings={settings}
onChange={changeSettings}
isLoading={false}
/>
</GridItem>
<GridItem sm={12} xl={6}>
<Drawer
ref={drawerRef}
panelHeader={<h4>{_("Planned Actions")}</h4>}
panelContent={<ProposalActionsDialog actions={actions} />}
>
<Stack hasGutter>
<ProposalActionsSummary
policy={spacePolicy}
system={systemDevices}
staging={stagingDevices}
errors={errors}
actions={actions}
spaceActions={settings.spaceActions}
devices={settings.installationDevices}
// @ts-expect-error: we do not know how to specify the type of
// drawerRef properly and TS does not find the "open" property
onActionsClick={drawerRef.current?.open}
Expand Down
Loading
Loading