From 772432e025f9b4efea4f405b0999f041c2ddf8ad Mon Sep 17 00:00:00 2001 From: Ivan Ortega Alba Date: Thu, 13 Mar 2025 15:02:03 +0100 Subject: [PATCH] Dashboards: Changes are not detected from worker (#102105) --- .../saving/DashboardSceneChangeTracker.ts | 6 ++-- .../saving/DetectChangesWorker.ts | 28 +++++++++++++++++-- .../__mocks__/createDetectChangesWorker.ts | 2 -- .../scene/DashboardScene.test.tsx | 27 +++++------------- 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts index 2bfdb9b8646..5d73f119759 100644 --- a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts +++ b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts @@ -137,9 +137,7 @@ export class DashboardSceneChangeTracker { return this._dashboard.state.meta.folderUid !== this._dashboard.getInitialState()?.meta.folderUid; } - private updateIsDirty(result: DashboardChangeInfo) { - const { hasChanges } = result; - + private updateIsDirty(hasChanges: boolean) { if (hasChanges || this.hasMetadataChanges()) { if (!this._dashboard.state.isDirty) { this._dashboard.setState({ isDirty: true }); @@ -161,7 +159,7 @@ export class DashboardSceneChangeTracker { } this._changesWorker!.onmessage = (e: MessageEvent) => { - this.updateIsDirty(e.data); + this.updateIsDirty(!!e.data.hasChanges); }; const performSaveModelDiff = getChangeTrackerDebouncer(this.detectSaveModelChanges.bind(this)); diff --git a/public/app/features/dashboard-scene/saving/DetectChangesWorker.ts b/public/app/features/dashboard-scene/saving/DetectChangesWorker.ts index 3da81d5ad5b..cb856ad8758 100644 --- a/public/app/features/dashboard-scene/saving/DetectChangesWorker.ts +++ b/public/app/features/dashboard-scene/saving/DetectChangesWorker.ts @@ -1,6 +1,7 @@ import { Dashboard } from '@grafana/schema'; +import { DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha0'; -import { getRawDashboardChanges } from './getDashboardChanges'; +import { jsonDiff } from '../settings/version-history/utils'; function _debounce(f: (...args: T[]) => void, timeout: number) { let timeoutId: NodeJS.Timeout | undefined = undefined; @@ -13,6 +14,29 @@ function _debounce(f: (...args: T[]) => void, timeout: number) { } self.onmessage = _debounce((e: MessageEvent<{ initial: Dashboard; changed: Dashboard }>) => { - const result = getRawDashboardChanges(e.data.initial, e.data.changed, false, false, false); + const result = detectDashboardChanges(e.data.initial, e.data.changed); self.postMessage(result); }, 500); + +export function detectDashboardChanges( + changedSaveModel: DashboardV2Spec | Dashboard, + initialSaveModel: DashboardV2Spec | Dashboard +) { + // Calculate differences using the non-transformed to v2 spec values to be able to compare the initial and changed dashboard values + const diff = jsonDiff(initialSaveModel, changedSaveModel); + const diffCount = Object.values(diff).reduce((acc, cur) => acc + cur.length, 0); + const hasMigratedToV2 = isDashboardV2Spec(changedSaveModel) && !isDashboardV2Spec(initialSaveModel); + + return { + changedSaveModel, + initialSaveModel, + diffs: diff, + diffCount, + hasChanges: diffCount > 0, + hasMigratedToV2, + }; +} + +export function isDashboardV2Spec(obj: Dashboard | DashboardV2Spec): obj is DashboardV2Spec { + return 'elements' in obj; +} diff --git a/public/app/features/dashboard-scene/saving/__mocks__/createDetectChangesWorker.ts b/public/app/features/dashboard-scene/saving/__mocks__/createDetectChangesWorker.ts index 746a759c558..a9120947e5c 100644 --- a/public/app/features/dashboard-scene/saving/__mocks__/createDetectChangesWorker.ts +++ b/public/app/features/dashboard-scene/saving/__mocks__/createDetectChangesWorker.ts @@ -8,8 +8,6 @@ jest.mocked(worker.postMessage).mockImplementation(() => { worker.onmessage?.({ data: { hasChanges: true, - hasTimeChanges: true, - hasVariableValueChanges: true, }, } as unknown as MessageEvent); }); diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx index 3a9ecd5cee2..69e96644509 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx @@ -81,7 +81,7 @@ locationUtil.initialize({ }); const worker = createWorker(); -mockResultsOfDetectChangesWorker({ hasChanges: true, hasTimeChanges: false, hasVariableValueChanges: false }); +mockResultsOfDetectChangesWorker({ hasChanges: true }); describe('DashboardScene', () => { describe('DashboardSrv.getCurrent compatibility', () => { @@ -224,7 +224,7 @@ describe('DashboardScene', () => { const prevMeta = { ...scene.state.meta }; // The worker only detects changes in the model, so the folder change should be detected anyway - mockResultsOfDetectChangesWorker({ hasChanges: false, hasTimeChanges: false, hasVariableValueChanges: false }); + mockResultsOfDetectChangesWorker({ hasChanges: false }); scene.setState({ meta: { @@ -706,7 +706,7 @@ describe('DashboardScene', () => { }); it('A change to a variable state should set isDirty true', () => { - mockResultsOfDetectChangesWorker({ hasChanges: true, hasTimeChanges: false, hasVariableValueChanges: true }); + mockResultsOfDetectChangesWorker({ hasChanges: true }); const variable = new TestVariable({ name: 'A' }); const scene = buildTestScene({ $variables: new SceneVariableSet({ variables: [variable] }), @@ -730,13 +730,10 @@ describe('DashboardScene', () => { scene.activate(); scene.onEnterEditMode(); - mockResultsOfDetectChangesWorker({ hasChanges: true, hasTimeChanges: false, hasVariableValueChanges: false }); + mockResultsOfDetectChangesWorker({ hasChanges: true }); variable.setState({ name: 'B' }); expect(scene.state.isDirty).toBe(true); - mockResultsOfDetectChangesWorker( - // No changes, it is the same name than before comparing saving models - { hasChanges: false, hasTimeChanges: false, hasVariableValueChanges: false } - ); + mockResultsOfDetectChangesWorker({ hasChanges: false }); variable.setState({ name: 'A' }); expect(scene.state.isDirty).toBe(false); }); @@ -959,21 +956,11 @@ function buildTestScene(overrides?: Partial) { return scene; } -function mockResultsOfDetectChangesWorker({ - hasChanges, - hasTimeChanges, - hasVariableValueChanges, -}: { - hasChanges: boolean; - hasTimeChanges: boolean; - hasVariableValueChanges: boolean; -}) { +function mockResultsOfDetectChangesWorker({ hasChanges = true }) { jest.mocked(worker.postMessage).mockImplementationOnce(() => { worker.onmessage?.({ data: { - hasChanges: hasChanges ?? true, - hasTimeChanges: hasTimeChanges ?? true, - hasVariableValueChanges: hasVariableValueChanges ?? true, + hasChanges, }, } as unknown as MessageEvent); });