Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dashboards: Data source template variable options now specify a current value using uid. #69259

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions devenv/datasources.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ datasources:
prometheusVersion: 2.40.0

- name: gdev-slow-prometheus
uid: gdev-slow-prometheus-uid
type: prometheus
access: proxy
url: http://localhost:3011
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,38 @@
"pluginVersion": "8.4.0-pre",
"title": "Panel Title",
"type": "text"
},
{
"gridPos": {
"h": 9,
"w": 12,
"x": 0,
"y": 9
},
"id": 2,
"options": {
"mode": "markdown",
"content": "VariableUnderTestText: ${VariableUnderTest:text}"
},
"pluginVersion": "8.4.0-pre",
"title": "Panel Title",
"type": "text"
},
{
"gridPos": {
"h": 9,
"w": 12,
"x": 0,
"y": 18
},
"id": 2,
"options": {
"mode": "markdown",
"content": "VariableUnderTestRaw: ${VariableUnderTest:raw}"
},
"pluginVersion": "8.4.0-pre",
"title": "Panel Title",
"type": "text"
}
],
"schemaVersion": 35,
Expand Down
3 changes: 2 additions & 1 deletion e2e/dashboards-suite/new-datasource-variable.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('Variables - Datasource', () => {
e2e.pages.Dashboard.SubMenu.submenuItemValueDropDownOptionTexts('gdev-slow-prometheus').click();

// Assert it was rendered
e2e().get('.markdown-html').should('include.text', 'VariableUnderTest: gdev-slow-prometheus');
e2e().get('.markdown-html').should('include.text', 'VariableUnderTest: gdev-slow-prometheus-uid');
e2e().get('.markdown-html').should('include.text', 'VariableUnderTestText: gdev-slow-prometheus');
});
});
3 changes: 2 additions & 1 deletion public/app/features/plugins/datasource_srv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,8 @@ export class DatasourceSrv implements DataSourceService {
// Support for multi-value variables with only one selected datasource
dsValue = dsValue[0];
}
const dsSettings = !Array.isArray(dsValue) && this.settingsMapByName[dsValue];
const dsSettings =
!Array.isArray(dsValue) && (this.settingsMapByName[dsValue] || this.settingsMapByUid[dsValue]);

