diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx index f7f49b5b5f8..8c35c9d8618 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx +++ b/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', () => { diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index 0d037f71285..aa3fee00f82 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/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) { diff --git a/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.test.ts b/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.test.ts index 454b7721a06..97750a6d500 100644 --- a/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.test.ts +++ b/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', diff --git a/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.ts b/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.ts index 1ad5608a47a..2eacfecf359 100644 --- a/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.ts +++ b/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({ diff --git a/public/app/features/dashboard-scene/utils/getVariablesCompatibility.ts b/public/app/features/dashboard-scene/utils/getVariablesCompatibility.ts index 54c47284639..6a3d1efa16a 100644 --- a/public/app/features/dashboard-scene/utils/getVariablesCompatibility.ts +++ b/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).