From 32fe723da627a3614d03d38753e7f4a31f89beb3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel.odegaard@gmail.com>
Date: Tue, 28 Apr 2015 08:44:48 +0200
Subject: [PATCH] More optimizations and unit tests for panel repeats, #1888

---
 .../features/dashboard/dynamicDashboardSrv.js | 46 +++++++++++++------
 .../test/specs/dynamicDashboardSrv-specs.js   | 37 +++++++++++----
 2 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/public/app/features/dashboard/dynamicDashboardSrv.js b/public/app/features/dashboard/dynamicDashboardSrv.js
index 82b40007174..3f053e4e9af 100644
--- a/public/app/features/dashboard/dynamicDashboardSrv.js
+++ b/public/app/features/dashboard/dynamicDashboardSrv.js
@@ -8,14 +8,20 @@ function (angular, _) {
   var module = angular.module('grafana.services');
 
   module.service('dynamicDashboardSrv', function()  {
+    var self = this;
+
     this.init = function(dashboard) {
-      this.iteration = 0;
+      this.dashboard = dashboard;
+      this.iteration = new Date().getTime();
 
       this.handlePanelRepeats(dashboard);
       this.handleRowRepeats(dashboard);
     };
 
     this.update = function(dashboard) {
+      this.dashboard = dashboard;
+      this.iteration = this.iteration + 1;
+
       this.handlePanelRepeats(dashboard);
       this.handleRowRepeats(dashboard);
     };
@@ -36,8 +42,6 @@ function (angular, _) {
     };
 
     this.handlePanelRepeats = function(dashboard) {
-      this.removeLinkedPanels(dashboard);
-
       var i, j, row, panel;
       for (i = 0; i < dashboard.rows.length; i++) {
         row = dashboard.rows[i];
@@ -46,6 +50,11 @@ function (angular, _) {
           if (panel.repeat) {
             this.repeatPanel(panel, row, dashboard);
           }
+          // clean up old left overs
+          else if (panel.repeatPanelId && panel.repeatIteration !== this.iteration) {
+            row.panels = _.without(row.panels, panel);
+            j = j - 1;
+          }
         }
       }
     };
@@ -111,13 +120,27 @@ function (angular, _) {
       });
     };
 
-    this.getRepeatPanel = function(sourcePanel, row) {
+    this.getPanelClone = function(sourcePanel, row, index) {
+      // if first clone return source
+      if (index === 0) {
+        return sourcePanel;
+      }
+
+      // first try finding an existing clone to use
       for (var i = 0; i < row.panels.length; i++) {
         var panel = row.panels[i];
-        if (panel.sourcePanel === sourcePanel) {
+        if (panel.repeatIteration !== this.iteration &&
+            panel.repeatPanelId === sourcePanel.id) {
+          panel.repeatIteration = this.iteration;
           return panel;
         }
       }
+
+      var clone = this.dashboard.duplicatePanel(sourcePanel, row);
+      clone.repeatIteration = this.iteration;
+      clone.repeatPanelId = sourcePanel.id;
+      clone.repeat = null;
+      return clone;
     };
 
     this.repeatPanel = function(panel, row, dashboard) {
@@ -135,16 +158,9 @@ function (angular, _) {
       }
 
       _.each(selected, function(option, index) {
-        if (index > 0) {
-          var copy = dashboard.duplicatePanel(panel, row);
-          copy.repeat = null;
-          copy.linked = true;
-          copy.scopedVars = {};
-          copy.scopedVars[variable.name] = option;
-        } else {
-          panel.scopedVars = {};
-          panel.scopedVars[variable.name] = option;
-        }
+        var copy = self.getPanelClone(panel, row, index);
+        copy.scopedVars = {};
+        copy.scopedVars[variable.name] = option;
       });
     };
 
diff --git a/public/test/specs/dynamicDashboardSrv-specs.js b/public/test/specs/dynamicDashboardSrv-specs.js
index f58fbf5cfc6..c4e2ff9204c 100644
--- a/public/test/specs/dynamicDashboardSrv-specs.js
+++ b/public/test/specs/dynamicDashboardSrv-specs.js
@@ -26,13 +26,10 @@ define([
           ctx.dash = ctx.dashboardSrv.create(model);
           ctx.dynamicDashboardSrv.init(ctx.dash);
           ctx.rows = ctx.dash.rows;
-
         }));
-
       };
 
       func(ctx);
-
     });
   }
 
@@ -59,10 +56,8 @@ define([
     });
 
     it('should mark panel repeated', function() {
-      expect(ctx.rows[0].panels[0].linked).to.be(undefined);
       expect(ctx.rows[0].panels[0].repeat).to.be('$apps');
-      expect(ctx.rows[0].panels[1].linked).to.be(true);
-      expect(ctx.rows[0].panels[1].repeat).to.be(null);
+      expect(ctx.rows[0].panels[1].repeatPanelId).to.be(2);
     });
 
     it('should set scopedVars on panels', function() {
@@ -70,6 +65,34 @@ define([
       expect(ctx.rows[0].panels[1].scopedVars.apps.value).to.be('se2');
     });
 
+    describe('After a second iteration', function() {
+      var repeatedPanelAfterIteration1;
+
+      beforeEach(function() {
+        repeatedPanelAfterIteration1 = ctx.rows[0].panels[1];
+        ctx.dynamicDashboardSrv.update(ctx.dash);
+      });
+
+      it('should have reused same panel instances', function() {
+        expect(ctx.rows[0].panels[1]).to.be(repeatedPanelAfterIteration1);
+      });
+
+      it('should have same panel count', function() {
+        expect(ctx.rows[0].panels.length).to.be(2);
+      });
+    });
+
+    describe('After a second iteration and selected values reduced', function() {
+      beforeEach(function() {
+        ctx.dash.templating.list[0].options[1].selected = false;
+        ctx.dynamicDashboardSrv.update(ctx.dash);
+      });
+
+      it('should clean up repeated panel', function() {
+        expect(ctx.rows[0].panels.length).to.be(1);
+      });
+    });
+
   });
 
   dynamicDashScenario('given dashboard with row repeat', function(ctx) {
@@ -110,8 +133,6 @@ define([
       expect(ctx.rows[0].panels[0].scopedVars.servers.value).to.be('se1');
       expect(ctx.rows[1].panels[0].scopedVars.servers.value).to.be('se2');
     });
-
   });
 
-
 });