From d6faa3d06f606410070b38ae78fc6665836358eb Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 27 Feb 2018 18:51:04 +0100 Subject: [PATCH 01/15] provisioning: improve UX when saving provisioned dashboards --- pkg/api/dashboard.go | 7 ++ pkg/api/dtos/dashboard.go | 1 + pkg/models/dashboards.go | 6 ++ pkg/services/provisioning/dashboards/types.go | 3 - .../sqlstore/dashboard_provisioning.go | 13 ++++ .../sqlstore/dashboard_provisioning_test.go | 10 +++ public/app/features/dashboard/all.ts | 1 + .../app/features/dashboard/dashboard_srv.ts | 11 +++ .../features/dashboard/dashnav/dashnav.html | 2 +- .../dashboard/save_provisioned_modal.ts | 74 +++++++++++++++++++ .../specs/save_provisioned_modal.jest.ts | 28 +++++++ 11 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 public/app/features/dashboard/save_provisioned_modal.ts create mode 100644 public/app/features/dashboard/specs/save_provisioned_modal.jest.ts diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 11a028cdd29..e5e4fc560e1 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -102,6 +102,13 @@ func GetDashboard(c *m.ReqContext) Response { meta.FolderUrl = query.Result.GetUrl() } + dpQuery := &m.GetProvisionedDashboardByDashboardId{DashboardId: dash.Id} + err = bus.Dispatch(dpQuery) + if dpQuery.Result != nil { + meta.CanEdit = true + meta.Provisioned = true + } + // make sure db version is in sync with json model version dash.Data.Set("version", dash.Version) diff --git a/pkg/api/dtos/dashboard.go b/pkg/api/dtos/dashboard.go index e4c66aebbda..39a6dca580d 100644 --- a/pkg/api/dtos/dashboard.go +++ b/pkg/api/dtos/dashboard.go @@ -28,6 +28,7 @@ type DashboardMeta struct { FolderId int64 `json:"folderId"` FolderTitle string `json:"folderTitle"` FolderUrl string `json:"folderUrl"` + Provisioned bool `json:"provisioned"` } type DashboardFullWithMeta struct { diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 4b771038df6..e4f0758fc19 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -317,6 +317,12 @@ type GetDashboardSlugByIdQuery struct { Result string } +type GetProvisionedDashboardByDashboardId struct { + DashboardId int64 + + Result *DashboardProvisioning +} + type GetProvisionedDashboardDataQuery struct { Name string diff --git a/pkg/services/provisioning/dashboards/types.go b/pkg/services/provisioning/dashboards/types.go index f742b321552..4a55351d3e4 100644 --- a/pkg/services/provisioning/dashboards/types.go +++ b/pkg/services/provisioning/dashboards/types.go @@ -55,9 +55,6 @@ func createDashboardJson(data *simplejson.Json, lastModified time.Time, cfg *Das dash.OrgId = cfg.OrgId dash.Dashboard.OrgId = cfg.OrgId dash.Dashboard.FolderId = folderId - if !cfg.Editable { - dash.Dashboard.Data.Set("editable", cfg.Editable) - } if dash.Dashboard.Title == "" { return nil, models.ErrDashboardTitleEmpty diff --git a/pkg/services/sqlstore/dashboard_provisioning.go b/pkg/services/sqlstore/dashboard_provisioning.go index 69409c3b873..99178d38f9c 100644 --- a/pkg/services/sqlstore/dashboard_provisioning.go +++ b/pkg/services/sqlstore/dashboard_provisioning.go @@ -8,6 +8,7 @@ import ( func init() { bus.AddHandler("sql", GetProvisionedDashboardDataQuery) bus.AddHandler("sql", SaveProvisionedDashboard) + bus.AddHandler("sql", GetProvisionedDataByDashboardId) } type DashboardExtras struct { @@ -17,6 +18,18 @@ type DashboardExtras struct { Value string } +func GetProvisionedDataByDashboardId(cmd *models.GetProvisionedDashboardByDashboardId) error { + result := &models.DashboardProvisioning{} + + _, err := x.Where("dashboard_id = ?", cmd.DashboardId).Get(result) + if err != nil { + return err + } + + cmd.Result = result + return nil +} + func SaveProvisionedDashboard(cmd *models.SaveProvisionedDashboardCommand) error { return inTransaction(func(sess *DBSession) error { err := saveDashboard(sess, cmd.DashboardCmd) diff --git a/pkg/services/sqlstore/dashboard_provisioning_test.go b/pkg/services/sqlstore/dashboard_provisioning_test.go index b752173b67d..89b3451a3ac 100644 --- a/pkg/services/sqlstore/dashboard_provisioning_test.go +++ b/pkg/services/sqlstore/dashboard_provisioning_test.go @@ -50,6 +50,16 @@ func TestDashboardProvisioningTest(t *testing.T) { So(query.Result[0].DashboardId, ShouldEqual, dashId) So(query.Result[0].Updated, ShouldEqual, now.Unix()) }) + + Convey("Can query for one provisioned dashboard", func() { + query := &models.GetProvisionedDashboardByDashboardId{DashboardId: cmd.Result.Id} + + err := GetProvisionedDataByDashboardId(query) + So(err, ShouldBeNil) + + So(query.Result.DashboardId, ShouldEqual, cmd.Result.Id) + So(query.Result.Updated, ShouldEqual, now.Unix()) + }) }) }) } diff --git a/public/app/features/dashboard/all.ts b/public/app/features/dashboard/all.ts index f2e2e3dcdc0..a8f491f3ddd 100644 --- a/public/app/features/dashboard/all.ts +++ b/public/app/features/dashboard/all.ts @@ -6,6 +6,7 @@ import './dashnav/dashnav'; import './submenu/submenu'; import './save_as_modal'; import './save_modal'; +import './save_provisioned_modal'; import './shareModalCtrl'; import './share_snapshot_ctrl'; import './dashboard_srv'; diff --git a/public/app/features/dashboard/dashboard_srv.ts b/public/app/features/dashboard/dashboard_srv.ts index 9d766fdfc3f..3aa7ca118fb 100644 --- a/public/app/features/dashboard/dashboard_srv.ts +++ b/public/app/features/dashboard/dashboard_srv.ts @@ -105,6 +105,10 @@ export class DashboardSrv { this.setCurrent(this.create(clone, this.dash.meta)); } + if (this.dash.meta.provisioned) { + return this.showDashboardProvisionedModal(); + } + if (!this.dash.meta.canSave && options.makeEditable !== true) { return Promise.resolve(); } @@ -120,6 +124,13 @@ export class DashboardSrv { return this.save(this.dash.getSaveModelClone(), options); } + showDashboardProvisionedModal() { + this.$rootScope.appEvent('show-modal', { + templateHtml: '', + modalClass: 'modal--narrow', + }); + } + showSaveAsModal() { this.$rootScope.appEvent('show-modal', { templateHtml: '', diff --git a/public/app/features/dashboard/dashnav/dashnav.html b/public/app/features/dashboard/dashnav/dashnav.html index 269d4b0bada..0c3f949ed7c 100644 --- a/public/app/features/dashboard/dashnav/dashnav.html +++ b/public/app/features/dashboard/dashnav/dashnav.html @@ -17,7 +17,7 @@
- diff --git a/public/app/features/dashboard/save_provisioned_modal.ts b/public/app/features/dashboard/save_provisioned_modal.ts new file mode 100644 index 00000000000..8121248b841 --- /dev/null +++ b/public/app/features/dashboard/save_provisioned_modal.ts @@ -0,0 +1,74 @@ +import coreModule from 'app/core/core_module'; + +const template = ` +
+
+

+ + Cannot save provisioned dashboards +

+ + + + +
+ +
+
+ This dashboard cannot be saved from Grafana's UI since it have been + provisioned from + another source. Please ask your Administrator for more info. +
+
+
+ +
+
+ +
+ + +
+
+
+`; + +export class SaveProvisionedDashboardModalCtrl { + dashboardJson: string; + dismiss: () => void; + + /** @ngInject */ + constructor(dashboardSrv) { + var dashboard = dashboardSrv.getCurrent().getSaveModelClone(); + delete dashboard.id; + this.dashboardJson = JSON.stringify(dashboard); + } + + getJsonForClipboard() { + return this.dashboardJson; + } +} + +export function saveProvisionedDashboardModalDirective() { + return { + restrict: 'E', + template: template, + controller: SaveProvisionedDashboardModalCtrl, + bindToController: true, + controllerAs: 'ctrl', + scope: { dismiss: '&' }, + }; +} + +coreModule.directive('saveProvisionedDashboardModal', saveProvisionedDashboardModalDirective); diff --git a/public/app/features/dashboard/specs/save_provisioned_modal.jest.ts b/public/app/features/dashboard/specs/save_provisioned_modal.jest.ts new file mode 100644 index 00000000000..1a6e1d35562 --- /dev/null +++ b/public/app/features/dashboard/specs/save_provisioned_modal.jest.ts @@ -0,0 +1,28 @@ +import { SaveProvisionedDashboardModalCtrl } from '../save_provisioned_modal'; +import { describe, it, expect } from 'test/lib/common'; + +describe('SaveProvisionedDashboardModalCtrl', () => { + var json = { + title: 'name', + id: 5, + }; + + var mockDashboardSrv = { + getCurrent: function() { + return { + id: 5, + meta: {}, + getSaveModelClone: function() { + return json; + }, + }; + }, + }; + + var ctrl = new SaveProvisionedDashboardModalCtrl(mockDashboardSrv); + + it('verify that the id have been removed', () => { + var copy = ctrl.getJsonForClipboard(); + expect(copy).toBe(`{"title":"name"}`); + }); +}); From 627df67992e58f64e12bdbae1317ca524cbaa1e7 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 27 Mar 2018 15:12:47 +0200 Subject: [PATCH 02/15] dashboards: reject updates of provisioned dashboards --- pkg/api/dashboard.go | 3 +- pkg/api/dashboard_test.go | 1 + pkg/models/dashboards.go | 42 ++++++++++--------- pkg/services/dashboards/dashboard_service.go | 22 ++++++++++ .../dashboards/dashboard_service_test.go | 33 ++++++++++++++- .../sqlstore/dashboard_provisioning.go | 7 +++- .../sqlstore/dashboard_provisioning_test.go | 7 ++++ .../dashboard_service_integration_test.go | 5 ++- 8 files changed, 96 insertions(+), 24 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index e5e4fc560e1..24751896f00 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -235,7 +235,8 @@ func PostDashboard(c *m.ReqContext, cmd m.SaveDashboardCommand) Response { err == m.ErrDashboardWithSameUIDExists || err == m.ErrFolderNotFound || err == m.ErrDashboardFolderCannotHaveParent || - err == m.ErrDashboardFolderNameExists { + err == m.ErrDashboardFolderNameExists || + err == m.ErrDashboardCannotSaveProvisionedDashboard { return Error(400, err.Error(), nil) } diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 0d87023ce40..3d0cb93f0db 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -720,6 +720,7 @@ func TestDashboardApiEndpoint(t *testing.T) { {SaveError: m.ErrDashboardUpdateAccessDenied, ExpectedStatusCode: 403}, {SaveError: m.ErrDashboardInvalidUid, ExpectedStatusCode: 400}, {SaveError: m.ErrDashboardUidToLong, ExpectedStatusCode: 400}, + {SaveError: m.ErrDashboardCannotSaveProvisionedDashboard, ExpectedStatusCode: 400}, {SaveError: m.UpdatePluginDashboardError{PluginId: "plug"}, ExpectedStatusCode: 412}, } diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index e4f0758fc19..1d52246dcc8 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -13,26 +13,28 @@ import ( // Typed errors var ( - ErrDashboardNotFound = errors.New("Dashboard not found") - ErrDashboardFolderNotFound = errors.New("Folder not found") - ErrDashboardSnapshotNotFound = errors.New("Dashboard snapshot not found") - ErrDashboardWithSameUIDExists = errors.New("A dashboard with the same uid already exists") - ErrDashboardWithSameNameInFolderExists = errors.New("A dashboard with the same name in the folder already exists") - ErrDashboardVersionMismatch = errors.New("The dashboard has been changed by someone else") - ErrDashboardTitleEmpty = errors.New("Dashboard title cannot be empty") - ErrDashboardFolderCannotHaveParent = errors.New("A Dashboard Folder cannot be added to another folder") - ErrDashboardContainsInvalidAlertData = errors.New("Invalid alert data. Cannot save dashboard") - ErrDashboardFailedToUpdateAlertData = errors.New("Failed to save alert data") - ErrDashboardsWithSameSlugExists = errors.New("Multiple dashboards with the same slug exists") - ErrDashboardFailedGenerateUniqueUid = errors.New("Failed to generate unique dashboard id") - ErrDashboardTypeMismatch = errors.New("Dashboard cannot be changed to a folder") - ErrDashboardFolderWithSameNameAsDashboard = errors.New("Folder name cannot be the same as one of its dashboards") - ErrDashboardWithSameNameAsFolder = errors.New("Dashboard name cannot be the same as folder") - ErrDashboardFolderNameExists = errors.New("A folder with that name already exists") - ErrDashboardUpdateAccessDenied = errors.New("Access denied to save dashboard") - ErrDashboardInvalidUid = errors.New("uid contains illegal characters") - ErrDashboardUidToLong = errors.New("uid to long. max 40 characters") - RootFolderName = "General" + ErrDashboardNotFound = errors.New("Dashboard not found") + ErrDashboardFolderNotFound = errors.New("Folder not found") + ErrDashboardSnapshotNotFound = errors.New("Dashboard snapshot not found") + ErrDashboardWithSameUIDExists = errors.New("A dashboard with the same uid already exists") + ErrDashboardWithSameNameInFolderExists = errors.New("A dashboard with the same name in the folder already exists") + ErrDashboardVersionMismatch = errors.New("The dashboard has been changed by someone else") + ErrDashboardTitleEmpty = errors.New("Dashboard title cannot be empty") + ErrDashboardFolderCannotHaveParent = errors.New("A Dashboard Folder cannot be added to another folder") + ErrDashboardContainsInvalidAlertData = errors.New("Invalid alert data. Cannot save dashboard") + ErrDashboardFailedToUpdateAlertData = errors.New("Failed to save alert data") + ErrDashboardsWithSameSlugExists = errors.New("Multiple dashboards with the same slug exists") + ErrDashboardFailedGenerateUniqueUid = errors.New("Failed to generate unique dashboard id") + ErrDashboardTypeMismatch = errors.New("Dashboard cannot be changed to a folder") + ErrDashboardFolderWithSameNameAsDashboard = errors.New("Folder name cannot be the same as one of its dashboards") + ErrDashboardWithSameNameAsFolder = errors.New("Dashboard name cannot be the same as folder") + ErrDashboardFolderNameExists = errors.New("A folder with that name already exists") + ErrDashboardUpdateAccessDenied = errors.New("Access denied to save dashboard") + ErrDashboardInvalidUid = errors.New("uid contains illegal characters") + ErrDashboardUidToLong = errors.New("uid to long. max 40 characters") + ErrDashboardCannotSaveProvisionedDashboard = errors.New("Cannot save provisioned dashboards") + ErrDashboardProvisioningDoesNotExist = errors.New("Dashboard provisioning does not exist") + RootFolderName = "General" ) type UpdatePluginDashboardError struct { diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index 02a6ffc8330..7c51c412d39 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -111,6 +111,11 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, return nil, models.ErrDashboardUpdateAccessDenied } + err := dr.validateDashboardIsNotProvisioned(dash.Id) + if err != nil { + return nil, err + } + cmd := &models.SaveDashboardCommand{ Dashboard: dash.Data, Message: dto.Message, @@ -129,6 +134,23 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, return cmd, nil } +func (dr *dashboardServiceImpl) validateDashboardIsNotProvisioned(dashboardId int64) error { + dpQuery := &models.GetProvisionedDashboardByDashboardId{DashboardId: dashboardId} + err := bus.Dispatch(dpQuery) + + // provisioned dashboards cannot be saved. So we can only save + // this dashboard if ErrDashboardProvisioningDoesNotExist is returned + if err != nil && err != models.ErrDashboardProvisioningDoesNotExist { + return err + } + + if err == nil { + return models.ErrDashboardCannotSaveProvisionedDashboard + } + + return nil +} + func (dr *dashboardServiceImpl) updateAlerting(cmd *models.SaveDashboardCommand, dto *SaveDashboardDTO) error { alertCmd := models.UpdateDashboardAlertsCommand{ OrgId: dto.OrgId, diff --git a/pkg/services/dashboards/dashboard_service_test.go b/pkg/services/dashboards/dashboard_service_test.go index 965b10655b3..65c9cabcdc5 100644 --- a/pkg/services/dashboards/dashboard_service_test.go +++ b/pkg/services/dashboards/dashboard_service_test.go @@ -14,7 +14,9 @@ import ( func TestDashboardService(t *testing.T) { Convey("Dashboard service tests", t, func() { - service := dashboardServiceImpl{} + bus.ClearBusHandlers() + + service := &dashboardServiceImpl{} origNewDashboardGuardian := guardian.New guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) @@ -54,6 +56,10 @@ func TestDashboardService(t *testing.T) { return nil }) + bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardByDashboardId) error { + return models.ErrDashboardProvisioningDoesNotExist + }) + testCases := []struct { Uid string Error error @@ -77,7 +83,32 @@ func TestDashboardService(t *testing.T) { } }) + Convey("Should return validation error if dashboard is provisioned", func() { + bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardByDashboardId) error { + cmd.Result = &models.DashboardProvisioning{} + return nil + }) + + bus.AddHandler("test", func(cmd *models.ValidateDashboardAlertsCommand) error { + return nil + }) + + bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + return nil + }) + + dto.Dashboard = models.NewDashboard("Dash") + dto.Dashboard.SetId(3) + dto.User = &models.SignedInUser{UserId: 1} + _, err := service.buildSaveDashboardCommand(dto, false) + So(err, ShouldEqual, models.ErrDashboardCannotSaveProvisionedDashboard) + }) + Convey("Should return validation error if alert data is invalid", func() { + bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardByDashboardId) error { + return models.ErrDashboardProvisioningDoesNotExist + }) + bus.AddHandler("test", func(cmd *models.ValidateDashboardAlertsCommand) error { return errors.New("error") }) diff --git a/pkg/services/sqlstore/dashboard_provisioning.go b/pkg/services/sqlstore/dashboard_provisioning.go index 99178d38f9c..b28b2fa1a53 100644 --- a/pkg/services/sqlstore/dashboard_provisioning.go +++ b/pkg/services/sqlstore/dashboard_provisioning.go @@ -21,12 +21,17 @@ type DashboardExtras struct { func GetProvisionedDataByDashboardId(cmd *models.GetProvisionedDashboardByDashboardId) error { result := &models.DashboardProvisioning{} - _, err := x.Where("dashboard_id = ?", cmd.DashboardId).Get(result) + exist, err := x.Where("dashboard_id = ?", cmd.DashboardId).Get(result) if err != nil { return err } + if !exist { + return models.ErrDashboardProvisioningDoesNotExist + } + cmd.Result = result + return nil } diff --git a/pkg/services/sqlstore/dashboard_provisioning_test.go b/pkg/services/sqlstore/dashboard_provisioning_test.go index 89b3451a3ac..4bafbbff8ca 100644 --- a/pkg/services/sqlstore/dashboard_provisioning_test.go +++ b/pkg/services/sqlstore/dashboard_provisioning_test.go @@ -60,6 +60,13 @@ func TestDashboardProvisioningTest(t *testing.T) { So(query.Result.DashboardId, ShouldEqual, cmd.Result.Id) So(query.Result.Updated, ShouldEqual, now.Unix()) }) + + Convey("Can query for one provisioned dashboard2", func() { + query := &models.GetProvisionedDashboardByDashboardId{DashboardId: 3000} + + err := GetProvisionedDataByDashboardId(query) + So(err, ShouldEqual, models.ErrDashboardProvisioningDoesNotExist) + }) }) }) } diff --git a/pkg/services/sqlstore/dashboard_service_integration_test.go b/pkg/services/sqlstore/dashboard_service_integration_test.go index d005270c33c..75405558412 100644 --- a/pkg/services/sqlstore/dashboard_service_integration_test.go +++ b/pkg/services/sqlstore/dashboard_service_integration_test.go @@ -19,7 +19,6 @@ func TestIntegratedDashboardService(t *testing.T) { var testOrgId int64 = 1 Convey("Given saved folders and dashboards in organization A", func() { - bus.AddHandler("test", func(cmd *models.ValidateDashboardAlertsCommand) error { return nil }) @@ -28,6 +27,10 @@ func TestIntegratedDashboardService(t *testing.T) { return nil }) + bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardByDashboardId) error { + return models.ErrDashboardProvisioningDoesNotExist + }) + savedFolder := saveTestFolder("Saved folder", testOrgId) savedDashInFolder := saveTestDashboard("Saved dash in folder", testOrgId, savedFolder.Id) saveTestDashboard("Other saved dash in folder", testOrgId, savedFolder.Id) From 7f5c2ebdd18e81b3be09bab6411d9a0748b2cb2a Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 27 Mar 2018 15:17:05 +0200 Subject: [PATCH 03/15] provisioning: better description for provisioned save modal --- public/app/features/dashboard/save_provisioned_modal.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/public/app/features/dashboard/save_provisioned_modal.ts b/public/app/features/dashboard/save_provisioned_modal.ts index 8121248b841..80c81b07e3f 100644 --- a/public/app/features/dashboard/save_provisioned_modal.ts +++ b/public/app/features/dashboard/save_provisioned_modal.ts @@ -15,9 +15,10 @@ const template = `
- This dashboard cannot be saved from Grafana's UI since it have been - provisioned from - another source. Please ask your Administrator for more info. + This dashboard cannot be saved from Grafana's UI since it has been provisioned from another source. + + More info about provisioning. +
From bcdb3ec84d434951f7cfc1a656976e1a8d8f6b7f Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 5 Apr 2018 20:28:04 +0200 Subject: [PATCH 04/15] provisioning: ux fixes when saving provisioned dashboards Wider modal. Tried to improve provisioning description text. Code editor for json. Button that allows to save the json to a file. --- .../app/features/dashboard/dashboard_srv.ts | 1 - .../dashboard/save_provisioned_modal.ts | 60 ++++++++++--------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/public/app/features/dashboard/dashboard_srv.ts b/public/app/features/dashboard/dashboard_srv.ts index 3aa7ca118fb..1890323da29 100644 --- a/public/app/features/dashboard/dashboard_srv.ts +++ b/public/app/features/dashboard/dashboard_srv.ts @@ -127,7 +127,6 @@ export class DashboardSrv { showDashboardProvisionedModal() { this.$rootScope.appEvent('show-modal', { templateHtml: '', - modalClass: 'modal--narrow', }); } diff --git a/public/app/features/dashboard/save_provisioned_modal.ts b/public/app/features/dashboard/save_provisioned_modal.ts index 80c81b07e3f..ba96ce0b0b9 100644 --- a/public/app/features/dashboard/save_provisioned_modal.ts +++ b/public/app/features/dashboard/save_provisioned_modal.ts @@ -1,11 +1,12 @@ +import angular from 'angular'; +import { saveAs } from 'file-saver'; import coreModule from 'app/core/core_module'; const template = `

- - Cannot save provisioned dashboards + Cannot save provisioned dashboard

@@ -13,47 +14,48 @@ const template = `
- -
+
+ This dashboard cannot be saved from Grafana's UI since it has been provisioned from another source. - - More info about provisioning. - -
+ Copy the JSON or save it to a file below. Then you can update your dashboard in corresponding provisioning source.
+ See + documentation for more information about provisioning. +
- + +
+
+ + + Cancel
- -
- - -
- +
`; export class SaveProvisionedDashboardModalCtrl { + dash: any; dashboardJson: string; dismiss: () => void; /** @ngInject */ constructor(dashboardSrv) { - var dashboard = dashboardSrv.getCurrent().getSaveModelClone(); - delete dashboard.id; - this.dashboardJson = JSON.stringify(dashboard); + this.dash = dashboardSrv.getCurrent().getSaveModelClone(); + delete this.dash.id; + this.dashboardJson = JSON.stringify(this.dash, null, 2); + } + + save() { + var blob = new Blob([angular.toJson(this.dash, true)], { + type: 'application/json;charset=utf-8', + }); + saveAs(blob, this.dash.title + '-' + new Date().getTime() + '.json'); } getJsonForClipboard() { From f619dd579e56b78dc23e5e9230395c0091b160a9 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 5 Apr 2018 20:51:47 +0200 Subject: [PATCH 05/15] provisioning: fix tests for save provisioned dashboard modal --- .../dashboard/specs/save_provisioned_modal.jest.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/public/app/features/dashboard/specs/save_provisioned_modal.jest.ts b/public/app/features/dashboard/specs/save_provisioned_modal.jest.ts index 1a6e1d35562..ce921cee8c8 100644 --- a/public/app/features/dashboard/specs/save_provisioned_modal.jest.ts +++ b/public/app/features/dashboard/specs/save_provisioned_modal.jest.ts @@ -1,5 +1,4 @@ import { SaveProvisionedDashboardModalCtrl } from '../save_provisioned_modal'; -import { describe, it, expect } from 'test/lib/common'; describe('SaveProvisionedDashboardModalCtrl', () => { var json = { @@ -21,8 +20,11 @@ describe('SaveProvisionedDashboardModalCtrl', () => { var ctrl = new SaveProvisionedDashboardModalCtrl(mockDashboardSrv); - it('verify that the id have been removed', () => { - var copy = ctrl.getJsonForClipboard(); - expect(copy).toBe(`{"title":"name"}`); + it('should remove id from dashboard model', () => { + expect(ctrl.dash.id).toBeUndefined(); + }); + + it('should remove id from dashboard model in clipboard json', () => { + expect(ctrl.getJsonForClipboard()).toBe(JSON.stringify({ title: 'name' }, null, 2)); }); }); From bcbe7a1c55c9ddffcac1b68f6c7a31ce6b232214 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 10 Apr 2018 09:31:35 +0200 Subject: [PATCH 06/15] provisioning: adds error handling --- pkg/api/dashboard.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 24751896f00..b24fb2584c4 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -104,6 +104,10 @@ func GetDashboard(c *m.ReqContext) Response { dpQuery := &m.GetProvisionedDashboardByDashboardId{DashboardId: dash.Id} err = bus.Dispatch(dpQuery) + if err != nil { + return Error(500, "Error while checking if dashboard is provisioned", err) + } + if dpQuery.Result != nil { meta.CanEdit = true meta.Provisioned = true From 7845eab5020dd0a1b5721c877155140dcce35b19 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 10 Apr 2018 09:32:05 +0200 Subject: [PATCH 07/15] provisioning: fixes typo --- pkg/models/dashboards.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 0ed1a038eeb..916adc909ef 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -32,7 +32,7 @@ var ( ErrDashboardUpdateAccessDenied = errors.New("Access denied to save dashboard") ErrDashboardInvalidUid = errors.New("uid contains illegal characters") ErrDashboardUidToLong = errors.New("uid to long. max 40 characters") - ErrDashboardCannotSaveProvisionedDashboard = errors.New("Cannot save provisioned dashboards") + ErrDashboardCannotSaveProvisionedDashboard = errors.New("Cannot save provisioned dashboard") ErrDashboardProvisioningDoesNotExist = errors.New("Dashboard provisioning does not exist") RootFolderName = "General" ) From 77e7887e5e12d3b252d19d458af3a327e9a2ad66 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 10 Apr 2018 09:35:54 +0200 Subject: [PATCH 08/15] provisioning: check provisiong before saveCmd --- pkg/services/dashboards/dashboard_service.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index 7c51c412d39..0f17270419f 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -93,6 +93,11 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, } } + err := dr.validateDashboardIsNotProvisioned(dash.Id) + if err != nil { + return nil, err + } + validateBeforeSaveCmd := models.ValidateDashboardBeforeSaveCommand{ OrgId: dto.OrgId, Dashboard: dash, @@ -111,11 +116,6 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, return nil, models.ErrDashboardUpdateAccessDenied } - err := dr.validateDashboardIsNotProvisioned(dash.Id) - if err != nil { - return nil, err - } - cmd := &models.SaveDashboardCommand{ Dashboard: dash.Data, Message: dto.Message, From 0f9b592428ea6e42adcc89915588bd02165c268a Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 10 Apr 2018 10:48:10 +0200 Subject: [PATCH 09/15] provisioning: fixes broken tests --- pkg/api/dashboard_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 3d0cb93f0db..c84a9707edb 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -42,6 +42,11 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) + bus.AddHandler("test", func(query *m.GetProvisionedDashboardByDashboardId) error { + query.Result = nil + return nil + }) + viewerRole := m.ROLE_VIEWER editorRole := m.ROLE_EDITOR @@ -192,6 +197,11 @@ func TestDashboardApiEndpoint(t *testing.T) { fakeDash.HasAcl = true setting.ViewersCanEdit = false + bus.AddHandler("test", func(query *m.GetProvisionedDashboardByDashboardId) error { + query.Result = nil + return nil + }) + bus.AddHandler("test", func(query *m.GetDashboardsBySlugQuery) error { dashboards := []*m.Dashboard{fakeDash} query.Result = dashboards @@ -625,6 +635,11 @@ func TestDashboardApiEndpoint(t *testing.T) { dashTwo.FolderId = 3 dashTwo.HasAcl = false + bus.AddHandler("test", func(query *m.GetProvisionedDashboardByDashboardId) error { + query.Result = nil + return nil + }) + bus.AddHandler("test", func(query *m.GetDashboardsBySlugQuery) error { dashboards := []*m.Dashboard{dashOne, dashTwo} query.Result = dashboards @@ -751,6 +766,11 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) + bus.AddHandler("test", func(query *m.GetProvisionedDashboardByDashboardId) error { + query.Result = nil + return nil + }) + bus.AddHandler("test", func(query *m.GetDashboardVersionQuery) error { query.Result = &m.DashboardVersion{ Data: simplejson.NewFromAny(map[string]interface{}{ From 949484b949dfab2f0279fa0037fdaea9fcb281cf Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 10 Apr 2018 12:30:37 +0200 Subject: [PATCH 10/15] provisioning: simplify db query the GetProvisionedDashboardQuery wasent used for anything else than check if a dashboard is provisioned or not. So we simplified this query to make it more maintainable. --- pkg/api/dashboard.go | 6 ++--- pkg/api/dashboard_test.go | 16 ++++++------ pkg/models/dashboards.go | 8 +++--- pkg/services/dashboards/dashboard_service.go | 25 ++++++------------- .../dashboards/dashboard_service_test.go | 14 ++++++----- .../sqlstore/dashboard_provisioning.go | 8 ++---- .../sqlstore/dashboard_provisioning_test.go | 12 ++++----- .../dashboard_service_integration_test.go | 5 ++-- 8 files changed, 41 insertions(+), 53 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index b24fb2584c4..6628b7830bc 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -102,13 +102,13 @@ func GetDashboard(c *m.ReqContext) Response { meta.FolderUrl = query.Result.GetUrl() } - dpQuery := &m.GetProvisionedDashboardByDashboardId{DashboardId: dash.Id} - err = bus.Dispatch(dpQuery) + isDashboardProvisioned := &m.IsDashboardProvisionedQuery{DashboardId: dash.Id} + err = bus.Dispatch(isDashboardProvisioned) if err != nil { return Error(500, "Error while checking if dashboard is provisioned", err) } - if dpQuery.Result != nil { + if isDashboardProvisioned.Result { meta.CanEdit = true meta.Provisioned = true } diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index c84a9707edb..ccde2382787 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -42,8 +42,8 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) - bus.AddHandler("test", func(query *m.GetProvisionedDashboardByDashboardId) error { - query.Result = nil + bus.AddHandler("test", func(query *m.IsDashboardProvisionedQuery) error { + query.Result = false return nil }) @@ -197,8 +197,8 @@ func TestDashboardApiEndpoint(t *testing.T) { fakeDash.HasAcl = true setting.ViewersCanEdit = false - bus.AddHandler("test", func(query *m.GetProvisionedDashboardByDashboardId) error { - query.Result = nil + bus.AddHandler("test", func(query *m.IsDashboardProvisionedQuery) error { + query.Result = false return nil }) @@ -635,8 +635,8 @@ func TestDashboardApiEndpoint(t *testing.T) { dashTwo.FolderId = 3 dashTwo.HasAcl = false - bus.AddHandler("test", func(query *m.GetProvisionedDashboardByDashboardId) error { - query.Result = nil + bus.AddHandler("test", func(query *m.IsDashboardProvisionedQuery) error { + query.Result = false return nil }) @@ -766,8 +766,8 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) - bus.AddHandler("test", func(query *m.GetProvisionedDashboardByDashboardId) error { - query.Result = nil + bus.AddHandler("test", func(query *m.IsDashboardProvisionedQuery) error { + query.Result = false return nil }) diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 916adc909ef..9fbcd57b88a 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -33,8 +33,8 @@ var ( ErrDashboardInvalidUid = errors.New("uid contains illegal characters") ErrDashboardUidToLong = errors.New("uid to long. max 40 characters") ErrDashboardCannotSaveProvisionedDashboard = errors.New("Cannot save provisioned dashboard") - ErrDashboardProvisioningDoesNotExist = errors.New("Dashboard provisioning does not exist") - RootFolderName = "General" + //ErrDashboardProvisioningDoesNotExist = errors.New("Dashboard provisioning does not exist") + RootFolderName = "General" ) type UpdatePluginDashboardError struct { @@ -319,10 +319,10 @@ type GetDashboardSlugByIdQuery struct { Result string } -type GetProvisionedDashboardByDashboardId struct { +type IsDashboardProvisionedQuery struct { DashboardId int64 - Result *DashboardProvisioning + Result bool } type GetProvisionedDashboardDataQuery struct { diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index 0f17270419f..ad93509eef3 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -93,11 +93,17 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, } } - err := dr.validateDashboardIsNotProvisioned(dash.Id) + isDashboardProvisioned := &models.IsDashboardProvisionedQuery{DashboardId: dash.Id} + err := bus.Dispatch(isDashboardProvisioned) + if err != nil { return nil, err } + if isDashboardProvisioned.Result { + return nil, models.ErrDashboardCannotSaveProvisionedDashboard + } + validateBeforeSaveCmd := models.ValidateDashboardBeforeSaveCommand{ OrgId: dto.OrgId, Dashboard: dash, @@ -134,23 +140,6 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, return cmd, nil } -func (dr *dashboardServiceImpl) validateDashboardIsNotProvisioned(dashboardId int64) error { - dpQuery := &models.GetProvisionedDashboardByDashboardId{DashboardId: dashboardId} - err := bus.Dispatch(dpQuery) - - // provisioned dashboards cannot be saved. So we can only save - // this dashboard if ErrDashboardProvisioningDoesNotExist is returned - if err != nil && err != models.ErrDashboardProvisioningDoesNotExist { - return err - } - - if err == nil { - return models.ErrDashboardCannotSaveProvisionedDashboard - } - - return nil -} - func (dr *dashboardServiceImpl) updateAlerting(cmd *models.SaveDashboardCommand, dto *SaveDashboardDTO) error { alertCmd := models.UpdateDashboardAlertsCommand{ OrgId: dto.OrgId, diff --git a/pkg/services/dashboards/dashboard_service_test.go b/pkg/services/dashboards/dashboard_service_test.go index 65c9cabcdc5..0db9fe82b60 100644 --- a/pkg/services/dashboards/dashboard_service_test.go +++ b/pkg/services/dashboards/dashboard_service_test.go @@ -56,8 +56,9 @@ func TestDashboardService(t *testing.T) { return nil }) - bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardByDashboardId) error { - return models.ErrDashboardProvisioningDoesNotExist + bus.AddHandler("test", func(cmd *models.IsDashboardProvisionedQuery) error { + cmd.Result = false + return nil }) testCases := []struct { @@ -84,8 +85,8 @@ func TestDashboardService(t *testing.T) { }) Convey("Should return validation error if dashboard is provisioned", func() { - bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardByDashboardId) error { - cmd.Result = &models.DashboardProvisioning{} + bus.AddHandler("test", func(cmd *models.IsDashboardProvisionedQuery) error { + cmd.Result = true return nil }) @@ -105,8 +106,9 @@ func TestDashboardService(t *testing.T) { }) Convey("Should return validation error if alert data is invalid", func() { - bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardByDashboardId) error { - return models.ErrDashboardProvisioningDoesNotExist + bus.AddHandler("test", func(cmd *models.IsDashboardProvisionedQuery) error { + cmd.Result = false + return nil }) bus.AddHandler("test", func(cmd *models.ValidateDashboardAlertsCommand) error { diff --git a/pkg/services/sqlstore/dashboard_provisioning.go b/pkg/services/sqlstore/dashboard_provisioning.go index b28b2fa1a53..33fbb01c5b7 100644 --- a/pkg/services/sqlstore/dashboard_provisioning.go +++ b/pkg/services/sqlstore/dashboard_provisioning.go @@ -18,7 +18,7 @@ type DashboardExtras struct { Value string } -func GetProvisionedDataByDashboardId(cmd *models.GetProvisionedDashboardByDashboardId) error { +func GetProvisionedDataByDashboardId(cmd *models.IsDashboardProvisionedQuery) error { result := &models.DashboardProvisioning{} exist, err := x.Where("dashboard_id = ?", cmd.DashboardId).Get(result) @@ -26,11 +26,7 @@ func GetProvisionedDataByDashboardId(cmd *models.GetProvisionedDashboardByDashbo return err } - if !exist { - return models.ErrDashboardProvisioningDoesNotExist - } - - cmd.Result = result + cmd.Result = exist return nil } diff --git a/pkg/services/sqlstore/dashboard_provisioning_test.go b/pkg/services/sqlstore/dashboard_provisioning_test.go index 4bafbbff8ca..7ef45df3152 100644 --- a/pkg/services/sqlstore/dashboard_provisioning_test.go +++ b/pkg/services/sqlstore/dashboard_provisioning_test.go @@ -52,20 +52,20 @@ func TestDashboardProvisioningTest(t *testing.T) { }) Convey("Can query for one provisioned dashboard", func() { - query := &models.GetProvisionedDashboardByDashboardId{DashboardId: cmd.Result.Id} + query := &models.IsDashboardProvisionedQuery{DashboardId: cmd.Result.Id} err := GetProvisionedDataByDashboardId(query) So(err, ShouldBeNil) - So(query.Result.DashboardId, ShouldEqual, cmd.Result.Id) - So(query.Result.Updated, ShouldEqual, now.Unix()) + So(query.Result, ShouldBeTrue) }) - Convey("Can query for one provisioned dashboard2", func() { - query := &models.GetProvisionedDashboardByDashboardId{DashboardId: 3000} + Convey("Can query for none provisioned dashboard", func() { + query := &models.IsDashboardProvisionedQuery{DashboardId: 3000} err := GetProvisionedDataByDashboardId(query) - So(err, ShouldEqual, models.ErrDashboardProvisioningDoesNotExist) + So(err, ShouldBeNil) + So(query.Result, ShouldBeFalse) }) }) }) diff --git a/pkg/services/sqlstore/dashboard_service_integration_test.go b/pkg/services/sqlstore/dashboard_service_integration_test.go index 75405558412..bbc9128d692 100644 --- a/pkg/services/sqlstore/dashboard_service_integration_test.go +++ b/pkg/services/sqlstore/dashboard_service_integration_test.go @@ -27,8 +27,9 @@ func TestIntegratedDashboardService(t *testing.T) { return nil }) - bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardByDashboardId) error { - return models.ErrDashboardProvisioningDoesNotExist + bus.AddHandler("test", func(cmd *models.IsDashboardProvisionedQuery) error { + cmd.Result = false + return nil }) savedFolder := saveTestFolder("Saved folder", testOrgId) From f424d13b53c38a7c5447907cc83f6a4a6fc2e7e3 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 10 Apr 2018 12:35:06 +0200 Subject: [PATCH 11/15] provisioning: dont override permissions --- pkg/api/dashboard.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 6628b7830bc..c2ab6dd9a1a 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -109,7 +109,6 @@ func GetDashboard(c *m.ReqContext) Response { } if isDashboardProvisioned.Result { - meta.CanEdit = true meta.Provisioned = true } From 4f44376564c904a834d612d6a3dcc49816cf9c5c Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 13 Apr 2018 17:11:52 +0200 Subject: [PATCH 12/15] remove comment/unused variable --- pkg/models/dashboards.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 9fbcd57b88a..90242857182 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -33,8 +33,7 @@ var ( ErrDashboardInvalidUid = errors.New("uid contains illegal characters") ErrDashboardUidToLong = errors.New("uid to long. max 40 characters") ErrDashboardCannotSaveProvisionedDashboard = errors.New("Cannot save provisioned dashboard") - //ErrDashboardProvisioningDoesNotExist = errors.New("Dashboard provisioning does not exist") - RootFolderName = "General" + RootFolderName = "General" ) type UpdatePluginDashboardError struct { From 79d8937ce537e09540028aa16ed0749912f9fbc6 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 13 Apr 2018 17:45:32 +0200 Subject: [PATCH 13/15] provisioned dashboard validation should not be made from provisioning service Provisioned dashboard validation should not be made when creating/updating a folder or when provisioning service are trying to provision/create/update dashboard --- pkg/services/dashboards/dashboard_service.go | 26 +++++------ .../dashboards/dashboard_service_test.go | 43 ++++++++++++++++++- pkg/services/dashboards/folder_service.go | 4 +- .../dashboards/folder_service_test.go | 9 ++++ 4 files changed, 66 insertions(+), 16 deletions(-) diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index ad93509eef3..c48215ebaed 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -57,7 +57,7 @@ func (dr *dashboardServiceImpl) GetProvisionedDashboardData(name string) ([]*mod return cmd.Result, nil } -func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, validateAlerts bool) (*models.SaveDashboardCommand, error) { +func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, validateAlerts bool, validateProvisionedDashboard bool) (*models.SaveDashboardCommand, error) { dash := dto.Dashboard dash.Title = strings.TrimSpace(dash.Title) @@ -93,15 +93,17 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, } } - isDashboardProvisioned := &models.IsDashboardProvisionedQuery{DashboardId: dash.Id} - err := bus.Dispatch(isDashboardProvisioned) + if validateProvisionedDashboard { + isDashboardProvisioned := &models.IsDashboardProvisionedQuery{DashboardId: dash.Id} + err := bus.Dispatch(isDashboardProvisioned) - if err != nil { - return nil, err - } + if err != nil { + return nil, err + } - if isDashboardProvisioned.Result { - return nil, models.ErrDashboardCannotSaveProvisionedDashboard + if isDashboardProvisioned.Result { + return nil, models.ErrDashboardCannotSaveProvisionedDashboard + } } validateBeforeSaveCmd := models.ValidateDashboardBeforeSaveCommand{ @@ -159,7 +161,7 @@ func (dr *dashboardServiceImpl) SaveProvisionedDashboard(dto *SaveDashboardDTO, UserId: 0, OrgRole: models.ROLE_ADMIN, } - cmd, err := dr.buildSaveDashboardCommand(dto, true) + cmd, err := dr.buildSaveDashboardCommand(dto, true, false) if err != nil { return nil, err } @@ -189,7 +191,7 @@ func (dr *dashboardServiceImpl) SaveFolderForProvisionedDashboards(dto *SaveDash UserId: 0, OrgRole: models.ROLE_ADMIN, } - cmd, err := dr.buildSaveDashboardCommand(dto, false) + cmd, err := dr.buildSaveDashboardCommand(dto, false, false) if err != nil { return nil, err } @@ -208,7 +210,7 @@ func (dr *dashboardServiceImpl) SaveFolderForProvisionedDashboards(dto *SaveDash } func (dr *dashboardServiceImpl) SaveDashboard(dto *SaveDashboardDTO) (*models.Dashboard, error) { - cmd, err := dr.buildSaveDashboardCommand(dto, true) + cmd, err := dr.buildSaveDashboardCommand(dto, true, true) if err != nil { return nil, err } @@ -227,7 +229,7 @@ func (dr *dashboardServiceImpl) SaveDashboard(dto *SaveDashboardDTO) (*models.Da } func (dr *dashboardServiceImpl) ImportDashboard(dto *SaveDashboardDTO) (*models.Dashboard, error) { - cmd, err := dr.buildSaveDashboardCommand(dto, false) + cmd, err := dr.buildSaveDashboardCommand(dto, false, true) if err != nil { return nil, err } diff --git a/pkg/services/dashboards/dashboard_service_test.go b/pkg/services/dashboards/dashboard_service_test.go index 0db9fe82b60..c6b5a45e977 100644 --- a/pkg/services/dashboards/dashboard_service_test.go +++ b/pkg/services/dashboards/dashboard_service_test.go @@ -79,13 +79,15 @@ func TestDashboardService(t *testing.T) { dto.Dashboard.SetUid(tc.Uid) dto.User = &models.SignedInUser{} - _, err := service.buildSaveDashboardCommand(dto, true) + _, err := service.buildSaveDashboardCommand(dto, true, false) So(err, ShouldEqual, tc.Error) } }) Convey("Should return validation error if dashboard is provisioned", func() { + provisioningValidated := false bus.AddHandler("test", func(cmd *models.IsDashboardProvisionedQuery) error { + provisioningValidated = true cmd.Result = true return nil }) @@ -101,7 +103,8 @@ func TestDashboardService(t *testing.T) { dto.Dashboard = models.NewDashboard("Dash") dto.Dashboard.SetId(3) dto.User = &models.SignedInUser{UserId: 1} - _, err := service.buildSaveDashboardCommand(dto, false) + _, err := service.SaveDashboard(dto) + So(provisioningValidated, ShouldBeTrue) So(err, ShouldEqual, models.ErrDashboardCannotSaveProvisionedDashboard) }) @@ -121,6 +124,42 @@ func TestDashboardService(t *testing.T) { }) }) + Convey("Save provisioned dashboard validation", func() { + dto := &SaveDashboardDTO{} + + Convey("Should not return validation error if dashboard is provisioned", func() { + provisioningValidated := false + bus.AddHandler("test", func(cmd *models.IsDashboardProvisionedQuery) error { + provisioningValidated = true + cmd.Result = true + return nil + }) + + bus.AddHandler("test", func(cmd *models.ValidateDashboardAlertsCommand) error { + return nil + }) + + bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + return nil + }) + + bus.AddHandler("test", func(cmd *models.SaveProvisionedDashboardCommand) error { + return nil + }) + + bus.AddHandler("test", func(cmd *models.UpdateDashboardAlertsCommand) error { + return nil + }) + + dto.Dashboard = models.NewDashboard("Dash") + dto.Dashboard.SetId(3) + dto.User = &models.SignedInUser{UserId: 1} + _, err := service.SaveProvisionedDashboard(dto, nil) + So(err, ShouldBeNil) + So(provisioningValidated, ShouldBeFalse) + }) + }) + Reset(func() { guardian.New = origNewDashboardGuardian }) diff --git a/pkg/services/dashboards/folder_service.go b/pkg/services/dashboards/folder_service.go index ae92952056e..b521b0e5213 100644 --- a/pkg/services/dashboards/folder_service.go +++ b/pkg/services/dashboards/folder_service.go @@ -104,7 +104,7 @@ func (dr *dashboardServiceImpl) CreateFolder(cmd *models.CreateFolderCommand) er User: dr.user, } - saveDashboardCmd, err := dr.buildSaveDashboardCommand(dto, false) + saveDashboardCmd, err := dr.buildSaveDashboardCommand(dto, false, false) if err != nil { return toFolderError(err) } @@ -141,7 +141,7 @@ func (dr *dashboardServiceImpl) UpdateFolder(existingUid string, cmd *models.Upd Overwrite: cmd.Overwrite, } - saveDashboardCmd, err := dr.buildSaveDashboardCommand(dto, false) + saveDashboardCmd, err := dr.buildSaveDashboardCommand(dto, false, false) if err != nil { return toFolderError(err) } diff --git a/pkg/services/dashboards/folder_service_test.go b/pkg/services/dashboards/folder_service_test.go index 6c0413d1878..a488b41434f 100644 --- a/pkg/services/dashboards/folder_service_test.go +++ b/pkg/services/dashboards/folder_service_test.go @@ -108,11 +108,19 @@ func TestFolderService(t *testing.T) { return nil }) + provisioningValidated := false + + bus.AddHandler("test", func(query *models.IsDashboardProvisionedQuery) error { + provisioningValidated = true + return nil + }) + Convey("When creating folder should not return access denied error", func() { err := service.CreateFolder(&models.CreateFolderCommand{ Title: "Folder", }) So(err, ShouldBeNil) + So(provisioningValidated, ShouldBeFalse) }) Convey("When updating folder should not return access denied error", func() { @@ -121,6 +129,7 @@ func TestFolderService(t *testing.T) { Title: "Folder", }) So(err, ShouldBeNil) + So(provisioningValidated, ShouldBeFalse) }) Convey("When deleting folder by uid should not return access denied error", func() { From ff4e38c1d858842b876b67cdba3cbb4e86eeeba7 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 13 Apr 2018 17:54:41 +0200 Subject: [PATCH 14/15] provisioned dashboard validation should be made when importing a dashboard Had to add the validate before save command before validating provisioned dashboard so that the import would get a proper dashboard id --- pkg/services/dashboards/dashboard_service.go | 20 +++++------ .../dashboards/dashboard_service_test.go | 36 +++++++++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index c48215ebaed..ba4c44fc49f 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -93,6 +93,16 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, } } + validateBeforeSaveCmd := models.ValidateDashboardBeforeSaveCommand{ + OrgId: dto.OrgId, + Dashboard: dash, + Overwrite: dto.Overwrite, + } + + if err := bus.Dispatch(&validateBeforeSaveCmd); err != nil { + return nil, err + } + if validateProvisionedDashboard { isDashboardProvisioned := &models.IsDashboardProvisionedQuery{DashboardId: dash.Id} err := bus.Dispatch(isDashboardProvisioned) @@ -106,16 +116,6 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, } } - validateBeforeSaveCmd := models.ValidateDashboardBeforeSaveCommand{ - OrgId: dto.OrgId, - Dashboard: dash, - Overwrite: dto.Overwrite, - } - - if err := bus.Dispatch(&validateBeforeSaveCmd); err != nil { - return nil, err - } - guard := guardian.New(dash.GetDashboardIdForSavePermissionCheck(), dto.OrgId, dto.User) if canSave, err := guard.CanSave(); err != nil || !canSave { if err != nil { diff --git a/pkg/services/dashboards/dashboard_service_test.go b/pkg/services/dashboards/dashboard_service_test.go index c6b5a45e977..2ade42facbe 100644 --- a/pkg/services/dashboards/dashboard_service_test.go +++ b/pkg/services/dashboards/dashboard_service_test.go @@ -160,6 +160,42 @@ func TestDashboardService(t *testing.T) { }) }) + Convey("Import dashboard validation", func() { + dto := &SaveDashboardDTO{} + + Convey("Should return validation error if dashboard is provisioned", func() { + provisioningValidated := false + bus.AddHandler("test", func(cmd *models.IsDashboardProvisionedQuery) error { + provisioningValidated = true + cmd.Result = true + return nil + }) + + bus.AddHandler("test", func(cmd *models.ValidateDashboardAlertsCommand) error { + return nil + }) + + bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + return nil + }) + + bus.AddHandler("test", func(cmd *models.SaveProvisionedDashboardCommand) error { + return nil + }) + + bus.AddHandler("test", func(cmd *models.UpdateDashboardAlertsCommand) error { + return nil + }) + + dto.Dashboard = models.NewDashboard("Dash") + dto.Dashboard.SetId(3) + dto.User = &models.SignedInUser{UserId: 1} + _, err := service.ImportDashboard(dto) + So(provisioningValidated, ShouldBeTrue) + So(err, ShouldEqual, models.ErrDashboardCannotSaveProvisionedDashboard) + }) + }) + Reset(func() { guardian.New = origNewDashboardGuardian }) From 2fc1558daf86d3c786398b4e4503fb0c677510d0 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 17 Apr 2018 11:03:46 +0200 Subject: [PATCH 15/15] revert changes of add panel button to require save permission This resolves an issue where the add panel button was shown on the default home dashboard --- public/app/features/dashboard/dashnav/dashnav.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/features/dashboard/dashnav/dashnav.html b/public/app/features/dashboard/dashnav/dashnav.html index 0c3f949ed7c..269d4b0bada 100644 --- a/public/app/features/dashboard/dashnav/dashnav.html +++ b/public/app/features/dashboard/dashnav/dashnav.html @@ -17,7 +17,7 @@
-