diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index d0f8e67f9c4..0609c70ed1c 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -424,31 +424,29 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) ( } var nestedFolder *folder.Folder - if s.features.IsEnabled(featuremgmt.FlagNestedFolders) { - cmd := &folder.CreateFolderCommand{ - // TODO: Today, if a UID isn't specified, the dashboard store - // generates a new UID. The new folder store will need to do this as - // well, but for now we take the UID from the newly created folder. - UID: dash.UID, - OrgID: cmd.OrgID, - Title: cmd.Title, - Description: cmd.Description, - ParentUID: cmd.ParentUID, - } - nestedFolder, err = s.nestedFolderCreate(ctx, cmd) - if err != nil { - // We'll log the error and also roll back the previously-created - // (legacy) folder. - logger.Error("error saving folder to nested folder store", "error", err) - // do not shallow create error if the legacy folder delete fails - if deleteErr := s.dashboardStore.DeleteDashboard(ctx, &dashboards.DeleteDashboardCommand{ - ID: createdFolder.ID, - OrgID: createdFolder.OrgID, - }); deleteErr != nil { - logger.Error("error deleting folder after failed save to nested folder store", "error", err) - } - return dashboards.FromDashboard(dash), err + cmd = &folder.CreateFolderCommand{ + // TODO: Today, if a UID isn't specified, the dashboard store + // generates a new UID. The new folder store will need to do this as + // well, but for now we take the UID from the newly created folder. + UID: dash.UID, + OrgID: cmd.OrgID, + Title: cmd.Title, + Description: cmd.Description, + ParentUID: cmd.ParentUID, + } + nestedFolder, err = s.nestedFolderCreate(ctx, cmd) + if err != nil { + // We'll log the error and also roll back the previously-created + // (legacy) folder. + logger.Error("error saving folder to nested folder store", "error", err) + // do not shallow create error if the legacy folder delete fails + if deleteErr := s.dashboardStore.DeleteDashboard(ctx, &dashboards.DeleteDashboardCommand{ + ID: createdFolder.ID, + OrgID: createdFolder.OrgID, + }); deleteErr != nil { + logger.Error("error deleting folder after failed save to nested folder store", "error", err) } + return dashboards.FromDashboard(dash), err } f := dashboards.FromDashboard(dash) @@ -459,6 +457,8 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) ( } func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) { + logger := s.log.FromContext(ctx) + if cmd.SignedInUser == nil { return nil, folder.ErrBadRequest.Errorf("missing signed in user") } @@ -469,10 +469,6 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) ( return nil, err } - if !s.features.IsEnabled(featuremgmt.FlagNestedFolders) { - return dashFolder, nil - } - foldr, err := s.store.Update(ctx, folder.UpdateFolderCommand{ UID: cmd.UID, OrgID: cmd.OrgID, @@ -481,6 +477,11 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) ( SignedInUser: user, }) if err != nil { + if errors.Is(err, folder.ErrFolderNotFound) { + logger.Warn("attempt to update folder that does not exist in the folders table", "folderUID", cmd.UID) + return dashFolder, nil + } + return nil, err } @@ -611,15 +612,13 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e result := []string{cmd.UID} err = s.db.InTransaction(ctx, func(ctx context.Context) error { - if s.features.IsEnabled(featuremgmt.FlagNestedFolders) { - subfolders, err := s.nestedFolderDelete(ctx, cmd) + subfolders, err := s.nestedFolderDelete(ctx, cmd) - if err != nil { - logger.Error("the delete folder on folder table failed with err: ", "error", err) - return err - } - result = append(result, subfolders...) + if err != nil { + logger.Error("the delete folder on folder table failed with err: ", "error", err) + return err } + result = append(result, subfolders...) dashFolders, err := s.dashboardFolderStore.GetFolders(ctx, cmd.OrgID, result) if err != nil { diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index 7d03bb84fc3..28d8afcbaf8 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -178,12 +178,13 @@ func TestIntegrationFolderService(t *testing.T) { t.Run("Given user has permission to save", func(t *testing.T) { origNewGuardian := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true, CanViewValue: true}) service.features = featuremgmt.WithFeatures() t.Run("When creating folder should not return access denied error", func(t *testing.T) { dash := dashboards.NewDashboardFolder("Test-Folder") dash.ID = rand.Int63() + dash.UID = util.GenerateShortUID() f := dashboards.FromDashboard(dash) dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil) @@ -193,7 +194,7 @@ func TestIntegrationFolderService(t *testing.T) { actualFolder, err := service.Create(context.Background(), &folder.CreateFolderCommand{ OrgID: orgID, Title: dash.Title, - UID: "someuid", + UID: dash.UID, SignedInUser: usr, }) require.NoError(t, err) @@ -241,6 +242,7 @@ func TestIntegrationFolderService(t *testing.T) { f.ID = rand.Int63() f.UID = util.GenerateShortUID() folderStore.On("GetFolders", mock.Anything, orgID, []string{f.UID}).Return(map[string]*folder.Folder{f.UID: f}, nil) + folderStore.On("GetFolderByUID", mock.Anything, orgID, f.UID).Return(f, nil) var actualCmd *dashboards.DeleteDashboardCommand dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { @@ -582,6 +584,7 @@ func TestIntegrationNestedFolderService(t *testing.T) { depth: 1, forceDelete: true, dashboardErr: dashboards.ErrFolderNotFound, + folderErr: folder.ErrFolderNotFound, libPanelParentErr: model.ErrLibraryElementNotFound, desc: "With nested folder feature flag off and force deletion of rules", }, @@ -723,7 +726,7 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) { func TestNestedFolderService(t *testing.T) { t.Run("with feature flag unset", func(t *testing.T) { - t.Run("When create folder, no create in folder table done", func(t *testing.T) { + t.Run("Should create a folder in both dashboard and folders tables", func(t *testing.T) { g := guardian.New guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) t.Cleanup(func() { @@ -748,8 +751,7 @@ func TestNestedFolderService(t *testing.T) { SignedInUser: usr, }) require.NoError(t, err) - // CreateFolder should not call the folder store create if the feature toggle is not enabled. - require.False(t, nestedFolderStore.CreateCalled) + require.True(t, nestedFolderStore.CreateCalled) }) }) diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index f08ae8d9ede..a867a2c3ab7 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -140,7 +140,7 @@ func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) return folder.ErrInternal.Errorf("failed to get affected row: %w", err) } if affected == 0 { - return folder.ErrInternal.Errorf("no folders are updated") + return folder.ErrInternal.Errorf("no folders are updated: %w", folder.ErrFolderNotFound) } foldr, err = ss.Get(ctx, folder.GetFolderQuery{