From 8d17c2200627295e0f168422922c827e5ee2e27c Mon Sep 17 00:00:00 2001 From: Alex Khomenko Date: Fri, 10 Oct 2025 11:10:13 +0200 Subject: [PATCH] Restore dashboards: Surface restore error details (#112219) * Restore dashboards: Better surface error messages * Fix resource check * Fix folder picker font size * Add smoke tests * cleanup * Only show valid resources --- public/app/features/apiserver/guards.ts | 2 - .../RecentlyDeletedActions.test.tsx | 120 ++++++++++++++++++ .../components/RecentlyDeletedActions.tsx | 46 ++++++- .../components/RestoreModal.tsx | 7 +- .../dashboard/api/UnifiedDashboardAPI.ts | 3 +- 5 files changed, 166 insertions(+), 12 deletions(-) create mode 100644 public/app/features/browse-dashboards/components/RecentlyDeletedActions.test.tsx diff --git a/public/app/features/apiserver/guards.ts b/public/app/features/apiserver/guards.ts index a43a0734dd5..21022195360 100644 --- a/public/app/features/apiserver/guards.ts +++ b/public/app/features/apiserver/guards.ts @@ -21,8 +21,6 @@ export function isResource(value: unknown): } return ( - typeof value.apiVersion === 'string' && - typeof value.kind === 'string' && typeof metadata.name === 'string' && typeof metadata.resourceVersion === 'string' && typeof metadata.creationTimestamp === 'string' && diff --git a/public/app/features/browse-dashboards/components/RecentlyDeletedActions.test.tsx b/public/app/features/browse-dashboards/components/RecentlyDeletedActions.test.tsx new file mode 100644 index 00000000000..cc1646a1282 --- /dev/null +++ b/public/app/features/browse-dashboards/components/RecentlyDeletedActions.test.tsx @@ -0,0 +1,120 @@ +import { render, screen } from 'test/test-utils'; + +import { DataFrameView, FieldType, toDataFrame } from '@grafana/data'; +import { setBackendSrv } from '@grafana/runtime'; +import { setupMockServer } from '@grafana/test-utils/server'; +import { backendSrv } from 'app/core/services/backend_srv'; + +import { deletedDashboardsCache } from '../../search/service/deletedDashboardsCache'; +import { DashboardQueryResult } from '../../search/service/types'; +import { SearchLayout, EventTrackingNamespace } from '../../search/types'; +import { TrashStateManager, useRecentlyDeletedStateManager } from '../api/useRecentlyDeletedStateManager'; +import { useActionSelectionState } from '../state/hooks'; + +import { RecentlyDeletedActions } from './RecentlyDeletedActions'; + +jest.mock('../api/useRecentlyDeletedStateManager'); +jest.mock('../state/hooks'); +jest.mock('../../search/service/deletedDashboardsCache'); + +setBackendSrv(backendSrv); +setupMockServer(); + +const mockUseRecentlyDeletedStateManager = useRecentlyDeletedStateManager as jest.MockedFunction< + typeof useRecentlyDeletedStateManager +>; +const mockUseActionSelectionState = useActionSelectionState as jest.MockedFunction; + +describe('RecentlyDeletedActions', () => { + const mockDoSearchWithDebounce = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + + const mockDataFrame = toDataFrame({ + name: 'DeletedDashboards', + fields: [ + { name: 'kind', type: FieldType.string, config: {}, values: ['dashboard'] }, + { name: 'name', type: FieldType.string, config: {}, values: ['Test Dashboard'] }, + { name: 'uid', type: FieldType.string, config: {}, values: ['dashboard-1'] }, + { name: 'url', type: FieldType.string, config: {}, values: ['/d/dashboard-1'] }, + { name: 'panel_type', type: FieldType.string, config: {}, values: [''] }, + { name: 'tags', type: FieldType.other, config: {}, values: [[]] }, + { name: 'location', type: FieldType.string, config: {}, values: ['folder-1'] }, + { name: 'ds_uid', type: FieldType.other, config: {}, values: [[]] }, + { name: 'score', type: FieldType.number, config: {}, values: [0] }, + { name: 'explain', type: FieldType.other, config: {}, values: [{}] }, + ], + }); + + const mockView = new DataFrameView(mockDataFrame); + + const mockStateManager = { + doSearchWithDebounce: mockDoSearchWithDebounce, + state: { + query: '', + tag: [], + starred: false, + layout: SearchLayout.List, + deleted: true, + eventTrackingNamespace: 'manage_dashboards' as EventTrackingNamespace, + result: { + view: mockView, + }, + }, + } as unknown as TrashStateManager; + + mockUseRecentlyDeletedStateManager.mockReturnValue([ + { + query: '', + tag: [], + starred: false, + layout: SearchLayout.List, + deleted: true, + eventTrackingNamespace: 'manage_dashboards', + result: { + view: mockView, + loadMoreItems: function (startIndex: number, stopIndex: number): Promise { + return Promise.resolve(); + }, + isItemLoaded: function (index: number) { + return true; + }, + totalRows: 0, + }, + }, + mockStateManager, + ]); + + (deletedDashboardsCache.clear as jest.Mock) = jest.fn(); + (deletedDashboardsCache.getAsResourceList as jest.Mock) = jest.fn().mockResolvedValue({ + items: [ + { + metadata: { name: 'dashboard-1' }, + }, + ], + }); + }); + + it('renders restore button', () => { + mockUseActionSelectionState.mockReturnValue({ + dashboard: {}, + folder: {}, + }); + + render(); + + expect(screen.getByRole('button', { name: 'Restore' })).toBeInTheDocument(); + }); + + it('restore button is visible when dashboards are selected', () => { + mockUseActionSelectionState.mockReturnValue({ + dashboard: { 'dashboard-1': true }, + folder: {}, + }); + + render(); + + expect(screen.getByRole('button', { name: 'Restore' })).toBeInTheDocument(); + }); +}); diff --git a/public/app/features/browse-dashboards/components/RecentlyDeletedActions.tsx b/public/app/features/browse-dashboards/components/RecentlyDeletedActions.tsx index ba25904d43a..aac486a75ff 100644 --- a/public/app/features/browse-dashboards/components/RecentlyDeletedActions.tsx +++ b/public/app/features/browse-dashboards/components/RecentlyDeletedActions.tsx @@ -25,8 +25,9 @@ export function RecentlyDeletedActions() { const [isBulkRestoreLoading, setIsBulkRestoreLoading] = useState(false); const [isRestoreModalOpen, setIsRestoreModalOpen] = useState(false); - const showRestoreNotifications = (successful: string[], failedCount: number) => { + const showRestoreNotifications = (successful: string[], failed: Array<{ uid: string; error: string }>) => { const successCount = successful.length; + const failedCount = failed.length; if (successCount === 0 && failedCount === 0) { return; @@ -36,6 +37,8 @@ export function RecentlyDeletedActions() { let message = t('browse-dashboards.restore.success', 'Dashboards restored successfully'); if (failedCount > 0) { + const firstError = failed[0]?.error; + if (successCount > 0) { // Partial success alertType = AppEvents.alertWarning.name; @@ -48,12 +51,18 @@ export function RecentlyDeletedActions() { count: failedCount, }); message = `${successMessage}. ${failedMessage}.`; + if (firstError) { + message += `. ${firstError}`; + } } else { // All failed alertType = AppEvents.alertError.name; - message = t('browse-dashboards.restore.all-failed', 'Failed to restore {{count}} dashboard', { + message = t('browse-dashboards.restore.all-failed', 'Failed to restore {{count}} dashboard.', { count: failedCount, }); + if (firstError) { + message += `. ${firstError}`; + } } } @@ -95,6 +104,22 @@ export function RecentlyDeletedActions() { stateManager.doSearchWithDebounce(); }; + const getErrorMessage = (error: unknown) => { + if (error instanceof Error) { + return error.message; + } + if (typeof error === 'string') { + return error; + } + if (error && typeof error === 'object' && 'message' in error) { + return String(error.message); + } + if (error) { + return JSON.stringify(error); + } + return ''; + }; + const onRestore = async (restoreTarget: string) => { const resultsView = stateManager.state.result?.view.toArray(); if (!resultsView) { @@ -124,20 +149,27 @@ export function RecentlyDeletedActions() { // Separate successful and failed restores const successful: string[] = []; - const failed: string[] = []; + const failed: Array<{ uid: string; error: string }> = []; results.forEach((result, index) => { const dashboardUid = selectedDashboards[index]; - if (result.status === 'rejected' || result.value.error) { - failed.push(dashboardUid); + if (result.status === 'rejected') { + const errorMessage = getErrorMessage(result.reason); + if (errorMessage) { + failed.push({ uid: dashboardUid, error: errorMessage }); + } + } else if (result.value.error) { + const errorMessage = getErrorMessage(result.value.error); + if (errorMessage) { + failed.push({ uid: dashboardUid, error: errorMessage }); + } } else if ('data' in result.value && result.value.data?.name) { successful.push(result.value.data.name); } }); // Show consolidated notification - const failedCount = failed.length; - showRestoreNotifications(successful, failedCount); + showRestoreNotifications(successful, failed); const parentUIDs = new Set(); for (const uid of selectedDashboards) { diff --git a/public/app/features/browse-dashboards/components/RestoreModal.tsx b/public/app/features/browse-dashboards/components/RestoreModal.tsx index ebf5443e841..8801f3b9946 100644 --- a/public/app/features/browse-dashboards/components/RestoreModal.tsx +++ b/public/app/features/browse-dashboards/components/RestoreModal.tsx @@ -2,7 +2,7 @@ import { useState } from 'react'; import { Trans, t } from '@grafana/i18n'; import { reportInteraction } from '@grafana/runtime'; -import { ConfirmModal, Space, Text } from '@grafana/ui'; +import { ConfirmModal, Field, Space, Text } from '@grafana/ui'; import { FolderPicker } from '../../../core/components/Select/FolderPicker'; @@ -61,7 +61,10 @@ export const RestoreModal = ({ - + {/* Field wrapper resets font-size to 14px, preventing cascade from parent Text components */} + + + } confirmText={ diff --git a/public/app/features/dashboard/api/UnifiedDashboardAPI.ts b/public/app/features/dashboard/api/UnifiedDashboardAPI.ts index 613403420e5..5e63cedb0f5 100644 --- a/public/app/features/dashboard/api/UnifiedDashboardAPI.ts +++ b/public/app/features/dashboard/api/UnifiedDashboardAPI.ts @@ -80,7 +80,8 @@ export class UnifiedDashboardAPI return { ...v2Response, - items: [...filteredV1Items, ...filteredV2Items], + // Make sure we display only valid resources + items: [...filteredV1Items, ...filteredV2Items].filter(isResource), }; }