review: misc fixes

Signed-off-by: Mason Hu <[email protected]>
canonical · Feb 17, 2025 · b11a5c5 · b11a5c5
1 parent a0bcbd3
commit b11a5c5
Show file tree

Hide file tree

Showing 12 changed files with 80 additions and 102 deletions.
diff --git a/src/pages/instances/CreateInstance.tsx b/src/pages/instances/CreateInstance.tsx
@@ -101,7 +101,6 @@ import BootForm, {
   BootFormValues,
   bootPayload,
 } from "components/forms/BootForm";
-import InstanceProfilesWarning from "./InstanceProfilesWarning";
 
 export type CreateInstanceFormValues = InstanceDetailsFormValues &
   FormDeviceValues &
@@ -375,7 +374,7 @@ const CreateInstance: FC = () => {
       return ["default"];
     }
 
-    return [profiles[0].name];
+    return [];
   };
 
   useEffect(() => {
@@ -472,11 +471,6 @@ const CreateInstance: FC = () => {
         )}
         <Row className="form-contents" key={section}>
           <Col size={12}>
-            <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
@@ -123,9 +123,9 @@ const EditInstance: FC<Props> = ({ instance }) => {
   useEffect(updateFormHeight, [section]);
   useEventListener("resize", updateFormHeight);
 
-  const editRestriction = !canEditInstance(instance)
-    ? "You do not have permission to edit this instance"
-    : undefined;
+  const editRestriction = canEditInstance(instance)
+    ? undefined
+    : "You do not have permission to edit this instance";
 
   const formik = useFormik<EditInstanceFormValues>({
     initialValues: getInstanceEditValues(instance, editRestriction),
@@ -215,10 +215,13 @@ const EditInstance: FC<Props> = ({ instance }) => {
         )}
         <Row className="form-contents" key={section}>
           <Col size={12}>
-            <InstanceProfilesWarning
-              instanceProfiles={instance.profiles}
-              profiles={profiles}
-            />
+            {section !== slugify(YAML_CONFIGURATION) && (
+              <InstanceProfilesWarning
+                instanceProfiles={instance.profiles}
+                profiles={profiles}
+                isEditing
+              />
+            )}
             {(section === slugify(MAIN_CONFIGURATION) || !section) && (
               <EditInstanceDetails formik={formik} project={project} />
             )}

diff --git a/src/pages/instances/InstanceOverviewProfiles.tsx b/src/pages/instances/InstanceOverviewProfiles.tsx
@@ -6,7 +6,6 @@ import Loader from "components/Loader";
 import type { LxdInstance } from "types/instance";
 import { fetchProfiles } from "api/profiles";
 import ResourceLink from "components/ResourceLink";
-import InstanceProfilesWarning from "./InstanceProfilesWarning";
 
 interface Props {
   instance: LxdInstance;
@@ -37,11 +36,7 @@ const InstanceOverviewProfiles: FC<Props> = ({ instance, onFailure }) => {
   ];
 
   const profileRows = instance.profiles.map((profile) => {
-    if (profiles.length < 1) {
-      return {
-        columns: undefined,
-      };
-    }
+    const profileExists = profiles.some((item) => item.name === profile);
     const description =
       profiles.filter((item) => item.name === profile)[0]?.description ?? "";
     return {
@@ -52,7 +47,11 @@ const InstanceOverviewProfiles: FC<Props> = ({ instance, onFailure }) => {
             <ResourceLink
               type="profile"
               value={profile}
-              to={`/ui/project/${instance.project}/profile/${profile}`}
+              to={
+                profileExists
+                  ? `/ui/project/${instance.project}/profile/${profile}`
+                  : ""
+              }
             />
           ),
           role: "rowheader",
@@ -72,24 +71,11 @@ const InstanceOverviewProfiles: FC<Props> = ({ instance, onFailure }) => {
     };
   });
 
-  const getContent = () => {
-    if (isLoading) {
-      return <Loader text="Loading profiles..." />;
-    }
-
-    if (!profiles.length) {
-      return (
-        <InstanceProfilesWarning
-          instanceProfiles={instance.profiles}
-          profiles={profiles}
-        />
-      );
-    }
-
-    return <MainTable headers={profileHeaders} rows={profileRows} sortable />;
-  };
+  if (isLoading) {
+    return <Loader text="Loading profiles..." />;
+  }
 
-  return getContent();
+  return <MainTable headers={profileHeaders} rows={profileRows} sortable />;
 };
 
 export default InstanceOverviewProfiles;
diff --git a/src/pages/instances/InstanceProfilesWarning.tsx b/src/pages/instances/InstanceProfilesWarning.tsx
@@ -5,22 +5,26 @@ import { LxdProfile } from "types/profile";
 interface Props {
   instanceProfiles: string[];
   profiles?: LxdProfile[];
-  isCreating?: boolean;
+  isEditing?: boolean;
 }
 
 const InstanceProfilesWarning: FC<Props> = ({
   instanceProfiles,
   profiles,
-  isCreating,
+  isEditing,
 }) => {
-  const editContext = !isCreating
-    ? "This may cause inherited configuration values to be displayed incorrectly."
+  const editContext = isEditing
+    ? " This may cause inherited configuration values to be displayed incorrectly."
     : "";
 
+  if (!instanceProfiles.length) {
+    return null;
+  }
+
   if (!profiles?.length) {
     return (
       <Notification severity="caution" title="Restricted permissions">
-        You do not have permission to view profiles in the current project.{" "}
+        You do not have permission to view profiles in the current project.
         {editContext}
       </Notification>
     );

diff --git a/src/pages/instances/InstanceTerminal.tsx b/src/pages/instances/InstanceTerminal.tsx
@@ -218,15 +218,21 @@ const InstanceTerminal: FC<Props> = ({ instance, refreshInstance }) => {
 
   const { handleStart, isLoading: isStartLoading } = useInstanceStart(instance);
 
+  if (!canExec) {
+    return (
+      <Notification severity="caution" title="Restricted permissions">
+        You do not have permission to use the terminal for this instance.
+      </Notification>
+    );
+  }
+
   return (
     <div className="instance-terminal-tab">
       {canConnect && (
         <>
-          {canExec && (
-            <div className="p-panel__controls">
-              <ReconnectTerminalBtn reconnect={setPayload} payload={payload} />
-            </div>
-          )}
+          <div className="p-panel__controls">
+            <ReconnectTerminalBtn reconnect={setPayload} payload={payload} />
+          </div>
           <NotificationRow />
           {isLoading && <Loader text="Loading terminal session..." />}
           {controlWs && (
@@ -244,7 +250,7 @@ const InstanceTerminal: FC<Props> = ({ instance, refreshInstance }) => {
           )}
         </>
       )}
-      {!canConnect && canExec && (
+      {!canConnect && (
         <EmptyState
           className="empty-state"
           image={<Icon name="pods" className="empty-state-icon" />}
@@ -270,11 +276,6 @@ const InstanceTerminal: FC<Props> = ({ instance, refreshInstance }) => {
           </ActionButton>
         </EmptyState>
       )}
-      {!canExec && (
-        <Notification severity="caution" title="Restricted permissions">
-          You do not have permission to use the terminal for this instance.
-        </Notification>
-      )}
     </div>
   );
 };

diff --git a/src/pages/instances/actions/CreateImageFromInstanceBtn.tsx b/src/pages/instances/actions/CreateImageFromInstanceBtn.tsx
@@ -32,7 +32,7 @@ const CreateImageFromInstanceBtn: FC<Props> = ({
 
   const getDisabledReason = () => {
     if (!canCreateImages(project)) {
-      return `You do not have permission to create images in project ${instance.project}`;
+      return `You do not have permission to create images in this project`;
     }
 
     const isDisabled =

diff --git a/src/pages/instances/actions/InstanceBulkAction.tsx b/src/pages/instances/actions/InstanceBulkAction.tsx
@@ -114,16 +114,6 @@ const InstanceBulkAction: FC<Props> = ({
       return null;
     }
 
-    if (allRestricted) {
-      return (
-        <Fragment key="restricted">
-          - You do not have permission to {confirmLabel.toLowerCase()} the
-          selected {pluralize("instance", instances.length)}.
-          <br />
-        </Fragment>
-      );
-    }
-
     return (
       <Fragment key="restricted">
         - No action for <b>{restrictedInstances.length}</b>{" "}
@@ -137,7 +127,7 @@ const InstanceBulkAction: FC<Props> = ({
   return (
     <ConfirmationButton
       appearance="base"
-      disabled={isDisabled || !hasChangedStates}
+      disabled={isDisabled || !hasChangedStates || allRestricted}
       loading={isLoading}
       className="u-no-margin--right u-no-margin--bottom bulk-action has-icon"
       confirmationModalProps={{
@@ -153,10 +143,14 @@ const InstanceBulkAction: FC<Props> = ({
         onConfirm: onClick,
         confirmButtonLabel: confirmLabel,
         confirmButtonAppearance: confirmAppearance,
-        confirmButtonDisabled: allRestricted,
       }}
       shiftClickEnabled
       showShiftClickHint
+      onHoverText={
+        allRestricted
+          ? `You do not have permission to ${confirmLabel.toLowerCase()} the selected ${pluralize("instance", instances.length)}`
+          : confirmLabel
+      }
     >
       <Icon name={icon} />
       <span>{confirmLabel}</span>

diff --git a/src/pages/instances/actions/InstanceBulkDelete.tsx b/src/pages/instances/actions/InstanceBulkDelete.tsx
@@ -100,8 +100,8 @@ const InstanceBulkDelete: FC<Props> = ({ instances, onStart, onFinish }) => {
 
     return (
       <Fragment key="restricted-instances">
-        - {restrictedCount} restricted {pluralize("instance", deleteCount)} will
-        be ignored
+        - {restrictedCount} {pluralize("instance", deleteCount)} that you do not
+        have permission to delete will be ignored
         <br />
       </Fragment>
     );
@@ -125,7 +125,11 @@ const InstanceBulkDelete: FC<Props> = ({ instances, onStart, onFinish }) => {
     <div className="p-segmented-control bulk-actions">
       <div className="p-segmented-control__list bulk-action-frame">
         <ConfirmationButton
-          onHoverText="Delete instances"
+          onHoverText={
+            restrictedCount === totalCount
+              ? `You do not have permission to delete the selected ${pluralize("instance", instances.length)}`
+              : "Delete instances"
+          }
           appearance="base"
           className="u-no-margin--bottom has-icon"
           loading={isLoading}

diff --git a/src/pages/instances/actions/snapshots/CreateInstanceFromSnapshotBtn.tsx b/src/pages/instances/actions/snapshots/CreateInstanceFromSnapshotBtn.tsx
@@ -23,7 +23,7 @@ const CreateInstanceFromSnapshotBtn: FC<Props> = ({
   const { data: projects = [] } = useProjects();
   const { canCreateInstances } = useProjectEntitlements();
 
-  const projectsWithPermission = projects.filter(canCreateInstances);
+  const validTargetProjects = projects.filter(canCreateInstances);
 
   return (
     <>
@@ -41,12 +41,12 @@ const CreateInstanceFromSnapshotBtn: FC<Props> = ({
         hasIcon
         dense
         aria-label="Create instance"
-        disabled={isDeleting || isRestoring || !projectsWithPermission.length}
+        disabled={isDeleting || isRestoring || !validTargetProjects.length}
         onClick={openPortal}
         title={
-          projectsWithPermission.length
+          validTargetProjects.length > 0
             ? "Create instance"
-            : "You do not have permission to create instances in any project"
+            : "You do not have permission to create instances"
         }
       >
         <Icon name="plus" />

diff --git a/src/pages/instances/actions/snapshots/InstanceSnapshotActions.tsx b/src/pages/instances/actions/snapshots/InstanceSnapshotActions.tsx
@@ -38,9 +38,9 @@ const InstanceSnapshotActions: FC<Props> = ({
   const [restoreState, setRestoreState] = useState(true);
   const queryClient = useQueryClient();
   const { canManageInstanceSnapshots } = useInstanceEntitlements();
-  const disabledReason = !canManageInstanceSnapshots(instance)
-    ? "You do not have permission to manage snapshots"
-    : undefined;
+  const disabledReason = canManageInstanceSnapshots(instance)
+    ? undefined
+    : "You do not have permission to manage snapshots for this instance";
 
   const handleDelete = () => {
     setDeleting(true);

diff --git a/src/pages/instances/forms/CreateImageFromInstanceForm.tsx b/src/pages/instances/forms/CreateImageFromInstanceForm.tsx
@@ -86,7 +86,7 @@ const CreateImageFromInstanceForm: FC<Props> = ({ instance, close }) => {
           eventQueue.set(
             operation.metadata.id,
             (event) => {
-              if (alias && canCreateImageAliases(project)) {
+              if (alias) {
                 const fingerprint = event.metadata.metadata?.fingerprint ?? "";
                 void createImageAlias(fingerprint, alias, instance.project)
                   .then(clearCache)
@@ -159,7 +159,7 @@ const CreateImageFromInstanceForm: FC<Props> = ({ instance, close }) => {
           title={
             canCreateImageAliases(project)
               ? ""
-              : `You do not have permission to create image aliases in project ${instance.project}`
+              : `You do not have permission to create image aliases in this project`
           }
         />
         <Input

diff --git a/src/pages/profiles/ProfileSelector.tsx b/src/pages/profiles/ProfileSelector.tsx
@@ -60,17 +60,15 @@ const ProfileSelector: FC<Props> = ({
 
   profiles.sort(defaultFirst);
 
-  // determine if any selected profile is not in the profiles list
-  // this indicates that there are profiles set on the instance that the user does not have permission to view
-  const profileNames = profiles.map((profile) => profile.name);
-  const restrictedProfileNames = (initialProfiles || []).filter(
-    (profile) => !profileNames.find((name) => name === profile),
-  );
+  // we combine the initial instance profiles and the profiles from the API list endpoint into a deduplicated list
+  // this way, we take into account profiles that the user does not have permission to view
+  const profileNames = [
+    ...new Set(
+      profiles.map((profile) => profile.name).concat(initialProfiles ?? []),
+    ),
+  ];
 
-  const allProfileNames = [...profileNames, ...restrictedProfileNames];
-  const unselected = selected.length
-    ? allProfileNames.filter((name) => !selected.includes(name))
-    : allProfileNames;
+  const unselected = profileNames.filter((name) => !selected.includes(name));
 
   const addProfile = () => {
     const nextProfile = unselected[0];
@@ -81,25 +79,19 @@ const ProfileSelector: FC<Props> = ({
 
   const profileOptions = (currSelectIndex: number) => {
     const profileOptions: { label: string; value: string }[] = [];
-    const profileSelectFilter = (profileName: string) =>
-      !selected.includes(profileName) ||
-      selected.indexOf(profileName) === currSelectIndex;
-
-    profileNames.filter(profileSelectFilter).forEach((name) => {
-      profileOptions.push({
-        label: name,
-        value: name,
-      });
-    });
 
-    if (restrictedProfileNames.length) {
-      restrictedProfileNames.filter(profileSelectFilter).forEach((name) => {
+    profileNames
+      .filter(
+        (profileName: string) =>
+          !selected.includes(profileName) ||
+          selected.indexOf(profileName) === currSelectIndex,
+      )
+      .forEach((name) => {
         profileOptions.push({
           label: name,
           value: name,
         });
       });
-    }
 
     return profileOptions;
   };