From d983629650885ea480c00e2e0f83e9281cc42d72 Mon Sep 17 00:00:00 2001
From: Ivan Ortega Alba <ivanortegaalba@gmail.com>
Date: Mon, 8 Apr 2024 12:06:50 +0200
Subject: [PATCH] Detect folder changes when saving a dashboard (#85378)

---
 .betterer.results                             |  4 -
 .../saving/DashboardSceneChangeTracker.ts     | 32 ++++---
 .../saving/SaveDashboardAsForm.tsx            |  4 +-
 .../saving/SaveDashboardDrawer.tsx            | 23 +++--
 .../saving/SaveDashboardForm.tsx              |  1 -
 .../getDashboardChangesFromScene.test.ts      | 11 +++
 .../saving/getDashboardChangesFromScene.ts    | 21 +++-
 .../saving/getSaveDashboardChange.ts          | 95 -------------------
 .../dashboard-scene/saving/shared.tsx         |  1 +
 .../scene/DashboardScene.test.tsx             | 21 +++-
 .../settings/GeneralSettingsEditView.tsx      |  1 -
 .../SaveDashboard/SaveDashboardDiff.tsx       | 46 ++++++---
 12 files changed, 117 insertions(+), 143 deletions(-)
 delete mode 100644 public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts

diff --git a/.betterer.results b/.betterer.results
index a316ba2da3a..a6e30ae9f67 100644
--- a/.betterer.results
+++ b/.betterer.results
@@ -2489,10 +2489,6 @@ exports[`better eslint`] = {
       [0, 0, 0, "Do not use any type assertions.", "0"],
       [0, 0, 0, "Do not use any type assertions.", "1"]
     ],
-    "public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts:5381": [
-      [0, 0, 0, "Do not use any type assertions.", "0"],
-      [0, 0, 0, "Do not use any type assertions.", "1"]
-    ],
     "public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],
diff --git a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts
index 26991da1f77..6fd1df0bc81 100644
--- a/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts
+++ b/public/app/features/dashboard-scene/saving/DashboardSceneChangeTracker.ts
@@ -35,61 +35,65 @@ export class DashboardSceneChangeTracker {
         Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'intervals') ||
         Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'refresh')
       ) {
-        this.detectChanges();
+        this.detectSaveModelChanges();
       }
     }
     if (payload.changedObject instanceof behaviors.CursorSync) {
-      this.detectChanges();
+      this.detectSaveModelChanges();
     }
     if (payload.changedObject instanceof SceneDataLayerSet) {
-      this.detectChanges();
+      this.detectSaveModelChanges();
     }
     if (payload.changedObject instanceof DashboardGridItem) {
-      this.detectChanges();
+      this.detectSaveModelChanges();
     }
     if (payload.changedObject instanceof SceneGridLayout) {
-      this.detectChanges();
+      this.detectSaveModelChanges();
     }
     if (payload.changedObject instanceof DashboardScene) {
       if (Object.keys(payload.partialUpdate).some((key) => PERSISTED_PROPS.includes(key))) {
-        this.detectChanges();
+        this.detectSaveModelChanges();
       }
     }
     if (payload.changedObject instanceof SceneTimeRange) {
-      this.detectChanges();
+      this.detectSaveModelChanges();
     }
     if (payload.changedObject instanceof DashboardControls) {
       if (Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'hideTimeControls')) {
-        this.detectChanges();
+        this.detectSaveModelChanges();
       }
     }
     if (payload.changedObject instanceof SceneVariableSet) {
-      this.detectChanges();
+      this.detectSaveModelChanges();
     }
     if (payload.changedObject instanceof DashboardAnnotationsDataLayer) {
       if (!Object.prototype.hasOwnProperty.call(payload.partialUpdate, 'data')) {
-        this.detectChanges();
+        this.detectSaveModelChanges();
       }
     }
     if (payload.changedObject instanceof behaviors.LiveNowTimer) {
-      this.detectChanges();
+      this.detectSaveModelChanges();
     }
     if (isSceneVariableInstance(payload.changedObject)) {
-      this.detectChanges();
+      this.detectSaveModelChanges();
     }
   }
 
