From 5ce3e40cc934b5a2060d788676e8084efa3693a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Sat, 17 Sep 2016 15:27:53 +0200 Subject: [PATCH] feat(templating): more work on new variable handling code, #6048 --- public/app/core/utils/kbn.js | 2 +- public/app/features/templating/all.ts | 2 + .../features/templating/constant_variable.ts | 34 +++++++ .../specs/variabe_srv_init_specs.ts | 88 +++++++++++++++++++ .../specs/variable_srv_init_specs.ts | 29 ++++++ public/app/features/templating/variable.ts | 13 +-- .../app/features/templating/variable_srv.ts | 29 +++--- public/test/specs/templateValuesSrv-specs.js | 31 ------- 8 files changed, 177 insertions(+), 51 deletions(-) create mode 100644 public/app/features/templating/constant_variable.ts create mode 100644 public/app/features/templating/specs/variabe_srv_init_specs.ts diff --git a/public/app/core/utils/kbn.js b/public/app/core/utils/kbn.js index 4b7281c55e2..cf80d671d71 100644 --- a/public/app/core/utils/kbn.js +++ b/public/app/core/utils/kbn.js @@ -10,7 +10,7 @@ function($, _, moment) { kbn.valueFormats = {}; kbn.regexEscape = function(value) { - return value.replace(/[\\^$*+?.()|[\]{}\/]/g, '\\$&') + return value.replace(/[\\^$*+?.()|[\]{}\/]/g, '\\$&'); }; ///// HELPER FUNCTIONS ///// diff --git a/public/app/features/templating/all.ts b/public/app/features/templating/all.ts index d72a76f5a65..c6e8dfb82ea 100644 --- a/public/app/features/templating/all.ts +++ b/public/app/features/templating/all.ts @@ -7,6 +7,7 @@ import {IntervalVariable} from './interval_variable'; import {QueryVariable} from './query_variable'; import {DatasourceVariable} from './datasource_variable'; import {CustomVariable} from './custom_variable'; +import {ConstantVariable} from './constant_variable'; export { VariableSrv, @@ -14,4 +15,5 @@ export { QueryVariable, DatasourceVariable, CustomVariable, + ConstantVariable, } diff --git a/public/app/features/templating/constant_variable.ts b/public/app/features/templating/constant_variable.ts new file mode 100644 index 00000000000..2fb32b336f7 --- /dev/null +++ b/public/app/features/templating/constant_variable.ts @@ -0,0 +1,34 @@ +/// + +import _ from 'lodash'; +import {Variable} from './variable'; +import {VariableSrv, variableConstructorMap} from './variable_srv'; + +export class ConstantVariable implements Variable { + query: string; + options: any[]; + + /** @ngInject */ + constructor(private model, private variableSrv) { + _.extend(this, model); + } + + setValue(option) { + this.variableSrv.setOptionAsCurrent(this, option); + } + + updateOptions() { + this.options = [{text: this.query.trim(), value: this.query.trim()}]; + this.setValue(this.options[0]); + } + + dependsOn(variable) { + return false; + } + + setValueFromUrl(urlValue) { + return this.variableSrv.setOptionFromUrl(this, urlValue); + } +} + +variableConstructorMap['constant'] = ConstantVariable; diff --git a/public/app/features/templating/specs/variabe_srv_init_specs.ts b/public/app/features/templating/specs/variabe_srv_init_specs.ts new file mode 100644 index 00000000000..080b1706736 --- /dev/null +++ b/public/app/features/templating/specs/variabe_srv_init_specs.ts @@ -0,0 +1,88 @@ +import {describe, beforeEach, it, sinon, expect, angularMocks} from 'test/lib/common'; + +import moment from 'moment'; +import helpers from 'test/specs/helpers'; +import '../all'; + +describe('VariableSrv Init', function() { + var ctx = new helpers.ControllerTestContext(); + + beforeEach(angularMocks.module('grafana.core')); + beforeEach(angularMocks.module('grafana.controllers')); + beforeEach(angularMocks.module('grafana.services')); + + beforeEach(ctx.providePhase(['datasourceSrv', 'timeSrv', 'templateSrv', '$location'])); + beforeEach(angularMocks.inject(($rootScope, $q, $location, $injector) => { + ctx.$q = $q; + ctx.$rootScope = $rootScope; + ctx.$location = $location; + ctx.variableSrv = $injector.get('variableSrv'); + ctx.variableSrv.init({templating: {list: []}}); + ctx.$rootScope.$digest(); + })); + + function describeInitSceneario(desc, fn) { + describe(desc, function() { + var scenario: any = { + urlParams: {}, + setup: setupFn => { + scenario.setupFn = setupFn; + } + }; + + beforeEach(function() { + scenario.setupFn(); + var ds: any = {}; + ds.metricFindQuery = sinon.stub().returns(ctx.$q.when(scenario.queryResult)); + ctx.datasourceSrv.get = sinon.stub().returns(ctx.$q.when(ds)); + ctx.datasourceSrv.getMetricSources = sinon.stub().returns(scenario.metricSources); + + ctx.$location.search = sinon.stub().returns(scenario.urlParams); + + ctx.dashboard = {templating: {list: scenario.variables}}; + ctx.variableSrv.init(ctx.dashboard); + ctx.$rootScope.$digest(); + + scenario.variables = ctx.variableSrv.variables; + }); + + fn(scenario); + }); + } + + describeInitSceneario('when setting query variable via url', scenario => { + scenario.setup(() => { + scenario.variables = [{ + name: 'apps', + type: 'query', + current: {text: "test", value: "test"}, + options: [{text: "test", value: "test"}] + }]; + scenario.urlParams["var-apps"] = "new"; + }); + + it('should update current value', () => { + expect(scenario.variables[0].current.value).to.be("new"); + expect(scenario.variables[0].current.text).to.be("new"); + }); + }); + + describeInitSceneario('when setting custom variable via url', scenario => { + scenario.setup(() => { + scenario.variables = [{ + name: 'apps', + type: 'custom', + current: {text: "test", value: "test"}, + options: [{text: "test", value: "test"}] + }]; + scenario.urlParams["var-apps"] = "new"; + }); + + it('should update current value', () => { + expect(scenario.variables[0].current.value).to.be("new"); + expect(scenario.variables[0].current.text).to.be("new"); + }); + }); + +}); + diff --git a/public/app/features/templating/specs/variable_srv_init_specs.ts b/public/app/features/templating/specs/variable_srv_init_specs.ts index 59c82ed26c5..2703b70bce3 100644 --- a/public/app/features/templating/specs/variable_srv_init_specs.ts +++ b/public/app/features/templating/specs/variable_srv_init_specs.ts @@ -109,5 +109,34 @@ describe('VariableSrv init', function() { }); }); + describeInitScenario('when template variable is present in url multiple times', scenario => { + scenario.setup(() => { + scenario.variables = [{ + name: 'apps', + type: 'query', + multi: true, + current: {text: "val1", value: "val1"}, + options: [{text: "val1", value: "val1"}, {text: 'val2', value: 'val2'}, {text: 'val3', value: 'val3', selected: true}] + }]; + scenario.urlParams["var-apps"] = ["val2", "val1"]; + }); + + it('should update current value', function() { + var variable = ctx.variableSrv.variables[0]; + expect(variable.current.value.length).to.be(2); + expect(variable.current.value[0]).to.be("val2"); + expect(variable.current.value[1]).to.be("val1"); + expect(variable.current.text).to.be("val2 + val1"); + expect(variable.options[0].selected).to.be(true); + expect(variable.options[1].selected).to.be(true); + }); + + it('should set options that are not in value to selected false', function() { + var variable = ctx.variableSrv.variables[0]; + expect(variable.options[2].selected).to.be(false); + }); + }); + + }); diff --git a/public/app/features/templating/variable.ts b/public/app/features/templating/variable.ts index da9c280aa33..c7938f3ebb5 100644 --- a/public/app/features/templating/variable.ts +++ b/public/app/features/templating/variable.ts @@ -3,6 +3,13 @@ import _ from 'lodash'; import kbn from 'app/core/utils/kbn'; +export interface Variable { + setValue(option); + updateOptions(); + dependsOn(variable); + setValueFromUrl(urlValue); +} + export function containsVariable(...args: any[]) { var variableName = args[args.length-1]; var str = args[0] || ''; @@ -17,12 +24,6 @@ export function containsVariable(...args: any[]) { return match !== null; } -export interface Variable { - setValue(option); - updateOptions(); - dependsOn(variable); - setValueFromUrl(urlValue); -} diff --git a/public/app/features/templating/variable_srv.ts b/public/app/features/templating/variable_srv.ts index a1409e4cc2e..dfc938543a8 100644 --- a/public/app/features/templating/variable_srv.ts +++ b/public/app/features/templating/variable_srv.ts @@ -29,9 +29,7 @@ export class VariableSrv { init(dashboard) { this.variableLock = {}; this.dashboard = dashboard; - this.variables = []; - - dashboard.templating.list.map(this.addVariable.bind(this)); + this.variables = dashboard.templating.list.map(this.createVariableFromModel.bind(this)); this.templateSrv.init(this.variables); var queryParams = this.$location.search(); @@ -60,13 +58,9 @@ export class VariableSrv { if (urlValue !== void 0) { return variable.setValueFromUrl(urlValue).then(lock.resolve); } + if (variable.refresh === 1 || variable.refresh === 2) { - return variable.updateOptions().then(() => { - // if (_.isEmpty(variable.current) && variable.options.length) { - // self.setVariableValue(variable, variable.options[0]); - // } - lock.resolve(); - }); + return variable.updateOptions().then(lock.resolve); } lock.resolve(); @@ -75,19 +69,28 @@ export class VariableSrv { }); } - addVariable(model) { + createVariableFromModel(model) { var ctor = variableConstructorMap[model.type]; if (!ctor) { throw "Unable to find variable constructor for " + model.type; } var variable = this.$injector.instantiate(ctor, {model: model}); - this.variables.push(variable); - this.dashboard.templating.list.push(model); - return variable; } + addVariable(model) { + var variable = this.createVariableFromModel(model); + this.variables.push(this.createVariableFromModel(variable)); + return variable; + } + + syncToDashboardModel() { + this.dashboard.templating.list = this.variables.map(variable => { + return variable.model; + }); + } + updateOptions(variable) { return variable.updateOptions(); } diff --git a/public/test/specs/templateValuesSrv-specs.js b/public/test/specs/templateValuesSrv-specs.js index 1807157ae35..f1d5375361e 100644 --- a/public/test/specs/templateValuesSrv-specs.js +++ b/public/test/specs/templateValuesSrv-specs.js @@ -53,37 +53,6 @@ define([ // }); }); - describe('when template variable is present in url multiple times', function() { - var variable = { - name: 'apps', - multi: true, - current: {text: "val1", value: "val1"}, - options: [{text: "val1", value: "val1"}, {text: 'val2', value: 'val2'}, {text: 'val3', value: 'val3', selected: true}] - }; - - beforeEach(function(done) { - var dashboard = { templating: { list: [variable] } }; - var urlParams = {}; - urlParams["var-apps"] = ["val2", "val1"]; - ctx.$location.search = sinon.stub().returns(urlParams); - ctx.service.init(dashboard).then(function() { done(); }); - ctx.$rootScope.$digest(); - }); - - it('should update current value', function() { - expect(variable.current.value.length).to.be(2); - expect(variable.current.value[0]).to.be("val2"); - expect(variable.current.value[1]).to.be("val1"); - expect(variable.current.text).to.be("val2 + val1"); - expect(variable.options[0].selected).to.be(true); - expect(variable.options[1].selected).to.be(true); - }); - - it('should set options that are not in value to selected false', function() { - expect(variable.options[2].selected).to.be(false); - }); - }); - }); });