Nested folders: Fix delete (#59627)

* Fix deleting subfolder

It used to fail with beause of missing signed in user

* Add logging

* fixup

* Fail request if deleting nested folder has failed

Before we only used to log the error

* Fix failing test

During failed nested folder creation
call the dashboard store deletion instead of the service one.
This commit is contained in:
Sofia Papagiannaki
2022-12-01 12:27:40 +02:00
committed by GitHub
parent 8ed2426e8e
commit 798a8ceb9c
2 changed files with 16 additions and 3 deletions

View File

@@ -299,8 +299,10 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
// (legacy) folder.
logger.Error("error saving folder to nested folder store", "error", err)
// do not shallow create error if the legacy folder delete fails
deleteErr := s.DeleteFolder(ctx, &folder.DeleteFolderCommand{UID: createdFolder.UID, OrgID: cmd.OrgID, ForceDeleteRules: true, SignedInUser: user})
if deleteErr != nil {
if deleteErr := s.dashboardStore.DeleteDashboard(ctx, &models.DeleteDashboardCommand{
Id: createdFolder.ID,
OrgId: createdFolder.OrgID,
}); deleteErr != nil {
logger.Error("error deleting folder after failed save to nested folder store", "error", err)
}
return folder.FromDashboard(dash), err
@@ -415,6 +417,7 @@ func (s *Service) DeleteFolder(ctx context.Context, cmd *folder.DeleteFolderComm
err := s.Delete(ctx, cmd)
if err != nil {
logger.Error("the delete folder on folder table failed with err: ", "error", err)
return err
}
}
@@ -460,6 +463,7 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
}
func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) error {
logger := s.log.FromContext(ctx)
if cmd.SignedInUser == nil {
return folder.ErrBadRequest.Errorf("missing signed in user")
}
@@ -478,13 +482,17 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
return err
}
for _, f := range folders {
err := s.Delete(ctx, &folder.DeleteFolderCommand{UID: f.UID, OrgID: f.OrgID, ForceDeleteRules: cmd.ForceDeleteRules})
logger.Info("deleting subfolder", "org_id", f.OrgID, "uid", f.UID)
err := s.Delete(ctx, &folder.DeleteFolderCommand{UID: f.UID, OrgID: f.OrgID, ForceDeleteRules: cmd.ForceDeleteRules, SignedInUser: cmd.SignedInUser})
if err != nil {
logger.Error("failed deleting subfolder", "org_id", f.OrgID, "uid", f.UID, "error", err)
return err
}
}
logger.Info("deleting folder", "org_id", cmd.OrgID, "uid", cmd.UID)
err = s.store.Delete(ctx, cmd.UID, cmd.OrgID)
if err != nil {
logger.Info("failed deleting folder", "org_id", cmd.OrgID, "uid", cmd.UID, "err", err)
return err
}
return nil

View File

@@ -476,6 +476,10 @@ func TestNestedFolderService(t *testing.T) {
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("models.SaveDashboardCommand")).Return(&models.Dashboard{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil)
var actualCmd *models.DeleteDashboardCommand
dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
actualCmd = args.Get(1).(*models.DeleteDashboardCommand)
}).Return(nil).Once()
// return an error from the folder store
store.ExpectedError = errors.New("FAILED")
@@ -491,6 +495,7 @@ func TestNestedFolderService(t *testing.T) {
// CreateFolder should also call the folder store's create method.
require.True(t, store.CreateCalled)
require.NotNil(t, actualCmd)
t.Cleanup(func() {
guardian.New = g