-  private detectChanges() {
+  private detectSaveModelChanges() {
     this._changesWorker?.postMessage({
       changed: transformSceneToSaveModel(this._dashboard),
       initial: this._dashboard.getInitialSaveModel(),
     });
   }
 
+  private hasMetadataChanges() {
+    return this._dashboard.state.meta.folderUid !== this._dashboard.getInitialState()?.meta.folderUid;
+  }
+
   private updateIsDirty(result: DashboardChangeInfo) {
     const { hasChanges } = result;
 
-    if (hasChanges) {
+    if (hasChanges || this.hasMetadataChanges()) {
       if (!this._dashboard.state.isDirty) {
         this._dashboard.setState({ isDirty: true });
       }
diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardAsForm.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardAsForm.tsx
index 5f201c9f6a1..29263390f67 100644
--- a/public/app/features/dashboard-scene/saving/SaveDashboardAsForm.tsx
+++ b/public/app/features/dashboard-scene/saving/SaveDashboardAsForm.tsx
@@ -9,7 +9,6 @@ import { validationSrv } from 'app/features/manage-dashboards/services/Validatio
 
 import { DashboardScene } from '../scene/DashboardScene';
 
-import { SaveDashboardDrawer } from './SaveDashboardDrawer';
 import { DashboardChangeInfo, NameAlreadyExistsError, SaveButton, isNameExistsError } from './shared';
 import { useSaveDashboard } from './useSaveDashboard';
 
@@ -23,11 +22,10 @@ interface SaveDashboardAsFormDTO {
 
 export interface Props {
   dashboard: DashboardScene;
-  drawer: SaveDashboardDrawer;
   changeInfo: DashboardChangeInfo;
 }
 
-export function SaveDashboardAsForm({ dashboard, drawer, changeInfo }: Props) {
+export function SaveDashboardAsForm({ dashboard, changeInfo }: Props) {
   const { changedSaveModel } = changeInfo;
 
   const { register, handleSubmit, setValue, formState, getValues, watch, trigger } = useForm<SaveDashboardAsFormDTO>({
diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx
index 26c08217605..9e25e914f07 100644
--- a/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx
+++ b/public/app/features/dashboard-scene/saving/SaveDashboardDrawer.tsx
@@ -46,19 +46,21 @@ export class SaveDashboardDrawer extends SceneObjectBase<SaveDashboardDrawerStat
       saveVariables,
       saveRefresh
     );
-    const { changedSaveModel, initialSaveModel, diffs, diffCount } = changeInfo;
+    const { changedSaveModel, initialSaveModel, diffs, diffCount, hasFolderChanges } = changeInfo;
+    const changesCount = diffCount + (hasFolderChanges ? 1 : 0);
     const dashboard = model.state.dashboardRef.resolve();
-    const isProvisioned = dashboard.state.meta.provisioned;
+    const { meta } = dashboard.useState();
+    const { provisioned: isProvisioned, folderTitle } = meta;
 
     const tabs = (
       <TabsBar>
         <Tab label={'Details'} active={!showDiff} onChangeTab={() => model.setState({ showDiff: false })} />
-        {diffCount > 0 && (
+        {changesCount > 0 && (
           <Tab
             label={'Changes'}
             active={showDiff}
             onChangeTab={() => model.setState({ showDiff: true })}
-            counter={diffCount}
+            counter={changesCount}
           />
         )}
       </TabsBar>
@@ -73,11 +75,20 @@ export class SaveDashboardDrawer extends SceneObjectBase<SaveDashboardDrawerStat
 
     const renderBody = () => {
       if (showDiff) {
-        return <SaveDashboardDiff diff={diffs} oldValue={initialSaveModel} newValue={changedSaveModel} />;
+        return (
+          <SaveDashboardDiff
+            diff={diffs}
+            oldValue={initialSaveModel}
+            newValue={changedSaveModel}
+            hasFolderChanges={hasFolderChanges}
+            oldFolder={dashboard.getInitialState()?.meta.folderTitle}
+            newFolder={folderTitle}
+          />
+        );
       }
 
       if (saveAsCopy || changeInfo.isNew) {
-        return <SaveDashboardAsForm dashboard={dashboard} changeInfo={changeInfo} drawer={model} />;
+        return <SaveDashboardAsForm dashboard={dashboard} changeInfo={changeInfo} />;
       }
 
       if (isProvisioned) {
diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx
index 3de8731f875..2ebef87618d 100644
--- a/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx
+++ b/public/app/features/dashboard-scene/saving/SaveDashboardForm.tsx
@@ -104,7 +104,6 @@ export function SaveDashboardForm({ dashboard, drawer, changeInfo }: Props) {
     <Stack gap={0} direction="column">
       <SaveDashboardFormCommonOptions drawer={drawer} changeInfo={changeInfo} />
       <Field label="Message">
-        {/* config.featureToggles.dashgpt * TOOD GenAIDashboardChangesButton */}
         <TextArea
           aria-label="message"
           value={options.message ?? ''}
diff --git a/public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.test.ts b/public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.test.ts
index 7ddab4930a7..6eaef939d75 100644
--- a/public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.test.ts
+++ b/public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.test.ts
@@ -44,6 +44,17 @@ describe('getDashboardChangesFromScene', () => {
     expect(result.diffCount).toBe(1);
   });
 
+  it('Can detect folder change', () => {
+    const dashboard = setup();
+
+    dashboard.state.meta.folderUid = 'folder-2';
+
+    const result = getDashboardChangesFromScene(dashboard, false);
+    expect(result.hasChanges).toBe(true);
+    expect(result.diffCount).toBe(0); // Diff count is 0 because the diff contemplate only the model
+    expect(result.hasFolderChanges).toBe(true);
+  });
+
   it('Can detect refresh changed', () => {
     const dashboard = setup();
 
diff --git a/public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.ts b/public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.ts
index 4d01fdce1bb..5b8a4a444b0 100644
--- a/public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.ts
+++ b/public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.ts
@@ -1,20 +1,35 @@
 import { DashboardScene } from '../scene/DashboardScene';
 import { transformSceneToSaveModel } from '../serialization/transformSceneToSaveModel';
 
-import { getDashboardChanges } from './getDashboardChanges';
+import { getDashboardChanges as getDashboardSaveModelChanges } from './getDashboardChanges';
 
+/**
+ * Get changes between the initial save model and the current scene.
+ * It also checks if the folder has changed.
+ * @param scene DashboardScene object
+ * @param saveTimeRange if true, compare the time range
+ * @param saveVariables if true, compare the variables
+ * @param saveRefresh if true, compare the refresh interval
+ * @returns
+ */
 export function getDashboardChangesFromScene(
   scene: DashboardScene,
   saveTimeRange?: boolean,
   saveVariables?: boolean,
   saveRefresh?: boolean
 ) {
-  const changeInfo = getDashboardChanges(
+  const changeInfo = getDashboardSaveModelChanges(
     scene.getInitialSaveModel()!,
     transformSceneToSaveModel(scene),
     saveTimeRange,
     saveVariables,
     saveRefresh
   );
-  return changeInfo;
+  const hasFolderChanges = scene.getInitialState()?.meta.folderUid !== scene.state.meta.folderUid;
+
+  return {
+    ...changeInfo,
+    hasFolderChanges,
+    hasChanges: changeInfo.hasChanges || hasFolderChanges,
+  };
 }
diff --git a/public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts b/public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts
deleted file mode 100644
index f802919b6e0..00000000000
--- a/public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts
+++ /dev/null
@@ -1,95 +0,0 @@
-import { isEqual } from 'lodash';
-
-import { AdHocVariableModel, TypedVariableModel } from '@grafana/data';
-import { Dashboard } from '@grafana/schema';
-
-import { DashboardScene } from '../scene/DashboardScene';
-import { transformSceneToSaveModel } from '../serialization/transformSceneToSaveModel';
-import { jsonDiff } from '../settings/version-history/utils';
-
-import { DashboardChangeInfo } from './shared';
-
-export function getSaveDashboardChange(
-  dashboard: DashboardScene,
-  saveTimeRange?: boolean,
-  saveVariables?: boolean,
-  saveRefresh?: boolean
-): DashboardChangeInfo {
-  const initialSaveModel = dashboard.getInitialSaveModel()!;
-
-  if (dashboard.state.editPanel) {
-    dashboard.state.editPanel.commitChanges();
-  }
-
-  const changedSaveModel = transformSceneToSaveModel(dashboard);
-  const hasTimeChanged = getHasTimeChanged(changedSaveModel, initialSaveModel);
-
-  const hasVariableValueChanges = applyVariableChanges(changedSaveModel, initialSaveModel, saveVariables);
-  const hasRefreshChanged = changedSaveModel.refresh !== initialSaveModel.refresh;
-
-  if (!saveTimeRange) {
-    changedSaveModel.time = initialSaveModel.time;
-  }
-
-  if (!saveRefresh) {
-    changedSaveModel.refresh = initialSaveModel.refresh;
-  }
-
-  const diff = jsonDiff(initialSaveModel, changedSaveModel);
-
-  let diffCount = 0;
-  for (const d of Object.values(diff)) {
-    diffCount += d.length;
-  }
-
-  return {
-    changedSaveModel,
-    initialSaveModel,
-    diffs: diff,
-    diffCount,
-    hasChanges: diffCount > 0,
-    hasTimeChanges: hasTimeChanged,
-    isNew: changedSaveModel.version === 0,
-    hasVariableValueChanges,
-    hasRefreshChange: hasRefreshChanged,
-  };
-}
-
-export function getHasTimeChanged(saveModel: Dashboard, originalSaveModel: Dashboard) {
-  return saveModel.time?.from !== originalSaveModel.time?.from || saveModel.time?.to !== originalSaveModel.time?.to;
-}
-
-export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Dashboard, saveVariables?: boolean) {
-  const originalVariables = originalSaveModel.templating?.list ?? [];
-  const variablesToSave = saveModel.templating?.list ?? [];
-  let hasVariableValueChanges = false;
-
-  for (const variable of variablesToSave) {
-    const original = originalVariables.find(({ name, type }) => name === variable.name && type === variable.type);
-
-    if (!original) {
-      continue;
-    }
-
-    // Old schema property that never should be in persisted model
-    if (original.current && Object.hasOwn(original.current, 'selected')) {
-      delete original.current.selected;
-    }
-
-    if (!isEqual(variable.current, original.current)) {
-      hasVariableValueChanges = true;
-    }
-
-    if (!saveVariables) {
-      const typed = variable as TypedVariableModel;
-      if (typed.type === 'adhoc') {
-        typed.filters = (original as AdHocVariableModel).filters;
-      } else {
-        variable.current = original.current;
-        variable.options = original.options;
-      }
-    }
-  }
-
-  return hasVariableValueChanges;
-}
diff --git a/public/app/features/dashboard-scene/saving/shared.tsx b/public/app/features/dashboard-scene/saving/shared.tsx
index 39010d3f86b..2681b9c1f0f 100644
--- a/public/app/features/dashboard-scene/saving/shared.tsx
+++ b/public/app/features/dashboard-scene/saving/shared.tsx
@@ -17,6 +17,7 @@ export interface DashboardChangeInfo {
   hasVariableValueChanges: boolean;
   hasRefreshChange: boolean;
   isNew?: boolean;
+  hasFolderChanges?: boolean;
 }
 
 export function isVersionMismatchError(error?: Error) {
diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx
index 42a916b3f6a..7c01aca36c8 100644
--- a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx
+++ b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx
@@ -175,7 +175,6 @@ describe('DashboardScene', () => {
         ${'tags'}        | ${['tag3', 'tag4']}
         ${'editable'}    | ${false}
         ${'links'}       | ${[]}
-        ${'meta'}        | ${{ folderUid: 'new-folder-uid', folderTitle: 'new-folder-title', hasUnsavedFolderChange: true }}
       `(
         'A change to $prop should set isDirty true',
         ({ prop, value }: { prop: keyof DashboardSceneState; value: unknown }) => {
@@ -189,6 +188,26 @@ describe('DashboardScene', () => {
         }
       );
 
+      it('A change to folderUid should set isDirty true', () => {
+        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 });
+
+        scene.setState({
+          meta: {
+            ...prevMeta,
+            folderUid: 'new-folder-uid',
+            folderTitle: 'new-folder-title',
+          },
+        });
+
+        expect(scene.state.isDirty).toBe(true);
+
+        scene.exitEditMode({ skipConfirm: true });
+        expect(scene.state.meta).toEqual(prevMeta);
+      });
+
       it('A change to refresh picker interval settings should set isDirty true', () => {
         const refreshPicker = dashboardSceneGraph.getRefreshPicker(scene)!;
         const prevState = [...refreshPicker.state.intervals!];
diff --git a/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx b/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx
index 2af52da8244..8c787cda9cb 100644
--- a/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx
+++ b/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx
@@ -101,7 +101,6 @@ export class GeneralSettingsEditView
       ...this._dashboard.state.meta,
       folderUid: newUID || this._dashboard.state.meta.folderUid,
       folderTitle: newTitle || this._dashboard.state.meta.folderTitle,
-      hasUnsavedFolderChange: true,
     };
 
     this._dashboard.setState({ meta: newMeta });
diff --git a/public/app/features/dashboard/components/SaveDashboard/SaveDashboardDiff.tsx b/public/app/features/dashboard/components/SaveDashboard/SaveDashboardDiff.tsx
index 97c0462df76..0cb291d4e50 100644
--- a/public/app/features/dashboard/components/SaveDashboard/SaveDashboardDiff.tsx
+++ b/public/app/features/dashboard/components/SaveDashboard/SaveDashboardDiff.tsx
@@ -13,9 +13,19 @@ interface SaveDashboardDiffProps {
 
   // calculated by parent so we can see summary in tabs
   diff?: Diffs;
+  hasFolderChanges?: boolean;
+  oldFolder?: string;
+  newFolder?: string;
 }
 
-export const SaveDashboardDiff = ({ diff, oldValue, newValue }: SaveDashboardDiffProps) => {
+export const SaveDashboardDiff = ({
+  diff,
+  oldValue,
+  newValue,
+  hasFolderChanges,
+  oldFolder,
+  newFolder,
+}: SaveDashboardDiffProps) => {
   const loader = useAsync(async () => {
     const oldJSON = JSON.stringify(oldValue ?? {}, null, 2);
     const newJSON = JSON.stringify(newValue ?? {}, null, 2);
@@ -48,23 +58,29 @@ export const SaveDashboardDiff = ({ diff, oldValue, newValue }: SaveDashboardDif
   }, [diff, oldValue, newValue]);
 
   const { value } = loader;
-  if (!value || !oldValue) {
-    return <Spinner />;
-  }
-
-  if (value.count < 1) {
-    return <div>No changes in this dashboard</div>;
-  }
 
   return (
     <Stack direction="column" gap={1}>
-      {value.schemaChange && value.schemaChange}
-      {value.showDiffs && value.diffs}
-
-      <Box paddingTop={2}>
-        <h4>Full JSON diff</h4>
-        {value.jsonView}
-      </Box>
+      {hasFolderChanges && (
+        <DiffGroup
+          diffs={[{ op: 'replace', value: newFolder, originalValue: oldFolder, path: [], startLineNumber: 0 }]}
+          key={'folder'}
+          title={'folder'}
+        />
+      )}
+      {(!value || !oldValue) && <Spinner />}
+      {value && value.count >= 1 ? (
+        <>
+          {value && value.schemaChange && value.schemaChange}
+          {value && value.showDiffs && value.diffs}
+          <Box paddingTop={1}>
+            <h4>Full JSON diff</h4>
+            {value.jsonView}
+          </Box>
+        </>
+      ) : (
+        <Box paddingTop={1}>No changes in the dashboard JSON</Box>
+      )}
     </Stack>
   );
 };