PanelLibrary: Adds delete Api (#29741)

* PanelLibrary: Adds delete

* Chore: fixes some comments

* Chore: changes after PR comments

* Refactor: deletes from correct org

pkg/services/librarypanels/api.go

@@ -18,6 +18,7 @@ func (lps *LibraryPanelService) registerAPIEndpoints() {
 
 	lps.RouteRegister.Group("/api/library-panels", func(libraryPanels routing.RouteRegister) {
 		libraryPanels.Post("/", middleware.ReqSignedIn, binding.Bind(addLibraryPanelCommand{}), api.Wrap(lps.createHandler))
+		libraryPanels.Delete("/:panelId", middleware.ReqSignedIn, api.Wrap(lps.deleteHandler))
 	})
 }
 
@@ -34,3 +35,17 @@ func (lps *LibraryPanelService) createHandler(c *models.ReqContext, cmd addLibra
 
 	return api.JSON(200, util.DynMap{"panel": panel})
 }
+
+// deleteHandler handles DELETE /api/library-panels/:panelId.
+func (lps *LibraryPanelService) deleteHandler(c *models.ReqContext) api.Response {
+	err := lps.deleteLibraryPanel(c, c.ParamsInt64(":panelId"))
+
+	if err != nil {
+		if errors.Is(err, errLibraryPanelNotFound) {
+			return api.Error(404, errLibraryPanelNotFound.Error(), err)
+		}
+		return api.Error(500, "Failed to delete library panel", err)
+	}
+
+	return api.Success("Library panel deleted")
+}

pkg/services/librarypanels/database.go

@@ -9,7 +9,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 )
 
-// createLibraryPanel function adds a LibraryPanel
+// createLibraryPanel function adds a Library Panel
 func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd addLibraryPanelCommand) (LibraryPanel, error) {
 	libraryPanel := LibraryPanel{
 		OrgID:    c.SignedInUser.OrgId,
@@ -39,3 +39,26 @@ func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd add
 
 	return libraryPanel, err
 }
+
+// deleteLibraryPanel function deletes a Library Panel
+func (lps *LibraryPanelService) deleteLibraryPanel(c *models.ReqContext, panelID int64) error {
+	orgID := c.SignedInUser.OrgId
+
+	err := 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)
+
+		if err != nil {
+			return err
+		}
+
+		if rowsAffected, err := result.RowsAffected(); err != nil {
+			return err
+		} else if rowsAffected != 1 {
+			return errLibraryPanelNotFound
+		}
+
+		return nil
+	})
+
+	return err
+}

pkg/services/librarypanels/librarypanels_test.go

