From c3936bbae2e1327194e66d0bb14e110b52a60609 Mon Sep 17 00:00:00 2001 From: Oscar Kilhed Date: Thu, 9 May 2024 17:48:20 +0200 Subject: [PATCH] DashboardScene: Discard panel changes disabled/enabled depending of changes (#87137) --------- Co-authored-by: Ivan Ortega Alba Co-authored-by: Dominik Prokop --- .../panel-edit/VizPanelManager.test.tsx | 33 +- .../panel-edit/VizPanelManager.tsx | 26 +- .../saving/DashboardSceneChangeTracker.ts | 47 +- .../saving/getDashboardChanges.test.ts | 423 ++++++++++++++++++ .../saving/getDashboardChanges.ts | 15 +- .../DashboardDatasourceBehaviour.test.tsx | 32 +- .../scene/NavToolbarActions.tsx | 22 + 7 files changed, 561 insertions(+), 37 deletions(-) create mode 100644 public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts diff --git a/public/app/features/dashboard-scene/panel-edit/VizPanelManager.test.tsx b/public/app/features/dashboard-scene/panel-edit/VizPanelManager.test.tsx index db2f71c3a6f..590c519956a 100644 --- a/public/app/features/dashboard-scene/panel-edit/VizPanelManager.test.tsx +++ b/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); }); }); diff --git a/public/app/features/dashboard-scene/panel-edit/VizPanelManager.tsx b/public/app/features/dashboard-scene/panel-edit/VizPanelManager.tsx index eae39a8c39d..2892dcd1663 100644 --- a/public/app/features/dashboard-scene/panel-edit/VizPanelManager.tsx +++ b/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 { 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; diff --git a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts index 26f3287cd7f..4bc186849e9 100644 --- a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts +++ b/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(); } } diff --git a/public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts b/public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts new file mode 100644 index 00000000000..1820d71733f --- /dev/null +++ b/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); + }); +}); diff --git a/public/app/features/dashboard-scene/saving/getDashboardChanges.ts b/public/app/features/dashboard-scene/saving/getDashboardChanges.ts index 4c3b4686030..74c71be1b5c 100644 --- a/public/app/features/dashboard-scene/saving/getDashboardChanges.ts +++ b/public/app/features/dashboard-scene/saving/getDashboardChanges.ts @@ -2,7 +2,7 @@ import jsonMap from 'json-source-map'; import type { AdHocVariableModel, TypedVariableModel } from '@grafana/data'; -import { Dashboard, VariableOption } from '@grafana/schema'; +import { Dashboard, Panel, VariableOption } from '@grafana/schema'; import { jsonDiff } from '../settings/version-history/utils'; @@ -105,3 +105,16 @@ export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Da return hasVariableValueChanges; } + +export function getPanelChanges(saveModel: Panel, originalSaveModel: Panel) { + const diff = jsonDiff(originalSaveModel, saveModel); + const diffCount = Object.values(diff).reduce((acc, cur) => acc + cur.length, 0); + + return { + changedSaveModel: saveModel, + initialSaveModel: originalSaveModel, + diffs: diff, + diffCount, + hasChanges: diffCount > 0, + }; +} diff --git a/public/app/features/dashboard-scene/scene/DashboardDatasourceBehaviour.test.tsx b/public/app/features/dashboard-scene/scene/DashboardDatasourceBehaviour.test.tsx index 86814f13b4b..de5e655eae2 100644 --- a/public/app/features/dashboard-scene/scene/DashboardDatasourceBehaviour.test.tsx +++ b/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,9 +576,12 @@ async function buildTestScene() { title: 'Panel A', pluginId: 'table', key: 'panel-1', - $data: new SceneQueryRunner({ - datasource: { uid: 'grafana' }, - queries: [{ refId: 'A', queryType: 'randomWalk' }], + $data: new SceneDataTransformer({ + transformations: [], + $data: new SceneQueryRunner({ + datasource: { uid: 'grafana' }, + queries: [{ refId: 'A', queryType: 'randomWalk' }], + }), }), }); @@ -586,10 +589,13 @@ async function buildTestScene() { title: 'Panel B', pluginId: 'table', key: 'panel-2', - $data: new SceneQueryRunner({ - datasource: { uid: SHARED_DASHBOARD_QUERY }, - queries: [{ refId: 'A', panelId: 1 }], - $behaviors: [new DashboardDatasourceBehaviour({})], + $data: new SceneDataTransformer({ + transformations: [], + $data: new SceneQueryRunner({ + datasource: { uid: SHARED_DASHBOARD_QUERY }, + queries: [{ refId: 'A', panelId: 1 }], + $behaviors: [new DashboardDatasourceBehaviour({})], + }), }), }); diff --git a/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx b/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx index b3e123b3947..a3c5c20e41d 100644 --- a/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx +++ b/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx @@ -66,6 +66,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 @@ -420,6 +421,7 @@ export function ToolbarActions({ dashboard }: Props) { onClick={editPanel?.onDiscard} tooltip="Discard panel changes" size="sm" + disabled={!isEditedPanelDirty || !isDirty} key="discard" fill="outline" variant="destructive" @@ -629,6 +631,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(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;