NewPanelEditor: Fixed cleanup that could cause crash (#22384)

* NewPanelEditor: Fixed cleanup that could cause crash

* Fixed unit test

public/app/core/constants.ts

@@ -10,3 +10,5 @@ export const MIN_PANEL_HEIGHT = GRID_CELL_HEIGHT * 3;
 export const LS_PANEL_COPY_KEY = 'panel-copy';
 export const LS_PANEL_COPY_KEY = 'panel-copy';
 
 
 export const PANEL_BORDER = 2;
 export const PANEL_BORDER = 2;
+
+export const EDIT_PANEL_ID = 23763571993;

public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.test.tsx

@@ -7,7 +7,7 @@ const setup = (propOverrides?: object) => {
   const props: Props = {
   const props: Props = {
     dashboard: {} as DashboardModel,
     dashboard: {} as DashboardModel,
     panel: {} as PanelModel,
     panel: {} as PanelModel,
-    addPanelToDashboard: jest.fn() as any,
+    addPanel: jest.fn() as any,
   };
   };
 
 
   Object.assign(props, propOverrides);
   Object.assign(props, propOverrides);

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

@@ -10,7 +10,7 @@ import store from 'app/core/store';
 // Store
 // Store
 import { store as reduxStore } from 'app/store/store';
 import { store as reduxStore } from 'app/store/store';
 import { updateLocation } from 'app/core/actions';
 import { updateLocation } from 'app/core/actions';
-import { addPanelToDashboard } from 'app/features/dashboard/state/reducers';
+import { addPanel } from 'app/features/dashboard/state/reducers';
 // Types
 // Types
 import { DashboardModel, PanelModel } from '../../state';
 import { DashboardModel, PanelModel } from '../../state';
 import { LS_PANEL_COPY_KEY } from 'app/core/constants';
 import { LS_PANEL_COPY_KEY } from 'app/core/constants';
@@ -23,7 +23,7 @@ export interface OwnProps {
 }
 }
 
 
 export interface DispatchProps {
 export interface DispatchProps {
-  addPanelToDashboard: typeof addPanelToDashboard;
+  addPanel: typeof addPanel;
 }
 }
 
 
 export type Props = OwnProps & DispatchProps;
 export type Props = OwnProps & DispatchProps;
@@ -197,6 +197,6 @@ export class AddPanelWidgetUnconnected extends React.Component<Props, State> {
   }
   }
 }
 }
 
 
-const mapDispatchToProps: MapDispatchToProps<DispatchProps, OwnProps> = { addPanelToDashboard };
+const mapDispatchToProps: MapDispatchToProps<DispatchProps, OwnProps> = { addPanel };
 
 
 export const AddPanelWidget = connect(null, mapDispatchToProps)(AddPanelWidgetUnconnected);
 export const AddPanelWidget = connect(null, mapDispatchToProps)(AddPanelWidgetUnconnected);

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

@@ -2,6 +2,7 @@ import { thunkTester } from '../../../../../../test/core/thunk/thunkTester';
 import { initialState } from './reducers';
 import { initialState } from './reducers';
 import { initPanelEditor, panelEditorCleanUp } from './actions';
 import { initPanelEditor, panelEditorCleanUp } from './actions';
 import { PanelEditorStateNew, closeCompleted } from './reducers';
 import { PanelEditorStateNew, closeCompleted } from './reducers';
+import { cleanUpEditPanel } from '../../../state/reducers';
 import { PanelModel, DashboardModel } from '../../../state';
 import { PanelModel, DashboardModel } from '../../../state';
 
 
 describe('panelEditor actions', () => {
 describe('panelEditor actions', () => {
@@ -51,8 +52,9 @@ describe('panelEditor actions', () => {
         .givenThunk(panelEditorCleanUp)
         .givenThunk(panelEditorCleanUp)
         .whenThunkIsDispatched();
         .whenThunkIsDispatched();
 
 
-      expect(dispatchedActions.length).toBe(1);
-      expect(dispatchedActions[0].type).toBe(closeCompleted.type);
+      expect(dispatchedActions.length).toBe(2);
+      expect(dispatchedActions[0].type).toBe(cleanUpEditPanel.type);
+      expect(dispatchedActions[1].type).toBe(closeCompleted.type);
       expect(sourcePanel.getOptions()).toEqual({ prop: true });
       expect(sourcePanel.getOptions()).toEqual({ prop: true });
       expect(sourcePanel.id).toEqual(12);
       expect(sourcePanel.id).toEqual(12);
     });
     });

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

@@ -9,6 +9,7 @@ import {
   setPanelEditorUIState,
   setPanelEditorUIState,
   PANEL_EDITOR_UI_STATE_STORAGE_KEY,
   PANEL_EDITOR_UI_STATE_STORAGE_KEY,
 } from './reducers';
 } from './reducers';
+import { cleanUpEditPanel } from '../../../state/reducers';
 import store from '../../../../../core/store';
 import store from '../../../../../core/store';
 
 
 export function initPanelEditor(sourcePanel: PanelModel, dashboard: DashboardModel): ThunkResult<void> {
 export function initPanelEditor(sourcePanel: PanelModel, dashboard: DashboardModel): ThunkResult<void> {
@@ -50,6 +51,7 @@ export function panelEditorCleanUp(): ThunkResult<void> {
     dashboard.exitPanelEditor();
     dashboard.exitPanelEditor();
     querySubscription.unsubscribe();
     querySubscription.unsubscribe();
 
 
+    dispatch(cleanUpEditPanel());
     dispatch(closeCompleted());
     dispatch(closeCompleted());
   };
   };
 }
 }

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

