Templating: Changing between variables with the same name now correctly triggers a dashboard refresh (#51490)

* user essentials mob! 🔱

* user essentials mob! 🔱

lastFile:public/app/features/variables/pickers/OptionsPicker/actions.ts

* user essentials mob! 🔱

lastFile:public/app/features/variables/pickers/OptionsPicker/actions.test.ts

* linting

* update betterer

Co-authored-by: kay delaney <kay@grafana.com>
This commit is contained in:
Ashley Harrison 2022-06-29 09:10:55 +01:00 committed by GitHub
parent 9595fd6b66
commit 683bbc7373
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 70 additions and 19 deletions

View File

@ -319,7 +319,7 @@ exports[`better eslint`] = {
[91, 13, 28, "Do not use any type assertions.", "2187961375"],
[93, 13, 24, "Do not use any type assertions.", "1019297299"]
],
"packages/grafana-data/src/datetime/rangeutil.ts:2182917545": [
"packages/grafana-data/src/datetime/rangeutil.ts:2234129242": [
[88, 18, 3, "Unexpected any. Specify a different type.", "193409811"],
[89, 27, 3, "Unexpected any. Specify a different type.", "193409811"],
[92, 33, 3, "Unexpected any. Specify a different type.", "193409811"],
@ -1715,7 +1715,7 @@ exports[`better eslint`] = {
"packages/grafana-ui/src/components/Graph/GraphContextMenu.tsx:3875310700": [
[22, 53, 3, "Unexpected any. Specify a different type.", "193409811"]
],
"packages/grafana-ui/src/components/Graph/utils.ts:3717868215": [
"packages/grafana-ui/src/components/Graph/utils.ts:3619837376": [
[104, 56, 3, "Unexpected any. Specify a different type.", "193409811"]
],
"packages/grafana-ui/src/components/GraphNG/GraphNG.tsx:94669281": [
@ -2299,7 +2299,7 @@ exports[`better eslint`] = {
[59, 12, 198, "Do not use any type assertions.", "2203342350"],
[65, 26, 341, "Do not use any type assertions.", "3792878509"]
],
"packages/grafana-ui/src/components/uPlot/config/UPlotAxisBuilder.ts:895023121": [
"packages/grafana-ui/src/components/uPlot/config/UPlotAxisBuilder.ts:3245297597": [
[23, 20, 3, "Unexpected any. Specify a different type.", "193409811"],
[170, 39, 3, "Unexpected any. Specify a different type.", "193409811"],
[174, 5, 13, "Do not use any type assertions.", "268797995"],
@ -7457,11 +7457,11 @@ exports[`better eslint`] = {
"public/app/features/variables/pickers/OptionsPicker/OptionPicker.test.tsx:2807633419": [
[51, 15, 3, "Unexpected any. Specify a different type.", "193409811"]
],
"public/app/features/variables/pickers/OptionsPicker/actions.test.ts:436404309": [
[365, 15, 14, "Do not use any type assertions.", "241960896"],
[365, 24, 3, "Unexpected any. Specify a different type.", "193409811"]
"public/app/features/variables/pickers/OptionsPicker/actions.test.ts:1831073775": [
[400, 15, 14, "Do not use any type assertions.", "241960896"],
[400, 24, 3, "Unexpected any. Specify a different type.", "193409811"]
],
"public/app/features/variables/pickers/OptionsPicker/actions.ts:1878014439": [
"public/app/features/variables/pickers/OptionsPicker/actions.ts:4177332433": [
[79, 74, 3, "Unexpected any. Specify a different type.", "193409811"],
[105, 61, 3, "Unexpected any. Specify a different type.", "193409811"]
],
@ -7781,18 +7781,18 @@ exports[`better eslint`] = {
[168, 77, 3, "Unexpected any. Specify a different type.", "193409811"],
[168, 100, 3, "Unexpected any. Specify a different type.", "193409811"]
],
"public/app/features/variables/utils.ts:3099442303": [
"public/app/features/variables/utils.ts:724230159": [
[58, 42, 3, "Unexpected any. Specify a different type.", "193409811"],
[74, 40, 3, "Unexpected any. Specify a different type.", "193409811"],
[108, 41, 3, "Unexpected any. Specify a different type.", "193409811"],
[145, 22, 3, "Unexpected any. Specify a different type.", "193409811"],
[158, 24, 30, "Do not use any type assertions.", "4095267826"],
[181, 41, 3, "Unexpected any. Specify a different type.", "193409811"],
[181, 47, 3, "Unexpected any. Specify a different type.", "193409811"],
[238, 42, 3, "Unexpected any. Specify a different type.", "193409811"],
[238, 48, 3, "Unexpected any. Specify a different type.", "193409811"],
[278, 44, 3, "Unexpected any. Specify a different type.", "193409811"],
[289, 45, 9, "Do not use any type assertions.", "744351187"]
[161, 22, 3, "Unexpected any. Specify a different type.", "193409811"],
[174, 24, 30, "Do not use any type assertions.", "4095267826"],
[197, 41, 3, "Unexpected any. Specify a different type.", "193409811"],
[197, 47, 3, "Unexpected any. Specify a different type.", "193409811"],
[254, 42, 3, "Unexpected any. Specify a different type.", "193409811"],
[254, 48, 3, "Unexpected any. Specify a different type.", "193409811"],
[294, 44, 3, "Unexpected any. Specify a different type.", "193409811"],
[305, 45, 9, "Do not use any type assertions.", "744351187"]
],
"public/app/plugins/datasource/alertmanager/ConfigEditor.tsx:2064418308": [
[50, 36, 41, "Do not use any type assertions.", "3906591734"]

View File

@ -190,6 +190,41 @@ describe('options picker actions', () => {
});
});
it('supports having variables with the same label and different values', async () => {
const options = [createOption('sameLabel', 'A'), createOption('sameLabel', 'B')];
const variable = createMultiVariable({
options,
current: createOption(['sameLabel'], ['A'], true),
includeAll: false,
});
const clearOthers = false;
const key = NavigationKey.selectAndClose;
// Open the menu and select the second option
const tester = await reduxTester<RootReducerType>()
.givenRootReducer(getRootReducer())
.whenActionIsDispatched(
toKeyedAction('key', addVariable(toVariablePayload(variable, { global: false, index: 0, model: variable })))
)
.whenActionIsDispatched(toKeyedAction('key', showOptions(variable)))
.whenActionIsDispatched(navigateOptions('key', NavigationKey.moveDown, clearOthers))
.whenActionIsDispatched(navigateOptions('key', NavigationKey.moveDown, clearOthers))
.whenAsyncActionIsDispatched(navigateOptions('key', key, clearOthers), true);
const option = createOption(['sameLabel'], ['B'], true);
// Check selecting the second option triggers variables to update
tester.thenDispatchedActionsShouldEqual(
toKeyedAction('key', toggleOption({ option: options[1], forceSelect: true, clearOthers })),
toKeyedAction('key', setCurrentVariableValue(toVariablePayload(variable, { option }))),
toKeyedAction('key', changeVariableProp(toVariablePayload(variable, { propName: 'queryValue', propValue: '' }))),
toKeyedAction('key', hideOptions()),
toKeyedAction('key', setCurrentVariableValue(toVariablePayload(variable, { option })))
);
expect(locationService.partial).toHaveBeenLastCalledWith({ 'var-Constant': ['B'] });
});
describe('when navigateOptions is dispatched with navigation key selectAndClose after highlighting the second option', () => {
it('then correct actions are dispatched', async () => {
const options = [createOption('A'), createOption('B'), createOption('C')];

View File

@ -8,7 +8,7 @@ import { getVariable, getVariablesState } from '../../state/selectors';
import { changeVariableProp, setCurrentVariableValue } from '../../state/sharedReducer';
import { KeyedVariableIdentifier } from '../../state/types';
import { VariableOption, VariableWithMultiSupport, VariableWithOptions } from '../../types';
import { containsSearchFilter, getCurrentText, toVariablePayload } from '../../utils';
import { containsSearchFilter, getCurrentValue, toVariablePayload } from '../../utils';
import { NavigationKey } from '../types';
import {
@ -90,7 +90,7 @@ export const commitChangesToVariable = (key: string, callback?: (updated: any) =
const updated = getVariable<VariableWithMultiSupport>(identifier, getState());
dispatch(toKeyedAction(key, hideOptions()));
if (getCurrentText(existing) === getCurrentText(updated)) {
if (getCurrentValue(existing) === getCurrentValue(updated)) {
return;
}

View File

@ -12,7 +12,7 @@ import { variableAdapters } from './adapters';
import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE } from './constants';
import { getVariablesState } from './state/selectors';
import { KeyedVariableIdentifier, VariableIdentifier, VariablePayload } from './state/types';
import { QueryVariableModel, TransactionStatus, VariableModel, VariableRefresh } from './types';
import { QueryVariableModel, TransactionStatus, VariableModel, VariableRefresh, VariableWithOptions } from './types';
/*
* This regex matches 3 types of variable reference with an optional format specifier
@ -130,6 +130,22 @@ export const getCurrentText = (variable: any): string => {
return variable.current.text;
};
export const getCurrentValue = (variable: VariableWithOptions): string | null => {
if (!variable || !variable.current || variable.current.value === undefined || variable.current.value === null) {
return null;
}
if (Array.isArray(variable.current.value)) {
return variable.current.value.toString();
}
if (typeof variable.current.value !== 'string') {
return null;
}
return variable.current.value;
};
export function getTemplatedRegex(variable: QueryVariableModel, templateSrv = getTemplateSrv()): string {
if (!variable) {
return '';