From dca6903167dca1cb8cf095f4ea4de88c6d358885 Mon Sep 17 00:00:00 2001
From: Mason Hu <mason.hu@canonical.com>
Date: Fri, 21 Feb 2025 10:09:14 +0200
Subject: [PATCH] rewview: review changes

Signed-off-by: Mason Hu <mason.hu@canonical.com>
---
 src/api/networks.tsx                          |  8 +++---
 src/api/storage-pools.tsx                     | 27 ++++++++++++++-----
 src/components/forms/DiskSizeSelector.tsx     |  4 +++
 src/pages/storage/EditStoragePool.tsx         |  7 ++---
 .../storage/actions/CreateStoragePoolBtn.tsx  |  6 ++---
 .../storage/forms/StoragePoolFormMain.tsx     |  1 +
 src/util/configInheritance.spec.ts            |  1 +
 src/util/storagePoolForm.tsx                  |  2 ++
 8 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/src/api/networks.tsx b/src/api/networks.tsx
index 4c229a3b7d..089e4d74eb 100644
--- a/src/api/networks.tsx
+++ b/src/api/networks.tsx
@@ -22,10 +22,10 @@ export const fetchNetworks = (
   target?: string,
 ): Promise<LxdNetwork[]> => {
   const targetParam = target ? `&target=${target}` : "";
-  const entitlements = `&${withEntitlementsQuery(
+  const entitlements = withEntitlementsQuery(
     isFineGrained,
     networkEntitlements,
-  )}`;
+  );
   return new Promise((resolve, reject) => {
     fetch(
       `/1.0/networks?project=${project}&recursion=1${targetParam}${entitlements}`,
@@ -81,10 +81,10 @@ export const fetchNetwork = (
   target?: string,
 ): Promise<LxdNetwork> => {
   const targetParam = target ? `&target=${target}` : "";
-  const entitlements = `&${withEntitlementsQuery(
+  const entitlements = withEntitlementsQuery(
     isFineGrained,
     networkEntitlements,
-  )}`;
+  );
   return new Promise((resolve, reject) => {
     fetch(
       `/1.0/networks/${name}?project=${project}${targetParam}${entitlements}`,
diff --git a/src/api/storage-pools.tsx b/src/api/storage-pools.tsx
index c3751d0407..4d9d0de85f 100644
--- a/src/api/storage-pools.tsx
+++ b/src/api/storage-pools.tsx
@@ -27,9 +27,12 @@ export const fetchStoragePool = (
   isFineGrained: boolean | null,
   target?: string,
 ): Promise<LxdStoragePool> => {
-  const entitlements = `&${withEntitlementsQuery(isFineGrained, storagePoolEntitlements)}`;
+  const entitlements = withEntitlementsQuery(
+    isFineGrained,
+    storagePoolEntitlements,
+  );
   return new Promise((resolve, reject) => {
-    const targetParam = `&target=${target}`;
+    const targetParam = target ? `&target=${target}` : "";
     fetch(`/1.0/storage-pools/${pool}?recursion=1${targetParam}${entitlements}`)
       .then(handleResponse)
       .then((data: LxdApiResponse<LxdStoragePool>) => resolve(data.metadata))
@@ -40,7 +43,10 @@ export const fetchStoragePool = (
 export const fetchStoragePools = (
   isFineGrained: boolean | null,
 ): Promise<LxdStoragePool[]> => {
-  const entitlements = `&${withEntitlementsQuery(isFineGrained, storagePoolEntitlements)}`;
+  const entitlements = withEntitlementsQuery(
+    isFineGrained,
+    storagePoolEntitlements,
+  );
   return new Promise((resolve, reject) => {
     fetch(`/1.0/storage-pools?recursion=1${entitlements}`)
       .then(handleResponse)
@@ -247,7 +253,10 @@ export const fetchStorageVolumes = (
   project: string,
   isFineGrained: boolean | null,
 ): Promise<LxdStorageVolume[]> => {
-  const entitlements = `&${withEntitlementsQuery(isFineGrained, storageVolumeEntitlements)}`;
+  const entitlements = withEntitlementsQuery(
+    isFineGrained,
+    storageVolumeEntitlements,
+  );
   return new Promise((resolve, reject) => {
     fetch(
       `/1.0/storage-pools/${pool}/volumes?project=${project}&recursion=1${entitlements}`,
@@ -264,7 +273,10 @@ export const fetchAllStorageVolumes = (
   project: string,
   isFineGrained: boolean | null,
 ): Promise<LxdStorageVolume[]> => {
-  const entitlements = `&${withEntitlementsQuery(isFineGrained, storageVolumeEntitlements)}`;
+  const entitlements = withEntitlementsQuery(
+    isFineGrained,
+    storageVolumeEntitlements,
+  );
   return new Promise((resolve, reject) => {
     fetch(`/1.0/storage-volumes?recursion=1&project=${project}${entitlements}`)
       .then(handleResponse)
@@ -282,7 +294,10 @@ export const fetchStorageVolume = (
   volume: string,
   isFineGrained: boolean | null,
 ): Promise<LxdStorageVolume> => {
-  const entitlements = `&${withEntitlementsQuery(isFineGrained, storageVolumeEntitlements)}`;
+  const entitlements = withEntitlementsQuery(
+    isFineGrained,
+    storageVolumeEntitlements,
+  );
   return new Promise((resolve, reject) => {
     fetch(
       `/1.0/storage-pools/${pool}/volumes/${type}/${volume}?project=${project}&recursion=1${entitlements}`,
diff --git a/src/components/forms/DiskSizeSelector.tsx b/src/components/forms/DiskSizeSelector.tsx
index ddf172cc8e..446256e7b7 100644
--- a/src/components/forms/DiskSizeSelector.tsx
+++ b/src/components/forms/DiskSizeSelector.tsx
@@ -12,6 +12,7 @@ interface Props {
   setMemoryLimit: (val: string) => void;
   disabled?: boolean;
   classname?: string;
+  disabledReason?: string;
 }
 
 const DiskSizeSelector: FC<Props> = ({
@@ -23,6 +24,7 @@ const DiskSizeSelector: FC<Props> = ({
   setMemoryLimit,
   disabled,
   classname,
+  disabledReason,
 }) => {
   const limit = parseMemoryLimit(value) ?? {
     value: 1,
@@ -55,6 +57,7 @@ const DiskSizeSelector: FC<Props> = ({
           value={value?.match(/^\d/) ? limit.value : ""}
           disabled={disabled}
           className={classname}
+          title={disabledReason}
         />
         <Select
           id={`memUnitSelect-${id}`}
@@ -68,6 +71,7 @@ const DiskSizeSelector: FC<Props> = ({
           value={limit.unit}
           disabled={disabled}
           className={classname}
+          title={disabledReason}
         />
       </div>
       {(help || helpTotal) && (
diff --git a/src/pages/storage/EditStoragePool.tsx b/src/pages/storage/EditStoragePool.tsx
index 27e19c5abd..235daec0f1 100644
--- a/src/pages/storage/EditStoragePool.tsx
+++ b/src/pages/storage/EditStoragePool.tsx
@@ -80,10 +80,11 @@ const EditStoragePool: FC<Props> = ({ pool }) => {
     : undefined;
 
   const formik = useFormik<StoragePoolFormValues>({
-    initialValues: {
-      ...toStoragePoolFormValues(pool, poolOnMembers),
+    initialValues: toStoragePoolFormValues(
+      pool,
+      poolOnMembers,
       editRestriction,
-    },
+    ),
     validationSchema: StoragePoolSchema,
     enableReinitialize: true,
     onSubmit: (values) => {
diff --git a/src/pages/storage/actions/CreateStoragePoolBtn.tsx b/src/pages/storage/actions/CreateStoragePoolBtn.tsx
index e0c1a43b05..511e39463f 100644
--- a/src/pages/storage/actions/CreateStoragePoolBtn.tsx
+++ b/src/pages/storage/actions/CreateStoragePoolBtn.tsx
@@ -20,9 +20,9 @@ const CreateStoragePoolBtn: FC<Props> = ({ project, className }) => {
       className={className}
       hasIcon={!isSmallScreen}
       title={
-        !canCreateStoragePools()
-          ? "You do not have permission to create storage pools"
-          : ""
+        canCreateStoragePools()
+          ? ""
+          : "You do not have permission to create storage pools"
       }
       onClick={() => navigate(`/ui/project/${project}/storage/pools/create`)}
       disabled={!canCreateStoragePools()}
diff --git a/src/pages/storage/forms/StoragePoolFormMain.tsx b/src/pages/storage/forms/StoragePoolFormMain.tsx
index 4e15f06268..4a62e9cdac 100644
--- a/src/pages/storage/forms/StoragePoolFormMain.tsx
+++ b/src/pages/storage/forms/StoragePoolFormMain.tsx
@@ -177,6 +177,7 @@ const StoragePoolFormMain: FC<Props> = ({ formik }) => {
                   !!formik.values.editRestriction ||
                   formik.values.driver === dirDriver
                 }
+                disabledReason={formik.values.editRestriction}
               />
             ))}
           {hasSource &&
diff --git a/src/util/configInheritance.spec.ts b/src/util/configInheritance.spec.ts
index b3ab7bf17a..0d0204831b 100644
--- a/src/util/configInheritance.spec.ts
+++ b/src/util/configInheritance.spec.ts
@@ -43,6 +43,7 @@ beforeEach(() => {
       },
     }),
   }));
+  // this mock is required for getStorageVolumeRowMetadata since we call useStoragePool that invokes useContext from react
   vi.mock(import("react"), async (importOriginal) => {
     const actual = await importOriginal();
     return {
diff --git a/src/util/storagePoolForm.tsx b/src/util/storagePoolForm.tsx
index bfbc29a417..7d9e0780e2 100644
--- a/src/util/storagePoolForm.tsx
+++ b/src/util/storagePoolForm.tsx
@@ -33,6 +33,7 @@ export const isStoragePoolWithSource = (driver: string) => {
 export const toStoragePoolFormValues = (
   pool: LxdStoragePool,
   poolOnMembers?: LXDStoragePoolOnClusterMember[],
+  editRestriction?: string,
 ): StoragePoolFormValues => {
   const sourcePerClusterMember: ClusterSpecificValues = {};
   const zfsPoolNamePerClusterMember: ClusterSpecificValues = {};
@@ -88,6 +89,7 @@ export const toStoragePoolFormValues = (
     zfs_export: pool.config?.["zfs.export"],
     zfs_pool_name: pool.config?.["zfs.pool_name"],
     zfsPoolNamePerClusterMember,
+    editRestriction,
   };
 };