[v11.0.x] Dashboard scene: Discard panel changes disabled/enabled depending of changes (#87570)

* DashboardScene: Discard panel changes disabled/enabled depending of changes (#87137)

---------

Co-authored-by: Ivan Ortega Alba <ivanortegaalba@gmail.com>
Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
(cherry picked from commit c3936bbae2)

* Add line number

---------

Co-authored-by: Oscar Kilhed <oscar.kilhed@grafana.com>

.betterer.results

@@ -2413,8 +2413,9 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"]
     ],
     "public/app/features/dashboard-scene/saving/getDashboardChanges.ts:5381": [
-      [0, 0, 0, "Do not use any type assertions.", "0"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
-      [0, 0, 0, "Do not use any type assertions.", "1"]
+      [0, 0, 0, "Do not use any type assertions.", "1"],
+      [0, 0, 0, "Do not use any type assertions.", "2"]
     ],
     "public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

public/app/features/dashboard-scene/panel-edit/VizPanelManager.test.tsx

@@ -1,6 +1,8 @@
 import { map, of } from 'rxjs';
 
 import { DataQueryRequest, DataSourceApi, DataSourceInstanceSettings, LoadingState, PanelData } from '@grafana/data';
+import { calculateFieldTransformer } from '@grafana/data/src/transformations/transformers/calculateField';
+import { mockTransformationsRegistry } from '@grafana/data/src/utils/tests/mockTransformationsRegistry';
 import { config, locationService } from '@grafana/runtime';
 import { SceneQueryRunner, VizPanel } from '@grafana/scenes';
 import { DataQuery, DataSourceJsonData, DataSourceRef } from '@grafana/schema';
@@ -178,12 +180,17 @@ jest.mock('@grafana/runtime', () => ({
   },
 }));
 
+mockTransformationsRegistry([calculateFieldTransformer]);
+
+jest.useFakeTimers();
+
 describe('VizPanelManager', () => {
   describe('When changing plugin', () => {
     it('Should successfully change from one viz type to another', () => {
       const { vizPanelManager } = setupTest('panel-1');
       expect(vizPanelManager.state.panel.state.pluginId).toBe('timeseries');
       vizPanelManager.changePluginType('table');
+
       expect(vizPanelManager.state.panel.state.pluginId).toBe('table');
     });
 
@@ -405,6 +412,8 @@ describe('VizPanelManager', () => {
           datasourceUid: 'gdev-prometheus',
         });
 
+        jest.runAllTimers(); // The detect panel changes is debounced
+        expect(vizPanelManager.state.isDirty).toBe(true);
         expect(vizPanelManager.state.datasource).toEqual(ds2Mock);
         expect(vizPanelManager.state.dsSettings).toEqual(instance2SettingsMock);
       });
@@ -472,6 +481,8 @@ describe('VizPanelManager', () => {
             },
           });
 
+          jest.runAllTimers(); // The detect panel changes is debounced
+          expect(vizPanelManager.state.isDirty).toBe(true);
           expect((panel.state.$timeRange?.state as PanelTimeRangeState).timeFrom).toBe('2h');
         });
 
