diff --git a/.betterer.results b/.betterer.results index ce4bc3b1eaf..f465107fe69 100644 --- a/.betterer.results +++ b/.betterer.results @@ -4088,7 +4088,10 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], "public/app/features/templating/template_srv.mock.ts:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"] + [0, 0, 0, "Do not use any type assertions.", "0"], + [0, 0, 0, "Do not use any type assertions.", "1"], + [0, 0, 0, "Do not use any type assertions.", "2"], + [0, 0, 0, "Do not use any type assertions.", "3"] ], "public/app/features/templating/template_srv.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/public/app/features/plugins/datasource_srv.ts b/public/app/features/plugins/datasource_srv.ts index 16f4c08f541..7e9b6affd1d 100644 --- a/public/app/features/plugins/datasource_srv.ts +++ b/public/app/features/plugins/datasource_srv.ts @@ -251,8 +251,10 @@ export class DatasourceSrv implements DataSourceService { continue; } let dsValue = variable.current.value === 'default' ? this.defaultName : variable.current.value; - if (Array.isArray(dsValue) && dsValue.length === 1) { - // Support for multi-value variables with only one selected datasource + // Support for multi-value DataSource (ds) variables + if (Array.isArray(dsValue)) { + // If the ds variable have multiple selected datasources + // We will use the first one dsValue = dsValue[0]; } const dsSettings = diff --git a/public/app/features/query/state/PanelQueryRunner.test.ts b/public/app/features/query/state/PanelQueryRunner.test.ts index 1d5e61cba4b..e57cc8ea7a1 100644 --- a/public/app/features/query/state/PanelQueryRunner.test.ts +++ b/public/app/features/query/state/PanelQueryRunner.test.ts @@ -3,8 +3,9 @@ const applyFieldOverridesMock = jest.fn(); // needs to be first in this file import { Subject } from 'rxjs'; // Importing this way to be able to spy on grafana/data + import * as grafanaData from '@grafana/data'; -import { DataSourceApi } from '@grafana/data'; +import { DataSourceApi, TypedVariableModel } from '@grafana/data'; import { DataSourceSrv, setDataSourceSrv, setEchoSrv } from '@grafana/runtime'; import { TemplateSrvMock } from 'app/features/templating/template_srv.mock'; @@ -46,7 +47,27 @@ jest.mock('app/features/dashboard/services/DashboardSrv', () => ({ })); jest.mock('app/features/templating/template_srv', () => ({ - getTemplateSrv: () => new TemplateSrvMock({}), + ...jest.requireActual('app/features/templating/template_srv'), + getTemplateSrv: () => + new TemplateSrvMock([ + { + name: 'server', + type: 'datasource', + current: { text: 'Server1', value: 'server' }, + options: [{ text: 'Server1', value: 'server1' }], + }, + //multi value variable + { + name: 'multi', + type: 'datasource', + multi: true, + current: { text: 'Server1,Server2', value: ['server-1', 'server-2'] }, + options: [ + { text: 'Server1', value: 'server1' }, + { text: 'Server2', value: 'server2' }, + ], + }, + ] as TypedVariableModel[]), })); interface ScenarioContext { @@ -405,4 +426,53 @@ describe('PanelQueryRunner', () => { snapshotData, } ); + + describeQueryRunnerScenario( + 'shouldAddErrorwhenDatasourceVariableIsMultiple', + (ctx) => { + it('should add error when datasource variable is multiple and not repeated', async () => { + // scopedVars is an object that represent the variables repeated in a panel + const scopedVars = { + server: { text: 'Server1', value: 'server-1' }, + }; + + // We are spying on the replace method of the TemplateSrvMock to check if the custom format function is being called + const spyReplace = jest.spyOn(TemplateSrvMock.prototype, 'replace'); + + const response = { + data: [ + { + target: 'hello', + datapoints: [ + [1, 1000], + [2, 2000], + ], + }, + ], + }; + + const datasource = { + name: '${multi}', + uid: '${multi}', + interval: ctx.dsInterval, + query: (options: grafanaData.DataQueryRequest) => { + ctx.queryCalledWith = options; + return Promise.resolve(response); + }, + getRef: () => ({ type: 'test', uid: 'TestDB-uid' }), + testDatasource: jest.fn(), + } as unknown as DataSourceApi; + + ctx.runner.shouldAddErrorWhenDatasourceVariableIsMultiple(datasource, scopedVars); + + // the test is checking implementation details :(, but it is the only way to check if the error will be added + // if the getTemplateSrv.replace is called with the custom format function,it means we will check + // if the error should be added + expect(spyReplace.mock.calls[0][2]).toBeInstanceOf(Function); + }); + }, + { + ...defaultPanelConfig, + } + ); }); diff --git a/public/app/features/query/state/PanelQueryRunner.ts b/public/app/features/query/state/PanelQueryRunner.ts index 940894c0460..87bb9bf93f5 100644 --- a/public/app/features/query/state/PanelQueryRunner.ts +++ b/public/app/features/query/state/PanelQueryRunner.ts @@ -275,6 +275,9 @@ export class PanelQueryRunner { return; } + //check if datasource is a variable datasource and if that variable has multiple values + const addErroDSVariable = this.shouldAddErrorWhenDatasourceVariableIsMultiple(datasource, scopedVars); + const request: DataQueryRequest = { app: app ?? CoreApp.Dashboard, requestId: getNextRequestId(), @@ -327,7 +330,7 @@ export class PanelQueryRunner { this.lastRequest = request; - this.pipeToSubject(runRequest(ds, request), panelId); + this.pipeToSubject(runRequest(ds, request), panelId, false, addErroDSVariable); } catch (err) { this.pipeToSubject( of({ @@ -341,7 +344,12 @@ export class PanelQueryRunner { } } - private pipeToSubject(observable: Observable, panelId?: number, skipPreProcess = false) { + private pipeToSubject( + observable: Observable, + panelId?: number, + skipPreProcess = false, + addErroDSVariable = false + ) { if (this.subscription) { this.subscription.unsubscribe(); } @@ -379,6 +387,17 @@ export class PanelQueryRunner { this.lastResult = next; + //add error message if datasource is a variable and has multiple values + if (addErroDSVariable) { + next.errors = [ + { + message: + 'Panel is using a variable datasource with multiple values without repeat option. Please configure the panel to be repeated by the same datasource variable.', + }, + ]; + next.state = LoadingState.Error; + } + // Store preprocessed query results for applying overrides later on in the pipeline this.subject.next(next); }, @@ -451,6 +470,28 @@ export class PanelQueryRunner { getLastRequest(): DataQueryRequest | undefined { return this.lastRequest; } + + shouldAddErrorWhenDatasourceVariableIsMultiple( + datasource: DataSourceRef | DataSourceApi | null, + scopedVars: ScopedVars | undefined + ): boolean { + let addWarningMessageMultipleDatasourceVariable = false; + + //If datasource is a variable + if (datasource?.uid?.startsWith('${')) { + // we can access the raw datasource variable values inside the replace function if we pass a custom format function + this.templateSrv.replace(datasource.uid, scopedVars, (value: string | string[]) => { + // if the variable has multiple values it means it's not being repeated + if (Array.isArray(value) && value.length > 1) { + addWarningMessageMultipleDatasourceVariable = true; + } + // return empty string to avoid replacing the variable + return ''; + }); + } + + return addWarningMessageMultipleDatasourceVariable; + } } async function getDataSource( diff --git a/public/app/features/templating/template_srv.mock.ts b/public/app/features/templating/template_srv.mock.ts index 53dad5ead93..2ff7c0abdaf 100644 --- a/public/app/features/templating/template_srv.mock.ts +++ b/public/app/features/templating/template_srv.mock.ts @@ -1,4 +1,4 @@ -import { ScopedVars, TimeRange, TypedVariableModel } from '@grafana/data'; +import { ScopedVars, TimeRange, TypedVariableModel, VariableOption } from '@grafana/data'; import { TemplateSrv } from '@grafana/runtime'; import { variableRegex } from '../variables/utils'; @@ -13,18 +13,37 @@ import { variableRegex } from '../variables/utils'; */ export class TemplateSrvMock implements TemplateSrv { private regex = variableRegex; - constructor(private variables: Record) {} + constructor(private variables: TypedVariableModel[]) {} getVariables(): TypedVariableModel[] { - return Object.keys(this.variables).map((key) => { - return { - type: 'custom', - name: key, - label: key, + if (!this.variables) { + return []; + } + + return this.variables.reduce((acc: TypedVariableModel[], variable) => { + const commonProps = { + type: variable.type ?? 'custom', + name: variable.name ?? 'test', + label: variable.label ?? 'test', }; - // TODO: we remove this type assertion in a later PR - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - }) as TypedVariableModel[]; + if (variable.type === 'datasource') { + acc.push({ + ...commonProps, + current: { + text: variable.current?.text, + value: variable.current?.value, + } as VariableOption, + options: variable.options ?? [], + multi: variable.multi ?? false, + includeAll: variable.includeAll ?? false, + } as TypedVariableModel); + } else { + acc.push({ + ...commonProps, + } as TypedVariableModel); + } + return acc as TypedVariableModel[]; + }, []); } replace(target?: string, scopedVars?: ScopedVars, format?: string | Function): string { @@ -35,8 +54,7 @@ export class TemplateSrvMock implements TemplateSrv { this.regex.lastIndex = 0; return target.replace(this.regex, (match, var1, var2, fmt2, var3, fieldPath, fmt3) => { - const variableName = var1 || var2 || var3; - return this.variables[variableName]; + return var1 || var2 || var3; }); }