From 57865b6a901171fe2a7195e980c126250abf0b74 Mon Sep 17 00:00:00 2001 From: Yunwen Zheng Date: Tue, 12 Aug 2025 09:45:08 -0400 Subject: [PATCH] Browse Dashboards: Prevent cross repo resource selection (#109500) * BrowseActions: when selecting item from a provisioned folder, disable all items from other provisioned folder. Display a tooltip when checkbox is disabled. --- .../useSelectionProvisioningStatus.ts | 44 ++++++++++++------- .../useSelectionRepoValidation.ts | 35 +++++++++++++++ .../components/BrowseActions/utils.ts | 41 +++++++++++++++++ .../BulkDeleteProvisionedResource.test.tsx | 16 +++++++ .../BulkDeleteProvisionedResource.tsx | 12 +++-- .../BulkMoveProvisionedResource.tsx | 13 ++++-- .../components/CheckboxCell.tsx | 30 ++++++++++++- .../browse-dashboards/components/utils.ts | 42 +++++++++++++++++- .../browse-dashboards/state/reducers.ts | 11 +++++ public/locales/en-US/grafana.json | 3 ++ 10 files changed, 222 insertions(+), 25 deletions(-) create mode 100644 public/app/features/browse-dashboards/components/BrowseActions/useSelectionRepoValidation.ts diff --git a/public/app/features/browse-dashboards/components/BrowseActions/useSelectionProvisioningStatus.ts b/public/app/features/browse-dashboards/components/BrowseActions/useSelectionProvisioningStatus.ts index cc4724b9ded..891dd1cecd1 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/useSelectionProvisioningStatus.ts +++ b/public/app/features/browse-dashboards/components/BrowseActions/useSelectionProvisioningStatus.ts @@ -12,6 +12,10 @@ import { useSelector } from 'app/types/store'; import { findItem } from '../../state/utils'; import { DashboardTreeSelection } from '../../types'; +// TODO: This will soon be remove after bulk action is merged in + +// This hook can be remove once searching endpoint returns provisioning status +// It is used to determine if the selected items are provisioned or not, which is currently missing from the search API export function useSelectionProvisioningStatus( selectedItems: Omit, isParentProvisioned: boolean @@ -23,7 +27,6 @@ export function useSelectionProvisioningStatus( const provisioningEnabled = config.featureToggles.provisioning; const [status, setStatus] = useState({ hasProvisioned: false, hasNonProvisioned: false }); - const [folderCache, setFolderCache] = useState>({}); const [dashboardCache, setDashboardCache] = useState>({}); @@ -38,6 +41,8 @@ export function useSelectionProvisioningStatus( [] ); + // Simplified: removed complex root folder tracking logic + const findItemInState = useCallback( (uid: string) => { const item = findItem(browseState.rootItems?.items || [], browseState.childrenByParentUID, uid); @@ -84,7 +89,6 @@ export function useSelectionProvisioningStatus( const checkItemProvisioning = useCallback( async (uid: string, isFolder: boolean): Promise => { if (isSearching) { - // If searching, we need provisioning status with fetching metadata return isFolder ? await getFolderMeta(uid) : await getDashboardMeta(uid); } @@ -93,7 +97,7 @@ export function useSelectionProvisioningStatus( return item?.managedBy === ManagerKind.Repo; } - // Check parent folder first + // Check parent folder first for dashboards const parent = item?.parentUID ? findItemInState(item.parentUID) : undefined; if (parent?.managedBy === ManagerKind.Repo) { return true; @@ -106,7 +110,7 @@ export function useSelectionProvisioningStatus( useEffect(() => { const checkProvisioningStatus = async () => { - // If the instance is provisioned or the parent folder is provisioned, we can skip checking individual items + // Early returns for simple cases if (isProvisionedInstance || isParentProvisioned) { setStatus({ hasProvisioned: true, hasNonProvisioned: false }); return; @@ -120,6 +124,12 @@ export function useSelectionProvisioningStatus( const folders = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]); const dashboards = Object.keys(selectedItems.dashboard).filter((uid) => selectedItems.dashboard[uid]); + // If no items selected + if (folders.length === 0 && dashboards.length === 0) { + setStatus({ hasProvisioned: false, hasNonProvisioned: false }); + return; + } + let hasProvisioned = false; let hasNonProvisioned = false; @@ -127,11 +137,18 @@ export function useSelectionProvisioningStatus( ...folders.map((uid) => ({ uid, isFolder: true })), ...dashboards.map((uid) => ({ uid, isFolder: false })), ]; + for (const { uid, isFolder } of allItems) { const isProvisioned = await checkItemProvisioning(uid, isFolder); - isProvisioned ? (hasProvisioned = true) : (hasNonProvisioned = true); + + if (isProvisioned) { + hasProvisioned = true; + } else { + hasNonProvisioned = true; + } + if (hasProvisioned && hasNonProvisioned) { - // If we have both provisioned and non-provisioned items, we can stop checking + // If we have both, we can stop checking break; } } @@ -140,15 +157,10 @@ export function useSelectionProvisioningStatus( }; checkProvisioningStatus(); - }, [ - selectedItems, - isProvisionedInstance, - isParentProvisioned, - isSearching, - findItemInState, - checkItemProvisioning, - provisioningEnabled, - ]); + }, [selectedItems, isProvisionedInstance, isParentProvisioned, checkItemProvisioning, provisioningEnabled]); - return status; + return { + hasProvisioned: status.hasProvisioned, + hasNonProvisioned: status.hasNonProvisioned, + }; } diff --git a/public/app/features/browse-dashboards/components/BrowseActions/useSelectionRepoValidation.ts b/public/app/features/browse-dashboards/components/BrowseActions/useSelectionRepoValidation.ts new file mode 100644 index 00000000000..30ee0957c17 --- /dev/null +++ b/public/app/features/browse-dashboards/components/BrowseActions/useSelectionRepoValidation.ts @@ -0,0 +1,35 @@ +import { useSelector } from 'app/types/store'; + +import { useChildrenByParentUIDState, rootItemsSelector } from '../../state/hooks'; +import { findItem } from '../../state/utils'; +import { DashboardTreeSelection } from '../../types'; +import { getItemRepositoryUid } from '../utils'; + +// This hook is responsible for validating if all selected resources (dashboard folders and dashboards) are in the same repository +export function useSelectionRepoValidation(selectedItems: Omit) { + const childrenByParentUID = useChildrenByParentUIDState(); + const rootItems = useSelector(rootItemsSelector)?.items ?? []; + + const getRepoUid = (uid: string) => { + const item = findItem(rootItems, childrenByParentUID, uid); + return item ? getItemRepositoryUid(item, rootItems, childrenByParentUID) : 'non_provisioned'; + }; + + const selectedUIDs = [ + ...Object.keys(selectedItems.folder || {}).filter((id) => selectedItems.folder[id]), + ...Object.keys(selectedItems.dashboard || {}).filter((id) => selectedItems.dashboard[id]), + ]; + + const repoUIDs = selectedUIDs.map(getRepoUid).filter((repoId): repoId is string => !!repoId); + + const selectedItemsRepoUID = repoUIDs.length > 0 ? repoUIDs[0] : undefined; + const isCrossRepo = new Set(repoUIDs).size > 1; + + const isInLockedRepo = (uid: string) => !selectedItemsRepoUID || getRepoUid(uid) === selectedItemsRepoUID; + + return { + selectedItemsRepoUID, + isInLockedRepo, + isCrossRepo, // true if items are from different repositories + }; +} diff --git a/public/app/features/browse-dashboards/components/BrowseActions/utils.ts b/public/app/features/browse-dashboards/components/BrowseActions/utils.ts index 2d6b4328c53..59a3979cf2f 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/utils.ts +++ b/public/app/features/browse-dashboards/components/BrowseActions/utils.ts @@ -1,4 +1,8 @@ import { t } from '@grafana/i18n'; +import { DashboardViewItem } from 'app/features/search/types'; + +import { findItem } from '../../state/utils'; +import { DashboardViewItemCollection } from '../../types'; export function buildBreakdownString( folderCount: number, @@ -26,3 +30,40 @@ export function buildBreakdownString( } return breakdownString; } + +// Utility: Get root folder for any item (reusing existing pattern from reducers.ts) +export function getItemRootFolder( + item: { uid: string; parentUID?: string; kind?: string }, + browseState: { + rootItems?: { items: DashboardViewItem[] }; + childrenByParentUID: Record; + } +): string | undefined { + const rootItems = browseState.rootItems?.items || []; + + // If it's already a root-level item, return its UID (only for folders) + if (!item.parentUID) { + return item.kind === 'folder' ? item.uid : undefined; + } + + // For nested items, traverse up to find root folder (same pattern as reducers.ts) + let nextParentUID = item.parentUID; + + while (nextParentUID) { + const parent = findItem(rootItems, browseState.childrenByParentUID, nextParentUID); + + // Safety check to prevent infinite loops (same as reducers.ts) + if (!parent) { + break; + } + + // Found the root folder (no parent) + if (!parent.parentUID) { + return parent.uid; + } + + nextParentUID = parent.parentUID; + } + + return undefined; +} diff --git a/public/app/features/browse-dashboards/components/BulkActions/BulkDeleteProvisionedResource.test.tsx b/public/app/features/browse-dashboards/components/BulkActions/BulkDeleteProvisionedResource.test.tsx index 34b61e59041..65b6686ef14 100644 --- a/public/app/features/browse-dashboards/components/BulkActions/BulkDeleteProvisionedResource.test.tsx +++ b/public/app/features/browse-dashboards/components/BulkActions/BulkDeleteProvisionedResource.test.tsx @@ -3,6 +3,8 @@ import { render } from 'test/test-utils'; import { Job, RepositoryView } from 'app/api/clients/provisioning/v0alpha1'; +import { useSelectionRepoValidation } from '../BrowseActions/useSelectionRepoValidation'; + import { BulkDeleteProvisionedResource } from './BulkDeleteProvisionedResource'; import { ResponseType } from './useBulkActionJob'; @@ -19,6 +21,14 @@ jest.mock('app/features/provisioning/hooks/useGetResourceRepositoryView', () => useGetResourceRepositoryView: jest.fn(), })); +jest.mock('../BrowseActions/useSelectionRepoValidation', () => ({ + useSelectionRepoValidation: jest.fn(), +})); + +const mockUseSelectionRepoValidation = useSelectionRepoValidation as jest.MockedFunction< + typeof useSelectionRepoValidation +>; + jest.mock('./useBulkActionJob', () => ({ useBulkActionJob: jest.fn(), })); @@ -103,6 +113,12 @@ function setup( describe('BulkDeleteProvisionedResource', () => { beforeEach(() => { jest.clearAllMocks(); + + mockUseSelectionRepoValidation.mockReturnValue({ + selectedItemsRepoUID: 'test-folder', + isInLockedRepo: jest.fn().mockReturnValue(false), + isCrossRepo: false, + }); }); afterEach(() => { diff --git a/public/app/features/browse-dashboards/components/BulkActions/BulkDeleteProvisionedResource.tsx b/public/app/features/browse-dashboards/components/BulkActions/BulkDeleteProvisionedResource.tsx index 786b31d6c8a..16970ee282d 100644 --- a/public/app/features/browse-dashboards/components/BulkActions/BulkDeleteProvisionedResource.tsx +++ b/public/app/features/browse-dashboards/components/BulkActions/BulkDeleteProvisionedResource.tsx @@ -11,13 +11,14 @@ import { getDefaultWorkflow, getWorkflowOptions } from 'app/features/dashboard-s import { generateTimestamp } from 'app/features/dashboard-scene/saving/provisioned/utils/timestamp'; import { JobStatus } from 'app/features/provisioning/Job/JobStatus'; import { useGetResourceRepositoryView } from 'app/features/provisioning/hooks/useGetResourceRepositoryView'; +import { GENERAL_FOLDER_UID } from 'app/features/search/constants'; import { DescendantCount } from '../BrowseActions/DescendantCount'; +import { useSelectionRepoValidation } from '../BrowseActions/useSelectionRepoValidation'; import { collectSelectedItems } from '../utils'; import { RepoInvalidStateBanner } from './RepoInvalidStateBanner'; import { DeleteJobSpec, useBulkActionJob } from './useBulkActionJob'; -import { useFolderNameFromSelection } from './useFolderNameFromSelection'; import { BulkActionFormData, BulkActionProvisionResourceProps } from './utils'; interface FormProps extends BulkActionProvisionResourceProps { @@ -117,9 +118,14 @@ export function BulkDeleteProvisionedResource({ selectedItems, onDismiss, }: BulkActionProvisionResourceProps) { - const folderName = useFolderNameFromSelection({ folderUid, selectedItems }); - const { repository, isReadOnlyRepo } = useGetResourceRepositoryView({ folderName }); + // Check if we're on the root browser dashboards page + const isRootPage = !folderUid || folderUid === GENERAL_FOLDER_UID; + const { selectedItemsRepoUID } = useSelectionRepoValidation(selectedItems); + // For root provisioned folders, the folder UID is the repository name + const { repository, isReadOnlyRepo } = useGetResourceRepositoryView({ + folderName: isRootPage ? selectedItemsRepoUID : folderUid, + }); const workflowOptions = getWorkflowOptions(repository); const timestamp = generateTimestamp(); diff --git a/public/app/features/browse-dashboards/components/BulkActions/BulkMoveProvisionedResource.tsx b/public/app/features/browse-dashboards/components/BulkActions/BulkMoveProvisionedResource.tsx index 22640003c20..f20d41d650d 100644 --- a/public/app/features/browse-dashboards/components/BulkActions/BulkMoveProvisionedResource.tsx +++ b/public/app/features/browse-dashboards/components/BulkActions/BulkMoveProvisionedResource.tsx @@ -14,13 +14,14 @@ import { getDefaultWorkflow, getWorkflowOptions } from 'app/features/dashboard-s import { generateTimestamp } from 'app/features/dashboard-scene/saving/provisioned/utils/timestamp'; import { JobStatus } from 'app/features/provisioning/Job/JobStatus'; import { useGetResourceRepositoryView } from 'app/features/provisioning/hooks/useGetResourceRepositoryView'; +import { GENERAL_FOLDER_UID } from 'app/features/search/constants'; import { DescendantCount } from '../BrowseActions/DescendantCount'; +import { useSelectionRepoValidation } from '../BrowseActions/useSelectionRepoValidation'; import { collectSelectedItems } from '../utils'; import { RepoInvalidStateBanner } from './RepoInvalidStateBanner'; import { MoveJobSpec, useBulkActionJob } from './useBulkActionJob'; -import { useFolderNameFromSelection } from './useFolderNameFromSelection'; import { BulkActionFormData, BulkActionProvisionResourceProps, getTargetFolderPathInRepo } from './utils'; interface FormProps extends BulkActionProvisionResourceProps { @@ -148,8 +149,12 @@ function FormContent({ initialValues, selectedItems, repository, workflowOptions } export function BulkMoveProvisionedResource({ folderUid, selectedItems, onDismiss }: BulkActionProvisionResourceProps) { - const folderName = useFolderNameFromSelection({ folderUid, selectedItems }); - const { repository, folder, isReadOnlyRepo } = useGetResourceRepositoryView({ folderName }); + // Check if we're on the root browser dashboards page + const isRootPage = !folderUid || folderUid === GENERAL_FOLDER_UID; + const { selectedItemsRepoUID } = useSelectionRepoValidation(selectedItems); + const { repository, folder, isReadOnlyRepo } = useGetResourceRepositoryView({ + folderName: isRootPage ? selectedItemsRepoUID : folderUid, + }); const workflowOptions = getWorkflowOptions(repository); const folderPath = folder?.metadata?.annotations?.[AnnoKeySourcePath] || ''; @@ -172,7 +177,7 @@ export function BulkMoveProvisionedResource({ folderUid, selectedItems, onDismis initialValues={initialValues} repository={repository} workflowOptions={workflowOptions} - folderPath={folderPath} + folderPath={isRootPage ? '/' : folderPath} /> ); } diff --git a/public/app/features/browse-dashboards/components/CheckboxCell.tsx b/public/app/features/browse-dashboards/components/CheckboxCell.tsx index 60995129f90..7867510d940 100644 --- a/public/app/features/browse-dashboards/components/CheckboxCell.tsx +++ b/public/app/features/browse-dashboards/components/CheckboxCell.tsx @@ -3,10 +3,13 @@ import { css } from '@emotion/css'; import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { t } from '@grafana/i18n'; -import { Checkbox, useStyles2 } from '@grafana/ui'; +import { Checkbox, Tooltip, useStyles2 } from '@grafana/ui'; +import { ManagerKind } from 'app/features/apiserver/types'; +import { useSelector } from 'app/types/store'; import { DashboardsTreeCellProps, SelectionState } from '../types'; +import { useSelectionRepoValidation } from './BrowseActions/useSelectionRepoValidation'; import { isSharedWithMe, canEditItemType } from './utils'; export default function CheckboxCell({ @@ -17,6 +20,11 @@ export default function CheckboxCell({ }: DashboardsTreeCellProps) { const item = row.item; + // Get current selection state for repository validation + const selectedItems = useSelector((state) => state.browseDashboards.selectedItems); + const { selectedItemsRepoUID, isInLockedRepo } = useSelectionRepoValidation(selectedItems); + + // Early returns for cases where we should show a spacer instead of checkbox if (!isSelected) { return ; } @@ -33,11 +41,31 @@ export default function CheckboxCell({ return ; } + // Disable checkbox for root provisioned folder itself + if (item.managedBy === ManagerKind.Repo && !item.parentUID) { + return ; + } + // Check if user can edit this specific item type if (permissions && !canEditItemType(item.kind, permissions)) { return ; } + if (selectedItemsRepoUID && !isInLockedRepo(item.uid)) { + return ( + + + + + + ); + } + const state = isSelected(item); return ( diff --git a/public/app/features/browse-dashboards/components/utils.ts b/public/app/features/browse-dashboards/components/utils.ts index 3443967fc62..ef72de4bbd7 100644 --- a/public/app/features/browse-dashboards/components/utils.ts +++ b/public/app/features/browse-dashboards/components/utils.ts @@ -1,7 +1,15 @@ import { config } from '@grafana/runtime'; import { contextSrv } from 'app/core/core'; +import { ManagerKind } from 'app/features/apiserver/types'; +import { DashboardViewItem } from 'app/features/search/types'; -import { DashboardTreeSelection, DashboardViewItemWithUIItems, BrowseDashboardsPermissions } from '../types'; +import { findItem } from '../state/utils'; +import { + DashboardTreeSelection, + DashboardViewItemWithUIItems, + BrowseDashboardsPermissions, + BrowseDashboardsState, +} from '../types'; import { ResourceRef } from './BulkActions/useBulkActionJob'; @@ -96,3 +104,35 @@ export function canSelectItems(permissions: BrowseDashboardsPermissions) { const canSelectDashboards = canEditDashboards || canDeleteDashboards; return Boolean(canSelectFolders || canSelectDashboards); } + +/** + * Finds the repository name for an item by traversing up the tree to find the root provisioned folder (managed by ManagerKind.Repo) + * This should be an edge case where user have multiple provisioned folders and try to managing resources on root folder + */ +export function getItemRepositoryUid( + item: DashboardViewItem, + rootItems: DashboardViewItem[], + childrenByParentUID: BrowseDashboardsState['childrenByParentUID'] +): string { + // For root provisioned folders, the UID is the repository name + if (item.managedBy === ManagerKind.Repo && !item.parentUID && item.kind === 'folder') { + return item.uid; + } + + // Traverse up the tree to find the root provisioned folder + let currentItem = item; + while (currentItem.parentUID) { + const parent = findItem(rootItems, childrenByParentUID, currentItem.parentUID); + if (!parent) { + break; + } + + if (parent.managedBy === ManagerKind.Repo && !parent.parentUID) { + return currentItem.parentUID; + } + + currentItem = parent; + } + + return 'non_provisioned'; +} diff --git a/public/app/features/browse-dashboards/state/reducers.ts b/public/app/features/browse-dashboards/state/reducers.ts index df1505f50d7..7f0004fd242 100644 --- a/public/app/features/browse-dashboards/state/reducers.ts +++ b/public/app/features/browse-dashboards/state/reducers.ts @@ -1,5 +1,6 @@ import { PayloadAction } from '@reduxjs/toolkit'; +import { ManagerKind } from 'app/features/apiserver/types'; import { DashboardViewItem, DashboardViewItemKind } from 'app/features/search/types'; import { GENERAL_FOLDER_UID } from '../../search/constants'; @@ -96,6 +97,11 @@ export function setItemSelectionState( return; } + // Prevent selection of root provisioned folders + if (item.managedBy === ManagerKind.Repo && !item.parentUID) { + return; + } + // Selecting a folder selects all children, and unselecting a folder deselects all children // so propagate the new selection state to all descendants function markChildren(kind: DashboardViewItemKind, uid: string) { @@ -178,6 +184,11 @@ export function setAllSelection( continue; } + // Skip all provisioned resources during "select all" on root level + if (child.managedBy === ManagerKind.Repo && !child.parentUID) { + continue; + } + state.selectedItems[child.kind][child.uid] = isSelected; if (child.kind !== 'folder') { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index d5bf489e26b..dab62229722 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -3556,6 +3556,9 @@ "total_other": "{{count}} item" }, "dashboards-tree": { + "checkbox": { + "disabled-not-in-same-repo": "This item is not in the same repository as the selected items." + }, "collapse-folder-button": "Collapse folder {{title}}", "expand-folder-button": "Expand folder {{title}}", "name-column": "Name",