DashboardScene: Fix explore to dashboard flow (#85140)

* DashboardScene: Fix explore to dashboard flow

* Tests

* Make sure dashboard is in edit mode when adding from explore

* Allow discarding changes when coming from explore

* Tests

public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx

@@ -11,7 +11,9 @@ import { config, getPluginLinkExtensions, locationService, setPluginImportUtils
 import { VizPanel } from '@grafana/scenes';
 import { Dashboard } from '@grafana/schema';
 import { getRouteComponentProps } from 'app/core/navigation/__mocks__/routeProps';
+import store from 'app/core/store';
 import { DashboardLoaderSrv, setDashboardLoaderSrv } from 'app/features/dashboard/services/DashboardLoaderSrv';
+import { DASHBOARD_FROM_LS_KEY } from 'app/features/dashboard/state/initDashboard';
 
 import { dashboardSceneGraph } from '../utils/dashboardSceneGraph';
 
@@ -128,6 +130,7 @@ describe('DashboardScenePage', () => {
     Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { configurable: true, value: 1000 });
     getPluginLinkExtensionsMock.mockRestore();
     getPluginLinkExtensionsMock.mockReturnValue({ extensions: [] });
+    store.delete(DASHBOARD_FROM_LS_KEY);
   });
 
   it('Can render dashboard', async () => {
@@ -236,6 +239,20 @@ describe('DashboardScenePage', () => {
       expect(await screen.queryByText('Start your new dashboard by adding a visualization')).not.toBeInTheDocument();
     });
   });
+
+  it('is in edit mode when coming from explore to an existing dashboard', async () => {
+    store.setObject(DASHBOARD_FROM_LS_KEY, { dashboard: simpleDashboard });
+
+    setup();
+
+    await waitForDashbordToRender();
+
+    const panelAMenu = await screen.findByLabelText('Menu for panel with title Panel A');
+    expect(panelAMenu).toBeInTheDocument();
+    await userEvent.click(panelAMenu);
+    const editMenuItem = await screen.findAllByText('Edit');
+    expect(editMenuItem).toHaveLength(1);
+  });
 });
 
 interface VizOptions {

public/app/features/dashboard-scene/pages/DashboardScenePage.tsx

@@ -1,12 +1,14 @@
 // Libraries
-import React, { useEffect } from 'react';
+import React, { useEffect, useMemo } from 'react';
 
 import { PageLayoutType } from '@grafana/data';
 import { Page } from 'app/core/components/Page/Page';
 import PageLoader from 'app/core/components/PageLoader/PageLoader';
 import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
+import store from 'app/core/store';
 import { DashboardPageRouteParams, DashboardPageRouteSearchParams } from 'app/features/dashboard/containers/types';
-import { DashboardRoutes } from 'app/types';
+import { DASHBOARD_FROM_LS_KEY } from 'app/features/dashboard/state/initDashboard';
+import { DashboardDTO, DashboardRoutes } from 'app/types';
 
 import { DashboardPrompt } from '../saving/DashboardPrompt';
 
@@ -20,6 +22,12 @@ export function DashboardScenePage({ match, route, queryParams, history }: Props
   // After scene migration is complete and we get rid of old dashboard we should refactor dashboardWatcher so this route reload is not need
   const routeReloadCounter = (history.location.state as any)?.routeReloadCounter;
 
+  // Check if the user is coming from Explore, it's indicated byt the dashboard existence in local storage
+  const comingFromExplore = useMemo(() => {
+    return Boolean(store.getObject<DashboardDTO>(DASHBOARD_FROM_LS_KEY));
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [match.params.uid, match.params.slug, match.params.type]);
+
   useEffect(() => {
     if (route.routeName === DashboardRoutes.Normal && match.params.type === 'snapshot') {
       stateManager.loadSnapshot(match.params.slug!);
@@ -28,6 +36,7 @@ export function DashboardScenePage({ match, route, queryParams, history }: Props
         uid: match.params.uid ?? '',
         route: route.routeName as DashboardRoutes,
         urlFolderUid: queryParams.folderUid,
+        keepDashboardFromExploreInLocalStorage: false,
       });
     }
 
@@ -44,6 +53,16 @@ export function DashboardScenePage({ match, route, queryParams, history }: Props
     match.params.type,
   ]);
 
+  // Effect that handles explore->dashboards workflow
+  useEffect(() => {
+    // When coming from explore and adding to an existing dashboard, we should enter edit mode
+    if (dashboard && comingFromExplore) {
+      if (route.routeName !== DashboardRoutes.New) {
+        dashboard.onEnterEditMode(comingFromExplore);
+      }
+    }
+  }, [dashboard, comingFromExplore, route.routeName]);
+
   if (!dashboard) {
     return (
       <Page layout={PageLayoutType.Canvas} data-testid={'dashboard-scene-page'}>

public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts

@@ -2,6 +2,8 @@ import { advanceBy } from 'jest-date-mock';
 
 import { locationService } from '@grafana/runtime';
 import { getUrlSyncManager } from '@grafana/scenes';
+import store from 'app/core/store';
+import { DASHBOARD_FROM_LS_KEY } from 'app/features/dashboard/state/initDashboard';
 import { DashboardRoutes } from 'app/types';
 
 import { DashboardScene } from '../scene/DashboardScene';
@@ -10,6 +12,10 @@ import { setupLoadDashboardMock } from '../utils/test-utils';
 import { DashboardScenePageStateManager, DASHBOARD_CACHE_TTL } from './DashboardScenePageStateManager';
 
 describe('DashboardScenePageStateManager', () => {
+  afterEach(() => {
+    store.delete(DASHBOARD_FROM_LS_KEY);
+  });
+
   describe('when fetching/loading a dashboard', () => {
     it('should call loader from server if the dashboard is not cached', async () => {
       const loadDashboardMock = setupLoadDashboardMock({ dashboard: { uid: 'fake-dash', editable: true }, meta: {} });
@@ -35,6 +41,17 @@ describe('DashboardScenePageStateManager', () => {
       expect(loader.state.loadError).toBe('Error: Dashboard not found');
     });
 
+    it('shoud fetch dashboard from local storage and remove it after if it exists', async () => {
+      const loader = new DashboardScenePageStateManager({});
+      const localStorageDashboard = { uid: 'fake-dash' };
+      store.setObject(DASHBOARD_FROM_LS_KEY, localStorageDashboard);
+
+      const result = await loader.fetchDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal });
+
+      expect(result).toEqual(localStorageDashboard);
+      expect(store.getObject(DASHBOARD_FROM_LS_KEY)).toBeUndefined();
+    });
+
     it('should initialize the dashboard scene with the loaded dashboard', async () => {
       setupLoadDashboardMock({ dashboard: { uid: 'fake-dash' }, meta: {} });
 
@@ -187,5 +204,36 @@ describe('DashboardScenePageStateManager', () => {
         expect(loadDashSpy).toHaveBeenCalledTimes(2);
       });
     });
+
+    describe('When coming from explore', () => {
+      it('shoud fetch dashboard from local storage and keep it there after when asked', async () => {
+        const loader = new DashboardScenePageStateManager({});
+        const localStorageDashboard = { uid: 'fake-dash' };
+        store.setObject(DASHBOARD_FROM_LS_KEY, { dashboard: localStorageDashboard });
+
+        const result = await loader.fetchDashboard({
+          uid: 'fake-dash',
+          route: DashboardRoutes.Normal,
+          keepDashboardFromExploreInLocalStorage: true,
+        });
+
+        expect(result).toEqual({ dashboard: localStorageDashboard });
+        expect(store.getObject(DASHBOARD_FROM_LS_KEY)).toEqual({ dashboard: localStorageDashboard });
+      });
+
+      it('shoud not store dashboard in cache when coming from Explore', async () => {
+        const loader = new DashboardScenePageStateManager({});
+        const localStorageDashboard = { uid: 'fake-dash' };
+        store.setObject(DASHBOARD_FROM_LS_KEY, { dashboard: localStorageDashboard });
+
+        await loader.loadDashboard({
+          uid: 'fake-dash',
+          route: DashboardRoutes.Normal,
+          keepDashboardFromExploreInLocalStorage: false,
+        });
+
+        expect(loader.getFromCache('fake-dash')).toBeNull();
+      });
+    });
   });
 });

public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts

@@ -3,8 +3,13 @@ import { config, getBackendSrv, isFetchError, locationService } from '@grafana/r
 import { updateNavIndex } from 'app/core/actions';
 import { StateManagerBase } from 'app/core/services/StateManagerBase';
 import { backendSrv } from 'app/core/services/backend_srv';
+import { default as localStorageStore } from 'app/core/store';
 import { dashboardLoaderSrv } from 'app/features/dashboard/services/DashboardLoaderSrv';
 import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv';
+import {
+  DASHBOARD_FROM_LS_KEY,
+  removeDashboardToFetchFromLocalStorage,
+} from 'app/features/dashboard/state/initDashboard';
 import { buildNavModel } from 'app/features/folders/state/navModel';
 import { store } from 'app/store/store';
 import { DashboardDTO, DashboardRoutes } from 'app/types';
@@ -36,6 +41,12 @@ export interface LoadDashboardOptions {
   uid: string;
   route: DashboardRoutes;
   urlFolderUid?: string;
+  // A temporary approach not to clean the dashboard from local storage when navigating from Explore to Dashboard
+  // We currently need it as there are two flows of fetching dashboard. The legacy one (initDashboard), uses the new one(DashboardScenePageStateManager.fetch) where the
+  // removal of the dashboard from local storage is implemented. So in the old flow we wouldn't be able to early return dashboard from local storage, if we prematurely
+  // removed it when prefetching the dashboard in DashboardPageProxy.
+  // This property will be removed when the old flow (initDashboard) is removed.
+  keepDashboardFromExploreInLocalStorage?: boolean;
 }
 
 export class DashboardScenePageStateManager extends StateManagerBase<DashboardScenePageState> {
@@ -46,7 +57,21 @@ export class DashboardScenePageStateManager extends StateManagerBase<DashboardSc
 
   // 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): Promise<DashboardDTO | null> {
+  public async fetchDashboard({
+    uid,
+    route,
+    urlFolderUid,
+    keepDashboardFromExploreInLocalStorage,
+  }: LoadDashboardOptions): Promise<DashboardDTO | null> {
+    const model = localStorageStore.getObject<DashboardDTO>(DASHBOARD_FROM_LS_KEY);
+
+    if (model) {
+      if (!keepDashboardFromExploreInLocalStorage) {
+        removeDashboardToFetchFromLocalStorage();
+      }
+      return model;
+    }
+
     const cacheKey = route === DashboardRoutes.Home ? HOME_DASHBOARD_CACHE_KEY : uid;
     const cachedDashboard = this.getFromCache(cacheKey);
 
@@ -156,20 +181,29 @@ export class DashboardScenePageStateManager extends StateManagerBase<DashboardSc
   }
 
   private async loadScene(options: LoadDashboardOptions): Promise<DashboardScene> {
+    const comingFromExplore = Boolean(
+      localStorageStore.getObject<DashboardDTO>(DASHBOARD_FROM_LS_KEY) &&
+        options.keepDashboardFromExploreInLocalStorage === false
+    );
+
     const rsp = await this.fetchDashboard(options);
 
     const fromCache = this.cache[options.uid];
 
+    // When coming from Explore, skip returnning scene from cache
+    if (!comingFromExplore) {
       if (fromCache && fromCache.state.version === rsp?.dashboard.version) {
         return fromCache;
       }
+    }
 
     this.setState({ isLoading: true });
 
     if (rsp?.dashboard) {
       const scene = transformSaveModelToScene(rsp);
 
-      if (options.uid) {
+      // Cache scene only if not coming from Explore, we don't want to cache temporary dashboard
+      if (options.uid && !comingFromExplore) {
         this.cache[options.uid] = scene;
       }
 

public/app/features/dashboard-scene/scene/DashboardScene.test.tsx

@@ -863,6 +863,53 @@ describe('DashboardScene', () => {
       }
     });
   });
+
+  describe('When coming from explore', () => {
+    // When coming from Explore the first panel in a dashboard is a temporary panel
+    it('should remove first panel from the grid when discarding changes', () => {
+      const scene = new DashboardScene({
+        title: 'hello',
+        uid: 'dash-1',
+        description: 'hello description',
+        editable: true,
+        $timeRange: new SceneTimeRange({
+          timeZone: 'browser',
+        }),
+        controls: new DashboardControls({}),
+        $behaviors: [new behaviors.CursorSync({})],
+        body: new SceneGridLayout({
+          children: [
+            new DashboardGridItem({
+              key: 'griditem-1',
+              x: 0,
+              body: new VizPanel({
+                title: 'Panel A',
+                key: 'panel-1',
+                pluginId: 'table',
+                $data: new SceneQueryRunner({ key: 'data-query-runner', queries: [{ refId: 'A' }] }),
+              }),
+            }),
+            new DashboardGridItem({
+              key: 'griditem-2',
+              body: new VizPanel({
+                title: 'Panel B',
+                key: 'panel-2',
+                pluginId: 'table',
+              }),
+            }),
+          ],
+        }),
+      });
+
+      scene.onEnterEditMode(true);
+      expect(scene.state.isEditing).toBe(true);
+      expect((scene.state.body as SceneGridLayout).state.children.length).toBe(2);
+
+      scene.exitEditMode({ skipConfirm: true });
+      expect(scene.state.isEditing).toBe(false);
+      expect((scene.state.body as SceneGridLayout).state.children.length).toBe(1);
+    });
+  });
 });
 
 function buildTestScene(overrides?: Partial<DashboardSceneState>) {

public/app/features/dashboard-scene/scene/DashboardScene.tsx

@@ -145,6 +145,11 @@ export class DashboardScene extends SceneObjectBase<DashboardSceneState> {
    */
   private _changeTracker: DashboardSceneChangeTracker;
 
+  /**
+   * Flag to indicate if the user came from Explore
+   */
+  private _fromExplore = false;
+
   public constructor(state: Partial<DashboardSceneState>) {
     super({
       title: 'Dashboard',
@@ -211,9 +216,11 @@ export class DashboardScene extends SceneObjectBase<DashboardSceneState> {
     getUrlSyncManager().cleanUp(this);
   }
 
-  public onEnterEditMode = () => {
+  public onEnterEditMode = (fromExplore = false) => {
+    this._fromExplore = fromExplore;
     // Save this state
     this._initialState = sceneUtils.cloneSceneObjectState(this.state);
+
     this._initialUrlState = locationService.getLocation();
 
     // Switch to edit mode
@@ -299,6 +306,10 @@ export class DashboardScene extends SceneObjectBase<DashboardSceneState> {
       })
     );
 
+    if (this._fromExplore) {
+      this.cleanupStateFromExplore();
+    }
+
     if (restoreInitialState) {
       //  Restore initial state and disable editing
       this.setState({ ...this._initialState, isEditing: false });
@@ -312,6 +323,20 @@ export class DashboardScene extends SceneObjectBase<DashboardSceneState> {
     this.propagateEditModeChange();
   }
 
+  private cleanupStateFromExplore() {
+    this._fromExplore = false;
+    // When coming from explore but discarding changes, remove the panel that explore is potentially adding.
+    if (this._initialSaveModel?.panels) {
+      this._initialSaveModel.panels = this._initialSaveModel.panels.slice(1);
+    }
+
+    if (this._initialState && this._initialState.body instanceof SceneGridLayout) {
+      this._initialState.body.setState({
+        children: this._initialState.body.state.children.slice(1),
+      });
+    }
+  }
+
   public canDiscard() {
     return this._initialState !== undefined;
   }

public/app/features/dashboard/containers/DashboardPageProxy.tsx

@@ -42,6 +42,7 @@ function DashboardPageProxy(props: DashboardPageProxyProps) {
     return stateManager.fetchDashboard({
       route: props.route.routeName as DashboardRoutes,
       uid: props.match.params.uid ?? '',
+      keepDashboardFromExploreInLocalStorage: true,
     });
   }, [props.match.params.uid, props.route.routeName]);
 

public/app/features/dashboard/state/initDashboard.ts

@@ -295,7 +295,7 @@ export function initDashboard(args: InitDashboardArgs): ThunkResult<void> {
   };
 }
 
-const DASHBOARD_FROM_LS_KEY = 'DASHBOARD_FROM_LS_KEY';
+export const DASHBOARD_FROM_LS_KEY = 'DASHBOARD_FROM_LS_KEY';
 
 export function setDashboardToFetchFromLocalStorage(model: DashboardDTO) {
   store.setObject(DASHBOARD_FROM_LS_KEY, model);