MoveProvisionedDashboard: bug fix when selecting root folder, error is showing (#109638)

* MoveProvisionedDashboard: root folder error fix * Remove provisioned badge when whole instance is provisioned, root folder checkbox fix * If root level only have one provisioned folder, allow select all on subitems * remove button tooltip and remove comment * regenerate API clients after schema updates --------- Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>
2025-08-20 11:50:39 -04:00
parent 03bcd604fc
commit 170c84c3f8
10 changed files with 217 additions and 46 deletions
@@ -1,6 +1,7 @@
 import { t } from '@grafana/i18n';
 import { Badge, Stack } from '@grafana/ui';
 import { useGetResourceRepositoryView } from 'app/features/provisioning/hooks/useGetResourceRepositoryView';
+import { useIsProvisionedInstance } from 'app/features/provisioning/hooks/useIsProvisionedInstance';
 import { getReadOnlyTooltipText } from 'app/features/provisioning/utils/repository';
 import { NestedFolderDTO } from 'app/features/search/service/types';
 import { FolderDTO, FolderListItemDTO } from 'app/types/folders';
@@ -14,7 +15,10 @@ export function FolderRepo({ folder }: Props) {
  // folder is not present
  // folder have parentUID
  // folder is not managed
-  const skipRender = !folder || ('parentUID' in folder && folder.parentUID) || !folder.managedBy;
+  // if whole instance is provisioned
+  const isProvisionedInstance = useIsProvisionedInstance();
+  const skipRender =
+    !folder || ('parentUID' in folder && folder.parentUID) || !folder.managedBy || isProvisionedInstance;

  const { isReadOnlyRepo, repoType } = useGetResourceRepositoryView({
    folderName: skipRender ? undefined : folder?.uid,
@@ -2,6 +2,7 @@ import { skipToken } from '@reduxjs/toolkit/query';

 import { config } from '@grafana/runtime';
 import { useGetFrontendSettingsQuery } from 'app/api/clients/provisioning/v0alpha1';
+import { useIsProvisionedInstance } from 'app/features/provisioning/hooks/useIsProvisionedInstance';
 import { getIsReadOnlyRepo } from 'app/features/provisioning/utils/repository';
 import { useSelector } from 'app/types/store';

@@ -15,6 +16,7 @@ export function useSelectionRepoValidation(selectedItems: Omit<DashboardTreeSele
  const provisioningEnabled = config.featureToggles.provisioning;
  const childrenByParentUID = useChildrenByParentUIDState();
  const rootItems = useSelector(rootItemsSelector)?.items ?? [];
+  const isProvisionedInstance = useIsProvisionedInstance();

  const { data: settingsData } = useGetFrontendSettingsQuery(!provisioningEnabled ? skipToken : undefined);
  // Function to grab repository configuration by UID
@@ -40,7 +42,13 @@ export function useSelectionRepoValidation(selectedItems: Omit<DashboardTreeSele
  const selectedItemsRepoUID = repoUIDs.length > 0 ? repoUIDs[0] : undefined;
  const isCrossRepo = new Set(repoUIDs).size > 1;

-  const isInLockedRepo = (uid: string) => !selectedItemsRepoUID || getRepoUid(uid) === selectedItemsRepoUID;
+  const isInLockedRepo = (uid: string) => {
+    // if whole instance is provisioned, all items are considered in the locked (same) repo
+    if (isProvisionedInstance) {
+      return true;
+    }
+    return !selectedItemsRepoUID || getRepoUid(uid) === selectedItemsRepoUID;
+  };
  const isUidInReadOnlyRepo = (uid: string) => {
    const repo = getRepositoryByUid(getRepoUid(uid));
    return repo ? getIsReadOnlyRepo(repo) : false;
@@ -1,9 +1,13 @@
-import { useCallback } from 'react';
+import { skipToken } from '@reduxjs/toolkit/query';
+import { useCallback, useMemo } from 'react';

 import { Trans, t } from '@grafana/i18n';
+import { config } from '@grafana/runtime';
 import { CallToActionCard, EmptyState, LinkButton, TextLink } from '@grafana/ui';
+import { useGetFrontendSettingsQuery } from 'app/api/clients/provisioning/v0alpha1';
+import { useIsProvisionedInstance } from 'app/features/provisioning/hooks/useIsProvisionedInstance';
 import { DashboardViewItem } from 'app/features/search/types';
-import { useDispatch } from 'app/types/store';
+import { useDispatch, useSelector } from 'app/types/store';

 import { PAGE_SIZE } from '../api/services';
 import { fetchNextChildrenPage } from '../state/actions';
@@ -13,6 +17,7 @@ import {
  useChildrenByParentUIDState,
  useBrowseLoadingStatus,
  useLoadNextChildrenPage,
+  rootItemsSelector,
 } from '../state/hooks';
 import { setFolderOpenState, setItemSelectionState, setAllSelection } from '../state/slice';
 import { BrowseDashboardsState, DashboardTreeSelection, SelectionState, BrowseDashboardsPermissions } from '../types';
@@ -34,6 +39,27 @@ export function BrowseView({ folderUID, width, height, permissions }: BrowseView
  const selectedItems = useCheckboxSelectionState();
  const childrenByParentUID = useChildrenByParentUIDState();
  const canSelect = canSelectItems(permissions);
+  const isProvisionedInstance = useIsProvisionedInstance();
+  const provisioningEnabled = config.featureToggles.provisioning;
+  const { data: settingsData } = useGetFrontendSettingsQuery(!provisioningEnabled ? skipToken : undefined);
+  const rootItems = useSelector(rootItemsSelector);
+
+  const excludeUIDs = useMemo(() => {
+    if (isProvisionedInstance || !provisioningEnabled) {
+      return [];
+    }
+    if (provisioningEnabled) {
+      // if only one repo folder and no local folders, then don't exclude it from selection
+      if (rootItems?.items.length === 1 && settingsData?.items.length === 1) {
+        return [];
+      }
+      // loop through settingsData to find all available repo name, and exclude them from select all action
+      // repo root folder is not actionable on browse dashboards page
+      return settingsData?.items.map((repo) => repo.name);
+    }
+
+    return [];
+  }, [isProvisionedInstance, settingsData, provisioningEnabled, rootItems]);

  const handleFolderClick = useCallback(
    (clickedFolderUID: string, isOpen: boolean) => {
@@ -164,7 +190,7 @@ export function BrowseView({ folderUID, width, height, permissions }: BrowseView
      height={height}
      isSelected={isSelected}
      onFolderClick={handleFolderClick}
-      onAllSelectionChange={(newState) => dispatch(setAllSelection({ isSelected: newState, folderUID }))}
+      onAllSelectionChange={(newState) => dispatch(setAllSelection({ isSelected: newState, folderUID, excludeUIDs }))}
      onItemSelectionChange={handleItemSelectionChange}
      isItemLoaded={isItemLoaded}
      requestLoadMore={handleLoadMore}
@@ -40,14 +40,24 @@ function FormContent({ initialValues, selectedItems, repository, workflowOptions
  // Hooks
  const { createBulkJob, isLoading: isCreatingJob } = useBulkActionJob();
  const methods = useForm<BulkActionFormData>({ defaultValues: initialValues });
-  const { handleSubmit, watch } = methods;
+  const {
+    handleSubmit,
+    watch,
+    setError,
+    clearErrors,
+    formState: { errors },
+  } = methods;
  const workflow = watch('workflow');

  // Get target folder data
  const { data: targetFolder } = useGetFolderQuery(targetFolderUID ? { name: targetFolderUID } : skipToken);

  const setupMoveOperation = () => {
-    const targetFolderPathInRepo = getTargetFolderPathInRepo({ targetFolder });
+    const targetFolderPathInRepo = getTargetFolderPathInRepo({
+      targetFolderUID,
+      targetFolder,
+      repoName: repository.name,
+    });
    const resources = collectSelectedItems(selectedItems);

    return { targetFolderPathInRepo, resources };
@@ -60,12 +70,15 @@ function FormContent({ initialValues, selectedItems, repository, workflowOptions
    const { targetFolderPathInRepo, resources } = setupMoveOperation();

    if (!targetFolderPathInRepo) {
-      throw new Error(
-        t(
+      setError('targetFolderUID', {
+        type: 'manual',
+        message: t(
          'browse-dashboards.bulk-move-resources-form.error-no-target-folder-path',
-          'Target folder path in repository is invalid, please select another folder.'
-        )
-      );
+          'Target folder path is invalid or empty, please select again.'
+        ),
+      });
+      setHasSubmitted(false);
+      return;
    }

    // Create the move job spec
@@ -73,7 +86,7 @@ function FormContent({ initialValues, selectedItems, repository, workflowOptions
      action: 'move',
      move: {
        ref: data.workflow === 'write' ? undefined : data.ref,
-        targetPath: `${targetFolderPathInRepo}/`,
+        targetPath: targetFolderPathInRepo,
        resources,
      },
    };
@@ -90,7 +103,7 @@ function FormContent({ initialValues, selectedItems, repository, workflowOptions
          result.error,
        ],
      });
-      setHasSubmitted(false); // Reset submit state so user can try again
+      setHasSubmitted(false);
    }
  };

@@ -110,8 +123,19 @@ function FormContent({ initialValues, selectedItems, repository, workflowOptions
          ) : (
            <>
              {/* Target folder selection */}
-              <Field noMargin label={t('browse-dashboards.bulk-move-resources-form.target-folder', 'Target Folder')}>
-                <FolderPicker value={targetFolderUID} onChange={setTargetFolderUID} />
+              <Field
+                noMargin
+                label={t('browse-dashboards.bulk-move-resources-form.target-folder', 'Target Folder')}
+                error={errors.targetFolderUID?.message}
+                invalid={!!errors.targetFolderUID}
+              >
+                <FolderPicker
+                  value={targetFolderUID}
+                  onChange={(uid) => {
+                    setTargetFolderUID(uid || '');
+                    clearErrors('targetFolderUID');
+                  }}
+                />
              </Field>
              <ResourceEditFormSharedFields
                resourceType="folder"
@@ -124,13 +148,8 @@ function FormContent({ initialValues, selectedItems, repository, workflowOptions

              <Stack gap={2}>
                <Button
-                  tooltip={
-                    !targetFolder
-                      ? t('browse-dashboards.bulk-move-resources-form.button-tooltip', 'Please select a target folder')
-                      : undefined
-                  }
                  type="submit"
-                  disabled={!!job || isCreatingJob || hasSubmitted || !targetFolder}
+                  disabled={!!job || isCreatingJob || hasSubmitted || targetFolderUID === undefined}
                >
                  {isCreatingJob
                    ? t('browse-dashboards.bulk-move-resources-form.button-moving', 'Moving...')
@@ -0,0 +1,59 @@
+import { AnnoKeySourcePath } from 'app/features/apiserver/types';
+
+import { getTargetFolderPathInRepo, getNestedFolderPath } from './utils';
+
+const MOCK_FOLDER = {
+  metadata: { annotations: { [AnnoKeySourcePath]: 'path/to/folder' } },
+  spec: { title: 'folder title' },
+  status: {},
+};
+
+describe('getTargetFolderPathInRepo', () => {
+  it('should return root path for empty UID', () => {
+    const result = getTargetFolderPathInRepo({ targetFolderUID: '' });
+    expect(result).toBe('/');
+  });
+
+  it('should return empty string for empty UID and hide prepend slash', () => {
+    const result = getTargetFolderPathInRepo({ targetFolderUID: '', hidePrependSlash: true });
+    expect(result).toBe('');
+  });
+
+  it('should return root path for repository root folder', () => {
+    const result = getTargetFolderPathInRepo({
+      targetFolder: MOCK_FOLDER,
+      repoName: 'my-repo',
+      targetFolderUID: '',
+    });
+    expect(result).toBe('/');
+  });
+
+  it('should return nested folder path', () => {
+    const result = getTargetFolderPathInRepo({
+      targetFolder: MOCK_FOLDER,
+      targetFolderUID: 'folder-uid',
+    });
+    expect(result).toBe('path/to/folder/');
+  });
+
+  it('should return undefined when invalid folder is provided', () => {
+    const result = getTargetFolderPathInRepo({
+      targetFolder: undefined,
+      targetFolderUID: 'folder-uid',
+    });
+    expect(result).toBeUndefined();
+  });
+});
+
+describe('getNestedFolderPath', () => {
+  it('should return undefined for invalid folder', () => {
+    const result = getNestedFolderPath(undefined);
+    expect(result).toBeUndefined();
+  });
+
+  it('should return empty string for empty UID and hide prepend slash', () => {
+    // @ts-expect-error
+    const result = getNestedFolderPath({});
+    expect(result).toBe('/');
+  });
+});
@@ -8,6 +8,7 @@ export type BulkActionFormData = {
  comment: string;
  ref: string;
  workflow?: WorkflowOption;
+  targetFolderUID?: string;
 };

 export interface BulkActionProvisionResourceProps {
@@ -16,12 +17,67 @@ export interface BulkActionProvisionResourceProps {
  onDismiss?: () => void;
 }

-export function getTargetFolderPathInRepo({ targetFolder }: { targetFolder?: Folder }): string | undefined {
+/**
+ * @example
+ * // Whole instance provisioned (root)
+ * getTargetFolderPathInRepo({ targetFolderUID: '' }) // returns "/"
+ *
+ * // Repository root folder
+ * getTargetFolderPathInRepo(...) // returns "/"
+ *
+ * // Nested folder
+ * getTargetFolderPathInRepo(...) // returns "path/to/folder/"
+ */
+
+type GetTargetFolderPathInRepoParams = {
+  targetFolderUID?: string;
+  targetFolder?: Folder;
+  repoName?: string;
+  hidePrependSlash?: boolean;
+};
+
+export function getTargetFolderPathInRepo({
+  targetFolderUID,
+  targetFolder,
+  repoName,
+  hidePrependSlash = false,
+}: GetTargetFolderPathInRepoParams): string | undefined {
+  const ROOT_PATH = '/';
+  const EMPTY_ROOT_PATH = ''; // this is used to prevent duplicate "/" in url
+
+  // Case 1: Whole instance is provisioned and no folder uid passed in (empty UID indicates root)
+  if (targetFolderUID === '') {
+    return hidePrependSlash ? EMPTY_ROOT_PATH : ROOT_PATH;
+  }
+
+  // Case 2: Target folder is the repository root folder
+  if (isRepositoryRootFolder(targetFolder, repoName)) {
+    return hidePrependSlash ? EMPTY_ROOT_PATH : ROOT_PATH;
+  }
+
+  // Case 3: Regular folder with source path annotation
+  return getNestedFolderPath(targetFolder);
+}
+
+/**
+ * Checks if the target folder is the repository root folder
+ */
+function isRepositoryRootFolder(targetFolder?: Folder, repoName?: string) {
+  return Boolean(targetFolder?.metadata?.name === repoName && repoName);
+}
+
+/**
+ * Gets the path for a nested folder from its annotations
+ */
+export function getNestedFolderPath(targetFolder?: Folder): string | undefined {
  if (!targetFolder) {
    return undefined;
  }
-  const folderAnnotations = targetFolder.metadata.annotations || {};
-  return folderAnnotations[AnnoKeySourcePath] || targetFolder.metadata.name || '';
+  const folderAnnotations = targetFolder?.metadata?.annotations || {};
+  const sourcePath = folderAnnotations[AnnoKeySourcePath] || '';
+
+  // Ensure path ends with slash
+  return sourcePath ? `${sourcePath}/` : '/';
 }

 export function getResourceTargetPath(currentPath: string, targetFolderPath: string): string {
@@ -5,6 +5,7 @@ import { selectors } from '@grafana/e2e-selectors';
 import { t } from '@grafana/i18n';
 import { Checkbox, Tooltip, useStyles2 } from '@grafana/ui';
 import { ManagerKind } from 'app/features/apiserver/types';
+import { useIsProvisionedInstance } from 'app/features/provisioning/hooks/useIsProvisionedInstance';
 import { getReadOnlyTooltipText } from 'app/features/provisioning/utils/repository';
 import { useSelector } from 'app/types/store';

@@ -24,6 +25,7 @@ export default function CheckboxCell({
  // Get current selection state for repository validation
  const selectedItems = useSelector((state) => state.browseDashboards.selectedItems);
  const { selectedItemsRepoUID, isInLockedRepo, isUidInReadOnlyRepo } = useSelectionRepoValidation(selectedItems);
+  const isProvisionedInstance = useIsProvisionedInstance();

  // Early returns for cases where we should show a spacer instead of checkbox
  if (!isSelected) {
@@ -42,8 +44,8 @@ export default function CheckboxCell({
    return <CheckboxSpacer />;
  }

-  // Disable checkbox for root provisioned folder itself
-  if (item.managedBy === ManagerKind.Repo && !item.parentUID) {
+  // Disable the checkbox for the root provisioned folder (if the entire instance is not provisioned)
+  if (!isProvisionedInstance && item.managedBy === ManagerKind.Repo && !item.parentUID) {
    return <CheckboxSpacer />;
  }

@@ -1,6 +1,5 @@
 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';
@@ -97,11 +96,6 @@ 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) {
@@ -145,9 +139,9 @@ export function setItemSelectionState(

 export function setAllSelection(
  state: BrowseDashboardsState,
-  action: PayloadAction<{ isSelected: boolean; folderUID: string | undefined }>
+  action: PayloadAction<{ isSelected: boolean; folderUID: string | undefined; excludeUIDs?: string[] }>
 ) {
-  const { isSelected, folderUID: folderUIDArg } = action.payload;
+  const { isSelected, folderUID: folderUIDArg, excludeUIDs } = action.payload;

  // If we're in the folder view for sharedwith me (currently not supported)
  // bail and don't select anything
@@ -184,8 +178,8 @@ export function setAllSelection(
          continue;
        }

-        // Skip all provisioned resources during "select all" on root level
-        if (child.managedBy === ManagerKind.Repo && !child.parentUID) {
+        // Skip items in the exclude list
+        if (excludeUIDs?.includes(child.uid)) {
          continue;
        }

@@ -1,3 +1,4 @@
+import { skipToken } from '@reduxjs/toolkit/query';
 import { useEffect, useState } from 'react';
 import { FormProvider, useForm } from 'react-hook-form';
 import { useNavigate } from 'react-router-dom-v5-compat';
@@ -62,7 +63,7 @@ export function MoveProvisionedDashboardForm({
    path: defaultValues.path,
  });

-  const { data: targetFolder } = useGetFolderQuery({ name: targetFolderUID! }, { skip: !targetFolderUID });
+  const { data: targetFolder } = useGetFolderQuery(targetFolderUID ? { name: targetFolderUID! } : skipToken);

  const [moveFile, moveRequest] = useCreateRepositoryFilesWithPathMutation();
  const [targetPath, setTargetPath] = useState<string>('');
@@ -71,17 +72,20 @@ export function MoveProvisionedDashboardForm({

  useEffect(() => {
    const currentSourcePath = currentFileData?.resource?.dryRun?.metadata?.annotations?.[AnnoKeySourcePath];
-    if (!targetFolderUID || !targetFolder || !currentSourcePath) {
+    if (!currentSourcePath || targetFolderUID === undefined) {
      return;
    }

-    const targetFolderPath = getTargetFolderPathInRepo({ targetFolder });
-
    const filename = currentSourcePath.split('/').pop();
-    const newPath = `${targetFolderPath}/${filename}`;
-
+    const targetFolderPath = getTargetFolderPathInRepo({
+      targetFolderUID,
+      targetFolder,
+      repoName: repository?.name,
+      hidePrependSlash: true,
+    });
+    const newPath = `${targetFolderPath}${filename}`;
    setTargetPath(newPath);
-  }, [currentFileData, targetFolder, targetFolderUID, targetFolderTitle]);
+  }, [currentFileData, targetFolder, targetFolderUID, targetFolderTitle, repository]);

  const handleSubmitForm = async ({ repo, path, comment }: ProvisionedDashboardFormData) => {
    if (!currentFileData?.resource?.file) {
@@ -3543,7 +3543,6 @@
      "button-cancel": "Cancel",
      "button-move": "Move",
      "button-moving": "Moving...",
-      "button-tooltip": "Please select a target folder",
      "error": {
        "read-only-message": "If you have direct access to the target, please make modifications directly in the target repository.",
        "read-only-saving-message": "Repository is read-only and provisioned in git. {{readOnlyMessage}}",
@@ -3552,7 +3551,7 @@
        "repository-not-found-title": "Repository not found"
      },
      "error-moving-resources": "Error moving resources",
-      "error-no-target-folder-path": "Target folder path in repository is invalid, please select another folder.",
+      "error-no-target-folder-path": "Target folder path is invalid or empty, please select again.",
      "move-warning": "This will move selected folders and their descendants. In total, this will affect:",
      "target-folder": "Target Folder"
    },