Dashboard Scene: Sync variable state with TemplateSrv result (#93327)

* Generate options for variables through TemplateSrv

* Add refresh when object changes

* Remove unnecesary static function

* Extract logic

* Add extra test case when variable changes and refresh event is triggered

* bring back old logic, query options should not live in the dashboard json

* add missing change

* Add support to keep variable options in query

* tests

* move refreshEvent to DashboardVariableDependency

---------

Co-authored-by: alexandra vargas <alexa1866@gmail.com>
Co-authored-by: Victor Marin <victor.marin@grafana.com>

public/app/features/dashboard-scene/scene/DashboardScene.test.tsx

@@ -1,5 +1,5 @@
 import { CoreApp, GrafanaConfig, LoadingState, getDefaultTimeRange, locationUtil, store } from '@grafana/data';
-import { locationService } from '@grafana/runtime';
+import { locationService, RefreshEvent } from '@grafana/runtime';
 import {
   sceneGraph,
   SceneGridLayout,
@@ -750,6 +750,21 @@ describe('DashboardScene', () => {
       variable.setState({ name: 'A' });
       expect(scene.state.isDirty).toBe(false);
     });
+
+    it('should trigger scene RefreshEvent when a scene variable changes', () => {
+      const varA = new TestVariable({ name: 'A', query: 'A.*', value: 'A.AA', text: '', options: [], delayMs: 0 });
+      const scene = buildTestScene({
+        $variables: new SceneVariableSet({ variables: [varA] }),
+      });
+
+      scene.activate();
+
+      const eventHandler = jest.fn();
+      // this RefreshEvent is from the scenes library
+      scene.subscribeToEvent(RefreshEvent, eventHandler);
+      varA.changeValueTo('A.AB');
+      expect(eventHandler).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('When a dashboard is restored', () => {

public/app/features/dashboard-scene/scene/DashboardScene.tsx

@@ -10,7 +10,7 @@ import {
   DataSourceGetTagKeysOptions,
   DataSourceGetTagValuesOptions,
 } from '@grafana/data';
-import { config, locationService } from '@grafana/runtime';
+import { config, locationService, RefreshEvent } from '@grafana/runtime';
 import {
   sceneGraph,
   SceneGridRow,
@@ -716,6 +716,9 @@ export class DashboardVariableDependency implements SceneVariableDependencyConfi
     if (hasChanged) {
       // Temp solution for some core panels (like dashlist) to know that variables have changed
       appEvents.publish(new VariablesChanged({ refreshAll: true, panelIds: [] }));
+      // Backwards compat with plugins that rely on the RefreshEvent when a
+      // variable changes. TODO: We should redirect plugin devs to use VariablesChanged event
+      this._dashboard.publishEvent(new RefreshEvent());
     }
 
     if (variable.state.name === PANEL_SEARCH_VAR) {

public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.test.ts

@@ -190,6 +190,62 @@ describe('sceneVariablesSetToVariables', () => {
     `);
   });
 
+  it('should handle Query variable when sceneVariablesSetToVariables should discard options', () => {
+    const variable = new QueryVariable({
+      name: 'test',
+      label: 'test-label',
+      description: 'test-desc',
+      value: ['selected-value'],
+      text: ['selected-value-text'],
+      datasource: { uid: 'fake-std', type: 'fake-std' },
+      query: 'query',
+      options: [
+        { label: 'test', value: 'test' },
+        { label: 'test1', value: 'test1' },
+        { label: 'test2', value: 'test2' },
+      ],
+      includeAll: true,
+      allValue: 'test-all',
+      isMulti: true,
+    });
+
+    const set = new SceneVariableSet({
+      variables: [variable],
+    });
+    const result = sceneVariablesSetToVariables(set);
+    expect(result).toHaveLength(1);
+    expect(result[0].options).toEqual([]);
+  });
+
+  it('should handle Query variable when sceneVariablesSetToVariables shoudl keep options', () => {
+    const variable = new QueryVariable({
+      name: 'test',
+      label: 'test-label',
+      description: 'test-desc',
+      value: ['test'],
+      text: ['test'],
+      datasource: { uid: 'fake-std', type: 'fake-std' },
+      query: 'query',
+      options: [
+        { label: 'test', value: 'test' },
+        { label: 'test1', value: 'test1' },
+        { label: 'test2', value: 'test2' },
+      ],
+      includeAll: true,
+      allValue: 'test-all',
+      isMulti: true,
+    });
+
+    const set = new SceneVariableSet({
+      variables: [variable],
+    });
+    const keepQueryOptions = true;
+    const result = sceneVariablesSetToVariables(set, keepQueryOptions);
+    expect(result).toHaveLength(1);
+    expect(result[0].options).not.toEqual([]);
+    expect(result[0].options?.length).toEqual(3);
+  });
+
   it('should handle DatasourceVariable', () => {
     const variable = new DataSourceVariable({
       name: 'test',

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

@@ -4,7 +4,16 @@ import { VariableHide, VariableModel, VariableOption, VariableRefresh, VariableS
 
 import { getIntervalsQueryFromNewIntervalModel } from '../utils/utils';
 
-export function sceneVariablesSetToVariables(set: SceneVariables) {
+/**
+ * Converts a SceneVariables object into an array of VariableModel objects.
+ * @param set - The SceneVariables object containing the variables to convert.
+ * @param keepQueryOptions - (Optional) A boolean flag indicating whether to keep the options for query variables.
+ *                           This should be set to `false` when variables are saved in the dashboard model,
+ *                           but should be set to `true` when variables are used in the templateSrv to keep them in sync.
+ *                           If `true`, the options for query variables are kept.
+ *  */
+
+export function sceneVariablesSetToVariables(set: SceneVariables, keepQueryOptions?: boolean) {
   const variables: VariableModel[] = [];
   for (const variable of set.state.variables) {
     const commonProperties = {
@@ -19,7 +28,7 @@ export function sceneVariablesSetToVariables(set: SceneVariables) {
       let options: VariableOption[] = [];
       // Not sure if we actually have to still support this option given
       // that it's not exposed in the UI
-      if (variable.state.refresh === VariableRefresh.never) {
+      if (variable.state.refresh === VariableRefresh.never || keepQueryOptions) {
         options = variableValueOptionsToVariableOptions(variable.state);
       }
       variables.push({

public/app/features/dashboard-scene/utils/getVariablesCompatibility.ts

@@ -5,7 +5,13 @@ import { sceneVariablesSetToVariables } from '../serialization/sceneVariablesSet
 
 export function getVariablesCompatibility(sceneObject: SceneObject): TypedVariableModel[] {
   const set = sceneGraph.getVariables(sceneObject);
-  const legacyModels = sceneVariablesSetToVariables(set);
+  const keepQueryOptions = true;
+
+  // `sceneVariablesSetToVariables` is also used when transforming the scene to a save model.
+  // In those cases, query options will be stripped out.
+  // However, when `getVariablesCompatibility` is called from `templateSrv`, it is used to get all variables in the scene.
+  // Therefore, options should be kept.
+  const legacyModels = sceneVariablesSetToVariables(set, keepQueryOptions);
 
   // Sadly templateSrv.getVariables returns TypedVariableModel but sceneVariablesSetToVariables return persisted schema model
   // They look close to identical (differ in what is optional in some places).