Unsaved changes: Do not show for snapshots, scripted and file based dashboards, Fixes #1707

CHANGELOG.md

@@ -3,7 +3,8 @@
 **Enhancements**
 - [Issue #1701](https://github.com/grafana/grafana/issues/1701). Share modal: Override UI theme via URL param for Share link, rendered panel, or embedded panel
 
-**FIxes**
+**Fixes**
+- [Issue #1707](https://github.com/grafana/grafana/issues/1707). Unsaved changes: Do not show for snapshots, scripted and file based dashboards
 - [Issue #1703](https://github.com/grafana/grafana/issues/1703). Unsaved changes: Do not show for users with role `Viewer`
 - [Issue #1675](https://github.com/grafana/grafana/issues/1675). Data source proxy: Fixed issue with Gzip enabled and data source proxy
 - [Issue #1681](https://github.com/grafana/grafana/issues/1681). MySQL session: fixed problem using mysql as session store

public/app/features/dashboard/dashboardCtrl.js

@@ -38,7 +38,7 @@ function (angular, $, config) {
       $rootScope.performance.panelsInitialized = 0;
       $rootScope.performance.panelsRendered = 0;
 
-      var dashboard = dashboardSrv.create(data.model);
+      var dashboard = dashboardSrv.create(data.model, data.meta);
 
       // init services
       timeSrv.init(dashboard);
@@ -47,11 +47,12 @@ function (angular, $, config) {
       // the rest of the dashboard can load
       templateValuesSrv.init(dashboard).then(function() {
         $scope.dashboard = dashboard;
+        $scope.dashboardMeta = dashboard.meta;
         $scope.dashboardViewState = dashboardViewStateSrv.create($scope);
-        $scope.initDashboardMeta(data.meta, $scope.dashboard);
 
         dashboardKeybindings.shortcuts($scope);
 
+        $scope.updateTopNavPartial();
         $scope.updateSubmenuVisibility();
         $scope.setWindowTitleAndTheme();
 
@@ -59,28 +60,10 @@ function (angular, $, config) {
       });
     };
 
-    $scope.initDashboardMeta = function(meta) {
-      meta.canShare = true;
-      meta.canSave = true;
-      meta.canEdit = true;
-      meta.canStar = true;
-
-      if (contextSrv.hasRole('Viewer')) {
-        meta.canSave = false;
-      }
-
-      if (meta.isHome) {
-        meta.canShare = false;
-        meta.canStar = false;
-        meta.canSave = false;
-        meta.canEdit = false;
-      }
-
-      if (meta.isSnapshot) {
+    $scope.updateTopNavPartial = function() {
+      if ($scope.dashboard.meta.isSnapshot) {
         $scope.topNavPartial = 'app/features/dashboard/partials/snapshotTopNav.html';
       }
-
-      $scope.dashboardMeta = meta;
     };
 
     $scope.updateSubmenuVisibility = function() {

public/app/features/dashboard/dashboardNavCtrl.js

@@ -52,7 +52,7 @@ function (angular, _) {
     };
 
     $scope.saveDashboard = function(options) {
-      var clone = angular.copy($scope.dashboard);
+      var clone = $scope.dashboard.getSaveModelClone();
 
       backendSrv.saveDashboard(clone, options).then(function(data) {
         $scope.dashboard.version = data.version;
@@ -118,7 +118,7 @@ function (angular, _) {
 
     $scope.saveDashboardAs = function() {
       var newScope = $rootScope.$new();
-      newScope.clone = angular.copy($scope.dashboard);
+      newScope.clone = $scope.dashboard.getSaveModelClone();
 
       $scope.appEvent('show-modal', {
         src: './app/features/dashboard/partials/saveDashboardAs.html',
@@ -127,7 +127,8 @@ function (angular, _) {
     };
 
     $scope.exportDashboard = function() {
-      var blob = new Blob([angular.toJson($scope.dashboard, true)], { type: "application/json;charset=utf-8" });
+      var clone = $scope.dashboard.getSaveModelClone();
+      var blob = new Blob([angular.toJson(clone, true)], { type: "application/json;charset=utf-8" });
       window.saveAs(blob, $scope.dashboard.title + '-' + new Date().getTime());
     };
 
@@ -144,7 +145,8 @@ function (angular, _) {
     };
 
     $scope.editJson = function() {
-      $scope.appEvent('show-json-editor', { object: $scope.dashboard });
+      var clone = $scope.dashboard.getSaveModelClone();
+      $scope.appEvent('show-json-editor', { object: clone });
     };
 
     $scope.stopPlaylist = function() {

public/app/features/dashboard/dashboardSrv.js

@@ -10,10 +10,9 @@ function (angular, $, kbn, _, moment) {
 
   var module = angular.module('grafana.services');
 
-  module.factory('dashboardSrv', function()  {
-
-    function DashboardModel (data) {
+  module.factory('dashboardSrv', function(contextSrv)  {
 
+    function DashboardModel (data, meta) {
       if (!data) {
         data = {};
       }
@@ -46,10 +45,43 @@ function (angular, $, kbn, _, moment) {
       }
 
       this._updateSchema(data);
+      this._initMeta(meta);
     }
 
     var p = DashboardModel.prototype;
 
+    p._initMeta = function(meta) {
+      meta = meta || {};
+      meta.canShare = true;
+      meta.canSave = true;
+      meta.canEdit = true;
+      meta.canStar = true;
+
+      if (contextSrv.hasRole('Viewer')) {
+        meta.canSave = false;
+      }
+
+      if (meta.isSnapshot) {
+        meta.canSave = false;
+      }
+
+      if (meta.isHome) {
+        meta.canShare = false;
+        meta.canStar = false;
+        meta.canSave = false;
+        meta.canEdit = false;
+      }
+
+      this.meta = meta;
+    };
+
+    // cleans meta data and other non peristent state
+    p.getSaveModelClone = function() {
+      var copy = angular.copy(this);
+      delete copy.meta;
+      return copy;
+    };
+
     p._ensureListExist = function (data) {
       if (!data) { data = {}; }
       if (!data.list) { data.list = []; }
@@ -276,8 +308,8 @@ function (angular, $, kbn, _, moment) {
     };
 
     return {
-      create: function(dashboard) {
-        return new DashboardModel(dashboard);
+      create: function(dashboard, meta) {
+        return new DashboardModel(dashboard, meta);
       }
     };
 

public/app/features/dashboard/shareSnapshotCtrl.js

@@ -48,7 +48,7 @@ function (angular, _) {
     };
 
     $scope.saveSnapshot = function(external) {
-      var dash = angular.copy($scope.dashboard);
+      var dash = $scope.dashboard.getSaveModelClone();
       $scope.scrubDashboard(dash);
 
       var cmdData = {

public/app/features/dashboard/unsavedChangesSrv.js

@@ -12,7 +12,7 @@ function(angular, _, config) {
 
   var module = angular.module('grafana.services');
 
-  module.service('unsavedChangesSrv', function($rootScope, $modal, $q, $location, $timeout, contextSrv) {
+  module.service('unsavedChangesSrv', function($rootScope, $modal, $q, $location, $timeout) {
 
     var self = this;
     var modalScope = $rootScope.$new();
@@ -36,8 +36,15 @@ function(angular, _, config) {
       self.originalPath = $location.path();
     });
 
+    this.ignoreChanges = function() {
+      if (!self.current) { return true; }
+
+      var meta = self.current.meta;
+      return !meta.canSave || meta.fromScript || meta.fromFile;
+    };
+
     window.onbeforeunload = function() {
-      if (contextSrv.hasRole('Viewer')) { return true; }
+      if (self.ignoreChanges()) { return; }
       if (self.has_unsaved_changes()) {
         return "There are unsaved changes to this dashboard";
       }
@@ -47,7 +54,7 @@ function(angular, _, config) {
       $rootScope.$on("$locationChangeStart", function(event, next) {
         // check if we should look for changes
         if (self.originalPath === $location.path()) { return true; }
-        if (contextSrv.hasRole('Viewer')) { return true; }
+        if (self.ignoreChanges()) { return true; }
 
         if (self.has_unsaved_changes()) {
           event.preventDefault();

public/app/features/panel/soloPanelCtrl.js

@@ -40,8 +40,8 @@ function (angular, $) {
       });
     };
 
-    $scope.initPanelScope = function(dashboard) {
-      $scope.dashboard = dashboardSrv.create(dashboard.model);
+    $scope.initPanelScope = function(dashData) {
+      $scope.dashboard = dashboardSrv.create(dashData.model, dashData.meta);
 
       $scope.row = {
         height: ($(window).height() - 10) + 'px',

public/app/routes/dashLoadControllers.js

@@ -48,7 +48,7 @@ function (angular, _, kbn, moment, $) {
       $location.path('');
       return;
     }
-    $scope.initDashboard({ meta: {}, model: window.grafanaImportDashboard }, $scope);
+    $scope.initDashboard({meta: {}, model: window.grafanaImportDashboard }, $scope);
   });
 
   module.controller('NewDashboardCtrl', function($scope) {
@@ -82,7 +82,7 @@ function (angular, _, kbn, moment, $) {
     };
 
     file_load($routeParams.jsonFile).then(function(result) {
-      $scope.initDashboard({meta: {}, model: result}, $scope);
+      $scope.initDashboard({meta: {fromFile: true}, model: result}, $scope);
     });
 
   });
@@ -127,7 +127,7 @@ function (angular, _, kbn, moment, $) {
     };
 
     script_load($routeParams.jsFile).then(function(result) {
-      $scope.initDashboard({meta: {}, model: result.data}, $scope);
+      $scope.initDashboard({meta: {fromScript: true}, model: result.data}, $scope);
     });
 
   });

public/test/specs/dashboardSrv-specs.js

@@ -1,191 +1,194 @@
 define([
+  'helpers',
   'features/dashboard/dashboardSrv'
-], function() {
+], function(helpers) {
   'use strict';
 
-  describe('when creating new dashboard with defaults only', function() {
-    var model;
+  describe('dashboardSrv', function() {
+    var _dashboardSrv;
+    var contextSrv = new helpers.ContextSrvStub();
 
     beforeEach(module('grafana.services'));
-    beforeEach(inject(function(dashboardSrv) {
-      model = dashboardSrv.create({});
+    beforeEach(module(function($provide) {
+      $provide.value('contextSrv', contextSrv);
     }));
 
-    it('should have title', function() {
-      expect(model.title).to.be('No Title');
-    });
-
-    it('should have default properties', function() {
-      expect(model.rows.length).to.be(0);
-      expect(model.nav.length).to.be(1);
-    });
-
-  });
-
-  describe('when getting next panel id', function() {
-    var model;
-
-    beforeEach(module('grafana.services'));
     beforeEach(inject(function(dashboardSrv) {
-      model = dashboardSrv.create({
-        rows: [{ panels: [{ id: 5 }]}]
+      _dashboardSrv = dashboardSrv;
+    }));
+
+    describe('when creating new dashboard with defaults only', function() {
+      var model;
+
+      beforeEach(function() {
+        model = _dashboardSrv.create({}, {});
       });
-    }));
 
-    it('should return max id + 1', function() {
-      expect(model.getNextPanelId()).to.be(6);
-    });
-  });
-
-  describe('row and panel manipulation', function() {
-    var dashboard;
-
-    beforeEach(module('grafana.services'));
-    beforeEach(inject(function(dashboardSrv) {
-      dashboard = dashboardSrv.create({});
-    }));
-
-    it('row span should sum spans', function() {
-      var spanLeft = dashboard.rowSpan({ panels: [{ span: 2 }, { span: 3 }] });
-      expect(spanLeft).to.be(5);
-    });
-
-    it('adding default should split span in half', function() {
-      dashboard.rows = [{ panels: [{ span: 12, id: 7 }] }];
-      dashboard.add_panel({span: 4}, dashboard.rows[0]);
-
-      expect(dashboard.rows[0].panels[0].span).to.be(6);
-      expect(dashboard.rows[0].panels[1].span).to.be(6);
-      expect(dashboard.rows[0].panels[1].id).to.be(8);
-    });
-
-    it('duplicate panel should try to add it to same row', function() {
-      var panel = { span: 4, attr: '123', id: 10 };
-      dashboard.rows = [{ panels: [panel] }];
-      dashboard.duplicatePanel(panel, dashboard.rows[0]);
-
-      expect(dashboard.rows[0].panels[0].span).to.be(4);
-      expect(dashboard.rows[0].panels[1].span).to.be(4);
-      expect(dashboard.rows[0].panels[1].attr).to.be('123');
-      expect(dashboard.rows[0].panels[1].id).to.be(11);
-    });
-
-  });
-
-  describe('when creating dashboard with editable false', function() {
-    var model;
-
-    beforeEach(module('grafana.services'));
-    beforeEach(inject(function(dashboardSrv) {
-      model = dashboardSrv.create({
-        editable: false
+      it('should have title', function() {
+        expect(model.title).to.be('No Title');
+      });
+
+      it('should have meta', function() {
+        expect(model.meta.canSave).to.be(false);
+        expect(model.meta.canShare).to.be(true);
+      });
+
+      it('should have default properties', function() {
+        expect(model.rows.length).to.be(0);
+        expect(model.nav.length).to.be(1);
       });
-    }));
 
-    it('should set editable false', function() {
-      expect(model.editable).to.be(false);
     });
 
-  });
+    describe('when getting next panel id', function() {
+      var model;
 
-  describe('when creating dashboard with old schema', function() {
-    var model;
-    var graph;
+      beforeEach(function() {
+        model = _dashboardSrv.create({
+          rows: [{ panels: [{ id: 5 }]}]
+        });
+      });
 
-    beforeEach(module('grafana.services'));
-    beforeEach(inject(function(dashboardSrv) {
-      model = dashboardSrv.create({
-        services: { filter: { time: { from: 'now-1d', to: 'now'}, list: [{}] }},
-        pulldowns: [
-          {
-            type: 'filtering',
-            enable: true
-          },
-          {
-            type: 'annotations',
+      it('should return max id + 1', function() {
+        expect(model.getNextPanelId()).to.be(6);
+      });
+    });
+
+    describe('row and panel manipulation', function() {
+      var dashboard;
+
+      beforeEach(function() {
+        dashboard = _dashboardSrv.create({});
+      });
+
+      it('row span should sum spans', function() {
+        var spanLeft = dashboard.rowSpan({ panels: [{ span: 2 }, { span: 3 }] });
+        expect(spanLeft).to.be(5);
+      });
+
+      it('adding default should split span in half', function() {
+        dashboard.rows = [{ panels: [{ span: 12, id: 7 }] }];
+        dashboard.add_panel({span: 4}, dashboard.rows[0]);
+
+        expect(dashboard.rows[0].panels[0].span).to.be(6);
+        expect(dashboard.rows[0].panels[1].span).to.be(6);
+        expect(dashboard.rows[0].panels[1].id).to.be(8);
+      });
+
+      it('duplicate panel should try to add it to same row', function() {
+        var panel = { span: 4, attr: '123', id: 10 };
+        dashboard.rows = [{ panels: [panel] }];
+        dashboard.duplicatePanel(panel, dashboard.rows[0]);
+
+        expect(dashboard.rows[0].panels[0].span).to.be(4);
+        expect(dashboard.rows[0].panels[1].span).to.be(4);
+        expect(dashboard.rows[0].panels[1].attr).to.be('123');
+        expect(dashboard.rows[0].panels[1].id).to.be(11);
+      });
+
+    });
+
+    describe('when creating dashboard with editable false', function() {
+      var model;
+
+      beforeEach(function() {
+        model = _dashboardSrv.create({
+          editable: false
+        });
+      });
+
+      it('should set editable false', function() {
+        expect(model.editable).to.be(false);
+      });
+
+    });
+
+    describe('when creating dashboard with old schema', function() {
+      var model;
+      var graph;
+
+      beforeEach(function() {
+        model = _dashboardSrv.create({
+          services: { filter: { time: { from: 'now-1d', to: 'now'}, list: [{}] }},
+          pulldowns: [
+            {type: 'filtering', enable: true},
+            {type: 'annotations', enable: true, annotations: [{name: 'old'}]}
+          ],
+          rows: [
+            {
+              panels: [
+                {type: 'graphite', legend: true, aliasYAxis: { test: 2 }, grid: { min: 1, max: 10 }}
+              ]
+            }
+          ]
+        });
+
+        graph = model.rows[0].panels[0];
+      });
+
+      it('should have title', function() {
+        expect(model.title).to.be('No Title');
+      });
+
+      it('should have panel id', function() {
+        expect(graph.id).to.be(1);
+      });
+
+      it('should move time and filtering list', function() {
+        expect(model.time.from).to.be('now-1d');
+        expect(model.templating.list[0].allFormat).to.be('glob');
+      });
+
+      it('graphite panel should change name too graph', function() {
+        expect(graph.type).to.be('graph');
+      });
+
+      it('update legend setting', function() {
+        expect(graph.legend.show).to.be(true);
+      });
+
+      it('update grid options', function() {
+        expect(graph.grid.leftMin).to.be(1);
+        expect(graph.grid.leftMax).to.be(10);
+      });
+
+      it('move aliasYAxis to series override', function() {
+        expect(graph.seriesOverrides[0].alias).to.be("test");
+        expect(graph.seriesOverrides[0].yaxis).to.be(2);
+      });
+
+      it('should move pulldowns to new schema', function() {
+        expect(model.annotations.list[0].name).to.be('old');
+      });
+
+      it('dashboard schema version should be set to latest', function() {
+        expect(model.schemaVersion).to.be(6);
+      });
+
+    });
+
+    describe('when creating dashboard model with missing list for annoations or templating', function() {
+      var model;
+
+      beforeEach(function() {
+        model = _dashboardSrv.create({
+          annotations: {
             enable: true,
-            annotations: [{name: 'old'}]
+          },
+          templating: {
+            enable: true
           }
-        ],
-        rows: [
-          {
-            panels: [
-              {
-                type: 'graphite',
-                legend: true,
-                aliasYAxis: { test: 2 },
-                grid: { min: 1, max: 10 }
-              }
-            ]
-          }
-        ]
+        });
       });
 
-      graph = model.rows[0].panels[0];
+      it('should add empty list', function() {
+        expect(model.annotations.list.length).to.be(0);
+        expect(model.templating.list.length).to.be(0);
+      });
 
-    }));
-
-    it('should have title', function() {
-      expect(model.title).to.be('No Title');
-    });
-
-    it('should have panel id', function() {
-      expect(graph.id).to.be(1);
-    });
-
-    it('should move time and filtering list', function() {
-      expect(model.time.from).to.be('now-1d');
-      expect(model.templating.list[0].allFormat).to.be('glob');
-    });
-
-    it('graphite panel should change name too graph', function() {
-      expect(graph.type).to.be('graph');
-    });
-
-    it('update legend setting', function() {
-      expect(graph.legend.show).to.be(true);
-    });
-
-    it('update grid options', function() {
-      expect(graph.grid.leftMin).to.be(1);
-      expect(graph.grid.leftMax).to.be(10);
-    });
-
-    it('move aliasYAxis to series override', function() {
-      expect(graph.seriesOverrides[0].alias).to.be("test");
-      expect(graph.seriesOverrides[0].yaxis).to.be(2);
-    });
-
-    it('should move pulldowns to new schema', function() {
-      expect(model.annotations.list[0].name).to.be('old');
-    });
-
-    it('dashboard schema version should be set to latest', function() {
-      expect(model.schemaVersion).to.be(6);
     });
 
   });
 
-  describe('when creating dashboard model with missing list for annoations or templating', function() {
-    var model;
-
-    beforeEach(module('grafana.services'));
-    beforeEach(inject(function(dashboardSrv) {
-      model = dashboardSrv.create({
-        annotations: {
-          enable: true,
-        },
-        templating: {
-          enable: true
-        }
-      });
-    }));
-
-    it('should add empty list', function() {
-      expect(model.annotations.list.length).to.be(0);
-      expect(model.templating.list.length).to.be(0);
-    });
-
-  });
 
 });

public/test/specs/helpers.js

@@ -115,6 +115,12 @@ define([
     };
   }
 
+  function ContextSrvStub() {
+    this.hasRole = function() {
+      return true;
+    };
+  }
+
   function TemplateSrvStub() {
     this.variables = [];
     this.templateSettings = { interpolate : /\[\[([\s\S]+?)\]\]/g };
@@ -134,6 +140,7 @@ define([
   return {
     ControllerTestContext: ControllerTestContext,
     TimeSrvStub: TimeSrvStub,
+    ContextSrvStub: ContextSrvStub,
     ServiceTestContext: ServiceTestContext
   };
 

public/test/specs/soloPanelCtrl-specs.js

@@ -10,7 +10,9 @@ define([
     var backendSrv = {};
     var routeParams = {};
     var search = {};
-    var contextSrv = {};
+    var contextSrv = {
+      hasRole: sinon.stub().returns(true)
+    };
 
     beforeEach(module('grafana.routes'));
     beforeEach(module('grafana.services'));
@@ -43,7 +45,8 @@ define([
                 ]
               }
             ]
-          }
+          },
+          meta: {}
         };
 
         routeParams.slug = "my dash";