Nested folders: Fix missing URL from folder responses (#68082)

* Nested folders: Set URL in folder responses always

* Apply suggestions from code review

Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com>

pkg/services/folder/folderimpl/sqlstore.go

@@ -5,10 +5,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/grafana/dskit/concurrency"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/infra/slugify"
-	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/setting"
@@ -74,7 +73,7 @@ func (ss *sqlStore) Create(ctx context.Context, cmd folder.CreateFolderCommand)
 		}
 		return nil
 	})
-	return foldr, err
+	return foldr.WithURL(), err
 }
 
 func (ss *sqlStore) Delete(ctx context.Context, uid string, orgID int64) error {
@@ -159,7 +158,7 @@ func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand)
 		return nil
 	})
 
-	return foldr, err
+	return foldr.WithURL(), err
 }
 
 func (ss *sqlStore) Get(ctx context.Context, q folder.GetFolderQuery) (*folder.Folder, error) {
@@ -185,8 +184,7 @@ func (ss *sqlStore) Get(ctx context.Context, q folder.GetFolderQuery) (*folder.F
 		}
 		return nil
 	})
-	foldr.URL = dashboards.GetFolderURL(foldr.UID, slugify.Slugify(foldr.Title))
+	return foldr.WithURL(), err
-	return foldr, err
 }
 
 func (ss *sqlStore) GetParents(ctx context.Context, q folder.GetParentsQuery) ([]*folder.Folder, error) {
@@ -218,6 +216,13 @@ func (ss *sqlStore) GetParents(ctx context.Context, q folder.GetParentsQuery) ([
 		}); err != nil {
 			return nil, err
 		}
+
+		if err := concurrency.ForEachJob(ctx, len(folders), len(folders), func(ctx context.Context, idx int) error {
+			folders[idx].WithURL()
+			return nil
+		}); err != nil {
+			ss.log.Debug("failed to set URL to folders", "err", err)
+		}
 	default:
 		ss.log.Debug("recursive CTE subquery is not supported; it fallbacks to the iterative implementation")
 		return ss.getParentsMySQL(ctx, q)
@@ -257,6 +262,13 @@ func (ss *sqlStore) GetChildren(ctx context.Context, q folder.GetChildrenQuery)
 		if err != nil {
 			return folder.ErrDatabaseError.Errorf("failed to get folder children: %w", err)
 		}
+
+		if err := concurrency.ForEachJob(ctx, len(folders), len(folders), func(ctx context.Context, idx int) error {
+			folders[idx].WithURL()
+			return nil
+		}); err != nil {
+			ss.log.Debug("failed to set URL to folders", "err", err)
+		}
 		return nil
 	})
 	return folders, err
