review: review changes

Signed-off-by: Mason Hu <[email protected]>
canonical · Feb 13, 2025 · c43c795 · c43c795
1 parent 0c86b2d
commit c43c795
Show file tree

Hide file tree

Showing 19 changed files with 170 additions and 141 deletions.
diff --git a/src/components/ConfigurationRow.tsx b/src/components/ConfigurationRow.tsx
@@ -75,6 +75,22 @@ export const getConfigurationRow = ({
     }
   };
 
+  const isDisabled = () => {
+    return disabled || !!formik.values.editRestriction;
+  };
+
+  const getDisabledReasonOrTitle = (title?: string) => {
+    if (formik.values.editRestriction) {
+      return formik.values.editRestriction;
+    }
+
+    if (disabledReason) {
+      return disabledReason;
+    }
+
+    return title;
+  };
+
   const getForm = (): ReactNode => {
     return (
       <div className="override-form">
@@ -86,7 +102,7 @@ export const getConfigurationRow = ({
               onBlur: formik.handleBlur,
               onChange: formik.handleChange,
               value,
-              disabled: disabled || !!formik.values.editRestriction,
+              disabled: isDisabled(),
               help: (
                 <ConfigFieldDescription
                   description={
@@ -104,10 +120,8 @@ export const getConfigurationRow = ({
           onClick={toggleDefault}
           type="button"
           appearance="base"
-          title={
-            formik.values.editRestriction ?? disabledReason ?? "Clear override"
-          }
-          disabled={disabled || !!formik.values.editRestriction}
+          title={getDisabledReasonOrTitle("Clear override")}
+          disabled={isDisabled()}
           hasIcon
           className="u-no-margin--bottom"
         >
@@ -128,10 +142,7 @@ export const getConfigurationRow = ({
   const wrapDisabledTooltip = (children: ReactNode): ReactNode => {
     if ((disabled && disabledReason) || formik.values.editRestriction) {
       return (
-        <Tooltip
-          message={formik.values.editRestriction ?? disabledReason}
-          position="right"
-        >
+        <Tooltip message={getDisabledReasonOrTitle()} position="right">
           {children}
         </Tooltip>
       );
@@ -141,16 +152,6 @@ export const getConfigurationRow = ({
 
   const renderOverride = (): ReactNode => {
     if (formik.values.readOnly) {
-      const getTitle = () => {
-        if (formik.values.editRestriction) {
-          return formik.values.editRestriction;
-        }
-        if (disabled) {
-          return disabledReason;
-        }
-
-        return isOverridden ? "Edit" : "Create override";
-      };
       return (
         <>
           {overrideValue}
@@ -165,8 +166,10 @@ export const getConfigurationRow = ({
             className="u-no-margin--bottom"
             type="button"
             appearance="base"
-            title={getTitle()}
-            disabled={disabled || !!formik.values.editRestriction}
+            title={getDisabledReasonOrTitle(
+              isOverridden ? "Edit" : "Create override",
+            )}
+            disabled={isDisabled()}
             hasIcon
           >
             <Icon name="edit" />
@@ -182,7 +185,7 @@ export const getConfigurationRow = ({
         onClick={toggleDefault}
         className="u-no-margin--bottom"
         type="button"
-        disabled={disabled || !!formik.values.editRestriction}
+        disabled={isDisabled()}
         appearance="base"
         title={formik.values.editRestriction ?? "Create override"}
         hasIcon

diff --git a/src/components/forms/FormSubmitBtn.tsx b/src/components/forms/FormSubmitBtn.tsx
@@ -17,14 +17,8 @@ const FormSubmitBtn: FC<Props> = ({ formik, disabled, isYaml = false }) => {
     <ActionButton
       appearance="positive"
       loading={formik.isSubmitting}
-      disabled={
-        !formik.isValid ||
-        disabled ||
-        changeCount === 0 ||
-        !!formik.values.editRestriction
-      }
+      disabled={!formik.isValid || disabled || changeCount === 0}
       onClick={() => void formik.submitForm()}
-      title={formik.values.editRestriction}
     >
       {changeCount === 0 || isYaml
         ? "Save changes"

diff --git a/src/components/forms/InheritedDeviceRow.tsx b/src/components/forms/InheritedDeviceRow.tsx
@@ -83,46 +83,47 @@ export const getInheritedDeviceRow = ({
         )}
       </div>
     ),
-    override:
-      readOnly || disabledReason ? (
-        overrideValue ? (
-          <div className="mono-font">
-            <b>{overrideValue}</b>
-          </div>
-        ) : (
-          ""
-        )
-      ) : overrideValue ? (
-        <div className="override-form">
-          <div>{overrideForm}</div>
-          {clearOverride && (
-            <div>
-              <Button
-                onClick={clearOverride}
-                type="button"
-                appearance="base"
-                title="Clear override"
-                hasIcon
-                className="u-no-margin--bottom"
-              >
-                <Icon name="close" className="clear-configuration-icon" />
-              </Button>
-            </div>
-          )}
+    override: readOnly ? (
+      overrideValue ? (
+        <div className="mono-font">
+          <b>{overrideValue}</b>
         </div>
       ) : (
-        addOverride && (
-          <Button
-            onClick={addOverride}
-            type="button"
-            appearance="base"
-            title="Create override"
-            className="u-no-margin--bottom"
-            hasIcon
-          >
-            <Icon name="edit" />
-          </Button>
-        )
-      ),
+        ""
+      )
+    ) : overrideValue ? (
+      <div className="override-form">
+        <div>{overrideForm}</div>
+        {clearOverride && (
+          <div>
+            <Button
+              onClick={clearOverride}
+              type="button"
+              appearance="base"
+              title={disabledReason ?? "Clear override"}
+              hasIcon
+              className="u-no-margin--bottom"
+              disabled={!!disabledReason}
+            >
+              <Icon name="close" className="clear-configuration-icon" />
+            </Button>
+          </div>
+        )}
+      </div>
+    ) : (
+      addOverride && (
+        <Button
+          onClick={addOverride}
+          type="button"
+          appearance="base"
+          title={disabledReason ?? "Create override"}
+          className="u-no-margin--bottom"
+          hasIcon
+          disabled={!!disabledReason}
+        >
+          <Icon name="edit" />
+        </Button>
+      )
+    ),
   });
 };
diff --git a/src/components/forms/NetworkDevicesForm.tsx b/src/components/forms/NetworkDevicesForm.tsx
@@ -216,7 +216,7 @@ or remove the originating item"
                         }}
                         type="button"
                         appearance="base"
-                        title={formik.values.editRestriction || "Edit network"}
+                        title={formik.values.editRestriction ?? "Edit network"}
                         className="u-no-margin--top"
                         hasIcon
                         dense
@@ -235,7 +235,7 @@ or remove the originating item"
                       appearance="base"
                       hasIcon
                       dense
-                      title={formik.values.editRestriction || "Detach network"}
+                      title={formik.values.editRestriction ?? "Detach network"}
                       disabled={!!formik.values.editRestriction}
                     >
                       <Icon name="disconnect" />
@@ -258,13 +258,8 @@ or remove the originating item"
               }}
               type="button"
               hasIcon
-              disabled={
-                !!formik.values.editRestriction || !managedNetworks.length
-              }
-              title={
-                formik.values.editRestriction ??
-                (!managedNetworks.length ? "No networks available" : "")
-              }
+              disabled={!!formik.values.editRestriction}
+              title={formik.values.editRestriction}
             >
               <Icon name="plus" />
               <span>Attach network</span>

diff --git a/src/components/forms/OtherDeviceForm.tsx b/src/components/forms/OtherDeviceForm.tsx
@@ -142,7 +142,7 @@ const OtherDeviceForm: FC<Props> = ({ formik, project }) => {
             appearance="base"
             hasIcon
             dense
-            title={formik.values.editRestriction || "Detach device"}
+            title={formik.values.editRestriction ?? "Detach device"}
             disabled={!!formik.values.editRestriction}
           >
             <Icon name="disconnect" />
@@ -207,7 +207,7 @@ const OtherDeviceForm: FC<Props> = ({ formik, project }) => {
             appearance="base"
             hasIcon
             dense
-            title={formik.values.editRestriction || "Detach GPU"}
+            title={formik.values.editRestriction ?? "Detach GPU"}
             disabled={!!formik.values.editRestriction}
           >
             <Icon name="disconnect" />

diff --git a/src/components/forms/ProxyDeviceForm.tsx b/src/components/forms/ProxyDeviceForm.tsx
@@ -108,9 +108,7 @@ const ProxyDeviceForm: FC<Props> = ({ formik, project }) => {
           options={options}
           help={<ConfigFieldDescription description={help} />}
           className="u-no-margin--bottom"
-          disabled={
-            disabledText != undefined || !!formik.values.editRestriction
-          }
+          disabled={!!disabledText || !!formik.values.editRestriction}
           title={formik.values.editRestriction ?? disabledText}
         />
       ),

diff --git a/src/pages/instances/CreateInstance.tsx b/src/pages/instances/CreateInstance.tsx
@@ -101,7 +101,7 @@ import BootForm, {
   BootFormValues,
   bootPayload,
 } from "components/forms/BootForm";
-import { instanceProfilesWarning } from "util/instanceEdit";
+import InstanceProfilesWarning from "./InstanceProfilesWarning";
 
 export type CreateInstanceFormValues = InstanceDetailsFormValues &
   FormDeviceValues &
@@ -439,7 +439,6 @@ const CreateInstance: FC = () => {
   const networkError = hasNetworkError(formik);
   const hasErrors =
     !formik.isValid || !formik.values.image || diskError || networkError;
-  const isCreating = true;
 
   return (
     <BaseLayout title="Create an instance" contentClassName="create-instance">
@@ -455,7 +454,11 @@ const CreateInstance: FC = () => {
         )}
         <Row className="form-contents" key={section}>
           <Col size={12}>
-            {instanceProfilesWarning([], profiles, isCreating)}
+            <InstanceProfilesWarning
+              instanceProfiles={[]}
+              profiles={profiles}
+              isCreating={true}
+            />
             <NotificationRow />
             {section === MAIN_CONFIGURATION && (
               <InstanceCreateDetailsForm

diff --git a/src/pages/instances/EditInstance.tsx b/src/pages/instances/EditInstance.tsx
@@ -47,7 +47,6 @@ import {
   getInstanceEditValues,
   getInstancePayload,
   InstanceEditSchema,
-  instanceProfilesWarning,
 } from "util/instanceEdit";
 import { slugify } from "util/slugify";
 import { useEventQueue } from "context/eventQueue";
@@ -68,6 +67,7 @@ import InstanceLinkChip from "./InstanceLinkChip";
 import BootForm, { BootFormValues } from "components/forms/BootForm";
 import { useInstanceEntitlements } from "util/entitlements/instances";
 import { fetchProfiles } from "api/profiles";
+import InstanceProfilesWarning from "./InstanceProfilesWarning";
 
 export interface InstanceEditDetailsFormValues {
   name: string;
@@ -214,7 +214,10 @@ const EditInstance: FC<Props> = ({ instance }) => {
         )}
         <Row className="form-contents" key={section}>
           <Col size={12}>
-            {instanceProfilesWarning(instance.profiles, profiles)}
+            <InstanceProfilesWarning
+              instanceProfiles={instance.profiles}
+              profiles={profiles}
+            />
             {(section === slugify(MAIN_CONFIGURATION) || !section) && (
               <EditInstanceDetails formik={formik} project={project} />
             )}

diff --git a/src/pages/instances/InstanceConsole.tsx b/src/pages/instances/InstanceConsole.tsx
@@ -59,6 +59,14 @@ const InstanceConsole: FC<Props> = ({ instance }) => {
 
   const { handleStart, isLoading } = useInstanceStart(instance);
 
+  if (!canAccessInstanceConsole(instance)) {
+    return (
+      <Notification severity="caution" title="Restricted permissions">
+        You do not have permission to access the console for this instance.
+      </Notification>
+    );
+  }
+
   return (
     <div className="instance-console-tab">
       {isVm && canAccessInstanceConsole(instance) && (
@@ -147,11 +155,11 @@ const InstanceConsole: FC<Props> = ({ instance }) => {
           showNotRunningInfo={showNotRunningInfo}
         />
       )}
-      {!canAccessInstanceConsole(instance) && (
+      {/* {!canAccessInstanceConsole(instance) && (
         <Notification severity="caution" title="Restricted permissions">
           You do not have permission to access the console for this instance.
         </Notification>
-      )}
+      )} */}
     </div>
   );
 };

diff --git a/src/pages/instances/InstanceOverviewProfiles.tsx b/src/pages/instances/InstanceOverviewProfiles.tsx
@@ -6,7 +6,7 @@ import Loader from "components/Loader";
 import type { LxdInstance } from "types/instance";
 import { fetchProfiles } from "api/profiles";
 import ResourceLink from "components/ResourceLink";
-import { instanceProfilesWarning } from "util/instanceEdit";
+import InstanceProfilesWarning from "./InstanceProfilesWarning";
 
 interface Props {
   instance: LxdInstance;
@@ -78,7 +78,12 @@ const InstanceOverviewProfiles: FC<Props> = ({ instance, onFailure }) => {
     }
 
     if (!profiles.length) {
-      return instanceProfilesWarning(instance.profiles, profiles);
+      return (
+        <InstanceProfilesWarning
+          instanceProfiles={instance.profiles}
+          profiles={profiles}
+        />
+      );
     }
 
     return <MainTable headers={profileHeaders} rows={profileRows} sortable />;