Search: Fix empty folder details for nested folder items (#76504)

* Introduce dashboard.folder_uid column

* Add data migration

* Search: Fix empty folder details for nested folders

* Set `dashboard.folder_uid` and update tests

* Add unique index

* lint

Ignore cyclomatic complexity of func
`(*DashboardServiceImpl).BuildSaveDashboardCommand

* Fix search by folder UID
This commit is contained in:
Sofia Papagiannaki
2023-10-24 10:04:45 +03:00
committed by GitHub
parent 442e533803
commit 03a626f1d6
28 changed files with 706 additions and 236 deletions
@@ -37,7 +37,7 @@ func TestIntegrationDashboardFolderStore(t *testing.T) {
sqlStore = db.InitTestDB(t)
folderStore := ProvideDashboardFolderStore(sqlStore)
folder2 = insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "prod")
_ = insertTestDashboard(t, dashboardStore, title, orgId, folder2.ID, "prod")
_ = insertTestDashboard(t, dashboardStore, title, orgId, folder2.ID, folder2.UID, "prod")
folder1 = insertTestFolder(t, dashboardStore, title, orgId, 0, "prod")
t.Run("GetFolderByTitle should find the folder", func(t *testing.T) {
@@ -52,7 +52,7 @@ func TestIntegrationDashboardFolderStore(t *testing.T) {
sqlStore := db.InitTestDB(t)
folderStore := ProvideDashboardFolderStore(sqlStore)
folder := insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "prod")
dash := insertTestDashboard(t, dashboardStore, "Very Unique Name", orgId, folder.ID, "prod")
dash := insertTestDashboard(t, dashboardStore, "Very Unique Name", orgId, folder.ID, folder.UID, "prod")
t.Run("should return folder by UID", func(t *testing.T) {
d, err := folderStore.GetFolderByUID(context.Background(), orgId, folder.UID)
@@ -76,7 +76,7 @@ func TestIntegrationDashboardFolderStore(t *testing.T) {
sqlStore := db.InitTestDB(t)
folderStore := ProvideDashboardFolderStore(sqlStore)
folder := insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "prod")
dash := insertTestDashboard(t, dashboardStore, "Very Unique Name", orgId, folder.ID, "prod")
dash := insertTestDashboard(t, dashboardStore, "Very Unique Name", orgId, folder.ID, folder.UID, "prod")
t.Run("should return folder by ID", func(t *testing.T) {
d, err := folderStore.GetFolderByID(context.Background(), orgId, folder.ID)
@@ -96,12 +96,13 @@ func TestIntegrationDashboardFolderStore(t *testing.T) {
})
}
func insertTestDashboard(t *testing.T, dashboardStore dashboards.Store, title string, orgId int64, folderId int64, tags ...any) *dashboards.Dashboard {
func insertTestDashboard(t *testing.T, dashboardStore dashboards.Store, title string, orgId int64, folderID int64, folderUID string, tags ...any) *dashboards.Dashboard {
t.Helper()
cmd := dashboards.SaveDashboardCommand{
OrgID: orgId,
FolderID: folderId,
IsFolder: false,
OrgID: orgId,
FolderID: folderID,
FolderUID: folderUID,
IsFolder: false,
Dashboard: simplejson.NewFromAny(map[string]any{
"id": nil,
"title": title,
@@ -116,12 +117,13 @@ func insertTestDashboard(t *testing.T, dashboardStore dashboards.Store, title st
return dash
}
func insertTestFolder(t *testing.T, dashboardStore dashboards.Store, title string, orgId int64, folderId int64, tags ...any) *dashboards.Dashboard {
func insertTestFolder(t *testing.T, dashboardStore dashboards.Store, title string, orgId int64, folderId int64, folderUID string, tags ...any) *dashboards.Dashboard {
t.Helper()
cmd := dashboards.SaveDashboardCommand{
OrgID: orgId,
FolderID: folderId,
IsFolder: true,
OrgID: orgId,
FolderID: folderId,
FolderUID: folderUID,
IsFolder: true,
Dashboard: simplejson.NewFromAny(map[string]any{
"id": nil,
"title": title,
+42 -14
View File
@@ -276,6 +276,9 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
return nil, folder.ErrBadRequest.Errorf("missing signed in user")
}
dashFolder := dashboards.NewDashboardFolder(cmd.Title)
dashFolder.OrgID = cmd.OrgID
if s.features.IsEnabled(featuremgmt.FlagNestedFolders) && cmd.ParentUID != "" {
// Check that the user is allowed to create a subfolder in this folder
evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.ParentUID))
@@ -286,11 +289,9 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
if !hasAccess {
return nil, dashboards.ErrFolderAccessDenied
}
dashFolder.FolderUID = cmd.ParentUID
}
dashFolder := dashboards.NewDashboardFolder(cmd.Title)
dashFolder.OrgID = cmd.OrgID
trimmedUID := strings.TrimSpace(cmd.UID)
if trimmedUID == accesscontrol.GeneralFolderUID {
return nil, dashboards.ErrFolderInvalidUID
@@ -325,7 +326,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
User: user,
}
saveDashboardCmd, err := s.BuildSaveDashboardCommand(ctx, dto)
saveDashboardCmd, err := s.buildSaveDashboardCommand(ctx, dto)
if err != nil {
return nil, toFolderError(err)
}
@@ -419,6 +420,10 @@ func (s *Service) legacyUpdate(ctx context.Context, cmd *folder.UpdateFolderComm
}
dashFolder := queryResult
if cmd.NewParentUID != nil {
dashFolder.FolderUID = *cmd.NewParentUID
}
currentTitle := dashFolder.Title
if !dashFolder.IsFolder {
@@ -447,7 +452,7 @@ func (s *Service) legacyUpdate(ctx context.Context, cmd *folder.UpdateFolderComm
Overwrite: cmd.Overwrite,
}
saveDashboardCmd, err := s.BuildSaveDashboardCommand(ctx, dto)
saveDashboardCmd, err := s.buildSaveDashboardCommand(ctx, dto)
if err != nil {
return nil, toFolderError(err)
}
@@ -621,7 +626,7 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
// if the current folder is already a parent of newparent, we should return error
for _, parent := range parents {
if parent.UID == cmd.UID {
return nil, folder.ErrCircularReference
return nil, folder.ErrCircularReference.Errorf("failed to move folder")
}
}
@@ -629,12 +634,34 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
if cmd.NewParentUID != "" {
newParentUID = cmd.NewParentUID
}
return s.store.Update(ctx, folder.UpdateFolderCommand{
UID: cmd.UID,
OrgID: cmd.OrgID,
NewParentUID: &newParentUID,
SignedInUser: cmd.SignedInUser,
})
var f *folder.Folder
if err := s.db.InTransaction(ctx, func(ctx context.Context) error {
if f, err = s.store.Update(ctx, folder.UpdateFolderCommand{
UID: cmd.UID,
OrgID: cmd.OrgID,
NewParentUID: &newParentUID,
SignedInUser: cmd.SignedInUser,
}); err != nil {
return folder.ErrInternal.Errorf("failed to move folder: %w", err)
}
if _, err := s.legacyUpdate(ctx, &folder.UpdateFolderCommand{
UID: cmd.UID,
OrgID: cmd.OrgID,
NewParentUID: &newParentUID,
SignedInUser: cmd.SignedInUser,
// bypass optimistic locking used for dashboards
Overwrite: true,
}); err != nil {
return folder.ErrInternal.Errorf("failed to move legacy folder: %w", err)
}
return nil
}); err != nil {
return nil, err
}
return f, nil
}
// nestedFolderDelete inspects the folder referenced by the cmd argument, deletes all the entries for
@@ -735,9 +762,9 @@ func (s *Service) getNestedFolders(ctx context.Context, orgID int64, uid string)
return result, nil
}
// BuildSaveDashboardCommand is a simplified version on DashboardServiceImpl.BuildSaveDashboardCommand
// buildSaveDashboardCommand is a simplified version on DashboardServiceImpl.buildSaveDashboardCommand
// keeping only the meaningful functionality for folders
func (s *Service) BuildSaveDashboardCommand(ctx context.Context, dto *dashboards.SaveDashboardDTO) (*dashboards.SaveDashboardCommand, error) {
func (s *Service) buildSaveDashboardCommand(ctx context.Context, dto *dashboards.SaveDashboardDTO) (*dashboards.SaveDashboardCommand, error) {
dash := dto.Dashboard
dash.OrgID = dto.OrgID
@@ -807,6 +834,7 @@ func (s *Service) BuildSaveDashboardCommand(ctx context.Context, dto *dashboards
Overwrite: dto.Overwrite,
UserID: userID,
FolderID: dash.FolderID,
FolderUID: dash.FolderUID,
IsFolder: dash.IsFolder,
PluginID: dash.PluginID,
}
+10 -8
View File
@@ -416,8 +416,8 @@ func TestIntegrationNestedFolderService(t *testing.T) {
require.NoError(t, err)
subfolder, err := serviceWithFlagOn.dashboardFolderStore.GetFolderByUID(context.Background(), orgID, ancestorUIDs[1])
require.NoError(t, err)
_ = insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in parent", orgID, parent.ID, "prod")
_ = insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in subfolder", orgID, subfolder.ID, "prod")
_ = insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in parent", orgID, parent.ID, parent.UID, "prod")
_ = insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in subfolder", orgID, subfolder.ID, subfolder.UID, "prod")
_ = createRule(t, alertStore, parent.UID, "parent alert")
_ = createRule(t, alertStore, subfolder.UID, "sub alert")
@@ -491,8 +491,8 @@ func TestIntegrationNestedFolderService(t *testing.T) {
require.NoError(t, err)
subfolder, err := serviceWithFlagOn.dashboardFolderStore.GetFolderByUID(context.Background(), orgID, ancestorUIDs[1])
require.NoError(t, err)
_ = insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in parent", orgID, parent.ID, "prod")
_ = insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in subfolder", orgID, subfolder.ID, "prod")
_ = insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in parent", orgID, parent.ID, parent.UID, "prod")
_ = insertTestDashboard(t, serviceWithFlagOn.dashboardStore, "dashboard in subfolder", orgID, subfolder.ID, subfolder.UID, "prod")
_ = createRule(t, alertStore, parent.UID, "parent alert")
_ = createRule(t, alertStore, subfolder.UID, "sub alert")
@@ -996,9 +996,10 @@ func TestNestedFolderService(t *testing.T) {
nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("newFolder")}}
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: nestedFolderUser})
_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: nestedFolderUser})
require.NoError(t, err)
require.NotNil(t, f)
// the folder is set inside InTransaction() but the fake one is called
// require.NotNil(t, f)
})
t.Run("move to the root folder without folder creation permissions fails", func(t *testing.T) {
@@ -1032,9 +1033,10 @@ func TestNestedFolderService(t *testing.T) {
nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersCreate: {}}
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "", OrgID: orgID, SignedInUser: nestedFolderUser})
_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "", OrgID: orgID, SignedInUser: nestedFolderUser})
require.NoError(t, err)
require.NotNil(t, f)
// the folder is set inside InTransaction() but the fake one is called
// require.NotNil(t, f)
})
t.Run("move when parentUID in the current subtree returns error from nested folder service", func(t *testing.T) {