Browse Source

fix(templating): another fix for repeating panels and having old panels/scoped vars stuck after switching repeat variable, fixes #5795

Torkel Ödegaard 9 years ago
parent
commit
58df60f670

+ 10 - 5
public/app/features/dashboard/dynamic_dashboard_srv.ts

@@ -27,10 +27,19 @@ export class DynamicDashboardSrv {
     this.iteration = (this.iteration || new Date().getTime()) + 1;
     this.iteration = (this.iteration || new Date().getTime()) + 1;
 
 
     var cleanUpOnly = options.cleanUpOnly;
     var cleanUpOnly = options.cleanUpOnly;
-
     var i, j, row, panel;
     var i, j, row, panel;
+
+    // cleanup scopedVars
     for (i = 0; i < this.dashboard.rows.length; i++) {
     for (i = 0; i < this.dashboard.rows.length; i++) {
       row = this.dashboard.rows[i];
       row = this.dashboard.rows[i];
+      for (j = 0; j < row.panels.length; j++) {
+        delete row.panels[j].scopedVars;
+      }
+    }
+
+    for (i = 0; i < this.dashboard.rows.length; i++) {
+      row = this.dashboard.rows[i];
+
       // handle row repeats
       // handle row repeats
       if (row.repeat) {
       if (row.repeat) {
         if (!cleanUpOnly) {
         if (!cleanUpOnly) {
@@ -54,10 +63,6 @@ export class DynamicDashboardSrv {
           // clean up old left overs
           // clean up old left overs
           row.panels = _.without(row.panels, panel);
           row.panels = _.without(row.panels, panel);
           j = j - 1;
           j = j - 1;
-        } else if (row.repeat || row.repeatRowId) {
-          continue;
-        } else if (!_.isEmpty(panel.scopedVars) && panel.repeatIteration !== this.iteration) {
-          panel.scopedVars = {};
         }
         }
       }
       }
     }
     }

+ 21 - 2
public/app/features/dashboard/specs/dynamic_dashboard_srv_specs.ts

@@ -93,10 +93,29 @@ dynamicDashScenario('given dashboard with panel repeat', function(ctx) {
     });
     });
   });
   });
 
 
+  describe('After a second iteration with different variable', function() {
+    beforeEach(function() {
+      ctx.dash.templating.list.push({
+        name: 'server',
+        current: { text: 'se1, se2, se3', value: ['se1']},
+        options: [{text: 'se1', value: 'se1', selected: true}]
+      });
+      ctx.rows[0].panels[0].repeat = "server";
+      ctx.dynamicDashboardSrv.update(ctx.dash);
+    });
+
+    it('should remove scopedVars value for last variable', function() {
+      expect(ctx.rows[0].panels[0].scopedVars.apps).to.be(undefined);
+    });
+
+    it('should have new variable value in scopedVars', function() {
+      expect(ctx.rows[0].panels[0].scopedVars.server.value).to.be("se1");
+    });
+  });
+
   describe('After a second iteration and selected values reduced', function() {
   describe('After a second iteration and selected values reduced', function() {
     beforeEach(function() {
     beforeEach(function() {
       ctx.dash.templating.list[0].options[1].selected = false;
       ctx.dash.templating.list[0].options[1].selected = false;
-
       ctx.dynamicDashboardSrv.update(ctx.dash);
       ctx.dynamicDashboardSrv.update(ctx.dash);
     });
     });
 
 
@@ -116,7 +135,7 @@ dynamicDashScenario('given dashboard with panel repeat', function(ctx) {
     });
     });
 
 
     it('should remove scoped vars from reused panel', function() {
     it('should remove scoped vars from reused panel', function() {
-      expect(ctx.rows[0].panels[0].scopedVars).to.be.empty();
+      expect(ctx.rows[0].panels[0].scopedVars).to.be(undefined);
     });
     });
   });
   });
 
 

+ 2 - 1
public/app/features/dashboard/specs/exporter_specs.ts

