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 <dominik.prokop@grafana.com>
2025-04-08 13:23:15 +03:00
parent 762315f181
commit 11bed7b25d
3 changed files with 100 additions and 57 deletions
@@ -41,6 +41,25 @@ const setupDashboardAPI = (
  }));
 };

+const setupV1FailureV2Success = (
+  v2Response: DashboardWithAccessInfo<DashboardV2Spec> = {
+    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({
@@ -168,6 +168,11 @@ abstract class DashboardScenePageStateManagerBase<T>
          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<T>
          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<T>
  ): Promise<T> {
    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<void> {
+    return this.withVersionHandling((manager) => manager.loadDashboard.call(this, options));
+  }
 }

 const managers: {
@@ -180,7 +180,7 @@ export function transformSaveModelSchemaV2ToScene(dto: DashboardWithAccessInfo<D
        preserveDashboardSceneStateInLocalStorage,
        addPanelsOnLoadBehavior,
        new DashboardReloadBehavior({
-          reloadOnParamsChange: config.featureToggles.reloadDashboardsOnParamsChange,
+          reloadOnParamsChange: config.featureToggles.reloadDashboardsOnParamsChange && false,
          uid: dashboardId?.toString(),
          version: 1,
        }),