if (dsSettings) {
const key = `$\{${variable.name}\}`;
Expand Down
43 changes: 39 additions & 4 deletions public/app/features/plugins/tests/datasource_srv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ const templateSrv: any = {
value: 'BBB',
},
},
{
type: 'datasource',
name: 'datasourceByUid',
current: {
value: 'uid-code-DDDD',
},
},
{
type: 'datasource',
name: 'datasourceDefault',
Expand All @@ -32,6 +39,7 @@ const templateSrv: any = {
}

let result = v.replace('${datasource}', 'BBB');
result = result.replace('${datasourceByUid}', 'DDDD');
result = result.replace('${datasourceDefault}', 'default');
return result;
},
Expand Down Expand Up @@ -103,6 +111,12 @@ describe('datasource_srv', () => {
meta: { metrics: true },
isDefault: true,
},
DDDD: {
type: 'test-db',
name: 'DDDD',
uid: 'uid-code-DDDD',
meta: { metrics: true },
},
Jaeger: {
type: 'jaeger-db',
name: 'Jaeger',
Expand Down Expand Up @@ -165,7 +179,7 @@ describe('datasource_srv', () => {
expect(dataSourceSrv.getInstanceSettings({ uid: 'uid-code-mmm' })).toBe(ds);
});

it('should work with variable', () => {
it('should work with variable by ds name', () => {
const ds = dataSourceSrv.getInstanceSettings('${datasource}');
expect(ds?.name).toBe('${datasource}');
expect(ds?.uid).toBe('${datasource}');
Expand All @@ -177,6 +191,18 @@ describe('datasource_srv', () => {
`);
});

it('should work with variable by ds value (uid)', () => {
const ds = dataSourceSrv.getInstanceSettings('${datasourceByUid}');
expect(ds?.name).toBe('${datasourceByUid}');
expect(ds?.uid).toBe('${datasourceByUid}');
expect(ds?.rawRef).toMatchInlineSnapshot(`
{
"type": "test-db",
"uid": "uid-code-DDDD",
}
`);
});

it('should work with variable via scopedVars', () => {
const ds = dataSourceSrv.getInstanceSettings('${datasource}', {
datasource: { text: 'Prom', value: 'uid-code-aaa' },
Expand Down Expand Up @@ -247,7 +273,7 @@ describe('datasource_srv', () => {
describe('when getting external metric sources', () => {
it('should return list of explore sources', () => {
const externalSources = dataSourceSrv.getExternal();
expect(externalSources.length).toBe(6);
expect(externalSources.length).toBe(7);
});
});

Expand All @@ -260,8 +286,9 @@ describe('datasource_srv', () => {

it('Can get list of data sources with variables: true', () => {
const list = dataSourceSrv.getList({ metrics: true, variables: true });
expect(list[0].name).toBe('${datasourceDefault}');
expect(list[1].name).toBe('${datasource}');
expect(list[0].name).toBe('${datasourceByUid}');
expect(list[1].name).toBe('${datasourceDefault}');
expect(list[2].name).toBe('${datasource}');
});

it('Can get list of data sources with tracing: true', () => {
Expand Down Expand Up @@ -300,6 +327,14 @@ describe('datasource_srv', () => {
"type": "test-db",
"uid": "uid-code-BBB",
},
{
"meta": {
"metrics": true,
},
"name": "DDDD",
"type": "test-db",
"uid": "uid-code-DDDD",
},
{
"meta": {
"annotations": true,
Expand Down
2 changes: 1 addition & 1 deletion public/app/features/variables/datasource/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const dataSourceVariableSlice = createSlice({
}

if (isValid(source, regex)) {
options.push({ text: source.name, value: source.name, selected: false });
options.push({ text: source.name, value: source.uid, selected: false });
}

if (isDefault(source, regex)) {
Expand Down
2 changes: 1 addition & 1 deletion public/app/features/variables/shared/testing/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { DataSourceInstanceSettings, DataSourceJsonData, DataSourcePluginMeta }
export function getDataSourceInstanceSetting(name: string, meta: DataSourcePluginMeta): DataSourceInstanceSettings {
return {
id: 1,
uid: '',
uid: name,
type: '',
name,
meta,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@ import { VariableOption, VariableWithOptions } from 'app/features/variables/type
import { VariableBuilder } from './variableBuilder';

export class OptionsVariableBuilder<T extends VariableWithOptions> extends VariableBuilder<T> {
withOptions(...texts: string[]) {
withOptions(...options: Array<string | { text: string; value: string }>) {
this.variable.options = [];
for (let index = 0; index < texts.length; index++) {
this.variable.options.push({
text: texts[index],
value: texts[index],
selected: false,
});
for (let index = 0; index < options.length; index++) {
const option = options[index];

if (typeof option === 'string') {
this.variable.options.push({
text: option,
value: option,
selected: false,
});
} else {
this.variable.options.push({ ...option, selected: false });
}
}
return this;
}
Expand Down
68 changes: 68 additions & 0 deletions public/app/features/variables/state/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,74 @@ describe('shared actions', () => {
});
});

describe('and not multivalue, but with currentValue specified', () => {
const A = { text: 'A', value: 'a-uid' };
const B = { text: 'B', value: 'b-uid' };
const C = { text: 'C', value: 'c-uid' };

it.each`
withOptions | currentText | currentValue | defaultValue | expected
${[A, B, C]} | ${undefined} | ${undefined} | ${undefined} | ${A}
${[A, B, C]} | ${'B'} | ${'b-uid'} | ${undefined} | ${B}
${[A, B, C]} | ${'B'} | ${undefined} | ${undefined} | ${B}
${[A, B, C]} | ${undefined} | ${'b-uid'} | ${undefined} | ${B}
${[A, B, C]} | ${'Old B'} | ${'b-uid'} | ${undefined} | ${B}
${[A, B, C]} | ${undefined} | ${'x-uid'} | ${'b-uid'} | ${B}
${[A, B, C]} | ${undefined} | ${'b-uid'} | ${'c-uid'} | ${B}
${[A, B, C]} | ${undefined} | ${'x-uid'} | ${undefined} | ${A}
${undefined} | ${undefined} | ${'b-uid'} | ${undefined} | ${'should not dispatch setCurrentVariableValue'}
`(
'then correct actions are dispatched',
async ({ withOptions, currentText, currentValue, defaultValue, expected }) => {
let custom;
const key = 'key';
if (!withOptions) {
custom = customBuilder()
.withId('0')
.withRootStateKey(key)
.withCurrent(currentText, currentValue)
.withoutOptions()
.build();
} else {
custom = customBuilder()
.withId('0')
.withRootStateKey(key)
.withOptions(...withOptions)
.withCurrent(currentText, currentValue)
.build();
}

const tester = await reduxTester<TemplatingReducerType>()
.givenRootReducer(getTemplatingRootReducer())
.whenActionIsDispatched(
toKeyedAction(key, addVariable(toVariablePayload(custom, { global: false, index: 0, model: custom })))
)
.whenAsyncActionIsDispatched(
validateVariableSelectionState(toKeyedVariableIdentifier(custom), defaultValue),
true
);

await tester.thenDispatchedActionsPredicateShouldEqual((dispatchedActions) => {
const expectedActions: AnyAction[] = withOptions
? [
toKeyedAction(
key,
setCurrentVariableValue(
toVariablePayload(
{ type: 'custom', id: '0' },
{ option: { text: expected.text, value: expected.value, selected: false } }
)
)
),
]
: [];
expect(dispatchedActions).toEqual(expectedActions);
return true;
});
}
);
});

describe('and multivalue', () => {
it.each`
withOptions | withCurrent | defaultValue | expectedText | expectedSelected
Expand Down
7 changes: 5 additions & 2 deletions public/app/features/variables/state/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
ensureStringValues,
ExtendedUrlQueryMap,
getCurrentText,
getCurrentValue,
getVariableRefresh,
hasOngoingTransaction,
toKeyedVariableIdentifier,
Expand Down Expand Up @@ -522,14 +523,16 @@ export const validateVariableSelectionState = (

// 1. find the current value
const text = getCurrentText(variableInState);
option = variableInState.options?.find((v) => v.text === text);
const value = getCurrentValue(variableInState);

option = variableInState.options?.find((v: VariableOption) => v.text === text || v.value === value);
if (option) {
return setValue(variableInState, option);
}

// 2. find the default value
if (defaultValue) {
option = variableInState.options?.find((v) => v.text === defaultValue);
option = variableInState.options?.find((v) => v.text === defaultValue || v.value === defaultValue);
if (option) {
return setValue(variableInState, option);
}
Expand Down
Loading