@@ -47,7 +47,8 @@ describe('given dashboard with repeated panels', function() {
     });
     });
     dash.rows.push({
     dash.rows.push({
       repeat: null,
       repeat: null,
-      repeatRowId: 1
+      repeatRowId: 1,
+      panels: [],
     });
     });
 
 
     var datasourceSrvStub = {
     var datasourceSrvStub = {

+ 0 - 269
public/test/specs/dynamicDashboardSrv-specs.js

@@ -1,269 +0,0 @@
-define([
-  'app/features/dashboard/dynamic_dashboard_srv',
-  'app/features/dashboard/dashboardSrv'
-], function() {
-  'use strict';
-
-  function dynamicDashScenario(desc, func)  {
-
-    describe(desc, function() {
-      var ctx = {};
-
-      ctx.setup = function (setupFunc) {
-
-        beforeEach(module('grafana.services'));
-        beforeEach(module('grafana.core'));
-        beforeEach(module(function($provide) {
-          $provide.value('contextSrv', {
-            user: { timezone: 'utc'}
-          });
-        }));
-
-        beforeEach(inject(function(dynamicDashboardSrv, dashboardSrv) {
-          ctx.dynamicDashboardSrv = dynamicDashboardSrv;
-          ctx.dashboardSrv = dashboardSrv;
-
-          var model = {
-            rows: [],
-            templating: { list: [] }
-          };
-
-          setupFunc(model);
-          ctx.dash = ctx.dashboardSrv.create(model);
-          ctx.dynamicDashboardSrv.init(ctx.dash);
-          ctx.rows = ctx.dash.rows;
-        }));
-      };
-
-      func(ctx);
-    });
-  }
-
-  dynamicDashScenario('given dashboard with panel repeat', function(ctx) {
-    ctx.setup(function(dash) {
-      dash.rows.push({
-        panels: [{id: 2, repeat: 'apps'}]
-      });
-      dash.templating.list.push({
-        name: 'apps',
-        current: {
-          text: 'se1, se2, se3',
-          value: ['se1', 'se2', 'se3']
-        },
-        options: [
-        {text: 'se1', value: 'se1', selected: true},
-        {text: 'se2', value: 'se2', selected: true},
-        {text: 'se3', value: 'se3', selected: true},
-        {text: 'se4', value: 'se4', selected: false}
-        ]
-      });
-    });
-
-    it('should repeat panel one time', function() {
-      expect(ctx.rows[0].panels.length).to.be(3);
-    });
-
-    it('should mark panel repeated', function() {
-      expect(ctx.rows[0].panels[0].repeat).to.be('apps');
-      expect(ctx.rows[0].panels[1].repeatPanelId).to.be(2);
-    });
-
-    it('should set scopedVars on panels', function() {
-      expect(ctx.rows[0].panels[0].scopedVars.apps.value).to.be('se1');
-      expect(ctx.rows[0].panels[1].scopedVars.apps.value).to.be('se2');
-      expect(ctx.rows[0].panels[2].scopedVars.apps.value).to.be('se3');
-    });
-
-    describe('After a second iteration', function() {
-      var repeatedPanelAfterIteration1;
-
-      beforeEach(function() {
-        repeatedPanelAfterIteration1 = ctx.rows[0].panels[1];
-        ctx.rows[0].panels[0].fill = 10;
-        ctx.dynamicDashboardSrv.update(ctx.dash);
-      });
-
-      it('should have reused same panel instances', function() {
-        expect(ctx.rows[0].panels[1]).to.be(repeatedPanelAfterIteration1);
-      });
-
-      it('reused panel should copy properties from source', function() {
-        expect(ctx.rows[0].panels[1].fill).to.be(10);
-      });
-
-      it('should have same panel count', function() {
-        expect(ctx.rows[0].panels.length).to.be(3);
-      });
-    });
-
-    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(2);
-      });
-    });
-
-    describe('After a second iteration and panel repeat is turned off', function() {
-      beforeEach(function() {
-        ctx.rows[0].panels[0].repeat = null;
-        ctx.dynamicDashboardSrv.update(ctx.dash);
-      });
-
-      it('should clean up repeated panel', function() {
-        expect(ctx.rows[0].panels.length).to.be(1);
-      });
-
-      it('should remove scoped vars from reused panel', function() {
-        expect(ctx.rows[0].panels[0].scopedVars).to.be.empty();
-      });
-    });
-
-  });
-
-  dynamicDashScenario('given dashboard with row repeat', function(ctx) {
-    ctx.setup(function(dash) {
-      dash.rows.push({
-        repeat: 'servers',
-        panels: [{id: 2}]
-      });
-      dash.rows.push({panels: []});
-      dash.templating.list.push({
-        name: 'servers',
-        current: {
-          text: 'se1, se2',
-          value: ['se1', 'se2']
-        },
-        options: [
-          {text: 'se1', value: 'se1', selected: true},
-          {text: 'se2', value: 'se2', selected: true},
-        ]
-      });
-    });
-
-    it('should repeat row one time', function() {
-      expect(ctx.rows.length).to.be(3);
-    });
-
-    it('should keep panel ids on first row', function() {
-      expect(ctx.rows[0].panels[0].id).to.be(2);
-    });
-
-    it('should keep first row as repeat', function() {
-      expect(ctx.rows[0].repeat).to.be('servers');
-    });
-
-    it('should clear repeat field on repeated row', function() {
-      expect(ctx.rows[1].repeat).to.be(null);
-    });
-
-    it('should add scopedVars to rows', function() {
-      expect(ctx.rows[0].scopedVars.servers.value).to.be('se1');
-      expect(ctx.rows[1].scopedVars.servers.value).to.be('se2');
-    });
-
-    it('should generate a repeartRowId based on repeat row index', function() {
-      expect(ctx.rows[1].repeatRowId).to.be(1);
-      expect(ctx.rows[1].repeatIteration).to.be(ctx.dynamicDashboardSrv.iteration);
-    });
-
-    it('should set scopedVars on row panels', function() {
-      expect(ctx.rows[0].panels[0].scopedVars.servers.value).to.be('se1');
-      expect(ctx.rows[1].panels[0].scopedVars.servers.value).to.be('se2');
-    });
-
-    describe('After a second iteration', function() {
-      var repeatedRowAfterFirstIteration;
-
-      beforeEach(function() {
-        repeatedRowAfterFirstIteration = ctx.rows[1];
-        ctx.rows[0].height = 500;
-        ctx.dynamicDashboardSrv.update(ctx.dash);
-      });
-
-      it('should still only have 2 rows', function() {
-        expect(ctx.rows.length).to.be(3);
-      });
-
-      it.skip('should have updated props from source', function() {
-        expect(ctx.rows[1].height).to.be(500);
-      });
-
-      it('should reuse row instance', function() {
-        expect(ctx.rows[1]).to.be(repeatedRowAfterFirstIteration);
-      });
-    });
-
-    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 remove repeated second row', function() {
-        expect(ctx.rows.length).to.be(2);
-      });
-    });
-  });
-
-  dynamicDashScenario('given dashboard with row repeat and panel repeat', function(ctx) {
-    ctx.setup(function(dash) {
-      dash.rows.push({
-        repeat: 'servers',
-        panels: [{id: 2, repeat: 'metric'}]
-      });
-      dash.templating.list.push({
-        name: 'servers',
-        current: { text: 'se1, se2', value: ['se1', 'se2'] },
-        options: [
-          {text: 'se1', value: 'se1', selected: true},
-          {text: 'se2', value: 'se2', selected: true},
-        ]
-      });
-      dash.templating.list.push({
-        name: 'metric',
-        current: { text: 'm1, m2', value: ['m1', 'm2'] },
-        options: [
-          {text: 'm1', value: 'm1', selected: true},
-          {text: 'm2', value: 'm2', selected: true},
-        ]
-      });
-    });
-
-    it('should repeat row one time', function() {
-      expect(ctx.rows.length).to.be(2);
-    });
-
-    it('should repeat panel on both rows', function() {
-      expect(ctx.rows[0].panels.length).to.be(2);
-      expect(ctx.rows[1].panels.length).to.be(2);
-    });
-
-    it('should keep panel ids on first row', function() {
-      expect(ctx.rows[0].panels[0].id).to.be(2);
-    });
-
-    it('should mark second row as repeated', function() {
-      expect(ctx.rows[0].repeat).to.be('servers');
-    });
-
-    it('should clear repeat field on repeated row', function() {
-      expect(ctx.rows[1].repeat).to.be(null);
-    });
-
-    it('should generate a repeartRowId based on repeat row index', function() {
-      expect(ctx.rows[1].repeatRowId).to.be(1);
-    });
-
-    it('should set scopedVars on row panels', function() {
-      expect(ctx.rows[0].panels[0].scopedVars.servers.value).to.be('se1');
-      expect(ctx.rows[1].panels[0].scopedVars.servers.value).to.be('se2');
-    });
-
-  });
-
-});