@@ -515,7 +526,7 @@ describe('VizPanelManager', () => {
       });
 
       describe('max data points and interval', () => {
-        it('max data points', async () => {
+        it('should update max data points', async () => {
           const { vizPanelManager } = setupTest('panel-1');
           vizPanelManager.activate();
           await Promise.resolve();
@@ -534,10 +545,12 @@ describe('VizPanelManager', () => {
             maxDataPoints: 100,
           });
 
+          jest.runAllTimers(); // The detect panel changes is debounced
+          expect(vizPanelManager.state.isDirty).toBe(true);
           expect(dataObj.state.maxDataPoints).toBe(100);
         });
 
-        it('max data points', async () => {
+        it('should update min interval', async () => {
           const { vizPanelManager } = setupTest('panel-1');
           vizPanelManager.activate();
           await Promise.resolve();
@@ -556,6 +569,8 @@ describe('VizPanelManager', () => {
             minInterval: '1s',
           });
 
+          jest.runAllTimers(); // The detect panel changes is debounced
+          expect(vizPanelManager.state.isDirty).toBe(true);
           expect(dataObj.state.minInterval).toBe('1s');
         });
       });
@@ -579,6 +594,8 @@ describe('VizPanelManager', () => {
             queries: [],
           });
 
+          jest.runAllTimers(); // The detect panel changes is debounced
+          expect(vizPanelManager.state.isDirty).toBe(true);
           expect(dataObj.state.cacheTimeout).toBe('60');
           expect(dataObj.state.queryCachingTTL).toBe(200000);
         });
@@ -616,6 +633,8 @@ describe('VizPanelManager', () => {
           },
         } as DataSourceInstanceSettings);
 
+        jest.runAllTimers(); // The detect panel changes is debounced
+        expect(vizPanelManager.state.isDirty).toBe(true);
         expect(vizPanelManager.queryRunner.state.datasource).toEqual({
           uid: 'gdev-prometheus',
           type: 'grafana-prometheus-datasource',
@@ -643,6 +662,8 @@ describe('VizPanelManager', () => {
           },
         } as DataSourceInstanceSettings);
 
+        jest.runAllTimers(); // The detect panel changes is debounced
+        expect(vizPanelManager.state.isDirty).toBe(true);
         expect(vizPanelManager.queryRunner.state.datasource).toEqual({
           uid: SHARED_DASHBOARD_QUERY,
           type: 'datasource',
@@ -670,6 +691,8 @@ describe('VizPanelManager', () => {
           },
         } as DataSourceInstanceSettings);
 
+        jest.runAllTimers(); // The detect panel changes is debounced
+        expect(vizPanelManager.state.isDirty).toBe(true);
         expect(vizPanelManager.queryRunner.state.datasource).toEqual({
           uid: 'gdev-prometheus',
           type: 'grafana-prometheus-datasource',
@@ -691,6 +714,8 @@ describe('VizPanelManager', () => {
       vizPanelManager.dataTransformer.reprocessTransformations = reprocessMock;
       vizPanelManager.changeTransformations([{ id: 'calculateField', options: {} }]);
 
+      jest.runAllTimers(); // The detect panel changes is debounced
+      expect(vizPanelManager.state.isDirty).toBe(true);
       expect(reprocessMock).toHaveBeenCalledTimes(1);
       expect(vizPanelManager.dataTransformer.state.transformations).toEqual([{ id: 'calculateField', options: {} }]);
     });
@@ -716,6 +741,8 @@ describe('VizPanelManager', () => {
           },
         ]);
 
+        jest.runAllTimers(); // The detect panel changes is debounced
+        expect(vizPanelManager.state.isDirty).toBe(true);
         expect(vizPanelManager.queryRunner.state.queries).toEqual([
           {
             datasource: {
@@ -763,6 +790,8 @@ describe('VizPanelManager', () => {
           },
         ]);
 
+        jest.runAllTimers(); // The detect panel changes is debounced
+        expect(vizPanelManager.state.isDirty).toBe(true);
         expect(vizPanelManager.queryRunner.state.queries[0].panelId).toBe(panelWithQueriesOnly.id);
       });
     });

public/app/features/dashboard-scene/panel-edit/VizPanelManager.tsx

@@ -1,4 +1,5 @@
 import { css } from '@emotion/css';
+import { debounce } from 'lodash';
 import React, { useEffect } from 'react';
 
 import {
@@ -20,6 +21,7 @@ import {
   SceneObjectBase,
   SceneObjectRef,
   SceneObjectState,
+  SceneObjectStateChangedEvent,
   SceneQueryRunner,
   VizPanel,
   sceneUtils,
@@ -34,10 +36,12 @@ import { updateQueries } from 'app/features/query/state/updateQueries';
 import { GrafanaQuery } from 'app/plugins/datasource/grafana/types';
 import { QueryGroupOptions } from 'app/types';
 
+import { DashboardSceneChangeTracker } from '../saving/DashboardSceneChangeTracker';
+import { getPanelChanges } from '../saving/getDashboardChanges';
 import { DashboardGridItem, RepeatDirection } from '../scene/DashboardGridItem';
 import { LibraryVizPanel } from '../scene/LibraryVizPanel';
 import { PanelTimeRange, PanelTimeRangeState } from '../scene/PanelTimeRange';
-import { gridItemToPanel } from '../serialization/transformSceneToSaveModel';
+import { gridItemToPanel, vizPanelToPanel } from '../serialization/transformSceneToSaveModel';
 import { getDashboardSceneFor, getPanelIdForVizPanel, getQueryRunnerFor } from '../utils/utils';
 
 export interface VizPanelManagerState extends SceneObjectState {
@@ -49,6 +53,7 @@ export interface VizPanelManagerState extends SceneObjectState {
   repeat?: string;
   repeatDirection?: RepeatDirection;
   maxPerRow?: number;
+  isDirty?: boolean;
 }
 
 export enum DisplayMode {
@@ -95,8 +100,27 @@ export class VizPanelManager extends SceneObjectBase<VizPanelManagerState> {
 
   private _onActivate() {
     this.loadDataSource();
+    const changesSub = this.subscribeToEvent(SceneObjectStateChangedEvent, this._handleStateChange);
+
+    return () => {
+      changesSub.unsubscribe();
+    };
   }
 
+  private _detectPanelModelChanges = debounce(() => {
+    const { hasChanges } = getPanelChanges(
+      vizPanelToPanel(this.state.sourcePanel.resolve()),
+      vizPanelToPanel(this.state.panel)
+    );
+    this.setState({ isDirty: hasChanges });
+  }, 250);
+
+  private _handleStateChange = (event: SceneObjectStateChangedEvent) => {
+    if (DashboardSceneChangeTracker.isUpdatingPersistedState(event)) {
+      this._detectPanelModelChanges();
+    }
+  };
+
   private async loadDataSource() {
     const dataObj = this.state.panel.state.$data;
 

public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts

@@ -36,10 +36,10 @@ export class DashboardSceneChangeTracker {
     this._dashboard = dashboard;
   }
 
-  private onStateChanged({ payload }: SceneObjectStateChangedEvent) {
+  static isUpdatingPersistedState({ payload }: SceneObjectStateChangedEvent) {
     // If there are no changes in the state, the check is not needed
     if (Object.keys(payload.partialUpdate).length === 0) {
-      return;
+      return false;
     }
 
     // Any change in the panel should trigger a change detection
@@ -50,7 +50,7 @@ export class DashboardSceneChangeTracker {
       payload.changedObject instanceof DashboardGridItem ||
       payload.changedObject instanceof PanelTimeRange
     ) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     // VizPanelManager includes the repeat configuration
     if (payload.changedObject instanceof VizPanelManager) {
@@ -59,27 +59,27 @@ export class DashboardSceneChangeTracker {
         Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'repeatDirection') ||
         Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'maxPerRow')
       ) {
-        return this.detectSaveModelChanges();
+        return true;
       }
     }
     // SceneQueryRunner includes the DS configuration
     if (payload.changedObject instanceof SceneQueryRunner) {
       if (!Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'data')) {
-        return this.detectSaveModelChanges();
+        return true;
       }
     }
     // SceneDataTransformer includes the transformation configuration
     if (payload.changedObject instanceof SceneDataTransformer) {
       if (!Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'data')) {
-        return this.detectSaveModelChanges();
+        return true;
       }
     }
     if (payload.changedObject instanceof VizPanelLinks) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     if (payload.changedObject instanceof LibraryVizPanel) {
       if (Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'name')) {
-        return this.detectSaveModelChanges();
+        return true;
       }
     }
     if (payload.changedObject instanceof SceneRefreshPicker) {
@@ -87,47 +87,54 @@ export class DashboardSceneChangeTracker {
         Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'intervals') ||
         Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'refresh')
       ) {
-        return this.detectSaveModelChanges();
+        return true;
       }
     }
     if (payload.changedObject instanceof behaviors.CursorSync) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     if (payload.changedObject instanceof SceneDataLayerSet) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     if (payload.changedObject instanceof DashboardGridItem) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     if (payload.changedObject instanceof SceneGridLayout) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     if (payload.changedObject instanceof DashboardScene) {
       if (Object.keys(payload.partialUpdate).some((key) => PERSISTED_PROPS.includes(key))) {
-        return this.detectSaveModelChanges();
+        return true;
       }
     }
     if (payload.changedObject instanceof SceneTimeRange) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     if (payload.changedObject instanceof DashboardControls) {
       if (Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'hideTimeControls')) {
-        return this.detectSaveModelChanges();
+        return true;
       }
     }
     if (payload.changedObject instanceof SceneVariableSet) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     if (payload.changedObject instanceof DashboardAnnotationsDataLayer) {
       if (!Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'data')) {
-        return this.detectSaveModelChanges();
+        return true;
       }
     }
     if (payload.changedObject instanceof behaviors.LiveNowTimer) {
-      return this.detectSaveModelChanges();
+      return true;
     }
     if (isSceneVariableInstance(payload.changedObject)) {
-      return this.detectSaveModelChanges();
+      return true;
+    }
+    return false;
+  }
+
+  private onStateChanged(event: SceneObjectStateChangedEvent) {
+    if (DashboardSceneChangeTracker.isUpdatingPersistedState(event)) {
+      this.detectSaveModelChanges();
     }
   }
 

public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts

@@ -0,0 +1,423 @@
+import { Dashboard, Panel } from '@grafana/schema';
+
+import { getDashboardChanges, getPanelChanges } from './getDashboardChanges';
+
+describe('getDashboardChanges', () => {
+  const initial: Dashboard = {
+    id: 1,
+    title: 'Dashboard 1',
+    time: {
+      from: 'now-7d',
+      to: 'now',
+    },
+    refresh: '1h',
+    version: 1,
+    schemaVersion: 1,
+    templating: {
+      list: [
+        {
+          name: 'var1',
+          type: 'query',
+          query: 'query1',
+          current: {
+            value: 'value1',
+            text: 'text1',
+          },
+          options: [],
+        },
+      ],
+    },
+  };
+  it('should return the correct result when no changes', () => {
+    const changed = { ...initial };
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...changed,
+      },
+      diffs: {},
+      diffCount: 0,
+      hasChanges: false,
+      hasTimeChanges: false,
+      isNew: false,
+      hasVariableValueChanges: false,
+      hasRefreshChange: false,
+    };
+
+    const result = getDashboardChanges(initial, changed, false, false, false);
+
+    expect(result).toEqual(expectedChanges);
+  });
+
+  it('should return the correct result when is new', () => {
+    const newDashInitial = {
+      ...initial,
+      version: 0,
+    };
+    const changed = {
+      ...newDashInitial,
+      version: 0,
+    };
+
+    const expectedChanges = {
+      changedSaveModel: {
+        ...newDashInitial,
+      },
+      initialSaveModel: {
+        ...changed,
+      },
+      diffs: {},
+      diffCount: 0,
+      hasChanges: false,
+      hasTimeChanges: false,
+      isNew: true,
+      hasVariableValueChanges: false,
+      hasRefreshChange: false,
+    };
+
+    const result = getDashboardChanges(newDashInitial, changed, false, false, false);
+
+    expect(result).toEqual(expectedChanges);
+  });
+
+  it('should return the correct result when the time changes but they are not preserved', () => {
+    const changed = {
+      ...initial,
+      time: {
+        from: 'now-1d',
+        to: 'now',
+      },
+    };
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...initial,
+      },
+      diffs: {},
+      diffCount: 0,
+      hasChanges: false,
+      hasTimeChanges: true,
+      isNew: false,
+      hasVariableValueChanges: false,
+      hasRefreshChange: false,
+    };
+
+    const result = getDashboardChanges(initial, changed, false, false, false);
+
+    expect(result).toEqual(expectedChanges);
+  });
+
+  it('should return the correct result when the time changes and they are preserved', () => {
+    const changed = {
+      ...initial,
+      time: {
+        from: 'now-1d',
+        to: 'now',
+      },
+    };
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...changed,
+      },
+      diffs: {
+        time: [
+          {
+            endLineNumber: expect.any(Number),
+            op: 'replace',
+            originalValue: 'now-7d',
+            path: ['time', 'from'],
+            startLineNumber: expect.any(Number),
+            value: 'now-1d',
+          },
+        ],
+      },
+      diffCount: 1,
+      hasChanges: true,
+      hasTimeChanges: true,
+      isNew: false,
+      hasVariableValueChanges: false,
+      hasRefreshChange: false,
+    };
+
+    const result = getDashboardChanges(initial, changed, true, false, false);
+
+    expect(result).toEqual(expectedChanges);
+  });
+
+  it('should return the correct result when the refresh changes but it is not preserved', () => {
+    const changed = {
+      ...initial,
+      refresh: '2h',
+    };
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...initial,
+      },
+      diffs: {},
+      diffCount: 0,
+      hasChanges: false,
+      hasTimeChanges: false,
+      isNew: false,
+      hasVariableValueChanges: false,
+      hasRefreshChange: true,
+    };
+
+    const result = getDashboardChanges(initial, changed, false, false, false);
+
+    expect(result).toEqual(expectedChanges);
+  });
+
+  it('should return the correct result when the refresh changes and it is preserved', () => {
+    const changed = {
+      ...initial,
+      refresh: '2h',
+    };
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...changed,
+      },
+      diffs: {
+        refresh: [
+          {
+            endLineNumber: expect.any(Number),
+            op: 'replace',
+            originalValue: '1h',
+            path: ['refresh'],
+            startLineNumber: expect.any(Number),
+            value: '2h',
+          },
+        ],
+      },
+      diffCount: 1,
+      hasChanges: true,
+      hasTimeChanges: false,
+      isNew: false,
+      hasVariableValueChanges: false,
+      hasRefreshChange: true,
+    };
+
+    const result = getDashboardChanges(initial, changed, false, false, true);
+
+    expect(result).toEqual(expectedChanges);
+  });
+
+  it('should return the correct result when the variable value changes but it is not preserved', () => {
+    const changed = {
+      ...initial,
+      templating: {
+        list: [
+          {
+            name: 'var1',
+            type: 'query',
+            query: 'query1',
+            current: {
+              value: 'value2',
+              text: 'text1',
+            },
+            options: [],
+          },
+        ],
+      },
+    } as Dashboard;
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...initial,
+      },
+      diffs: {},
+      diffCount: 0,
+      hasChanges: false,
+      hasTimeChanges: false,
+      isNew: false,
+      hasVariableValueChanges: true,
+      hasRefreshChange: false,
+    };
+
+    const result = getDashboardChanges(initial, changed, false, false, false);
+
+    expect(result).toEqual(expectedChanges);
+  });
+
+  it('should return the correct result when the variable value changes', () => {
+    const changed = {
+      ...initial,
+      templating: {
+        list: [
+          {
+            name: 'var1',
+            type: 'query',
+            query: 'query1',
+            current: {
+              value: 'value2',
+              text: 'text1',
+            },
+            options: [],
+          },
+        ],
+      },
+    } as Dashboard;
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...changed,
+      },
+      diffs: {
+        templating: [
+          {
+            endLineNumber: 17,
+            op: 'replace',
+            originalValue: 'value1',
+            path: ['templating', 'list', '0', 'current', 'value'],
+            startLineNumber: 17,
+            value: 'value2',
+          },
+        ],
+      },
+      diffCount: 1,
+      hasChanges: true,
+      hasTimeChanges: false,
+      isNew: false,
+      hasVariableValueChanges: true,
+      hasRefreshChange: false,
+    };
+
+    const result = getDashboardChanges(initial, changed, false, true, false);
+
+    expect(result).toEqual(expectedChanges);
+  });
+});
+
+describe('getPanelChanges', () => {
+  const initial: Panel = {
+    id: 1,
+    type: 'graph',
+    title: 'Panel 1',
+    gridPos: {
+      x: 0,
+      y: 0,
+      w: 12,
+      h: 8,
+    },
+    targets: [
+      {
+        refId: 'A',
+        query: 'query1',
+      },
+    ],
+  };
+
+  it('should return the correct result when no changes', () => {
+    const changed = { ...initial };
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...changed,
+      },
+      diffs: {},
+      diffCount: 0,
+      hasChanges: false,
+    };
+
+    expect(getPanelChanges(initial, changed)).toEqual(expectedChanges);
+  });
+
+  it('should return the correct result when there is some changes', () => {
+    const changed = {
+      ...initial,
+      title: 'Panel 2',
+      type: 'table',
+      gridPos: {
+        ...initial.gridPos,
+        x: 1,
+      },
+      targets: [
+        {
+          refId: 'A',
+          query: 'query2',
+        },
+      ],
+    } as Panel;
+
+    const expectedChanges = {
+      initialSaveModel: {
+        ...initial,
+      },
+      changedSaveModel: {
+        ...changed,
+      },
+      diffs: {
+        title: [
+          {
+            endLineNumber: 3,
+            op: 'replace',
+            originalValue: 'Panel 1',
+            path: ['title'],
+            startLineNumber: 3,
+            value: 'Panel 2',
+          },
+        ],
+        type: [
+          {
+            endLineNumber: 2,
+            op: 'replace',
+            originalValue: 'graph',
+            path: ['type'],
+            startLineNumber: 2,
+            value: 'table',
+          },
+        ],
+        gridPos: [
+          {
+            endLineNumber: 5,
+            op: 'replace',
+            originalValue: 0,
+            path: ['gridPos', 'x'],
+            startLineNumber: 5,
+            value: 1,
+          },
+        ],
+        targets: [
+          {
+            endLineNumber: 13,
+            op: 'replace',
+            originalValue: 'query1',
+            path: ['targets', '0', 'query'],
+            startLineNumber: 13,
+            value: 'query2',
+          },
+        ],
+      },
+      diffCount: 4,
+      hasChanges: true,
+    };
+
+    expect(getPanelChanges(changed, initial)).toEqual(expectedChanges);
+  });
+});