@@ -281,7 +293,8 @@ func (ss *sqlStore) getParentsMySQL(ctx context.Context, cmd folder.GetParentsQu
 			if !ok {
 				break
 			}
-			folders = append(folders, f)
+
+			folders = append(folders, f.WithURL())
 			uid = f.ParentUID
 			if len(folders) > folder.MaxNestedFolderDepth {
 				return folder.ErrMaximumDepthReached.Errorf("failed to get parent folders iteratively")

pkg/services/folder/folderimpl/sqlstore_test.go

@@ -72,6 +72,7 @@ func TestIntegrationCreate(t *testing.T) {
 		assert.NotEmpty(t, f.ID)
 		assert.Equal(t, uid, f.UID)
 		assert.Empty(t, f.ParentUID)
+		assert.NotEmpty(t, f.URL)
 
 		ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{
 			UID:   &f.UID,
@@ -81,6 +82,7 @@ func TestIntegrationCreate(t *testing.T) {
 		assert.Equal(t, folderTitle, ff.Title)
 		assert.Equal(t, folderDsc, ff.Description)
 		assert.Empty(t, ff.ParentUID)
+		assert.NotEmpty(t, ff.URL)
 
 		assertAncestorUIDs(t, folderStore, f, []string{folder.GeneralFolderUID})
 	})
@@ -96,6 +98,7 @@ func TestIntegrationCreate(t *testing.T) {
 		require.Equal(t, "parent", parent.Title)
 		require.NotEmpty(t, parent.ID)
 		assert.Equal(t, parentUID, parent.UID)
+		assert.NotEmpty(t, parent.URL)
 
 		t.Cleanup(func() {
 			err := folderStore.Delete(context.Background(), parent.UID, orgID)
@@ -122,6 +125,7 @@ func TestIntegrationCreate(t *testing.T) {
 		assert.NotEmpty(t, f.ID)
 		assert.Equal(t, uid, f.UID)
 		assert.Equal(t, parentUID, f.ParentUID)
+		assert.NotEmpty(t, f.URL)
 
 		assertAncestorUIDs(t, folderStore, f, []string{folder.GeneralFolderUID, parent.UID})
 		assertChildrenUIDs(t, folderStore, parent, []string{f.UID})
@@ -134,6 +138,7 @@ func TestIntegrationCreate(t *testing.T) {
 		assert.Equal(t, folderTitle, ff.Title)
 		assert.Equal(t, folderDsc, ff.Description)
 		assert.Equal(t, parentUID, ff.ParentUID)
+		assert.NotEmpty(t, ff.URL)
 	})
 }
 
@@ -264,6 +269,7 @@ func TestIntegrationUpdate(t *testing.T) {
 		assert.Equal(t, newTitle, updated.Title)
 		assert.Equal(t, newDesc, updated.Description)
 		assert.Equal(t, parent.UID, updated.ParentUID)
+		assert.NotEmpty(t, updated.URL)
 		// assert.GreaterOrEqual(t, updated.Updated.UnixNano(), existingUpdated.UnixNano())
 
 		updated, err = folderStore.Get(context.Background(), folder.GetFolderQuery{
@@ -275,6 +281,7 @@ func TestIntegrationUpdate(t *testing.T) {
 		assert.Equal(t, newDesc, updated.Description)
 		// parent should not change
 		assert.Equal(t, parent.UID, updated.ParentUID)
+		assert.NotEmpty(t, updated.URL)
 
 		f = updated
 	})
@@ -300,6 +307,7 @@ func TestIntegrationUpdate(t *testing.T) {
 		assert.Equal(t, newUID, updated.UID)
 		assert.Equal(t, existingTitle, updated.Title)
 		assert.Equal(t, existingDesc, updated.Description)
+		assert.NotEmpty(t, updated.URL)
 	})
 
 	t.Run("updating folder parent UID", func(t *testing.T) {
@@ -374,6 +382,7 @@ func TestIntegrationUpdate(t *testing.T) {
 				assert.Equal(t, existingTitle, updated.Title)
 				assert.Equal(t, existingDesc, updated.Description)
 				assert.Equal(t, existingUID, updated.UID)
+				assert.NotEmpty(t, updated.URL)
 			})
 		}
 	})
@@ -423,6 +432,7 @@ func TestIntegrationGet(t *testing.T) {
 		//assert.Equal(t, folder.GeneralFolderUID, ff.ParentUID)
 		assert.NotEmpty(t, ff.Created)
 		assert.NotEmpty(t, ff.Updated)
+		assert.NotEmpty(t, ff.URL)
 	})
 
 	t.Run("get folder by title should succeed", func(t *testing.T) {
@@ -439,6 +449,7 @@ func TestIntegrationGet(t *testing.T) {
 		//assert.Equal(t, folder.GeneralFolderUID, ff.ParentUID)
 		assert.NotEmpty(t, ff.Created)
 		assert.NotEmpty(t, ff.Updated)
+		assert.NotEmpty(t, ff.URL)
 	})
 
 	t.Run("get folder by title should succeed", func(t *testing.T) {
@@ -454,6 +465,7 @@ func TestIntegrationGet(t *testing.T) {
 		//assert.Equal(t, folder.GeneralFolderUID, ff.ParentUID)
 		assert.NotEmpty(t, ff.Created)
 		assert.NotEmpty(t, ff.Updated)
+		assert.NotEmpty(t, ff.URL)
 	})
 }
 
@@ -518,6 +530,7 @@ func TestIntegrationGetParents(t *testing.T) {
 		require.NoError(t, err)
 		parentUIDs := make([]string, 0)
 		for _, p := range parents {
+			assert.NotEmpty(t, p.URL)
 			parentUIDs = append(parentUIDs, p.UID)
 		}
 		require.Equal(t, []string{uid1}, parentUIDs)
@@ -570,6 +583,7 @@ func TestIntegrationGetChildren(t *testing.T) {
 
 		childrenUIDs := make([]string, 0, len(children))
 		for _, c := range children {
+			assert.NotEmpty(t, c.URL)
 			childrenUIDs = append(childrenUIDs, c.UID)
 		}
 
@@ -586,6 +600,7 @@ func TestIntegrationGetChildren(t *testing.T) {
 
 		childrenUIDs := make([]string, 0, len(children))
 		for _, c := range children {
+			assert.NotEmpty(t, c.URL)
 			childrenUIDs = append(childrenUIDs, c.UID)
 		}
 		assert.Equal(t, []string{parent.UID}, childrenUIDs)
@@ -618,6 +633,7 @@ func TestIntegrationGetChildren(t *testing.T) {
 
 		childrenUIDs = make([]string, 0, len(children))
 		for _, c := range children {
+			assert.NotEmpty(t, c.URL)
 			childrenUIDs = append(childrenUIDs, c.UID)
 		}
 
@@ -635,6 +651,7 @@ func TestIntegrationGetChildren(t *testing.T) {
 
 		childrenUIDs = make([]string, 0, len(children))
 		for _, c := range children {
+			assert.NotEmpty(t, c.URL)
 			childrenUIDs = append(childrenUIDs, c.UID)
 		}
 
@@ -652,6 +669,7 @@ func TestIntegrationGetChildren(t *testing.T) {
 
 		childrenUIDs = make([]string, 0, len(children))
 		for _, c := range children {
+			assert.NotEmpty(t, c.URL)
 			childrenUIDs = append(childrenUIDs, c.UID)
 		}
 
@@ -669,6 +687,7 @@ func TestIntegrationGetChildren(t *testing.T) {
 
 		childrenUIDs = make([]string, 0, len(children))
 		for _, c := range children {
+			assert.NotEmpty(t, c.URL)
 			childrenUIDs = append(childrenUIDs, c.UID)
 		}
 

pkg/services/folder/model.go

@@ -1,9 +1,12 @@
 package folder
 
 import (
+	"fmt"
 	"time"
 
+	"github.com/grafana/grafana/pkg/infra/slugify"
 	"github.com/grafana/grafana/pkg/services/user"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
@@ -48,6 +51,16 @@ func (f *Folder) IsGeneral() bool {
 	return f.ID == GeneralFolder.ID && f.Title == GeneralFolder.Title
 }
 
+func (f *Folder) WithURL() *Folder {
+	if f == nil || f.URL != "" {
+		return f
+	}
+
+	// copy of dashboards.GetFolderURL()
+	f.URL = fmt.Sprintf("%s/dashboards/f/%s/%s", setting.AppSubUrl, f.UID, slugify.Slugify(f.Title))
+	return f
+}
+
 // NewFolder tales a title and returns a Folder with the Created and Updated
 // fields set to the current time.
 func NewFolder(title string, description string) *Folder {