feat(thresholds): refactoring and adding many unit tests

CHANGELOG.md

@@ -7,7 +7,7 @@
 * **Templating**: Update panel repeats for variables that change on time refresh, closes [#5021](https://github.com/grafana/grafana/issues/5021)
 * **Elasticsearch**: Support to set Precision Threshold for Unique Count metric, closes [#4689](https://github.com/grafana/grafana/issues/4689)
 
-#b 3.1.1 (unreleased / v3.1.x branch)
+# 3.1.1 (unreleased / v3.1.x branch)
 * **IFrame embedding**: Fixed issue of using full iframe height, fixes [#5605](https://github.com/grafana/grafana/issues/5606)
 * **Panel PNG rendering**: Fixed issue detecting render completion, fixes [#5605](https://github.com/grafana/grafana/issues/5606)
 * **Elasticsearch**: Fixed issue with templating query and json parse error, fixes [#5615](https://github.com/grafana/grafana/issues/5615)

public/app/features/alerting/alert_tab_ctrl.ts

@@ -129,7 +129,8 @@ export class AlertTabCtrl {
   graphThresholdChanged(evt) {
     for (var condition of this.alert.conditions) {
       if (condition.type === 'query') {
-        condition.evaluator.params[0] = evt.threshold.value;
+        condition.evaluator.params[evt.handleIndex] = evt.threshold.value;
+        this.evaluatorParamsChanged();
         break;
       }
     }
@@ -140,7 +141,7 @@ export class AlertTabCtrl {
       type: 'query',
       query: {params: ['A', '5m', 'now']},
       reducer: {type: 'avg', params: []},
-      evaluator: {type: '>', params: [null]},
+      evaluator: {type: 'gt', params: [null]},
     };
   }
 
@@ -171,7 +172,7 @@ export class AlertTabCtrl {
   }
 
   delete() {
-    this.alert.enabled = false;
+    this.panel.alert = {enabled: false};
     this.initModel();
   }
 

public/app/plugins/panel/graph/graph.js

@@ -5,7 +5,7 @@ define([
   'lodash',
   'app/core/utils/kbn',
   './graph_tooltip',
-  './thresholds',
+  './threshold_manager',
   'jquery.flot',
   'jquery.flot.selection',
   'jquery.flot.time',
@@ -15,15 +15,12 @@ define([
   'jquery.flot.crosshair',
   './jquery.flot.events',
 ],
-function (angular, $, moment, _, kbn, GraphTooltip, thresholds) {
+function (angular, $, moment, _, kbn, GraphTooltip, thresholdManExports) {
   'use strict';
 
   var module = angular.module('grafana.directives');
   var labelWidthCache = {};
 
-  // systemjs export
-  var ThresholdControls = thresholds.ThresholdControls;
-
   module.directive('grafanaGraph', function($rootScope, timeSrv) {
     return {
       restrict: 'A',
@@ -37,7 +34,7 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) {
         var legendSideLastValue = null;
         var rootScope = scope.$root;
         var panelWidth = 0;
-        var thresholdControls = new ThresholdControls(ctrl);
+        var thresholdManager = new thresholdManExports.ThresholdManager(ctrl);
 
         rootScope.onAppEvent('setCrosshair', function(event, info) {
           // do not need to to this if event is from this panel
@@ -161,7 +158,7 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) {
             rightLabel[0].style.marginTop = (getLabelWidth(panel.yaxes[1].label, rightLabel) / 2) + 'px';
           }
 
-          thresholdControls.draw(plot);
+          thresholdManager.draw(plot);
         }
 
         function processOffsetHook(plot, gridMargin) {
@@ -180,7 +177,7 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) {
           }
 
           // give space to alert editing
-          thresholdControls.prepare(elem);
+          thresholdManager.prepare(elem);
 
           var stack = panel.stack ? true : null;
 
@@ -252,7 +249,7 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) {
           }
 
           addTimeAxis(options);
-          addGridThresholds(options, panel);
+          thresholdManager.addPlotOptions(options, panel);
           addAnnotations(options);
           configureAxisOptions(data, options);
 
@@ -315,82 +312,6 @@ function (angular, $, moment, _, kbn, GraphTooltip, thresholds) {
           };
         }
 
-        function addGridThresholds(options, panel) {
-          if (!panel.thresholds || panel.thresholds.length === 0) {
-            return;
-          }
-
-          var gtLimit = Infinity;
-          var ltLimit = -Infinity;
-          var i, threshold, other;
-
-          for (i = 0; i < panel.thresholds.length; i++) {
-            threshold = panel.thresholds[i];
-            if (!_.isNumber(threshold.value)) {
-              continue;
-            }
-
-            var limit;
-            switch(threshold.op) {
-              case 'gt': {
-                limit = gtLimit;
-                // if next threshold is less then op and greater value, then use that as limit
-                if (panel.thresholds.length > i+1) {
-                  other = panel.thresholds[i+1];
-                  if (other.value > threshold.value) {
-                    limit = other.value;
-                    ltLimit = limit;
-                  }
-                }
-                break;
-              }
-              case 'lt': {
-                limit = ltLimit;
-                // if next threshold is less then op and greater value, then use that as limit
-                if (panel.thresholds.length > i+1) {
-                  other = panel.thresholds[i+1];
-                  if (other.value < threshold.value) {
-                    limit = other.value;
-                    gtLimit = limit;
-                  }
-                }
-                break;
-              }
-            }
-
-            switch(threshold.colorMode) {
-              case 'critical': {
-                threshold.fillColor = 'rgba(234, 112, 112, 0.12)';
-                threshold.lineColor = 'rgba(237, 46, 24, 0.60)';
-                break;
-              }
-              case 'warning': {
-                threshold.fillColor = 'rgba(235, 138, 14, 0.12)';
-                threshold.lineColor = 'rgba(247, 149, 32, 0.60)';
-                break;
-              }
-              case 'ok': {
-                threshold.fillColor = 'rgba(11, 237, 50, 0.090)';
-                threshold.lineColor = 'rgba(6,163,69, 0.60)';
-                break;
-              }
-              case 'custom': {
-                threshold.fillColor = threshold.fillColor;
-                threshold.lineColor = threshold.lineColor;
-                break;
-              }
-            }
-
-            // fill
-            if (threshold.fill) {
-              options.grid.markings.push({yaxis: {from: threshold.value, to: limit}, color: threshold.fillColor});
-            }
-            if (threshold.line) {
-              options.grid.markings.push({yaxis: {from: threshold.value, to: threshold.value}, color: threshold.lineColor});
-            }
-          }
-        }
-
         function addAnnotations(options) {
           if(!annotations || annotations.length === 0) {
             return;

public/app/plugins/panel/graph/specs/graph_specs.ts

@@ -112,37 +112,6 @@ describe('grafanaGraph', function() {
     });
   });
 
-  graphScenario('grid thresholds 100, 200', function(ctx) {
-    ctx.setup(function(ctrl) {
-      ctrl.panel.thresholds = [
-        {op: "gt", value: 300, fillColor: 'red', lineColor: 'blue', fill: true, line: true},
-        {op: "gt", value: 200, fillColor: '#ed2e18', fill: true}
-      ];
-    });
-
-    it('should add fill for threshold with fill: true', function() {
-      var markings = ctx.plotOptions.grid.markings;
-
-      expect(markings[0].yaxis.from).to.be(300);
-      expect(markings[0].yaxis.to).to.be(Infinity);
-      expect(markings[0].color).to.be('red');
-    });
-
-    it('should add line', function() {
-      var markings = ctx.plotOptions.grid.markings;
-
-      expect(markings[1].yaxis.from).to.be(300);
-      expect(markings[1].yaxis.to).to.be(300);
-      expect(markings[1].color).to.be('blue');
-    });
-
-    it('should add fill for second thresholds to previous threshold', function() {
-      var markings = ctx.plotOptions.grid.markings;
-      expect(markings[2].yaxis.from).to.be(200);
-      expect(markings[2].yaxis.to).to.be(300);
-    });
-  });
-
   graphScenario('when logBase is log 10', function(ctx) {
     ctx.setup(function(ctrl) {
       ctrl.panel.yaxes[0].logBase = 10;

public/app/plugins/panel/graph/specs/threshold_manager_specs.ts

@@ -0,0 +1,115 @@
+///<reference path="../../../../headers/common.d.ts" />
+
+import {describe, beforeEach, it, sinon, expect, angularMocks} from '../../../../../test/lib/common';
+
+import {ThresholdManager} from '../threshold_manager';
+
+describe('ThresholdManager', function() {
+
+  function plotOptionsScenario(desc, func)  {
+    describe(desc, function() {
+      var ctx: any = {
+        panel: {
+          thresholds: [],
+        },
+        options: {
+          grid: {markings: []},
+        },
+        panelCtrl: {},
+      };
+
+      ctx.setup = function(thresholds) {
+        ctx.panel.thresholds = thresholds;
+        var manager = new ThresholdManager(ctx.panelCtrl);
+        manager.addPlotOptions(ctx.options, ctx.panel);
+      };
+
+      func(ctx);
+    });
+  }
+
+  describe("When creating plot markings", () => {
+
+    plotOptionsScenario("for simple gt threshold", ctx => {
+      ctx.setup([
+        {op: 'gt', value: 300, fill: true, line: true, colorMode: 'critical'},
+      ]);
+
+      it('should add fill for threshold with fill: true', function() {
+        var markings = ctx.options.grid.markings;
+
+        expect(markings[0].yaxis.from).to.be(300);
+        expect(markings[0].yaxis.to).to.be(Infinity);
+        expect(markings[0].color).to.be('rgba(234, 112, 112, 0.12)');
+      });
+
+      it('should add line', function() {
+        var markings = ctx.options.grid.markings;
+        expect(markings[1].yaxis.from).to.be(300);
+        expect(markings[1].yaxis.to).to.be(300);
+        expect(markings[1].color).to.be('rgba(237, 46, 24, 0.60)');
+      });
+    });
+
+    plotOptionsScenario("for two gt thresholds", ctx => {
+      ctx.setup([
+        {op: 'gt', value: 200, fill: true, colorMode: 'warning'},
+        {op: 'gt', value: 300, fill: true, colorMode: 'critical'},
+      ]);
+
+     it('should add fill for first thresholds to next threshold', function() {
+         var markings = ctx.options.grid.markings;
+         expect(markings[0].yaxis.from).to.be(200);
+         expect(markings[0].yaxis.to).to.be(300);
+     });
+
+     it('should add fill for last thresholds to infinity', function() {
+       var markings = ctx.options.grid.markings;
+       expect(markings[1].yaxis.from).to.be(300);
+       expect(markings[1].yaxis.to).to.be(Infinity);
+     });
+
+    });
+
+    plotOptionsScenario("for lt then gt threshold (inside)", ctx => {
+      ctx.setup([
+        {op: 'lt', value: 300, fill: true, colorMode: 'critical'},
+        {op: 'gt', value: 200, fill: true, colorMode: 'critical'},
+      ]);
+
+     it('should add fill for first thresholds to next threshold', function() {
+         var markings = ctx.options.grid.markings;
+         expect(markings[0].yaxis.from).to.be(300);
+         expect(markings[0].yaxis.to).to.be(200);
+     });
+
+     it('should add fill for last thresholds to itself', function() {
+       var markings = ctx.options.grid.markings;
+       expect(markings[1].yaxis.from).to.be(200);
+       expect(markings[1].yaxis.to).to.be(200);
+     });
+
+    });
+
+    plotOptionsScenario("for gt then lt threshold (outside)", ctx => {
+      ctx.setup([
+        {op: 'gt', value: 300, fill: true, colorMode: 'critical'},
+        {op: 'lt', value: 200, fill: true, colorMode: 'critical'},
+      ]);
+
+     it('should add fill for first thresholds to next threshold', function() {
+         var markings = ctx.options.grid.markings;
+         expect(markings[0].yaxis.from).to.be(300);
+         expect(markings[0].yaxis.to).to.be(Infinity);
+     });
+
+     it('should add fill for last thresholds to itself', function() {
+       var markings = ctx.options.grid.markings;
+       expect(markings[1].yaxis.from).to.be(200);
+       expect(markings[1].yaxis.to).to.be(-Infinity);
+     });
+
+    });
+
+  });
+});

public/app/plugins/panel/graph/thresholds.ts → public/app/plugins/panel/graph/threshold_manager.ts

@@ -4,7 +4,7 @@ import 'jquery.flot';
 import $ from 'jquery';
 import _ from 'lodash';
 
-export class ThresholdControls {
+export class ThresholdManager {
   plot: any;
   placeholder: any;
   height: any;
@@ -69,7 +69,7 @@ export class ThresholdControls {
       // trigger digest and render
       panelCtrl.$scope.$apply(function() {
         panelCtrl.render();
-        panelCtrl.events.emit('threshold-changed', {threshold: model, index: handleIndex});
+        panelCtrl.events.emit('threshold-changed', {threshold: model, handleIndex: handleIndex});
       });
     }
 
@@ -145,5 +145,82 @@ export class ThresholdControls {
     this.placeholder.on('mousedown', '.alert-handle', this.initDragging.bind(this));
     this.needsCleanup = true;
   }
+
+  addPlotOptions(options, panel) {
+    if (!panel.thresholds || panel.thresholds.length === 0) {
+      return;
+    }
+
+    var gtLimit = Infinity;
+    var ltLimit = -Infinity;
+    var i, threshold, other;
+
+    for (i = 0; i < panel.thresholds.length; i++) {
+      threshold = panel.thresholds[i];
+      if (!_.isNumber(threshold.value)) {
+        continue;
+      }
+
+      var limit;
+      switch (threshold.op) {
+        case 'gt': {
+          limit = gtLimit;
+          // if next threshold is less then op and greater value, then use that as limit
+          if (panel.thresholds.length > i+1) {
+            other = panel.thresholds[i+1];
+            if (other.value > threshold.value) {
+              limit = other.value;
+              ltLimit = limit;
+            }
+          }
+          break;
+        }
+        case 'lt': {
+          limit = ltLimit;
+          // if next threshold is less then op and greater value, then use that as limit
+          if (panel.thresholds.length > i+1) {
+            other = panel.thresholds[i+1];
+            if (other.value < threshold.value) {
+              limit = other.value;
+              gtLimit = limit;
+            }
+          }
+          break;
+        }
+      }
+
+      switch (threshold.colorMode) {
+        case 'critical': {
+          threshold.fillColor = 'rgba(234, 112, 112, 0.12)';
+          threshold.lineColor = 'rgba(237, 46, 24, 0.60)';
+          break;
+        }
+        case 'warning': {
+          threshold.fillColor = 'rgba(235, 138, 14, 0.12)';
+          threshold.lineColor = 'rgba(247, 149, 32, 0.60)';
+          break;
+        }
+        case 'ok': {
+          threshold.fillColor = 'rgba(11, 237, 50, 0.090)';
+          threshold.lineColor = 'rgba(6,163,69, 0.60)';
+          break;
+        }
+        case 'custom': {
+          threshold.fillColor = threshold.fillColor;
+          threshold.lineColor = threshold.lineColor;
+          break;
+        }
+      }
+
+      // fill
+      if (threshold.fill) {
+        options.grid.markings.push({yaxis: {from: threshold.value, to: limit}, color: threshold.fillColor});
+      }
+      if (threshold.line) {
+        options.grid.markings.push({yaxis: {from: threshold.value, to: threshold.value}, color: threshold.lineColor});
+      }
+    }
+  }
+
 }