From b6fb007f41bd5fd830f161bf613fd304d2ee30d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Feb 2025 06:36:39 +0000 Subject: [PATCH 01/21] storage: remove guided schema --- rust/agama-lib/share/storage.schema.json | 248 ----------------------- 1 file changed, 248 deletions(-) diff --git a/rust/agama-lib/share/storage.schema.json b/rust/agama-lib/share/storage.schema.json index 00461d4e87..0ddae32c14 100644 --- a/rust/agama-lib/share/storage.schema.json +++ b/rust/agama-lib/share/storage.schema.json @@ -594,254 +594,6 @@ } } } - }, - "guided": { - "title": "Guided settings", - "$comment": "This guided section will be extracted to a separate schema. Only storage and legacyAutoyastStorage will be offered as valid schemas for the storage config.", - "type": "object", - "additionalProperties": false, - "properties": { - "target": { - "anyOf": [ - { - "title": "Target for installing", - "description": "Indicates whether to install in a disk or a new LVM.", - "enum": ["disk", "newLvmVg"] - }, - { - "title": "Target disk", - "description": "Indicates to install in a specific disk device.", - "type": "object", - "additionalProperties": false, - "required": ["disk"], - "properties": { - "disk": { - "title": "Device name", - "type": "string", - "examples": ["/dev/vda"] - } - } - }, - { - "title": "New LVM", - "description": "Indicates to install in a new LVM created over some specific devices.", - "type": "object", - "additionalProperties": false, - "required": ["newLvmVg"], - "properties": { - "newLvmVg": { - "description": "List of devices in which to create the physical volumes.", - "type": "array", - "items": { - "title": "Device name", - "type": "string", - "examples": ["/dev/vda"] - } - } - } - } - ] - }, - "boot": { - "$ref": "#/$defs/boot" - }, - "encryption": { - "title": "Encryption", - "description": "Indicates the options for encrypting the new partitions.", - "type": "object", - "additionalProperties": false, - "required": ["password"], - "properties": { - "password": { - "$ref": "#/$defs/encryptionPassword" - }, - "method": { - "title": "Encryption method", - "description": "Method used to encrypt the devices.", - "enum": ["luks2", "tpm_fde"] - }, - "pbkdFunction": { - "$ref": "#/$defs/encryptionPbkdFunction" - } - } - }, - "space": { - "title": "Space policy", - "description": "Indicates how to find space for the new partitions.", - "type": "object", - "additionalProperties": false, - "properties": { - "policy": { - "enum": ["delete", "resize", "keep", "custom"] - }, - "actions": { - "type": "array" - } - }, - "if": { - "properties": { - "policy": { "const": "custom" } - } - }, - "then": { - "required": ["policy", "actions"], - "properties": { - "actions": { - "title": "Custom actions", - "description": "Indicates what to do with specific devices.", - "type": "array", - "items": { - "anyOf": [ - { - "title": "Force delete", - "description": "Indicates to delete a specific device.", - "type": "object", - "required": ["forceDelete"], - "additionalProperties": false, - "properties": { - "forceDelete": { - "description": "Name of the device to delete.", - "type": "string", - "examples": ["/dev/vda"] - } - } - }, - { - "title": "Allow shinking", - "description": "Indicates whether a specific device can be shrunk if needed.", - "type": "object", - "required": ["resize"], - "additionalProperties": false, - "properties": { - "resize": { - "description": "Name of the shrinkable device.", - "type": "string", - "examples": ["/dev/vda"] - } - } - } - ] - } - } - } - }, - "else": { - "required": ["policy"], - "properties": { - "actions": { - "type": "array", - "maxItems": 0 - } - } - } - }, - "volumes": { - "title": "System volumes", - "description": "List of volumes (file systems) to create.", - "type": "array", - "items": { - "type": "object", - "additionalProperties": false, - "required": ["mount"], - "properties": { - "mount": { - "title": "Mount properties", - "type": "object", - "additionalProperties": false, - "required": ["path"], - "properties": { - "path": { - "title": "Mount path", - "type": "string", - "examples": ["/dev/vda"] - }, - "options": { - "title": "Mount options", - "description": "Options to add to the fourth field of fstab.", - "type": "array", - "items": { - "type": "string" - } - } - } - }, - "filesystem": { - "$ref": "#/$defs/filesystemType" - }, - "size": { - "$ref": "#/$defs/size" - }, - "target": { - "title": "Volume target", - "description": "Options to indicate the location of a volume.", - "anyOf": [ - { - "title": "Default target", - "description": "The volume is created in the target device for installing.", - "const": "default" - }, - { - "title": "New partition", - "description": "The volume is created over a new partition in a specific disk.", - "type": "object", - "required": ["newPartition"], - "additionalProperties": false, - "properties": { - "newPartition": { - "description": "Name of a disk device.", - "type": "string", - "examples": ["/dev/vda"] - } - } - }, - { - "title": "Dedicated LVM volume group", - "description": "The volume is created over a new dedicated LVM.", - "type": "object", - "additionalProperties": false, - "required": ["newVg"], - "properties": { - "newVg": { - "description": "Name of a disk device.", - "type": "string", - "examples": ["/dev/vda"] - } - } - }, - { - "title": "Re-used existing device", - "description": "The volume is created over an existing device.", - "type": "object", - "additionalProperties": false, - "required": ["device"], - "properties": { - "device": { - "description": "Name of a device.", - "type": "string", - "examples": ["/dev/vda1"] - } - } - }, - { - "title": "Re-used existing file system", - "description": "An existing file system is reused (without formatting).", - "type": "object", - "additionalProperties": false, - "required": ["filesystem"], - "properties": { - "filesystem": { - "description": "Name of a device containing the file system.", - "type": "string", - "examples": ["/dev/vda1"] - } - } - } - ] - } - } - } - } - } } } } From 4b8a45df5ae3f941cfe34ac443f86e843faa37e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 4 Feb 2025 14:39:55 +0000 Subject: [PATCH 02/21] storage: generate model only if possible --- service/lib/agama/storage/proposal.rb | 33 +++- service/test/agama/storage/proposal_test.rb | 204 ++++++++++++++------ 2 files changed, 173 insertions(+), 64 deletions(-) diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index 394cfd68ef..c132d93e56 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022-2024] SUSE LLC +# Copyright (c) [2022-2025] SUSE LLC # # All Rights Reserved. # @@ -21,6 +21,7 @@ require "agama/issue" require "agama/storage/actions_generator" +require "agama/storage/config_checker" require "agama/storage/config_conversions" require "agama/storage/config_solver" require "agama/storage/proposal_settings" @@ -93,10 +94,16 @@ def storage_json # Config model according to the JSON schema. # - # @return [Hash, nil] nil if there is no config. + # The config model is generated only if the config has not errors and all the config features + # are supported by the model. + # + # @return [Hash, nil] nil if the config model cannot be generated. def model_json config = config(solved: true) - return unless config + return unless config && model_supported?(config) + + issues = Agama::Storage::ConfigChecker.new(config, product_config).issues + return if issues.any?(&:error?) ConfigConversions::ToModel.new(config).convert end @@ -282,6 +289,26 @@ def config(solved: false) solved ? strategy.config : source_config end + # Whether the config model supports all features of the given config. + # + # @param config [Storage::Config] + # @return [Boolean] + def model_supported?(config) + unsupported_configs = [ + config.volume_groups, + config.md_raids, + config.btrfs_raids, + config.nfs_mounts + ].flatten + + encryptable_configs = [ + config.drives, + config.partitions + ].flatten + + unsupported_configs.empty? && encryptable_configs.none?(&:encryption) + end + # Calculates a proposal from guided JSON settings. # # @param guided_json [Hash] e.g., { "target": { "disk": "/dev/vda" } }. diff --git a/service/test/agama/storage/proposal_test.rb b/service/test/agama/storage/proposal_test.rb index b3df1a5052..3992ca175e 100644 --- a/service/test/agama/storage/proposal_test.rb +++ b/service/test/agama/storage/proposal_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022-2024] SUSE LLC +# Copyright (c) [2022-2025] SUSE LLC # # All Rights Reserved. # @@ -335,83 +335,165 @@ def drive(partitions) end end + context "if an AutoYaST proposal has been calculated" do + before do + subject.calculate_from_json(autoyast_json) + end + + let(:autoyast_json) do + { + legacyAutoyastStorage: [ + { device: "/dev/vda" } + ] + } + end + + it "returns nil" do + expect(subject.model_json).to be_nil + end + end + context "if an agama proposal has been calculated" do before do subject.calculate_from_json(config_json) end - let(:config_json) do - { - storage: { - drives: [ - { - alias: "root", - partitions: [ - { - filesystem: { path: "/" } + context "and the config contains an encrypted drive" do + let(:config_json) do + { + storage: { + drives: [ + { + encryption: { + luks1: { password: "12345" } } - ] - } - ] + } + ] + } } - } + end + + it "returns nil" do + expect(subject.model_json).to be_nil + end end - it "returns the config model" do - expect(subject.model_json).to eq( + context "and the config contains an encrypted partition" do + let(:config_json) do { - boot: { - configure: true, - device: { - default: true, - name: "/dev/sda" - } - }, - drives: [ - { - name: "/dev/sda", - alias: "root", - spacePolicy: "keep", - partitions: [ - { - mountPath: "/", - filesystem: { - reuse: false, - default: true, - type: "ext4" - }, - size: { - default: true, - min: 0 - }, - delete: false, - deleteIfNeeded: false, - resize: false, - resizeIfNeeded: false - } - ] - } - ] + storage: { + drives: [ + { + partitions: [ + { + encryption: { + luks1: { password: "12345" } + } + } + ] + } + ] + } } - ) + end + + it "returns nil" do + expect(subject.model_json).to be_nil + end end - end - context "if an AutoYaST proposal has been calculated" do - before do - subject.calculate_from_json(autoyast_json) + context "and the config contains volume groups" do + let(:config_json) do + { + storage: { + volumeGroups: [ + { + name: "vg0" + } + ] + } + } + end + + it "returns nil" do + expect(subject.model_json).to be_nil + end end - let(:autoyast_json) do - { - legacyAutoyastStorage: [ - { device: "/dev/vda" } - ] - } + context "and the config has errors" do + let(:config_json) do + { + storage: { + drives: [ + { + name: "unknown" + } + ] + } + } + end + + it "returns nil" do + expect(subject.model_json).to be_nil + end end - it "returns nil" do - expect(subject.model_json).to be_nil + context "and the config has not errors" do + let(:config_json) do + { + storage: { + drives: [ + { + alias: "root", + partitions: [ + { + filesystem: { path: "/" } + } + ] + } + ] + } + } + end + + it "returns the config model" do + expect(subject.model_json).to eq( + { + boot: { + configure: true, + device: { + default: true, + name: "/dev/sda" + } + }, + drives: [ + { + name: "/dev/sda", + alias: "root", + spacePolicy: "keep", + partitions: [ + { + mountPath: "/", + filesystem: { + reuse: false, + default: true, + type: "ext4" + }, + size: { + default: true, + min: 0 + }, + delete: false, + deleteIfNeeded: false, + resize: false, + resizeIfNeeded: false + } + ] + } + ] + } + ) + end end end end From 954b08358863e4dee61aa7616768ee92846f1e0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Feb 2025 16:02:53 +0000 Subject: [PATCH 03/21] web: improve useAvailableDevices query --- web/src/queries/storage.ts | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/web/src/queries/storage.ts b/web/src/queries/storage.ts index 9068153cd6..2c710107fa 100644 --- a/web/src/queries/storage.ts +++ b/web/src/queries/storage.ts @@ -50,12 +50,6 @@ const devicesQuery = (scope: "result" | "system") => ({ staleTime: Infinity, }); -const usableDevicesQuery = { - queryKey: ["storage", "usableDevices"], - queryFn: fetchUsableDevices, - staleTime: Infinity, -}; - const productParamsQuery = { queryKey: ["storage", "encryptionMethods"], queryFn: fetchProductParams, @@ -107,22 +101,34 @@ const useDevices = ( return data; }; -/** - * Hook that returns the list of available devices for installation. - */ -const useAvailableDevices = (): StorageDevice[] => { - const findDevice = (devices: StorageDevice[], sid: number) => { +const availableDevices = async (devices: StorageDevice[]): Promise => { + const findDevice = (devices: StorageDevice[], sid: number): StorageDevice | undefined => { const device = devices.find((d) => d.sid === sid); - if (device === undefined) console.warn("Device not found:", sid); return device; }; - const devices = useDevices("system", { suspense: true }); - const { data } = useSuspenseQuery(usableDevicesQuery); + const availableDevices = await fetchUsableDevices(); + + return availableDevices + .map((sid: number) => findDevice(devices, sid)) + .filter((d: StorageDevice | undefined) => d); +}; - return data.map((sid) => findDevice(devices, sid)).filter((d) => d); +const availableDevicesQuery = (devices: StorageDevice[]) => ({ + queryKey: ["storage", "availableDevices"], + queryFn: () => availableDevices(devices), + staleTime: Infinity, +}); + +/** + * Hook that returns the list of available devices for installation. + */ +const useAvailableDevices = (): StorageDevice[] => { + const devices = useDevices("system", { suspense: true }); + const { data } = useSuspenseQuery(availableDevicesQuery(devices)); + return data; }; /** From 0712819c41473b330097589bec9b4ed6a29ffb6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 6 Feb 2025 06:45:46 +0000 Subject: [PATCH 04/21] web: improve query for getting the volumes --- web/src/api/storage.ts | 35 ++---------- web/src/components/storage/PartitionPage.tsx | 4 +- .../components/storage/ProposalPage.test.tsx | 9 +-- .../storage/ProposalTransactionalInfo.tsx | 4 +- web/src/components/storage/utils.ts | 4 +- web/src/queries/storage.ts | 56 +++++++++---------- web/src/queries/storage/config-model.ts | 8 +-- web/src/types/storage.ts | 41 +------------- 8 files changed, 48 insertions(+), 113 deletions(-) diff --git a/web/src/api/storage.ts b/web/src/api/storage.ts index 91907b490b..b2355c2ff2 100644 --- a/web/src/api/storage.ts +++ b/web/src/api/storage.ts @@ -23,8 +23,6 @@ import { get, post, put } from "~/api/http"; import { Job } from "~/types/job"; import { Action, config, configModel, ProductParams, Volume } from "~/api/storage/types"; -import { fetchDevices } from "~/api/storage/devices"; -import { StorageDevice, Volume as StorageVolume, VolumeTarget } from "~/types/storage"; /** * Starts the storage probing process. @@ -54,37 +52,13 @@ const fetchUsableDevices = (): Promise => get(`/api/storage/proposal/u const fetchProductParams = (): Promise => get("/api/storage/product/params"); -const fetchDefaultVolume = (mountPath: string): Promise => { +const fetchVolume = (mountPath: string): Promise => { const path = encodeURIComponent(mountPath); return get(`/api/storage/product/volume_for?mount_path=${path}`); }; -const fetchVolumeTemplates = async (): Promise => { - const buildVolume = ( - rawVolume: Volume, - devices: StorageDevice[], - productMountPoints: string[], - ): StorageVolume => ({ - ...rawVolume, - outline: { - ...rawVolume.outline, - // Indicate whether a volume is defined by the product. - productDefined: productMountPoints.includes(rawVolume.mountPath), - }, - minSize: rawVolume.minSize || 0, - transactional: rawVolume.transactional || false, - target: rawVolume.target as VolumeTarget, - targetDevice: devices.find((d) => d.name === rawVolume.targetDevice), - }); - - const systemDevices = await fetchDevices("system"); - const product = await fetchProductParams(); - const mountPoints = ["", ...product.mountPoints]; - const rawVolumes = await Promise.all(mountPoints.map(fetchDefaultVolume)); - return rawVolumes - .filter((v) => v !== undefined) - .map((v) => buildVolume(v, systemDevices, product.mountPoints)); -}; +const fetchVolumes = (mountPaths: string[]): Promise => + Promise.all(mountPaths.map(fetchVolume)); const fetchActions = (): Promise => get("/api/storage/devices/actions"); @@ -109,8 +83,7 @@ export { solveConfigModel, fetchUsableDevices, fetchProductParams, - fetchDefaultVolume, - fetchVolumeTemplates, + fetchVolumes, fetchActions, fetchStorageJobs, findStorageJob, diff --git a/web/src/components/storage/PartitionPage.tsx b/web/src/components/storage/PartitionPage.tsx index 815fed3c41..179a906245 100644 --- a/web/src/components/storage/PartitionPage.tsx +++ b/web/src/components/storage/PartitionPage.tsx @@ -52,11 +52,11 @@ import { useSolvedConfigModel, addPartition, } from "~/queries/storage/config-model"; -import { StorageDevice, Volume } from "~/types/storage"; +import { StorageDevice } from "~/types/storage"; import { baseName, deviceSize, parseToBytes } from "~/components/storage/utils"; import { _, formatList } from "~/i18n"; import { sprintf } from "sprintf-js"; -import { configModel } from "~/api/storage/types"; +import { configModel, Volume } from "~/api/storage/types"; import { STORAGE as PATHS } from "~/routes/paths"; import { compact } from "~/utils"; diff --git a/web/src/components/storage/ProposalPage.test.tsx b/web/src/components/storage/ProposalPage.test.tsx index 9b6901952a..be52b51564 100644 --- a/web/src/components/storage/ProposalPage.test.tsx +++ b/web/src/components/storage/ProposalPage.test.tsx @@ -29,7 +29,8 @@ import React from "react"; import { screen } from "@testing-library/react"; import { installerRender } from "~/test-utils"; import ProposalPage from "~/components/storage/ProposalPage"; -import { Action, StorageDevice, Volume, VolumeTarget } from "~/types/storage"; +import { Action, StorageDevice } from "~/types/storage"; +import { Volume } from "~/api/storage/types"; jest.mock("~/queries/issues", () => ({ ...jest.requireActual("~/queries/issues"), @@ -79,7 +80,8 @@ const vdb: StorageDevice = { const volume = (mountPath: string): Volume => { return { mountPath, - target: VolumeTarget.DEFAULT, + mountOptions: [], + target: "default", fsType: "Btrfs", minSize: 1024, maxSize: 1024, @@ -94,7 +96,6 @@ const volume = (mountPath: string): Volume => { snapshotsAffectSizes: false, sizeRelevantVolumes: [], adjustByRam: false, - productDefined: false, }, }; }; @@ -106,7 +107,7 @@ jest.mock("~/queries/storage", () => ({ useDevices: () => [vda, vdb], useAvailableDevices: () => [vda, vdb], useVolumeDevices: () => [vda, vdb], - useVolumeTemplates: () => [volume("/")], + useVolumes: () => [volume("/")], useProductParams: () => ({ encryptionMethods: [], mountPoints: ["/", "swap"], diff --git a/web/src/components/storage/ProposalTransactionalInfo.tsx b/web/src/components/storage/ProposalTransactionalInfo.tsx index 2a5d3e9321..a0fcf86e7e 100644 --- a/web/src/components/storage/ProposalTransactionalInfo.tsx +++ b/web/src/components/storage/ProposalTransactionalInfo.tsx @@ -25,7 +25,7 @@ import { Alert } from "@patternfly/react-core"; import { _ } from "~/i18n"; import { sprintf } from "sprintf-js"; import { useProduct } from "~/queries/software"; -import { useVolumeTemplates } from "~/queries/storage"; +import { useVolumes } from "~/queries/storage"; import { isTransactionalSystem } from "~/components/storage/utils"; /** @@ -36,7 +36,7 @@ import { isTransactionalSystem } from "~/components/storage/utils"; */ export default function ProposalTransactionalInfo() { const { selectedProduct } = useProduct({ suspense: true }); - const volumes = useVolumeTemplates(); + const volumes = useVolumes(); if (!isTransactionalSystem(volumes)) return; diff --git a/web/src/components/storage/utils.ts b/web/src/components/storage/utils.ts index 559f0cfd14..fc598ca03c 100644 --- a/web/src/components/storage/utils.ts +++ b/web/src/components/storage/utils.ts @@ -32,8 +32,8 @@ import xbytes from "xbytes"; import { _, N_ } from "~/i18n"; -import { PartitionSlot, StorageDevice, Volume } from "~/types/storage"; -import { configModel } from "~/api/storage/types"; +import { PartitionSlot, StorageDevice } from "~/types/storage"; +import { configModel, Volume } from "~/api/storage/types"; import { sprintf } from "sprintf-js"; /** diff --git a/web/src/queries/storage.ts b/web/src/queries/storage.ts index 2c710107fa..e537903aa2 100644 --- a/web/src/queries/storage.ts +++ b/web/src/queries/storage.ts @@ -26,16 +26,15 @@ import { fetchConfig, setConfig, fetchActions, - fetchVolumeTemplates, + fetchVolumes, fetchProductParams, fetchUsableDevices, reprobe, } from "~/api/storage"; import { fetchDevices, fetchDevicesDirty } from "~/api/storage/devices"; import { useInstallerClient } from "~/context/installer"; -import { config, ProductParams } from "~/api/storage/types"; -import { Action, StorageDevice, Volume } from "~/types/storage"; - +import { config, ProductParams, Volume } from "~/api/storage/types"; +import { Action, StorageDevice } from "~/types/storage"; import { QueryHookOptions } from "~/types/queries"; const configQuery = { @@ -44,24 +43,6 @@ const configQuery = { staleTime: Infinity, }; -const devicesQuery = (scope: "result" | "system") => ({ - queryKey: ["storage", "devices", scope], - queryFn: () => fetchDevices(scope), - staleTime: Infinity, -}); - -const productParamsQuery = { - queryKey: ["storage", "encryptionMethods"], - queryFn: fetchProductParams, - staleTime: Infinity, -}; - -const volumeTemplatesQuery = { - queryKey: ["storage", "volumeTemplates"], - queryFn: fetchVolumeTemplates, - staleTime: Infinity, -}; - /** * Hook that returns the unsolved config. */ @@ -85,6 +66,12 @@ const useConfigMutation = () => { return useMutation(query); }; +const devicesQuery = (scope: "result" | "system") => ({ + queryKey: ["storage", "devices", scope], + queryFn: () => fetchDevices(scope), + staleTime: Infinity, +}); + /** * Hook that returns the list of storage devices for the given scope. * @@ -131,6 +118,12 @@ const useAvailableDevices = (): StorageDevice[] => { return data; }; +const productParamsQuery = { + queryKey: ["storage", "encryptionMethods"], + queryFn: fetchProductParams, + staleTime: Infinity, +}; + /** * Hook that returns the product parameters (e.g., mount points). */ @@ -140,19 +133,26 @@ const useProductParams = (options?: QueryHookOptions): ProductParams => { return data; }; +const volumesQuery = (mountPaths: string[]) => ({ + queryKey: ["storage", "volumes"], + queryFn: () => fetchVolumes(mountPaths), + staleTime: Infinity, +}); + /** - * Hook that returns the volume templates for the current product. + * Hook that returns the volumes for the current product. */ -const useVolumeTemplates = (): Volume[] => { - const { data } = useSuspenseQuery(volumeTemplatesQuery); +const useVolumes = (): Volume[] => { + const product = useProductParams({ suspense: true }); + const mountPoints = ["", ...product.mountPoints]; + const { data } = useSuspenseQuery(volumesQuery(mountPoints)); return data; }; function useVolume(mountPoint: string): Volume { - const volumes = useVolumeTemplates(); + const volumes = useVolumes(); const volume = volumes.find((v) => v.mountPath === mountPoint); const defaultVolume = volumes.find((v) => v.mountPath === ""); - return volume || defaultVolume; } @@ -250,7 +250,7 @@ export { useDevices, useAvailableDevices, useProductParams, - useVolumeTemplates, + useVolumes, useVolume, useVolumeDevices, useActions, diff --git a/web/src/queries/storage/config-model.ts b/web/src/queries/storage/config-model.ts index 9644afc2b0..ab267f292d 100644 --- a/web/src/queries/storage/config-model.ts +++ b/web/src/queries/storage/config-model.ts @@ -22,10 +22,10 @@ import { useMutation, useQuery, useQueryClient, useSuspenseQuery } from "@tanstack/react-query"; import { fetchConfigModel, setConfigModel, solveConfigModel } from "~/api/storage"; -import { configModel } from "~/api/storage/types"; +import { configModel, Volume } from "~/api/storage/types"; import { QueryHookOptions } from "~/types/queries"; -import { SpacePolicyAction, Volume } from "~/types/storage"; -import { useVolumeTemplates } from "~/queries/storage"; +import { SpacePolicyAction } from "~/types/storage"; +import { useVolumes } from "~/queries/storage"; function copyModel(model: configModel.Config): configModel.Config { return JSON.parse(JSON.stringify(model)); @@ -415,7 +415,7 @@ export type ModelHook = { export function useModel(): ModelHook { const model = useConfigModel(); const { mutate } = useConfigModelMutation(); - const volumes = useVolumeTemplates(); + const volumes = useVolumes(); return { model, diff --git a/web/src/types/storage.ts b/web/src/types/storage.ts index ebba8e7724..6167e4591a 100644 --- a/web/src/types/storage.ts +++ b/web/src/types/storage.ts @@ -97,43 +97,6 @@ type SpacePolicyAction = { value: "delete" | "resizeIfNeeded"; }; -type Volume = { - mountPath: string; - target: VolumeTarget; - targetDevice?: StorageDevice; - fsType: string; - minSize: number; - maxSize?: number; - autoSize: boolean; - snapshots: boolean; - transactional: boolean; - outline: VolumeOutline; -}; - -type VolumeOutline = { - required: boolean; - productDefined: boolean; - fsTypes: string[]; - adjustByRam: boolean; - supportAutoSize: boolean; - snapshotsConfigurable: boolean; - snapshotsAffectSizes: boolean; - sizeRelevantVolumes: string[]; -}; - -/** - * Enum for the possible volume targets. - * - * @readonly - */ -enum VolumeTarget { - DEFAULT = "default", - NEW_PARTITION = "new_partition", - NEW_VG = "new_vg", - DEVICE = "device", - FILESYSTEM = "filesystem", -} - /** * Enum for the encryption method values * @@ -173,8 +136,6 @@ export type { ShrinkingInfo, SpacePolicyAction, StorageDevice, - Volume, - VolumeOutline, }; -export { EncryptionMethods, VolumeTarget }; +export { EncryptionMethods }; From b79c909971531cdf20228a8c849c52f5361d73f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 7 Feb 2025 14:26:44 +0000 Subject: [PATCH 05/21] web: rename query --- web/src/queries/storage.ts | 6 +++--- web/src/queries/storage/zfcp.ts | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/web/src/queries/storage.ts b/web/src/queries/storage.ts index e537903aa2..c34c01c47f 100644 --- a/web/src/queries/storage.ts +++ b/web/src/queries/storage.ts @@ -185,7 +185,7 @@ const useVolumeDevices = (): StorageDevice[] => { return [...availableDevices, ...mds, ...vgs]; }; -const proposalActionsQuery = { +const actionsQuery = { queryKey: ["storage", "devices", "actions"], queryFn: fetchActions, }; @@ -193,8 +193,8 @@ const proposalActionsQuery = { /** * Hook that returns the actions to perform in the storage devices. */ -const useActions = (): Action[] | undefined => { - const { data } = useSuspenseQuery(proposalActionsQuery); +const useActions = (): Action[] => { + const { data } = useSuspenseQuery(actionsQuery); return data; }; diff --git a/web/src/queries/storage/zfcp.ts b/web/src/queries/storage/zfcp.ts index b8c180c0e6..294fde95b8 100644 --- a/web/src/queries/storage/zfcp.ts +++ b/web/src/queries/storage/zfcp.ts @@ -70,12 +70,13 @@ const useZFCPDisks = (): ZFCPDisk[] => { }; /** - * Hook that returns zFCP config. + * Hook that returns whether zFCP is supported. */ const useZFCPSupported = (): boolean => { const { data: supported } = useSuspenseQuery(zfcpSupportedQuery); return supported; }; + /** * Hook that returns zFCP config. */ From 3397c573aa9ea5656255fcf531dffe3ee1708211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 7 Feb 2025 14:27:22 +0000 Subject: [PATCH 06/21] web: add useDASDSupported query --- web/src/queries/storage/dasd.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/web/src/queries/storage/dasd.ts b/web/src/queries/storage/dasd.ts index e044ba0ad6..85eab88388 100644 --- a/web/src/queries/storage/dasd.ts +++ b/web/src/queries/storage/dasd.ts @@ -28,6 +28,7 @@ import { enableDiag, fetchDASDDevices, formatDASD, + supportedDASD, } from "~/api/storage/dasd"; import { useInstallerClient } from "~/context/installer"; import React from "react"; @@ -51,6 +52,19 @@ const useDASDDevices = () => { return devices.map((d) => ({ ...d, hexId: hex(d.id) })); }; +const dasdSupportedQuery = { + queryKey: ["dasd", "supported"], + queryFn: supportedDASD, +}; + +/** + * Hook that returns whether DASD is supported. + */ +const useDASDSupported = (): boolean => { + const { data: supported } = useSuspenseQuery(dasdSupportedQuery); + return supported; +}; + /** * Returns a query for retrieving the running dasd format jobs */ @@ -239,6 +253,7 @@ const useFormatDASDMutation = () => { export { useDASDDevices, + useDASDSupported, useDASDDevicesChanges, useDASDFormatJobChanges, useDASDRunningFormatJobs, From a0762f978db8ca7595a02b5cccce4524c212de6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 7 Feb 2025 14:36:54 +0000 Subject: [PATCH 07/21] web: wait for config mutation --- web/src/queries/storage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/queries/storage.ts b/web/src/queries/storage.ts index c34c01c47f..d07095f265 100644 --- a/web/src/queries/storage.ts +++ b/web/src/queries/storage.ts @@ -59,7 +59,7 @@ const useConfig = (options?: QueryHookOptions): config.Config => { const useConfigMutation = () => { const queryClient = useQueryClient(); const query = { - mutationFn: (config: config.Config) => setConfig(config), + mutationFn: async (config: config.Config) => await setConfig(config), onSuccess: () => queryClient.invalidateQueries({ queryKey: ["storage"] }), }; From 9cfc9c7bcad2024c451b04582e7aa436d85e8623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 7 Feb 2025 14:28:17 +0000 Subject: [PATCH 08/21] web: adapt proposal page layout according to the scenario --- .../components/storage/ProposalPage.test.tsx | 303 +++++++++++++----- web/src/components/storage/ProposalPage.tsx | 228 +++++++++---- .../storage/ProposalResultTable.tsx | 2 +- 3 files changed, 398 insertions(+), 135 deletions(-) diff --git a/web/src/components/storage/ProposalPage.test.tsx b/web/src/components/storage/ProposalPage.test.tsx index be52b51564..acf071c503 100644 --- a/web/src/components/storage/ProposalPage.test.tsx +++ b/web/src/components/storage/ProposalPage.test.tsx @@ -29,39 +29,9 @@ import React from "react"; import { screen } from "@testing-library/react"; import { installerRender } from "~/test-utils"; import ProposalPage from "~/components/storage/ProposalPage"; -import { Action, StorageDevice } from "~/types/storage"; -import { Volume } from "~/api/storage/types"; +import { StorageDevice } from "~/types/storage"; -jest.mock("~/queries/issues", () => ({ - ...jest.requireActual("~/queries/issues"), - useIssuesChanges: jest.fn(), - useIssues: () => [], -})); - -jest.mock("./ProposalResultSection", () => () =>
result section
); -jest.mock("./ProposalTransactionalInfo", () => () =>
trasactional info
); - -const vda: StorageDevice = { - sid: 59, - type: "disk", - isDrive: true, - description: "", - vendor: "Micron", - model: "Micron 1100 SATA", - driver: ["ahci", "mmcblk"], - bus: "IDE", - transport: "usb", - dellBOSS: false, - sdCard: true, - active: true, - name: "/dev/vda", - size: 1e12, - systems: ["Windows 11", "openSUSE Leap 15.2"], - udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"], - udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], -}; - -const vdb: StorageDevice = { +const disk: StorageDevice = { sid: 60, type: "disk", isDrive: true, @@ -70,55 +40,236 @@ const vdb: StorageDevice = { model: "Unknown", driver: ["ahci", "mmcblk"], bus: "IDE", - name: "/dev/vdb", + name: "/dev/vda", size: 1e6, }; -/** - * Returns a volume specification with the given path. - */ -const volume = (mountPath: string): Volume => { - return { - mountPath, - mountOptions: [], - target: "default", - fsType: "Btrfs", - minSize: 1024, - maxSize: 1024, - autoSize: false, - snapshots: false, - transactional: false, - outline: { - required: false, - fsTypes: ["Btrfs"], - supportAutoSize: false, - snapshotsConfigurable: false, - snapshotsAffectSizes: false, - sizeRelevantVolumes: [], - adjustByRam: false, - }, - }; -}; - -const mockActions: Action[] = []; - +const mockUseAvailableDevices = jest.fn(); +const mockUseConfigMutation = jest.fn(); +const mockUseDeprecated = jest.fn(); +const mockUseDeprecatedChanges = jest.fn(); +const mockUseReprobeMutation = jest.fn(); jest.mock("~/queries/storage", () => ({ ...jest.requireActual("~/queries/storage"), - useDevices: () => [vda, vdb], - useAvailableDevices: () => [vda, vdb], - useVolumeDevices: () => [vda, vdb], - useVolumes: () => [volume("/")], - useProductParams: () => ({ - encryptionMethods: [], - mountPoints: ["/", "swap"], - }), - useActions: () => mockActions, - useDeprecated: () => false, - useDeprecatedChanges: jest.fn(), - useProposalMutation: jest.fn(), + useAvailableDevices: () => mockUseAvailableDevices(), + useConfigMutation: () => mockUseConfigMutation(), + useDeprecated: () => mockUseDeprecated(), + useDeprecatedChanges: () => mockUseDeprecatedChanges(), + useReprobeMutation: () => mockUseReprobeMutation(), +})); + +const mockUseConfigModel = jest.fn(); +jest.mock("~/queries/storage/config-model", () => ({ + ...jest.requireActual("~/queries/storage/config-model"), + useConfigModel: () => mockUseConfigModel(), +})); + +const mockUseZFCPSupported = jest.fn(); +jest.mock("~/queries/storage/zfcp", () => ({ + ...jest.requireActual("~/queries/storage/zfcp"), + useZFCPSupported: () => mockUseZFCPSupported(), +})); + +const mockUseDASDSupported = jest.fn(); +jest.mock("~/queries/storage/dasd", () => ({ + ...jest.requireActual("~/queries/storage/dasd"), + useDASDSupported: () => mockUseDASDSupported(), +})); + +const mockUseIssues = jest.fn(); +jest.mock("~/queries/issues", () => ({ + ...jest.requireActual("~/queries/issues"), + useIssues: () => mockUseIssues(), })); -it("renders the device, settings and result sections", () => { - installerRender(); - screen.findByText("Device"); +jest.mock("./ProposalResultSection", () => () =>
result
); +jest.mock("./ProposalTransactionalInfo", () => () =>
trasactional info
); +jest.mock("./ConfigEditor", () => () =>
installation devices
); +jest.mock("./AddExistingDeviceMenu", () => () =>
add device menu
); +jest.mock("./ConfigEditorMenu", () => () =>
config editor menu
); + +beforeEach(() => { + mockUseConfigMutation.mockReturnValue({ mutate: jest.fn() }); + mockUseReprobeMutation.mockReturnValue({ mutateAsync: jest.fn() }); + mockUseDeprecated.mockReturnValue(false); + mockUseIssues.mockReturnValue([]); +}); + +describe("if there are not devices", () => { + beforeEach(() => { + mockUseAvailableDevices.mockReturnValue([]); + }); + + it("renders an option for activating iSCSI", () => { + installerRender(); + expect(screen.queryByRole("link", { name: "Activate iSCSI" })).toBeInTheDocument(); + }); + + it("does not render the installation devices", () => { + installerRender(); + expect(screen.queryByText("installation devices")).not.toBeInTheDocument(); + }); + + it("does not render the result", () => { + installerRender(); + expect(screen.queryByText("result")).not.toBeInTheDocument(); + }); + + describe("if zFCP is not supported", () => { + beforeEach(() => { + mockUseZFCPSupported.mockReturnValue(false); + }); + + it("does not render an option for activating zFCP", () => { + installerRender(); + expect(screen.queryByRole("link", { name: "Activate zFCP" })).not.toBeInTheDocument(); + }); + }); + + describe("if DASD is not supported", () => { + beforeEach(() => { + mockUseDASDSupported.mockReturnValue(false); + }); + + it("does not render an option for activating DASD", () => { + installerRender(); + expect(screen.queryByRole("link", { name: "Activate DASD" })).not.toBeInTheDocument(); + }); + }); + + describe("if zFCP is supported", () => { + beforeEach(() => { + mockUseZFCPSupported.mockReturnValue(true); + }); + + it("renders an option for activating zFCP", () => { + installerRender(); + expect(screen.queryByRole("link", { name: "Activate zFCP" })).toBeInTheDocument(); + }); + }); + + describe("if DASD is supported", () => { + beforeEach(() => { + mockUseDASDSupported.mockReturnValue(true); + }); + + it("renders an option for activating DASD", () => { + installerRender(); + expect(screen.queryByRole("link", { name: "Activate DASD" })).toBeInTheDocument(); + }); + }); +}); + +describe("if there is not model", () => { + beforeEach(() => { + mockUseAvailableDevices.mockReturnValue([disk]); + mockUseConfigModel.mockReturnValue(null); + }); + + describe("and there are errors", () => { + beforeEach(() => { + mockUseIssues.mockReturnValue([ + { + description: "Error", + kind: "storage", + details: "", + source: 2, + severity: 1, + }, + ]); + }); + + it("renders an option for reseting the config", () => { + installerRender(); + expect(screen.queryByRole("button", { name: "Reset" })).toBeInTheDocument(); + }); + + it("does not render the installation devices", () => { + installerRender(); + expect(screen.queryByText("installation devices")).not.toBeInTheDocument(); + }); + + it("does not render the result", () => { + installerRender(); + expect(screen.queryByText("result")).not.toBeInTheDocument(); + }); + }); + + describe("and there are not errors", () => { + beforeEach(() => { + mockUseIssues.mockReturnValue([]); + }); + + it("renders an option for reseting the config", () => { + installerRender(); + expect(screen.queryByRole("button", { name: "Reset" })).toBeInTheDocument(); + }); + + it("does not render the installation devices", async () => { + installerRender(); + expect(screen.queryByText("installation devices")).not.toBeInTheDocument(); + }); + + it("renders the result", () => { + installerRender(); + expect(screen.queryByText("result")).toBeInTheDocument(); + }); + }); +}); + +describe("if there is a model", () => { + beforeEach(() => { + mockUseAvailableDevices.mockReturnValue([disk]); + mockUseConfigModel.mockReturnValue({ drives: [] }); + }); + + describe("and there are errors", () => { + beforeEach(() => { + mockUseIssues.mockReturnValue([ + { + description: "Error", + kind: "storage", + details: "", + source: 2, + severity: 1, + }, + ]); + }); + + it("renders an error message", () => { + installerRender(); + expect(screen.queryByText(/Adjust the settings/)).toBeInTheDocument(); + }); + + it("renders the installation devices", () => { + installerRender(); + expect(screen.queryByText("installation devices")).toBeInTheDocument(); + }); + + it("does not render the result", () => { + installerRender(); + expect(screen.queryByText("result")).not.toBeInTheDocument(); + }); + }); + + describe("and there are not errors", () => { + beforeEach(() => { + mockUseIssues.mockReturnValue([]); + }); + + it("does not render an error message", () => { + installerRender(); + expect(screen.queryByText(/Adjust the settings/)).not.toBeInTheDocument(); + }); + + it("renders the installation devices", () => { + installerRender(); + expect(screen.queryByText("installation devices")).toBeInTheDocument(); + }); + + it("renders the result", () => { + installerRender(); + expect(screen.queryByText("result")).toBeInTheDocument(); + }); + }); }); diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index 9c3e058332..222eeecd1d 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -21,24 +21,176 @@ */ import React from "react"; -import { Content, Grid, GridItem, SplitItem } from "@patternfly/react-core"; -import { Page } from "~/components/core/"; -import { Loading } from "~/components/layout"; -import EncryptionField from "~/components/storage/EncryptionField"; +import { + Alert, + Button, + Content, + Grid, + GridItem, + Split, + SplitItem, + EmptyState, + EmptyStateBody, + EmptyStateFooter, +} from "@patternfly/react-core"; +import { Page, Link } from "~/components/core/"; +import { Icon, Loading } from "~/components/layout"; import ProposalResultSection from "./ProposalResultSection"; import ProposalTransactionalInfo from "./ProposalTransactionalInfo"; import ProposalFailedInfo from "./ProposalFailedInfo"; import ConfigEditor from "./ConfigEditor"; import ConfigEditorMenu from "./ConfigEditorMenu"; import AddExistingDeviceMenu from "./AddExistingDeviceMenu"; -import { toValidationError } from "~/utils"; -import { useIssues } from "~/queries/issues"; import { IssueSeverity } from "~/types/issues"; -import { useDeprecated, useDeprecatedChanges, useReprobeMutation } from "~/queries/storage"; +import { + useAvailableDevices, + useConfigMutation, + useDeprecated, + useDeprecatedChanges, + useReprobeMutation, +} from "~/queries/storage"; +import { useConfigModel } from "~/queries/storage/config-model"; +import { useZFCPSupported } from "~/queries/storage/zfcp"; +import { useDASDSupported } from "~/queries/storage/dasd"; +import { useIssues } from "~/queries/issues"; +import { STORAGE as PATHS } from "~/routes/paths"; import { _ } from "~/i18n"; -export default function ProposalPage() { +/** @todo Call to an API method to reset the default config instead of setting a config. */ +function useResetConfig() { + const { mutate } = useConfigMutation(); + + return () => + mutate({ + drives: [ + { + partitions: [{ search: "*", delete: true }, { generate: "default" }], + }, + ], + }); +} + +function ResetEmptyState() { + const reset = useResetConfig(); + + const description = _( + "The current storage config cannot be edited. Do you want to reset to the default config?", + ); + return ( + } + status="warning" + > + {description} + + + + + ); +} + +function DevicesEmptyState() { + const isZFCPSupported = useZFCPSupported(); + const isDASDSupported = useDASDSupported(); + + const description = _( + "There are not devices available for the installation. Do you want to activate devices?", + ); + + return ( + } + status="warning" + > + {description} + + + + + {_("Activate iSCSI")} + + + {isZFCPSupported && ( + + + {_("Activate zFCP")} + + + )} + {isDASDSupported && ( + + + {_("Activate DASD")} + + + )} + + + + ); +} + +type ProposalSectionsProps = { + isEditable: boolean; + isValid: boolean; +}; + +function ProposalSections({ isEditable, isValid }: ProposalSectionsProps): React.ReactNode { + const reset = useResetConfig(); + + return ( + + + + {!isEditable && ( + + <> + {_( + "The current storage config cannot be edited. Do you want to reset to the default config?", + )} + + + + )} + {isEditable && ( + + + + + + + + + + + } + > + + + + )} + {isValid && } + + ); +} + +export default function ProposalPage(): React.ReactNode { const isDeprecated = useDeprecated(); + const model = useConfigModel({ suspense: true }); + const devices = useAvailableDevices(); + const issues = useIssues("storage"); const { mutateAsync: reprobe } = useReprobeMutation(); useDeprecatedChanges(); @@ -47,63 +199,23 @@ export default function ProposalPage() { if (isDeprecated) reprobe().catch(console.log); }, [isDeprecated, reprobe]); - const errors = useIssues("storage") - .filter((s) => s.severity === IssueSeverity.Error) - .map(toValidationError); - - const validProposal = errors.length === 0; - - if (isDeprecated) { - return ( - - - {_("Storage")} - - - - - - - ); - } + const errors = issues.filter((s) => s.severity === IssueSeverity.Error); + const isValid = !errors.length; + const isEditable = !!model; + const isDevicesEmpty = !devices.length; return ( {_("Storage")} - - - - - - - - - - - - - - - - } - > - - - - - - - {validProposal && } - + {isDeprecated && } + {!isDeprecated && !isDevicesEmpty && (isEditable || isValid) && ( + + )} + {!isDeprecated && !isDevicesEmpty && !isEditable && !isValid && } + {!isDeprecated && isDevicesEmpty && } ); diff --git a/web/src/components/storage/ProposalResultTable.tsx b/web/src/components/storage/ProposalResultTable.tsx index 9352122d71..66e3cfc085 100644 --- a/web/src/components/storage/ProposalResultTable.tsx +++ b/web/src/components/storage/ProposalResultTable.tsx @@ -146,7 +146,7 @@ type ProposalResultTableProps = { */ export default function ProposalResultTable({ devicesManager }: ProposalResultTableProps) { const model = useConfigModel({ suspense: true }); - const devices = devicesManager.usedDevices(model.drives.map((d) => d.name)); + const devices = devicesManager.usedDevices(model?.drives.map((d) => d.name) || []); return ( Date: Tue, 11 Feb 2025 12:42:01 +0000 Subject: [PATCH 09/21] web: fix tests --- web/src/components/storage/PartitionPage.test.tsx | 8 ++++---- .../storage/ProposalTransactionalInfo.test.tsx | 10 +++++----- web/src/components/storage/utils.test.ts | 9 +++++---- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/web/src/components/storage/PartitionPage.test.tsx b/web/src/components/storage/PartitionPage.test.tsx index cf81997cf0..ca747ac661 100644 --- a/web/src/components/storage/PartitionPage.test.tsx +++ b/web/src/components/storage/PartitionPage.test.tsx @@ -24,8 +24,8 @@ import React from "react"; import { screen, within } from "@testing-library/react"; import { installerRender, mockParams } from "~/test-utils"; import PartitionPage from "./PartitionPage"; -import { StorageDevice, Volume, VolumeTarget } from "~/types/storage"; -import { configModel } from "~/api/storage/types"; +import { StorageDevice } from "~/types/storage"; +import { configModel, Volume } from "~/api/storage/types"; import { gib } from "./utils"; jest.mock("~/queries/issues", () => ({ @@ -105,7 +105,8 @@ const mockSolvedConfigModel: configModel.Config = { const homeVolumeMock: Volume = { mountPath: "/home", - target: VolumeTarget.DEFAULT, + mountOptions: [], + target: "default", fsType: "Btrfs", minSize: 1024, maxSize: 1024, @@ -120,7 +121,6 @@ const homeVolumeMock: Volume = { snapshotsAffectSizes: false, sizeRelevantVolumes: [], adjustByRam: false, - productDefined: false, }, }; diff --git a/web/src/components/storage/ProposalTransactionalInfo.test.tsx b/web/src/components/storage/ProposalTransactionalInfo.test.tsx index 7bcea38451..8633dd7e03 100644 --- a/web/src/components/storage/ProposalTransactionalInfo.test.tsx +++ b/web/src/components/storage/ProposalTransactionalInfo.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2024] SUSE LLC + * Copyright (c) [2024-2025] SUSE LLC * * All Rights Reserved. * @@ -24,7 +24,7 @@ import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { ProposalTransactionalInfo } from "~/components/storage"; -import { Volume, VolumeTarget } from "~/types/storage"; +import { Volume } from "~/api/storage/types"; let mockVolumes: Volume[] = []; jest.mock("~/queries/software", () => ({ @@ -37,12 +37,13 @@ jest.mock("~/queries/software", () => ({ jest.mock("~/queries/storage", () => ({ ...jest.requireActual("~/queries/storage"), - useVolumeTemplates: () => mockVolumes, + useVolumes: () => mockVolumes, })); const rootVolume: Volume = { mountPath: "/", - target: VolumeTarget.DEFAULT, + mountOptions: [], + target: "default", fsType: "Btrfs", minSize: 1024, maxSize: 2048, @@ -57,7 +58,6 @@ const rootVolume: Volume = { snapshotsAffectSizes: true, sizeRelevantVolumes: [], adjustByRam: false, - productDefined: true, }, }; diff --git a/web/src/components/storage/utils.test.ts b/web/src/components/storage/utils.test.ts index cc1e091ab6..814571a1e1 100644 --- a/web/src/components/storage/utils.test.ts +++ b/web/src/components/storage/utils.test.ts @@ -1,5 +1,5 @@ /* - * Copyright (c) [2023-2024] SUSE LLC + * Copyright (c) [2023-2025] SUSE LLC * * All Rights Reserved. * @@ -20,7 +20,8 @@ * find current contact information at www.suse.com. */ -import { StorageDevice, Volume, VolumeTarget } from "~/types/storage"; +import { StorageDevice } from "~/types/storage"; +import { Volume } from "~/api/storage/types"; import { deviceSize, deviceBaseName, @@ -40,7 +41,8 @@ import { const volume = (properties: object = {}): Volume => { const testVolume: Volume = { mountPath: "/test", - target: VolumeTarget.DEFAULT, + mountOptions: [], + target: "default", fsType: "Btrfs", minSize: 1024, maxSize: 2048, @@ -55,7 +57,6 @@ const volume = (properties: object = {}): Volume => { snapshotsAffectSizes: false, sizeRelevantVolumes: [], adjustByRam: false, - productDefined: false, }, }; From b5417a95207cf0e9f308050d75d6b5a540a7fd48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 11 Feb 2025 12:42:19 +0000 Subject: [PATCH 10/21] web: remove unused component --- .../storage/SnapshotsField.test.tsx | 70 ------------------ web/src/components/storage/SnapshotsField.tsx | 74 ------------------- 2 files changed, 144 deletions(-) delete mode 100644 web/src/components/storage/SnapshotsField.test.tsx delete mode 100644 web/src/components/storage/SnapshotsField.tsx diff --git a/web/src/components/storage/SnapshotsField.test.tsx b/web/src/components/storage/SnapshotsField.test.tsx deleted file mode 100644 index 372e81227b..0000000000 --- a/web/src/components/storage/SnapshotsField.test.tsx +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright (c) [2024-2025] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License as published by the Free - * Software Foundation; either version 2 of the License, or (at your option) - * any later version. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -// @ts-check - -import React from "react"; -import { screen } from "@testing-library/react"; -import { plainRender } from "~/test-utils"; -import SnapshotsField, { SnapshotsFieldProps } from "~/components/storage/SnapshotsField"; -import { Volume, VolumeTarget } from "~/types/storage"; - -const rootVolume: Volume = { - mountPath: "/", - target: VolumeTarget.DEFAULT, - fsType: "Btrfs", - minSize: 1024, - autoSize: true, - snapshots: true, - transactional: false, - outline: { - required: true, - fsTypes: ["ext4", "btrfs"], - supportAutoSize: true, - snapshotsConfigurable: false, - snapshotsAffectSizes: true, - adjustByRam: false, - sizeRelevantVolumes: ["/home"], - productDefined: true, - }, -}; - -const onChangeFn = jest.fn(); - -let props: SnapshotsFieldProps; - -describe("SnapshotsField", () => { - it("reflects snapshots status", () => { - props = { rootVolume: { ...rootVolume, snapshots: true }, onChange: onChangeFn }; - plainRender(); - const checkbox: HTMLInputElement = screen.getByRole("switch"); - expect(checkbox.value).toEqual("on"); - }); - - it("allows toggling snapshots status", async () => { - props = { rootVolume: { ...rootVolume, snapshots: true }, onChange: onChangeFn }; - const { user } = plainRender(); - const checkbox: HTMLInputElement = screen.getByRole("switch"); - await user.click(checkbox); - expect(onChangeFn).toHaveBeenCalledWith({ active: false }); - }); -}); diff --git a/web/src/components/storage/SnapshotsField.tsx b/web/src/components/storage/SnapshotsField.tsx deleted file mode 100644 index ed220e1bf4..0000000000 --- a/web/src/components/storage/SnapshotsField.tsx +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright (c) [2024-2025] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License as published by the Free - * Software Foundation; either version 2 of the License, or (at your option) - * any later version. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -// @ts-check - -import React from "react"; -import { Split, Switch } from "@patternfly/react-core"; -import { _ } from "~/i18n"; -import { hasFS } from "~/components/storage/utils"; -import textStyles from "@patternfly/react-styles/css/utilities/Text/text"; -import { Volume } from "~/types/storage"; - -export type SnapshotsFieldProps = { - rootVolume: Volume; - onChange?: (config: SnapshotsConfig) => void; -}; - -export type SnapshotsConfig = { - active: boolean; -}; - -/** - * Allows to define snapshots enablement - * @component - */ -export default function SnapshotsField({ rootVolume, onChange }: SnapshotsFieldProps) { - const isChecked = hasFS(rootVolume, "Btrfs") && rootVolume.snapshots; - - const label = _("Use Btrfs snapshots for the root file system"); - - const switchState = () => { - if (onChange) onChange({ active: !isChecked }); - }; - - return ( - - -
-
{label}
-
- {_( - "Allows to boot to a previous version of the \ -system after configuration changes or software upgrades.", - )} -
-
-
- ); -} From f04001416a2440f5e9c76f939723e52c9a3ee7f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 13 Feb 2025 06:14:51 +0000 Subject: [PATCH 11/21] fix(storage): improve error message --- service/lib/agama/storage/config_checkers/boot.rb | 10 ++++++++-- service/test/agama/storage/config_checker_test.rb | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/service/lib/agama/storage/config_checkers/boot.rb b/service/lib/agama/storage/config_checkers/boot.rb index 4bc0cebd3a..013385d79c 100644 --- a/service/lib/agama/storage/config_checkers/boot.rb +++ b/service/lib/agama/storage/config_checkers/boot.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -55,7 +55,13 @@ def device_alias def missing_alias_issue return unless configure? && device_alias.nil? - error(_("There is no boot device alias")) + # Currently this situation only happens because the config solver was not able to find + # a device config containing a root volume. The message could become inaccurate if the + # solver logic changes. + error( + _("The boot device cannot be automatically selected because there is no root (/) " \ + "file system") + ) end # @return [Issue, nil] diff --git a/service/test/agama/storage/config_checker_test.rb b/service/test/agama/storage/config_checker_test.rb index 3f4185df19..b675e68b28 100644 --- a/service/test/agama/storage/config_checker_test.rb +++ b/service/test/agama/storage/config_checker_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2025] SUSE LLC # # All Rights Reserved. # @@ -288,7 +288,7 @@ issue = issues.first expect(issue.error?).to eq(true) - expect(issue.description).to eq("There is no boot device alias") + expect(issue.description).to match(/there is no root \(\/\) file system/) end end From 93f6e95e1e8a4b732e5337edb4e6a4f42a50effb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 13 Feb 2025 06:16:54 +0000 Subject: [PATCH 12/21] storage: generate model even for config with errors --- service/lib/agama/dbus/interfaces/issues.rb | 2 +- service/lib/agama/storage/proposal.rb | 6 +----- service/test/agama/storage/proposal_test.rb | 16 +++++++++++++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/service/lib/agama/dbus/interfaces/issues.rb b/service/lib/agama/dbus/interfaces/issues.rb index 007a402b63..9a8cae97f9 100644 --- a/service/lib/agama/dbus/interfaces/issues.rb +++ b/service/lib/agama/dbus/interfaces/issues.rb @@ -36,7 +36,7 @@ module Issues # # @return [Array] The description, details, source # and severity of each issue. - # Source: 1 for system, 2 for config and 3 for unknown. + # Source: 1 for system, 2 for config and 0 for unknown. # Severity: 0 for warn and 1 for error. def dbus_issues issues.map do |issue| diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index c132d93e56..bec29f2975 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -94,17 +94,13 @@ def storage_json # Config model according to the JSON schema. # - # The config model is generated only if the config has not errors and all the config features - # are supported by the model. + # The config model is generated only if all the config features are supported by the model. # # @return [Hash, nil] nil if the config model cannot be generated. def model_json config = config(solved: true) return unless config && model_supported?(config) - issues = Agama::Storage::ConfigChecker.new(config, product_config).issues - return if issues.any?(&:error?) - ConfigConversions::ToModel.new(config).convert end diff --git a/service/test/agama/storage/proposal_test.rb b/service/test/agama/storage/proposal_test.rb index 3992ca175e..99f0878bf8 100644 --- a/service/test/agama/storage/proposal_test.rb +++ b/service/test/agama/storage/proposal_test.rb @@ -426,15 +426,25 @@ def drive(partitions) storage: { drives: [ { - name: "unknown" + search: "unknown" } ] } } end - it "returns nil" do - expect(subject.model_json).to be_nil + it "returns the config model" do + expect(subject.model_json).to eq( + { + boot: { + configure: true, + device: { + default: true + } + }, + drives: [] + } + ) end end From e93b6cb97bf924f4f045ad0a70de2f99f93b9b00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 13 Feb 2025 06:17:42 +0000 Subject: [PATCH 13/21] fix(storage): set proposal issues as system issues --- service/lib/agama/storage/proposal.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index bec29f2975..44c07d9438 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -370,7 +370,7 @@ def storage_manager def failed_issue Issue.new( _("Cannot accommodate the required file systems for installation"), - source: Issue::Source::CONFIG, + source: Issue::Source::SYSTEM, severity: Issue::Severity::ERROR ) end @@ -382,7 +382,7 @@ def exception_issue(error) Issue.new( _("A problem ocurred while calculating the storage setup"), details: error.message, - source: Issue::Source::CONFIG, + source: Issue::Source::SYSTEM, severity: Issue::Severity::ERROR ) end From b20812dc76f8330c78d03e7c5764c3bacc025caa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 13 Feb 2025 06:19:35 +0000 Subject: [PATCH 14/21] web: do not edit storage model if there are config errors --- .../components/storage/ProposalPage.test.tsx | 75 ++++++++++++------ web/src/components/storage/ProposalPage.tsx | 77 +++++++++++++++---- 2 files changed, 114 insertions(+), 38 deletions(-) diff --git a/web/src/components/storage/ProposalPage.test.tsx b/web/src/components/storage/ProposalPage.test.tsx index acf071c503..5aa9c085a3 100644 --- a/web/src/components/storage/ProposalPage.test.tsx +++ b/web/src/components/storage/ProposalPage.test.tsx @@ -30,6 +30,7 @@ import { screen } from "@testing-library/react"; import { installerRender } from "~/test-utils"; import ProposalPage from "~/components/storage/ProposalPage"; import { StorageDevice } from "~/types/storage"; +import { Issue } from "~/types/issues"; const disk: StorageDevice = { sid: 60, @@ -44,6 +45,22 @@ const disk: StorageDevice = { size: 1e6, }; +const systemError: Issue = { + description: "System error", + kind: "storage", + details: "", + source: 1, + severity: 1, +}; + +const configError: Issue = { + description: "Config error", + kind: "storage", + details: "", + source: 2, + severity: 1, +}; + const mockUseAvailableDevices = jest.fn(); const mockUseConfigMutation = jest.fn(); const mockUseDeprecated = jest.fn(); @@ -160,26 +177,18 @@ describe("if there are not devices", () => { }); }); -describe("if there is not model", () => { +describe("if there is not a model", () => { beforeEach(() => { mockUseAvailableDevices.mockReturnValue([disk]); mockUseConfigModel.mockReturnValue(null); }); - describe("and there are errors", () => { + describe("and there are system errors", () => { beforeEach(() => { - mockUseIssues.mockReturnValue([ - { - description: "Error", - kind: "storage", - details: "", - source: 2, - severity: 1, - }, - ]); + mockUseIssues.mockReturnValue([systemError]); }); - it("renders an option for reseting the config", () => { + it("renders an option for resetting the config", () => { installerRender(); expect(screen.queryByRole("button", { name: "Reset" })).toBeInTheDocument(); }); @@ -195,7 +204,7 @@ describe("if there is not model", () => { }); }); - describe("and there are not errors", () => { + describe("and there are not system errors", () => { beforeEach(() => { mockUseIssues.mockReturnValue([]); }); @@ -223,17 +232,35 @@ describe("if there is a model", () => { mockUseConfigModel.mockReturnValue({ drives: [] }); }); - describe("and there are errors", () => { + describe("and there are config errors", () => { + beforeEach(() => { + mockUseIssues.mockReturnValue([configError]); + }); + + it("renders the config errors", () => { + installerRender(); + expect(screen.queryByText("Config error")).toBeInTheDocument(); + }); + + it("renders an option for resetting the config", () => { + installerRender(); + expect(screen.queryByRole("button", { name: "Reset" })).toBeInTheDocument(); + }); + + it("does not render the installation devices", () => { + installerRender(); + expect(screen.queryByText("installation devices")).not.toBeInTheDocument(); + }); + + it("does not render the result", () => { + installerRender(); + expect(screen.queryByText("result")).not.toBeInTheDocument(); + }); + }); + + describe("and there are not config errors but there are system errors", () => { beforeEach(() => { - mockUseIssues.mockReturnValue([ - { - description: "Error", - kind: "storage", - details: "", - source: 2, - severity: 1, - }, - ]); + mockUseIssues.mockReturnValue([systemError]); }); it("renders an error message", () => { @@ -252,7 +279,7 @@ describe("if there is a model", () => { }); }); - describe("and there are not errors", () => { + describe("and there are neither config errors nor system errors", () => { beforeEach(() => { mockUseIssues.mockReturnValue([]); }); diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index 222eeecd1d..c2b390adba 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -32,6 +32,8 @@ import { EmptyState, EmptyStateBody, EmptyStateFooter, + List, + ListItem, } from "@patternfly/react-core"; import { Page, Link } from "~/components/core/"; import { Icon, Loading } from "~/components/layout"; @@ -41,7 +43,7 @@ import ProposalFailedInfo from "./ProposalFailedInfo"; import ConfigEditor from "./ConfigEditor"; import ConfigEditorMenu from "./ConfigEditorMenu"; import AddExistingDeviceMenu from "./AddExistingDeviceMenu"; -import { IssueSeverity } from "~/types/issues"; +import { Issue, IssueSeverity, IssueSource } from "~/types/issues"; import { useAvailableDevices, useConfigMutation, @@ -70,20 +72,51 @@ function useResetConfig() { }); } -function ResetEmptyState() { +type ErrorsEmptyStateProps = { + errors: Issue[]; +}; + +function ErrorsEmptyState({ errors }: ErrorsEmptyStateProps) { const reset = useResetConfig(); - const description = _( - "The current storage config cannot be edited. Do you want to reset to the default config?", + return ( + } + status="warning" + > + +

{_("The current storage settings contains the following issues:")}

+ + {errors.map((e, i) => ( + {e.description} + ))} + +
+ +

{_("Do you want to reset to the default settings?")}

+ +
+
); +} + +function ConfigEmptyState() { + const reset = useResetConfig(); + return ( } status="warning" > - {description} + +

{_("The current storage settings cannot be edited.")}

+
+

{_("Do you want to reset to the default settings?")}

@@ -147,10 +180,10 @@ function ProposalSections({ isEditable, isValid }: ProposalSectionsProps): React {!isEditable && ( - + <> {_( - "The current storage config cannot be edited. Do you want to reset to the default config?", + "The current storage settings cannot be edited. Do you want to reset to the default settings?", )} + + + + ); +} From 43e340c33275bd38a3c0da283ad6b2489c406114 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 14 Feb 2025 14:26:32 +0000 Subject: [PATCH 19/21] web: improve ProposalPage code --- .../components/storage/ProposalPage.test.tsx | 51 +++---- web/src/components/storage/ProposalPage.tsx | 127 +++++++----------- web/src/queries/issues.ts | 26 +++- web/src/queries/storage.ts | 17 +++ 4 files changed, 118 insertions(+), 103 deletions(-) diff --git a/web/src/components/storage/ProposalPage.test.tsx b/web/src/components/storage/ProposalPage.test.tsx index 5aa9c085a3..61e47cf918 100644 --- a/web/src/components/storage/ProposalPage.test.tsx +++ b/web/src/components/storage/ProposalPage.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022-2024] SUSE LLC + * Copyright (c) [2022-2025] SUSE LLC * * All Rights Reserved. * @@ -62,14 +62,14 @@ const configError: Issue = { }; const mockUseAvailableDevices = jest.fn(); -const mockUseConfigMutation = jest.fn(); +const mockUseResetConfigMutation = jest.fn(); const mockUseDeprecated = jest.fn(); const mockUseDeprecatedChanges = jest.fn(); const mockUseReprobeMutation = jest.fn(); jest.mock("~/queries/storage", () => ({ ...jest.requireActual("~/queries/storage"), useAvailableDevices: () => mockUseAvailableDevices(), - useConfigMutation: () => mockUseConfigMutation(), + useResetConfigMutation: () => mockUseResetConfigMutation(), useDeprecated: () => mockUseDeprecated(), useDeprecatedChanges: () => mockUseDeprecatedChanges(), useReprobeMutation: () => mockUseReprobeMutation(), @@ -93,23 +93,31 @@ jest.mock("~/queries/storage/dasd", () => ({ useDASDSupported: () => mockUseDASDSupported(), })); -const mockUseIssues = jest.fn(); +const mockUseSystemErrors = jest.fn(); +const mockUseConfigErrors = jest.fn(); jest.mock("~/queries/issues", () => ({ ...jest.requireActual("~/queries/issues"), - useIssues: () => mockUseIssues(), + useSystemErrors: () => mockUseSystemErrors(), + useConfigErrors: () => mockUseConfigErrors(), })); -jest.mock("./ProposalResultSection", () => () =>
result
); jest.mock("./ProposalTransactionalInfo", () => () =>
trasactional info
); +jest.mock("./ProposalFailedInfo", () => () =>
failed info
); +jest.mock("./UnsupportedModelInfo", () => () =>
unsupported info
); +jest.mock("./ProposalResultSection", () => () =>
result
); jest.mock("./ConfigEditor", () => () =>
installation devices
); jest.mock("./AddExistingDeviceMenu", () => () =>
add device menu
); jest.mock("./ConfigEditorMenu", () => () =>
config editor menu
); +jest.mock("~/components/product/ProductRegistrationAlert", () => () => ( +
registration alert
+)); beforeEach(() => { - mockUseConfigMutation.mockReturnValue({ mutate: jest.fn() }); + mockUseResetConfigMutation.mockReturnValue({ mutate: jest.fn() }); mockUseReprobeMutation.mockReturnValue({ mutateAsync: jest.fn() }); mockUseDeprecated.mockReturnValue(false); - mockUseIssues.mockReturnValue([]); + mockUseSystemErrors.mockReturnValue([]); + mockUseConfigErrors.mockReturnValue([]); }); describe("if there are not devices", () => { @@ -185,7 +193,7 @@ describe("if there is not a model", () => { describe("and there are system errors", () => { beforeEach(() => { - mockUseIssues.mockReturnValue([systemError]); + mockUseSystemErrors.mockReturnValue([systemError]); }); it("renders an option for resetting the config", () => { @@ -206,12 +214,12 @@ describe("if there is not a model", () => { describe("and there are not system errors", () => { beforeEach(() => { - mockUseIssues.mockReturnValue([]); + mockUseSystemErrors.mockReturnValue([]); }); - it("renders an option for reseting the config", () => { + it("renders an unsupported model alert", async () => { installerRender(); - expect(screen.queryByRole("button", { name: "Reset" })).toBeInTheDocument(); + expect(screen.queryByText("unsupported info")).toBeInTheDocument(); }); it("does not render the installation devices", async () => { @@ -232,9 +240,10 @@ describe("if there is a model", () => { mockUseConfigModel.mockReturnValue({ drives: [] }); }); - describe("and there are config errors", () => { + describe("and there are config errors and system errors", () => { beforeEach(() => { - mockUseIssues.mockReturnValue([configError]); + mockUseConfigErrors.mockReturnValue([configError]); + mockUseSystemErrors.mockReturnValue([systemError]); }); it("renders the config errors", () => { @@ -260,12 +269,12 @@ describe("if there is a model", () => { describe("and there are not config errors but there are system errors", () => { beforeEach(() => { - mockUseIssues.mockReturnValue([systemError]); + mockUseSystemErrors.mockReturnValue([systemError]); }); - it("renders an error message", () => { + it("renders a failed proposal failed", () => { installerRender(); - expect(screen.queryByText(/Adjust the settings/)).toBeInTheDocument(); + expect(screen.queryByText("failed info")).toBeInTheDocument(); }); it("renders the installation devices", () => { @@ -281,12 +290,8 @@ describe("if there is a model", () => { describe("and there are neither config errors nor system errors", () => { beforeEach(() => { - mockUseIssues.mockReturnValue([]); - }); - - it("does not render an error message", () => { - installerRender(); - expect(screen.queryByText(/Adjust the settings/)).not.toBeInTheDocument(); + mockUseSystemErrors.mockReturnValue([]); + mockUseConfigErrors.mockReturnValue([]); }); it("renders the installation devices", () => { diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index c2b390adba..3ba70222e6 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -22,7 +22,6 @@ import React from "react"; import { - Alert, Button, Content, Grid, @@ -40,13 +39,13 @@ import { Icon, Loading } from "~/components/layout"; import ProposalResultSection from "./ProposalResultSection"; import ProposalTransactionalInfo from "./ProposalTransactionalInfo"; import ProposalFailedInfo from "./ProposalFailedInfo"; +import UnsupportedModelInfo from "./UnsupportedModelInfo"; import ConfigEditor from "./ConfigEditor"; import ConfigEditorMenu from "./ConfigEditorMenu"; import AddExistingDeviceMenu from "./AddExistingDeviceMenu"; -import { Issue, IssueSeverity, IssueSource } from "~/types/issues"; import { useAvailableDevices, - useConfigMutation, + useResetConfigMutation, useDeprecated, useDeprecatedChanges, useReprobeMutation, @@ -54,30 +53,13 @@ import { import { useConfigModel } from "~/queries/storage/config-model"; import { useZFCPSupported } from "~/queries/storage/zfcp"; import { useDASDSupported } from "~/queries/storage/dasd"; -import { useIssues } from "~/queries/issues"; +import { useSystemErrors, useConfigErrors } from "~/queries/issues"; import { STORAGE as PATHS } from "~/routes/paths"; import { _ } from "~/i18n"; -/** @todo Call to an API method to reset the default config instead of setting a config. */ -function useResetConfig() { - const { mutate } = useConfigMutation(); - - return () => - mutate({ - drives: [ - { - partitions: [{ search: "*", delete: true }, { generate: "default" }], - }, - ], - }); -} - -type ErrorsEmptyStateProps = { - errors: Issue[]; -}; - -function ErrorsEmptyState({ errors }: ErrorsEmptyStateProps) { - const reset = useResetConfig(); +function InvalidConfigEmptyState(): React.ReactNode { + const errors = useConfigErrors("storage"); + const { mutate: reset } = useResetConfigMutation(); return ( -

{_("The current storage settings contains the following issues:")}

+ + {_("The current storage settings contains the following issues:")} + {errors.map((e, i) => ( {e.description} @@ -94,8 +78,8 @@ function ErrorsEmptyState({ errors }: ErrorsEmptyStateProps) {
-

{_("Do you want to reset to the default settings?")}

-
@@ -103,8 +87,8 @@ function ErrorsEmptyState({ errors }: ErrorsEmptyStateProps) { ); } -function ConfigEmptyState() { - const reset = useResetConfig(); +function UnknowConfigEmptyState(): React.ReactNode { + const { mutate: reset } = useResetConfigMutation(); return ( -

{_("The current storage settings cannot be edited.")}

+ {_("The current storage settings cannot be edited.")}
-

{_("Do you want to reset to the default settings?")}

-
@@ -125,7 +109,7 @@ function ConfigEmptyState() { ); } -function DevicesEmptyState() { +function UnavailableDevicesEmptyState(): React.ReactNode { const isZFCPSupported = useZFCPSupported(); const isDASDSupported = useDASDSupported(); @@ -167,31 +151,26 @@ function DevicesEmptyState() { ); } -type ProposalSectionsProps = { - isEditable: boolean; - isValid: boolean; -}; +function ProposalEmptyState(): React.ReactNode { + const model = useConfigModel({ suspense: true }); + const availableDevices = useAvailableDevices(); -function ProposalSections({ isEditable, isValid }: ProposalSectionsProps): React.ReactNode { - const reset = useResetConfig(); + if (!availableDevices.length) return ; + + return model ? : ; +} + +function ProposalSections(): React.ReactNode { + const model = useConfigModel({ suspense: true }); + const systemErrors = useSystemErrors("storage"); + const hasResult = !systemErrors.length; return ( - {!isEditable && ( - - <> - {_( - "The current storage settings cannot be edited. Do you want to reset to the default settings?", - )} - - - - )} - {isEditable && ( + + {model && ( )} - {isValid && } + {hasResult && } ); } /** - * @fixme The UI for editing a config model is not prepared yet to properly work with a model that - * contains errors. For that reason, a config is considered as unknown (non-editable) if there is - * no model and also if there is some config error. In the future, components like ConfigEditor - * should be extended in order to make them to work with a model containg issues. + * @fixme Extract components like ProposalSections, UnknownConfigEmptyState, etc, to separate files + * and test them individually. The proposal page should simply mount all those components. */ export default function ProposalPage(): React.ReactNode { const isDeprecated = useDeprecated(); const model = useConfigModel({ suspense: true }); - const devices = useAvailableDevices(); - const issues = useIssues("storage"); + const availableDevices = useAvailableDevices(); + const systemErrors = useSystemErrors("storage"); + const configErrors = useConfigErrors("storage"); const { mutateAsync: reprobe } = useReprobeMutation(); useDeprecatedChanges(); @@ -238,15 +216,16 @@ export default function ProposalPage(): React.ReactNode { if (isDeprecated) reprobe().catch(console.log); }, [isDeprecated, reprobe]); - const systemErrors = issues - .filter((s) => s.severity === IssueSeverity.Error) - .filter((s) => s.source === IssueSource.System); - const configErrors = issues - .filter((s) => s.severity === IssueSeverity.Error) - .filter((s) => s.source === IssueSource.Config); - const isValid = !systemErrors.length && !configErrors.length; - const isEditable = model && !configErrors.length; - const isDevicesEmpty = !devices.length; + /** + * @fixme For now, a config model is only considered as editable if there is no config error. The + * UI for handling a model is not prepared yet to properly work with a model generated from a + * config with errors. Components like ConfigEditor should be adapted in order to properly manage + * those scenarios. + */ + const isModelEditable = model && !configErrors.length; + const hasDevices = !!availableDevices.length; + const hasResult = !systemErrors.length; + const showSections = hasDevices && (isModelEditable || hasResult); return ( @@ -255,16 +234,8 @@ export default function ProposalPage(): React.ReactNode { {isDeprecated && } - {!isDeprecated && isDevicesEmpty && } - {!isDeprecated && !isDevicesEmpty && !isEditable && !isValid && !model && ( - - )} - {!isDeprecated && !isDevicesEmpty && !isEditable && !isValid && model && ( - - )} - {!isDeprecated && !isDevicesEmpty && (isEditable || isValid) && ( - - )} + {!isDeprecated && !showSections && } + {!isDeprecated && showSections && } ); diff --git a/web/src/queries/issues.ts b/web/src/queries/issues.ts index 1e87878319..41b5888f63 100644 --- a/web/src/queries/issues.ts +++ b/web/src/queries/issues.ts @@ -23,7 +23,7 @@ import React from "react"; import { useQueryClient, useSuspenseQueries, useSuspenseQuery } from "@tanstack/react-query"; import { useInstallerClient } from "~/context/installer"; -import { IssuesList, IssuesScope } from "~/types/issues"; +import { IssuesList, IssuesScope, IssueSeverity, IssueSource } from "~/types/issues"; import { fetchIssues } from "~/api/issues"; const scopesFromPath = { @@ -86,4 +86,26 @@ const useIssuesChanges = () => { }, [client, queryClient]); }; -export { useIssues, useAllIssues, useIssuesChanges }; +/** + * Returns the system errors for the given scope. + */ +const useSystemErrors = (scope: IssuesScope) => { + const issues = useIssues(scope); + + return issues + .filter((i) => i.severity === IssueSeverity.Error) + .filter((i) => i.source === IssueSource.System); +}; + +/** + * Returns the config errors for the given scope. + */ +const useConfigErrors = (scope: IssuesScope) => { + const issues = useIssues(scope); + + return issues + .filter((i) => i.severity === IssueSeverity.Error) + .filter((i) => i.source === IssueSource.Config); +}; + +export { useIssues, useAllIssues, useIssuesChanges, useSystemErrors, useConfigErrors }; diff --git a/web/src/queries/storage.ts b/web/src/queries/storage.ts index d07095f265..29e0277e16 100644 --- a/web/src/queries/storage.ts +++ b/web/src/queries/storage.ts @@ -66,6 +66,22 @@ const useConfigMutation = () => { return useMutation(query); }; +/** @todo Call to an API method to reset the default config instead of setting a config. */ +const useResetConfigMutation = () => { + const { mutate } = useConfigMutation(); + + return { + mutate: () => + mutate({ + drives: [ + { + partitions: [{ search: "*", delete: true }, { generate: "default" }], + }, + ], + }), + }; +}; + const devicesQuery = (scope: "result" | "system") => ({ queryKey: ["storage", "devices", scope], queryFn: () => fetchDevices(scope), @@ -247,6 +263,7 @@ const useReprobeMutation = () => { export { useConfig, useConfigMutation, + useResetConfigMutation, useDevices, useAvailableDevices, useProductParams, From 53df2e50beeb376010598df4b51c8445cbf17f7e Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 14 Feb 2025 17:10:08 +0000 Subject: [PATCH 20/21] web: Adapt wording of some alerts and empty states --- .../components/storage/ProposalPage.test.tsx | 14 +++---- web/src/components/storage/ProposalPage.tsx | 38 +++++++++++++------ .../storage/UnsupportedModelInfo.test.tsx | 4 +- .../storage/UnsupportedModelInfo.tsx | 11 ++++-- 4 files changed, 43 insertions(+), 24 deletions(-) diff --git a/web/src/components/storage/ProposalPage.test.tsx b/web/src/components/storage/ProposalPage.test.tsx index 61e47cf918..ce0b3dc426 100644 --- a/web/src/components/storage/ProposalPage.test.tsx +++ b/web/src/components/storage/ProposalPage.test.tsx @@ -127,7 +127,7 @@ describe("if there are not devices", () => { it("renders an option for activating iSCSI", () => { installerRender(); - expect(screen.queryByRole("link", { name: "Activate iSCSI" })).toBeInTheDocument(); + expect(screen.queryByRole("link", { name: /iSCSI/ })).toBeInTheDocument(); }); it("does not render the installation devices", () => { @@ -147,7 +147,7 @@ describe("if there are not devices", () => { it("does not render an option for activating zFCP", () => { installerRender(); - expect(screen.queryByRole("link", { name: "Activate zFCP" })).not.toBeInTheDocument(); + expect(screen.queryByRole("link", { name: /zFCP/ })).not.toBeInTheDocument(); }); }); @@ -158,7 +158,7 @@ describe("if there are not devices", () => { it("does not render an option for activating DASD", () => { installerRender(); - expect(screen.queryByRole("link", { name: "Activate DASD" })).not.toBeInTheDocument(); + expect(screen.queryByRole("link", { name: /DASD/ })).not.toBeInTheDocument(); }); }); @@ -169,7 +169,7 @@ describe("if there are not devices", () => { it("renders an option for activating zFCP", () => { installerRender(); - expect(screen.queryByRole("link", { name: "Activate zFCP" })).toBeInTheDocument(); + expect(screen.queryByRole("link", { name: /zFCP/ })).toBeInTheDocument(); }); }); @@ -180,7 +180,7 @@ describe("if there are not devices", () => { it("renders an option for activating DASD", () => { installerRender(); - expect(screen.queryByRole("link", { name: "Activate DASD" })).toBeInTheDocument(); + expect(screen.queryByRole("link", { name: /DASD/ })).toBeInTheDocument(); }); }); }); @@ -198,7 +198,7 @@ describe("if there is not a model", () => { it("renders an option for resetting the config", () => { installerRender(); - expect(screen.queryByRole("button", { name: "Reset" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /Reset/ })).toBeInTheDocument(); }); it("does not render the installation devices", () => { @@ -253,7 +253,7 @@ describe("if there is a model", () => { it("renders an option for resetting the config", () => { installerRender(); - expect(screen.queryByRole("button", { name: "Reset" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /Reset/ })).toBeInTheDocument(); }); it("does not render the installation devices", () => { diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index 3ba70222e6..a17fe66285 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -55,7 +55,7 @@ import { useZFCPSupported } from "~/queries/storage/zfcp"; import { useDASDSupported } from "~/queries/storage/dasd"; import { useSystemErrors, useConfigErrors } from "~/queries/issues"; import { STORAGE as PATHS } from "~/routes/paths"; -import { _ } from "~/i18n"; +import { _, n_ } from "~/i18n"; function InvalidConfigEmptyState(): React.ReactNode { const errors = useConfigErrors("storage"); @@ -69,7 +69,11 @@ function InvalidConfigEmptyState(): React.ReactNode { > - {_("The current storage settings contains the following issues:")} + {n_( + "The current storage configuration has the following issue:", + "The current storage configuration has the following issues:", + errors.length, + )} {errors.map((e, i) => ( @@ -78,9 +82,13 @@ function InvalidConfigEmptyState(): React.ReactNode { - {_("Do you want to reset to the default settings?")} + + {_( + "You may want to discard those settings and start from scratch with a simple configuration.", + )} +
@@ -92,17 +100,23 @@ function UnknowConfigEmptyState(): React.ReactNode { return ( } status="warning" > - {_("The current storage settings cannot be edited.")} + + {_("The storage configuration uses elements not supported by this interface.")} + - {_("Do you want to reset to the default settings?")} + + {_( + "You may want to discard the current settings and start from scratch with a simple configuration.", + )} + @@ -114,7 +128,7 @@ function UnavailableDevicesEmptyState(): React.ReactNode { const isDASDSupported = useDASDSupported(); const description = _( - "There are not devices available for the installation. Do you want to activate devices?", + "There are not disks available for the installation. You may need to configure some device.", ); return ( @@ -128,20 +142,20 @@ function UnavailableDevicesEmptyState(): React.ReactNode { - {_("Activate iSCSI")} + {_("Connect to iSCSI targets")} {isZFCPSupported && ( - {_("Activate zFCP")} + {_("Activate zFCP disks")} )} {isDASDSupported && ( - {_("Activate DASD")} + {_("Manage DASD devices")} )} diff --git a/web/src/components/storage/UnsupportedModelInfo.test.tsx b/web/src/components/storage/UnsupportedModelInfo.test.tsx index 6c602f04fe..434a0645e0 100644 --- a/web/src/components/storage/UnsupportedModelInfo.test.tsx +++ b/web/src/components/storage/UnsupportedModelInfo.test.tsx @@ -48,12 +48,12 @@ describe("if there is not a model", () => { it("renders an alert", () => { plainRender(); - expect(screen.queryByText(/settings cannot be edited/)).toBeInTheDocument(); + expect(screen.queryByText(/Unable to modify the settings/)).toBeInTheDocument(); }); it("renders a button for resetting the config", () => { plainRender(); - expect(screen.queryByRole("button", { name: "Reset" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /Reset/ })).toBeInTheDocument(); }); }); diff --git a/web/src/components/storage/UnsupportedModelInfo.tsx b/web/src/components/storage/UnsupportedModelInfo.tsx index 70ec9eaacd..f2a7b46970 100644 --- a/web/src/components/storage/UnsupportedModelInfo.tsx +++ b/web/src/components/storage/UnsupportedModelInfo.tsx @@ -36,18 +36,23 @@ export default function UnsupportedModelInfo(): React.ReactNode { if (model) return null; return ( - + {_( - "The current storage settings cannot be edited. Do you want to reset to the default settings?", + "The storage configuration is valid (see result below) but uses elements not supported by this interface.", + )} + + + {_( + "You can proceed to install with the current settings or you may want to discard the configuration and start from scratch with a simple one.", )} From 5bfccf4718b17bc9559a81b6841f18afec713a47 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 14 Feb 2025 17:33:27 +0000 Subject: [PATCH 21/21] web: Wording fixes from review --- web/src/components/storage/ProposalPage.tsx | 4 ++-- web/src/components/storage/UnsupportedModelInfo.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index a17fe66285..525fcc8392 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -88,7 +88,7 @@ function InvalidConfigEmptyState(): React.ReactNode { )}
@@ -116,7 +116,7 @@ function UnknowConfigEmptyState(): React.ReactNode { )} diff --git a/web/src/components/storage/UnsupportedModelInfo.tsx b/web/src/components/storage/UnsupportedModelInfo.tsx index f2a7b46970..3fa20a2c27 100644 --- a/web/src/components/storage/UnsupportedModelInfo.tsx +++ b/web/src/components/storage/UnsupportedModelInfo.tsx @@ -52,7 +52,7 @@ export default function UnsupportedModelInfo(): React.ReactNode {