From 0eac9aff7f82a6953f82415f757ad05aac266654 Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Wed, 4 Oct 2023 11:19:22 +0300 Subject: [PATCH] Nested Folders: Fix fetching a folder by title (#74725) Modify folder store Get() to consider folder parent --- pkg/services/folder/folderimpl/sqlstore.go | 10 ++++- .../folder/folderimpl/sqlstore_test.go | 38 ++++++++++++++++--- pkg/services/folder/model.go | 14 ++++--- pkg/services/folder/service.go | 5 ++- 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index 7bac08fe0d7..dee0ceedf8d 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -166,7 +166,15 @@ func (ss *sqlStore) Get(ctx context.Context, q folder.GetFolderQuery) (*folder.F case q.ID != nil: exists, err = sess.SQL("SELECT * FROM folder WHERE id = ?", q.ID).Get(foldr) case q.Title != nil: - exists, err = sess.SQL("SELECT * FROM folder WHERE title = ? AND org_id = ?", q.Title, q.OrgID).Get(foldr) + args := []any{*q.Title, q.OrgID} + s := "SELECT * FROM folder WHERE title = ? AND org_id = ?" + if q.ParentUID != nil { + s = s + " AND parent_uid = ?" + args = append(args, *q.ParentUID) + } else { + s = s + " AND parent_uid IS NULL" + } + exists, err = sess.SQL(s, args...).Get(foldr) default: return folder.ErrBadRequest.Errorf("one of ID, UID, or Title must be included in the command") } diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index 21f7ba88009..82d32cd3259 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -384,6 +384,17 @@ func TestIntegrationGet(t *testing.T) { }) require.NoError(t, err) + // create subfolder with same title + subfolderUID := util.GenerateShortUID() + subfolder, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: folderTitle, + Description: folderDsc, + OrgID: orgID, + UID: subfolderUID, + ParentUID: f.UID, + }) + require.NoError(t, err) + t.Cleanup(func() { err := folderStore.Delete(context.Background(), f.UID, orgID) require.NoError(t, err) @@ -405,15 +416,14 @@ func TestIntegrationGet(t *testing.T) { assert.Equal(t, f.OrgID, ff.OrgID) assert.Equal(t, f.Title, ff.Title) assert.Equal(t, f.Description, ff.Description) - //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) { + t.Run("get folder by title should return folder under root", func(t *testing.T) { ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ - Title: &f.Title, + Title: &folderTitle, OrgID: orgID, }) require.NoError(t, err) @@ -422,13 +432,30 @@ func TestIntegrationGet(t *testing.T) { assert.Equal(t, f.OrgID, ff.OrgID) assert.Equal(t, f.Title, ff.Title) assert.Equal(t, f.Description, ff.Description) - //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) { + t.Run("get folder by title and parent UID should return subfolder", func(t *testing.T) { + ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ + Title: &folderTitle, + OrgID: orgID, + ParentUID: &f.UID, + }) + require.NoError(t, err) + assert.Equal(t, subfolder.ID, ff.ID) + assert.Equal(t, subfolder.UID, ff.UID) + assert.Equal(t, subfolder.OrgID, ff.OrgID) + assert.Equal(t, subfolder.Title, ff.Title) + assert.Equal(t, subfolder.Description, ff.Description) + assert.Equal(t, subfolder.ParentUID, ff.ParentUID) + assert.NotEmpty(t, subfolder.Created) + assert.NotEmpty(t, subfolder.Updated) + assert.NotEmpty(t, subfolder.URL) + }) + + t.Run("get folder by ID should succeed", func(t *testing.T) { ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ ID: &f.ID, }) @@ -438,7 +465,6 @@ func TestIntegrationGet(t *testing.T) { assert.Equal(t, f.OrgID, ff.OrgID) assert.Equal(t, f.Title, ff.Title) assert.Equal(t, f.Description, ff.Description) - //assert.Equal(t, folder.GeneralFolderUID, ff.ParentUID) assert.NotEmpty(t, ff.Created) assert.NotEmpty(t, ff.Updated) assert.NotEmpty(t, ff.URL) diff --git a/pkg/services/folder/model.go b/pkg/services/folder/model.go index 13566c201c7..8890d310584 100644 --- a/pkg/services/folder/model.go +++ b/pkg/services/folder/model.go @@ -126,13 +126,15 @@ type DeleteFolderCommand struct { // GetFolderQuery is used for all folder Get requests. Only one of UID, ID, or // Title should be set; if multiple fields are set by the caller the dashboard -// service will select the field with the most specificity, in order: ID, UID, -// Title. +// service will select the field with the most specificity, in order: UID, ID +// Title. If Title is set, it will fetch the folder in the root folder. +// Callers can additionally set the ParentUID field to fetch a folder by title under a specific folder. type GetFolderQuery struct { - UID *string - ID *int64 - Title *string - OrgID int64 + UID *string + ParentUID *string + ID *int64 + Title *string + OrgID int64 SignedInUser identity.Requester `json:"-"` } diff --git a/pkg/services/folder/service.go b/pkg/services/folder/service.go index 0d7a8218651..f3dda822d65 100644 --- a/pkg/services/folder/service.go +++ b/pkg/services/folder/service.go @@ -15,7 +15,10 @@ type Service interface { // GetFolder takes a GetFolderCommand and returns a folder matching the // request. One of ID, UID, or Title must be included. If multiple values // are included in the request, Grafana will select one in order of - // specificity (ID, UID, Title). + // specificity (UID, ID,Title). + // If Title is set, it will fetch the folder in the root folder. + // If nested folders are enabled, callers can additionally set the ParentUID field + // to fetch a folder by title under a specific folder. Get(ctx context.Context, cmd *GetFolderQuery) (*Folder, error) // Update is used to update a folder's UID, Title and Description. To change