From 953eec7326161f6f14581c7b434a41ab550a46fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 3 Sep 2014 08:53:08 +0200 Subject: [PATCH] Fixes and polish for the graphite query editor, #117 --- src/app/controllers/graphiteTarget.js | 12 +++++------ src/app/directives/addGraphiteFunc.js | 1 + src/app/services/graphite/gfunc.js | 24 ++++++++++------------ src/test/specs/gfunc-specs.js | 18 +++++++++------- src/test/specs/graphiteTargetCtrl-specs.js | 4 ++-- 5 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/app/controllers/graphiteTarget.js b/src/app/controllers/graphiteTarget.js index 5671dd0dd04..5d7aa4e0132 100644 --- a/src/app/controllers/graphiteTarget.js +++ b/src/app/controllers/graphiteTarget.js @@ -61,7 +61,7 @@ function (angular, _, config, gfunc, Parser) { switch(astNode.type) { case 'function': - var innerFunc = gfunc.createFuncInstance(astNode.name); + var innerFunc = gfunc.createFuncInstance(astNode.name, { withDefaultParams: false }); _.each(astNode.params, function(param, index) { parseTargeRecursive(param, innerFunc, index); @@ -225,20 +225,20 @@ function (angular, _, config, gfunc, Parser) { }; $scope.addFunction = function(funcDef) { - var newFunc = gfunc.createFuncInstance(funcDef); + var newFunc = gfunc.createFuncInstance(funcDef, { withDefaultParams: true }); newFunc.added = true; $scope.functions.push(newFunc); $scope.moveAliasFuncLast(); $scope.smartlyHandleNewAliasByNode(newFunc); - if (!newFunc.params.length && newFunc.added) { - $scope.targetChanged(); - } - if ($scope.segments.length === 1 && $scope.segments[0].value === 'select metric') { $scope.segments = []; } + + if (!newFunc.params.length && newFunc.added) { + $scope.targetChanged(); + } }; $scope.moveAliasFuncLast = function() { diff --git a/src/app/directives/addGraphiteFunc.js b/src/app/directives/addGraphiteFunc.js index dc0a77a1ae0..e66689969ca 100644 --- a/src/app/directives/addGraphiteFunc.js +++ b/src/app/directives/addGraphiteFunc.js @@ -40,6 +40,7 @@ function (angular, app, _, $, gfunc) { var funcDef = gfunc.getFuncDef(value); if (!funcDef) { // try find close match + value = value.toLowerCase(); funcDef = _.find(allFunctions, function(funcName) { return funcName.toLowerCase().indexOf(value) === 0; }); diff --git a/src/app/services/graphite/gfunc.js b/src/app/services/graphite/gfunc.js index 8c2d218991b..1fc1402cf0c 100644 --- a/src/app/services/graphite/gfunc.js +++ b/src/app/services/graphite/gfunc.js @@ -503,9 +503,14 @@ function (_) { categories[catName] = _.sortBy(funcList, 'name'); }); - function FuncInstance(funcDef) { + function FuncInstance(funcDef, options) { this.def = funcDef; - this.params = funcDef.defaultParams.slice(0); + this.params = []; + + if (options && options.withDefaultParams) { + this.params = funcDef.defaultParams.slice(0); + } + this.updateText(); } @@ -526,7 +531,7 @@ function (_) { parameters.unshift(metricExp); } - return str + parameters.join(',') + ')'; + return str + parameters.join(', ') + ')'; }; FuncInstance.prototype._hasMultipleParamsInString = function(strValue, index) { @@ -567,27 +572,20 @@ function (_) { } var text = this.def.name + '('; - _.each(this.def.params, function(param, index) { - if (param.optional && this.params[index] === undefined) { - return; - } - - text += this.params[index] + ', '; - }, this); - text = text.substring(0, text.length - 2); + text += this.params.join(', '); text += ')'; this.text = text; }; return { - createFuncInstance: function(funcDef) { + createFuncInstance: function(funcDef, options) { if (_.isString(funcDef)) { if (!index[funcDef]) { throw { message: 'Method not found ' + name }; } funcDef = index[funcDef]; } - return new FuncInstance(funcDef); + return new FuncInstance(funcDef, options); }, getFuncDef: function(name) { diff --git a/src/test/specs/gfunc-specs.js b/src/test/specs/gfunc-specs.js index fbedb378747..21f32e56113 100644 --- a/src/test/specs/gfunc-specs.js +++ b/src/test/specs/gfunc-specs.js @@ -9,9 +9,8 @@ define([ var func = gfunc.createFuncInstance('sumSeries'); expect(func).to.be.ok(); expect(func.def.name).to.equal('sumSeries'); - expect(func.def.params.length).to.equal(0); - expect(func.def.defaultParams.length).to.equal(0); - expect(func.def.defaultParams.length).to.equal(0); + expect(func.def.params.length).to.equal(5); + expect(func.def.defaultParams.length).to.equal(1); }); it('should return func instance with shortName', function() { @@ -42,11 +41,16 @@ define([ expect(func.render('hello.metric')).to.equal("sumSeries(hello.metric)"); }); + it('should include default params if options enable it', function() { + var func = gfunc.createFuncInstance('scaleToSeconds', { withDefaultParams: true }); + expect(func.render('hello')).to.equal("scaleToSeconds(hello, 1)"); + }); + it('should handle metric param and int param and string param', function() { var func = gfunc.createFuncInstance('groupByNode'); func.params[0] = 5; func.params[1] = 'avg'; - expect(func.render('hello.metric')).to.equal("groupByNode(hello.metric,5,'avg')"); + expect(func.render('hello.metric')).to.equal("groupByNode(hello.metric, 5, 'avg')"); }); it('should handle function with no metric param', function() { @@ -57,8 +61,8 @@ define([ it('should handle function multiple series params', function() { var func = gfunc.createFuncInstance('asPercent'); - func.params[0] = '$B'; - expect(func.render('$A')).to.equal("asPercent($A,$B)"); + func.params[0] = '#B'; + expect(func.render('#A')).to.equal("asPercent(#A, #B)"); }); }); @@ -72,7 +76,7 @@ define([ describe('when updating func param', function() { it('should update param value and update text representation', function() { - var func = gfunc.createFuncInstance('summarize'); + var func = gfunc.createFuncInstance('summarize', { withDefaultParams: true }); func.updateParam('1h', 0); expect(func.params[0]).to.be('1h'); expect(func.text).to.be('summarize(1h, sum)'); diff --git a/src/test/specs/graphiteTargetCtrl-specs.js b/src/test/specs/graphiteTargetCtrl-specs.js index 978bf3ba4ba..57f3870cb5d 100644 --- a/src/test/specs/graphiteTargetCtrl-specs.js +++ b/src/test/specs/graphiteTargetCtrl-specs.js @@ -56,7 +56,7 @@ define([ }); it('should update target', function() { - expect(ctx.scope.target.target).to.be('aliasByNode(test.prod.*.count,2)'); + expect(ctx.scope.target.target).to.be('aliasByNode(test.prod.*.count, 2)'); }); it('should call get_data', function() { @@ -129,7 +129,7 @@ define([ }); it('should rebuld target after expression model', function() { - expect(ctx.scope.target.target).to.be('aliasByNode(scaleToSeconds(test.prod.*,1),2)'); + expect(ctx.scope.target.target).to.be('aliasByNode(scaleToSeconds(test.prod.*, 1), 2)'); }); it('should call get_data', function() {