Dashboard scenes: Fix variable saving inconsistencies (#88163)

* Fix variable saving inconsistencies

* add tests

* for variable changes to be detected, save variables need to be true

* Fix for issue with buildNewDashboardSaveModel creating filter variable withouth filter property

* remove unused import

.betterer.results

@@ -1487,7 +1487,9 @@ exports[`better eslint`] = {
     "public/app/features/dashboard-scene/saving/getDashboardChanges.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
       [0, 0, 0, "Do not use any type assertions.", "1"],
-      [0, 0, 0, "Do not use any type assertions.", "2"]
+      [0, 0, 0, "Do not use any type assertions.", "2"],
+      [0, 0, 0, "Do not use any type assertions.", "3"],
+      [0, 0, 0, "Do not use any type assertions.", "4"]
     ],
     "public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"]
@@ -1495,6 +1497,9 @@ exports[`better eslint`] = {
     "public/app/features/dashboard-scene/serialization/angularMigration.test.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"]
     ],
+    "public/app/features/dashboard-scene/serialization/buildNewDashboardSaveModel.ts:5381": [
+      [0, 0, 0, "Do not use any type assertions.", "0"]
+    ],
     "public/app/features/dashboard-scene/serialization/transformSaveModelToScene.test.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],

public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts

@@ -1,6 +1,63 @@
+import { AdHocVariableModel } from '@grafana/data';
 import { Dashboard, Panel } from '@grafana/schema';
 
-import { getDashboardChanges, getPanelChanges } from './getDashboardChanges';
+import { adHocVariableFiltersEqual, getDashboardChanges, getPanelChanges } from './getDashboardChanges';
+
+describe('adHocVariableFiltersEqual', () => {
+  it('should compare empty filters', () => {
+    expect(
+      adHocVariableFiltersEqual(
+        { filters: [] } as unknown as AdHocVariableModel,
+        { filters: [] } as unknown as AdHocVariableModel
+      )
+    ).toBeTruthy();
+  });
+
+  it('should compare different length filter arrays', () => {
+    expect(
+      adHocVariableFiltersEqual(
+        { filters: [] } as unknown as AdHocVariableModel,
+        { filters: [{ value: '', key: '', operator: '' }] } as unknown as AdHocVariableModel
+      )
+    ).toBeFalsy();
+  });
+
+  it('should compare equal filter arrays', () => {
+    expect(
+      adHocVariableFiltersEqual(
+        { filters: [{ value: 'asd', key: 'qwe', operator: 'wer' }] } as unknown as AdHocVariableModel,
+        { filters: [{ value: 'asd', key: 'qwe', operator: 'wer' }] } as unknown as AdHocVariableModel
+      )
+    ).toBeTruthy();
+  });
+
+  it('should compare different filter arrays where operator differs', () => {
+    expect(
+      adHocVariableFiltersEqual(
+        { filters: [{ value: 'asd', key: 'qwe', operator: 'wer' }] } as unknown as AdHocVariableModel,
+        { filters: [{ value: 'asd', key: 'qwe', operator: 'weee' }] } as unknown as AdHocVariableModel
+      )
+    ).toBeFalsy();
+  });
+
+  it('should compare different filter arrays where key differs', () => {
+    expect(
+      adHocVariableFiltersEqual(
+        { filters: [{ value: 'asd', key: 'qwe', operator: 'wer' }] } as unknown as AdHocVariableModel,
+        { filters: [{ value: 'asd', key: 'qwer', operator: 'wer' }] } as unknown as AdHocVariableModel
+      )
+    ).toBeFalsy();
+  });
+
+  it('should compare different filter arrays where value differs', () => {
+    expect(
+      adHocVariableFiltersEqual(
+        { filters: [{ value: 'asd', key: 'qwe', operator: 'wer' }] } as unknown as AdHocVariableModel,
+        { filters: [{ value: 'asdio', key: 'qwe', operator: 'wer' }] } as unknown as AdHocVariableModel
+      )
+    ).toBeFalsy();
+  });
+});
 
 describe('getDashboardChanges', () => {
   const initial: Dashboard = {

public/app/features/dashboard-scene/saving/getDashboardChanges.ts

@@ -71,6 +71,25 @@ export function getHasTimeChanged(saveModel: Dashboard, originalSaveModel: Dashb
   return saveModel.time?.from !== originalSaveModel.time?.from || saveModel.time?.to !== originalSaveModel.time?.to;
 }
 
+export function adHocVariableFiltersEqual(a: AdHocVariableModel, b: AdHocVariableModel) {
+  if (a.filters === undefined || b.filters === undefined) {
+    throw new Error('AdHoc variable missing filter property');
+  }
+
+  if (a.filters.length !== b.filters.length) {
+    return false;
+  }
+
+  for (let i = 0; i < a.filters.length; i++) {
+    const aFilter = a.filters[i];
+    const bFilter = b.filters[i];
+    if (aFilter.key !== bFilter.key || aFilter.operator !== bFilter.operator || aFilter.value !== bFilter.value) {
+      return false;
+    }
+  }
+  return true;
+}
+
 export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Dashboard, saveVariables?: boolean) {
   const originalVariables = originalSaveModel.templating?.list ?? [];
   const variablesToSave = saveModel.templating?.list ?? [];
@@ -90,13 +109,18 @@ export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Da
 
     if (!isEqual(variable.current, original.current)) {
       hasVariableValueChanges = true;
+    } else if (
+      variable.type === 'adhoc' &&
+      !adHocVariableFiltersEqual(variable as AdHocVariableModel, original as AdHocVariableModel)
+    ) {
+      hasVariableValueChanges = true;
     }
 
     if (!saveVariables) {
       const typed = variable as TypedVariableModel;
       if (typed.type === 'adhoc') {
         typed.filters = (original as AdHocVariableModel).filters;
-      } else if (typed.type !== 'groupby') {
+      } else {
         variable.current = original.current;
         variable.options = original.options;
       }

public/app/features/dashboard-scene/saving/getDashboardChangesFromScene.test.ts

@@ -162,7 +162,7 @@ describe('getDashboardChangesFromScene', () => {
 
         const variable = sceneGraph.lookupVariable('GroupBy', dashboard) as GroupByVariable;
         variable.setState({ defaultOptions: [{ text: 'Host', value: 'host' }] });
-        const result = getDashboardChangesFromScene(dashboard, false, false);
+        const result = getDashboardChangesFromScene(dashboard, false, true);
 
         expect(result.hasVariableValueChanges).toBe(false);
         expect(result.hasChanges).toBe(true);

public/app/features/dashboard-scene/serialization/buildNewDashboardSaveModel.ts

@@ -16,8 +16,9 @@ export async function buildNewDashboardSaveModel(urlFolderUid?: string): Promise
         uid: defaultDs.uid,
       };
 
-      const fitlerVariable: VariableModel = {
+      const filterVariable = {
         datasource: datasourceRef,
+        filters: [],
         name: 'Filter',
         type: 'adhoc',
       };
@@ -28,7 +29,7 @@ export async function buildNewDashboardSaveModel(urlFolderUid?: string): Promise
         type: 'groupby',
       };
 
-      variablesList = (variablesList || []).concat([fitlerVariable, groupByVariable]);
+      variablesList = (variablesList || []).concat([filterVariable as VariableModel, groupByVariable]);
     }
   }