From d03007de01ef4e1234e1250dff3d56f73436a639 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Fri, 2 Jul 2021 18:19:08 +0200 Subject: [PATCH] Dashboards: Add IsFolder field into models.GetDashboardQuery (#36214) (#36388) * Add IsFolder field into models.GetDashboardQuery * Reverted folderId - return dummy success when calling get folder with id 0 * Moved condition to upper level - add test (cherry picked from commit 084c9c8746a01a9da7f6ee73a79b9e476088f9cf) Co-authored-by: Dimitris Sotirakis --- pkg/services/dashboards/folder_service.go | 3 + .../dashboards/folder_service_test.go | 75 ++++++++++--------- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/pkg/services/dashboards/folder_service.go b/pkg/services/dashboards/folder_service.go index 4d3e1299362..79cdbfaa32b 100644 --- a/pkg/services/dashboards/folder_service.go +++ b/pkg/services/dashboards/folder_service.go @@ -61,6 +61,9 @@ func (dr *dashboardServiceImpl) GetFolders(limit int64) ([]*models.Folder, error } func (dr *dashboardServiceImpl) GetFolderByID(id int64) (*models.Folder, error) { + if id == 0 { + return &models.Folder{Id: id, Title: "General"}, nil + } query := models.GetDashboardQuery{OrgId: dr.orgId, Id: id} dashFolder, err := getFolder(query) if err != nil { diff --git a/pkg/services/dashboards/folder_service_test.go b/pkg/services/dashboards/folder_service_test.go index 9853a2665b0..49818d26806 100644 --- a/pkg/services/dashboards/folder_service_test.go +++ b/pkg/services/dashboards/folder_service_test.go @@ -7,21 +7,20 @@ import ( "github.com/grafana/grafana/pkg/dashboards" "github.com/grafana/grafana/pkg/models" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/services/guardian" - - . "github.com/smartystreets/goconvey/convey" ) func TestFolderService(t *testing.T) { - Convey("Folder service tests", t, func() { + t.Run("Folder service tests", func(t *testing.T) { service := dashboardServiceImpl{ orgId: 1, user: &models.SignedInUser{UserId: 1}, dashboardStore: &fakeDashboardStore{}, } - Convey("Given user has no permissions", func() { + t.Run("Given user has no permissions", func(t *testing.T) { origNewGuardian := guardian.New guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{}) @@ -38,41 +37,47 @@ func TestFolderService(t *testing.T) { validationError: models.ErrDashboardUpdateAccessDenied, } - Convey("When get folder by id should return access denied error", func() { + t.Run("When get folder by id should return access denied error", func(t *testing.T) { _, err := service.GetFolderByID(1) - So(err, ShouldEqual, models.ErrFolderAccessDenied) + require.Equal(t, err, models.ErrFolderAccessDenied) }) - Convey("When get folder by uid should return access denied error", func() { + t.Run("When get folder by id, with id = 0 should return default folder", func(t *testing.T) { + folder, err := service.GetFolderByID(0) + require.NoError(t, err) + require.Equal(t, folder, &models.Folder{Id: 0, Title: "General"}) + }) + + t.Run("When get folder by uid should return access denied error", func(t *testing.T) { _, err := service.GetFolderByUID("uid") - So(err, ShouldEqual, models.ErrFolderAccessDenied) + require.Equal(t, err, models.ErrFolderAccessDenied) }) - Convey("When creating folder should return access denied error", func() { + t.Run("When creating folder should return access denied error", func(t *testing.T) { _, err := service.CreateFolder("Folder", "") - So(err, ShouldEqual, models.ErrFolderAccessDenied) + require.Equal(t, err, models.ErrFolderAccessDenied) }) - Convey("When updating folder should return access denied error", func() { + t.Run("When updating folder should return access denied error", func(t *testing.T) { err := service.UpdateFolder("uid", &models.UpdateFolderCommand{ Uid: "uid", Title: "Folder", }) - So(err, ShouldEqual, models.ErrFolderAccessDenied) + require.Equal(t, err, models.ErrFolderAccessDenied) }) - Convey("When deleting folder by uid should return access denied error", func() { + t.Run("When deleting folder by uid should return access denied error", func(t *testing.T) { _, err := service.DeleteFolder("uid") - So(err, ShouldNotBeNil) - So(err, ShouldEqual, models.ErrFolderAccessDenied) + require.Error(t, err) + require.Equal(t, err, models.ErrFolderAccessDenied) }) - Reset(func() { + t.Cleanup(func() { guardian.New = origNewGuardian }) }) - Convey("Given user has permission to save", func() { + t.Run("Given user has permission to save", func(t *testing.T) { origNewGuardian := guardian.New guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) @@ -102,30 +107,30 @@ func TestFolderService(t *testing.T) { return nil }) - Convey("When creating folder should not return access denied error", func() { + t.Run("When creating folder should not return access denied error", func(t *testing.T) { _, err := service.CreateFolder("Folder", "") - So(err, ShouldBeNil) + require.NoError(t, err) }) - Convey("When updating folder should not return access denied error", func() { + t.Run("When updating folder should not return access denied error", func(t *testing.T) { err := service.UpdateFolder("uid", &models.UpdateFolderCommand{ Uid: "uid", Title: "Folder", }) - So(err, ShouldBeNil) + require.NoError(t, err) }) - Convey("When deleting folder by uid should not return access denied error", func() { + t.Run("When deleting folder by uid should not return access denied error", func(t *testing.T) { _, err := service.DeleteFolder("uid") - So(err, ShouldBeNil) + require.NoError(t, err) }) - Reset(func() { + t.Cleanup(func() { guardian.New = origNewGuardian }) }) - Convey("Given user has permission to view", func() { + t.Run("Given user has permission to view", func(t *testing.T) { origNewGuardian := guardian.New guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: true}) @@ -138,26 +143,26 @@ func TestFolderService(t *testing.T) { return nil }) - Convey("When get folder by id should return folder", func() { + t.Run("When get folder by id should return folder", func(t *testing.T) { f, _ := service.GetFolderByID(1) - So(f.Id, ShouldEqual, dashFolder.Id) - So(f.Uid, ShouldEqual, dashFolder.Uid) - So(f.Title, ShouldEqual, dashFolder.Title) + require.Equal(t, f.Id, dashFolder.Id) + require.Equal(t, f.Uid, dashFolder.Uid) + require.Equal(t, f.Title, dashFolder.Title) }) - Convey("When get folder by uid should return folder", func() { + t.Run("When get folder by uid should return folder", func(t *testing.T) { f, _ := service.GetFolderByUID("uid") - So(f.Id, ShouldEqual, dashFolder.Id) - So(f.Uid, ShouldEqual, dashFolder.Uid) - So(f.Title, ShouldEqual, dashFolder.Title) + require.Equal(t, f.Id, dashFolder.Id) + require.Equal(t, f.Uid, dashFolder.Uid) + require.Equal(t, f.Title, dashFolder.Title) }) - Reset(func() { + t.Cleanup(func() { guardian.New = origNewGuardian }) }) - Convey("Should map errors correct", func() { + t.Run("Should map errors correct", func(t *testing.T) { testCases := []struct { ActualError error ExpectedError error