@@ -4,6 +4,8 @@ import (
 	"testing"
 	"time"
 
+	"gopkg.in/macaron.v1"
+
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/models"
@@ -13,32 +15,81 @@ import (
 )
 
 func TestCreateLibraryPanel(t *testing.T) {
-	t.Run("should fail if library panel already exists", func(t *testing.T) {
+	libraryPanelScenario(t, "When an admin tries to create a library panel", "then it should fail if library panel already exists", func(t *testing.T) {
-		lps, context := setupTestEnv(t, models.ROLE_EDITOR)
+		lps, context := setupTestEnv(t, models.ROLE_ADMIN, map[string]string{})
-		command := addLibraryPanelCommand{
+		command := getCreateCommand(1, "Text - Library Panel")
-			FolderID: 1,
-			Title:    "Text - Library Panel",
-			Model: []byte(`
-	{
-      "datasource": "${DS_GDEV-TESTDATA}",
-      "id": 1,
-      "title": "Text - Library Panel",
-      "type": "text"
-    }
-`),
-		}
 
 		response := lps.createHandler(&context, command)
 		require.Equal(t, 200, response.Status())
 
 		response = lps.createHandler(&context, command)
 		require.Equal(t, 400, response.Status())
-
-		t.Cleanup(registry.ClearOverrides)
 	})
 }
 
-func setupMigrations(cfg *setting.Cfg) LibraryPanelService {
+func TestDeleteLibraryPanel(t *testing.T) {
+	libraryPanelScenario(t, "When an admin tries to delete a library panel that does not exist", "then it should fail", func(t *testing.T) {
+		lps, context := setupTestEnv(t, models.ROLE_ADMIN, map[string]string{":panelId": "74"})
+
+		response := lps.deleteHandler(&context)
+		require.Equal(t, 404, response.Status())
+	})
+
+	libraryPanelScenario(t, "When an admin tries to delete a library panel that exists", "then it should succeed", func(t *testing.T) {
+		lps, context := setupTestEnv(t, models.ROLE_ADMIN, map[string]string{":panelId": "1"})
+		command := getCreateCommand(1, "Text - Library Panel")
+
+		response := lps.createHandler(&context, command)
+		require.Equal(t, 200, response.Status())
+
+		response = lps.deleteHandler(&context)
+		require.Equal(t, 200, response.Status())
+	})
+
+	libraryPanelScenario(t, "When an admin tries to delete a library panel in another org", "then it should fail", func(t *testing.T) {
+		params := map[string]string{":panelId": "1"}
+		lps, context := setupTestEnv(t, models.ROLE_ADMIN, params)
+		command := getCreateCommand(1, "Text - Library Panel")
+
+		response := lps.createHandler(&context, command)
+		require.Equal(t, 200, response.Status())
+
+		user := getTestUser(models.ROLE_ADMIN, 2)
+		context = getTestContext(user, params)
+
+		response = lps.deleteHandler(&context)
+		require.Equal(t, 404, response.Status())
+	})
+}
+
+func libraryPanelScenario(t *testing.T, when string, then string, fn func(t *testing.T)) {
+	t.Run(when, func(t *testing.T) {
+		t.Run(then, func(t *testing.T) {
+			fn(t)
+			t.Cleanup(registry.ClearOverrides)
+		})
+	})
+}
+
+func setupTestEnv(t *testing.T, orgRole models.RoleType, params macaron.Params) (LibraryPanelService, models.ReqContext) {
+	cfg := setting.NewCfg()
+	cfg.FeatureToggles = map[string]bool{"panelLibrary": true} // Everything in this service is behind the feature toggle "panelLibrary"
+
+	// Because the LibraryPanelService is behind a feature toggle we need to override the service in the registry
+	// with a Cfg that contains the feature toggle so that Migrations are run properly
+	service := overrideLibraryPanelServiceInRegistry(cfg)
+
+	sqlStore := sqlstore.InitTestDB(t)
+	// We need to assign SQLStore after the override and migrations are done
+	service.SQLStore = sqlStore
+
+	user := getTestUser(orgRole, 1)
+	context := getTestContext(user, params)
+
+	return service, context
+}
+
+func overrideLibraryPanelServiceInRegistry(cfg *setting.Cfg) LibraryPanelService {
 	lps := LibraryPanelService{
 		SQLStore: nil,
 		Cfg:      cfg,
@@ -59,42 +110,42 @@ func setupMigrations(cfg *setting.Cfg) LibraryPanelService {
 	return lps
 }
 
-func setupTestEnv(t *testing.T, orgRole models.RoleType) (LibraryPanelService, models.ReqContext) {
+func getTestUser(orgRole models.RoleType, orgID int64) models.SignedInUser {
-	cfg := setting.NewCfg()
-	cfg.FeatureToggles = map[string]bool{"panelLibrary": true}
-
-	service := setupMigrations(cfg)
-
-	sqlStore := sqlstore.InitTestDB(t)
-	service.SQLStore = sqlStore
-
 	user := models.SignedInUser{
-		UserId:         1,
+		UserId:     1,
-		OrgId:          1,
+		OrgId:      orgID,
-		OrgName:        "",
+		OrgRole:    orgRole,
-		OrgRole:        orgRole,
+		LastSeenAt: time.Now(),
-		Login:          "",
-		Name:           "",
-		Email:          "",
-		ApiKeyId:       0,
-		OrgCount:       0,
-		IsGrafanaAdmin: false,
-		IsAnonymous:    false,
-		HelpFlags1:     0,
-		LastSeenAt:     time.Now(),
-		Teams:          nil,
 	}
 
+	return user
+}
+
+func getTestContext(user models.SignedInUser, params macaron.Params) models.ReqContext {
+	macronContext := macaron.Context{}
+	macronContext.ReplaceAllParams(params)
+
 	context := models.ReqContext{
-		Context:        nil,
+		Context:      &macronContext,
-		SignedInUser:   &user,
+		SignedInUser: &user,
-		UserToken:      nil,
-		IsSignedIn:     false,
-		IsRenderCall:   false,
-		AllowAnonymous: false,
-		SkipCache:      false,
-		Logger:         nil,
 	}
 
-	return service, context
+	return context
+}
+
+func getCreateCommand(folderID int64, title string) addLibraryPanelCommand {
+	command := addLibraryPanelCommand{
+		FolderID: folderID,
+		Title:    title,
+		Model: []byte(`
+			{
+			  "datasource": "${DS_GDEV-TESTDATA}",
+			  "id": 1,
+			  "title": "Text - Library Panel",
+			  "type": "text"
+			}
+		`),
+	}
+
+	return command
 }

pkg/services/librarypanels/models.go

@@ -2,7 +2,7 @@ package librarypanels
 
 import (
 	"encoding/json"
-	"fmt"
+	"errors"
 	"time"
 )
 
@@ -22,8 +22,10 @@ type LibraryPanel struct {
 }
 
 var (
-	// errLibraryPanelAlreadyAdded is an error when you add a library panel that already exists.
+	// errLibraryPanelAlreadyAdded is an error for when the user tries to add a library panel that already exists.
-	errLibraryPanelAlreadyAdded = fmt.Errorf("library panel with that title already exists")
+	errLibraryPanelAlreadyAdded = errors.New("library panel with that title already exists")
+	// errLibraryPanelNotFound is an error for when a library panel can't be found.
+	errLibraryPanelNotFound = errors.New("library panel could not be found")
 )
 
 // Commands