public/app/features/dashboard-scene/saving/getDashboardChanges.ts

@@ -1,10 +1,34 @@
-import { compare, Operation } from 'fast-json-patch';
 // @ts-ignore
 import jsonMap from 'json-source-map';
-import { flow, get, isEqual, sortBy, tail } from 'lodash';
 
-import { AdHocVariableModel, TypedVariableModel } from '@grafana/data';
+import type { AdHocVariableModel, TypedVariableModel } from '@grafana/data';
-import { Dashboard } from '@grafana/schema';
+import { Dashboard, Panel, VariableOption } from '@grafana/schema';
+
+import { jsonDiff } from '../settings/version-history/utils';
+
+export function get(obj: any, keys: string[]) {
+  try {
+    let val = obj;
+    for (const key of keys) {
+      val = val[key];
+    }
+    return val;
+  } catch (err) {
+    return undefined;
+  }
+}
+
+export function deepEqual(a: string | string[], b: string | string[]) {
+  return (
+    typeof a === typeof b &&
+    ((typeof a === 'string' && a === b) ||
+      (Array.isArray(a) && a.length === b.length && a.every((val, i) => val === b[i])))
+  );
+}
+
+export function isEqual(a: VariableOption | undefined, b: VariableOption | undefined) {
+  return a === b || (a && b && a.selected === b.selected && deepEqual(a.text, b.text) && deepEqual(a.value, b.value));
+}
 
 export function getDashboardChanges(
   initial: Dashboard,
@@ -28,11 +52,8 @@ export function getDashboardChanges(
   }
 
   const diff = jsonDiff(initialSaveModel, changedSaveModel);
+  const diffCount = Object.values(diff).reduce((acc, cur) => acc + cur.length, 0);
 
-  let diffCount = 0;
-  for (const d of Object.values(diff)) {
-    diffCount += d.length;
-  }
   return {
     changedSaveModel,
     initialSaveModel,
@@ -63,7 +84,7 @@ export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Da
     }
 
     // Old schema property that never should be in persisted model
-    if (original.current && Object.hasOwn(original.current, 'selected')) {
+    if (original.current) {
       delete original.current.selected;
     }
 
@@ -75,80 +96,25 @@ export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Da
       const typed = variable as TypedVariableModel;
       if (typed.type === 'adhoc') {
         typed.filters = (original as AdHocVariableModel).filters;
-      } else {
+      } else if (typed.type !== 'groupby') {
-        if (typed.type !== 'groupby') {
         variable.current = original.current;
         variable.options = original.options;
       }
     }
   }
-  }
 
   return hasVariableValueChanges;
 }
 
