From 115dfc0fd8df61a048eca1bab12ae62afa833d62 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel@grafana.com>
Date: Fri, 2 Feb 2024 10:53:41 +0100
Subject: [PATCH] DashboardScene: Detect updated saved dashboard version and
 load new scene (#81715)

---
 .../DashboardScenePageStateManager.test.ts    | 34 +++++++++
 .../pages/DashboardScenePageStateManager.ts   | 73 +++++++++----------
 2 files changed, 69 insertions(+), 38 deletions(-)

diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts
index 7c80bc740df..0bd53cb8db3 100644
--- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts
+++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts
@@ -111,6 +111,40 @@ describe('DashboardScenePageStateManager', () => {
     });
 
     describe('caching', () => {
+      it('should take scene from cache if it exists', async () => {
+        setupLoadDashboardMock({ dashboard: { uid: 'fake-dash', version: 10 }, meta: {} });
+
+        const loader = new DashboardScenePageStateManager({});
+
+        await loader.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal });
+
+        loader.state.dashboard?.onEnterEditMode();
+
+        expect(loader.state.dashboard?.state.isEditing).toBe(true);
+
+        loader.clearState();
+
+        // now load it again
+        await loader.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal });
+
+        // should still be editing
+        expect(loader.state.dashboard?.state.isEditing).toBe(true);
+        expect(loader.state.dashboard?.state.version).toBe(10);
+
+        loader.clearState();
+
+        loader.setDashboardCache('fake-dash', {
+          dashboard: { title: 'new version', uid: 'fake-dash', version: 11, schemaVersion: 30 },
+          meta: {},
+        });
+
+        // now load a third time
+        await loader.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal });
+
+        expect(loader.state.dashboard!.state.isEditing).toBe(undefined);
+        expect(loader.state.dashboard!.state.version).toBe(11);
+      });
+
       it('should cache the dashboard DTO', async () => {
         setupLoadDashboardMock({ dashboard: { uid: 'fake-dash' }, meta: {} });
 
diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts
index c65ccb124dc..56c0dee4992 100644
--- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts
+++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts
@@ -21,7 +21,7 @@ export interface DashboardScenePageState {
   loadError?: string;
 }
 
-export const DASHBOARD_CACHE_TTL = 2000;
+export const DASHBOARD_CACHE_TTL = 500;
 
 /** Only used by cache in loading home in DashboardPageProxy and initDashboard (Old arch), can remove this after old dashboard arch is gone */
 export const HOME_DASHBOARD_CACHE_KEY = '__grafana_home_uid__';
@@ -29,6 +29,7 @@ export const HOME_DASHBOARD_CACHE_KEY = '__grafana_home_uid__';
 interface DashboardCacheEntry {
   dashboard: DashboardDTO;
   ts: number;
+  cacheKey: string;
 }
 
 export interface LoadDashboardOptions {
@@ -39,12 +40,13 @@ export interface LoadDashboardOptions {
 
 export class DashboardScenePageStateManager extends StateManagerBase<DashboardScenePageState> {
   private cache: Record<string, DashboardScene> = {};
+
   // This is a simplistic, short-term cache for DashboardDTOs to avoid fetching the same dashboard multiple times across a short time span.
-  private dashboardCache: Map<string, DashboardCacheEntry> = new Map();
+  private dashboardCache?: DashboardCacheEntry;
 
   // To eventualy replace the fetchDashboard function from Dashboard redux state management.
   // For now it's a simplistic version to support Home and Normal dashboard routes.
-  public async fetchDashboard({ uid, route, urlFolderUid }: LoadDashboardOptions) {
+  public async fetchDashboard({ uid, route, urlFolderUid }: LoadDashboardOptions): Promise<DashboardDTO | null> {
     const cacheKey = route === DashboardRoutes.Home ? HOME_DASHBOARD_CACHE_KEY : uid;
     const cachedDashboard = this.getFromCache(cacheKey);
 
@@ -52,7 +54,7 @@ export class DashboardScenePageStateManager extends StateManagerBase<DashboardSc
       return cachedDashboard;
     }
 
-    let rsp: DashboardDTO | undefined;
+    let rsp: DashboardDTO;
 
     try {
       switch (route) {
@@ -84,30 +86,24 @@ export class DashboardScenePageStateManager extends StateManagerBase<DashboardSc
           }
       }
 
-      if (rsp) {
-        if (rsp.meta.url && route !== DashboardRoutes.Embedded) {
-          const dashboardUrl = locationUtil.stripBaseFromUrl(rsp.meta.url);
-          const currentPath = locationService.getLocation().pathname;
-          if (dashboardUrl !== currentPath) {
-            // Spread current location to persist search params used for navigation
-            locationService.replace({
-              ...locationService.getLocation(),
-              pathname: dashboardUrl,
-            });
-            console.log('not correct url correcting', dashboardUrl, currentPath);
-          }
-        }
-
-        // Populate nav model in global store according to the folder
-        await this.initNavModel(rsp);
-
-        // Do not cache new dashboards
-        if (uid) {
-          this.dashboardCache.set(uid, { dashboard: rsp, ts: Date.now() });
-        } else if (route === DashboardRoutes.Home) {
-          this.dashboardCache.set(HOME_DASHBOARD_CACHE_KEY, { dashboard: rsp, ts: Date.now() });
+      if (rsp.meta.url && route !== DashboardRoutes.Embedded) {
+        const dashboardUrl = locationUtil.stripBaseFromUrl(rsp.meta.url);
+        const currentPath = locationService.getLocation().pathname;
+        if (dashboardUrl !== currentPath) {
+          // Spread current location to persist search params used for navigation
+          locationService.replace({
+            ...locationService.getLocation(),
+            pathname: dashboardUrl,
+          });
+          console.log('not correct url correcting', dashboardUrl, currentPath);
         }
       }
+
+      // Populate nav model in global store according to the folder
+      await this.initNavModel(rsp);
+
+      // Do not cache new dashboards
+      this.dashboardCache = { dashboard: rsp, ts: Date.now(), cacheKey };
     } catch (e) {
       // Ignore cancelled errors
       if (isFetchError(e) && e.cancelled) {
@@ -133,15 +129,16 @@ export class DashboardScenePageStateManager extends StateManagerBase<DashboardSc
   }
 
   private async loadScene(options: LoadDashboardOptions): Promise<DashboardScene> {
+    const rsp = await this.fetchDashboard(options);
+
     const fromCache = this.cache[options.uid];
-    if (fromCache) {
+
+    if (fromCache && fromCache.state.version === rsp?.dashboard.version) {
       return fromCache;
     }
 
     this.setState({ isLoading: true });
 
-    const rsp = await this.fetchDashboard(options);
-
     if (rsp?.dashboard) {
       const scene = transformSaveModelToScene(rsp);
 
@@ -155,20 +152,20 @@ export class DashboardScenePageStateManager extends StateManagerBase<DashboardSc
     throw new Error('Dashboard not found');
   }
 
-  public getFromCache(uid: string) {
-    const cachedDashboard = this.dashboardCache.get(uid);
+  public getFromCache(cacheKey: string) {
+    const cachedDashboard = this.dashboardCache;
 
-    if (cachedDashboard && !this.hasExpired(cachedDashboard)) {
+    if (
+      cachedDashboard &&
+      cachedDashboard.cacheKey === cacheKey &&
+      Date.now() - cachedDashboard?.ts < DASHBOARD_CACHE_TTL
+    ) {
       return cachedDashboard.dashboard;
     }
 
     return null;
   }
 
-  private hasExpired(entry: DashboardCacheEntry) {
-    return Date.now() - entry.ts > DASHBOARD_CACHE_TTL;
-  }
-
   private async initNavModel(dashboard: DashboardDTO) {
     // only the folder API has information about ancestors
     // get parent folder (if it exists) and put it in the store
@@ -188,8 +185,8 @@ export class DashboardScenePageStateManager extends StateManagerBase<DashboardSc
     this.setState({ dashboard: undefined, loadError: undefined, isLoading: false, panelEditor: undefined });
   }
 
-  public setDashboardCache(uid: string, dashboard: DashboardDTO) {
-    this.dashboardCache.set(uid, { dashboard, ts: Date.now() });
+  public setDashboardCache(cacheKey: string, dashboard: DashboardDTO) {
+    this.dashboardCache = { dashboard, ts: Date.now(), cacheKey };
   }
 }