PanelEditor: fixes save/apply for undefined props in restoreModel (#23939)

* PanelEditor: fixes save/apply for undefined props in restoreModel

* Refactor: changes after PR comments

* Refactor: changes sourcePanel refresh strategy

* Added unit tests and minor refactoring of method, starting with cleanup, then setting properties from model

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

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>
Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

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

@@ -349,7 +349,7 @@ const mapStateToProps: MapStateToProps<ConnectedProps, OwnProps, StoreState> = (
   return {
     location: state.location,
     plugin: plugin,
-    panel: state.panelEditor.getPanel(),
+    panel,
     data: state.panelEditor.getData(),
     initDone: state.panelEditor.initDone,
     tabs: getPanelEditorTabs(state.location, plugin),

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

@@ -1,13 +1,13 @@
-import { PanelModel, DashboardModel } from '../../../state';
+import { DashboardModel, PanelModel } from '../../../state';
 import { PanelData } from '@grafana/data';
 import { ThunkResult } from 'app/types';
 import {
-  setEditorPanelData,
-  updateEditorInitState,
   closeCompleted,
-  PanelEditorUIState,
-  setPanelEditorUIState,
   PANEL_EDITOR_UI_STATE_STORAGE_KEY,
+  PanelEditorUIState,
+  setEditorPanelData,
+  setPanelEditorUIState,
+  updateEditorInitState,
 } from './reducers';
 import { cleanUpEditPanel, panelModelAndPluginReady } from '../../../state/reducers';
 import store from '../../../../../core/store';

public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx

@@ -2,21 +2,18 @@
 import React, { PureComponent } from 'react';
 import classNames from 'classnames';
 import { Unsubscribable } from 'rxjs';
-import { connect, MapStateToProps, MapDispatchToProps } from 'react-redux';
+import { connect, MapDispatchToProps, MapStateToProps } from 'react-redux';
-
 // Components
 import { PanelHeader } from './PanelHeader/PanelHeader';
-
 // Utils & Services
 import { getTimeSrv, TimeSrv } from '../services/TimeSrv';
-import { getAngularLoader, AngularComponent } from '@grafana/runtime';
+import { AngularComponent, getAngularLoader } from '@grafana/runtime';
 import { setPanelAngularComponent } from '../state/reducers';
 import config from 'app/core/config';
-
 // Types
 import { DashboardModel, PanelModel } from '../state';
 import { StoreState } from 'app/types';
-import { LoadingState, DefaultTimeRange, PanelData, PanelPlugin, PanelEvents } from '@grafana/data';
+import { DefaultTimeRange, LoadingState, PanelData, PanelEvents, PanelPlugin } from '@grafana/data';
 import { updateLocation } from 'app/core/actions';
 import { PANEL_BORDER } from 'app/core/constants';
 
@@ -95,8 +92,12 @@ export class PanelChromeAngularUnconnected extends PureComponent<Props, State> {
   onPanelRenderEvent = (payload?: any) => {
     const { alertState } = this.state;
 
-    if (payload && payload.alertState) {
+    if (payload && payload.alertState && this.props.panel.alert) {
       this.setState({ alertState: payload.alertState });
+    } else if (payload && payload.alertState && !this.props.panel.alert) {
+      // when user deletes alert in panel editor the source panel needs to refresh as this is in the mutable state and
+      // will not automatically re render
+      this.setState({ alertState: undefined });
     } else if (payload && alertState) {
       this.setState({ alertState: undefined });
     } else {

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

@@ -290,5 +290,51 @@ describe('PanelModel', () => {
         expect(panelQueryRunner).toBe(sameQueryRunner);
       });
     });
+
+    describe('restoreModel', () => {
+      it('Should clean state and set properties from model', () => {
+        model.restoreModel({
+          title: 'New title',
+          options: { new: true },
+        });
+        expect(model.title).toBe('New title');
+        expect(model.options.new).toBe(true);
+      });
+
+      it('Should delete properties that are now gone on new model', () => {
+        model.someProperty = 'value';
+        model.restoreModel({
+          title: 'New title',
+          options: {},
+        });
+
+        expect(model.someProperty).toBeUndefined();
+      });
+
+      it('Should preserve must keep properties', () => {
+        model.id = 10;
+        model.gridPos = { x: 0, y: 0, h: 10, w: 10 };
+        model.restoreModel({
+          title: 'New title',
+          options: {},
+        });
+
+        expect(model.id).toBe(10);
+        expect(model.gridPos.h).toBe(10);
+      });
+
+      it('Should remove old angular panel specfic props', () => {
+        model.axes = [{ prop: 1 }];
+        model.thresholds = [];
+
+        model.restoreModel({
+          title: 'New title',
+          options: {},
+        });
+
+        expect(model.axes).toBeUndefined();
+        expect(model.thresholds).toBeUndefined();
+      });
+    });
   });
 });

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

@@ -12,10 +12,10 @@ import {
   DataQueryResponseData,
   DataTransformerConfig,
   eventFactory,
+  FieldConfigSource,
   PanelEvents,
   PanelPlugin,
   ScopedVars,
-  FieldConfigSource,
 } from '@grafana/data';
 import { EDIT_PANEL_ID } from 'app/core/constants';
 
@@ -158,6 +158,35 @@ export class PanelModel implements DataConfigSource {
 
   /** Given a persistened PanelModel restores property values */
   restoreModel(model: any) {
+    // Start with clean-up
+    for (const property of Object.keys(this)) {
+      if (notPersistedProperties[property]) {
+        continue;
+      }
+
+      if (mustKeepProps[property]) {
+        continue;
+      }
+
+      if (model[property]) {
+        continue;
+      }
+
+      if (!this.hasOwnProperty(property)) {
+        continue;
+      }
+
+      if (typeof (this as any)[property] === 'function') {
+        continue;
+      }
+
+      if (typeof (this as any)[property] === 'symbol') {
+        continue;
+      }
+
+      delete (this as any)[property];
+    }
+
     // copy properties from persisted model
     for (const property in model) {
       (this as any)[property] = model[property];