From 9a6f0903744093b1071dded4a5103ed5387733ba Mon Sep 17 00:00:00 2001 From: BChoudhury-ms Date: Wed, 3 Dec 2025 07:43:13 +0530 Subject: [PATCH] Refactor Container Copy Permissions Screen: Group-based Validation and Improved Loading UX (#2269) * grouped permissions and added styles * Adding loading overlay for the permission sections --- src/Common/LoadingOverlay.tsx | 31 ++++ .../ContainerCopy/ContainerCopyMessages.ts | 9 ++ src/Explorer/ContainerCopy/CopyJobUtils.ts | 10 +- .../AssignPermissions/AssignPermissions.tsx | 87 +++++++++--- .../AssignPermissions/OnlineCopyEnabled.tsx | 4 +- .../AssignPermissions/PointInTimeRestore.tsx | 2 + .../hooks/usePermissionsSection.tsx | 133 ++++++++++++------ .../Screens/Components/PopoverContainer.tsx | 10 +- .../Utils/useCopyJobNavigation.ts | 8 +- .../ContainerCopy/containerCopyStyles.less | 11 +- 10 files changed, 228 insertions(+), 77 deletions(-) create mode 100644 src/Common/LoadingOverlay.tsx diff --git a/src/Common/LoadingOverlay.tsx b/src/Common/LoadingOverlay.tsx new file mode 100644 index 000000000..320576533 --- /dev/null +++ b/src/Common/LoadingOverlay.tsx @@ -0,0 +1,31 @@ +import { Overlay, Spinner, SpinnerSize } from "@fluentui/react"; +import React from "react"; + +interface LoadingOverlayProps { + isLoading: boolean; + label: string; +} + +const LoadingOverlay: React.FC = ({ isLoading, label }) => { + if (!isLoading) { + return null; + } + + return ( + + + + ); +}; + +export default LoadingOverlay; diff --git a/src/Explorer/ContainerCopy/ContainerCopyMessages.ts b/src/Explorer/ContainerCopy/ContainerCopyMessages.ts index 69ed69967..526b6ffab 100644 --- a/src/Explorer/ContainerCopy/ContainerCopyMessages.ts +++ b/src/Explorer/ContainerCopy/ContainerCopyMessages.ts @@ -55,11 +55,20 @@ export default { "To copy data from the source to the destination container, ensure that the managed identity of the destination account has read access to the source account by completing the following steps.", intraAccountOnlineDescription: (accountName: string) => `Follow the steps below to enable online copy on your "${accountName}" account.`, + commonConfiguration: { + title: "Common configuration", + description: "Basic permissions required for copy operations", + }, + onlineConfiguration: { + title: "Online copy configuration", + description: "Additional permissions required for online copy operations", + }, }, toggleBtn: { onText: "On", offText: "Off", }, + popoverOverlaySpinnerLabel: "Please wait while we process your request...", addManagedIdentity: { title: "System-assigned managed identity enabled.", description: diff --git a/src/Explorer/ContainerCopy/CopyJobUtils.ts b/src/Explorer/ContainerCopy/CopyJobUtils.ts index 7f97367db..75cc4acd2 100644 --- a/src/Explorer/ContainerCopy/CopyJobUtils.ts +++ b/src/Explorer/ContainerCopy/CopyJobUtils.ts @@ -1,5 +1,5 @@ import { DatabaseAccount } from "Contracts/DataModels"; -import { CopyJobErrorType, CopyJobType } from "./Types/CopyJobTypes"; +import { CopyJobContextState, CopyJobErrorType, CopyJobType } from "./Types/CopyJobTypes"; const azurePortalMpacEndpoint = "https://ms.portal.azure.com/"; @@ -115,6 +115,14 @@ export function getAccountDetailsFromResourceId(accountId: string | undefined) { return { subscriptionId, resourceGroup, accountName }; } +export function getContainerIdentifiers(container: CopyJobContextState["source"] | CopyJobContextState["target"]) { + return { + accountId: container?.account?.id || "", + databaseId: container?.databaseId || "", + containerId: container?.containerId || "", + }; +} + export function isIntraAccountCopy(sourceAccountId: string | undefined, targetAccountId: string | undefined): boolean { const sourceAccountDetails = getAccountDetailsFromResourceId(sourceAccountId); const targetAccountDetails = getAccountDetailsFromResourceId(targetAccountId); diff --git a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/AssignPermissions.tsx b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/AssignPermissions.tsx index 1f3861753..7b1f96241 100644 --- a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/AssignPermissions.tsx +++ b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/AssignPermissions.tsx @@ -8,7 +8,8 @@ import ContainerCopyMessages from "../../../ContainerCopyMessages"; import { useCopyJobContext } from "../../../Context/CopyJobContext"; import { isIntraAccountCopy } from "../../../CopyJobUtils"; import { CopyJobMigrationType } from "../../../Enums/CopyJobEnums"; -import usePermissionSections, { PermissionSectionConfig } from "./hooks/usePermissionsSection"; +import { useCopyJobPrerequisitesCache } from "../../Utils/useCopyJobPrerequisitesCache"; +import usePermissionSections, { PermissionGroupConfig, PermissionSectionConfig } from "./hooks/usePermissionsSection"; const PermissionSection: React.FC = ({ id, title, Component, completed, disabled }) => ( @@ -30,43 +31,91 @@ const PermissionSection: React.FC = ({ id, title, Compo ); -const AssignPermissions = () => { - const { copyJobState } = useCopyJobContext(); - const permissionSections = usePermissionSections(copyJobState); +const PermissionGroup: React.FC = ({ title, description, sections }) => { const [openItems, setOpenItems] = React.useState([]); + useEffect(() => { + const firstIncompleteSection = sections.find((section) => !section.completed); + const nextOpenItems = firstIncompleteSection ? [firstIncompleteSection.id] : []; + if (JSON.stringify(openItems) !== JSON.stringify(nextOpenItems)) { + setOpenItems(nextOpenItems); + } + }, [sections]); + + return ( + + + + {title} + + {description && ( + + {description} + + )} + + + + {sections.map((section) => ( + + ))} + + + ); +}; + +const AssignPermissions = () => { + const { setValidationCache } = useCopyJobPrerequisitesCache(); + const { copyJobState } = useCopyJobContext(); + const permissionGroups = usePermissionSections(copyJobState); + + const totalSectionsCount = React.useMemo( + () => permissionGroups.reduce((total, group) => total + group.sections.length, 0), + [permissionGroups], + ); + const indentLevels = React.useMemo( () => Array(copyJobState.migrationType === CopyJobMigrationType.Online ? 5 : 3).fill({ level: 0, width: "100%" }), - [], + [copyJobState.migrationType], ); const isSameAccount = isIntraAccountCopy(copyJobState?.source?.account?.id, copyJobState?.target?.account?.id); useEffect(() => { - const firstIncompleteSection = permissionSections.find((section) => !section.completed); - const nextOpenItems = firstIncompleteSection ? [firstIncompleteSection.id] : []; - if (JSON.stringify(openItems) !== JSON.stringify(nextOpenItems)) { - setOpenItems(nextOpenItems); - } - }, [permissionSections]); + return () => { + setValidationCache(new Map()); + }; + }, []); return ( - - + + {isSameAccount && copyJobState.migrationType === CopyJobMigrationType.Online ? ContainerCopyMessages.assignPermissions.intraAccountOnlineDescription( copyJobState?.source?.account?.name || "", ) : ContainerCopyMessages.assignPermissions.crossAccountDescription} - - {permissionSections?.length === 0 ? ( + + + {totalSectionsCount === 0 ? ( ) : ( - - {permissionSections.map((section) => ( - + + {permissionGroups.map((group) => ( + ))} - + )} ); diff --git a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/OnlineCopyEnabled.tsx b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/OnlineCopyEnabled.tsx index 9d9279e1a..1c1d6bfd5 100644 --- a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/OnlineCopyEnabled.tsx +++ b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/OnlineCopyEnabled.tsx @@ -1,8 +1,9 @@ import { Link, PrimaryButton, Stack } from "@fluentui/react"; -import { CapabilityNames } from "Common/Constants"; import { DatabaseAccount } from "Contracts/DataModels"; import React from "react"; import { fetchDatabaseAccount } from "Utils/arm/databaseAccountUtils"; +import { CapabilityNames } from "../../../../../Common/Constants"; +import LoadingOverlay from "../../../../../Common/LoadingOverlay"; import { logError } from "../../../../../Common/Logger"; import { update as updateDatabaseAccount } from "../../../../../Utils/arm/generatedClients/cosmos/databaseAccounts"; import ContainerCopyMessages from "../../../ContainerCopyMessages"; @@ -119,6 +120,7 @@ const OnlineCopyEnabled: React.FC = () => { return ( + {ContainerCopyMessages.onlineCopyEnabled.description(source?.account?.name || "")}  diff --git a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/PointInTimeRestore.tsx b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/PointInTimeRestore.tsx index eb6ed683e..f62331677 100644 --- a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/PointInTimeRestore.tsx +++ b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/PointInTimeRestore.tsx @@ -2,6 +2,7 @@ import { Link, PrimaryButton, Stack, Text } from "@fluentui/react"; import { DatabaseAccount } from "Contracts/DataModels"; import React, { useEffect, useRef, useState } from "react"; import { fetchDatabaseAccount } from "Utils/arm/databaseAccountUtils"; +import LoadingOverlay from "../../../../../Common/LoadingOverlay"; import { logError } from "../../../../../Common/Logger"; import ContainerCopyMessages from "../../../ContainerCopyMessages"; import { useCopyJobContext } from "../../../Context/CopyJobContext"; @@ -109,6 +110,7 @@ const PointInTimeRestore: React.FC = () => { return ( + {ContainerCopyMessages.pointInTimeRestore.description(source.account?.name ?? "")} {tooltipContent && ( diff --git a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/hooks/usePermissionsSection.tsx b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/hooks/usePermissionsSection.tsx index 01687101c..8ee6d8355 100644 --- a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/hooks/usePermissionsSection.tsx +++ b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/AssignPermissions/hooks/usePermissionsSection.tsx @@ -2,7 +2,7 @@ import { useEffect, useMemo, useRef, useState } from "react"; import { CapabilityNames } from "../../../../../../Common/Constants"; import { fetchRoleAssignments, fetchRoleDefinitions, RoleDefinitionType } from "../../../../../../Utils/arm/RbacUtils"; import ContainerCopyMessages from "../../../../ContainerCopyMessages"; -import { getAccountDetailsFromResourceId, isIntraAccountCopy } from "../../../../CopyJobUtils"; +import { getAccountDetailsFromResourceId, getContainerIdentifiers, isIntraAccountCopy } from "../../../../CopyJobUtils"; import { BackupPolicyType, CopyJobMigrationType, @@ -26,6 +26,13 @@ export interface PermissionSectionConfig { validate?: (state: CopyJobContextState) => boolean | Promise; } +export interface PermissionGroupConfig { + id: string; + title: string; + description: string; + sections: PermissionSectionConfig[]; +} + export const SECTION_IDS = { addManagedIdentity: "addManagedIdentity", defaultManagedIdentity: "defaultManagedIdentity", @@ -127,26 +134,81 @@ export function checkTargetHasReaderRoleOnSource(roleDefinitions: RoleDefinition } /** - * Returns the permission sections configuration for the Assign Permissions screen. - * Memoizes derived values for performance and decouples logic for testability. + * Validates sections within a group sequentially. */ -const usePermissionSections = (state: CopyJobContextState): PermissionSectionConfig[] => { - const sourceAccountId = state?.source?.account?.id || ""; - const targetAccountId = state?.target?.account?.id || ""; +const validateSectionsInGroup = async ( + sections: PermissionSectionConfig[], + state: CopyJobContextState, + validationCache: Map, +): Promise => { + const result: PermissionSectionConfig[] = []; + + for (let i = 0; i < sections.length; i++) { + const section = sections[i]; + + if (validationCache.has(section.id) && validationCache.get(section.id) === true) { + result.push({ ...section, completed: true }); + continue; + } + + if (section.validate) { + const isValid = await section.validate(state); + validationCache.set(section.id, isValid); + result.push({ ...section, completed: isValid }); + + if (!isValid) { + // Mark remaining sections in this group as incomplete + for (let j = i + 1; j < sections.length; j++) { + result.push({ ...sections[j], completed: false }); + } + break; + } + } else { + validationCache.set(section.id, false); + result.push({ ...section, completed: false }); + } + } + + return result; +}; + +/** + * Returns the permission groups configuration for the Assign Permissions screen. + * Groups validate independently but sections within each group validate sequentially. + */ +const usePermissionSections = (state: CopyJobContextState): PermissionGroupConfig[] => { + const sourceAccount = getContainerIdentifiers(state.source); + const targetAccount = getContainerIdentifiers(state.target); const { validationCache, setValidationCache } = useCopyJobPrerequisitesCache(); - const [permissionSections, setPermissionSections] = useState(null); + const [permissionGroups, setPermissionGroups] = useState(null); const isValidatingRef = useRef(false); - const sectionToValidate = useMemo(() => { - const isSameAccount = isIntraAccountCopy(sourceAccountId, targetAccountId); + const groupsToValidate = useMemo(() => { + const isSameAccount = isIntraAccountCopy(sourceAccount.accountId, targetAccount.accountId); + const commonSections = isSameAccount ? [] : [...PERMISSION_SECTIONS_CONFIG]; + const groups: PermissionGroupConfig[] = []; - const baseSections = isSameAccount ? [] : [...PERMISSION_SECTIONS_CONFIG]; - if (state.migrationType === CopyJobMigrationType.Online) { - return [...baseSections, ...PERMISSION_SECTIONS_FOR_ONLINE_JOBS]; + if (commonSections.length > 0) { + groups.push({ + id: "commonConfigs", + title: ContainerCopyMessages.assignPermissions.commonConfiguration.title, + description: ContainerCopyMessages.assignPermissions.commonConfiguration.description, + sections: commonSections, + }); } - return baseSections; - }, [sourceAccountId, targetAccountId, state.migrationType]); + + if (state.migrationType === CopyJobMigrationType.Online) { + groups.push({ + id: "onlineConfigs", + title: ContainerCopyMessages.assignPermissions.onlineConfiguration.title, + description: ContainerCopyMessages.assignPermissions.onlineConfiguration.description, + sections: [...PERMISSION_SECTIONS_FOR_ONLINE_JOBS], + }); + } + + return groups; + }, [sourceAccount.accountId, targetAccount.accountId, state.migrationType]); const memoizedValidationCache = useMemo(() => { if (state.migrationType === CopyJobMigrationType.Offline) { @@ -157,52 +219,39 @@ const usePermissionSections = (state: CopyJobContextState): PermissionSectionCon }, [state.migrationType]); useEffect(() => { - const validateSections = async () => { + const validateGroups = async () => { if (isValidatingRef.current) { return; } isValidatingRef.current = true; - const result: PermissionSectionConfig[] = []; const newValidationCache = new Map(memoizedValidationCache); - for (let i = 0; i < sectionToValidate.length; i++) { - const section = sectionToValidate[i]; + // Validate all groups independently (in parallel) + const validatedGroups = await Promise.all( + groupsToValidate.map(async (group) => { + const validatedSections = await validateSectionsInGroup(group.sections, state, newValidationCache); - if (newValidationCache.has(section.id) && newValidationCache.get(section.id) === true) { - result.push({ ...section, completed: true }); - continue; - } - if (section.validate) { - const isValid = await section.validate(state); - newValidationCache.set(section.id, isValid); - result.push({ ...section, completed: isValid }); - - if (!isValid) { - for (let j = i + 1; j < sectionToValidate.length; j++) { - result.push({ ...sectionToValidate[j], completed: false }); - } - break; - } - } else { - newValidationCache.set(section.id, false); - result.push({ ...section, completed: false }); - } - } + return { + ...group, + sections: validatedSections, + }; + }), + ); setValidationCache(newValidationCache); - setPermissionSections(result); + setPermissionGroups(validatedGroups); isValidatingRef.current = false; }; - validateSections(); + validateGroups(); return () => { isValidatingRef.current = false; }; - }, [state, sectionToValidate]); + }, [state, groupsToValidate]); - return permissionSections ?? []; + return permissionGroups ?? []; }; export default usePermissionSections; diff --git a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/Components/PopoverContainer.tsx b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/Components/PopoverContainer.tsx index eec4f5402..5a76d66eb 100644 --- a/src/Explorer/ContainerCopy/CreateCopyJob/Screens/Components/PopoverContainer.tsx +++ b/src/Explorer/ContainerCopy/CreateCopyJob/Screens/Components/PopoverContainer.tsx @@ -2,6 +2,8 @@ /* eslint-disable react/display-name */ import { DefaultButton, PrimaryButton, Stack, Text } from "@fluentui/react"; import React from "react"; +import LoadingOverlay from "../../../../../Common/LoadingOverlay"; +import ContainerCopyMessages from "../../../ContainerCopyMessages"; interface PopoverContainerProps { isLoading?: boolean; @@ -19,17 +21,13 @@ const PopoverContainer: React.FC = React.memo( tokens={{ childrenGap: 20 }} style={{ maxWidth: 450 }} > + {title} {children} - + diff --git a/src/Explorer/ContainerCopy/CreateCopyJob/Utils/useCopyJobNavigation.ts b/src/Explorer/ContainerCopy/CreateCopyJob/Utils/useCopyJobNavigation.ts index b7b543909..dd8059547 100644 --- a/src/Explorer/ContainerCopy/CreateCopyJob/Utils/useCopyJobNavigation.ts +++ b/src/Explorer/ContainerCopy/CreateCopyJob/Utils/useCopyJobNavigation.ts @@ -2,7 +2,7 @@ import { useCallback, useMemo, useReducer, useState } from "react"; import { useSidePanel } from "../../../../hooks/useSidePanel"; import { submitCreateCopyJob } from "../../Actions/CopyJobActions"; import { useCopyJobContext } from "../../Context/CopyJobContext"; -import { isIntraAccountCopy } from "../../CopyJobUtils"; +import { getContainerIdentifiers, isIntraAccountCopy } from "../../CopyJobUtils"; import { CopyJobMigrationType } from "../../Enums/CopyJobEnums"; import { useCopyJobPrerequisitesCache } from "./useCopyJobPrerequisitesCache"; import { SCREEN_KEYS, useCreateCopyJobScreensList } from "./useCreateCopyJobScreensList"; @@ -71,12 +71,6 @@ export function useCopyJobNavigation() { useSidePanel.getState().closeSidePanel(); }, []); - const getContainerIdentifiers = (container: typeof copyJobState.source | typeof copyJobState.target) => ({ - accountId: container?.account?.id || "", - databaseId: container?.databaseId || "", - containerId: container?.containerId || "", - }); - const areContainersIdentical = () => { const { source, target } = copyJobState; const sourceIds = getContainerIdentifiers(source); diff --git a/src/Explorer/ContainerCopy/containerCopyStyles.less b/src/Explorer/ContainerCopy/containerCopyStyles.less index e7901efbc..05d9facec 100644 --- a/src/Explorer/ContainerCopy/containerCopyStyles.less +++ b/src/Explorer/ContainerCopy/containerCopyStyles.less @@ -19,6 +19,10 @@ .createCopyJobScreensContainer { height: 100%; padding: 1em 1.5em; + + .pointInTimeRestoreContainer, .onlineCopyContainer { + position: relative; + } label { padding: 0; @@ -59,6 +63,7 @@ } } .popover-container { + border-radius: 6px; button[disabled] { cursor: not-allowed; opacity: 0.8; @@ -66,7 +71,7 @@ } .foreground { z-index: 10; - background-color: white; + background-color: #f9f9f9; padding: 20px; box-shadow: 0 4px 8px rgba(0, 0, 0, 0.2); transform: translate(0%, -9%); @@ -89,6 +94,10 @@ .panelFormWrapper .panelMainContent { padding: 0; } + + .createCopyJobScreensFooter { + margin-top: 50px; + } } .monitorCopyJobs {