diff --git a/pkg/services/librarypanels/api.go b/pkg/services/librarypanels/api.go index 73f0eb3e12e..e4e46b165df 100644 --- a/pkg/services/librarypanels/api.go +++ b/pkg/services/librarypanels/api.go @@ -18,9 +18,9 @@ func (lps *LibraryPanelService) registerAPIEndpoints() { lps.RouteRegister.Group("/api/library-panels", func(libraryPanels routing.RouteRegister) { libraryPanels.Post("/", middleware.ReqSignedIn, binding.Bind(createLibraryPanelCommand{}), api.Wrap(lps.createHandler)) - libraryPanels.Delete("/:panelId", middleware.ReqSignedIn, api.Wrap(lps.deleteHandler)) - libraryPanels.Get("/:panelId", middleware.ReqSignedIn, api.Wrap(lps.getHandler)) + libraryPanels.Delete("/:uid", middleware.ReqSignedIn, api.Wrap(lps.deleteHandler)) libraryPanels.Get("/", middleware.ReqSignedIn, api.Wrap(lps.getAllHandler)) + libraryPanels.Get("/:uid", middleware.ReqSignedIn, api.Wrap(lps.getHandler)) }) } @@ -37,9 +37,9 @@ func (lps *LibraryPanelService) createHandler(c *models.ReqContext, cmd createLi return api.JSON(200, util.DynMap{"result": panel}) } -// deleteHandler handles DELETE /api/library-panels/:panelId +// deleteHandler handles DELETE /api/library-panels/:uid func (lps *LibraryPanelService) deleteHandler(c *models.ReqContext) api.Response { - err := lps.deleteLibraryPanel(c, c.ParamsInt64(":panelId")) + err := lps.deleteLibraryPanel(c, c.Params(":uid")) if err != nil { if errors.Is(err, errLibraryPanelNotFound) { return api.Error(404, errLibraryPanelNotFound.Error(), err) @@ -50,9 +50,9 @@ func (lps *LibraryPanelService) deleteHandler(c *models.ReqContext) api.Response return api.Success("Library panel deleted") } -// getHandler handles GET /api/library-panels/:panelId +// getHandler handles GET /api/library-panels/:uid func (lps *LibraryPanelService) getHandler(c *models.ReqContext) api.Response { - libraryPanel, err := lps.getLibraryPanel(c, c.ParamsInt64(":panelId")) + libraryPanel, err := lps.getLibraryPanel(c, c.Params(":uid")) if err != nil { if errors.Is(err, errLibraryPanelNotFound) { return api.Error(404, errLibraryPanelNotFound.Error(), err) diff --git a/pkg/services/librarypanels/database.go b/pkg/services/librarypanels/database.go index 3c68f9fddb5..8c1177dd337 100644 --- a/pkg/services/librarypanels/database.go +++ b/pkg/services/librarypanels/database.go @@ -5,6 +5,8 @@ import ( "fmt" "time" + "github.com/grafana/grafana/pkg/util" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -15,7 +17,8 @@ func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd cre libraryPanel := LibraryPanel{ OrgID: c.SignedInUser.OrgId, FolderID: cmd.FolderID, - Title: cmd.Title, + UID: util.GenerateShortUID(), + Name: cmd.Name, Model: cmd.Model, Created: time.Now(), @@ -25,8 +28,8 @@ func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd cre UpdatedBy: c.SignedInUser.UserId, } err := lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { - if res, err := session.Query("SELECT 1 FROM library_panel WHERE org_id=? AND folder_id=? AND title=?", - c.SignedInUser.OrgId, cmd.FolderID, cmd.Title); err != nil { + if res, err := session.Query("SELECT 1 FROM library_panel WHERE org_id=? AND folder_id=? AND name=?", + c.SignedInUser.OrgId, cmd.FolderID, cmd.Name); err != nil { return err } else if len(res) == 1 { return errLibraryPanelAlreadyAdded @@ -42,10 +45,10 @@ func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd cre } // deleteLibraryPanel deletes a Library Panel -func (lps *LibraryPanelService) deleteLibraryPanel(c *models.ReqContext, panelID int64) error { +func (lps *LibraryPanelService) deleteLibraryPanel(c *models.ReqContext, uid string) error { orgID := c.SignedInUser.OrgId return lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { - result, err := session.Exec("DELETE FROM library_panel WHERE id=? and org_id=?", panelID, orgID) + result, err := session.Exec("DELETE FROM library_panel WHERE uid=? and org_id=?", uid, orgID) if err != nil { return err } @@ -61,16 +64,15 @@ func (lps *LibraryPanelService) deleteLibraryPanel(c *models.ReqContext, panelID } // getLibraryPanel gets a Library Panel. -func (lps *LibraryPanelService) getLibraryPanel(c *models.ReqContext, panelID int64) (LibraryPanel, error) { +func (lps *LibraryPanelService) getLibraryPanel(c *models.ReqContext, uid string) (LibraryPanel, error) { orgID := c.SignedInUser.OrgId var libraryPanel LibraryPanel err := lps.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { libraryPanels := make([]LibraryPanel, 0) - err := session.SQL("SELECT * FROM library_panel WHERE id=? and org_id=?", panelID, orgID).Find(&libraryPanels) + err := session.SQL("SELECT * FROM library_panel WHERE uid=? and org_id=?", uid, orgID).Find(&libraryPanels) if err != nil { return err } - if len(libraryPanels) == 0 { return errLibraryPanelNotFound } diff --git a/pkg/services/librarypanels/librarypanels.go b/pkg/services/librarypanels/librarypanels.go index 3c6821d0017..d933a6a4618 100644 --- a/pkg/services/librarypanels/librarypanels.go +++ b/pkg/services/librarypanels/librarypanels.go @@ -52,7 +52,8 @@ func (lps *LibraryPanelService) AddMigration(mg *migrator.Migrator) { {Name: "id", Type: migrator.DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true}, {Name: "org_id", Type: migrator.DB_BigInt, Nullable: false}, {Name: "folder_id", Type: migrator.DB_BigInt, Nullable: false}, - {Name: "title", Type: migrator.DB_NVarchar, Length: 255, Nullable: false}, + {Name: "uid", Type: migrator.DB_NVarchar, Length: 40, Nullable: false}, + {Name: "name", Type: migrator.DB_NVarchar, Length: 255, Nullable: false}, {Name: "model", Type: migrator.DB_Text, Nullable: false}, {Name: "created", Type: migrator.DB_DateTime, Nullable: false}, {Name: "created_by", Type: migrator.DB_BigInt, Nullable: false}, diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index edcc3af14ad..ab3e98f2481 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -19,7 +19,6 @@ func TestCreateLibraryPanel(t *testing.T) { testScenario(t, "When an admin tries to create a library panel that already exists, it should fail", func(t *testing.T, sc scenarioContext) { command := getCreateCommand(1, "Text - Library Panel") - response := sc.service.createHandler(sc.reqContext, command) require.Equal(t, 200, response.Status()) @@ -40,9 +39,15 @@ func TestDeleteLibraryPanel(t *testing.T) { command := getCreateCommand(1, "Text - Library Panel") response := sc.service.createHandler(sc.reqContext, command) require.Equal(t, 200, response.Status()) + + var result libraryPanelResult + err := json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + + sc.reqContext.ReplaceAllParams(map[string]string{":uid": result.Result.UID}) response = sc.service.deleteHandler(sc.reqContext) require.Equal(t, 200, response.Status()) - }, scenarioConfig{params: map[string]string{":panelId": "1"}}) + }) testScenario(t, "When an admin tries to delete a library panel in another org, it should fail", func(t *testing.T, sc scenarioContext) { @@ -50,21 +55,25 @@ func TestDeleteLibraryPanel(t *testing.T) { response := sc.service.createHandler(sc.reqContext, command) require.Equal(t, 200, response.Status()) + var result libraryPanelResult + err := json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + + sc.reqContext.ReplaceAllParams(map[string]string{":uid": result.Result.UID}) sc.reqContext.SignedInUser.OrgId = 2 sc.reqContext.SignedInUser.OrgRole = models.ROLE_ADMIN response = sc.service.deleteHandler(sc.reqContext) require.Equal(t, 404, response.Status()) - }, scenarioConfig{ - params: map[string]string{":panelId": "1"}, }) } func TestGetLibraryPanel(t *testing.T) { testScenario(t, "When an admin tries to get a library panel that does not exist, it should fail", func(t *testing.T, sc scenarioContext) { + sc.reqContext.ReplaceAllParams(map[string]string{":uid": "unknown"}) response := sc.service.getHandler(sc.reqContext) require.Equal(t, 404, response.Status()) - }, scenarioConfig{params: map[string]string{":panelId": "74"}}) + }) testScenario(t, "When an admin tries to get a library panel that exists, it should succeed and return correct result", func(t *testing.T, sc scenarioContext) { @@ -72,29 +81,35 @@ func TestGetLibraryPanel(t *testing.T) { response := sc.service.createHandler(sc.reqContext, command) require.Equal(t, 200, response.Status()) + var result libraryPanelResult + err := json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + + sc.reqContext.ReplaceAllParams(map[string]string{":uid": result.Result.UID}) response = sc.service.getHandler(sc.reqContext) require.Equal(t, 200, response.Status()) + err = json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + require.Equal(t, int64(1), result.Result.FolderID) + require.Equal(t, "Text - Library Panel", result.Result.Name) + }) + + testScenario(t, "When an admin tries to get a library panel that exists in an other org, it should fail", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Text - Library Panel") + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) var result libraryPanelResult err := json.Unmarshal(response.Body(), &result) require.NoError(t, err) - require.Equal(t, int64(1), result.Result.FolderID) - require.Equal(t, "Text - Library Panel", result.Result.Title) - }, scenarioConfig{params: map[string]string{":panelId": "1"}}) - - testScenario(t, "When an admin tries to get a library panel that exists in an other org, it should fail", - func(t *testing.T, sc scenarioContext) { - command := getCreateCommand(1, "Text - Library Panel") - - response := sc.service.createHandler(sc.reqContext, command) - require.Equal(t, 200, response.Status()) + sc.reqContext.ReplaceAllParams(map[string]string{":uid": result.Result.UID}) sc.reqContext.SignedInUser.OrgId = 2 sc.reqContext.SignedInUser.OrgRole = models.ROLE_ADMIN - response = sc.service.getHandler(sc.reqContext) require.Equal(t, 404, response.Status()) - }, scenarioConfig{params: map[string]string{":panelId": "1"}}) + }) } func TestGetAllLibraryPanels(t *testing.T) { @@ -128,9 +143,9 @@ func TestGetAllLibraryPanels(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, len(result.Result)) require.Equal(t, int64(1), result.Result[0].FolderID) - require.Equal(t, "Text - Library Panel", result.Result[0].Title) + require.Equal(t, "Text - Library Panel", result.Result[0].Name) require.Equal(t, int64(1), result.Result[1].FolderID) - require.Equal(t, "Text - Library Panel2", result.Result[1].Title) + require.Equal(t, "Text - Library Panel2", result.Result[1].Name) }) testScenario(t, "When an admin tries to get all library panels in a different org, none should be returned", @@ -148,11 +163,10 @@ func TestGetAllLibraryPanels(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, len(result.Result)) require.Equal(t, int64(1), result.Result[0].FolderID) - require.Equal(t, "Text - Library Panel", result.Result[0].Title) + require.Equal(t, "Text - Library Panel", result.Result[0].Name) sc.reqContext.SignedInUser.OrgId = 2 sc.reqContext.SignedInUser.OrgRole = models.ROLE_ADMIN - response = sc.service.getAllHandler(sc.reqContext) require.Equal(t, 200, response.Status()) @@ -168,7 +182,8 @@ type libraryPanel struct { ID int64 `json:"ID"` OrgID int64 `json:"OrgID"` FolderID int64 `json:"FolderID"` - Title string `json:"Title"` + UID string `json:"UID"` + Name string `json:"Name"` } type libraryPanelResult struct { @@ -200,15 +215,15 @@ func overrideLibraryPanelServiceInRegistry(cfg *setting.Cfg) LibraryPanelService return lps } -func getCreateCommand(folderID int64, title string) createLibraryPanelCommand { +func getCreateCommand(folderID int64, name string) createLibraryPanelCommand { command := createLibraryPanelCommand{ FolderID: folderID, - Title: title, + Name: name, Model: []byte(` { "datasource": "${DS_GDEV-TESTDATA}", "id": 1, - "title": "Text - Library Panel", + "name": "Text - Library Panel", "type": "text" } `), @@ -224,15 +239,9 @@ type scenarioContext struct { user models.SignedInUser } -type scenarioConfig struct { - params map[string]string - orgID int64 - role models.RoleType -} - // testScenario is a wrapper around t.Run performing common setup for library panel tests. // It takes your real test function as a callback. -func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioContext), cfgs ...scenarioConfig) { +func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioContext)) { t.Helper() t.Run(desc, func(t *testing.T) { @@ -241,17 +250,6 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo ctx := macaron.Context{} orgID := int64(1) role := models.ROLE_ADMIN - for _, cfg := range cfgs { - if len(cfg.params) > 0 { - ctx.ReplaceAllParams(cfg.params) - } - if cfg.orgID != 0 { - orgID = cfg.orgID - } - if cfg.role != "" { - role = cfg.role - } - } cfg := setting.NewCfg() // Everything in this service is behind the feature toggle "panelLibrary" diff --git a/pkg/services/librarypanels/models.go b/pkg/services/librarypanels/models.go index 07e7d9815d5..711ba4da252 100644 --- a/pkg/services/librarypanels/models.go +++ b/pkg/services/librarypanels/models.go @@ -8,10 +8,11 @@ import ( // LibraryPanel is the model for library panel definitions. type LibraryPanel struct { - ID int64 `xorm:"pk autoincr 'id'"` - OrgID int64 `xorm:"org_id"` - FolderID int64 `xorm:"folder_id"` - Title string + ID int64 `xorm:"pk autoincr 'id'"` + OrgID int64 `xorm:"org_id"` + FolderID int64 `xorm:"folder_id"` + UID string `xorm:"uid"` + Name string Model json.RawMessage Created time.Time @@ -23,7 +24,7 @@ type LibraryPanel struct { var ( // errLibraryPanelAlreadyAdded is an error for when the user tries to add a library panel that already exists. - errLibraryPanelAlreadyAdded = errors.New("library panel with that title already exists") + errLibraryPanelAlreadyAdded = errors.New("library panel with that name already exists") // errLibraryPanelNotFound is an error for when a library panel can't be found. errLibraryPanelNotFound = errors.New("library panel could not be found") ) @@ -33,6 +34,6 @@ var ( // createLibraryPanelCommand is the command for adding a LibraryPanel type createLibraryPanelCommand struct { FolderID int64 `json:"folderId"` - Title string `json:"title"` + Name string `json:"name"` Model json.RawMessage `json:"model"` }