From 55b014974de049110e0c88fa8d82c4c34e1e1124 Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Tue, 20 Dec 2022 15:00:33 +0200 Subject: [PATCH] Nested folders: Refactor folder update (#60323) * Nested folders: Refactor folder update * Apply suggestions from code review --- pkg/api/folder.go | 10 +- pkg/api/folder_test.go | 12 ++- pkg/kindsys/report.go | 10 +- pkg/models/folders.go | 32 ------ pkg/services/folder/folderimpl/folder.go | 101 ++++++++++++------ pkg/services/folder/folderimpl/folder_test.go | 61 +++++++++-- pkg/services/folder/folderimpl/sqlstore.go | 37 ++++--- .../folder/folderimpl/sqlstore_test.go | 24 +++-- pkg/services/folder/folderimpl/store_fake.go | 24 ++--- pkg/services/folder/foldertest/foldertest.go | 5 +- pkg/services/folder/model.go | 18 +++- pkg/services/folder/service.go | 5 +- public/api-merged.json | 14 ++- public/api-spec.json | 14 ++- public/openapi3.json | 48 +++++++-- 15 files changed, 267 insertions(+), 148 deletions(-) diff --git a/pkg/api/folder.go b/pkg/api/folder.go index c72c9e22b63..9166e20f915 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -201,11 +201,15 @@ func (hs *HTTPServer) MoveFolder(c *models.ReqContext) response.Response { // 409: conflictError // 500: internalServerError func (hs *HTTPServer) UpdateFolder(c *models.ReqContext) response.Response { - cmd := models.UpdateFolderCommand{} + cmd := folder.UpdateFolderCommand{} if err := web.Bind(c.Req, &cmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } - result, err := hs.folderService.Update(c.Req.Context(), c.SignedInUser, c.OrgID, web.Params(c.Req)[":uid"], &cmd) + + cmd.OrgID = c.OrgID + cmd.UID = web.Params(c.Req)[":uid"] + cmd.SignedInUser = c.SignedInUser + result, err := hs.folderService.Update(c.Req.Context(), &cmd) if err != nil { return apierrors.ToFolderErrorResponse(err) } @@ -320,7 +324,7 @@ type UpdateFolderParams struct { // // in:body // required:true - Body models.UpdateFolderCommand `json:"body"` + Body folder.UpdateFolderCommand `json:"body"` } // swagger:parameters getFolderByID diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go index 4de90331a16..7ff1decbd26 100644 --- a/pkg/api/folder_test.go +++ b/pkg/api/folder_test.go @@ -90,8 +90,9 @@ func TestFoldersAPIEndpoint(t *testing.T) { }) t.Run("Given a correct request for updating a folder", func(t *testing.T) { - cmd := models.UpdateFolderCommand{ - Title: "Folder upd", + title := "Folder upd" + cmd := folder.UpdateFolderCommand{ + NewTitle: &title, } folderService.ExpectedFolder = &folder.Folder{ID: 1, UID: "uid", Title: "Folder upd"} @@ -125,8 +126,9 @@ func TestFoldersAPIEndpoint(t *testing.T) { {Error: dashboards.ErrFolderFailedGenerateUniqueUid, ExpectedStatusCode: 500}, } - cmd := models.UpdateFolderCommand{ - Title: "Folder upd", + title := "Folder upd" + cmd := folder.UpdateFolderCommand{ + NewTitle: &title, } for _, tc := range testCases { @@ -278,7 +280,7 @@ func callUpdateFolder(sc *scenarioContext) { } func updateFolderScenario(t *testing.T, desc string, url string, routePattern string, folderService folder.Service, - cmd models.UpdateFolderCommand, fn scenarioFunc) { + cmd folder.UpdateFolderCommand, fn scenarioFunc) { setUpRBACGuardian(t) t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { hs := HTTPServer{ diff --git a/pkg/kindsys/report.go b/pkg/kindsys/report.go index 9303a654717..cc01aa78e40 100644 --- a/pkg/kindsys/report.go +++ b/pkg/kindsys/report.go @@ -39,7 +39,7 @@ func main() { // static list of planned core kinds so that we can inject ones that // haven't been started on yet as "planned" -var plannedCoreKinds = []string { +var plannedCoreKinds = []string{ "Dashboard", "Playlist", "Team", @@ -88,11 +88,11 @@ func buildKindStateReport() KindStateReport { } r.Core = append(r.Core, kindsys.CoreStructuredProperties{ CommonProperties: kindsys.CommonProperties{ - Name: kn, - PluralName: kn + "s", - MachineName: machinize(kn), + Name: kn, + PluralName: kn + "s", + MachineName: machinize(kn), PluralMachineName: machinize(kn) + "s", - Maturity: "planned", + Maturity: "planned", }, }) } diff --git a/pkg/models/folders.go b/pkg/models/folders.go index 64b348a34d0..c19471b37b7 100644 --- a/pkg/models/folders.go +++ b/pkg/models/folders.go @@ -1,7 +1,6 @@ package models import ( - "strings" "time" "github.com/grafana/grafana/pkg/services/user" @@ -47,27 +46,6 @@ func DashboardToFolder(dash *Dashboard) *Folder { } } -// UpdateDashboardModel updates an existing model from command into model for update -func (cmd *UpdateFolderCommand) UpdateDashboardModel(dashFolder *Dashboard, orgId int64, userId int64) { - dashFolder.OrgId = orgId - dashFolder.Title = strings.TrimSpace(cmd.Title) - dashFolder.Data.Set("title", dashFolder.Title) - - if cmd.Uid != "" { - dashFolder.SetUid(cmd.Uid) - } - - dashFolder.SetVersion(cmd.Version) - dashFolder.IsFolder = true - - if userId == 0 { - userId = -1 - } - - dashFolder.UpdatedBy = userId - dashFolder.UpdateSlug() -} - // // COMMANDS // @@ -83,16 +61,6 @@ type MoveFolderCommand struct { ParentUID *string `json:"parentUid"` } -type UpdateFolderCommand struct { - Uid string `json:"uid"` - Title string `json:"title"` - Version int `json:"version"` - Description string `json:"description"` - Overwrite bool `json:"overwrite"` - - Result *Folder `json:"-"` -} - // // QUERIES // diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index e4970114a8c..9cbef310ecb 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -366,33 +366,33 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) ( return f, nil } -func (s *Service) Update(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*folder.Folder, error) { - foldr, err := s.legacyUpdate(ctx, user, orgID, existingUid, cmd) +func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) { + if cmd.SignedInUser == nil { + return nil, folder.ErrBadRequest.Errorf("missing signed in user") + } + user := cmd.SignedInUser + + foldr, err := s.legacyUpdate(ctx, cmd) if err != nil { return nil, err } if s.features.IsEnabled(featuremgmt.FlagNestedFolders) { - if cmd.Uid != "" { - if !util.IsValidShortUID(cmd.Uid) { + if cmd.NewUID != nil && *cmd.NewUID != "" { + if !util.IsValidShortUID(*cmd.NewUID) { return nil, dashboards.ErrDashboardInvalidUid - } else if util.IsShortUIDTooLong(cmd.Uid) { + } else if util.IsShortUIDTooLong(*cmd.NewUID) { return nil, dashboards.ErrDashboardUidTooLong } } - getFolder, err := s.store.Get(ctx, folder.GetFolderQuery{ - UID: &existingUid, - OrgID: orgID, - }) - if err != nil { - return nil, err - } foldr, err := s.store.Update(ctx, folder.UpdateFolderCommand{ - Folder: getFolder, - NewUID: &cmd.Uid, - NewTitle: &cmd.Title, - NewDescription: &cmd.Description, + UID: cmd.UID, + OrgID: cmd.OrgID, + NewUID: cmd.NewUID, + NewTitle: cmd.NewTitle, + NewDescription: cmd.NewDescription, + SignedInUser: user, }) if err != nil { return nil, err @@ -402,10 +402,10 @@ func (s *Service) Update(ctx context.Context, user *user.SignedInUser, orgID int return foldr, nil } -func (s *Service) legacyUpdate(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*folder.Folder, error) { +func (s *Service) legacyUpdate(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) { logger := s.log.FromContext(ctx) - query := models.GetDashboardQuery{OrgId: orgID, Uid: existingUid} + query := models.GetDashboardQuery{OrgId: cmd.OrgID, Uid: cmd.UID} _, err := s.dashboardStore.GetDashboard(ctx, &query) if err != nil { return nil, toFolderError(err) @@ -418,12 +418,17 @@ func (s *Service) legacyUpdate(ctx context.Context, user *user.SignedInUser, org return nil, dashboards.ErrFolderNotFound } - cmd.UpdateDashboardModel(dashFolder, orgID, user.UserID) + if cmd.SignedInUser == nil { + return nil, folder.ErrBadRequest.Errorf("missing signed in user") + } + user := cmd.SignedInUser + + prepareForUpdate(dashFolder, cmd.OrgID, cmd.SignedInUser.UserID, cmd) dto := &dashboards.SaveDashboardDTO{ Dashboard: dashFolder, - OrgId: orgID, - User: user, + OrgId: cmd.OrgID, + User: cmd.SignedInUser, Overwrite: cmd.Overwrite, } @@ -438,7 +443,7 @@ func (s *Service) legacyUpdate(ctx context.Context, user *user.SignedInUser, org } var foldr *folder.Folder - foldr, err = s.dashboardStore.GetFolderByID(ctx, orgID, dash.Id) + foldr, err = s.dashboardStore.GetFolderByID(ctx, cmd.OrgID, dash.Id) if err != nil { return nil, err } @@ -449,7 +454,7 @@ func (s *Service) legacyUpdate(ctx context.Context, user *user.SignedInUser, org Title: foldr.Title, ID: dash.Id, UID: dash.Uid, - OrgID: orgID, + OrgID: cmd.OrgID, }); err != nil { logger.Error("failed to publish FolderTitleUpdated event", "folder", foldr.Title, "user", user.UserID, "error", err) } @@ -457,6 +462,32 @@ func (s *Service) legacyUpdate(ctx context.Context, user *user.SignedInUser, org return foldr, nil } +// prepareForUpdate updates an existing dashboard model from command into model for folder update +func prepareForUpdate(dashFolder *models.Dashboard, orgId int64, userId int64, cmd *folder.UpdateFolderCommand) { + dashFolder.OrgId = orgId + + title := dashFolder.Title + if cmd.NewTitle != nil && *cmd.NewTitle != "" { + title = *cmd.NewTitle + } + dashFolder.Title = strings.TrimSpace(title) + dashFolder.Data.Set("title", dashFolder.Title) + + if cmd.NewUID != nil && *cmd.NewUID != "" { + dashFolder.SetUid(*cmd.NewUID) + } + + dashFolder.SetVersion(cmd.Version) + dashFolder.IsFolder = true + + if userId == 0 { + userId = -1 + } + + dashFolder.UpdatedBy = userId + dashFolder.UpdateSlug() +} + func (s *Service) DeleteFolder(ctx context.Context, cmd *folder.DeleteFolderCommand) error { logger := s.log.FromContext(ctx) if cmd.SignedInUser == nil { @@ -501,23 +532,20 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol return nil, folder.ErrBadRequest.Errorf("missing signed in user") } - foldr, err := s.Get(ctx, &folder.GetFolderQuery{ - UID: &cmd.UID, - OrgID: cmd.OrgID, - SignedInUser: cmd.SignedInUser, - }) + g, err := guardian.NewByUID(ctx, cmd.UID, cmd.OrgID, cmd.SignedInUser) if err != nil { return nil, err } - - // if the new parent is the same as the current parent, we don't need to do anything - if foldr.ParentUID == cmd.NewParentUID { - return foldr, nil + if canSave, err := g.CanSave(); err != nil || !canSave { + if err != nil { + return nil, toFolderError(err) + } + return nil, dashboards.ErrFolderAccessDenied } // here we get the folder, we need to get the height of current folder // and the depth of the new parent folder, the sum can't bypass 8 - folderHeight, err := s.store.GetHeight(ctx, foldr.UID, cmd.OrgID, &cmd.NewParentUID) + folderHeight, err := s.store.GetHeight(ctx, cmd.UID, cmd.OrgID, &cmd.NewParentUID) if err != nil { return nil, err } @@ -533,13 +561,16 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol // if the current folder is already a parent of newparent, we should return error for _, parent := range parents { - if parent.UID == foldr.UID { + if parent.UID == cmd.UID { return nil, folder.ErrCircularReference } } return s.store.Update(ctx, folder.UpdateFolderCommand{ - Folder: foldr, + UID: cmd.UID, + OrgID: cmd.OrgID, + NewParentUID: &cmd.NewParentUID, + SignedInUser: cmd.SignedInUser, }) } diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index d337d732377..45c4047addf 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "github.com/xorcare/pointer" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" @@ -96,9 +95,10 @@ func TestIntegrationFolderService(t *testing.T) { require.Equal(t, err, dashboards.ErrFolderAccessDenied) }) + var zeroInt int64 = 0 t.Run("When get folder by id, with id = 0 should return default folder", func(t *testing.T) { foldr, err := service.Get(context.Background(), &folder.GetFolderQuery{ - ID: pointer.Int64(0), + ID: &zeroInt, OrgID: orgID, SignedInUser: usr, }) @@ -126,15 +126,18 @@ func TestIntegrationFolderService(t *testing.T) { require.Equal(t, err, dashboards.ErrFolderAccessDenied) }) + title := "Folder-TEST" t.Run("When updating folder should return access denied error", func(t *testing.T) { dashStore.On("GetDashboard", mock.Anything, mock.AnythingOfType("*models.GetDashboardQuery")).Run(func(args mock.Arguments) { folder := args.Get(1).(*models.GetDashboardQuery) folder.Result = models.NewDashboard("dashboard-test") folder.Result.IsFolder = true }).Return(&models.Dashboard{}, nil) - _, err := service.Update(context.Background(), usr, orgID, folderUID, &models.UpdateFolderCommand{ - Uid: folderUID, - Title: "Folder-TEST", + _, err := service.Update(context.Background(), &folder.UpdateFolderCommand{ + UID: folderUID, + OrgID: orgID, + NewTitle: &title, + SignedInUser: usr, }) require.Equal(t, err, dashboards.ErrFolderAccessDenied) }) @@ -207,12 +210,15 @@ func TestIntegrationFolderService(t *testing.T) { dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("models.SaveDashboardCommand")).Return(dashboardFolder, nil) dashStore.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.Id).Return(f, nil) - req := &models.UpdateFolderCommand{ - Uid: dashboardFolder.Uid, - Title: "TEST-Folder", + title := "TEST-Folder" + req := &folder.UpdateFolderCommand{ + UID: dashboardFolder.Uid, + OrgID: orgID, + NewTitle: &title, + SignedInUser: usr, } - reqResult, err := service.Update(context.Background(), usr, orgID, dashboardFolder.Uid, req) + reqResult, err := service.Update(context.Background(), req) require.NoError(t, err) require.Equal(t, f, reqResult) }) @@ -393,7 +399,7 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) { func TestNestedFolderService(t *testing.T) { t.Run("with feature flag unset", func(t *testing.T) { - store := &FakeStore{} + store := NewFakeStore() dashStore := dashboards.FakeDashboardStore{} dashboardsvc := dashboards.FakeDashboardService{} // nothing enabled yet @@ -449,7 +455,7 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("with nested folder feature flag on", func(t *testing.T) { - store := &FakeStore{} + store := NewFakeStore() dashStore := &dashboards.FakeDashboardStore{} dashboardsvc := &dashboards.FakeDashboardService{} // nothing enabled yet @@ -583,6 +589,34 @@ func TestNestedFolderService(t *testing.T) { }) }) + t.Run("move, no view permission should fail", func(t *testing.T) { + // This test creates and deletes the dashboard, so needs some extra setup. + g := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: false}) + t.Cleanup(func() { + guardian.New = g + }) + + store.ExpectedError = nil + store.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"} + _, err := foldersvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr}) + require.Error(t, err, dashboards.ErrFolderAccessDenied) + }) + + t.Run("move, no save permission should fail", func(t *testing.T) { + // This test creates and deletes the dashboard, so needs some extra setup. + g := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: false, CanViewValue: true}) + t.Cleanup(func() { + guardian.New = g + }) + + store.ExpectedError = nil + store.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"} + _, err := foldersvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr}) + require.Error(t, err, dashboards.ErrFolderAccessDenied) + }) + t.Run("move, no error", func(t *testing.T) { // This test creates and deletes the dashboard, so needs some extra setup. g := guardian.New @@ -593,6 +627,11 @@ func TestNestedFolderService(t *testing.T) { store.ExpectedError = nil store.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"} + store.ExpectedParentFolders = []*folder.Folder{ + {UID: "newFolder", ParentUID: "newFolder"}, + {UID: "newFolder2", ParentUID: "newFolder2"}, + {UID: "newFolder3", ParentUID: "newFolder3"}, + } f, err := foldersvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr}) require.NoError(t, err) require.NotNil(t, f) diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index 81358e3234c..91e9f057a16 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -91,33 +91,34 @@ func (ss *sqlStore) Delete(ctx context.Context, uid string, orgID int64) error { } func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) { - if cmd.Folder == nil { - return nil, folder.ErrBadRequest.Errorf("invalid update command: missing folder") - } + updated := time.Now() + uid := cmd.UID - cmd.Folder.Updated = time.Now() - existingUID := cmd.Folder.UID + var foldr *folder.Folder err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { sql := strings.Builder{} sql.Write([]byte("UPDATE folder SET ")) columnsToUpdate := []string{"updated = ?"} - args := []interface{}{cmd.Folder.Updated} + args := []interface{}{updated} if cmd.NewDescription != nil { columnsToUpdate = append(columnsToUpdate, "description = ?") - cmd.Folder.Description = *cmd.NewDescription - args = append(args, cmd.Folder.Description) + args = append(args, *cmd.NewDescription) } if cmd.NewTitle != nil { columnsToUpdate = append(columnsToUpdate, "title = ?") - cmd.Folder.Title = *cmd.NewTitle - args = append(args, cmd.Folder.Title) + args = append(args, *cmd.NewTitle) } if cmd.NewUID != nil { columnsToUpdate = append(columnsToUpdate, "uid = ?") - cmd.Folder.UID = *cmd.NewUID - args = append(args, cmd.Folder.UID) + uid = *cmd.NewUID + args = append(args, *cmd.NewUID) + } + + if cmd.NewParentUID != nil { + columnsToUpdate = append(columnsToUpdate, "parent_uid = ?") + args = append(args, *cmd.NewParentUID) } if len(columnsToUpdate) == 0 { @@ -126,7 +127,7 @@ func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) sql.Write([]byte(strings.Join(columnsToUpdate, ", "))) sql.Write([]byte(" WHERE uid = ? AND org_id = ?")) - args = append(args, existingUID, cmd.Folder.OrgID) + args = append(args, cmd.UID, cmd.OrgID) args = append([]interface{}{sql.String()}, args...) @@ -142,10 +143,18 @@ func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) if affected == 0 { return folder.ErrInternal.Errorf("no folders are updated") } + + foldr, err = ss.Get(ctx, folder.GetFolderQuery{ + UID: &uid, + OrgID: cmd.OrgID, + }) + if err != nil { + return err + } return nil }) - return cmd.Folder, err + return foldr, err } func (ss *sqlStore) Get(ctx context.Context, q folder.GetFolderQuery) (*folder.Folder, error) { diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index 05843c3df62..f844e525220 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" @@ -156,7 +155,7 @@ func TestIntegrationDelete(t *testing.T) { }) */ - ancestorUIDs := CreateSubTree(t, folderStore, orgID, accesscontrol.GeneralFolderUID, folder.MaxNestedFolderDepth, "") + ancestorUIDs := CreateSubTree(t, folderStore, orgID, "", folder.MaxNestedFolderDepth, "") require.Len(t, ancestorUIDs, folder.MaxNestedFolderDepth) t.Cleanup(func() { @@ -235,9 +234,7 @@ func TestIntegrationUpdate(t *testing.T) { _, err = folderStore.Update(context.Background(), folder.UpdateFolderCommand{}) require.Error(t, err) - _, err = folderStore.Update(context.Background(), folder.UpdateFolderCommand{ - Folder: &folder.Folder{}, - }) + _, err = folderStore.Update(context.Background(), folder.UpdateFolderCommand{}) require.Error(t, err) }) @@ -246,7 +243,8 @@ func TestIntegrationUpdate(t *testing.T) { newDesc := "new desc" // existingUpdated := f.Updated updated, err := folderStore.Update(context.Background(), folder.UpdateFolderCommand{ - Folder: f, + UID: f.UID, + OrgID: f.OrgID, NewTitle: &newTitle, NewDescription: &newDesc, }) @@ -264,6 +262,8 @@ func TestIntegrationUpdate(t *testing.T) { require.NoError(t, err) assert.Equal(t, newTitle, updated.Title) assert.Equal(t, newDesc, updated.Description) + + f = updated }) t.Run("updating folder UID should succeed", func(t *testing.T) { @@ -271,7 +271,8 @@ func TestIntegrationUpdate(t *testing.T) { existingTitle := f.Title existingDesc := f.Description updated, err := folderStore.Update(context.Background(), folder.UpdateFolderCommand{ - Folder: f, + UID: f.UID, + OrgID: f.OrgID, NewUID: &newUID, }) require.NoError(t, err) @@ -641,13 +642,16 @@ func CreateOrg(t *testing.T, db *sqlstore.SQLStore) int64 { func CreateSubTree(t *testing.T, store *sqlStore, orgID int64, parentUID string, depth int, prefix string) []string { t.Helper() - ancestorUIDs := []string{parentUID} + ancestorUIDs := []string{} + if parentUID != "" { + ancestorUIDs = append(ancestorUIDs, parentUID) + } for i := 0; i < depth; i++ { title := fmt.Sprintf("%sfolder-%d", prefix, i) cmd := folder.CreateFolderCommand{ Title: title, OrgID: orgID, - ParentUID: ancestorUIDs[len(ancestorUIDs)-1], + ParentUID: parentUID, UID: util.GenerateShortUID(), } f, err := store.Create(context.Background(), cmd) @@ -668,6 +672,8 @@ func CreateSubTree(t *testing.T, store *sqlStore, orgID int64, parentUID string, require.Equal(t, ancestorUIDs, parentUIDs) ancestorUIDs = append(ancestorUIDs, f.UID) + + parentUID = f.UID } return ancestorUIDs diff --git a/pkg/services/folder/folderimpl/store_fake.go b/pkg/services/folder/folderimpl/store_fake.go index 1810b1f18d4..9a8a47a4ae8 100644 --- a/pkg/services/folder/folderimpl/store_fake.go +++ b/pkg/services/folder/folderimpl/store_fake.go @@ -6,7 +6,7 @@ import ( "github.com/grafana/grafana/pkg/services/folder" ) -type FakeStore struct { +type fakeStore struct { ExpectedChildFolders []*folder.Folder ExpectedParentFolders []*folder.Folder ExpectedFolder *folder.Folder @@ -16,42 +16,42 @@ type FakeStore struct { DeleteCalled bool } -func NewFakeStore() *FakeStore { - return &FakeStore{} +func NewFakeStore() *fakeStore { + return &fakeStore{} } -var _ store = (*FakeStore)(nil) +var _ store = (*fakeStore)(nil) -func (f *FakeStore) Create(ctx context.Context, cmd folder.CreateFolderCommand) (*folder.Folder, error) { +func (f *fakeStore) Create(ctx context.Context, cmd folder.CreateFolderCommand) (*folder.Folder, error) { f.CreateCalled = true return f.ExpectedFolder, f.ExpectedError } -func (f *FakeStore) Delete(ctx context.Context, uid string, orgID int64) error { +func (f *fakeStore) Delete(ctx context.Context, uid string, orgID int64) error { f.DeleteCalled = true return f.ExpectedError } -func (f *FakeStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) { +func (f *fakeStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) { return f.ExpectedFolder, f.ExpectedError } -func (f *FakeStore) Move(ctx context.Context, cmd folder.MoveFolderCommand) error { +func (f *fakeStore) Move(ctx context.Context, cmd folder.MoveFolderCommand) error { return f.ExpectedError } -func (f *FakeStore) Get(ctx context.Context, cmd folder.GetFolderQuery) (*folder.Folder, error) { +func (f *fakeStore) Get(ctx context.Context, cmd folder.GetFolderQuery) (*folder.Folder, error) { return f.ExpectedFolder, f.ExpectedError } -func (f *FakeStore) GetParents(ctx context.Context, cmd folder.GetParentsQuery) ([]*folder.Folder, error) { +func (f *fakeStore) GetParents(ctx context.Context, cmd folder.GetParentsQuery) ([]*folder.Folder, error) { return f.ExpectedParentFolders, f.ExpectedError } -func (f *FakeStore) GetChildren(ctx context.Context, cmd folder.GetChildrenQuery) ([]*folder.Folder, error) { +func (f *fakeStore) GetChildren(ctx context.Context, cmd folder.GetChildrenQuery) ([]*folder.Folder, error) { return f.ExpectedChildFolders, f.ExpectedError } -func (f *FakeStore) GetHeight(ctx context.Context, folderUID string, orgID int64, parentUID *string) (int, error) { +func (f *fakeStore) GetHeight(ctx context.Context, folderUID string, orgID int64, parentUID *string) (int, error) { return f.ExpectedFolderHeight, f.ExpectedError } diff --git a/pkg/services/folder/foldertest/foldertest.go b/pkg/services/folder/foldertest/foldertest.go index 7380ccb531b..638e59e55ea 100644 --- a/pkg/services/folder/foldertest/foldertest.go +++ b/pkg/services/folder/foldertest/foldertest.go @@ -3,9 +3,7 @@ package foldertest import ( "context" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/folder" - "github.com/grafana/grafana/pkg/services/user" ) type FakeService struct { @@ -29,8 +27,7 @@ func (s *FakeService) Create(ctx context.Context, cmd *folder.CreateFolderComman func (s *FakeService) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.Folder, error) { return s.ExpectedFolder, s.ExpectedError } -func (s *FakeService) Update(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*folder.Folder, error) { - cmd.Result = s.ExpectedFolder.ToLegacyModel() +func (s *FakeService) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) { return s.ExpectedFolder, s.ExpectedError } func (s *FakeService) DeleteFolder(ctx context.Context, cmd *folder.DeleteFolderCommand) error { diff --git a/pkg/services/folder/model.go b/pkg/services/folder/model.go index dae038f4012..3c026425744 100644 --- a/pkg/services/folder/model.go +++ b/pkg/services/folder/model.go @@ -76,10 +76,20 @@ type CreateFolderCommand struct { // UpdateFolderCommand captures the information required by the folder service // to update a folder. Use Move to update a folder's parent folder. type UpdateFolderCommand struct { - Folder *Folder `json:"folder"` // The extant folder - NewUID *string `json:"uid" xorm:"uid"` - NewTitle *string `json:"title"` - NewDescription *string `json:"description"` + UID string `json:"-"` + OrgID int64 `json:"-"` + // NewUID it's an optional parameter used for overriding the existing folder UID + NewUID *string `json:"uid"` // keep same json tag with the legacy command for not breaking the existing APIs + // NewTitle it's an optional parameter used for overriding the existing folder title + NewTitle *string `json:"title"` // keep same json tag with the legacy command for not breaking the existing APIs + // NewDescription it's an optional parameter used for overriding the existing folder description + NewDescription *string `json:"description"` // keep same json tag with the legacy command for not breaking the existing APIs + NewParentUID *string `json:"-"` + + // Version only used by the legacy folder implementation + Version int `json:"version"` + // Overwrite only used by the legacy folder implementation + Overwrite bool `json:"overwrite"` SignedInUser *user.SignedInUser `json:"-"` } diff --git a/pkg/services/folder/service.go b/pkg/services/folder/service.go index 760232335cf..97b70c56f61 100644 --- a/pkg/services/folder/service.go +++ b/pkg/services/folder/service.go @@ -2,9 +2,6 @@ package folder import ( "context" - - "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/user" ) type Service interface { @@ -20,7 +17,7 @@ type Service interface { // Update is used to update a folder's UID, Title and Description. To change // a folder's parent folder, use Move. - Update(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*Folder, error) + Update(ctx context.Context, cmd *UpdateFolderCommand) (*Folder, error) DeleteFolder(ctx context.Context, cmd *DeleteFolderCommand) error MakeUserAdmin(ctx context.Context, orgID int64, userID, folderID int64, setViewAndEditPermissions bool) error // Move changes a folder's parent folder to the requested new parent. diff --git a/public/api-merged.json b/public/api-merged.json index aadd50031fa..b0f5e2826a3 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -16553,7 +16553,7 @@ } } }, - "SearchServiceAccountsResult": { + "SearchOrgServiceAccountsResult": { "description": "swagger: model", "type": "object", "properties": { @@ -17864,21 +17864,27 @@ } }, "UpdateFolderCommand": { + "description": "UpdateFolderCommand captures the information required by the folder service\nto update a folder. Use Move to update a folder's parent folder.", "type": "object", "properties": { "description": { + "description": "NewDescription it's an optional parameter used for overriding the existing folder description", "type": "string" }, "overwrite": { + "description": "Overwrite only used by the legacy folder implementation", "type": "boolean" }, "title": { + "description": "NewTitle it's an optional parameter used for overriding the existing folder title", "type": "string" }, "uid": { + "description": "NewUID it's an optional parameter used for overriding the existing folder UID", "type": "string" }, "version": { + "description": "Version only used by the legacy folder implementation", "type": "integer", "format": "int64" } @@ -18049,6 +18055,10 @@ "Editor", "Admin" ] + }, + "serviceAccountId": { + "type": "integer", + "format": "int64" } } }, @@ -20106,7 +20116,7 @@ "searchOrgServiceAccountsWithPagingResponse": { "description": "(empty)", "schema": { - "$ref": "#/definitions/SearchServiceAccountsResult" + "$ref": "#/definitions/SearchOrgServiceAccountsResult" } }, "searchOrgsResponse": { diff --git a/public/api-spec.json b/public/api-spec.json index 5114eaa198b..1144cf60bec 100644 --- a/public/api-spec.json +++ b/public/api-spec.json @@ -13618,7 +13618,7 @@ } } }, - "SearchServiceAccountsResult": { + "SearchOrgServiceAccountsResult": { "description": "swagger: model", "type": "object", "properties": { @@ -14493,21 +14493,27 @@ } }, "UpdateFolderCommand": { + "description": "UpdateFolderCommand captures the information required by the folder service\nto update a folder. Use Move to update a folder's parent folder.", "type": "object", "properties": { "description": { + "description": "NewDescription it's an optional parameter used for overriding the existing folder description", "type": "string" }, "overwrite": { + "description": "Overwrite only used by the legacy folder implementation", "type": "boolean" }, "title": { + "description": "NewTitle it's an optional parameter used for overriding the existing folder title", "type": "string" }, "uid": { + "description": "NewUID it's an optional parameter used for overriding the existing folder UID", "type": "string" }, "version": { + "description": "Version only used by the legacy folder implementation", "type": "integer", "format": "int64" } @@ -14678,6 +14684,10 @@ "Editor", "Admin" ] + }, + "serviceAccountId": { + "type": "integer", + "format": "int64" } } }, @@ -16047,7 +16057,7 @@ "searchOrgServiceAccountsWithPagingResponse": { "description": "", "schema": { - "$ref": "#/definitions/SearchServiceAccountsResult" + "$ref": "#/definitions/SearchOrgServiceAccountsResult" } }, "searchOrgsResponse": { diff --git a/public/openapi3.json b/public/openapi3.json index f66dd7e8f50..1a1661abe8b 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -1581,7 +1581,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/SearchServiceAccountsResult" + "$ref": "#/components/schemas/SearchOrgServiceAccountsResult" } } }, @@ -6955,6 +6955,12 @@ ], "type": "object" }, + "ProvisionedAlertRules": { + "items": { + "$ref": "#/components/schemas/ProvisionedAlertRule" + }, + "type": "array" + }, "PushoverConfig": { "properties": { "expire": { @@ -7837,7 +7843,7 @@ }, "type": "object" }, - "SearchServiceAccountsResult": { + "SearchOrgServiceAccountsResult": { "description": "swagger: model", "properties": { "page": { @@ -8914,7 +8920,6 @@ "type": "string" }, "URL": { - "description": "The general form represented is:\n\n[scheme:][//[userinfo@]host][/]path[?query][#fragment]\n\nURLs that do not start with a slash after the scheme are interpreted as:\n\nscheme:opaque[?query][#fragment]\n\nNote that the Path field is stored in decoded form: /%47%6f%2f becomes /Go/.\nA consequence is that it is impossible to tell which slashes in the Path were\nslashes in the raw URL and which were %2f. This distinction is rarely important,\nbut when it is, the code should use RawPath, an optional field which only gets\nset if the default encoding is different from Path.\n\nURL's String method uses the EscapedPath method to obtain the path. See the\nEscapedPath method for more details.", "properties": { "ForceQuery": { "type": "boolean" @@ -8925,6 +8930,9 @@ "Host": { "type": "string" }, + "OmitHost": { + "type": "boolean" + }, "Opaque": { "type": "string" }, @@ -8947,7 +8955,7 @@ "$ref": "#/components/schemas/Userinfo" } }, - "title": "A URL represents a parsed URL (technically, a URI reference).", + "title": "URL is a custom URL type that allows validation at configuration load time.", "type": "object" }, "UpdateAlertNotificationCommand": { @@ -9145,20 +9153,26 @@ "type": "object" }, "UpdateFolderCommand": { + "description": "UpdateFolderCommand captures the information required by the folder service\nto update a folder. Use Move to update a folder's parent folder.", "properties": { "description": { + "description": "NewDescription it's an optional parameter used for overriding the existing folder description", "type": "string" }, "overwrite": { + "description": "Overwrite only used by the legacy folder implementation", "type": "boolean" }, "title": { + "description": "NewTitle it's an optional parameter used for overriding the existing folder title", "type": "string" }, "uid": { + "description": "NewUID it's an optional parameter used for overriding the existing folder UID", "type": "string" }, "version": { + "description": "Version only used by the legacy folder implementation", "format": "int64", "type": "integer" } @@ -9329,6 +9343,10 @@ "Admin" ], "type": "string" + }, + "serviceAccountId": { + "format": "int64", + "type": "integer" } }, "type": "object" @@ -9851,6 +9869,7 @@ "type": "object" }, "gettableAlert": { + "description": "GettableAlert gettable alert", "properties": { "annotations": { "$ref": "#/components/schemas/labelSet" @@ -9969,7 +9988,6 @@ "type": "array" }, "integration": { - "description": "Integration integration", "properties": { "lastNotifyAttempt": { "description": "A timestamp indicating the last attempt to deliver a notification regardless of the outcome.\nFormat: date-time", @@ -10113,7 +10131,6 @@ "type": "array" }, "postableSilence": { - "description": "PostableSilence postable silence", "properties": { "comment": { "description": "comment", @@ -12888,6 +12905,25 @@ } }, "/api/v1/provisioning/alert-rules": { + "get": { + "operationId": "RouteGetAlertRules", + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProvisionedAlertRules" + } + } + }, + "description": "ProvisionedAlertRules" + } + }, + "summary": "Get all the alert rules.", + "tags": [ + "provisioning" + ] + }, "post": { "operationId": "RoutePostAlertRule", "parameters": [