-export type Diff = {
+export function getPanelChanges(saveModel: Panel, originalSaveModel: Panel) {
-  op: 'add' | 'replace' | 'remove' | 'copy' | 'test' | '_get' | 'move';
+  const diff = jsonDiff(originalSaveModel, saveModel);
-  value: unknown;
+  const diffCount = Object.values(diff).reduce((acc, cur) => acc + cur.length, 0);
-  originalValue: unknown;
-  path: string[];
-  startLineNumber: number;
-};
-
-export type Diffs = {
-  [key: string]: Diff[];
-};
-
-export type JSONValue = string | Dashboard;
-
-export const jsonDiff = (lhs: JSONValue, rhs: JSONValue): Diffs => {
-  const diffs = compare(lhs, rhs);
-  const lhsMap = jsonMap.stringify(lhs, null, 2);
-  const rhsMap = jsonMap.stringify(rhs, null, 2);
-
-  const getDiffInformation = (diffs: Operation[]): Diff[] => {
-    return diffs.map((diff) => {
-      let originalValue = undefined;
-      let value = undefined;
-      let startLineNumber = 0;
-
-      const path = tail(diff.path.split('/'));
-
-      if (diff.op === 'replace' && rhsMap.pointers[diff.path]) {
-        originalValue = get(lhs, path);
-        value = diff.value;
-        startLineNumber = rhsMap.pointers[diff.path].value.line;
-      }
-      if (diff.op === 'add' && rhsMap.pointers[diff.path]) {
-        value = diff.value;
-        startLineNumber = rhsMap.pointers[diff.path].value.line;
-      }
-      if (diff.op === 'remove' && lhsMap.pointers[diff.path]) {
-        originalValue = get(lhs, path);
-        startLineNumber = lhsMap.pointers[diff.path].value.line;
-      }
 
   return {
-        op: diff.op,
+    changedSaveModel: saveModel,
-        value,
+    initialSaveModel: originalSaveModel,
-        path,
+    diffs: diff,
-        originalValue,
+    diffCount,
-        startLineNumber,
+    hasChanges: diffCount > 0,
-      };
-    });
   };
-
+}
-  const sortByLineNumber = (diffs: Diff[]) => sortBy(diffs, 'startLineNumber');
-  const groupByPath = (diffs: Diff[]) =>
-    diffs.reduce<Record<string, Diff[]>>((acc, value) => {
-      const groupKey: string = value.path[0];
-      if (!acc[groupKey]) {
-        acc[groupKey] = [];
-      }
-      acc[groupKey].push(value);
-      return acc;
-    }, {});
-
-  //   return 1;
-  return flow([getDiffInformation, sortByLineNumber, groupByPath])(diffs);
-};

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

