PanelEdit: Fixed timing and state related issues (#22131)

* PanelEdit: Fixed timing and state related issues

* Added unit test and changed some logic

* Fixed id logic

* Updated snapshots

* Update public/app/features/dashboard/state/PanelModel.ts

Co-Authored-By: Dominik Prokop <dominik.prokop@grafana.com>

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

packages/grafana-runtime/src/config.ts

@@ -54,6 +54,7 @@ export class GrafanaBootConfig {
   loginHint: any;
   passwordHint: any;
   loginError: any;
+  navTree: any;
   viewersCanEdit = false;
   editorsCanAdmin = false;
   disableSanitizeHtml = false;
@@ -98,6 +99,7 @@ export class GrafanaBootConfig {
 const bootData = (window as any).grafanaBootData || {
   settings: {},
   user: {},
+  navTree: [],
 };
 
 const options = bootData.settings;

packages/grafana-ui/src/components/Cascader/Cascader.tsx

@@ -209,7 +209,6 @@ export class Cascader extends React.PureComponent<CascaderProps, CascaderState>
                 value={activeLabel}
                 onKeyDown={this.onInputKeyDown}
                 onChange={() => {}}
-                size={size || 'md'}
                 suffix={focusCascade ? <Icon name="caret-up" /> : <Icon name="caret-down" />}
               />
             </div>

public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx

@@ -25,6 +25,7 @@ import { setDisplayMode, toggleOptionsView, setDiscardChanges } from './state/re
 import { FieldConfigEditor } from './FieldConfigEditor';
 import { OptionsGroup } from './OptionsGroup';
 import { getPanelEditorTabs } from './state/selectors';
+import { getPanelStateById } from '../../state/selectors';
 
 interface OwnProps {
   dashboard: DashboardModel;
@@ -281,7 +282,7 @@ export class PanelEditorUnconnected extends PureComponent<Props> {
 
 const mapStateToProps: MapStateToProps<ConnectedProps, OwnProps, StoreState> = (state, props) => {
   const panel = state.panelEditorNew.getPanel();
-  const plugin = state.plugins.panels[panel.type];
+  const { plugin } = getPanelStateById(state.dashboard, panel.id);
 
   return {
     location: state.location,

public/app/features/dashboard/components/PanelEditor/state/actions.test.ts

@@ -0,0 +1,60 @@
+import { thunkTester } from '../../../../../../test/core/thunk/thunkTester';
+import { initialState } from './reducers';
+import { initPanelEditor, panelEditorCleanUp } from './actions';
+import { PanelEditorStateNew, closeCompleted } from './reducers';
+import { PanelModel, DashboardModel } from '../../../state';
+
+describe('panelEditor actions', () => {
+  describe('initPanelEditor', () => {
+    it('initPanelEditor should create edit panel model as clone', async () => {
+      const dashboard = new DashboardModel({
+        panels: [{ id: 12, type: 'graph' }],
+      });
+      const sourcePanel = new PanelModel({ id: 12, type: 'graph' });
+
+      const dispatchedActions = await thunkTester({
+        panelEditorNew: { ...initialState },
+      })
+        .givenThunk(initPanelEditor)
+        .whenThunkIsDispatched(sourcePanel, dashboard);
+
+      expect(dispatchedActions.length).toBe(1);
+      expect(dispatchedActions[0].payload.sourcePanel).toBe(sourcePanel);
+      expect(dispatchedActions[0].payload.panel).not.toBe(sourcePanel);
+      expect(dispatchedActions[0].payload.panel.id).not.toBe(sourcePanel.id);
+    });
+  });
+
+  describe('panelEditorCleanUp', () => {
+    it('create update source panel', async () => {
+      const sourcePanel = new PanelModel({ id: 12, type: 'graph' });
+      const dashboard = new DashboardModel({
+        panels: [{ id: 12, type: 'graph' }],
+      });
+
+      const panel = sourcePanel.getEditClone();
+      panel.updateOptions({ prop: true });
+
+      const state: PanelEditorStateNew = {
+        ...initialState,
+        getPanel: () => panel,
+        getSourcePanel: () => sourcePanel,
+        querySubscription: { unsubscribe: jest.fn() },
+      };
+
+      const dispatchedActions = await thunkTester({
+        panelEditorNew: state,
+        dashboard: {
+          getModel: () => dashboard,
+        },
+      })
+        .givenThunk(panelEditorCleanUp)
+        .whenThunkIsDispatched();
+
+      expect(dispatchedActions.length).toBe(1);
+      expect(dispatchedActions[0].type).toBe(closeCompleted.type);
+      expect(sourcePanel.getOptions()).toEqual({ prop: true });
+      expect(sourcePanel.id).toEqual(12);
+    });
+  });
+});

public/app/features/dashboard/components/PanelEditor/state/actions.ts

@@ -25,10 +25,18 @@ export function initPanelEditor(sourcePanel: PanelModel, dashboard: DashboardMod
 export function panelEditorCleanUp(): ThunkResult<void> {
   return (dispatch, getStore) => {
     const dashboard = getStore().dashboard.getModel();
-    const { getPanel, querySubscription, shouldDiscardChanges } = getStore().panelEditorNew;
+    const { getPanel, getSourcePanel, querySubscription, shouldDiscardChanges } = getStore().panelEditorNew;
 
     if (!shouldDiscardChanges) {
-      dashboard.updatePanel(getPanel());
+      const panel = getPanel();
+      const modifiedSaveModel = panel.getSaveModel();
+      const sourcePanel = getSourcePanel();
+
+      // restore the source panel id before we update source panel
+      modifiedSaveModel.id = sourcePanel.id;
+
+      sourcePanel.restoreModel(modifiedSaveModel);
+      sourcePanel.getQueryRunner().pipeDataToSubject(panel.getQueryRunner().getLastResult());
     }
 
     dashboard.exitPanelEditor();

public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap

@@ -66,6 +66,7 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
               "y": 0,
             },
             "id": 1,
+            "options": Object {},
             "restoreModel": [Function],
             "targets": Array [
               Object {
@@ -94,7 +95,6 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
         "timezone": "",
         "title": "My dashboard",
         "uid": null,
-        "updatePanel": [Function],
         "version": 0,
       }
     }
@@ -180,6 +180,7 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
                     "y": 0,
                   },
                   "id": 1,
+                  "options": Object {},
                   "restoreModel": [Function],
                   "targets": Array [
                     Object {
@@ -208,7 +209,6 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
               "timezone": "",
               "title": "My dashboard",
               "uid": null,
-              "updatePanel": [Function],
               "version": 0,
             }
           }
@@ -274,6 +274,7 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
                     "y": 0,
                   },
                   "id": 1,
+                  "options": Object {},
                   "restoreModel": [Function],
                   "targets": Array [
                     Object {
@@ -302,7 +303,6 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
               "timezone": "",
               "title": "My dashboard",
               "uid": null,
-              "updatePanel": [Function],
               "version": 0,
             }
           }
@@ -400,6 +400,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
               "y": 0,
             },
             "id": 1,
+            "options": Object {},
             "restoreModel": [Function],
             "targets": Array [
               Object {
@@ -428,7 +429,6 @@ exports[`DashboardPage When dashboard has editview url state should render setti
         "timezone": "",
         "title": "My dashboard",
         "uid": null,
-        "updatePanel": [Function],
         "version": 0,
       }
     }
@@ -512,6 +512,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
                   "y": 0,
                 },
                 "id": 1,
+                "options": Object {},
                 "restoreModel": [Function],
                 "targets": Array [
                   Object {
@@ -540,7 +541,6 @@ exports[`DashboardPage When dashboard has editview url state should render setti
             "timezone": "",
             "title": "My dashboard",
             "uid": null,
-            "updatePanel": [Function],
             "version": 0,
           }
         }
@@ -609,6 +609,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
                     "y": 0,
                   },
                   "id": 1,
+                  "options": Object {},
                   "restoreModel": [Function],
                   "targets": Array [
                     Object {
@@ -637,7 +638,6 @@ exports[`DashboardPage When dashboard has editview url state should render setti
               "timezone": "",
               "title": "My dashboard",
               "uid": null,
-              "updatePanel": [Function],
               "version": 0,
             }
           }
@@ -703,6 +703,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
                     "y": 0,
                   },
                   "id": 1,
+                  "options": Object {},
                   "restoreModel": [Function],
                   "targets": Array [
                     Object {
@@ -731,7 +732,6 @@ exports[`DashboardPage When dashboard has editview url state should render setti
               "timezone": "",
               "title": "My dashboard",
               "uid": null,
-              "updatePanel": [Function],
               "version": 0,
             }
           }

public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap

@@ -142,6 +142,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 1,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -169,6 +170,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 2,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -196,6 +198,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 3,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -223,6 +226,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 4,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -251,7 +255,6 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           "timezone": "",
           "title": "My dashboard",
           "uid": null,
-          "updatePanel": [Function],
           "version": 0,
         }
       }
@@ -274,6 +277,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           },
           "id": 1,
           "isInView": false,
+          "options": Object {},
           "restoreModel": [Function],
           "targets": Array [
             Object {
@@ -385,6 +389,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 1,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -412,6 +417,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 2,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -439,6 +445,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 3,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -466,6 +473,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 4,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -494,7 +502,6 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           "timezone": "",
           "title": "My dashboard",
           "uid": null,
-          "updatePanel": [Function],
           "version": 0,
         }
       }
@@ -517,6 +524,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           },
           "id": 2,
           "isInView": false,
+          "options": Object {},
           "restoreModel": [Function],
           "targets": Array [
             Object {
@@ -628,6 +636,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 1,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -655,6 +664,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 2,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -682,6 +692,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 3,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -709,6 +720,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 4,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -737,7 +749,6 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           "timezone": "",
           "title": "My dashboard",
           "uid": null,
-          "updatePanel": [Function],
           "version": 0,
         }
       }
@@ -760,6 +771,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           },
           "id": 3,
           "isInView": false,
+          "options": Object {},
           "restoreModel": [Function],
           "targets": Array [
             Object {
@@ -871,6 +883,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 1,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -898,6 +911,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 2,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -925,6 +939,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 3,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -952,6 +967,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
               },
               "id": 4,
               "isInView": false,
+              "options": Object {},
               "restoreModel": [Function],
               "targets": Array [
                 Object {
@@ -980,7 +996,6 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           "timezone": "",
           "title": "My dashboard",
           "uid": null,
-          "updatePanel": [Function],
           "version": 0,
         }
       }
@@ -1003,6 +1018,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           },
           "id": 4,
           "isInView": false,
+          "options": Object {},
           "restoreModel": [Function],
           "targets": Array [
             Object {

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

@@ -969,16 +969,4 @@ export class DashboardModel {
       panel.render();
     }
   }
-
-  updatePanel = (panelModel: PanelModel) => {
-    let index = 0;
-    for (const panel of this.panels) {
-      if (panel.id === panelModel.id) {
-        this.panels[index].restoreModel(panelModel.getSaveModel());
-        this.panels[index].getQueryRunner().pipeDataToSubject(panelModel.getQueryRunner().getLastResult());
-        break;
-      }
-      index++;
-    }
-  };
 }

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

@@ -88,6 +88,7 @@ const defaults: any = {
   targets: [{ refId: 'A' }],
   cachedPluginOptions: {},
   transparent: false,
+  options: {},
 };
 
 export class PanelModel {
@@ -179,7 +180,6 @@ export class PanelModel {
 
   updateOptions(options: object) {
     this.options = options;
-
     this.render();
   }
 
@@ -354,7 +354,12 @@ export class PanelModel {
   }
 
   getEditClone() {
-    const clone = new PanelModel(this.getSaveModel());
+    const sourceModel = this.getSaveModel();
+
+    // Temporary id for the clone, restored later in redux action when changes are saved
+    sourceModel.id = 23763571993;
+
+    const clone = new PanelModel(sourceModel);
     const sourceQueryRunner = this.getQueryRunner();
 
     // pipe last result to new clone query runner

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

@@ -3,7 +3,7 @@ import { getBackendSrv } from '@grafana/runtime';
 import { createSuccessNotification } from 'app/core/copy/appNotification';
 // Actions
 import { loadPluginDashboards } from '../../plugins/state/actions';
-import { loadDashboardPermissions, dashboardPanelTypeChanged } from './reducers';
+import { loadDashboardPermissions, panelModelAndPluginReady } from './reducers';
 import { notifyApp } from 'app/core/actions';
 import { loadPanelPlugin } from 'app/features/plugins/state/actions';
 // Types
@@ -122,6 +122,8 @@ export function initDashboardPanel(panel: PanelModel): ThunkResult<void> {
     if (!panel.plugin) {
       panel.pluginLoaded(plugin);
     }
+
+    dispatch(panelModelAndPluginReady({ panelId: panel.id, plugin }));
   };
 }
 
@@ -139,6 +141,7 @@ export function changePanelPlugin(panel: PanelModel, pluginId: string): ThunkRes
     }
 
     panel.changePlugin(plugin);
-    dispatch(dashboardPanelTypeChanged({ panelId: panel.id, pluginId }));
+
+    dispatch(panelModelAndPluginReady({ panelId: panel.id, plugin }));
   };
 }

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

@@ -4,6 +4,7 @@ import {
   DashboardState,
   DashboardAclDTO,
   DashboardInitError,
+  PanelState,
   QueriesToUpdateOnDashboardLoad,
 } from 'app/types';
 import { processAclItems } from 'app/core/utils/acl';
@@ -11,6 +12,7 @@ import { panelEditorReducer } from '../panel_editor/state/reducers';
 import { panelEditorReducerNew } from '../components/PanelEditor/state/reducers';
 import { DashboardModel } from './DashboardModel';
 import { PanelModel } from './PanelModel';
+import { PanelPlugin } from '@grafana/data';
 
 export const initialState: DashboardState = {
   initPhase: DashboardInitPhase.NotStarted,
@@ -72,16 +74,24 @@ const dashbardSlice = createSlice({
     clearDashboardQueriesToUpdateOnLoad: (state, action: PayloadAction) => {
       state.modifiedQueries = null;
     },
-    dashboardPanelTypeChanged: (state, action: PayloadAction<DashboardPanelTypeChangedPayload>) => {
+    panelModelAndPluginReady: (state: DashboardState, action: PayloadAction<PanelModelAndPluginReadyPayload>) => {
-      state.panels[action.payload.panelId] = { pluginId: action.payload.pluginId };
+      updatePanelState(state, action.payload.panelId, { plugin: action.payload.plugin });
     },
     addPanelToDashboard: (state, action: PayloadAction<AddPanelPayload>) => {},
   },
 });
 
-export interface DashboardPanelTypeChangedPayload {
+export function updatePanelState(state: DashboardState, panelId: number, ps: Partial<PanelState>) {
+  if (!state.panels[panelId]) {
+    state.panels[panelId] = ps as PanelState;
+  } else {
+    Object.assign(state.panels[panelId], ps);
+  }
+}
+
+export interface PanelModelAndPluginReadyPayload {
   panelId: number;
-  pluginId: string;
+  plugin: PanelPlugin;
 }
 
 export interface AddPanelPayload {
@@ -98,7 +108,7 @@ export const {
   cleanUpDashboard,
   setDashboardQueriesToUpdateOnLoad,
   clearDashboardQueriesToUpdateOnLoad,
-  dashboardPanelTypeChanged,
+  panelModelAndPluginReady,
   addPanelToDashboard,
 } = dashbardSlice.actions;
 

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

@@ -0,0 +1,9 @@
+import { DashboardState, PanelState } from 'app/types';
+
+export function getPanelStateById(state: DashboardState, panelId: number): PanelState {
+  if (!panelId) {
+    return {} as PanelState;
+  }
+
+  return state.panels[panelId] ?? ({} as PanelState);
+}

public/app/types/dashboard.ts

@@ -1,5 +1,5 @@
 import { DashboardAcl } from './acl';
-import { DataQuery } from '@grafana/data';
+import { DataQuery, PanelPlugin } from '@grafana/data';
 import { DashboardModel } from 'app/features/dashboard/state/DashboardModel';
 
 export interface DashboardDTO {
@@ -69,6 +69,7 @@ export interface QueriesToUpdateOnDashboardLoad {
 
 export interface PanelState {
   pluginId: string;
+  plugin?: PanelPlugin;
 }
 
 export interface DashboardState {