Nested Folders: Some API fixes (#59298)

* Nested Folders: Fix API responses

* Fix panic during deletions

* Add test
This commit is contained in:
Sofia Papagiannaki 2022-11-24 15:59:47 +02:00 committed by GitHub
parent 1a6b46e98d
commit 8e6d343981
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 16 deletions

View File

@ -20,10 +20,10 @@ type Folder struct {
Created time.Time `json:"created"`
UpdatedBy string `json:"updatedBy"`
Updated time.Time `json:"updated"`
Version int `json:"version"`
Version int `json:"version,omitempty"`
AccessControl accesscontrol.Metadata `json:"accessControl,omitempty"`
// only used if nested folders are enabled
ParentUID string `json:"parentUid"`
ParentUID string `json:"parentUid,omitempty"`
}
type FolderSearchHit struct {

View File

@ -251,6 +251,7 @@ func (hs *HTTPServer) newToFolderDto(c *models.ReqContext, g guardian.DashboardG
Updated: folder.Updated,
Version: folder.Version,
AccessControl: hs.getAccessControlMetadata(c, c.OrgID, dashboards.ScopeFoldersPrefix, folder.UID),
ParentUID: folder.ParentUID,
}
}

View File

@ -97,15 +97,21 @@ func (s *Service) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.
}
if s.features.IsEnabled(featuremgmt.FlagNestedFolders) {
if ok, err := s.accessControl.Evaluate(ctx, cmd.SignedInUser, accesscontrol.EvalPermission(
dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(*cmd.UID),
)); !ok {
f, err := s.store.Get(ctx, *cmd)
if err != nil {
return nil, err
}
g := guardian.New(ctx, f.ID, f.OrgID, cmd.SignedInUser)
if canView, err := g.CanView(); err != nil || !canView {
if err != nil {
return nil, toFolderError(err)
}
return nil, dashboards.ErrFolderAccessDenied
}
return s.store.Get(ctx, *cmd)
return f, err
}
switch {
@ -274,6 +280,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
logger.Error("Could not make user admin", "folder", createdFolder.Title, "user", userID, "error", permissionErr)
}
var nestedFolder *folder.Folder
if s.features.IsEnabled(featuremgmt.FlagNestedFolders) {
cmd := &folder.CreateFolderCommand{
// TODO: Today, if a UID isn't specified, the dashboard store
@ -285,7 +292,8 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
Description: cmd.Description,
ParentUID: cmd.ParentUID,
}
if err := s.nestedFolderCreate(ctx, cmd); err != nil {
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)
@ -296,10 +304,13 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
}
return folder.FromDashboard(dash), err
}
// The folder UID is specified (or generated) during creation, so we'll
// stop here and return the created model.Folder.
}
return folder.FromDashboard(dash), nil
f := folder.FromDashboard(dash)
if nestedFolder != nil && nestedFolder.ParentUID != "" {
f.ParentUID = nestedFolder.ParentUID
}
return f, nil
}
func (s *Service) Update(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*folder.Folder, error) {
@ -494,14 +505,13 @@ func (s *Service) MakeUserAdmin(ctx context.Context, orgID int64, userID, folder
return s.dashboardService.MakeUserAdmin(ctx, orgID, userID, folderID, setViewAndEditPermissions)
}
func (s *Service) nestedFolderCreate(ctx context.Context, cmd *folder.CreateFolderCommand) error {
func (s *Service) nestedFolderCreate(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) {
if cmd.ParentUID != "" {
if err := s.validateParent(ctx, cmd.OrgID, cmd.ParentUID); err != nil {
return err
return nil, err
}
}
_, err := s.store.Create(ctx, *cmd)
return err
return s.store.Create(ctx, *cmd)
}
func (s *Service) validateParent(ctx context.Context, orgID int64, parentUID string) error {

View File

@ -333,10 +333,11 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) {
log: log.New("test-folder-service"),
}
t.Run("create folder", func(t *testing.T) {
folderStore.ExpectedFolder = &folder.Folder{}
folderStore.ExpectedFolder = &folder.Folder{ParentUID: util.GenerateShortUID()}
res, err := folderService.Create(context.Background(), &folder.CreateFolderCommand{SignedInUser: usr})
require.NoError(t, err)
require.NotNil(t, res.UID)
require.NotEmpty(t, res.ParentUID)
})
t.Run("get parents folder", func(t *testing.T) {
@ -497,6 +498,13 @@ func TestNestedFolderService(t *testing.T) {
})
t.Run("move, no error", func(t *testing.T) {
// This test creates and deletes the dashboard, so needs some extra setup.
g := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true, CanViewValue: true})
t.Cleanup(func() {
guardian.New = g
})
store.ExpectedError = nil
store.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"}
f, err := foldersvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr})
@ -512,7 +520,7 @@ func TestNestedFolderService(t *testing.T) {
dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&models.Folder{}, nil)
g := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true, CanViewValue: true})
err := foldersvc.DeleteFolder(context.Background(), &folder.DeleteFolderCommand{UID: "myFolder", OrgID: orgID, SignedInUser: usr})
require.NoError(t, err)