From ae0e970beffa4ce4213f2d2b9f4e7cf01b008d4b Mon Sep 17 00:00:00 2001 From: Ivan Ortega Alba Date: Thu, 5 Sep 2024 20:52:05 +0200 Subject: [PATCH] DetectChanges: Serialize message payload to avoid non-clonable props (#92831) Detect changes: Serialize message payload to avoid non-clonable properties --- .../DashboardSceneChangeTracker.test.ts | 44 +++++++++++++++++++ .../saving/DashboardSceneChangeTracker.ts | 14 ++++-- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.test.ts b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.test.ts index 413035cf22b..474907a347f 100644 --- a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.test.ts +++ b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.test.ts @@ -1,7 +1,23 @@ +import { SceneObjectStateChangedEvent } from '@grafana/scenes'; +import { Dashboard } from '@grafana/schema'; +import { CorsWorker } from 'app/core/utils/CorsWorker'; import * as createDetectChangesWorker from 'app/features/dashboard-scene/saving/createDetectChangesWorker'; +import { DashboardScene } from '../scene/DashboardScene'; + import { DashboardSceneChangeTracker } from './DashboardSceneChangeTracker'; +jest.mock('../serialization/transformSceneToSaveModel', () => { + return { + transformSceneToSaveModel: () => { + return { + title: 'updated dashboard', + invalidProp: () => 'function', + }; + }, + }; +}); + describe('DashboardSceneChangeTracker', () => { it('should set _changesWorker to undefined when terminate is called', () => { const terminate = jest.fn(); @@ -20,4 +36,32 @@ describe('DashboardSceneChangeTracker', () => { changeTracker.terminate(); expect(changeTracker['_changesWorker']).toBeUndefined(); }); + + it('should remove non clonable properties before sending to worker', () => { + const scene = new DashboardScene({}); + const postMessage = jest.fn(); + + jest.spyOn(createDetectChangesWorker, 'createWorker').mockImplementation(() => { + return { + postMessage, + } as unknown as CorsWorker; + }); + jest.spyOn(DashboardSceneChangeTracker, 'isUpdatingPersistedState').mockImplementation(() => { + return true; + }); + jest.spyOn(scene, 'getInitialSaveModel').mockReturnValue({ + title: 'initial dashboard', + invalidProp: () => 'function', + } as unknown as Dashboard); + + const changeTracker = new DashboardSceneChangeTracker(scene); + changeTracker.startTrackingChanges(); + + scene.publishEvent({ type: SceneObjectStateChangedEvent.type, payload: { a: 1 } }); + + expect(postMessage).toHaveBeenCalledWith({ + initial: { title: 'initial dashboard' }, + changed: { title: 'updated dashboard' }, + }); + }); }); diff --git a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts index dc31efab048..d99c4132687 100644 --- a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts +++ b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts @@ -139,10 +139,16 @@ export class DashboardSceneChangeTracker { } private detectSaveModelChanges() { - this._changesWorker?.postMessage({ - changed: transformSceneToSaveModel(this._dashboard), - initial: this._dashboard.getInitialSaveModel(), - }); + const changedDashboard = transformSceneToSaveModel(this._dashboard); + const initialDashboard = this._dashboard.getInitialSaveModel(); + + // Objects must be stringify to ensure they are clonable, so they don't contain functions + const changed = + typeof changedDashboard === 'object' ? JSON.parse(JSON.stringify(changedDashboard)) : changedDashboard; + const initial = + typeof initialDashboard === 'object' ? JSON.parse(JSON.stringify(initialDashboard)) : initialDashboard; + + this._changesWorker?.postMessage({ initial, changed }); } private hasMetadataChanges() {