From 27bc0c19ab845094c5d4cbf83532fb4bb99474dd Mon Sep 17 00:00:00 2001 From: Ivan Ortega Alba Date: Tue, 9 Apr 2024 17:48:10 +0200 Subject: [PATCH] DashboardScene: Detect changes when editing a panel (#85708) --- .../saving/DashboardSceneChangeTracker.ts | 76 ++++++++-- .../scene/DashboardScene.test.tsx | 133 +++++++++++++++++- 2 files changed, 195 insertions(+), 14 deletions(-) diff --git a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts index 6fd1df0bc81..26f3287cd7f 100644 --- a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts +++ b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts @@ -2,19 +2,26 @@ import { Unsubscribable } from 'rxjs'; import { SceneDataLayerSet, + SceneDataTransformer, SceneGridLayout, SceneObjectStateChangedEvent, + SceneQueryRunner, SceneRefreshPicker, SceneTimeRange, SceneVariableSet, + VizPanel, behaviors, } from '@grafana/scenes'; import { createWorker } from 'app/features/dashboard-scene/saving/createDetectChangesWorker'; +import { VizPanelManager } from '../panel-edit/VizPanelManager'; import { DashboardAnnotationsDataLayer } from '../scene/DashboardAnnotationsDataLayer'; import { DashboardControls } from '../scene/DashboardControls'; import { DashboardGridItem } from '../scene/DashboardGridItem'; import { DashboardScene, PERSISTED_PROPS } from '../scene/DashboardScene'; +import { LibraryVizPanel } from '../scene/LibraryVizPanel'; +import { VizPanelLinks } from '../scene/PanelLinks'; +import { PanelTimeRange } from '../scene/PanelTimeRange'; import { transformSceneToSaveModel } from '../serialization/transformSceneToSaveModel'; import { isSceneVariableInstance } from '../settings/variables/utils'; @@ -30,52 +37,97 @@ export class DashboardSceneChangeTracker { } private onStateChanged({ payload }: SceneObjectStateChangedEvent) { + // If there are no changes in the state, the check is not needed + if (Object.keys(payload.partialUpdate).length === 0) { + return; + } + + // Any change in the panel should trigger a change detection + // The VizPanelManager includes configuration for the panel like repeat + // The PanelTimeRange includes the overrides configuration + if ( + payload.changedObject instanceof VizPanel || + payload.changedObject instanceof DashboardGridItem || + payload.changedObject instanceof PanelTimeRange + ) { + return this.detectSaveModelChanges(); + } + // VizPanelManager includes the repeat configuration + if (payload.changedObject instanceof VizPanelManager) { + if ( + Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'repeat') || + Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'repeatDirection') || + Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'maxPerRow') + ) { + return this.detectSaveModelChanges(); + } + } + // SceneQueryRunner includes the DS configuration + if (payload.changedObject instanceof SceneQueryRunner) { + if (!Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'data')) { + return this.detectSaveModelChanges(); + } + } + // SceneDataTransformer includes the transformation configuration + if (payload.changedObject instanceof SceneDataTransformer) { + if (!Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'data')) { + return this.detectSaveModelChanges(); + } + } + if (payload.changedObject instanceof VizPanelLinks) { + return this.detectSaveModelChanges(); + } + if (payload.changedObject instanceof LibraryVizPanel) { + if (Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'name')) { + return this.detectSaveModelChanges(); + } + } if (payload.changedObject instanceof SceneRefreshPicker) { if ( Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'intervals') || Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'refresh') ) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } } if (payload.changedObject instanceof behaviors.CursorSync) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } if (payload.changedObject instanceof SceneDataLayerSet) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } if (payload.changedObject instanceof DashboardGridItem) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } if (payload.changedObject instanceof SceneGridLayout) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } if (payload.changedObject instanceof DashboardScene) { if (Object.keys(payload.partialUpdate).some((key) => PERSISTED_PROPS.includes(key))) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } } if (payload.changedObject instanceof SceneTimeRange) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } if (payload.changedObject instanceof DashboardControls) { if (Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'hideTimeControls')) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } } if (payload.changedObject instanceof SceneVariableSet) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } if (payload.changedObject instanceof DashboardAnnotationsDataLayer) { if (!Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'data')) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } } if (payload.changedObject instanceof behaviors.LiveNowTimer) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } if (isSceneVariableInstance(payload.changedObject)) { - this.detectSaveModelChanges(); + return this.detectSaveModelChanges(); } } diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx index 7c01aca36c8..43e4cf7907d 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx @@ -1,4 +1,4 @@ -import { CoreApp } from '@grafana/data'; +import { CoreApp, LoadingState, getDefaultTimeRange } from '@grafana/data'; import { sceneGraph, SceneGridLayout, @@ -9,6 +9,7 @@ import { VizPanel, SceneGridRow, behaviors, + SceneDataTransformer, } from '@grafana/scenes'; import { Dashboard, DashboardCursorSync } from '@grafana/schema'; import appEvents from 'app/core/app_events'; @@ -30,6 +31,7 @@ import { DashboardControls } from './DashboardControls'; import { DashboardGridItem } from './DashboardGridItem'; import { DashboardScene, DashboardSceneState } from './DashboardScene'; import { LibraryVizPanel } from './LibraryVizPanel'; +import { PanelTimeRange } from './PanelTimeRange'; import { RowActions } from './row-actions/RowActions'; jest.mock('../settings/version-history/HistorySrv'); @@ -270,6 +272,125 @@ describe('DashboardScene', () => { expect(dashboardSceneGraph.getCursorSync(scene)!.state).toEqual(initialState); }); + it('A change to a any VizPanel state should set isDirty true', () => { + const panel = sceneGraph.findObject(scene, (p) => p instanceof VizPanel) as VizPanel; + const prevTitle = panel.state.title; + panel.setState({ title: 'new title' }); + + expect(scene.state.isDirty).toBe(true); + + scene.exitEditMode({ skipConfirm: true }); + const restoredPanel = sceneGraph.findObject(scene, (p) => p instanceof VizPanel) as VizPanel; + expect(restoredPanel.state.title).toBe(prevTitle); + }); + + it('A change to any DashboardGridItem state should set isDirty true', () => { + const dashboardGridItem = sceneGraph.findObject( + scene, + (p) => p instanceof DashboardGridItem + ) as DashboardGridItem; + const prevValue = dashboardGridItem.state.variableName; + + dashboardGridItem.setState({ variableName: 'var1', repeatDirection: 'h', maxPerRow: 2 }); + + expect(scene.state.isDirty).toBe(true); + + scene.exitEditMode({ skipConfirm: true }); + const restoredDashboardGridItem = sceneGraph.findObject( + scene, + (p) => p instanceof DashboardGridItem + ) as DashboardGridItem; + expect(restoredDashboardGridItem.state.variableName).toBe(prevValue); + }); + + it('A change to any LibraryVizPanel name should set isDirty true', () => { + const libraryVizPanel = sceneGraph.findObject(scene, (p) => p instanceof LibraryVizPanel) as LibraryVizPanel; + const prevValue = libraryVizPanel.state.name; + + libraryVizPanel.setState({ name: 'new name' }); + + expect(scene.state.isDirty).toBe(true); + + scene.exitEditMode({ skipConfirm: true }); + const restoredLibraryVizPanel = sceneGraph.findObject( + scene, + (p) => p instanceof LibraryVizPanel + ) as LibraryVizPanel; + expect(restoredLibraryVizPanel.state.name).toBe(prevValue); + }); + + it('A change to any PanelTimeRange state should set isDirty true', () => { + const panelTimeRange = sceneGraph.findObject(scene, (p) => p instanceof PanelTimeRange) as PanelTimeRange; + const prevValue = panelTimeRange.state.from; + + panelTimeRange.setState({ from: 'now-1h', to: 'now' }); + + expect(scene.state.isDirty).toBe(true); + + scene.exitEditMode({ skipConfirm: true }); + const restoredPanelTimeRange = sceneGraph.findObject( + scene, + (p) => p instanceof PanelTimeRange + ) as PanelTimeRange; + expect(restoredPanelTimeRange.state.from).toEqual(prevValue); + }); + + it('A change to any SceneQueryRunner state should set isDirty true', () => { + const queryRunner = sceneGraph.findObject(scene, (p) => p instanceof SceneQueryRunner) as SceneQueryRunner; + const prevValue = queryRunner.state.queries; + + queryRunner.setState({ queries: [{ refId: 'A', datasource: { uid: 'fake-uid', type: 'test' } }] }); + + expect(scene.state.isDirty).toBe(true); + + scene.exitEditMode({ skipConfirm: true }); + const restoredQueryRunner = sceneGraph.findObject( + scene, + (p) => p instanceof SceneQueryRunner + ) as SceneQueryRunner; + expect(restoredQueryRunner.state.queries).toEqual(prevValue); + }); + + it('A change to any SceneDataTransformer state should set isDirty true', () => { + const dataTransformer = sceneGraph.findObject( + scene, + (p) => p instanceof SceneDataTransformer + ) as SceneDataTransformer; + const prevValue = dataTransformer.state.transformations; + + dataTransformer.setState({ transformations: [{ id: 'fake-transformation', options: {} }] }); + + expect(scene.state.isDirty).toBe(true); + + scene.exitEditMode({ skipConfirm: true }); + const restoredDataTransformer = sceneGraph.findObject( + scene, + (p) => p instanceof SceneDataTransformer + ) as SceneDataTransformer; + expect(restoredDataTransformer.state.transformations).toEqual(prevValue); + }); + + it('A change to any SceneDataTransformer data should NOT set isDirty true', () => { + const dataTransformer = sceneGraph.findObject( + scene, + (p) => p instanceof SceneDataTransformer + ) as SceneDataTransformer; + const prevValue = dataTransformer.state.data; + const newData = { state: LoadingState.Done, timeRange: getDefaultTimeRange(), series: [] }; + + dataTransformer.setState({ data: newData }); + + expect(scene.state.isDirty).toBeFalsy(); + + scene.exitEditMode({ skipConfirm: true }); + + const restoredDataTransformer = sceneGraph.findObject( + scene, + (p) => p instanceof SceneDataTransformer + ) as SceneDataTransformer; + expect(restoredDataTransformer.state.data).toEqual(prevValue); + }); + it.each([ { hasChanges: true, hasTimeChanges: false, hasVariableValueChanges: false }, { hasChanges: true, hasTimeChanges: true, hasVariableValueChanges: false }, @@ -967,7 +1088,15 @@ function buildTestScene(overrides?: Partial) { title: 'Panel A', key: 'panel-1', pluginId: 'table', - $data: new SceneQueryRunner({ key: 'data-query-runner', queries: [{ refId: 'A' }] }), + $timeRange: new PanelTimeRange({ + from: 'now-12h', + to: 'now', + timeZone: 'browser', + }), + $data: new SceneDataTransformer({ + transformations: [], + $data: new SceneQueryRunner({ key: 'data-query-runner', queries: [{ refId: 'A' }] }), + }), }), }), new DashboardGridItem({