From c43c7955302f858b76cb83458dae2aa1d105e9fc Mon Sep 17 00:00:00 2001 From: Mason Hu Date: Thu, 13 Feb 2025 14:03:30 +0200 Subject: [PATCH] review: review changes Signed-off-by: Mason Hu --- src/components/ConfigurationRow.tsx | 47 +++++------ src/components/forms/FormSubmitBtn.tsx | 8 +- src/components/forms/InheritedDeviceRow.tsx | 79 ++++++++++--------- src/components/forms/NetworkDevicesForm.tsx | 13 +-- src/components/forms/OtherDeviceForm.tsx | 4 +- src/components/forms/ProxyDeviceForm.tsx | 4 +- src/pages/instances/CreateInstance.tsx | 9 ++- src/pages/instances/EditInstance.tsx | 7 +- src/pages/instances/InstanceConsole.tsx | 12 ++- .../instances/InstanceOverviewProfiles.tsx | 9 ++- .../instances/InstanceProfilesWarning.tsx | 44 +++++++++++ .../instances/actions/InstanceBulkAction.tsx | 17 +++- src/pages/profiles/CreateProfile.tsx | 4 +- src/pages/profiles/EditProfile.tsx | 4 +- src/pages/projects/ProjectSelectTable.tsx | 4 +- src/pages/storage/CustomVolumeSelectModal.tsx | 2 +- src/util/entitlements/instances.tsx | 8 +- src/util/instanceEdit.tsx | 34 -------- src/util/proxyDevices.tsx | 2 +- 19 files changed, 170 insertions(+), 141 deletions(-) create mode 100644 src/pages/instances/InstanceProfilesWarning.tsx diff --git a/src/components/ConfigurationRow.tsx b/src/components/ConfigurationRow.tsx index 08822dd84a..33ba5a63a8 100644 --- 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 (
@@ -86,7 +102,7 @@ export const getConfigurationRow = ({ onBlur: formik.handleBlur, onChange: formik.handleChange, value, - disabled: disabled || !!formik.values.editRestriction, + disabled: isDisabled(), help: ( @@ -128,10 +142,7 @@ export const getConfigurationRow = ({ const wrapDisabledTooltip = (children: ReactNode): ReactNode => { if ((disabled && disabledReason) || formik.values.editRestriction) { return ( - + {children} ); @@ -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 > @@ -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 index 67716139c6..666c10677b 100644 --- a/src/components/forms/FormSubmitBtn.tsx +++ b/src/components/forms/FormSubmitBtn.tsx @@ -17,14 +17,8 @@ const FormSubmitBtn: FC = ({ formik, disabled, isYaml = false }) => { 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 index 04ecedc6d8..30af58a15d 100644 --- a/src/components/forms/InheritedDeviceRow.tsx +++ b/src/components/forms/InheritedDeviceRow.tsx @@ -83,46 +83,47 @@ export const getInheritedDeviceRow = ({ )}
), - override: - readOnly || disabledReason ? ( - overrideValue ? ( -
- {overrideValue} -
- ) : ( - "" - ) - ) : overrideValue ? ( -
-
{overrideForm}
- {clearOverride && ( -
- -
- )} + override: readOnly ? ( + overrideValue ? ( +
+ {overrideValue}
) : ( - addOverride && ( - - ) - ), + "" + ) + ) : overrideValue ? ( +
+
{overrideForm}
+ {clearOverride && ( +
+ +
+ )} +
+ ) : ( + addOverride && ( + + ) + ), }); }; diff --git a/src/components/forms/NetworkDevicesForm.tsx b/src/components/forms/NetworkDevicesForm.tsx index aac17310be..b190aec763 100644 --- 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} > @@ -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} > Attach network diff --git a/src/components/forms/OtherDeviceForm.tsx b/src/components/forms/OtherDeviceForm.tsx index 53516061a7..e6a658f8e7 100644 --- a/src/components/forms/OtherDeviceForm.tsx +++ b/src/components/forms/OtherDeviceForm.tsx @@ -142,7 +142,7 @@ const OtherDeviceForm: FC = ({ formik, project }) => { appearance="base" hasIcon dense - title={formik.values.editRestriction || "Detach device"} + title={formik.values.editRestriction ?? "Detach device"} disabled={!!formik.values.editRestriction} > @@ -207,7 +207,7 @@ const OtherDeviceForm: FC = ({ formik, project }) => { appearance="base" hasIcon dense - title={formik.values.editRestriction || "Detach GPU"} + title={formik.values.editRestriction ?? "Detach GPU"} disabled={!!formik.values.editRestriction} > diff --git a/src/components/forms/ProxyDeviceForm.tsx b/src/components/forms/ProxyDeviceForm.tsx index d2426c5f5e..2bf2e15a59 100644 --- a/src/components/forms/ProxyDeviceForm.tsx +++ b/src/components/forms/ProxyDeviceForm.tsx @@ -108,9 +108,7 @@ const ProxyDeviceForm: FC = ({ formik, project }) => { options={options} 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 index 2d357050a0..4a92ec9986 100644 --- 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 ( @@ -455,7 +454,11 @@ const CreateInstance: FC = () => { )} - {instanceProfilesWarning([], profiles, isCreating)} + {section === MAIN_CONFIGURATION && ( = ({ instance }) => { )} - {instanceProfilesWarning(instance.profiles, profiles)} + {(section === slugify(MAIN_CONFIGURATION) || !section) && ( )} diff --git a/src/pages/instances/InstanceConsole.tsx b/src/pages/instances/InstanceConsole.tsx index 7b8e808f1e..d85ef38f96 100644 --- a/src/pages/instances/InstanceConsole.tsx +++ b/src/pages/instances/InstanceConsole.tsx @@ -59,6 +59,14 @@ const InstanceConsole: FC = ({ instance }) => { const { handleStart, isLoading } = useInstanceStart(instance); + if (!canAccessInstanceConsole(instance)) { + return ( + + You do not have permission to access the console for this instance. + + ); + } + return (
{isVm && canAccessInstanceConsole(instance) && ( @@ -147,11 +155,11 @@ const InstanceConsole: FC = ({ instance }) => { showNotRunningInfo={showNotRunningInfo} /> )} - {!canAccessInstanceConsole(instance) && ( + {/* {!canAccessInstanceConsole(instance) && ( You do not have permission to access the console for this instance. - )} + )} */}
); }; diff --git a/src/pages/instances/InstanceOverviewProfiles.tsx b/src/pages/instances/InstanceOverviewProfiles.tsx index a854e6076d..5934b0ef6e 100644 --- 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 = ({ instance, onFailure }) => { } if (!profiles.length) { - return instanceProfilesWarning(instance.profiles, profiles); + return ( + + ); } return ; diff --git a/src/pages/instances/InstanceProfilesWarning.tsx b/src/pages/instances/InstanceProfilesWarning.tsx new file mode 100644 index 0000000000..b7e377e9d1 --- /dev/null +++ b/src/pages/instances/InstanceProfilesWarning.tsx @@ -0,0 +1,44 @@ +import { Notification } from "@canonical/react-components"; +import { FC } from "react"; +import { LxdProfile } from "types/profile"; + +interface Props { + instanceProfiles: string[]; + profiles?: LxdProfile[]; + isCreating?: boolean; +} + +const InstanceProfilesWarning: FC = ({ + instanceProfiles, + profiles, + isCreating, +}) => { + const editContext = !isCreating + ? "This may cause inherited configuration values to be displayed incorrectly." + : ""; + + if (!profiles?.length) { + return ( + + You do not have permission to view profiles in the current project.{" "} + {editContext} + + ); + } + + const profilesSet = new Set(profiles.map((profile) => profile.name)); + for (const instanceProfile of instanceProfiles) { + if (!profilesSet.has(instanceProfile)) { + return ( + + You do not have permission to view some profiles applied to this + instance. {editContext} + + ); + } + } + + return null; +}; + +export default InstanceProfilesWarning; diff --git a/src/pages/instances/actions/InstanceBulkAction.tsx b/src/pages/instances/actions/InstanceBulkAction.tsx index e3a7932fa5..1cf8c0f7f5 100644 --- a/src/pages/instances/actions/InstanceBulkAction.tsx +++ b/src/pages/instances/actions/InstanceBulkAction.tsx @@ -108,15 +108,27 @@ const InstanceBulkAction: FC = ({ } }; + const allRestricted = restrictedInstances.length === instances.length; const getRestrictedInstances = () => { if (restrictedInstances.length === 0) { return null; } + if (allRestricted) { + return ( + + - You do not have permission to {confirmLabel.toLowerCase()} the + selected {pluralize("instance", instances.length)}. +
+ + ); + } + return ( - - No action for {restrictedInstances.length} restricted - instances. + - No action for {restrictedInstances.length}{" "} + {pluralize("instance", restrictedInstances.length)} that you do not have + permission to {confirmLabel.toLowerCase()}.
); @@ -141,6 +153,7 @@ const InstanceBulkAction: FC = ({ onConfirm: onClick, confirmButtonLabel: confirmLabel, confirmButtonAppearance: confirmAppearance, + confirmButtonDisabled: allRestricted, }} shiftClickEnabled showShiftClickHint diff --git a/src/pages/profiles/CreateProfile.tsx b/src/pages/profiles/CreateProfile.tsx index 45e237dd41..83b0b0c793 100644 --- a/src/pages/profiles/CreateProfile.tsx +++ b/src/pages/profiles/CreateProfile.tsx @@ -86,9 +86,7 @@ export type CreateProfileFormValues = ProfileDetailsFormValues & MigrationFormValues & BootFormValues & CloudInitFormValues & - YamlFormValues & { - editRestriction?: string; - }; + YamlFormValues; const CreateProfile: FC = () => { const docBaseLink = useDocs(); diff --git a/src/pages/profiles/EditProfile.tsx b/src/pages/profiles/EditProfile.tsx index c6a5670d80..e85dc72b5d 100644 --- a/src/pages/profiles/EditProfile.tsx +++ b/src/pages/profiles/EditProfile.tsx @@ -79,9 +79,7 @@ export type EditProfileFormValues = ProfileDetailsFormValues & MigrationFormValues & BootFormValues & CloudInitFormValues & - YamlFormValues & { - editRestriction?: string; - }; + YamlFormValues; interface Props { profile: LxdProfile; diff --git a/src/pages/projects/ProjectSelectTable.tsx b/src/pages/projects/ProjectSelectTable.tsx index b8f040cb2b..fefe86e1cd 100644 --- a/src/pages/projects/ProjectSelectTable.tsx +++ b/src/pages/projects/ProjectSelectTable.tsx @@ -43,8 +43,8 @@ const ProjectSelectTable: FC = ({ onSelect, disableProject }) => { return { key: project.name, className: classnames("u-row", { - "u-text--muted": getDisableReason(), - "u-row--disabled": getDisableReason(), + "u-text--muted": !!getDisableReason(), + "u-row--disabled": !!getDisableReason(), }), columns: [ { diff --git a/src/pages/storage/CustomVolumeSelectModal.tsx b/src/pages/storage/CustomVolumeSelectModal.tsx index 7e2f4e254e..6e90fd1061 100644 --- a/src/pages/storage/CustomVolumeSelectModal.tsx +++ b/src/pages/storage/CustomVolumeSelectModal.tsx @@ -203,7 +203,7 @@ const CustomVolumeSelectModal: FC = ({ title={ canCreateStorageVolumes(currentProject) ? "" - : "You do not have permission to create storage volumes for this project" + : "You do not have permission to create storage volumes in this project" } > Create volume diff --git a/src/util/entitlements/instances.tsx b/src/util/entitlements/instances.tsx index 0428c1f188..fa4e4ab1ee 100644 --- a/src/util/entitlements/instances.tsx +++ b/src/util/entitlements/instances.tsx @@ -43,12 +43,12 @@ export const useInstanceEntitlements = () => { ); return { - canUpdateInstanceState, + canAccessInstanceConsole, + canDeleteInstance, canEditInstance, + canExecInstance, canManageInstanceBackups, - canDeleteInstance, canManageInstanceSnapshots, - canExecInstance, - canAccessInstanceConsole, + canUpdateInstanceState, }; }; diff --git a/src/util/instanceEdit.tsx b/src/util/instanceEdit.tsx index d5dcab890a..209d0adfc9 100644 --- a/src/util/instanceEdit.tsx +++ b/src/util/instanceEdit.tsx @@ -15,7 +15,6 @@ import { EditProfileFormValues } from "pages/profiles/EditProfile"; import { migrationPayload } from "components/forms/MigrationForm"; import { ConfigurationRowFormikProps } from "components/ConfigurationRow"; import { bootPayload } from "components/forms/BootForm"; -import { Notification } from "@canonical/react-components"; const getEditValues = ( item: LxdProfile | LxdInstance, @@ -138,36 +137,3 @@ export const ensureEditMode = (formik: ConfigurationRowFormikProps) => { void formik.setFieldValue("readOnly", false); } }; - -export const instanceProfilesWarning = ( - instanceProfiles: string[], - profiles?: LxdProfile[], - isCreating?: boolean, -) => { - const editContext = !isCreating - ? "This may cause inherited configuration values to be displayed incorrectly." - : ""; - - if (!profiles?.length) { - return ( - - You do not have permission to view profiles in the current project.{" "} - {editContext} - - ); - } - - const profilesSet = new Set(profiles.map((profile) => profile.name)); - for (const instanceProfile of instanceProfiles) { - if (!profilesSet.has(instanceProfile)) { - return ( - - You do not have permission to view some profiles applied to this - instance. {editContext} - - ); - } - } - - return null; -}; diff --git a/src/util/proxyDevices.tsx b/src/util/proxyDevices.tsx index 8ff1863fd2..1b07a49e0a 100644 --- a/src/util/proxyDevices.tsx +++ b/src/util/proxyDevices.tsx @@ -68,7 +68,7 @@ export const getProxyAddress = ( (connectionType === "connect" && device.nat === "true") } title={ - formik.values.editRestriction || + formik.values.editRestriction ?? (device.nat ? "This is determined by the listen type when nat mode is enabled" : undefined)