diff --git a/CHANGELOG.md b/CHANGELOG.md index 96a7f7ce0c1..0fc4e886b64 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/public/app/features/dashboard/dashboardCtrl.js b/public/app/features/dashboard/dashboardCtrl.js index 3eab543ff73..4a1c38d0c5f 100644 --- a/public/app/features/dashboard/dashboardCtrl.js +++ b/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() { diff --git a/public/app/features/dashboard/dashboardNavCtrl.js b/public/app/features/dashboard/dashboardNavCtrl.js index 5334852d967..11595e30479 100644 --- a/public/app/features/dashboard/dashboardNavCtrl.js +++ b/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() { diff --git a/public/app/features/dashboard/dashboardSrv.js b/public/app/features/dashboard/dashboardSrv.js index 51afefee14a..1064892745b 100644 --- a/public/app/features/dashboard/dashboardSrv.js +++ b/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); } }; diff --git a/public/app/features/dashboard/shareSnapshotCtrl.js b/public/app/features/dashboard/shareSnapshotCtrl.js index bcdbee57d18..9601ab48f0b 100644 --- a/public/app/features/dashboard/shareSnapshotCtrl.js +++ b/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 = { diff --git a/public/app/features/dashboard/unsavedChangesSrv.js b/public/app/features/dashboard/unsavedChangesSrv.js index 543536b3d27..15eb465e333 100644 --- a/public/app/features/dashboard/unsavedChangesSrv.js +++ b/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(); diff --git a/public/app/features/panel/soloPanelCtrl.js b/public/app/features/panel/soloPanelCtrl.js index 02727694e41..f593f8ee950 100644 --- a/public/app/features/panel/soloPanelCtrl.js +++ b/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', diff --git a/public/app/routes/dashLoadControllers.js b/public/app/routes/dashLoadControllers.js index 51db273eac0..b19b0c9664f 100644 --- a/public/app/routes/dashLoadControllers.js +++ b/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); }); }); diff --git a/public/test/specs/dashboardSrv-specs.js b/public/test/specs/dashboardSrv-specs.js index 2ee4a5df4c9..19f81f1cc01 100644 --- a/public/test/specs/dashboardSrv-specs.js +++ b/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); - }); - - }); }); diff --git a/public/test/specs/helpers.js b/public/test/specs/helpers.js index a0f7c186331..12a2978272f 100644 --- a/public/test/specs/helpers.js +++ b/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 }; diff --git a/public/test/specs/soloPanelCtrl-specs.js b/public/test/specs/soloPanelCtrl-specs.js index cab22b6f627..44c9f2080cc 100644 --- a/public/test/specs/soloPanelCtrl-specs.js +++ b/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";