From dfc33a70b7b8d0c01f0b1be415f7f6affd386c67 Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Wed, 1 Nov 2023 17:01:54 +0200 Subject: [PATCH] Dashboards: Fix creating dashboard under folder using deprecated API (#77501) * Dashboards: Add integration tests for creating a dashboard * Fix creating dashboard under folder using deprecated API * Update swagger response * Fix comments --- pkg/api/dashboard.go | 17 +- pkg/services/folder/folderimpl/folder.go | 2 +- pkg/services/folder/service.go | 4 +- .../api/dashboards/api_dashboards_test.go | 163 ++++++++++++++++++ public/api-merged.json | 4 + public/openapi3.json | 4 + 6 files changed, 185 insertions(+), 9 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 91b38518335..008031c85cc 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -521,12 +521,13 @@ func (hs *HTTPServer) postDashboard(c *contextmodel.ReqContext, cmd dashboards.S c.TimeRequest(metrics.MApiDashboardSave) return response.JSON(http.StatusOK, util.DynMap{ - "status": "success", - "slug": dashboard.Slug, - "version": dashboard.Version, - "id": dashboard.ID, - "uid": dashboard.UID, - "url": dashboard.GetURL(), + "status": "success", + "slug": dashboard.Slug, + "version": dashboard.Version, + "id": dashboard.ID, + "uid": dashboard.UID, + "url": dashboard.GetURL(), + "folderUid": dashboard.FolderUID, }) } @@ -1301,6 +1302,10 @@ type PostDashboardResponse struct { // required: true // example: /d/nHz3SXiiz/my-dashboard URL string `json:"url"` + + // FolderUID The unique identifier (uid) of the folder the dashboard belongs to. + // required: false + FolderUID string `json:"folderUid"` } `json:"body"` } diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 67d38d7fa21..b943c24fd44 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -116,7 +116,7 @@ func (s *Service) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder. var dashFolder *folder.Folder var err error switch { - case cmd.UID != nil: + case cmd.UID != nil && *cmd.UID != "": dashFolder, err = s.getFolderByUID(ctx, cmd.OrgID, *cmd.UID) if err != nil { return nil, err diff --git a/pkg/services/folder/service.go b/pkg/services/folder/service.go index 0d7a8218651..03d2bc3a777 100644 --- a/pkg/services/folder/service.go +++ b/pkg/services/folder/service.go @@ -13,9 +13,9 @@ type Service interface { Create(ctx context.Context, cmd *CreateFolderCommand) (*Folder, error) // GetFolder takes a GetFolderCommand and returns a folder matching the - // request. One of ID, UID, or Title must be included. If multiple values + // request. One of UID, ID 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). Get(ctx context.Context, cmd *GetFolderQuery) (*Folder, error) // Update is used to update a folder's UID, Title and Description. To change diff --git a/pkg/tests/api/dashboards/api_dashboards_test.go b/pkg/tests/api/dashboards/api_dashboards_test.go index 78d89989fa1..e269553c7a5 100644 --- a/pkg/tests/api/dashboards/api_dashboards_test.go +++ b/pkg/tests/api/dashboards/api_dashboards_test.go @@ -15,9 +15,11 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/services/dashboardimport" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgimpl" "github.com/grafana/grafana/pkg/services/plugindashboards" @@ -28,6 +30,7 @@ import ( "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/userimpl" "github.com/grafana/grafana/pkg/tests/testinfra" + "github.com/grafana/grafana/pkg/util" ) func TestIntegrationDashboardQuota(t *testing.T) { @@ -272,3 +275,163 @@ providers: }) }) } + +func TestIntegrationCreate(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + // Setup Grafana and its Database + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + DisableAnonymous: true, + }) + + grafanaListedAddr, store := testinfra.StartGrafana(t, dir, path) + // Create user + createUser(t, store, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleAdmin), + Password: "admin", + Login: "admin", + }) + + t.Run("create dashboard should succeed", func(t *testing.T) { + dashboardDataOne, err := simplejson.NewJson([]byte(`{"title":"just testing"}`)) + require.NoError(t, err) + buf1 := &bytes.Buffer{} + err = json.NewEncoder(buf1).Encode(dashboards.SaveDashboardCommand{ + Dashboard: dashboardDataOne, + }) + require.NoError(t, err) + u := fmt.Sprintf("http://admin:admin@%s/api/dashboards/db", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", buf1) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := io.ReadAll(resp.Body) + require.NoError(t, err) + var m util.DynMap + err = json.Unmarshal(b, &m) + require.NoError(t, err) + assert.NotEmpty(t, m["id"]) + assert.NotEmpty(t, m["uid"]) + }) + + t.Run("create dashboard under folder should succeed", func(t *testing.T) { + folder := createFolder(t, grafanaListedAddr, "test folder") + + dashboardDataOne, err := simplejson.NewJson([]byte(`{"title":"just testing"}`)) + require.NoError(t, err) + buf1 := &bytes.Buffer{} + err = json.NewEncoder(buf1).Encode(dashboards.SaveDashboardCommand{ + Dashboard: dashboardDataOne, + FolderUID: folder.Uid, + }) + require.NoError(t, err) + u := fmt.Sprintf("http://admin:admin@%s/api/dashboards/db", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", buf1) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := io.ReadAll(resp.Body) + require.NoError(t, err) + var m util.DynMap + err = json.Unmarshal(b, &m) + require.NoError(t, err) + assert.NotEmpty(t, m["id"]) + assert.NotEmpty(t, m["uid"]) + assert.Equal(t, folder.Uid, m["folderUid"]) + }) + + t.Run("create dashboard under folder (using deprecated folder sequential ID) should succeed", func(t *testing.T) { + folder := createFolder(t, grafanaListedAddr, "test folder 2") + + dashboardDataOne, err := simplejson.NewJson([]byte(`{"title":"just testing"}`)) + require.NoError(t, err) + buf1 := &bytes.Buffer{} + err = json.NewEncoder(buf1).Encode(dashboards.SaveDashboardCommand{ + Dashboard: dashboardDataOne, + FolderID: folder.Id, + }) + require.NoError(t, err) + u := fmt.Sprintf("http://admin:admin@%s/api/dashboards/db", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", buf1) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := io.ReadAll(resp.Body) + require.NoError(t, err) + var m util.DynMap + err = json.Unmarshal(b, &m) + require.NoError(t, err) + assert.NotEmpty(t, m["id"]) + assert.NotEmpty(t, m["uid"]) + assert.Equal(t, folder.Uid, m["folderUid"]) + }) + + t.Run("create dashboard under unknow folder should fail", func(t *testing.T) { + folderUID := "unknown" + // Import dashboard + dashboardDataOne, err := simplejson.NewJson([]byte(`{"title":"just testing"}`)) + require.NoError(t, err) + buf1 := &bytes.Buffer{} + err = json.NewEncoder(buf1).Encode(dashboards.SaveDashboardCommand{ + Dashboard: dashboardDataOne, + FolderUID: folderUID, + }) + require.NoError(t, err) + u := fmt.Sprintf("http://admin:admin@%s/api/dashboards/db", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", buf1) + require.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := io.ReadAll(resp.Body) + require.NoError(t, err) + var m util.DynMap + err = json.Unmarshal(b, &m) + require.NoError(t, err) + assert.Equal(t, "Folder not found", m["message"]) + }) +} + +func createFolder(t *testing.T, grafanaListedAddr string, title string) *dtos.Folder { + t.Helper() + + buf1 := &bytes.Buffer{} + err := json.NewEncoder(buf1).Encode(folder.CreateFolderCommand{ + Title: title, + }) + require.NoError(t, err) + u := fmt.Sprintf("http://admin:admin@%s/api/folders", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", buf1) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + var f *dtos.Folder + err = json.Unmarshal(b, &f) + require.NoError(t, err) + + return f +} diff --git a/public/api-merged.json b/public/api-merged.json index eb2274d65ac..b61b4a4aeb0 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -22456,6 +22456,10 @@ "url" ], "properties": { + "folderUid": { + "description": "FolderUID The unique identifier (uid) of the folder the dashboard belongs to.", + "type": "string" + }, "id": { "description": "ID The unique identifier (id) of the created/updated dashboard.", "type": "integer", diff --git a/public/openapi3.json b/public/openapi3.json index ff42e78e8ed..bb1220726fc 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -1579,6 +1579,10 @@ "application/json": { "schema": { "properties": { + "folderUid": { + "description": "FolderUID The unique identifier (uid) of the folder the dashboard belongs to.", + "type": "string" + }, "id": { "description": "ID The unique identifier (id) of the created/updated dashboard.", "example": 1,