Templating: global/system variables should be properly replaced in templated values. (#27394)

* Fixed so we try to use the variables in the redux store to replace values in template variables.

* First draft of working version.

* Including fieldPath when adding :text format.

* cleaned up code by introducing helper function.

* some minor refactoring.

* Added tests and support for multi variables.

* added test and code to handle the All scenario of a multivariable.

* fixed according to feedback.

* added docs.

* added text format to gdev dashboard.

* updated e2e tests.

* make sure we use the same function for formatting och variable lable.

* increased the number to 22.

* changed label for tests to be All.

* existing format should be respected.

devenv/dev-dashboards/feature-templating/global-variables-and-interpolation.json

@@ -34,7 +34,7 @@
       },
       "id": 11,
       "options": {
-        "content": "## Global variables\n\n* `__dashboard` = `${__dashboard}`\n* `__dashboard.name` = `${__dashboard.name}`\n* `__dashboard.uid` = `${__dashboard.uid}`\n* `__org.name` = `${__org.name}`\n* `__org.id` = `${__org.id}`\n* `__user.id` = `${__user.id}`\n* `__user.login` = `${__user.login}`\n         \n## Formats\n\n* `Server:raw` = `${Server:raw}`\n* `Server:regex` = `${Server:regex}`\n* `Server:lucene` = `${Server:lucene}`\n* `Server:glob` = `${Server:glob}`\n* `Server:pipe` = `${Server:pipe}`\n* `Server:distributed` = `${Server:distributed}`\n* `Server:csv` = `${Server:csv}`\n* `Server:html` = `${Server:html}`\n* `Server:json` = `${Server:json}`\n* `Server:percentencode` = `${Server:percentencode}`\n* `Server:singlequote` = `${Server:singlequote}`\n* `Server:doublequote` = `${Server:doublequote}`\n* `Server:sqlstring` = `${Server:sqlstring}`\n* `Server:date` = `${Server:date}`\n\n",
+        "content": "## Global variables\n\n* `__dashboard` = `${__dashboard}`\n* `__dashboard.name` = `${__dashboard.name}`\n* `__dashboard.uid` = `${__dashboard.uid}`\n* `__org.name` = `${__org.name}`\n* `__org.id` = `${__org.id}`\n* `__user.id` = `${__user.id}`\n* `__user.login` = `${__user.login}`\n         \n## Formats\n\n* `Server:raw` = `${Server:raw}`\n* `Server:regex` = `${Server:regex}`\n* `Server:lucene` = `${Server:lucene}`\n* `Server:glob` = `${Server:glob}`\n* `Server:pipe` = `${Server:pipe}`\n* `Server:distributed` = `${Server:distributed}`\n* `Server:csv` = `${Server:csv}`\n* `Server:html` = `${Server:html}`\n* `Server:json` = `${Server:json}`\n* `Server:percentencode` = `${Server:percentencode}`\n* `Server:singlequote` = `${Server:singlequote}`\n* `Server:doublequote` = `${Server:doublequote}`\n* `Server:sqlstring` = `${Server:sqlstring}`\n* `Server:date` = `${Server:date}`\n* `Server:text` = `${Server:text}`\n\n",
         "mode": "markdown"
       },
       "pluginVersion": "7.1.0",

docs/sources/variables/advanced-variable-format-options.md

@@ -143,3 +143,13 @@ servers = ["test'1", "test2"]
 String to interpolate: '${servers:sqlstring}'
 Interpolation result: "'test''1','test2'"
 ```
+
+## Text
+
+Formats single- and multi-valued variables into their text representation. For a single variable it will just return the text representation. For multi-valued variables it will return the text representation combined with `+`.
+
+```bash
+servers = ["test1", "test2"]
+String to interpolate: '${servers:text}'
+Interpolation result: "test1 + test2"
+```

e2e/suite1/specs/dashboard-templating.spec.ts

@@ -33,11 +33,12 @@ e2e.scenario({
       `Server:doublequote = "A'A\\"A","BB\\B","CCC"`,
       `Server:sqlstring = 'A''A"A','BB\\\B','CCC'`,
       `Server:date = null`,
