From 8a796c5b4b8149c6bfc69b0c8765656418839d56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 19 Sep 2016 15:15:15 +0200 Subject: [PATCH] feat(templating): progress on variable system refactoring, #6048 --- .../features/templating/constant_variable.ts | 17 +- public/app/features/templating/editorCtrl.js | 4 +- .../app/features/templating/query_variable.ts | 31 +++- .../templating/specs/query_variable_specs.ts | 39 +++++ .../templating/specs/variable_specs.ts | 19 +- public/app/features/templating/variable.ts | 8 + .../app/features/templating/variable_srv.ts | 164 +++++++++--------- 7 files changed, 192 insertions(+), 90 deletions(-) create mode 100644 public/app/features/templating/specs/query_variable_specs.ts diff --git a/public/app/features/templating/constant_variable.ts b/public/app/features/templating/constant_variable.ts index 2fb32b336f7..ea66b4a0af2 100644 --- a/public/app/features/templating/constant_variable.ts +++ b/public/app/features/templating/constant_variable.ts @@ -1,16 +1,29 @@ /// import _ from 'lodash'; -import {Variable} from './variable'; +import {Variable, assignModelProperties} from './variable'; import {VariableSrv, variableConstructorMap} from './variable_srv'; export class ConstantVariable implements Variable { query: string; options: any[]; + defaults = { + type: 'constant', + name: '', + query: '', + hide: 2, + refresh: 0, + }; + /** @ngInject */ constructor(private model, private variableSrv) { - _.extend(this, model); + assignModelProperties(this, model, this.defaults); + } + + getModel() { + assignModelProperties(this.model, this, this.defaults); + return this.model; } setValue(option) { diff --git a/public/app/features/templating/editorCtrl.js b/public/app/features/templating/editorCtrl.js index 7d60295168f..5383eb2f178 100644 --- a/public/app/features/templating/editorCtrl.js +++ b/public/app/features/templating/editorCtrl.js @@ -13,7 +13,7 @@ function (angular, _) { type: 'query', datasource: null, refresh: 0, - sort: 1, + sort: 0, name: '', hide: 0, options: [], @@ -37,7 +37,7 @@ function (angular, _) { ]; $scope.sortOptions = [ - {value: 0, text: "Without Sort"}, + {value: 0, text: "Query sort"}, {value: 1, text: "Alphabetical (asc)"}, {value: 2, text: "Alphabetical (desc)"}, {value: 3, text: "Numerical (asc)"}, diff --git a/public/app/features/templating/query_variable.ts b/public/app/features/templating/query_variable.ts index 6ba95b05e00..2816d0a47ed 100644 --- a/public/app/features/templating/query_variable.ts +++ b/public/app/features/templating/query_variable.ts @@ -2,7 +2,7 @@ import _ from 'lodash'; import kbn from 'app/core/utils/kbn'; -import {Variable, containsVariable} from './variable'; +import {Variable, containsVariable, assignModelProperties} from './variable'; import {VariableSrv, variableConstructorMap} from './variable_srv'; function getNoneOption() { @@ -16,11 +16,36 @@ export class QueryVariable implements Variable { sort: any; options: any; current: any; - includeAll: boolean; refresh: number; + hide: number; + name: string; + multi: boolean; + includeAll: boolean; + + defaults = { + type: 'query', + query: '', + regex: '', + sort: 1, + datasource: null, + refresh: 0, + hide: 0, + name: '', + multi: false, + includeAll: false, + options: [], + current: {text: '', value: ''}, + }; constructor(private model, private datasourceSrv, private templateSrv, private variableSrv, private $q) { - _.extend(this, model); + // copy model properties to this instance + assignModelProperties(this, model, this.defaults); + } + + getModel() { + // copy back model properties to model + assignModelProperties(this.model, this, this.defaults); + return this.model; } setValue(option){ diff --git a/public/app/features/templating/specs/query_variable_specs.ts b/public/app/features/templating/specs/query_variable_specs.ts new file mode 100644 index 00000000000..4040056bfe4 --- /dev/null +++ b/public/app/features/templating/specs/query_variable_specs.ts @@ -0,0 +1,39 @@ +import {describe, beforeEach, it, sinon, expect, angularMocks} from 'test/lib/common'; + +import {QueryVariable} from '../query_variable'; + +describe('QueryVariable', function() { + + describe('when creating from model', function() { + + it('should set defaults', function() { + var variable = new QueryVariable({}, null, null, null, null); + expect(variable.datasource).to.be(null); + expect(variable.refresh).to.be(0); + expect(variable.sort).to.be(1); + expect(variable.name).to.be(''); + expect(variable.hide).to.be(0); + expect(variable.options.length).to.be(0); + expect(variable.multi).to.be(false); + expect(variable.includeAll).to.be(false); + }); + + it('get model should copy changes back to model', () => { + var variable = new QueryVariable({}, null, null, null, null); + variable.options = [{text: 'test'}]; + variable.datasource = 'google'; + variable.regex = 'asd'; + variable.sort = 50; + + var model = variable.getModel(); + expect(model.options.length).to.be(1); + expect(model.options[0].text).to.be('test'); + expect(model.datasource).to.be('google'); + expect(model.regex).to.be('asd'); + expect(model.sort).to.be(50); + }); + + }); + +}); + diff --git a/public/app/features/templating/specs/variable_specs.ts b/public/app/features/templating/specs/variable_specs.ts index a11f6272425..9a974eae695 100644 --- a/public/app/features/templating/specs/variable_specs.ts +++ b/public/app/features/templating/specs/variable_specs.ts @@ -1,6 +1,6 @@ import {describe, beforeEach, it, sinon, expect, angularMocks} from 'test/lib/common'; -import {containsVariable} from '../variable'; +import {containsVariable, assignModelProperties} from '../variable'; describe('containsVariable', function() { @@ -40,3 +40,20 @@ describe('containsVariable', function() { }); +describe('assignModelProperties', function() { + + it('only set properties defined in defaults', function() { + var target: any = {test: 'asd'}; + assignModelProperties(target, {propA: 1, propB: 2}, {propB: 0}); + expect(target.propB).to.be(2); + expect(target.test).to.be('asd'); + }); + + it('use default value if not found on source', function() { + var target: any = {test: 'asd'}; + assignModelProperties(target, {propA: 1, propB: 2}, {propC: 10}); + expect(target.propC).to.be(10); + }); + +}); + diff --git a/public/app/features/templating/variable.ts b/public/app/features/templating/variable.ts index c7938f3ebb5..b9441b55840 100644 --- a/public/app/features/templating/variable.ts +++ b/public/app/features/templating/variable.ts @@ -8,6 +8,14 @@ export interface Variable { updateOptions(); dependsOn(variable); setValueFromUrl(urlValue); + getModel(); +} + + +export function assignModelProperties(target, source, defaults) { + _.forEach(defaults, function(value, key) { + target[key] = source[key] === undefined ? value : source[key]; + }); } export function containsVariable(...args: any[]) { diff --git a/public/app/features/templating/variable_srv.ts b/public/app/features/templating/variable_srv.ts index 767765296e5..e093514791d 100644 --- a/public/app/features/templating/variable_srv.ts +++ b/public/app/features/templating/variable_srv.ts @@ -106,7 +106,7 @@ export class VariableSrv { syncToDashboardModel() { this.dashboard.templating.list = this.variables.map(variable => { - return variable.model; + return variable.getModel(); }); } @@ -122,100 +122,100 @@ export class VariableSrv { // cascade updates to variables that use this variable var promises = _.map(this.variables, otherVariable => { - if (otherVariable === variable) { - return; - } - - if (otherVariable.dependsOn(variable)) { - return this.updateOptions(otherVariable); - } - }); - - return this.$q.all(promises); - } - - selectOptionsForCurrentValue(variable) { - var i, y, value, option; - var selected: any = []; - - for (i = 0; i < variable.options.length; i++) { - option = variable.options[i]; - option.selected = false; - if (_.isArray(variable.current.value)) { - for (y = 0; y < variable.current.value.length; y++) { - value = variable.current.value[y]; - if (option.value === value) { - option.selected = true; - selected.push(option); - } - } - } else if (option.value === variable.current.value) { - option.selected = true; - selected.push(option); - } + if (otherVariable === variable) { + return; } - return selected; + if (otherVariable.dependsOn(variable)) { + return this.updateOptions(otherVariable); + } + }); + + return this.$q.all(promises); + } + + selectOptionsForCurrentValue(variable) { + var i, y, value, option; + var selected: any = []; + + for (i = 0; i < variable.options.length; i++) { + option = variable.options[i]; + option.selected = false; + if (_.isArray(variable.current.value)) { + for (y = 0; y < variable.current.value.length; y++) { + value = variable.current.value[y]; + if (option.value === value) { + option.selected = true; + selected.push(option); + } + } + } else if (option.value === variable.current.value) { + option.selected = true; + selected.push(option); + } } - validateVariableSelectionState(variable) { - if (!variable.current) { - if (!variable.options.length) { return this.$q.when(); } + return selected; + } + + validateVariableSelectionState(variable) { + if (!variable.current) { + if (!variable.options.length) { return this.$q.when(); } + return variable.setValue(variable.options[0]); + } + + if (_.isArray(variable.current.value)) { + var selected = this.selectOptionsForCurrentValue(variable); + + // if none pick first + if (selected.length === 0) { + selected = variable.options[0]; + } else { + selected = { + value: _.map(selected, function(val) {return val.value;}), + text: _.map(selected, function(val) {return val.text;}).join(' + '), + }; + } + + return variable.setValue(selected); + } else { + var currentOption = _.find(variable.options, {text: variable.current.text}); + if (currentOption) { + return variable.setValue(currentOption); + } else { + if (!variable.options.length) { return Promise.resolve(); } return variable.setValue(variable.options[0]); } + } + } - if (_.isArray(variable.current.value)) { - var selected = this.selectOptionsForCurrentValue(variable); + setOptionFromUrl(variable, urlValue) { + var promise = this.$q.when(); - // if none pick first - if (selected.length === 0) { - selected = variable.options[0]; - } else { - selected = { - value: _.map(selected, function(val) {return val.value;}), - text: _.map(selected, function(val) {return val.text;}).join(' + '), - }; - } - - return variable.setValue(selected); - } else { - var currentOption = _.find(variable.options, {text: variable.current.text}); - if (currentOption) { - return variable.setValue(currentOption); - } else { - if (!variable.options.length) { return Promise.resolve(); } - return variable.setValue(variable.options[0]); - } - } + if (variable.refresh) { + promise = variable.updateOptions(); } - setOptionFromUrl(variable, urlValue) { - var promise = this.$q.when(); - - if (variable.refresh) { - promise = variable.updateOptions(); - } - - return promise.then(() => { - var option = _.find(variable.options, op => { - return op.text === urlValue || op.value === urlValue; - }); - - option = option || {text: urlValue, value: urlValue}; - return variable.setValue(option); + return promise.then(() => { + var option = _.find(variable.options, op => { + return op.text === urlValue || op.value === urlValue; }); + + option = option || {text: urlValue, value: urlValue}; + return variable.setValue(option); + }); + } + + setOptionAsCurrent(variable, option) { + variable.current = _.cloneDeep(option); + + if (_.isArray(variable.current.text)) { + variable.current.text = variable.current.text.join(' + '); } - setOptionAsCurrent(variable, option) { - variable.current = _.cloneDeep(option); - - if (_.isArray(variable.current.text)) { - variable.current.text = variable.current.text.join(' + '); - } - - this.selectOptionsForCurrentValue(variable); - return this.variableUpdated(variable); - } + this.selectOptionsForCurrentValue(variable); + return this.variableUpdated(variable); + } } coreModule.service('variableSrv', VariableSrv);