diff --git a/public/app/features/variables/state/actions.test.ts b/public/app/features/variables/state/actions.test.ts index 6f8c630b66e..1e01394b375 100644 --- a/public/app/features/variables/state/actions.test.ts +++ b/public/app/features/variables/state/actions.test.ts @@ -60,8 +60,7 @@ import { expect } from '../../../../test/lib/common'; import { ConstantVariableModel, VariableRefresh } from '../types'; import { updateVariableOptions } from '../query/reducer'; import { setVariableQueryRunner, VariableQueryRunner } from '../query/VariableQueryRunner'; -import { setDataSourceSrv } from '@grafana/runtime'; -import { LocationState } from 'app/types'; +import { setDataSourceSrv, setLocationService } from '@grafana/runtime'; variableAdapters.setInit(() => [ createQueryVariableAdapter(), @@ -146,8 +145,9 @@ describe('shared actions', () => { const list = [query, constant, datasource, custom, textbox]; const preloadedState = { templating: ({} as unknown) as TemplatingState, - location: ({ query: {} } as unknown) as LocationState, }; + const locationService: any = { getSearchObject: () => ({}) }; + setLocationService(locationService); const tester = await reduxTester({ preloadedState }) .givenRootReducer(getTemplatingRootReducer()) @@ -203,9 +203,10 @@ describe('shared actions', () => { const list = [stats, substats]; const query = { orgId: '1', 'var-stats': 'response', 'var-substats': ALL_VARIABLE_TEXT }; + const locationService: any = { getSearchObject: () => query }; + setLocationService(locationService); const preloadedState = { templating: ({} as unknown) as TemplatingState, - location: ({ query } as unknown) as LocationState, }; const tester = await reduxTester({ preloadedState }) @@ -223,6 +224,9 @@ describe('shared actions', () => { toVariablePayload(stats, { option: { text: ALL_VARIABLE_TEXT, value: ALL_VARIABLE_VALUE, selected: false } }) ), variableStateCompleted(toVariablePayload(stats)), + setCurrentVariableValue( + toVariablePayload(stats, { option: { text: ['response'], value: ['response'], selected: false } }) + ), variableStateFetching(toVariablePayload(substats)), updateVariableOptions( toVariablePayload(substats, { results: [{ text: '200' }, { text: '500' }], templatedRegex: '' }) @@ -232,7 +236,12 @@ describe('shared actions', () => { option: { text: [ALL_VARIABLE_TEXT], value: [ALL_VARIABLE_VALUE], selected: true }, }) ), - variableStateCompleted(toVariablePayload(substats)) + variableStateCompleted(toVariablePayload(substats)), + setCurrentVariableValue( + toVariablePayload(substats, { + option: { text: [ALL_VARIABLE_TEXT], value: [ALL_VARIABLE_VALUE], selected: false }, + }) + ) ); }); }); diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts index 6b91094a1c9..d110ea37a97 100644 --- a/public/app/features/variables/state/actions.ts +++ b/public/app/features/variables/state/actions.ts @@ -52,7 +52,7 @@ import { import { getBackendSrv } from '../../../core/services/backend_srv'; import { cleanVariables } from './variablesReducer'; import isEqual from 'lodash/isEqual'; -import { getCurrentText, getVariableRefresh } from '../utils'; +import { ensureStringValues, getCurrentText, getVariableRefresh } from '../utils'; import { store } from 'app/store/store'; import { getDatasourceSrv } from '../../plugins/datasource_srv'; import { cleanEditorState } from '../editor/reducer'; @@ -247,7 +247,8 @@ export const processVariable = ( const urlValue = queryParams['var-' + variable.name]; if (urlValue !== void 0) { - await variableAdapters.get(variable.type).setValueFromUrl(variable, urlValue ?? ''); + const stringUrlValue = ensureStringValues(urlValue); + await variableAdapters.get(variable.type).setValueFromUrl(variable, stringUrlValue); return; } @@ -283,6 +284,7 @@ export const setOptionFromUrl = ( urlValue: UrlQueryValue ): ThunkResult> => { return async (dispatch, getState) => { + const stringUrlValue = ensureStringValues(urlValue); const variable = getVariable(identifier.id, getState()); if (getVariableRefresh(variable) !== VariableRefresh.never) { // updates options @@ -296,23 +298,22 @@ export const setOptionFromUrl = ( } // Simple case. Value in url matches existing options text or value. let option = variableFromState.options.find((op) => { - return op.text === urlValue || op.value === urlValue; + return op.text === stringUrlValue || op.value === stringUrlValue; }); if (!option && isMulti(variableFromState)) { - if (variableFromState.allValue && urlValue === variableFromState.allValue) { + if (variableFromState.allValue && stringUrlValue === variableFromState.allValue) { option = { text: ALL_VARIABLE_TEXT, value: ALL_VARIABLE_VALUE, selected: false }; } } if (!option) { - let defaultText = urlValue as string | string[]; - const defaultValue = urlValue as string | string[]; + let defaultText = stringUrlValue; + const defaultValue = stringUrlValue; - if (Array.isArray(urlValue)) { + if (Array.isArray(stringUrlValue)) { // Multiple values in the url. We construct text as a list of texts from all matched options. - const urlValueArray = urlValue as string[]; - defaultText = urlValueArray.reduce((acc: string[], item: string) => { + defaultText = stringUrlValue.reduce((acc: string[], item: string) => { const foundOption = variableFromState.options.find((o) => o.value === item); if (!foundOption) { // @ts-ignore according to strict null errors this can never happen @@ -569,8 +570,9 @@ export const templateVarsChangedInUrl = (vars: UrlQueryMap): ThunkResult = }; const isVariableUrlValueDifferentFromCurrent = (variable: VariableModel, urlValue: any): boolean => { + const stringUrlValue = ensureStringValues(urlValue); // lodash isEqual handles array of value equality checks as well - return !isEqual(variableAdapters.get(variable.type).getValueForUrl(variable), urlValue); + return !isEqual(variableAdapters.get(variable.type).getValueForUrl(variable), stringUrlValue); }; const getQueryWithVariables = (getState: () => StoreState): UrlQueryMap => { diff --git a/public/app/features/variables/state/onTimeRangeUpdated.test.ts b/public/app/features/variables/state/onTimeRangeUpdated.test.ts index e006a69be1b..bcd8b4a0c43 100644 --- a/public/app/features/variables/state/onTimeRangeUpdated.test.ts +++ b/public/app/features/variables/state/onTimeRangeUpdated.test.ts @@ -65,7 +65,6 @@ const getTestContext = () => { const adapter = variableAdapters.get('interval'); const preloadedState = ({ dashboard, - location: { query: '' }, templating: ({ variables: { 'interval-0': { ...interval }, diff --git a/public/app/features/variables/state/setOptionFromUrl.test.ts b/public/app/features/variables/state/setOptionFromUrl.test.ts index be4fb967175..b1d3d5c5b4a 100644 --- a/public/app/features/variables/state/setOptionFromUrl.test.ts +++ b/public/app/features/variables/state/setOptionFromUrl.test.ts @@ -17,15 +17,15 @@ describe('when setOptionFromUrl is dispatched with a custom variable (no refresh ${['B']} | ${false} | ${'B'} ${'X'} | ${false} | ${'X'} ${''} | ${false} | ${''} - ${null} | ${false} | ${null} - ${undefined} | ${false} | ${undefined} + ${null} | ${false} | ${''} + ${undefined} | ${false} | ${''} ${'B'} | ${true} | ${['B']} ${['B']} | ${true} | ${['B']} ${'X'} | ${true} | ${['X']} ${''} | ${true} | ${['']} ${['A', 'B']} | ${true} | ${['A', 'B']} - ${null} | ${true} | ${[null]} - ${undefined} | ${true} | ${[undefined]} + ${null} | ${true} | ${['']} + ${undefined} | ${true} | ${['']} `('and urlValue is $urlValue then correct actions are dispatched', async ({ urlValue, expected, isMulti }) => { const custom = customBuilder().withId('0').withMulti(isMulti).withOptions('A', 'B', 'C').withCurrent('A').build(); diff --git a/public/app/features/variables/state/sharedReducer.ts b/public/app/features/variables/state/sharedReducer.ts index a31eafe2a36..e9bc0f2a795 100644 --- a/public/app/features/variables/state/sharedReducer.ts +++ b/public/app/features/variables/state/sharedReducer.ts @@ -9,6 +9,7 @@ import { variableAdapters } from '../adapters'; import { changeVariableNameSucceeded } from '../editor/reducer'; import { initialVariablesState, VariablesState } from './variablesReducer'; import { isQuery } from '../guard'; +import { ensureStringValues } from '../utils'; const sharedReducerSlice = createSlice({ name: 'templating/shared', @@ -121,10 +122,12 @@ const sharedReducerSlice = createSlice({ } const instanceState = getInstanceState(state, action.payload.id); - const current = { ...action.payload.data.option }; + const { option } = action.payload.data; + const current = { ...option, text: ensureStringValues(option?.text), value: ensureStringValues(option?.value) }; instanceState.current = current; instanceState.options = instanceState.options.map((option) => { + option.value = ensureStringValues(option.value); let selected = false; if (Array.isArray(current.value)) { for (let index = 0; index < current.value.length; index++) { diff --git a/public/app/features/variables/textbox/actions.ts b/public/app/features/variables/textbox/actions.ts index 2874305acab..8796d655a5a 100644 --- a/public/app/features/variables/textbox/actions.ts +++ b/public/app/features/variables/textbox/actions.ts @@ -7,6 +7,7 @@ import { toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../ import { setOptionFromUrl } from '../state/actions'; import { UrlQueryValue } from '@grafana/data'; import { changeVariableProp } from '../state/sharedReducer'; +import { ensureStringValues } from '../utils'; export const updateTextBoxVariableOptions = (identifier: VariableIdentifier): ThunkResult => { return async (dispatch, getState) => { @@ -23,7 +24,8 @@ export const setTextBoxVariableOptionsFromUrl = ( ): ThunkResult => async (dispatch, getState) => { const variableInState = getVariable(identifier.id, getState()); - dispatch(changeVariableProp(toVariablePayload(variableInState, { propName: 'query', propValue: urlValue }))); + const stringUrlValue = ensureStringValues(urlValue); + dispatch(changeVariableProp(toVariablePayload(variableInState, { propName: 'query', propValue: stringUrlValue }))); - await dispatch(setOptionFromUrl(toVariableIdentifier(variableInState), urlValue)); + await dispatch(setOptionFromUrl(toVariableIdentifier(variableInState), stringUrlValue)); }; diff --git a/public/app/features/variables/utils.test.ts b/public/app/features/variables/utils.test.ts index deee114779a..e6ed4bc3fa1 100644 --- a/public/app/features/variables/utils.test.ts +++ b/public/app/features/variables/utils.test.ts @@ -1,4 +1,4 @@ -import { findTemplateVarChanges, getCurrentText, getVariableRefresh, isAllVariable } from './utils'; +import { ensureStringValues, findTemplateVarChanges, getCurrentText, getVariableRefresh, isAllVariable } from './utils'; import { VariableRefresh } from './types'; import { UrlQueryMap } from '@grafana/data'; @@ -157,3 +157,19 @@ describe('findTemplateVarChanges', () => { expect(findTemplateVarChanges(a, b)!['var-test']).toEqual(['test']); }); }); + +describe('ensureStringValues', () => { + it.each` + value | expected + ${null} | ${''} + ${undefined} | ${''} + ${{}} | ${''} + ${{ current: {} }} | ${''} + ${1} | ${'1'} + ${[1, 2]} | ${['1', '2']} + ${'1'} | ${'1'} + ${['1', '2']} | ${['1', '2']} + `('when called with value:$value then result should be:$expected', ({ value, expected }) => { + expect(ensureStringValues(value)).toEqual(expected); + }); +}); diff --git a/public/app/features/variables/utils.ts b/public/app/features/variables/utils.ts index fdfdd3224e4..e2cf06f08e8 100644 --- a/public/app/features/variables/utils.ts +++ b/public/app/features/variables/utils.ts @@ -223,3 +223,23 @@ export function findTemplateVarChanges(query: UrlQueryMap, old: UrlQueryMap): Ur } return count ? changes : undefined; } + +export function ensureStringValues(value: any | any[]): string | string[] { + if (Array.isArray(value)) { + return value.map(String); + } + + if (value === null || value === undefined) { + return ''; + } + + if (typeof value === 'number') { + return value.toString(10); + } + + if (typeof value === 'string') { + return value; + } + + return ''; +}