Merge pull request #379 from KKoukiou/device-spec-device-id

storage: display names not IDs in the storage screens
rhinstaller · Aug 6, 2024 · aed0595 · aed0595
2 parents 4aa3bf0 + 18e0d08
commit aed0595
Show file tree

Hide file tree

Showing 12 changed files with 88 additions and 73 deletions.
diff --git a/src/actions/storage-actions.js b/src/actions/storage-actions.js
@@ -87,7 +87,6 @@ export const getDevicesAction = () => {
             return dispatch({
                 payload: {
                     actions,
-                    deviceNames: devices,
                     devices: deviceData,
                     existingSystems,
                     mountPoints,

diff --git a/src/apis/storage_devicetree.js b/src/apis/storage_devicetree.js
@@ -42,13 +42,13 @@ export class DeviceTree {
 }
 
 /**
- * @param {string} deviceName   A device name
+ * @param {string} device       A device ID
  * @param {string} password     A password
  *
  * @returns {Promise}           Resolves true if success otherwise false
  */
-export const unlockDevice = ({ deviceName, passphrase }) => {
-    return new DeviceTree().callHandler("UnlockDevice", [deviceName, passphrase]);
+export const unlockDevice = ({ device, passphrase }) => {
+    return new DeviceTree().callHandler("UnlockDevice", [device, passphrase]);
 };
 
 /**

diff --git a/src/apis/storage_partitioning_automatic_resizable.js b/src/apis/storage_partitioning_automatic_resizable.js
@@ -17,33 +17,33 @@
 import { DeviceTree } from "./storage_devicetree.js";
 
 /**
- * @param {string} deviceName   A device name
+ * @param {string} device       A device ID
  * @param {object} deviceTree   The device tree
  *
  * @returns {Promise}           Resolves or rejects the result of the operation
  */
-export const removeDevice = ({ deviceName, deviceTree }) => {
-    return new DeviceTree(deviceTree).callResizable("RemoveDevice", [deviceName]);
+export const removeDevice = ({ device, deviceTree }) => {
+    return new DeviceTree(deviceTree).callResizable("RemoveDevice", [device]);
 };
 
 /**
- * @param {string} deviceName   A device name
+ * @param {string} device       A device ID
  * @param {number} newSize      The new size of the device
  * @param {object} deviceTree   The device tree
  *
  * @returns {Promise}           Resolves or rejects the result of the operation
  */
-export const shrinkDevice = ({ deviceName, deviceTree, newSize }) => {
-    return new DeviceTree(deviceTree).callResizable("ShrinkDevice", [deviceName, newSize]);
+export const shrinkDevice = ({ device, deviceTree, newSize }) => {
+    return new DeviceTree(deviceTree).callResizable("ShrinkDevice", [device, newSize]);
 };
 
 /**
- * @param {string} deviceName   A device name
+ * @param {string} device       A device ID
  *
  * @returns {Promise}           Resolves or rejects the result of the operation
  *                              The result is a boolean indicating whether the device is shrinkable
  *                             or not
  */
-export const isDeviceShrinkable = ({ deviceName, deviceTree }) => {
-    return new DeviceTree(deviceTree).callResizable("IsDeviceShrinkable", [deviceName]);
+export const isDeviceShrinkable = ({ device, deviceTree }) => {
+    return new DeviceTree(deviceTree).callResizable("IsDeviceShrinkable", [device]);
 };
diff --git a/src/components/review/StorageReview.jsx b/src/components/review/StorageReview.jsx
@@ -82,20 +82,20 @@ const DeviceRow = ({ disk }) => {
         return null;
     }
 
-    const getDeviceRow = ([mount, name]) => {
-        const size = cockpit.format_bytes(devices[name].size.v);
-        const request = requests.find(request => request["device-spec"] === name);
-        const format = devices[name].formatData.type.v;
+    const getDeviceRow = ([mount, device]) => {
+        const size = cockpit.format_bytes(devices[device].size.v);
+        const request = requests.find(request => request["device-spec"] === device);
+        const format = devices[device].formatData.type.v;
         const action = (
             request === undefined || request.reformat
                 ? (format ? cockpit.format(_("format as $0"), format) : null)
                 : ((format === "biosboot") ? format : _("mount"))
         );
-        const parents = getParentPartitions(devices, name);
+        const parents = getParentPartitions(devices, device);
         const showMaybeType = () => {
-            if (checkDeviceOnStorageType(devices, name, "lvmvg")) {
+            if (checkDeviceOnStorageType(devices, device, "lvmvg")) {
                 return ", LVM";
-            } else if (checkDeviceOnStorageType(devices, name, "mdarray")) {
+            } else if (checkDeviceOnStorageType(devices, device, "mdarray")) {
                 return ", RAID";
             } else {
                 return "";
@@ -109,9 +109,9 @@ const DeviceRow = ({ disk }) => {
                     { title: cockpit.format("$0$1", parents.join(", "), showMaybeType()), width: 20 },
                     { title: size, width: 20 },
                     { title: action, width: 20 },
-                    { title: hasEncryptedAncestor(devices, name) ? (!request || request.reformat ? _("encrypt") : _("encrypted")) : "", width: 20 },
+                    { title: hasEncryptedAncestor(devices, device) ? (!request || request.reformat ? _("encrypt") : _("encrypted")) : "", width: 20 },
                 ],
-                props: { key: name },
+                props: { key: device },
             }
         );
     };
@@ -177,7 +177,7 @@ const DeviceRow = ({ disk }) => {
             return false;
         }
 
-        const parents = getDeviceAncestors(originalDevices, action["device-name"].v);
+        const parents = getDeviceAncestors(originalDevices, action["device-id"].v);
 
         return parents.includes(disk);
     });
@@ -211,14 +211,14 @@ const DeviceRow = ({ disk }) => {
  * @returns {boolean}   True is the device will be deleted according to the actions
  */
 const isDeviceDeleted = ({ actions, device }) => (
-    actions.find(action => action["device-name"].v === device && action["action-type"].v === "destroy")
+    actions.find(action => action["device-id"].v === device && action["action-type"].v === "destroy")
 );
 
 /**
  * @returns {boolean}   True is the device will be resized according to the actions
  */
 const isDeviceResized = ({ actions, device }) => (
-    actions.find(action => action["device-name"].v === device && action["action-type"].v === "resize")
+    actions.find(action => action["device-id"].v === device && action["action-type"].v === "resize")
 );
 
 const DeletedSystems = () => {
@@ -271,18 +271,19 @@ const AffectedSystems = ({ type }) => {
             const affectedChildren = children.filter(child => affectedDevices.includes(child));
 
             if (affectedDevices.includes(device) || affectedChildren.length) {
-                acc[device] = affectedChildren;
+                acc[device] = affectedChildren.map(child => originalDevices[child].name.v);
             }
             return acc;
         }, {});
 
         return Object.keys(affectedDevicesPartitiongMap)
                 .map(device => {
+                    const deviceName = originalDevices[device].name.v;
                     if (affectedDevicesPartitiongMap[device].length === 0) {
-                        return device;
+                        return deviceName;
                     }
 
-                    return `${device} (${affectedDevicesPartitiongMap[device].join(", ")})`;
+                    return `${deviceName} (${affectedDevicesPartitiongMap[device].join(", ")})`;
                 })
                 .join(", ");
     };

diff --git a/src/components/storage/CockpitStorageIntegration.jsx b/src/components/storage/CockpitStorageIntegration.jsx
@@ -63,7 +63,7 @@ import {
 
 import { getDevicesAction, setStorageScenarioAction } from "../../actions/storage-actions.js";
 
-import { getDeviceNameByPath } from "../../helpers/storage.js";
+import { getDeviceByName, getDeviceByPath } from "../../helpers/storage.js";
 
 import { EmptyStatePanel } from "cockpit-components-empty-state";
 
@@ -228,14 +228,16 @@ export const preparePartitioning = async ({ devices, newMountPoints }) => {
         const partitioning = await createPartitioning({ method: "MANUAL" });
         const requests = await gatherRequests({ partitioning });
 
-        const addRequest = (devicePath, object, isSubVolume = false) => {
+        const addRequest = (device, object, isSubVolume = false) => {
             const { content, dir, subvolumes, type } = object;
             let deviceSpec;
             if (!isSubVolume) {
-                deviceSpec = getDeviceNameByPath(devices, devicePath);
-            } else if (devices[devicePath]) {
-                deviceSpec = devicePath;
+                deviceSpec = getDeviceByPath(devices, device);
             } else {
+                deviceSpec = getDeviceByName(devices, device);
+            }
+
+            if (!deviceSpec) {
                 return;
             }
 
@@ -360,14 +362,21 @@ const CheckStorageDialog = ({
 
         const devicesToUnlock = (
             Object.keys(cockpitPassphrases)
-                    .map(dev => ({
-                        deviceName: devices[dev] ? dev : getDeviceNameByPath(devices, dev),
-                        passphrase: cockpitPassphrases[dev]
-                    })))
-                .filter(({ deviceName }) => {
+                    .map(dev => {
+                        let device = getDeviceByName(devices, dev);
+                        if (!device) {
+                            device = getDeviceByPath(devices, dev);
+                        }
+
+                        return ({
+                            device,
+                            passphrase: cockpitPassphrases[dev]
+                        });
+                    }))
+                .filter(({ device }) => {
                     return (
-                        devices[deviceName].formatData.type.v === "luks" &&
-                            devices[deviceName].formatData.attrs.v.has_key !== "True"
+                        devices[device].formatData.type.v === "luks" &&
+                            devices[device].formatData.attrs.v.has_key !== "True"
                     );
                 });
 

diff --git a/src/components/storage/Common.jsx b/src/components/storage/Common.jsx
@@ -75,17 +75,17 @@ export const useDiskFreeSpace = ({ devices, selectedDisks }) => {
     return diskFreeSpace;
 };
 
-export const useDuplicateDeviceNames = ({ deviceNames }) => {
+export const useDuplicateDeviceNames = ({ devices }) => {
     const [duplicateDeviceNames, setDuplicateDeviceNames] = useState([]);
 
     useEffect(() => {
         const update = async () => {
-            const _duplicateDeviceNames = findDuplicatesInArray(deviceNames);
+            const _duplicateDeviceNames = findDuplicatesInArray(Object.keys(devices).map(device => devices[device].name.v));
 
             setDuplicateDeviceNames(_duplicateDeviceNames);
         };
         update();
-    }, [deviceNames]);
+    }, [devices]);
 
     return duplicateDeviceNames;
 };

diff --git a/src/components/storage/EncryptedDevices.jsx b/src/components/storage/EncryptedDevices.jsx
@@ -112,7 +112,7 @@ const UnlockDialog = ({ dispatch, lockedLUKSDevices, onClose }) => {
         setInProgress(true);
         return Promise.allSettled(
             lockedLUKSDevices.map(device => (
-                unlockDevice({ deviceName: device, passphrase })
+                unlockDevice({ device, passphrase })
             ))
         ).then(
             res => {

diff --git a/src/components/storage/InstallationScenario.jsx b/src/components/storage/InstallationScenario.jsx
@@ -291,14 +291,14 @@ const InstallationScenarioSelector = ({
     showStorage,
 }) => {
     const { diskSelection, partitioning } = useContext(StorageContext);
-    const { deviceNames, devices, mountPoints } = useOriginalDeviceTree();
+    const { devices, mountPoints } = useOriginalDeviceTree();
     const selectedDisks = diskSelection.selectedDisks;
     const [scenarioAvailability, setScenarioAvailability] = useState(Object.fromEntries(
         scenarios.map((s) => [s.id, new AvailabilityState()])
     ));
     const diskTotalSpace = useDiskTotalSpace({ devices, selectedDisks });
     const diskFreeSpace = useDiskFreeSpace({ devices, selectedDisks });
-    const duplicateDeviceNames = useDuplicateDeviceNames({ deviceNames });
+    const duplicateDeviceNames = useDuplicateDeviceNames({ devices });
     const mountPointConstraints = useMountPointConstraints();
     const usablePartitions = useUsablePartitions({ devices, selectedDisks });
     const requiredSize = useRequiredSize();

diff --git a/src/components/storage/MountPointMapping.jsx b/src/components/storage/MountPointMapping.jsx
@@ -260,7 +260,9 @@ const DeviceColumnSelect = ({ deviceData, devices, handleRequestChange, idPrefix
     const [isOpen, setIsOpen] = useState(false);
 
     const device = request["device-spec"];
+    const deviceName = device && deviceData[device].name.v;
     const options = devices.map(device => {
+        const deviceName = deviceData[device].name.v;
         const formatType = deviceData[device]?.formatData.type.v;
         const format = deviceData[device]?.formatData.description.v;
         const size = cockpit.format_bytes(deviceData[device]?.total.v);
@@ -274,11 +276,11 @@ const DeviceColumnSelect = ({ deviceData, devices, handleRequestChange, idPrefix
 
         return (
             <SelectOption
-              data-value={device}
+              data-value={deviceName}
               isDisabled={isDisabled}
               description={description}
               key={device}
-              value={device}
+              value={deviceName}
             />
         );
     });
@@ -288,11 +290,12 @@ const DeviceColumnSelect = ({ deviceData, devices, handleRequestChange, idPrefix
           hasPlaceholderStyle
           isOpen={isOpen}
           placeholderText={_("Select a device")}
-          selections={device ? [device] : []}
+          selections={deviceName ? [deviceName] : []}
           variant={SelectVariant.single}
           onToggle={(_event, val) => setIsOpen(val)}
           onSelect={(_, selection) => {
-              handleRequestChange({ deviceSpec: selection, mountPoint: request["mount-point"], requestIndex });
+              const deviceSpec = devices.find(d => deviceData[d].name.v === selection);
+              handleRequestChange({ deviceSpec, mountPoint: request["mount-point"], requestIndex });
               setIsOpen(false);
           }}
           onClear={() => {

diff --git a/src/components/storage/ReclaimSpaceModal.jsx b/src/components/storage/ReclaimSpaceModal.jsx
@@ -74,12 +74,12 @@ export const ReclaimSpaceModal = ({ isFormDisabled, onClose, onNext }) => {
                 try {
                     if (action.type === "remove") {
                         await removeDevice({
-                            deviceName: device,
+                            device,
                             deviceTree: partitioning.deviceTree.path,
                         });
                     } else if (action.type === "shrink") {
                         await shrinkDevice({
-                            deviceName: device,
+                            device,
                             deviceTree: partitioning.deviceTree.path,
                             newSize: action.value,
                         });
@@ -301,24 +301,25 @@ const DeviceActions = ({ device, level, setUnappliedActions, unappliedActions })
         return null;
     }
 
+    const deviceId = device["device-id"].v;
     const parents = device.parents.v;
     const parentHasRemove = parents?.some((parent) => getDeviceActionOfType({ device: parent, type: "remove", unappliedActions }));
-    const hasBeenRemoved = parentHasRemove || getDeviceActionOfType({ device: device.name.v, type: "remove", unappliedActions });
-    const newDeviceSize = getDeviceActionOfType({ device: device.name.v, type: "shrink", unappliedActions })?.value;
-    const hasUnappliedActions = !parentHasRemove && unappliedActions[device.name.v].length > 0;
+    const hasBeenRemoved = parentHasRemove || getDeviceActionOfType({ device: deviceId, type: "remove", unappliedActions });
+    const newDeviceSize = getDeviceActionOfType({ device: deviceId, type: "shrink", unappliedActions })?.value;
+    const hasUnappliedActions = !parentHasRemove && unappliedActions[deviceId].length > 0;
 
     const onAction = (action, value = "") => {
         setUnappliedActions((prevUnappliedActions) => {
             const _unappliedActions = { ...prevUnappliedActions };
-            _unappliedActions[device.name.v].push({ type: action, value });
+            _unappliedActions[deviceId].push({ type: action, value });
 
             return _unappliedActions;
         });
     };
     const onUndo = () => {
         setUnappliedActions((prevUnappliedActions) => {
             const _unappliedActions = { ...prevUnappliedActions };
-            _unappliedActions[device.name.v].pop();
+            _unappliedActions[deviceId].pop();
 
             return _unappliedActions;
         });
@@ -385,21 +386,21 @@ const useIsDeviceShrinkable = ({ device }) => {
     useEffect(() => {
         const getIsShrinkable = async () => {
             const isShrinkable = await isDeviceShrinkable({
-                deviceName: device.name.v,
+                device,
                 deviceTree: partitioning.deviceTree.path,
             });
 
             setIsShrinkable(isShrinkable);
         };
         getIsShrinkable();
-    }, [device.name.v, partitioning.deviceTree.path]);
+    }, [device, partitioning.deviceTree.path]);
 
     return isShrinkable;
 };
 
 const DeviceActionShrink = ({ device, hasBeenRemoved, newDeviceSize, onAction }) => {
     const onShrink = value => onAction("shrink", value);
-    const isDeviceShrinkable = useIsDeviceShrinkable({ device });
+    const isDeviceShrinkable = useIsDeviceShrinkable({ device: device["device-id"].v });
     const shrinkButton = <ShrinkPopover device={device} isDisabled={!isDeviceShrinkable} onShrink={onShrink} />;
 
     if (hasBeenRemoved) {