From dece0288208a5d8b7cdb31bfb2483661f0ed88f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Fri, 27 Nov 2020 10:20:57 +0100 Subject: [PATCH] Variables: Fixes issue with upgrading legacy queries (#29375) * Variables: Fixes issue with upgrading legacy queries * Tests: adds tests for upgradeLegacyQueries --- .../app/features/variables/query/actions.ts | 36 +--- .../shared/testing/optionsVariableBuilder.ts | 2 +- .../shared/testing/queryVariableBuilder.ts | 5 + .../app/features/variables/state/actions.ts | 54 +++++- .../state/upgradeLegacyQueries.test.ts | 155 ++++++++++++++++++ 5 files changed, 212 insertions(+), 40 deletions(-) create mode 100644 public/app/features/variables/state/upgradeLegacyQueries.test.ts diff --git a/public/app/features/variables/query/actions.ts b/public/app/features/variables/query/actions.ts index b07d54b345d..38bfa614185 100644 --- a/public/app/features/variables/query/actions.ts +++ b/public/app/features/variables/query/actions.ts @@ -1,4 +1,4 @@ -import { DataQuery, DataSourceApi, DataSourcePluginMeta, DataSourceSelectItem } from '@grafana/data'; +import { DataSourcePluginMeta, DataSourceSelectItem } from '@grafana/data'; import { toDataQueryError } from '@grafana/runtime'; import { updateOptions } from '../state/actions'; @@ -9,7 +9,6 @@ import { getVariable } from '../state/selectors'; import { addVariableEditorError, changeVariableEditorExtended, removeVariableEditorError } from '../editor/reducer'; import { changeVariableProp } from '../state/sharedReducer'; import { toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../state/types'; -import { hasLegacyVariableSupport, hasStandardVariableSupport } from '../guard'; import { getVariableQueryEditor } from '../editor/getVariableQueryEditor'; import { Subscription } from 'rxjs'; import { getVariableQueryRunner } from './VariableQueryRunner'; @@ -26,7 +25,6 @@ export const updateQueryVariableOptions = ( dispatch(removeVariableEditorError({ errorProp: 'update' })); } const datasource = await getDatasourceSrv().get(variableInState.datasource ?? ''); - dispatch(upgradeLegacyQueries(identifier, datasource)); // we need to await the result from variableQueryRunner before moving on otherwise variables dependent on this // variable will have the wrong current value as input @@ -158,35 +156,3 @@ export function flattenQuery(query: any): any { return flattened; } - -export function upgradeLegacyQueries(identifier: VariableIdentifier, datasource: DataSourceApi): ThunkResult { - return function(dispatch, getState) { - if (hasLegacyVariableSupport(datasource)) { - return; - } - - if (!hasStandardVariableSupport(datasource)) { - return; - } - - const variable = getVariable(identifier.id, getState()); - if (isDataQuery(variable.query)) { - return; - } - - const query = { - refId: `${datasource.name}-${identifier.id}-Variable-Query`, - query: variable.query, - }; - - dispatch(changeVariableProp(toVariablePayload(identifier, { propName: 'query', propValue: query }))); - }; -} - -function isDataQuery(query: any): query is DataQuery { - if (!query) { - return false; - } - - return query.hasOwnProperty('refId') && typeof query.refId === 'string'; -} diff --git a/public/app/features/variables/shared/testing/optionsVariableBuilder.ts b/public/app/features/variables/shared/testing/optionsVariableBuilder.ts index b3ef1e14300..4d6e658c2de 100644 --- a/public/app/features/variables/shared/testing/optionsVariableBuilder.ts +++ b/public/app/features/variables/shared/testing/optionsVariableBuilder.ts @@ -28,7 +28,7 @@ export class OptionsVariableBuilder extends Varia return this; } - withQuery(query: string) { + withQuery(query: any) { this.variable.query = query; return this; } diff --git a/public/app/features/variables/shared/testing/queryVariableBuilder.ts b/public/app/features/variables/shared/testing/queryVariableBuilder.ts index 728e0a427c6..a12fb583f37 100644 --- a/public/app/features/variables/shared/testing/queryVariableBuilder.ts +++ b/public/app/features/variables/shared/testing/queryVariableBuilder.ts @@ -11,4 +11,9 @@ export class QueryVariableBuilder extends Datasour this.variable.tagsQuery = tagsQuery; return this; } + + withDatasource(datasource: string) { + this.variable.datasource = datasource; + return this; + } } diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts index 89fcdbdf025..238beb2006f 100644 --- a/public/app/features/variables/state/actions.ts +++ b/public/app/features/variables/state/actions.ts @@ -1,6 +1,6 @@ import angular from 'angular'; import castArray from 'lodash/castArray'; -import { LoadingState, TimeRange, UrlQueryMap, UrlQueryValue } from '@grafana/data'; +import { DataQuery, LoadingState, TimeRange, UrlQueryMap, UrlQueryValue } from '@grafana/data'; import { DashboardVariableModel, @@ -39,7 +39,7 @@ import { import { contextSrv } from 'app/core/services/context_srv'; import { getTemplateSrv, TemplateSrv } from '../../templating/template_srv'; import { alignCurrentWithMulti } from '../shared/multiOptions'; -import { isMulti } from '../guard'; +import { hasLegacyVariableSupport, hasStandardVariableSupport, isMulti, isQuery } from '../guard'; import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { DashboardModel } from 'app/features/dashboard/state'; import { createErrorNotification } from '../../../core/copy/appNotification'; @@ -54,6 +54,7 @@ import { cleanVariables } from './variablesReducer'; import isEqual from 'lodash/isEqual'; import { getCurrentText, getVariableRefresh } from '../utils'; import { store } from 'app/store/store'; +import { getDatasourceSrv } from '../../plugins/datasource_srv'; // process flow queryVariable // thunk => processVariables @@ -99,8 +100,11 @@ export const initDashboardTemplating = (list: VariableModel[]): ThunkResult { + return async function(dispatch, getState) { + const variable = getVariable(identifier.id, getState()); + + if (!isQuery(variable)) { + return; + } + + const datasource = await getDatasourceSrvFunc().get(variable.datasource ?? ''); + + if (hasLegacyVariableSupport(datasource)) { + return; + } + + if (!hasStandardVariableSupport(datasource)) { + return; + } + + if (isDataQueryType(variable.query)) { + return; + } + + const query = { + refId: `${datasource.name}-${identifier.id}-Variable-Query`, + query: variable.query, + }; + + dispatch(changeVariableProp(toVariablePayload(identifier, { propName: 'query', propValue: query }))); + }; +} + +function isDataQueryType(query: any): query is DataQuery { + if (!query) { + return false; + } + + return query.hasOwnProperty('refId') && typeof query.refId === 'string'; +} diff --git a/public/app/features/variables/state/upgradeLegacyQueries.test.ts b/public/app/features/variables/state/upgradeLegacyQueries.test.ts new file mode 100644 index 00000000000..cb29d3e76ab --- /dev/null +++ b/public/app/features/variables/state/upgradeLegacyQueries.test.ts @@ -0,0 +1,155 @@ +import { customBuilder, queryBuilder } from '../shared/testing/builders'; +import { VariableSupportType } from '@grafana/data'; +import { toVariableIdentifier } from './types'; +import { upgradeLegacyQueries } from './actions'; +import { changeVariableProp } from './sharedReducer'; +import { thunkTester } from '../../../../test/core/thunk/thunkTester'; +import { VariableModel } from '../types'; + +interface Args { + query?: any; + variable?: VariableModel; + datasource?: any; +} +function getTestContext({ query = '', variable, datasource }: Args = {}) { + variable = + variable ?? + queryBuilder() + .withId('query') + .withName('query') + .withQuery(query) + .withDatasource('test-data') + .build(); + const state = { + templating: { + variables: { + [variable.id]: variable, + }, + }, + }; + datasource = datasource ?? { + name: 'TestData', + metricFindQuery: () => undefined, + variables: { getType: () => VariableSupportType.Standard, toDataQuery: () => undefined }, + }; + const get = jest.fn().mockResolvedValue(datasource); + const getDatasourceSrv = jest.fn().mockReturnValue({ get }); + const identifier = toVariableIdentifier(variable); + + return { state, get, getDatasourceSrv, identifier }; +} + +describe('upgradeLegacyQueries', () => { + describe('when called with a query variable for a standard variable supported data source that has not been upgraded', () => { + it('then it should dispatch changeVariableProp', async () => { + const { state, identifier, get, getDatasourceSrv } = getTestContext({ query: '*' }); + + const dispatchedActions = await thunkTester(state) + .givenThunk(upgradeLegacyQueries) + .whenThunkIsDispatched(identifier, getDatasourceSrv); + + expect(dispatchedActions).toEqual([ + changeVariableProp({ + type: 'query', + id: 'query', + data: { + propName: 'query', + propValue: { + refId: 'TestData-query-Variable-Query', + query: '*', + }, + }, + }), + ]); + expect(get).toHaveBeenCalledTimes(1); + expect(get).toHaveBeenCalledWith('test-data'); + }); + }); + + describe('when called with a query variable for a standard variable supported data source that has been upgraded', () => { + it('then it should not dispatch any actions', async () => { + const { state, identifier, get, getDatasourceSrv } = getTestContext({ query: { refId: 'A' } }); + + const dispatchedActions = await thunkTester(state) + .givenThunk(upgradeLegacyQueries) + .whenThunkIsDispatched(identifier, getDatasourceSrv); + + expect(dispatchedActions).toEqual([]); + expect(get).toHaveBeenCalledTimes(1); + expect(get).toHaveBeenCalledWith('test-data'); + }); + }); + + describe('when called with a query variable for a legacy variable supported data source', () => { + it('then it should not dispatch any actions', async () => { + const datasource = { + name: 'TestData', + metricFindQuery: () => undefined, + }; + const { state, identifier, get, getDatasourceSrv } = getTestContext({ datasource }); + + const dispatchedActions = await thunkTester(state) + .givenThunk(upgradeLegacyQueries) + .whenThunkIsDispatched(identifier, getDatasourceSrv); + + expect(dispatchedActions).toEqual([]); + expect(get).toHaveBeenCalledTimes(1); + expect(get).toHaveBeenCalledWith('test-data'); + }); + }); + + describe('when called with a query variable for a custom variable supported data source', () => { + it('then it should not dispatch any actions', async () => { + const datasource = { + name: 'TestData', + metricFindQuery: () => undefined, + variables: { getType: () => VariableSupportType.Custom, query: () => undefined, editor: {} }, + }; + const { state, identifier, get, getDatasourceSrv } = getTestContext({ datasource }); + + const dispatchedActions = await thunkTester(state) + .givenThunk(upgradeLegacyQueries) + .whenThunkIsDispatched(identifier, getDatasourceSrv); + + expect(dispatchedActions).toEqual([]); + expect(get).toHaveBeenCalledTimes(1); + expect(get).toHaveBeenCalledWith('test-data'); + }); + }); + + describe('when called with a query variable for a datasource variable supported data source', () => { + it('then it should not dispatch any actions', async () => { + const datasource = { + name: 'TestData', + metricFindQuery: () => undefined, + variables: { getType: () => VariableSupportType.Datasource }, + }; + const { state, identifier, get, getDatasourceSrv } = getTestContext({ datasource }); + + const dispatchedActions = await thunkTester(state) + .givenThunk(upgradeLegacyQueries) + .whenThunkIsDispatched(identifier, getDatasourceSrv); + + expect(dispatchedActions).toEqual([]); + expect(get).toHaveBeenCalledTimes(1); + expect(get).toHaveBeenCalledWith('test-data'); + }); + }); + + describe('when called with a custom variable', () => { + it('then it should not dispatch any actions', async () => { + const variable = customBuilder() + .withId('custom') + .withName('custom') + .build(); + const { state, identifier, get, getDatasourceSrv } = getTestContext({ variable }); + + const dispatchedActions = await thunkTester(state) + .givenThunk(upgradeLegacyQueries) + .whenThunkIsDispatched(identifier, getDatasourceSrv); + + expect(dispatchedActions).toEqual([]); + expect(get).toHaveBeenCalledTimes(0); + }); + }); +});