diff --git a/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.test.tsx b/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.test.tsx index 00687677e94..a3ddd150785 100644 --- a/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.test.tsx +++ b/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.test.tsx @@ -49,6 +49,26 @@ jest.mock('app/features/dashboard-scene/components/Provisioned/ResourceEditFormS ResourceEditFormSharedFields: () =>
, })); +const MOCK_DATA = { + repository: { + name: 'test-repo', + namespace: 'default', + title: 'Test Repository', + type: 'git', + }, + resource: { + type: { + kind: 'Folder', + }, + upsert: { + apiVersion: 'v1', + kind: 'Folder', + metadata: { name: 'test-folder', uid: 'test-folder-uid' }, + spec: { title: 'Test Folder' }, + }, + }, +}; + const mockUseDeleteRepositoryFilesMutation = useDeleteRepositoryFilesWithPathMutation as jest.MockedFunction< typeof useDeleteRepositoryFilesWithPathMutation >; @@ -267,7 +287,13 @@ describe('DeleteProvisionedFolderForm', () => { describe('success handling', () => { it('should navigate to parent folder on successful write workflow', async () => { - const successState = { isLoading: false, isSuccess: true, isError: false, error: null }; + const successState = { + isLoading: false, + isSuccess: true, + isError: false, + error: null, + data: MOCK_DATA, + }; setup({}, defaultHookData, successState); await waitFor(() => { @@ -277,7 +303,13 @@ describe('DeleteProvisionedFolderForm', () => { it('should navigate to dashboards root when parent folder has no parentUid', async () => { const folderWithoutParent = { ...mockParentFolder, parentUid: undefined }; - const successState = { isLoading: false, isSuccess: true, isError: false, error: null }; + const successState = { + isLoading: false, + isSuccess: true, + isError: false, + error: null, + data: MOCK_DATA, + }; setup({ parentFolder: folderWithoutParent }, defaultHookData, successState); await waitFor(() => { @@ -292,13 +324,19 @@ describe('DeleteProvisionedFolderForm', () => { isSuccess: true, isError: false, error: null, - data: { urls: { newPullRequestURL: 'https://github.com/test/repo/pull/new' } }, + data: { + ...MOCK_DATA, + ref: 'feature-branch', + path: 'folders/test-folder.json', + urls: { newPullRequestURL: 'https://github.com/test/repo/pull/new' }, + }, }; const { mockNavigate } = setup({}, { ...defaultHookData, initialValues: branchFormData }, successState); await waitFor(() => { const expectedParams = new URLSearchParams(); expectedParams.set('new_pull_request_url', 'https://github.com/test/repo/pull/new'); + expectedParams.set('repo_type', 'git'); const expectedUrl = `/dashboards?${expectedParams.toString()}`; expect(mockNavigate).toHaveBeenCalledWith(expectedUrl); diff --git a/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.tsx b/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.tsx index 70638377a3a..f6fb26101f8 100644 --- a/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.tsx +++ b/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.tsx @@ -1,4 +1,3 @@ -import { useEffect } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import { useNavigate } from 'react-router-dom-v5-compat'; @@ -12,6 +11,10 @@ import { AnnoKeySourcePath } from 'app/features/apiserver/types'; import { ResourceEditFormSharedFields } from 'app/features/dashboard-scene/components/Provisioned/ResourceEditFormSharedFields'; import { BaseProvisionedFormData } from 'app/features/dashboard-scene/saving/shared'; import { buildResourceBranchRedirectUrl } from 'app/features/dashboard-scene/settings/utils'; +import { + useProvisionedRequestHandler, + ProvisionedOperationInfo, +} from 'app/features/dashboard-scene/utils/useProvisionedRequestHandler'; import { FolderDTO } from 'app/types/folders'; import { useProvisionedFolderFormData } from '../hooks/useProvisionedFolderFormData'; @@ -57,50 +60,51 @@ function FormContent({ initialValues, parentFolder, repository, workflowOptions, }); }; - // TODO: move to a hook if this useEffect shared mostly the same logic as in NewProvisionedFolderForm - useEffect(() => { - if (request.isSuccess && repository) { - const prUrl = request.data?.urls?.newPullRequestURL; - if (workflow === 'branch' && prUrl) { - const url = buildResourceBranchRedirectUrl({ - paramName: 'new_pull_request_url', - paramValue: prUrl, - repoType: request.data?.repository?.type, - }); - navigate(url); - return; - } - - if (workflow === 'write') { - getAppEvents().publish({ - type: AppEvents.alertSuccess.name, - payload: [ - t( - 'browse-dashboards.delete-provisioned-folder-form.alert-folder-deleted-successfully', - 'Folder deleted successfully' - ), - ], - }); - // Navigate back to parent folder if it exists, otherwise go to dashboards root - if (parentFolder?.parentUid) { - window.location.href = getFolderURL(parentFolder.parentUid); - } else { - window.location.href = '/dashboards'; - } - } - } - - if (request.isError) { - getAppEvents().publish({ - type: AppEvents.alertError.name, - payload: [ - t('browse-dashboards.delete-provisioned-folder-form.api-error', 'Failed to delete folder'), - request.error, - ], + const onBranchSuccess = ({ urls }: { urls?: Record }, info: ProvisionedOperationInfo) => { + const prUrl = urls?.newPullRequestURL; + if (prUrl) { + const url = buildResourceBranchRedirectUrl({ + paramName: 'new_pull_request_url', + paramValue: prUrl, + repoType: info.repoType, }); - return; + navigate(url); } - }, [request, repository, workflow, parentFolder, navigate]); + }; + + const onWriteSuccess = () => { + // Navigate back to parent folder if it exists, otherwise go to dashboards root + if (parentFolder?.parentUid) { + window.location.href = getFolderURL(parentFolder.parentUid); + } else { + window.location.href = '/dashboards'; + } + }; + + const onError = (error: unknown) => { + getAppEvents().publish({ + type: AppEvents.alertError.name, + payload: [t('browse-dashboards.delete-provisioned-folder-form.api-error', 'Failed to delete folder'), error], + }); + }; + + // Use the repository-type and resource-type aware provisioned request handler + useProvisionedRequestHandler({ + request, + workflow, + successMessage: t( + 'browse-dashboards.delete-provisioned-folder-form.success-message', + 'Folder deleted successfully' + ), + resourceType: 'folder', + repository, + handlers: { + onDismiss, + onBranchSuccess, + onWriteSuccess, + onError, + }, + }); return ( diff --git a/public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx b/public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx index a453490e07c..797f4bb98ba 100644 --- a/public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx +++ b/public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx @@ -1,5 +1,4 @@ import { css } from '@emotion/css'; -import { useEffect } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import { useNavigate } from 'react-router-dom-v5-compat'; @@ -13,6 +12,10 @@ import { AnnoKeySourcePath, Resource } from 'app/features/apiserver/types'; import { ResourceEditFormSharedFields } from 'app/features/dashboard-scene/components/Provisioned/ResourceEditFormSharedFields'; import { BaseProvisionedFormData } from 'app/features/dashboard-scene/saving/shared'; import { buildResourceBranchRedirectUrl } from 'app/features/dashboard-scene/settings/utils'; +import { + useProvisionedRequestHandler, + ProvisionedOperationInfo, +} from 'app/features/dashboard-scene/utils/useProvisionedRequestHandler'; import { PROVISIONING_URL } from 'app/features/provisioning/constants'; import { usePullRequestParam } from 'app/features/provisioning/hooks/usePullRequestParam'; import { FolderDTO } from 'app/types/folders'; @@ -44,58 +47,60 @@ function FormContent({ initialValues, repository, workflowOptions, folder, onDis }); const { handleSubmit, watch, register, formState } = methods; - const [workflow, ref, title] = watch(['workflow', 'ref', 'title']); + const [workflow, title] = watch(['workflow', 'title']); - // TODO: replace with useProvisionedRequestHandler hook - useEffect(() => { - const appEvents = getAppEvents(); - if (request.isSuccess && repository) { - onDismiss?.(); - - appEvents.publish({ - type: AppEvents.alertSuccess.name, - payload: [ - t( - 'browse-dashboards.new-provisioned-folder-form.alert-folder-created-successfully', - 'Folder created successfully' - ), - ], + const onBranchSuccess = ({ urls }: { urls?: Record }, info: ProvisionedOperationInfo) => { + const prUrl = urls?.newPullRequestURL; + if (prUrl) { + const url = buildResourceBranchRedirectUrl({ + paramName: 'new_pull_request_url', + paramValue: prUrl, + repoType: info.repoType, }); + navigate(url); + } + }; - const prUrl = request.data?.urls?.newPullRequestURL; - if (workflow === 'branch' && prUrl) { - const url = buildResourceBranchRedirectUrl({ - paramName: 'new_pull_request_url', - paramValue: prUrl, - repoType: request.data?.repository?.type, - }); - navigate(url); - return; - } - - // TODO: Update when the upsert type is fixed - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - const folder = request.data.resource?.upsert as Resource; - if (folder?.metadata?.name) { - navigate(`/dashboards/f/${folder?.metadata?.name}/`); - return; - } + const onWriteSuccess = (resource: Resource) => { + // Navigation for new folders (resource-specific concern) + if (resource?.metadata?.name) { + navigate(`/dashboards/f/${resource.metadata.name}/`); + return; + } + // Fallback to provisioning URL + if (repository?.name && request.data?.path) { let url = `${PROVISIONING_URL}/${repository.name}/file/${request.data.path}`; if (request.data.ref?.length) { url += '?ref=' + request.data.ref; } navigate(url); - } else if (request.isError) { - appEvents.publish({ - type: AppEvents.alertError.name, - payload: [ - t('browse-dashboards.new-provisioned-folder-form.alert-error-creating-folder', 'Error creating folder'), - request.error, - ], - }); } - }, [request.isSuccess, request.isError, request.error, ref, request.data, workflow, navigate, repository, onDismiss]); + }; + + const onError = (error: unknown) => { + getAppEvents().publish({ + type: AppEvents.alertError.name, + payload: [ + t('browse-dashboards.new-provisioned-folder-form.alert-error-creating-folder', 'Error creating folder'), + error, + ], + }); + }; + + // Use the repository-type and resource-type aware provisioned request handler + useProvisionedRequestHandler({ + request, + workflow, + repository, + resourceType: 'folder', + handlers: { + onDismiss, + onBranchSuccess, + onWriteSuccess: (_, resource) => onWriteSuccess(resource), + onError, + }, + }); const doSave = async ({ ref, title, workflow, comment }: BaseProvisionedFormData) => { const repoName = repository?.name; diff --git a/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboardForm.tsx b/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboardForm.tsx index 4387af82439..260c491bb57 100644 --- a/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboardForm.tsx +++ b/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboardForm.tsx @@ -18,7 +18,7 @@ import { useCreateOrUpdateRepositoryFile } from 'app/features/provisioning/hooks import { ResourceEditFormSharedFields } from '../../components/Provisioned/ResourceEditFormSharedFields'; import { buildResourceBranchRedirectUrl } from '../../settings/utils'; import { getDashboardUrl } from '../../utils/getDashboardUrl'; -import { useProvisionedRequestHandler } from '../../utils/useProvisionedRequestHandler'; +import { ProvisionedOperationInfo, useProvisionedRequestHandler } from '../../utils/useProvisionedRequestHandler'; import { SaveDashboardFormCommonOptions } from '../SaveDashboardForm'; import { ProvisionedDashboardFormData } from '../shared'; @@ -60,25 +60,15 @@ export function SaveProvisionedDashboardForm({ reset(defaultValues); }, [defaultValues, reset]); - const onRequestError = (error: unknown) => { + const onRequestError = (error: unknown, info: ProvisionedOperationInfo) => { appEvents.publish({ type: AppEvents.alertError.name, payload: [t('dashboard-scene.save-provisioned-dashboard-form.api-error', 'Error saving dashboard'), error], }); }; - const onWriteSuccess = () => { - panelEditor?.onDiscard(); - drawer.onClose(); - locationService.partial({ - viewPanel: null, - editPanel: null, - }); - }; - - const onNewDashboardSuccess = (upsert: Resource) => { - panelEditor?.onDiscard(); - drawer.onClose(); + const handleNewDashboard = (upsert: Resource) => { + // Navigation for new dashboards const url = locationUtil.assureBaseUrl( getDashboardUrl({ uid: upsert.metadata.name, @@ -86,34 +76,50 @@ export function SaveProvisionedDashboardForm({ currentQueryParams: window.location.search, }) ); - navigate(url); }; - const onBranchSuccess = (ref: string, path: string) => { + const onWriteSuccess = (_: ProvisionedOperationInfo, upsert: Resource) => { + if (isNew && upsert?.metadata.name) { + handleNewDashboard(upsert); + } else { + locationService.partial({ + viewPanel: null, + editPanel: null, + }); + } + }; + + const onBranchSuccess = (ref: string, path: string, info: ProvisionedOperationInfo, upsert: Resource) => { + if (isNew && upsert?.metadata?.name) { + handleNewDashboard(upsert); + } else { + const url = buildResourceBranchRedirectUrl({ + baseUrl: `${PROVISIONING_URL}/${defaultValues.repo}/dashboard/preview/${path}`, + paramName: 'ref', + paramValue: ref, + repoType: info.repoType, + }); + navigate(url); + } + }; + + const onDismiss = () => { + dashboard.setState({ isDirty: false }); panelEditor?.onDiscard(); drawer.onClose(); - - const url = buildResourceBranchRedirectUrl({ - baseUrl: `${PROVISIONING_URL}/${defaultValues.repo}/dashboard/preview/${path}`, - paramName: 'ref', - paramValue: ref, - repoType: request.data?.repository?.type, - }); - navigate(url); }; - useProvisionedRequestHandler({ - dashboard, + useProvisionedRequestHandler({ request, workflow, + resourceType: 'dashboard', handlers: { - onBranchSuccess: ({ ref, path }) => onBranchSuccess(ref, path), + onBranchSuccess: ({ ref, path }, info, resource) => onBranchSuccess(ref, path, info, resource), onWriteSuccess, - onNewDashboardSuccess, onError: onRequestError, + onDismiss, }, - isNew, }); // Submit handler for saving the form data diff --git a/public/app/features/dashboard-scene/settings/DeleteProvisionedDashboardForm.tsx b/public/app/features/dashboard-scene/settings/DeleteProvisionedDashboardForm.tsx index 53bb342cd21..9d101b2f8c5 100644 --- a/public/app/features/dashboard-scene/settings/DeleteProvisionedDashboardForm.tsx +++ b/public/app/features/dashboard-scene/settings/DeleteProvisionedDashboardForm.tsx @@ -11,7 +11,7 @@ import { PROVISIONING_URL } from 'app/features/provisioning/constants'; import { ResourceEditFormSharedFields } from '../components/Provisioned/ResourceEditFormSharedFields'; import { ProvisionedDashboardFormData } from '../saving/shared'; import { DashboardScene } from '../scene/DashboardScene'; -import { useProvisionedRequestHandler } from '../utils/useProvisionedRequestHandler'; +import { useProvisionedRequestHandler, ProvisionedOperationInfo } from '../utils/useProvisionedRequestHandler'; import { buildResourceBranchRedirectUrl } from './utils'; @@ -67,7 +67,7 @@ export function DeleteProvisionedDashboardForm({ const navigate = useNavigate(); - const onRequestError = (error: unknown) => { + const onError = (error: unknown) => { getAppEvents().publish({ type: AppEvents.alertError.name, payload: [t('dashboard-scene.delete-provisioned-dashboard-form.api-error', 'Failed to delete dashboard'), error], @@ -75,32 +75,36 @@ export function DeleteProvisionedDashboardForm({ }; const onWriteSuccess = () => { + dashboard.setState({ isDirty: false }); panelEditor?.onDiscard(); - onDismiss(); // TODO reset search state instead window.location.href = '/dashboards'; }; - const onBranchSuccess = (path: string, urls?: Record) => { + const onBranchSuccess = (path: string, info: ProvisionedOperationInfo, urls?: Record) => { panelEditor?.onDiscard(); - onDismiss(); const url = buildResourceBranchRedirectUrl({ baseUrl: `${PROVISIONING_URL}/${defaultValues.repo}/dashboard/preview/${path}`, paramName: 'pull_request_url', paramValue: urls?.newPullRequestURL, - repoType: request.data?.repository?.type, + repoType: info.repoType, }); navigate(url); }; useProvisionedRequestHandler({ - dashboard, request, workflow, + resourceType: 'dashboard', + successMessage: t( + 'dashboard-scene.delete-provisioned-dashboard-form.success-message', + 'Dashboard deleted successfully' + ), handlers: { - onBranchSuccess: ({ path, urls }) => onBranchSuccess(path, urls), + onDismiss, + onBranchSuccess: ({ path, urls }, info) => onBranchSuccess(path, info, urls), onWriteSuccess, - onError: onRequestError, + onError, }, }); diff --git a/public/app/features/dashboard-scene/settings/MoveProvisionedDashboardForm.tsx b/public/app/features/dashboard-scene/settings/MoveProvisionedDashboardForm.tsx index 8d431f3f6a1..2ad4f936309 100644 --- a/public/app/features/dashboard-scene/settings/MoveProvisionedDashboardForm.tsx +++ b/public/app/features/dashboard-scene/settings/MoveProvisionedDashboardForm.tsx @@ -18,7 +18,7 @@ import { getTargetFolderPathInRepo } from 'app/features/browse-dashboards/compon import { ResourceEditFormSharedFields } from '../components/Provisioned/ResourceEditFormSharedFields'; import { ProvisionedDashboardFormData } from '../saving/shared'; import { DashboardScene } from '../scene/DashboardScene'; -import { useProvisionedRequestHandler } from '../utils/useProvisionedRequestHandler'; +import { useProvisionedRequestHandler, ProvisionedOperationInfo } from '../utils/useProvisionedRequestHandler'; import { buildResourceBranchRedirectUrl } from './utils'; @@ -118,6 +118,7 @@ export function MoveProvisionedDashboardForm({ }; const onWriteSuccess = () => { + dashboard.setState({ isDirty: false }); panelEditor?.onDiscard(); if (targetFolderUID && targetFolderTitle) { onSuccess(targetFolderUID, targetFolderTitle); @@ -125,23 +126,40 @@ export function MoveProvisionedDashboardForm({ navigate('/dashboards'); }; - const onBranchSuccess = () => { + const onBranchSuccess = (info: ProvisionedOperationInfo) => { + dashboard.setState({ isDirty: false }); panelEditor?.onDiscard(); const url = buildResourceBranchRedirectUrl({ paramName: 'new_pull_request_url', paramValue: moveRequest?.data?.urls?.newPullRequestURL, - repoType: moveRequest?.data?.repository?.type, + repoType: info.repoType, }); navigate(url); }; + const onError = (error: unknown) => { + getAppEvents().publish({ + type: AppEvents.alertError.name, + payload: [ + t('dashboard-scene.move-provisioned-dashboard-form.alert-error-moving-dashboard', 'Error moving dashboard'), + error, + ], + }); + }; + useProvisionedRequestHandler({ - dashboard, request: moveRequest, workflow, + successMessage: t( + 'dashboard-scene.move-provisioned-dashboard-form.success-message', + 'Dashboard moved successfully' + ), + resourceType: 'dashboard', handlers: { - onBranchSuccess, + onBranchSuccess: (_, info) => onBranchSuccess(info), onWriteSuccess, + onDismiss, + onError, }, }); diff --git a/public/app/features/dashboard-scene/utils/useProvisionedRequestHandler.test.ts b/public/app/features/dashboard-scene/utils/useProvisionedRequestHandler.test.ts index 0587691ace0..d8c4f8a0a96 100644 --- a/public/app/features/dashboard-scene/utils/useProvisionedRequestHandler.test.ts +++ b/public/app/features/dashboard-scene/utils/useProvisionedRequestHandler.test.ts @@ -3,18 +3,10 @@ import { renderHook } from '@testing-library/react'; import { AppEvents } from '@grafana/data'; import { getAppEvents } from '@grafana/runtime'; import { Dashboard } from '@grafana/schema'; -import { - DeleteRepositoryFilesWithPathApiResponse, - GetRepositoryFilesWithPathApiResponse, - ResourceWrapper, -} from 'app/api/clients/provisioning/v0alpha1'; -import { Resource } from 'app/features/apiserver/types'; +import { ResourceWrapper } from 'app/api/clients/provisioning/v0alpha1'; -import { DashboardScene } from '../scene/DashboardScene'; +import { useProvisionedRequestHandler, RequestHandlers } from './useProvisionedRequestHandler'; -import { useProvisionedRequestHandler } from './useProvisionedRequestHandler'; - -// Mock dependencies jest.mock('@grafana/runtime', () => ({ getAppEvents: jest.fn(), })); @@ -30,9 +22,9 @@ describe('useProvisionedRequestHandler', () => { jest.clearAllMocks(); }); - describe('when request has an error', () => { - it('should call onError handler', () => { - const { request, handlers, dashboard } = setup({ + describe('error handling', () => { + it('should call onError handler with correct parameters', () => { + const { request, handlers } = setup({ requestOverrides: { isError: true, isSuccess: false, @@ -42,264 +34,127 @@ describe('useProvisionedRequestHandler', () => { renderHook(() => useProvisionedRequestHandler({ - dashboard, request, + repository: { + type: 'github', + name: 'test-repo', + target: 'folder', + title: 'Test Repository', + workflows: [], + }, + resourceType: 'dashboard', handlers, }) ); - expect(handlers.onError).toHaveBeenCalledWith(new Error('Test error')); + expect(handlers.onError).toHaveBeenCalledWith( + new Error('Test error'), + expect.objectContaining({ + resourceType: 'dashboard', + repoType: 'github', + }) + ); expect(handlers.onBranchSuccess).not.toHaveBeenCalled(); expect(handlers.onWriteSuccess).not.toHaveBeenCalled(); - expect(handlers.onNewDashboardSuccess).not.toHaveBeenCalled(); }); }); - describe('when request is successful', () => { - it('should set dashboard isDirty to false', () => { - const { request, handlers, dashboard } = setup({ + describe('success handling', () => { + it('should publish success event and call onDismiss', () => { + const { request, handlers, mockPublish } = setup({ requestOverrides: { isError: false, isSuccess: true, - data: { - ref: 'main', - path: '/path/to/dashboard', - }, - }, - workflowOverride: 'branch', - }); - - renderHook(() => - useProvisionedRequestHandler({ - dashboard, - request, - workflow: 'branch', - handlers, - }) - ); - - expect(dashboard.setState).toHaveBeenCalledWith({ isDirty: false }); - }); - - it('should publish success event', () => { - const { request, handlers, dashboard, mockPublish } = setup({ - requestOverrides: { - isError: false, - isSuccess: true, - data: {}, + data: createMockResourceWrapper(), }, }); renderHook(() => useProvisionedRequestHandler({ - dashboard, request, + resourceType: 'dashboard', handlers, }) ); expect(mockPublish).toHaveBeenCalledWith({ type: AppEvents.alertSuccess.name, - payload: ['Dashboard changes saved successfully'], + payload: ['Dashboard saved successfully'], }); + expect(handlers.onDismiss).toHaveBeenCalled(); }); - describe('branch workflow', () => { - it('should call onBranchSuccess when workflow is branch and data has ref and path', () => { - const { request, handlers, dashboard } = setup({ - requestOverrides: { - isError: false, - isSuccess: true, - data: { - ref: 'feature-branch', - path: '/path/to/dashboard.json', - urls: { compareURL: 'http://example.com/edit' }, - }, - }, - workflowOverride: 'branch', - }); + it('should call onBranchSuccess for branch workflow', () => { + const { request, handlers } = setup({ + requestOverrides: { + isError: false, + isSuccess: true, + data: createMockResourceWrapper({ + ref: 'feature-branch', + path: '/path/to/dashboard.json', + urls: { compareURL: 'http://example.com/edit' }, + }), + }, + }); - renderHook(() => - useProvisionedRequestHandler({ - dashboard, - request, - workflow: 'branch', - handlers, - }) - ); + renderHook(() => + useProvisionedRequestHandler({ + request, + workflow: 'branch', + resourceType: 'dashboard', + handlers, + }) + ); - expect(handlers.onBranchSuccess).toHaveBeenCalledWith({ + expect(handlers.onBranchSuccess).toHaveBeenCalledWith( + { ref: 'feature-branch', path: '/path/to/dashboard.json', urls: { compareURL: 'http://example.com/edit' }, - }); - expect(handlers.onWriteSuccess).not.toHaveBeenCalled(); - }); - - it('should not call onBranchSuccess when ref is missing', () => { - const { request, handlers, dashboard } = setup({ - requestOverrides: { - isError: false, - isSuccess: true, - data: { - path: '/path/to/dashboard.json', - }, - }, - workflowOverride: 'branch', - }); - - renderHook(() => - useProvisionedRequestHandler({ - dashboard, - request, - workflow: 'branch', - handlers, - }) - ); - - expect(handlers.onBranchSuccess).not.toHaveBeenCalled(); - expect(handlers.onWriteSuccess).toHaveBeenCalled(); - }); + }, + expect.objectContaining({ + resourceType: 'dashboard', + repoType: 'git', + workflow: 'branch', + }), + expect.any(Object) + ); + expect(handlers.onWriteSuccess).not.toHaveBeenCalled(); }); - describe('new dashboard flow', () => { - it('should call onNewDashboardSuccess when isNew is true and resource.upsert exists', () => { - const mockUpsertResource = { - metadata: { - name: 'test-dashboard', - uid: 'test-uid', - resourceVersion: '1', - creationTimestamp: new Date().toISOString(), - }, - spec: { title: 'Test Dashboard' } as Dashboard, - apiVersion: 'v1', - kind: 'Dashboard', - }; - - const mockResource = { - metadata: { - name: 'test-dashboard', - uid: 'test-uid', - resourceVersion: '1', - creationTimestamp: new Date().toISOString(), - }, - spec: { title: 'Test Dashboard' } as Dashboard, - apiVersion: 'v1', - kind: 'Dashboard', - upsert: mockUpsertResource, - } as Resource & { upsert: Resource }; - - const { request, handlers, dashboard } = setup({ - requestOverrides: { - isError: false, - isSuccess: true, - data: { - repository: 'test-repo', - resource: mockResource, - } as unknown as ProvisionedRequestData, - }, - }); - - renderHook(() => - useProvisionedRequestHandler({ - dashboard, - request, - handlers, - isNew: true, - }) - ); - - expect(handlers.onNewDashboardSuccess).toHaveBeenCalledWith(mockResource.upsert); - expect(handlers.onWriteSuccess).not.toHaveBeenCalled(); + it('should call onWriteSuccess for write workflow', () => { + const { request, handlers } = setup({ + requestOverrides: { + isError: false, + isSuccess: true, + data: createMockResourceWrapper(), + }, }); - it('should not call onNewDashboardSuccess when isNew is false', () => { - const { request, handlers, dashboard } = setup({ - requestOverrides: { - isError: false, - isSuccess: true, - data: { - repository: 'test-repo', - resource: { - upsert: { - apiVersion: 'v1', - kind: 'Dashboard', - metadata: { name: 'test-dashboard' }, - spec: { title: 'Test Dashboard' } as Dashboard, - }, - metadata: { name: 'test-dashboard' }, - spec: { title: 'Test Dashboard' } as Dashboard, - apiVersion: 'v1', - kind: 'Dashboard', - } as unknown as Resource, - } as unknown as ProvisionedRequestData, - }, - }); + renderHook(() => + useProvisionedRequestHandler({ + request, + workflow: 'write', + resourceType: 'dashboard', + handlers, + }) + ); - renderHook(() => - useProvisionedRequestHandler({ - dashboard, - request, - handlers, - isNew: false, - }) - ); - - expect(handlers.onNewDashboardSuccess).not.toHaveBeenCalled(); - expect(handlers.onWriteSuccess).toHaveBeenCalled(); - }); - - it('should not call onNewDashboardSuccess when resource.upsert is missing', () => { - const { request, handlers, dashboard } = setup({ - requestOverrides: { - isError: false, - isSuccess: true, - data: { - resource: {}, - } as ResourceWrapper, - }, - }); - - renderHook(() => - useProvisionedRequestHandler({ - dashboard, - request, - handlers, - isNew: true, - }) - ); - - expect(handlers.onNewDashboardSuccess).not.toHaveBeenCalled(); - expect(handlers.onWriteSuccess).toHaveBeenCalled(); - }); - }); - - describe('write workflow', () => { - it('should call onWriteSuccess as fallback', () => { - const { request, handlers, dashboard } = setup({ - requestOverrides: { - isError: false, - isSuccess: true, - data: {} as GetRepositoryFilesWithPathApiResponse, - }, - }); - - renderHook(() => - useProvisionedRequestHandler({ - dashboard, - request, - handlers, - }) - ); - - expect(handlers.onWriteSuccess).toHaveBeenCalled(); - }); + expect(handlers.onWriteSuccess).toHaveBeenCalledWith( + expect.objectContaining({ + resourceType: 'dashboard', + repoType: 'git', + workflow: 'write', + }), + expect.any(Object) + ); + expect(handlers.onDismiss).toHaveBeenCalled(); }); }); - describe('when request is neither error nor success', () => { - it('should not call any handlers', () => { - const { request, handlers, dashboard, mockPublish } = setup({ + describe('edge cases', () => { + it('should not call any handlers when request is loading', () => { + const { request, handlers, mockPublish } = setup({ requestOverrides: { isError: false, isSuccess: false, @@ -309,7 +164,6 @@ describe('useProvisionedRequestHandler', () => { renderHook(() => useProvisionedRequestHandler({ - dashboard, request, handlers, }) @@ -318,15 +172,11 @@ describe('useProvisionedRequestHandler', () => { expect(handlers.onError).not.toHaveBeenCalled(); expect(handlers.onBranchSuccess).not.toHaveBeenCalled(); expect(handlers.onWriteSuccess).not.toHaveBeenCalled(); - expect(handlers.onNewDashboardSuccess).not.toHaveBeenCalled(); - expect(dashboard.setState).not.toHaveBeenCalled(); expect(mockPublish).not.toHaveBeenCalled(); }); - }); - describe('when request success but no data', () => { - it('should not call any handlers when data is undefined', () => { - const { request, handlers, dashboard, mockPublish } = setup({ + it('should not call handlers when success but no data', () => { + const { request, handlers, mockPublish } = setup({ requestOverrides: { isError: false, isSuccess: true, @@ -336,7 +186,6 @@ describe('useProvisionedRequestHandler', () => { renderHook(() => useProvisionedRequestHandler({ - dashboard, request, handlers, }) @@ -344,25 +193,18 @@ describe('useProvisionedRequestHandler', () => { expect(handlers.onWriteSuccess).not.toHaveBeenCalled(); expect(handlers.onBranchSuccess).not.toHaveBeenCalled(); - expect(dashboard.setState).not.toHaveBeenCalled(); expect(mockPublish).not.toHaveBeenCalled(); }); - }); - describe('optional handlers', () => { it('should not throw when optional handlers are not provided', () => { - const { request, dashboard } = setup({ - requestOverrides: { - isError: false, - isSuccess: true, - }, + const { request } = setup({ + requestOverrides: { isError: false, isSuccess: true }, handlersOverrides: {}, }); expect(() => { renderHook(() => useProvisionedRequestHandler({ - dashboard, request, handlers: {}, }) @@ -372,62 +214,69 @@ describe('useProvisionedRequestHandler', () => { }); }); -type ProvisionedRequestData = DeleteRepositoryFilesWithPathApiResponse | GetRepositoryFilesWithPathApiResponse; +// Helper function to create a properly structured mock ResourceWrapper +function createMockResourceWrapper(overrides: Partial = {}): ResourceWrapper { + return { + repository: { + name: 'test-repo', + namespace: 'default', + title: 'Test Repository', + type: 'git', + }, + resource: { + type: { + kind: 'Dashboard', + }, + upsert: { + apiVersion: 'v1', + kind: 'Dashboard', + metadata: { name: 'test-dashboard', uid: 'test-uid' }, + spec: { title: 'Test Dashboard' }, + }, + }, + ...overrides, + }; +} function setup({ requestOverrides = {}, handlersOverrides = {}, - workflowOverride, }: { requestOverrides?: Partial<{ isError: boolean; isSuccess: boolean; isLoading?: boolean; error?: unknown; - data?: Partial; + data?: ResourceWrapper; }>; - handlersOverrides?: Partial<{ - onBranchSuccess?: jest.Mock; - onWriteSuccess?: jest.Mock; - onNewDashboardSuccess?: jest.Mock; - onError?: jest.Mock; - }>; - workflowOverride?: string; + handlersOverrides?: Partial>; } = {}) { const mockPublish = jest.fn(); - const mockSetState = jest.fn(); mockGetAppEvents.mockReturnValue({ publish: mockPublish, } as unknown as ReturnType); - const dashboard = { - setState: mockSetState, - } as unknown as DashboardScene; - const request = { isError: false, isSuccess: false, isLoading: false, error: undefined, data: undefined, - ...(requestOverrides as ResourceWrapper), + ...requestOverrides, }; - const handlers = { + const handlers: RequestHandlers = { onError: jest.fn(), onBranchSuccess: jest.fn(), onWriteSuccess: jest.fn(), - onNewDashboardSuccess: jest.fn(), + onDismiss: jest.fn(), ...handlersOverrides, }; return { - dashboard, request, handlers, mockPublish, - mockSetState, - workflow: workflowOverride, }; } diff --git a/public/app/features/dashboard-scene/utils/useProvisionedRequestHandler.ts b/public/app/features/dashboard-scene/utils/useProvisionedRequestHandler.ts index d5b9ca4ba7b..45fb3f18b79 100644 --- a/public/app/features/dashboard-scene/utils/useProvisionedRequestHandler.ts +++ b/public/app/features/dashboard-scene/utils/useProvisionedRequestHandler.ts @@ -3,20 +3,32 @@ import { useEffect } from 'react'; import { AppEvents } from '@grafana/data'; import { t } from '@grafana/i18n'; import { getAppEvents } from '@grafana/runtime'; -import { Dashboard } from '@grafana/schema'; import { DeleteRepositoryFilesWithPathApiResponse, GetRepositoryFilesWithPathApiResponse, + RepositoryView, } from 'app/api/clients/provisioning/v0alpha1'; import { Resource } from 'app/features/apiserver/types'; +import { RepoType } from 'app/features/provisioning/Wizard/types'; -import { DashboardScene } from '../scene/DashboardScene'; +type ResourceType = 'dashboard' | 'folder'; // Add more as needed, e.g., 'alert', etc. -interface RequestHandlers { - onBranchSuccess?: (data: { ref: string; path: string; urls?: Record }) => void; - onWriteSuccess?: () => void; - onNewDashboardSuccess?: (resource: Resource) => void; - onError?: (error: unknown) => void; +// Information object that gets passed to all handlers +interface ProvisionedOperationInfo { + repoType: RepoType; + resourceType?: ResourceType; + workflow?: string; +} + +interface RequestHandlers { + onBranchSuccess?: ( + data: { ref: string; path: string; urls?: Record }, + info: ProvisionedOperationInfo, + resource: Resource + ) => void; + onWriteSuccess?: (info: ProvisionedOperationInfo, resource: Resource) => void; + onError?: (error: unknown, info: ProvisionedOperationInfo) => void; + onDismiss?: () => void; } interface ProvisionedRequest { @@ -27,51 +39,85 @@ interface ProvisionedRequest { data?: DeleteRepositoryFilesWithPathApiResponse | GetRepositoryFilesWithPathApiResponse; } -// This hook handles save new dashboard, edit existing dashboard, and delete dashboard response logic for provisioned dashboards. -export function useProvisionedRequestHandler({ - dashboard, +// Resource-specific configuration for different resource types +interface ResourceConfig { + defaultSuccessMessage: string; + supportedWorkflows: string[]; +} + +/** + * Generic hook for handling provisioned resource operations across any resource type and repository provider. + * + * This hook is intentionally decoupled from specific components (like DashboardScene) to promote reusability. + * Components are responsible for their own state management through specific workflow handlers. + */ +export function useProvisionedRequestHandler({ request, workflow, handlers, - isNew, + successMessage, + repository, + resourceType, }: { - dashboard: DashboardScene; request: ProvisionedRequest; workflow?: string; - handlers: RequestHandlers; - isNew?: boolean; + handlers: RequestHandlers; + successMessage?: string; + repository?: RepositoryView; + resourceType?: ResourceType; }) { useEffect(() => { + const repoType = repository?.type || 'git'; + const info: ProvisionedOperationInfo = { + repoType, + resourceType, + workflow, + }; + if (request.isError) { - handlers.onError?.(request.error); + handlers.onError?.(request.error, info); return; } if (request.isSuccess && request.data) { - dashboard.setState({ isDirty: false }); const { ref, path, urls, resource } = request.data; + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + const resourceData = resource.upsert as Resource; - // Branch workflow - if (workflow === 'branch' && ref && path) { - handlers.onBranchSuccess?.({ ref, path, urls }); - return; - } - - // Success message (could be configurable) + // Success message + const message = successMessage || getContextualSuccessMessage(info); getAppEvents().publish({ type: AppEvents.alertSuccess.name, - payload: [t('dashboard-scene.edit-provisioned-dashboard-form.success', 'Dashboard changes saved successfully')], + payload: [message], }); - // New dashboard flow - if (isNew && resource?.upsert && handlers.onNewDashboardSuccess) { - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - handlers.onNewDashboardSuccess(resource.upsert as Resource); - return; + // Branch workflow + if (workflow === 'branch' && handlers.onBranchSuccess && ref && path) { + const branchData = { ref, path, urls }; + handlers.onBranchSuccess?.(branchData, info, resourceData); } // Write workflow - handlers.onWriteSuccess?.(); + if (workflow === 'write' && handlers.onWriteSuccess) { + handlers.onWriteSuccess(info, resourceData); + } + + handlers.onDismiss?.(); } - }, [request, workflow, handlers, isNew, dashboard]); + }, [request, workflow, handlers, successMessage, repository, resourceType]); } + +function getContextualSuccessMessage(info: ProvisionedOperationInfo): string { + const { resourceType } = info; + + switch (resourceType) { + case 'dashboard': + return t('provisioned-resource-request-handler-dashboard', 'Dashboard saved successfully'); + case 'folder': + return t('provisioned-resource-request-handler-folder', 'Folder created successfully'); + default: + return t('provisioned-resource-request-handler', 'Resource saved successfully'); + } +} + +export type { ResourceType, ProvisionedOperationInfo, RequestHandlers, ResourceConfig }; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 40ccbf5b567..dd3d5609f99 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -3563,12 +3563,12 @@ "tags-column": "Tags" }, "delete-provisioned-folder-form": { - "alert-folder-deleted-successfully": "Folder deleted successfully", "api-error": "Failed to delete folder", "button-cancel": "Cancel", "button-delete": "Delete", "button-deleting": "Deleting...", - "delete-warning": "This will delete this folder and all its descendants. In total, this will affect:" + "delete-warning": "This will delete this folder and all its descendants. In total, this will affect:", + "success-message": "Folder deleted successfully" }, "descendant-count": { "title-unable-to-retrieve-descendant-information": "Unable to retrieve descendant information" @@ -3611,7 +3611,6 @@ }, "new-provisioned-folder-form": { "alert-error-creating-folder": "Error creating folder", - "alert-folder-created-successfully": "Folder created successfully", "button-create": "Create", "button-creating": "Creating...", "cancel": "Cancel", @@ -5645,6 +5644,7 @@ "delete-read-only-file-message": "This dashboard cannot be deleted directly from Grafana because the repository is read-only. To delete this dashboard, please remove the file from your Git repository.", "deleting": "Deleting...", "drawer-title": "Delete Provisioned Dashboard", + "success-message": "Dashboard deleted successfully", "title-this-repository-is-read-only": "This repository is read only" }, "description-label": { @@ -5657,9 +5657,6 @@ } } }, - "edit-provisioned-dashboard-form": { - "success": "Dashboard changes saved successfully" - }, "email-list": { "aria-label-emailmenu": "Toggle email menu" }, @@ -5818,6 +5815,7 @@ "usage-count_other": "Used on {{count}} dashboards" }, "move-provisioned-dashboard-form": { + "alert-error-moving-dashboard": "Error moving dashboard", "api-error": "Failed to move dashboard", "cancel-action": "Cancel", "current-file-not-found": "Current dashboard file could not be found", @@ -5827,6 +5825,7 @@ "move-action": "Move dashboard", "move-read-only-message": "This dashboard cannot be moved directly from Grafana because the repository is read-only. To move this dashboard, please move the file in your Git repository.", "moving": "Moving...", + "success-message": "Dashboard moved successfully", "target-path-label": "Target path", "title-this-repository-is-read-only": "This repository is read only" }, @@ -11015,6 +11014,9 @@ "title-created-branch-in-repo": "A new resource has been created in a branch in {{repoType}}.", "title-loaded-pull-request-in-repo": "This resource is loaded from the branch you just created in {{repoType}} and it is only visible to you" }, + "provisioned-resource-request-handler": "Resource saved successfully", + "provisioned-resource-request-handler-dashboard": "Dashboard saved successfully", + "provisioned-resource-request-handler-folder": "Folder created successfully", "provisioning": { "banner": { "message": "This feature is currently under active development. For the best experience and latest improvements, we recommend using the <2>nightly build of Grafana."