Nested folders: Fix moving folder under root (#65684)

* Nested folders: Fix moving folder under root

* Add store test for not empty parent after update

* Modify folder and document store update implementation

Move folder only if NewParentUID is not nil

* Apply suggestion from code review

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,
 	})
 }

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 {

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)
 

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.