@@ -15,11 +15,13 @@ export const DEFAULT_PANEL_EDITOR_UI_STATE: PanelEditorUIState = {
 };
 };
 
 
 export interface PanelEditorUIState {
 export interface PanelEditorUIState {
+  /* Visualization options pane visibility */
   isPanelOptionsVisible: boolean;
   isPanelOptionsVisible: boolean;
-  // annotating as number or string since size can be expressed as static value or percentage
+  /* Pixels or percentage */
   rightPaneSize: number | string;
   rightPaneSize: number | string;
-  // annotating as number or string since size can be expressed as static value or percentage
+  /* Pixels or percentage */
   topPaneSize: number | string;
   topPaneSize: number | string;
+  /* Visualization size mode */
   mode: DisplayMode;
   mode: DisplayMode;
 }
 }
 
 

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

@@ -14,6 +14,7 @@ import {
   ScopedVars,
   ScopedVars,
 } from '@grafana/data';
 } from '@grafana/data';
 import { AngularComponent } from '@grafana/runtime';
 import { AngularComponent } from '@grafana/runtime';
+import { EDIT_PANEL_ID } from 'app/core/constants';
 
 
 import config from 'app/core/config';
 import config from 'app/core/config';
 
 
@@ -357,7 +358,7 @@ export class PanelModel {
     const sourceModel = this.getSaveModel();
     const sourceModel = this.getSaveModel();
 
 
     // Temporary id for the clone, restored later in redux action when changes are saved
     // Temporary id for the clone, restored later in redux action when changes are saved
-    sourceModel.id = 23763571993;
+    sourceModel.id = EDIT_PANEL_ID;
 
 
     const clone = new PanelModel(sourceModel);
     const clone = new PanelModel(sourceModel);
     const sourceQueryRunner = this.getQueryRunner();
     const sourceQueryRunner = this.getQueryRunner();

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

@@ -7,6 +7,7 @@ import {
   PanelState,
   PanelState,
   QueriesToUpdateOnDashboardLoad,
   QueriesToUpdateOnDashboardLoad,
 } from 'app/types';
 } from 'app/types';
+import { EDIT_PANEL_ID } from 'app/core/constants';
 import { processAclItems } from 'app/core/utils/acl';
 import { processAclItems } from 'app/core/utils/acl';
 import { panelEditorReducer } from '../panel_editor/state/reducers';
 import { panelEditorReducer } from '../panel_editor/state/reducers';
 import { panelEditorReducerNew } from '../components/PanelEditor/state/reducers';
 import { panelEditorReducerNew } from '../components/PanelEditor/state/reducers';
@@ -64,6 +65,7 @@ const dashbardSlice = createSlice({
         state.getModel = () => null;
         state.getModel = () => null;
       }
       }
 
 
+      state.panels = {};
       state.initPhase = DashboardInitPhase.NotStarted;
       state.initPhase = DashboardInitPhase.NotStarted;
       state.isInitSlow = false;
       state.isInitSlow = false;
       state.initError = null;
       state.initError = null;
@@ -77,7 +79,12 @@ const dashbardSlice = createSlice({
     panelModelAndPluginReady: (state: DashboardState, action: PayloadAction<PanelModelAndPluginReadyPayload>) => {
     panelModelAndPluginReady: (state: DashboardState, action: PayloadAction<PanelModelAndPluginReadyPayload>) => {
       updatePanelState(state, action.payload.panelId, { plugin: action.payload.plugin });
       updatePanelState(state, action.payload.panelId, { plugin: action.payload.plugin });
     },
     },
-    addPanelToDashboard: (state, action: PayloadAction<AddPanelPayload>) => {},
+    cleanUpEditPanel: (state, action: PayloadAction) => {
+      delete state.panels[EDIT_PANEL_ID];
+    },
+    addPanel: (state, action: PayloadAction<PanelModel>) => {
+      state.panels[action.payload.id] = { pluginId: action.payload.type };
+    },
   },
   },
 });
 });
 
 
@@ -94,10 +101,6 @@ export interface PanelModelAndPluginReadyPayload {
   plugin: PanelPlugin;
   plugin: PanelPlugin;
 }
 }
 
 
-export interface AddPanelPayload {
-  panel: PanelModel;
-}
-
 export const {
 export const {
   loadDashboardPermissions,
   loadDashboardPermissions,
   dashboardInitFetching,
   dashboardInitFetching,
@@ -109,7 +112,8 @@ export const {
   setDashboardQueriesToUpdateOnLoad,
   setDashboardQueriesToUpdateOnLoad,
   clearDashboardQueriesToUpdateOnLoad,
   clearDashboardQueriesToUpdateOnLoad,
   panelModelAndPluginReady,
   panelModelAndPluginReady,
-  addPanelToDashboard,
+  addPanel,
+  cleanUpEditPanel,
 } = dashbardSlice.actions;
 } = dashbardSlice.actions;
 
 
 export const dashboardReducer = dashbardSlice.reducer;
 export const dashboardReducer = dashbardSlice.reducer;