From 11bed7b25d989ef7ccc09518bff31cb0dd138dc3 Mon Sep 17 00:00:00 2001 From: Ivan Ortega Alba Date: Tue, 8 Apr 2025 12:23:15 +0200 Subject: [PATCH] ReloadDashboard: Unified manager should delegate to the right manager (#103329) * ReloadDashboard: Unified manager should delegate to the right manager * Throw only when dashboard version error is detected --------- Co-authored-by: Dominik Prokop --- .../DashboardScenePageStateManager.test.ts | 124 +++++++++++------- .../pages/DashboardScenePageStateManager.ts | 31 ++++- .../transformSaveModelSchemaV2ToScene.ts | 2 +- 3 files changed, 100 insertions(+), 57 deletions(-) diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts index cf806916a28..7bdaaee666a 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts +++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts @@ -41,6 +41,25 @@ const setupDashboardAPI = ( })); }; +const setupV1FailureV2Success = ( + v2Response: DashboardWithAccessInfo = { + access: {}, + apiVersion: 'v2alpha1', + kind: 'DashboardWithAccessInfo', + metadata: { + name: 'fake-dash', + creationTimestamp: '', + resourceVersion: '1', + }, + spec: { ...defaultDashboardV2Spec() }, + } +) => { + const getDashSpy = jest.fn(); + setupLoadDashboardMockReject(new DashboardVersionError('v2alpha1')); + setupDashboardAPI(v2Response, getDashSpy); + return getDashSpy; +}; + describe('DashboardScenePageStateManager v1', () => { afterEach(() => { store.delete(DASHBOARD_FROM_LS_KEY); @@ -84,6 +103,7 @@ describe('DashboardScenePageStateManager v1', () => { }); const loader = new DashboardScenePageStateManager({}); + await loader.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal }); expect(loader.state.dashboard).toBeUndefined(); @@ -272,7 +292,12 @@ describe('DashboardScenePageStateManager v2', () => { }); const loader = new DashboardScenePageStateManagerV2({}); - await loader.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal }); + try { + await loader.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal }); + } catch (e) { + expect(e).toBeInstanceOf(Error); + expect((e as Error).message).toBe('Dashhboard not found'); + } // expect(loader.state.dashboard).toBeUndefined(); expect(loader.state.isLoading).toBe(false); @@ -580,23 +605,7 @@ describe('UnifiedDashboardScenePageStateManager', () => { }); it('should switch to v2 manager when loading v2 dashboard', async () => { - setupLoadDashboardMockReject(new DashboardVersionError('v2alpha1')); - - const getDashSpy = jest.fn(); - setupDashboardAPI( - { - access: {}, - apiVersion: 'v2alpha1', - kind: 'DashboardWithAccessInfo', - metadata: { - name: 'fake-dash', - creationTimestamp: '', - resourceVersion: '1', - }, - spec: { ...defaultDashboardV2Spec() }, - }, - getDashSpy - ); + const getDashSpy = setupV1FailureV2Success(); const manager = new UnifiedDashboardScenePageStateManager({}); await manager.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal }); @@ -606,22 +615,7 @@ describe('UnifiedDashboardScenePageStateManager', () => { }); it('should maintain active manager state between operations', async () => { - const getDashSpy = jest.fn(); - setupLoadDashboardMockReject(new DashboardVersionError('v2alpha1')); - setupDashboardAPI( - { - access: {}, - apiVersion: 'v2alpha1', - kind: 'DashboardWithAccessInfo', - metadata: { - name: 'fake-dash', - creationTimestamp: '', - resourceVersion: '1', - }, - spec: { ...defaultDashboardV2Spec() }, - }, - getDashSpy - ); + setupV1FailureV2Success(); const manager = new UnifiedDashboardScenePageStateManager({}); @@ -637,22 +631,7 @@ describe('UnifiedDashboardScenePageStateManager', () => { it.todo('should handle snapshot loading for both v1 and v2'); it('should handle dashboard reloading with current active manager', async () => { - const getDashSpy = jest.fn(); - setupDashboardAPI( - { - access: {}, - apiVersion: 'v2alpha1', - kind: 'DashboardWithAccessInfo', - metadata: { - name: 'fake-dash', - creationTimestamp: '', - resourceVersion: '1', - }, - spec: { ...defaultDashboardV2Spec() }, - }, - getDashSpy - ); - setupLoadDashboardMockReject(new DashboardVersionError('v2alpha1')); + setupV1FailureV2Success(); const manager = new UnifiedDashboardScenePageStateManager({}); @@ -698,6 +677,51 @@ describe('UnifiedDashboardScenePageStateManager', () => { }); }); + describe('reloadDashboard', () => { + it('should reload v1 dashboard with v1 manager', async () => { + const loadDashboardMock = setupLoadDashboardMock({ dashboard: { uid: 'fake-dash', editable: true }, meta: {} }); + + const manager = new UnifiedDashboardScenePageStateManager({}); + + await manager.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal }); + + expect(loadDashboardMock).toHaveBeenCalledWith('db', '', 'fake-dash', undefined); + expect(manager['activeManager']).toBeInstanceOf(DashboardScenePageStateManager); + + loadDashboardMock.mockClear(); + + const options = { version: 2, scopes: [], timeRange: { from: 'now-1h', to: 'now' }, variables: {} }; + await manager.reloadDashboard(options); + + expect(manager['activeManager']).toBeInstanceOf(DashboardScenePageStateManager); + expect(loadDashboardMock).toHaveBeenCalledWith('db', '', 'fake-dash', { + from: 'now-1h', + to: 'now', + version: 2, + scopes: [], + }); + }); + + it('should reload v2 dashboard with v2 manager', async () => { + setupV1FailureV2Success(); + + const manager = new UnifiedDashboardScenePageStateManager({}); + await manager.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal }); + + expect(manager['activeManager']).toBeInstanceOf(DashboardScenePageStateManagerV2); + + const options = { version: 2, scopes: [], timeRange: { from: 'now-1h', to: 'now' }, variables: {} }; + try { + await manager.reloadDashboard(options); + } catch (e) { + expect(e).toBeInstanceOf(Error); + expect((e as Error).message).toBe('Method not implemented.'); + } + + expect(manager['activeManager']).toBeInstanceOf(DashboardScenePageStateManagerV2); + }); + }); + describe('Home dashboard', () => { it('should handle home dashboard redirect', async () => { setBackendSrv({ diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts index 323db8450c9..e331893052a 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts +++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts @@ -168,6 +168,11 @@ abstract class DashboardScenePageStateManagerBase messageId, }, }); + // If the error is a DashboardVersionError, we want to throw it so that the error boundary is triggered + // This enables us to switch to the correct version of the dashboard + if (err instanceof DashboardVersionError) { + throw err; + } } } @@ -210,6 +215,11 @@ abstract class DashboardScenePageStateManagerBase messageId, }, }); + // If the error is a DashboardVersionError, we want to throw it so that the error boundary is triggered + // This enables us to switch to the correct version of the dashboard + if (err instanceof DashboardVersionError) { + throw err; + } } } @@ -463,6 +473,11 @@ export class DashboardScenePageStateManager extends DashboardScenePageStateManag status, }, }); + // If the error is a DashboardVersionError, we want to throw it so that the error boundary is triggered + // This enables us to switch to the correct version of the dashboard + if (err instanceof DashboardVersionError) { + throw err; + } } } } @@ -600,11 +615,7 @@ export class UnifiedDashboardScenePageStateManager extends DashboardScenePageSta operation: (manager: DashboardScenePageStateManager | DashboardScenePageStateManagerV2) => Promise ): Promise { try { - const result = await operation(this.activeManager); - // need to sync the state of the active manager with the unified manager - // in cases when components are subscribed to unified manager's state - this.setState(this.activeManager.state); - return result; + return await operation(this.activeManager); } catch (error) { if (error instanceof DashboardVersionError) { const manager = error.data.storedVersion === 'v2alpha1' ? this.v2Manager : this.v1Manager; @@ -613,6 +624,10 @@ export class UnifiedDashboardScenePageStateManager extends DashboardScenePageSta } else { throw error; } + } finally { + // need to sync the state of the active manager with the unified manager + // in cases when components are subscribed to unified manager's state + this.setState(this.activeManager.state); } } @@ -623,7 +638,7 @@ export class UnifiedDashboardScenePageStateManager extends DashboardScenePageSta } public async reloadDashboard(params: LoadDashboardOptions['params']) { - return this.withVersionHandling((manager) => manager.reloadDashboard(params)); + return this.withVersionHandling((manager) => manager.reloadDashboard.call(this, params)); } public getDashboardFromCache(uid: string) { @@ -683,6 +698,10 @@ export class UnifiedDashboardScenePageStateManager extends DashboardScenePageSta this.v1Manager.setDashboardCache(cacheKey, dashboard); } } + + public async loadDashboard(options: LoadDashboardOptions): Promise { + return this.withVersionHandling((manager) => manager.loadDashboard.call(this, options)); + } } const managers: { diff --git a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts index 10ccc34252f..bc3ecd039d7 100644 --- a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts +++ b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts @@ -180,7 +180,7 @@ export function transformSaveModelSchemaV2ToScene(dto: DashboardWithAccessInfo