diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index c3e61c980f7..2dae51cd872 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -543,10 +543,14 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol } } + newParentUID := "" + if cmd.NewParentUID != "" { + newParentUID = cmd.NewParentUID + } return s.store.Update(ctx, folder.UpdateFolderCommand{ UID: cmd.UID, OrgID: cmd.OrgID, - NewParentUID: &cmd.NewParentUID, + NewParentUID: &newParentUID, SignedInUser: cmd.SignedInUser, }) } diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index 98f06cf650f..12691f5d056 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -92,6 +92,10 @@ func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) uid := cmd.UID var foldr *folder.Folder + + if cmd.NewDescription == nil && cmd.NewTitle == nil && cmd.NewUID == nil && cmd.NewParentUID == nil { + return nil, folder.ErrBadRequest.Errorf("nothing to update") + } err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { sql := strings.Builder{} sql.Write([]byte("UPDATE folder SET ")) @@ -114,8 +118,12 @@ func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) } if cmd.NewParentUID != nil { - columnsToUpdate = append(columnsToUpdate, "parent_uid = ?") - args = append(args, *cmd.NewParentUID) + if *cmd.NewParentUID == "" { + columnsToUpdate = append(columnsToUpdate, "parent_uid = NULL") + } else { + columnsToUpdate = append(columnsToUpdate, "parent_uid = ?") + args = append(args, *cmd.NewParentUID) + } } if len(columnsToUpdate) == 0 { diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index 7899afd6a6a..d5a032dd3d6 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -27,7 +27,7 @@ func TestIntegrationCreate(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, &featuremgmt.FeatureManager{}) + folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) orgID := CreateOrg(t, db) @@ -142,7 +142,7 @@ func TestIntegrationDelete(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, &featuremgmt.FeatureManager{}) + folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) orgID := CreateOrg(t, db) @@ -189,18 +189,30 @@ func TestIntegrationUpdate(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, &featuremgmt.FeatureManager{}) + folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) orgID := CreateOrg(t, db) - // create folder - f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + // create parent folder + parent, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ Title: folderTitle, Description: folderDsc, OrgID: orgID, UID: util.GenerateShortUID(), }) require.NoError(t, err) + + // create subfolder + f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: folderTitle, + Description: folderDsc, + OrgID: orgID, + UID: util.GenerateShortUID(), + ParentUID: parent.UID, + }) + + require.NoError(t, err) + require.Equal(t, f.ParentUID, parent.UID) t.Cleanup(func() { err := folderStore.Delete(context.Background(), f.UID, orgID) require.NoError(t, err) @@ -250,6 +262,7 @@ func TestIntegrationUpdate(t *testing.T) { assert.Equal(t, f.UID, updated.UID) assert.Equal(t, newTitle, updated.Title) assert.Equal(t, newDesc, updated.Description) + assert.Equal(t, parent.UID, updated.ParentUID) // assert.GreaterOrEqual(t, updated.Updated.UnixNano(), existingUpdated.UnixNano()) updated, err = folderStore.Get(context.Background(), folder.GetFolderQuery{ @@ -259,6 +272,8 @@ func TestIntegrationUpdate(t *testing.T) { require.NoError(t, err) assert.Equal(t, newTitle, updated.Title) assert.Equal(t, newDesc, updated.Description) + // parent should not change + assert.Equal(t, parent.UID, updated.ParentUID) f = updated }) @@ -285,6 +300,82 @@ func TestIntegrationUpdate(t *testing.T) { assert.Equal(t, existingTitle, updated.Title) assert.Equal(t, existingDesc, updated.Description) }) + + t.Run("updating folder parent UID", func(t *testing.T) { + testCases := []struct { + desc string + reqNewParentUID *string + expectedError error + expectedParentUIDFunc func(existing string) string + }{ + { + desc: "should succeed when moving to other folder", + reqNewParentUID: util.Pointer("new"), + expectedParentUIDFunc: func(_ string) string { return "new" }, + }, + { + desc: "should succeed when moving to root folder (NewParentUID is empty)", + reqNewParentUID: util.Pointer(""), + expectedParentUIDFunc: func(_ string) string { return "" }, + }, + { + desc: "should do nothing when NewParentUID is nil", + reqNewParentUID: nil, + expectedError: folder.ErrBadRequest, + expectedParentUIDFunc: func(existingParentUID string) string { return existingParentUID }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + // create parent folder + parentUID := util.GenerateShortUID() + _, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "parent", + Description: "parent", + OrgID: orgID, + UID: parentUID, + }) + require.NoError(t, err) + + // create subfolder + UID := util.GenerateShortUID() + f, err = folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "subfolder", + Description: "subfolder", + OrgID: orgID, + UID: UID, + ParentUID: parentUID, + }) + require.NoError(t, err) + + existingTitle := f.Title + existingDesc := f.Description + existingUID := f.UID + updated, err := folderStore.Update(context.Background(), folder.UpdateFolderCommand{ + UID: f.UID, + OrgID: f.OrgID, + NewParentUID: tc.reqNewParentUID, + }) + if tc.expectedError == nil { + require.NoError(t, err) + assert.Equal(t, tc.expectedParentUIDFunc(parentUID), updated.ParentUID) + } else { + assert.ErrorIs(t, err, tc.expectedError) + } + + updated, err = folderStore.Get(context.Background(), folder.GetFolderQuery{ + UID: &f.UID, + OrgID: orgID, + }) + require.NoError(t, err) + assert.Equal(t, tc.expectedParentUIDFunc(parentUID), updated.ParentUID) + assert.Equal(t, existingTitle, updated.Title) + assert.Equal(t, existingDesc, updated.Description) + assert.Equal(t, existingUID, updated.UID) + }) + } + }) } func TestIntegrationGet(t *testing.T) { @@ -293,7 +384,7 @@ func TestIntegrationGet(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, &featuremgmt.FeatureManager{}) + folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) orgID := CreateOrg(t, db) @@ -371,7 +462,7 @@ func TestIntegrationGetParents(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, &featuremgmt.FeatureManager{}) + folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) orgID := CreateOrg(t, db) @@ -390,7 +481,7 @@ func TestIntegrationGetParents(t *testing.T) { require.NoError(t, err) }) - t.Run("get parents of root folder should should be empty", func(t *testing.T) { + t.Run("get parents of root folder should be empty", func(t *testing.T) { parents, err := folderStore.GetParents(context.Background(), folder.GetParentsQuery{}) require.NoError(t, err) require.Empty(t, parents) @@ -438,7 +529,7 @@ func TestIntegrationGetChildren(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, &featuremgmt.FeatureManager{}) + folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) orgID := CreateOrg(t, db) @@ -591,7 +682,7 @@ func TestIntegrationGetHeight(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, &featuremgmt.FeatureManager{}) + folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) orgID := CreateOrg(t, db) diff --git a/pkg/services/folder/folderimpl/store.go b/pkg/services/folder/folderimpl/store.go index 6155c58bb51..a286391a887 100644 --- a/pkg/services/folder/folderimpl/store.go +++ b/pkg/services/folder/folderimpl/store.go @@ -14,8 +14,10 @@ type store interface { // Delete deletes a folder from the folder store. Delete(ctx context.Context, uid string, orgID int64) error - // Update updates the given folder's UID, Title, and Description. - // Use Move to change a dashboard's parent ID. + // Update updates the given folder's UID, Title, and Description (update mode). + // If the NewParentUID field is not nil, it updates also the parent UID (move mode). + // If it's a non empty string, it moves the folder under the folder with the specific UID + // otherwise, it moves the folder under the root folder (parent_uid column is set to NULL). Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) // Get returns a folder.