+      `Server:text = All`,
     ];
 
     e2e()
       .get('.markdown-html li')
-      .should('have.length', 21)
+      .should('have.length', 22)
       .each(element => {
         items.push(element.text());
       })

public/app/features/templating/formatRegistry.ts

@@ -1,9 +1,16 @@
 import kbn from 'app/core/utils/kbn';
 import { Registry, RegistryItem, VariableModel, textUtil, dateTime } from '@grafana/data';
 import { map, isArray, replace } from 'lodash';
+import { formatVariableLabel } from '../variables/shared/formatVariable';
+
+export interface FormatOptions {
+  value: any;
+  text: string;
+  args: string[];
+}
 
 export interface FormatRegistryItem extends RegistryItem {
-  formatter(value: any, args: string[], variable: VariableModel): string;
+  formatter(options: FormatOptions, variable: VariableModel): string;
 }
 
 export const formatRegistry = new Registry<FormatRegistryItem>(() => {
@@ -12,7 +19,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'lucene',
       name: 'Lucene',
       description: 'Values are lucene escaped and multi-valued variables generate an OR expression',
-      formatter: value => {
+      formatter: ({ value }) => {
         if (typeof value === 'string') {
           return luceneEscape(value);
         }
@@ -32,13 +39,13 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'raw',
       name: 'raw',
       description: 'Keep value as is',
-      formatter: value => value,
+      formatter: ({ value }) => value,
     },
     {
       id: 'regex',
       name: 'Regex',
       description: 'Values are regex escaped and multi-valued variables generate a (<value>|<value>) expression',
-      formatter: value => {
+      formatter: ({ value }) => {
         if (typeof value === 'string') {
           return kbn.regexEscape(value);
         }
@@ -54,7 +61,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'pipe',
       name: 'Pipe',
       description: 'Values are seperated by | character',
-      formatter: value => {
+      formatter: ({ value }) => {
         if (typeof value === 'string') {
           return value;
         }
@@ -65,7 +72,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'distributed',
       name: 'Distributed',
       description: 'Multiple values are formatted like variable=value',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }, variable) => {
         if (typeof value === 'string') {
           return value;
         }
@@ -84,7 +91,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'csv',
       name: 'Csv',
       description: 'Comma seperated values',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }) => {
         if (isArray(value)) {
           return value.join(',');
         }
@@ -95,7 +102,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'html',
       name: 'HTML',
       description: 'HTML escaping of values',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }) => {
         if (isArray(value)) {
           return textUtil.escapeHtml(value.join(', '));
         }
@@ -106,7 +113,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'json',
       name: 'JSON',
       description: 'JSON stringify valu',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }) => {
         return JSON.stringify(value);
       },
     },
@@ -114,7 +121,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'percentencode',
       name: 'Percent encode',
       description: 'Useful for url escaping values',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }) => {
         // like glob, but url escaped
         if (isArray(value)) {
           return encodeURIComponentStrict('{' + value.join(',') + '}');
@@ -126,7 +133,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'singlequote',
       name: 'Single quote',
       description: 'Single quoted values',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }) => {
         // escape single quotes with backslash
         const regExp = new RegExp(`'`, 'g');
         if (isArray(value)) {
@@ -139,7 +146,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'doublequote',
       name: 'Double quote',
       description: 'Double quoted values',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }) => {
         // escape double quotes with backslash
         const regExp = new RegExp('"', 'g');
         if (isArray(value)) {
@@ -152,7 +159,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'sqlstring',
       name: 'SQL string',
       description: 'SQL string quoting and commas for use in IN statements and other scenarios',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }) => {
         // escape single quotes by pairing them
         const regExp = new RegExp(`'`, 'g');
         if (isArray(value)) {
@@ -165,7 +172,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'date',
       name: 'Date',
       description: 'Format date in different ways',
-      formatter: (value, args, variable) => {
+      formatter: ({ value, args }) => {
         const arg = args[0] ?? 'iso';
 
         switch (arg) {
@@ -184,13 +191,31 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       id: 'glob',
       name: 'Glob',
       description: 'Format multi valued variables using glob syntax, example {value1,value2}',
-      formatter: (value, args, variable) => {
+      formatter: ({ value }) => {
         if (isArray(value) && value.length > 1) {
           return '{' + value.join(',') + '}';
         }
         return value;
       },
     },
+    {
+      id: 'text',
+      name: 'Text',
+      description: 'Format variables in their text representation. Example in multi variable scenario A + B + C.',
+      formatter: (options, variable) => {
+        if (typeof options.text === 'string') {
+          return options.text;
+        }
+
+        const current = (variable as any)?.current;
+
+        if (!current) {
+          return options.value;
+        }
+
+        return formatVariableLabel(variable);
+      },
+    },
   ];
 
   return formats;

public/app/features/templating/template_srv.test.ts

@@ -95,11 +95,18 @@ describe('templateSrv', () => {
       expect(target).toBe('this.asd.filters');
     });
 
-    it('should replace ${test:glob} with scoped text', () => {
+    it('should replace ${test.name} with scoped text', () => {
+      const target = _templateSrv.replaceWithText('this.${test.name}.filters', {
+        test: { value: { name: 'mupp' }, text: 'asd' },
+      });
+      expect(target).toBe('this.mupp.filters');
+    });
+
+    it('should not replace ${test:glob} with scoped text', () => {
       const target = _templateSrv.replaceWithText('this.${test:glob}.filters', {
         test: { value: 'mupp', text: 'asd' },
       });
-      expect(target).toBe('this.asd.filters');
+      expect(target).toBe('this.mupp.filters');
     });
   });
 
@@ -595,6 +602,45 @@ describe('templateSrv', () => {
     });
   });
 
+  describe('replaceWithText can pass all / multi value', () => {
+    beforeEach(() => {
+      initTemplateSrv([
+        {
+          type: 'query',
+          name: 'server',
+          current: { value: ['server1', 'server2'], text: ['Server 1', 'Server 2'] },
+        },
+        {
+          type: 'textbox',
+          name: 'empty_on_init',
+          current: { value: '', text: '' },
+        },
+        {
+          type: 'query',
+          name: 'databases',
+          current: { value: '$__all', text: '' },
+          options: [{ value: '$__all' }, { value: 'db1', text: 'Database 1' }, { value: 'db2', text: 'Database 2' }],
+        },
+      ]);
+      _templateSrv.updateIndex();
+    });
+
+    it('should replace with text with variable label', () => {
+      const target = _templateSrv.replaceWithText('Server: $server');
+      expect(target).toBe('Server: Server 1 + Server 2');
+    });
+
+    it('should replace empty string-values with an empty string', () => {
+      const target = _templateSrv.replaceWithText('Hello $empty_on_init');
+      expect(target).toBe('Hello ');
+    });
+
+    it('should replace $__all with All', () => {
+      const target = _templateSrv.replaceWithText('Db: $databases');
+      expect(target).toBe('Db: All');
+    });
+  });
+
   describe('built in interval variables', () => {
     beforeEach(() => {
       initTemplateSrv([]);

public/app/features/templating/template_srv.ts

@@ -6,7 +6,8 @@ import { isAdHoc } from '../variables/guard';
 import { VariableModel } from '../variables/types';
 import { setTemplateSrv, TemplateSrv as BaseTemplateSrv } from '@grafana/runtime';
 import { variableAdapters } from '../variables/adapters';
-import { formatRegistry } from './formatRegistry';
+import { formatRegistry, FormatOptions } from './formatRegistry';
+import { ALL_VARIABLE_TEXT } from '../variables/state/types';
 
 interface FieldAccessorCache {
   [key: string]: (obj: any) => any;
@@ -107,7 +108,7 @@ export class TemplateSrv implements BaseTemplateSrv {
     return filters;
   }
 
-  formatValue(value: any, format: any, variable: any) {
+  formatValue(value: any, format: any, variable: any, text?: string) {
     // for some scopedVars there is no variable
     variable = variable || {};
 
@@ -133,7 +134,8 @@ export class TemplateSrv implements BaseTemplateSrv {
       throw new Error(`Variable format ${format} not found`);
     }
 
-    return formatItem.formatter(value, args, variable);
+    const options: FormatOptions = { value, args, text: text ?? value };
+    return formatItem.formatter(options, variable);
   }
 
   setGrafanaVariable(name: string, value: any) {
@@ -197,7 +199,7 @@ export class TemplateSrv implements BaseTemplateSrv {
     return values;
   }
 
-  getFieldAccessor(fieldPath: string) {
+  private getFieldAccessor(fieldPath: string) {
     const accessor = this.fieldAccessorCache[fieldPath];
     if (accessor) {
       return accessor;
@@ -206,7 +208,7 @@ export class TemplateSrv implements BaseTemplateSrv {
     return (this.fieldAccessorCache[fieldPath] = _.property(fieldPath));
   }
 
-  getVariableValue(variableName: string, fieldPath: string | undefined, scopedVars: ScopedVars) {
+  private getVariableValue(variableName: string, fieldPath: string | undefined, scopedVars: ScopedVars) {
     const scopedVar = scopedVars[variableName];
     if (!scopedVar) {
       return null;
@@ -219,6 +221,20 @@ export class TemplateSrv implements BaseTemplateSrv {
     return scopedVar.value;
   }
 
+  private getVariableText(variableName: string, value: any, scopedVars: ScopedVars) {
+    const scopedVar = scopedVars[variableName];
+
+    if (!scopedVar) {
+      return null;
+    }
+
+    if (scopedVar.value === value || typeof value !== 'string') {
+      return scopedVar.text;
+    }
+
+    return value;
+  }
+
   replace(target?: string, scopedVars?: ScopedVars, format?: string | Function): string {
     if (!target) {
       return target ?? '';
@@ -233,8 +249,10 @@ export class TemplateSrv implements BaseTemplateSrv {
 
       if (scopedVars) {
         const value = this.getVariableValue(variableName, fieldPath, scopedVars);
+        const text = this.getVariableText(variableName, value, scopedVars);
+
         if (value !== null && value !== undefined) {
-          return this.formatValue(value, fmt, variable);
+          return this.formatValue(value, fmt, variable, text);
         }
       }
 
@@ -248,8 +266,11 @@ export class TemplateSrv implements BaseTemplateSrv {
       }
 
       let value = variable.current.value;
+      let text = variable.current.text;
+
       if (this.isAllValue(value)) {
         value = this.getAllValue(variable);
+        text = ALL_VARIABLE_TEXT;
         // skip formatting of custom all values
         if (variable.allValue) {
           return this.replace(value);
@@ -258,14 +279,14 @@ export class TemplateSrv implements BaseTemplateSrv {
 
       if (fieldPath) {
         const fieldValue = this.getVariableValue(variableName, fieldPath, {
-          [variableName]: { value: value, text: '' },
+          [variableName]: { value, text },
         });
         if (fieldValue !== null && fieldValue !== undefined) {
-          return this.formatValue(fieldValue, fmt, variable);
+          return this.formatValue(fieldValue, fmt, variable, text);
         }
       }
 
-      const res = this.formatValue(value, fmt, variable);
+      const res = this.formatValue(value, fmt, variable, text);
       return res;
     });
   }
@@ -275,30 +296,8 @@ export class TemplateSrv implements BaseTemplateSrv {
   }
 
   replaceWithText(target: string, scopedVars?: ScopedVars) {
-    if (!target) {
+    deprecationWarning('template_srv.ts', 'replaceWithText()', 'replace(), and specify the :text format');
-      return target;
+    return this.replace(target, scopedVars, 'text');
-    }
-
-    let variable;
-    this.regex.lastIndex = 0;
-
-    return target.replace(this.regex, (match: any, var1: any, var2: any, fmt2: any, var3: any) => {
-      if (scopedVars) {
-        const option = scopedVars[var1 || var2 || var3];
-        if (option) {
-          return option.text;
-        }
-      }
-
-      variable = this.getVariableAtIndex(var1 || var2 || var3);
-      if (!variable) {
-        return match;
-      }
-
-      const value = this.grafanaVariables[variable.current.value];
-
-      return typeof value === 'string' ? value : variable.current.text;
-    });
   }
 
   fillVariableValuesForUrl = (params: any, scopedVars?: ScopedVars) => {

public/app/features/variables/pickers/OptionsPicker/OptionsPicker.tsx

@@ -10,6 +10,7 @@ import { VariableOption, VariableTag, VariableWithMultiSupport, VariableWithOpti
 import { VariableOptions } from '../shared/VariableOptions';
 import { isQuery } from '../../guard';
 import { VariablePickerProps } from '../types';
+import { formatVariableLabel } from '../../shared/formatVariable';
 
 interface OwnProps extends VariablePickerProps<VariableWithMultiSupport> {}
 
@@ -64,7 +65,7 @@ export class OptionsPickerUnconnected extends PureComponent<Props> {
       return null;
     }
 
-    const linkText = getLinkText(variable);
+    const linkText = formatVariableLabel(variable);
     const tags = getSelectedTags(variable);
 
     return <VariableLink text={linkText} tags={tags} onClick={this.onShowOptions} />;
@@ -104,44 +105,6 @@ const getSelectedTags = (variable: VariableWithOptions): VariableTag[] => {
   return variable.tags.filter(t => t.selected);
 };
 
-const getLinkText = (variable: VariableWithOptions) => {
-  const { current, options } = variable;
-
-  if (!current.tags || current.tags.length === 0) {
-    if (Array.isArray(current.text)) {
-      return current.text.join(' + ');
-    }
-    return current.text;
-  }
-
-  // filer out values that are in selected tags
-  const selectedAndNotInTag = options.filter(option => {
-    if (!option.selected) {
-      return false;
-    }
-
-    if (!current || !current.tags || !current.tags.length) {
-      return false;
-    }
-
-    for (let i = 0; i < current.tags.length; i++) {
-      const tag = current.tags[i];
-      const foundIndex = tag?.values?.findIndex(v => v === option.value);
-      if (foundIndex && foundIndex !== -1) {
-        return false;
-      }
-    }
-    return true;
-  });
-
-  // convert values to text
-  const currentTexts = selectedAndNotInTag.map(s => s.text);
-
-  // join texts
-  const newLinkText = currentTexts.join(' + ');
-  return newLinkText.length > 0 ? `${newLinkText} + ` : newLinkText;
-};
-
 const mapDispatchToProps: MapDispatchToProps<DispatchProps, OwnProps> = {
   showOptions,
   commitChangesToVariable,

public/app/features/variables/shared/formatVariable.ts

@@ -0,0 +1,51 @@
+import { VariableModel } from '@grafana/data';
+import { VariableWithOptions } from '../types';
+
+export const formatVariableLabel = (variable: VariableModel) => {
+  if (!isVariableWithOptions(variable)) {
+    return variable.name;
+  }
+
+  const { current, options = [] } = variable;
+
+  if (!current.tags || current.tags.length === 0) {
+    if (Array.isArray(current.text)) {
+      return current.text.join(' + ');
+    }
+    return current.text;
+  }
+
+  // filer out values that are in selected tags
+  const selectedAndNotInTag = options.filter(option => {
+    if (!option.selected) {
+      return false;
+    }
+
+    if (!current || !current.tags || !current.tags.length) {
+      return false;
+    }
+
+    for (let i = 0; i < current.tags.length; i++) {
+      const tag = current.tags[i];
+      const foundIndex = tag?.values?.findIndex(v => v === option.value);
+      if (foundIndex && foundIndex !== -1) {
+        return false;
+      }
+    }
+    return true;
+  });
+
+  // convert values to text
+  const currentTexts = selectedAndNotInTag.map(s => s.text);
+
+  // join texts
+  const newLinkText = currentTexts.join(' + ');
+  return newLinkText.length > 0 ? `${newLinkText} + ` : newLinkText;
+};
+
+const isVariableWithOptions = (variable: VariableModel): variable is VariableWithOptions => {
+  return (
+    Array.isArray((variable as VariableWithOptions)?.options) ||
+    typeof (variable as VariableWithOptions)?.current === 'object'
+  );
+};