@@ -11,7 +11,7 @@ import {
 } from '@grafana/data';
 import { getPanelPlugin } from '@grafana/data/test/__mocks__/pluginMocks';
 import { setPluginImportUtils } from '@grafana/runtime';
-import { SceneGridLayout, SceneQueryRunner, VizPanel } from '@grafana/scenes';
+import { SceneDataTransformer, SceneGridLayout, SceneQueryRunner, VizPanel } from '@grafana/scenes';
 import { SHARED_DASHBOARD_QUERY } from 'app/plugins/datasource/dashboard';
 import { DASHBOARD_DATASOURCE_PLUGIN_ID } from 'app/plugins/datasource/dashboard/types';
 
@@ -104,12 +104,12 @@ describe('DashboardDatasourceBehaviour', () => {
 
     it('Should re-run query of dashboardDS panel when source query re-runs', async () => {
       // spy on runQueries that will be called by the behaviour
-      const spy = jest.spyOn(dashboardDSPanel.state.$data as SceneQueryRunner, 'runQueries');
+      const spy = jest.spyOn(dashboardDSPanel.state.$data!.state.$data as SceneQueryRunner, 'runQueries');
 
       // deactivate scene to mimic going into panel edit
       sceneDeactivate();
       // run source panel queries and update request ID
-      (sourcePanel.state.$data as SceneQueryRunner).runQueries();
+      (sourcePanel.state.$data!.state.$data as SceneQueryRunner).runQueries();
 
       await new Promise((r) => setTimeout(r, 1));
 
@@ -121,7 +121,7 @@ describe('DashboardDatasourceBehaviour', () => {
 
     it('Should not run query of dashboardDS panel when source panel queries do not change', async () => {
       // spy on runQueries
-      const spy = jest.spyOn(dashboardDSPanel.state.$data as SceneQueryRunner, 'runQueries');
+      const spy = jest.spyOn(dashboardDSPanel.state.$data!.state.$data as SceneQueryRunner, 'runQueries');
 
       // deactivate scene to mimic going into panel edit
       sceneDeactivate();
@@ -273,10 +273,10 @@ describe('DashboardDatasourceBehaviour', () => {
 
     it('Should exit behaviour early if not in a dashboard scene', async () => {
       // spy on runQueries
-      const spy = jest.spyOn(dashboardDSPanel.state.$data as SceneQueryRunner, 'runQueries');
+      const spy = jest.spyOn(dashboardDSPanel.state.$data!.state.$data as SceneQueryRunner, 'runQueries');
 
       const vizPanelManager = new VizPanelManager({
-        panel: dashboardDSPanel.clone({ $data: undefined }),
+        panel: dashboardDSPanel.clone(),
         $data: dashboardDSPanel.state.$data?.clone(),
         sourcePanel: dashboardDSPanel.getRef(),
       });
@@ -576,21 +576,27 @@ async function buildTestScene() {
     title: 'Panel A',
     pluginId: 'table',
     key: 'panel-1',
+    $data: new SceneDataTransformer({
+      transformations: [],
       $data: new SceneQueryRunner({
         datasource: { uid: 'grafana' },
         queries: [{ refId: 'A', queryType: 'randomWalk' }],
       }),
+    }),
   });
 
   const dashboardDSPanel = new VizPanel({
     title: 'Panel B',
     pluginId: 'table',
     key: 'panel-2',
+    $data: new SceneDataTransformer({
+      transformations: [],
       $data: new SceneQueryRunner({
         datasource: { uid: SHARED_DASHBOARD_QUERY },
         queries: [{ refId: 'A', panelId: 1 }],
         $behaviors: [new DashboardDatasourceBehaviour({})],
       }),
+    }),
   });
 
   const scene = new DashboardScene({

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

@@ -65,6 +65,7 @@ export function ToolbarActions({ dashboard }: Props) {
   const styles = useStyles2(getStyles);
   const isEditingPanel = Boolean(editPanel);
   const isViewingPanel = Boolean(viewPanelScene);
+  const isEditedPanelDirty = useVizManagerDirty(editPanel);
   const isEditingLibraryPanel = useEditingLibraryPanel(editPanel);
   const hasCopiedPanel = Boolean(copiedPanel);
   // Means we are not in settings view, fullscreen panel or edit panel
@@ -412,6 +413,7 @@ export function ToolbarActions({ dashboard }: Props) {
         onClick={editPanel?.onDiscard}
         tooltip="Discard panel changes"
         size="sm"
+        disabled={!isEditedPanelDirty || !isDirty}
         key="discard"
         fill="outline"
         variant="destructive"
@@ -621,6 +623,26 @@ function useEditingLibraryPanel(panelEditor?: PanelEditor) {
   return isEditingLibraryPanel;
 }
 
+// This hook handles when panelEditor is not defined to avoid conditionally hook usage
+function useVizManagerDirty(panelEditor?: PanelEditor) {
+  const [isDirty, setIsDirty] = useState<Boolean>(false);
+
+  useEffect(() => {
+    if (panelEditor) {
+      const unsub = panelEditor.state.vizManager.subscribeToState((vizManagerState) =>
+        setIsDirty(vizManagerState.isDirty || false)
+      );
+      return () => {
+        unsub.unsubscribe();
+      };
+    }
+    setIsDirty(false);
+    return;
+  }, [panelEditor]);
+
+  return isDirty;
+}
+
 interface ToolbarAction {
   group: string;
   condition?: boolean | string;

public/app/features/dashboard-scene/settings/version-history/DiffTitle.tsx

@@ -12,7 +12,14 @@ type DiffTitleProps = {
   title: string;
 };
 
-const replaceDiff: Diff = { op: 'replace', originalValue: undefined, path: [''], value: undefined, startLineNumber: 0 };
+const replaceDiff: Diff = {
+  op: 'replace',
+  originalValue: undefined,
+  path: [''],
+  value: undefined,
+  startLineNumber: 0,
+  endLineNumber: 0,
+};
 
 export const DiffTitle = ({ diff, title }: DiffTitleProps) => {
   const styles = useStyles2(getDiffTitleStyles);

public/app/features/dashboard-scene/settings/version-history/utils.test.ts

@@ -216,6 +216,7 @@ describe('jsonDiff', () => {
     const expected = {
       description: [
         {
+          endLineNumber: 14,
           op: 'add',
           originalValue: undefined,
           path: ['description'],
@@ -225,6 +226,7 @@ describe('jsonDiff', () => {
       ],
       graphTooltip: [
         {
+          endLineNumber: 17,
           op: 'replace',
           originalValue: 0,
           path: ['graphTooltip'],
@@ -234,6 +236,7 @@ describe('jsonDiff', () => {
       ],
       panels: [
         {
+          endLineNumber: 23,
           op: 'add',
           originalValue: undefined,
           path: ['panels', '0'],
@@ -245,6 +248,7 @@ describe('jsonDiff', () => {
       ],
       tags: [
         {
+          endLineNumber: 27,
           op: 'add',
           originalValue: undefined,
           path: ['tags', '0'],
@@ -254,6 +258,7 @@ describe('jsonDiff', () => {
       ],
       timepicker: [
         {
+          endLineNumber: 49,
           op: 'add',
           originalValue: undefined,
           path: ['timepicker', 'refresh_intervals'],
@@ -263,6 +268,7 @@ describe('jsonDiff', () => {
       ],
       timezone: [
         {
+          endLineNumber: 51,
           op: 'replace',
           originalValue: '',
           path: ['timezone'],
@@ -272,6 +278,7 @@ describe('jsonDiff', () => {
       ],
       title: [
         {
+          endLineNumber: 52,
           op: 'replace',
           originalValue: 'test dashboard',
           path: ['title'],
@@ -281,6 +288,7 @@ describe('jsonDiff', () => {
       ],
       version: [
         {
+          endLineNumber: 54,
           op: 'replace',
           originalValue: 2,
           path: ['version'],

public/app/features/dashboard-scene/settings/version-history/utils.ts

@@ -3,21 +3,20 @@ import { compare, Operation } from 'fast-json-patch';
 import jsonMap from 'json-source-map';
 import { flow, get, isArray, isEmpty, last, sortBy, tail, toNumber, isNaN } from 'lodash';
 
-import { Dashboard } from '@grafana/schema';
-
 export type Diff = {
   op: 'add' | 'replace' | 'remove' | 'copy' | 'test' | '_get' | 'move';
   value: unknown;
   originalValue: unknown;
   path: string[];
   startLineNumber: number;
+  endLineNumber: number;
 };
 
 export type Diffs = {
   [key: string]: Diff[];
 };
 
-export type JSONValue = string | Dashboard;
+type JSONValue = string | Object;
 
 export const jsonDiff = (lhs: JSONValue, rhs: JSONValue): Diffs => {
   const diffs = compare(lhs, rhs);
@@ -29,6 +28,7 @@ export const jsonDiff = (lhs: JSONValue, rhs: JSONValue): Diffs => {
       let originalValue = undefined;
       let value = undefined;
       let startLineNumber = 0;
+      let endLineNumber = 0;
 
       const path = tail(diff.path.split('/'));
 
@@ -36,14 +36,17 @@ export const jsonDiff = (lhs: JSONValue, rhs: JSONValue): Diffs => {
         originalValue = get(lhs, path);
         value = diff.value;
         startLineNumber = rhsMap.pointers[diff.path].value.line;
+        endLineNumber = rhsMap.pointers[diff.path].valueEnd.line;
       }
       if (diff.op === 'add' && rhsMap.pointers[diff.path]) {
         value = diff.value;
         startLineNumber = rhsMap.pointers[diff.path].value.line;
+        endLineNumber = rhsMap.pointers[diff.path].valueEnd.line;
       }
       if (diff.op === 'remove' && lhsMap.pointers[diff.path]) {
         originalValue = get(lhs, path);
         startLineNumber = lhsMap.pointers[diff.path].value.line;
+        endLineNumber = lhsMap.pointers[diff.path].valueEnd.line;
       }
 
       return {
@@ -52,6 +55,7 @@ export const jsonDiff = (lhs: JSONValue, rhs: JSONValue): Diffs => {
         path,
         originalValue,
         startLineNumber,
+        endLineNumber,
       };
     });
   };

public/app/features/dashboard/components/SaveDashboard/SaveDashboardDiff.tsx

@@ -63,7 +63,16 @@ export const SaveDashboardDiff = ({
     <Stack direction="column" gap={1}>
       {hasFolderChanges && (
         <DiffGroup
-          diffs={[{ op: 'replace', value: newFolder, originalValue: oldFolder, path: [], startLineNumber: 0 }]}
+          diffs={[
+            {
+              op: 'replace',
+              value: newFolder,
+              originalValue: oldFolder,
+              path: [],
+              startLineNumber: 0,
+              endLineNumber: 0,
+            },
+          ]}
           key={'folder'}
           title={'folder'}
         />