From bab84c64dc7e7e115eebdcdb213e38e6ad7684fb Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Tue, 2 Sep 2025 12:41:29 +0200 Subject: [PATCH] Folders: Migrate bulk move action to new API (#110150) --- .../api/clients/folder/v1beta1/hooks.test.ts | 107 ++++++++++++++---- .../app/api/clients/folder/v1beta1/hooks.ts | 85 +++++++++++--- .../app/api/clients/folder/v1beta1/index.ts | 27 ++++- .../app/api/clients/folder/v1beta1/utils.ts | 15 ++- public/app/features/apiserver/types.ts | 4 + .../api/browseDashboardsAPI.ts | 95 +++++++++------- .../BrowseActions/BrowseActions.tsx | 16 ++- public/locales/en-US/grafana.json | 4 +- 8 files changed, 266 insertions(+), 87 deletions(-) diff --git a/public/app/api/clients/folder/v1beta1/hooks.test.ts b/public/app/api/clients/folder/v1beta1/hooks.test.ts index 99f74fe7512..1dd64562abe 100644 --- a/public/app/api/clients/folder/v1beta1/hooks.test.ts +++ b/public/app/api/clients/folder/v1beta1/hooks.test.ts @@ -5,31 +5,51 @@ import { config, setBackendSrv } from '@grafana/runtime'; import { setupMockServer } from '@grafana/test-utils/server'; import { getFolderFixtures } from '@grafana/test-utils/unstable'; import { backendSrv } from 'app/core/services/backend_srv'; -import { useDeleteFoldersMutation as useDeleteFoldersMutationLegacy } from 'app/features/browse-dashboards/api/browseDashboardsAPI'; +import { + useDeleteFoldersMutation as useDeleteFoldersMutationLegacy, + useMoveFoldersMutation as useMoveFoldersMutationLegacy, +} from 'app/features/browse-dashboards/api/browseDashboardsAPI'; -import { useGetFolderQueryFacade, useDeleteMultipleFoldersMutationFacade } from './hooks'; +import { AnnoKeyFolder } from '../../../../features/apiserver/types'; + +import { + useGetFolderQueryFacade, + useDeleteMultipleFoldersMutationFacade, + useMoveMultipleFoldersMutationFacade, +} from './hooks'; import { setupCreateFolder } from './test-utils'; -import { useDeleteFolderMutation } from './index'; +import { useDeleteFolderMutation, useUpdateFolderMutation } from './index'; // Mocks for the hooks used inside useGetFolderQueryFacade jest.mock('./index', () => ({ ...jest.requireActual('./index'), useDeleteFolderMutation: jest.fn(), + useUpdateFolderMutation: jest.fn(), })); +const publishMockFn = jest.fn(); jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), getAppEvents: jest.fn(() => ({ - publish: jest.fn(), + publish: publishMockFn, })), })); -const mockGetAppEvents = jest.mocked(require('@grafana/runtime').getAppEvents); jest.mock('app/features/browse-dashboards/api/browseDashboardsAPI', () => ({ ...jest.requireActual('app/features/browse-dashboards/api/browseDashboardsAPI'), useDeleteFoldersMutation: jest.fn(), + useMoveFoldersMutation: jest.fn(), })); + +const dispatchMockFn = jest.fn(); +jest.mock('../../../../types/store', () => { + return { + ...jest.requireActual('../../../../types/store'), + useDispatch: () => dispatchMockFn, + }; +}); + setBackendSrv(backendSrv); setupMockServer(); @@ -54,9 +74,14 @@ const renderFolderHook = async () => { }; const originalToggles = { ...config.featureToggles }; -const originalAppSubUrl = String(config.appSubUrl); +afterAll(() => { + // Restore the original feature toggle value changed during tests + config.featureToggles = originalToggles; +}); describe('useGetFolderQueryFacade', () => { + const originalAppSubUrl = String(config.appSubUrl); + beforeEach(() => { config.appSubUrl = '/grafana'; }); @@ -130,31 +155,16 @@ describe('useGetFolderQueryFacade', () => { }); describe('useDeleteMultipleFoldersMutationFacade', () => { - const dispatchMock = jest.fn(); const mockDeleteFolder = jest.fn(() => ({ error: undefined })); const mockDeleteFolderLegacy = jest.fn(() => ({ error: undefined })); - const publishMock = jest.fn(); - - const oldToggleValue = config.featureToggles.foldersAppPlatformAPI; - - afterAll(() => { - config.featureToggles.foldersAppPlatformAPI = oldToggleValue; - }); beforeEach(() => { - mockDeleteFolder.mockClear(); - mockDeleteFolderLegacy.mockClear(); + jest.clearAllMocks(); (useDeleteFolderMutation as jest.Mock).mockReturnValue([mockDeleteFolder]); (useDeleteFoldersMutationLegacy as jest.Mock).mockReturnValue([mockDeleteFolderLegacy]); - - // Mock useDispatch - jest.spyOn(require('../../../../types/store'), 'useDispatch').mockReturnValue(dispatchMock); }); it('deletes multiple folders and publishes success alert', async () => { - mockGetAppEvents.mockReturnValue({ - publish: publishMock, - }); config.featureToggles.foldersAppPlatformAPI = true; const folderUIDs = ['uid1', 'uid2']; const deleteFolders = useDeleteMultipleFoldersMutationFacade(); @@ -166,13 +176,13 @@ describe('useDeleteMultipleFoldersMutationFacade', () => { expect(mockDeleteFolder).toHaveBeenCalledWith({ name: 'uid2' }); // Should publish success alert - expect(publishMock).toHaveBeenCalledWith({ + expect(publishMockFn).toHaveBeenCalledWith({ type: AppEvents.alertSuccess.name, payload: ['Folder deleted'], }); // Should dispatch refreshParents - expect(dispatchMock).toHaveBeenCalled(); + expect(dispatchMockFn).toHaveBeenCalled(); }); it('uses legacy call when flag is false', async () => { @@ -187,6 +197,55 @@ describe('useDeleteMultipleFoldersMutationFacade', () => { }); }); +describe('useMoveMultipleFoldersMutationFacade', () => { + const mockUpdateFolder = jest.fn(() => ({ error: undefined })); + const mockMoveFolders = jest.fn(() => ({ error: undefined })); + + beforeEach(() => { + jest.clearAllMocks(); + (useUpdateFolderMutation as jest.Mock).mockReturnValue([mockUpdateFolder]); + (useMoveFoldersMutationLegacy as jest.Mock).mockReturnValue([mockMoveFolders]); + }); + + it('moves multiple folders and publishes success alert', async () => { + config.featureToggles.foldersAppPlatformAPI = true; + const folderUIDs = ['uid1', 'uid2']; + const [moveFolders] = useMoveMultipleFoldersMutationFacade(); + await moveFolders({ folderUIDs, destinationUID: 'uid3' }); + + // Should call deleteFolder for each UID + expect(mockUpdateFolder).toHaveBeenCalledTimes(folderUIDs.length); + expect(mockUpdateFolder).toHaveBeenCalledWith({ + name: 'uid1', + patch: { metadata: { annotations: { [AnnoKeyFolder]: 'uid3' } } }, + }); + expect(mockUpdateFolder).toHaveBeenCalledWith({ + name: 'uid2', + patch: { metadata: { annotations: { [AnnoKeyFolder]: 'uid3' } } }, + }); + + // Should publish a success alert + expect(publishMockFn).toHaveBeenCalledWith({ + type: AppEvents.alertSuccess.name, + payload: ['Folder moved'], + }); + + // Should dispatch refreshParents + expect(dispatchMockFn).toHaveBeenCalled(); + }); + + it('uses legacy call when flag is false', async () => { + config.featureToggles.foldersAppPlatformAPI = false; + const folderUIDs = ['uid1', 'uid2']; + const [moveFolders] = useMoveMultipleFoldersMutationFacade(); + await moveFolders({ folderUIDs, destinationUID: 'uid3' }); + + // Should call deleteFolder for each UID + expect(mockMoveFolders).toHaveBeenCalledTimes(1); + expect(mockMoveFolders).toHaveBeenCalledWith({ folderUIDs, destinationUID: 'uid3' }); + }); +}); + describe('useCreateFolder', () => { describe.each([ // app platform diff --git a/public/app/api/clients/folder/v1beta1/hooks.ts b/public/app/api/clients/folder/v1beta1/hooks.ts index c8f5e279dea..7db4e810260 100644 --- a/public/app/api/clients/folder/v1beta1/hooks.ts +++ b/public/app/api/clients/folder/v1beta1/hooks.ts @@ -10,6 +10,9 @@ import { useGetFolderQuery as useGetFolderQueryLegacy, useDeleteFoldersMutation as useDeleteFoldersMutationLegacy, useNewFolderMutation as useLegacyNewFolderMutation, + useMoveFoldersMutation as useMoveFoldersMutationLegacy, + MoveFoldersArgs, + DeleteFoldersArgs, } from 'app/features/browse-dashboards/api/browseDashboardsAPI'; import { dispatch } from 'app/store/store'; import { FolderDTO, NewFolder } from 'app/types/folders'; @@ -38,6 +41,7 @@ import { useGetFolderParentsQuery, useDeleteFolderMutation, useCreateFolderMutation, + useUpdateFolderMutation, Folder, CreateFolderApiArg, } from './index'; @@ -202,29 +206,84 @@ export function useDeleteMultipleFoldersMutationFacade() { return deleteFolders; } - return async function deleteFolders({ folderUIDs }: { folderUIDs: string[] }) { + return async function deleteFolders({ folderUIDs }: DeleteFoldersArgs) { + const successMessage = t('folders.api.folder-deleted-success', 'Folder deleted'); + // Delete all the folders sequentially // TODO error handling here for (const folderUID of folderUIDs) { // This also shows warning alert - if (await isProvisionedFolderCheck(dispatch, folderUID)) { - continue; - } - const result = await deleteFolder({ name: folderUID }); - if (!result.error) { - // Before this was done in backend srv automatically because the old API sent a message wiht 200 request. see - // public/app/core/services/backend_srv.ts#L341-L361. New API does not do that so we do it here. - getAppEvents().publish({ - type: AppEvents.alertSuccess.name, - payload: [t('folders.api.folder-deleted-success', 'Folder deleted')], - }); - dispatch(refreshParents(folderUIDs)); + const isProvisioned = await isProvisionedFolderCheck(dispatch, folderUID); + + if (!isProvisioned) { + const result = await deleteFolder({ name: folderUID }); + if (!result.error) { + // Before this was done in backend srv automatically because the old API sent a message wiht 200 request. see + // public/app/core/services/backend_srv.ts#L341-L361. New API does not do that so we do it here. + getAppEvents().publish({ + type: AppEvents.alertSuccess.name, + payload: [successMessage], + }); + } } } + + dispatch(refreshParents(folderUIDs)); return { data: undefined }; }; } +export function useMoveMultipleFoldersMutationFacade() { + const moveFoldersLegacyResult = useMoveFoldersMutationLegacy(); + const [updateFolder, updateFolderData] = useUpdateFolderMutation(); + const dispatch = useDispatch(); + + if (!config.featureToggles.foldersAppPlatformAPI) { + return moveFoldersLegacyResult; + } + + async function moveFolders({ folderUIDs, destinationUID }: MoveFoldersArgs) { + const provisionedWarning = t( + 'folders.api.folder-move-error-provisioned', + 'Cannot move provisioned folder. To move it, move it in the repository and synchronise to apply the changes.' + ); + const successMessage = t('folders.api.folder-moved-success', 'Folder moved'); + + // Move all the folders sequentially one by one + for (const folderUID of folderUIDs) { + // isProvisionedFolderCheck also shows a warning alert + const isFolderProvisioned = await isProvisionedFolderCheck(dispatch, folderUID, { warning: provisionedWarning }); + + // If provisioned, we just skip this folder + if (!isFolderProvisioned) { + const result = await updateFolder({ + name: folderUID, + patch: { metadata: { annotations: { [AnnoKeyFolder]: destinationUID } } }, + }); + if (!result.error) { + getAppEvents().publish({ + type: AppEvents.alertSuccess.name, + payload: [successMessage], + }); + } + } + } + + // Refresh the state of the parent folders to update the UI after folders are moved + dispatch( + refetchChildren({ + parentUID: destinationUID, + pageSize: PAGE_SIZE, + }) + ); + dispatch(refreshParents(folderUIDs)); + + return { data: undefined }; + } + + return [moveFolders, updateFolderData] as const; +} + export function useCreateFolder() { const [createFolder, result] = useCreateFolderMutation(); const legacyHook = useLegacyNewFolderMutation(); diff --git a/public/app/api/clients/folder/v1beta1/index.ts b/public/app/api/clients/folder/v1beta1/index.ts index 984c6f173ea..78aafad3098 100644 --- a/public/app/api/clients/folder/v1beta1/index.ts +++ b/public/app/api/clients/folder/v1beta1/index.ts @@ -18,11 +18,34 @@ export const folderAPIv1beta1 = generatedAPI.enhanceEndpoints({ // We don't want delete to invalidate getFolder tags, as that would lead to unnecessary 404s invalidatesTags: (result, error) => (error ? [] : [{ type: 'Folder', id: 'LIST' }]), }, + updateFolder: { + query: (queryArg) => ({ + url: `/folders/${queryArg.name}`, + method: 'PATCH', + // We need to stringify the body and set the correct header for the call to work with k8s api. + body: JSON.stringify(queryArg.patch), + headers: { + 'Content-Type': 'application/strategic-merge-patch+json', + }, + params: { + pretty: queryArg.pretty, + dryRun: queryArg.dryRun, + fieldManager: queryArg.fieldManager, + fieldValidation: queryArg.fieldValidation, + force: queryArg.force, + }, + }), + }, }, }); -export const { useGetFolderQuery, useGetFolderParentsQuery, useDeleteFolderMutation, useCreateFolderMutation } = - folderAPIv1beta1; +export const { + useGetFolderQuery, + useGetFolderParentsQuery, + useDeleteFolderMutation, + useCreateFolderMutation, + useUpdateFolderMutation, +} = folderAPIv1beta1; // eslint-disable-next-line no-barrel-files/no-barrel-files export { type Folder, type FolderList, type CreateFolderApiArg } from './endpoints.gen'; diff --git a/public/app/api/clients/folder/v1beta1/utils.ts b/public/app/api/clients/folder/v1beta1/utils.ts index 96606671723..19e16b624c0 100644 --- a/public/app/api/clients/folder/v1beta1/utils.ts +++ b/public/app/api/clients/folder/v1beta1/utils.ts @@ -8,7 +8,11 @@ import { useDispatch } from '../../../../types/store'; import { folderAPIv1beta1 as folderAPI } from './index'; -export async function isProvisionedFolderCheck(dispatch: ReturnType, folderUID: string) { +export async function isProvisionedFolderCheck( + dispatch: ReturnType, + folderUID: string, + options?: { warning?: string } +) { if (config.featureToggles.provisioning) { const folder = await dispatch(folderAPI.endpoints.getFolder.initiate({ name: folderUID })); // TODO: taken from browseDashboardAPI as it is, but this error handling should be moved up to UI code. @@ -16,10 +20,11 @@ export async function isProvisionedFolderCheck(dispatch: ReturnType; + dashboardUIDs: string[]; +} + +export interface MoveFoldersArgs { + destinationUID: string; + folderUIDs: string[]; } export interface ImportInputs { @@ -217,37 +221,13 @@ export const browseDashboardsAPI = createApi({ }, }), - // move *multiple* items (folders and dashboards). used in the move modal. - moveItems: builder.mutation({ + // move *multiple* dashboards. used in the move modal. + moveDashboards: builder.mutation({ invalidatesTags: ['getFolder'], - queryFn: async ({ selectedItems, destinationUID }, _api, _extraOptions, baseQuery) => { - const selectedDashboards = Object.keys(selectedItems.dashboard).filter((uid) => selectedItems.dashboard[uid]); - const selectedFolders = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]); - - // Move all the folders sequentially - // TODO error handling here - for (const folderUID of selectedFolders) { - if (config.featureToggles.provisioning) { - const folder = await dispatch(folderAPI.endpoints.getFolder.initiate({ name: folderUID })); - if (isProvisionedFolder(folder.data)) { - appEvents.publish({ - type: AppEvents.alertWarning.name, - payload: ['Cannot move provisioned folder'], - }); - continue; - } - } - - await baseQuery({ - url: `/folders/${folderUID}/move`, - method: 'POST', - body: { parentUID: destinationUID }, - }); - } - + queryFn: async ({ dashboardUIDs, destinationUID }, _api, _extraOptions, baseQuery) => { // Move all the dashboards sequentially // TODO error handling here - for (const dashboardUID of selectedDashboards) { + for (const dashboardUID of dashboardUIDs) { const fullDash = await getDashboardAPI().getDashboardDTO(dashboardUID); const dashboard = isDashboardV2Resource(fullDash) ? fullDash.spec : fullDash.dashboard; const k8s = isDashboardV2Resource(fullDash) ? fullDash.metadata : undefined; @@ -271,9 +251,7 @@ export const browseDashboardsAPI = createApi({ } return { data: undefined }; }, - onQueryStarted: ({ destinationUID, selectedItems }, { queryFulfilled, dispatch }) => { - const selectedDashboards = Object.keys(selectedItems.dashboard).filter((uid) => selectedItems.dashboard[uid]); - const selectedFolders = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]); + onQueryStarted: ({ destinationUID, dashboardUIDs }, { queryFulfilled, dispatch }) => { queryFulfilled.then(() => { dispatch( refetchChildren({ @@ -281,7 +259,47 @@ export const browseDashboardsAPI = createApi({ pageSize: PAGE_SIZE, }) ); - dispatch(refreshParents([...selectedFolders, ...selectedDashboards])); + dispatch(refreshParents(dashboardUIDs)); + }); + }, + }), + + // move *multiple* folders. used in the move modal. + moveFolders: builder.mutation({ + invalidatesTags: ['getFolder'], + queryFn: async ({ folderUIDs, destinationUID }, _api, _extraOptions, baseQuery) => { + // Move all the folders sequentially + // TODO error handling here + for (const folderUID of folderUIDs) { + if ( + await isProvisionedFolderCheck(dispatch, folderUID, { + warning: t( + 'folders.api.folder-move-error-provisioned', + 'Cannot move provisioned folder. To move it, move it in the repository and synchronise to apply the changes.' + ), + }) + ) { + continue; + } + + await baseQuery({ + url: `/folders/${folderUID}/move`, + method: 'POST', + body: { parentUID: destinationUID }, + }); + } + + return { data: undefined }; + }, + onQueryStarted: ({ destinationUID, folderUIDs }, { queryFulfilled, dispatch }) => { + queryFulfilled.then(() => { + dispatch( + refetchChildren({ + parentUID: destinationUID, + pageSize: PAGE_SIZE, + }) + ); + dispatch(refreshParents(folderUIDs)); }); }, }), @@ -499,7 +517,8 @@ export const { useGetFolderQuery, useLazyGetFolderQuery, useMoveFolderMutation, - useMoveItemsMutation, + useMoveDashboardsMutation, + useMoveFoldersMutation, useNewFolderMutation, useSaveDashboardMutation, useSaveFolderMutation, diff --git a/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx b/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx index 7a1fef2634d..d703846925b 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx @@ -13,8 +13,11 @@ import { ShowModalReactEvent } from 'app/types/events'; import { FolderDTO } from 'app/types/folders'; import { useDispatch } from 'app/types/store'; -import { useDeleteMultipleFoldersMutationFacade } from '../../../../api/clients/folder/v1beta1/hooks'; -import { useDeleteDashboardsMutation, useMoveItemsMutation } from '../../api/browseDashboardsAPI'; +import { + useDeleteMultipleFoldersMutationFacade, + useMoveMultipleFoldersMutationFacade, +} from '../../../../api/clients/folder/v1beta1/hooks'; +import { useDeleteDashboardsMutation, useMoveDashboardsMutation } from '../../api/browseDashboardsAPI'; import { useActionSelectionState } from '../../state/hooks'; import { setAllSelection } from '../../state/slice'; import { DashboardTreeSelection } from '../../types'; @@ -35,7 +38,8 @@ export function BrowseActions({ folderDTO }: Props) { const selectedItems = useActionSelectionState(); const [deleteDashboards] = useDeleteDashboardsMutation(); const deleteFolders = useDeleteMultipleFoldersMutationFacade(); - const [moveItems] = useMoveItemsMutation(); + const [moveFolders] = useMoveMultipleFoldersMutationFacade(); + const [moveDashboards] = useMoveDashboardsMutation(); const [, stateManager] = useSearchStateManager(); const provisioningEnabled = config.featureToggles.provisioning; @@ -65,7 +69,11 @@ export function BrowseActions({ folderDTO }: Props) { }; const onMove = async (destinationUID: string) => { - await moveItems({ selectedItems, destinationUID }); + const selectedDashboards = Object.keys(selectedItems.dashboard).filter((uid) => selectedItems.dashboard[uid]); + const selectedFolders = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]); + + await moveFolders({ folderUIDs: selectedFolders, destinationUID }); + await moveDashboards({ dashboardUIDs: selectedDashboards, destinationUID }); trackAction('move', selectedItems); onActionComplete(); }; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 3d7f02a77ff..c4ffe67cb4f 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -7566,7 +7566,9 @@ "folders": { "api": { "folder-delete-error-provisioned": "Cannot delete provisioned folder. To remove it, delete it from the repository and synchronise to apply the changes.", - "folder-deleted-success": "Folder deleted" + "folder-deleted-success": "Folder deleted", + "folder-move-error-provisioned": "Cannot move provisioned folder. To move it, move it in the repository and synchronise to apply the changes.", + "folder-moved-success": "Folder moved" }, "get-loading-nav": { "main": {