Query history: Create API to delete query from query history (#44653)

* Query history: Add delete and refactor

* Update docs/sources/http_api/query_history.md

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>

docs/sources/http_api/query_history.md

@@ -49,11 +49,63 @@ JSON body schema:
 `DELETE /api/query-history/:uid`
 
 Deletes the query in query history that matches the specified uid. It requires that the user is logged in and that Query history feature is enabled in config file.
-  "message": "Query successfully added to query history",
+
+**Example Request**:
+
+```http
+DELETE /api/query-history/P8zM2I1nz HTTP/1.1
+Accept: application/json
+Content-Type: application/json
+Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
+```
+
+**Example Response**:
+
+```http
+HTTP/1.1 200
+Content-Type: application/json
+
+```
+
+Status codes:
 
 - **200** – OK
 - **404** - Query in query history not found
 - **500** – Unable to delete query from the database
 
 - **200** – OK
-- **500** – Errors (invalid JSON, missing or invalid fields)
+- **400** - Errors (invalid JSON, missing or invalid fields)
+- **500** – Unable to add query to the database
+
+### Delete query from Query history by UID
+
+`DELETE /api/query-history/:uid`
+
+Deletes the query in query history that matches the specified uid. It requires that the user is logged in and that Query history feature is enabled in config file.
+
+**Example Request**:
+
+```http
+DELETE /api/query-history/P8zM2I1nz HTTP/1.1
+Accept: application/json
+Content-Type: application/json
+Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
+```
+
+**Example Response**:
+
+```http
+HTTP/1.1 200
+Content-Type: application/json
+
+{
+    "message": "Query deleted",
+    "id": 28
+}
+```
+
+Status codes:
+
+- **200** – OK
+- **404** - Query in query history not found
+- **500** – Unable to delete query from the database

pkg/services/queryhistory/api.go

@@ -7,12 +7,14 @@ import (
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/web"
 )
 
 func (s *QueryHistoryService) registerAPIEndpoints() {
 	s.RouteRegister.Group("/api/query-history", func(entities routing.RouteRegister) {
 		entities.Post("/", middleware.ReqSignedIn, routing.Wrap(s.createHandler))
+		entities.Delete("/:uid", middleware.ReqSignedIn, routing.Wrap(s.deleteHandler))
 	})
 }
 
@@ -22,10 +24,31 @@ func (s *QueryHistoryService) createHandler(c *models.ReqContext) response.Respo
 		return response.Error(http.StatusBadRequest, "bad request data", err)
 	}
 
-	err := s.CreateQueryInQueryHistory(c.Req.Context(), c.SignedInUser, cmd)
+	query, err := s.CreateQueryInQueryHistory(c.Req.Context(), c.SignedInUser, cmd)
 	if err != nil {
-		return response.Error(500, "Failed to create query history", err)
+		return response.Error(http.StatusInternalServerError, "Failed to create query history", err)
 	}
 
-	return response.Success("Query successfully added to query history")
+	return response.JSON(http.StatusOK, QueryHistoryResponse{Result: query})
+}
+
+func (s *QueryHistoryService) deleteHandler(c *models.ReqContext) response.Response {
+	queryUID := web.Params(c.Req)[":uid"]
+	if len(queryUID) == 0 {
+		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
+	}
+
+	if !util.IsValidShortUID(queryUID) {
+		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
+	}
+
+	id, err := s.DeleteQueryFromQueryHistory(c.Req.Context(), c.SignedInUser, queryUID)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Failed to delete query from query history", err)
+	}
+
+	return response.JSON(http.StatusOK, DeleteQueryFromQueryHistoryResponse{
+		Message: "Query deleted",
+		ID:      id,
+	})
 }

pkg/services/queryhistory/database.go

@@ -9,12 +9,12 @@ import (
 	"github.com/grafana/grafana/pkg/util"
 )
 
-func (s QueryHistoryService) createQuery(ctx context.Context, user *models.SignedInUser, cmd CreateQueryInQueryHistoryCommand) error {
+func (s QueryHistoryService) createQuery(ctx context.Context, user *models.SignedInUser, cmd CreateQueryInQueryHistoryCommand) (QueryHistoryDTO, error) {
 	queryHistory := QueryHistory{
-		OrgId:         user.OrgId,
+		OrgID:         user.OrgId,
-		Uid:           util.GenerateShortUID(),
+		UID:           util.GenerateShortUID(),
 		Queries:       cmd.Queries,
-		DatasourceUid: cmd.DatasourceUid,
+		DatasourceUID: cmd.DatasourceUID,
 		CreatedBy:     user.UserId,
 		CreatedAt:     time.Now().Unix(),
 		Comment:       "",
@@ -25,8 +25,32 @@ func (s QueryHistoryService) createQuery(ctx context.Context, user *models.Signe
 		return err
 	})
 	if err != nil {
-		return err
+		return QueryHistoryDTO{}, err
 	}
 
-	return nil
+	dto := QueryHistoryDTO{
+		UID:           queryHistory.UID,
+		DatasourceUID: queryHistory.DatasourceUID,
+		CreatedBy:     queryHistory.CreatedBy,
+		CreatedAt:     queryHistory.CreatedAt,
+		Comment:       queryHistory.Comment,
+		Queries:       queryHistory.Queries,
+		Starred:       false,
+	}
+
+	return dto, nil
+}
+
+func (s QueryHistoryService) deleteQuery(ctx context.Context, user *models.SignedInUser, UID string) (int64, error) {
+	var queryID int64
+	err := s.SQLStore.WithDbSession(ctx, func(session *sqlstore.DBSession) error {
+		id, err := session.Where("org_id = ? AND created_by = ? AND uid = ?", user.OrgId, user.UserId, UID).Delete(QueryHistory{})
+		if id == 0 {
+			return ErrQueryNotFound
+		}
+		queryID = id
+		return err
+	})
+
+	return queryID, err
 }

pkg/services/queryhistory/models.go

@@ -1,21 +1,48 @@
 package queryhistory
 
 import (
+	"errors"
+
 	"github.com/grafana/grafana/pkg/components/simplejson"
 )
 
+var (
+	ErrQueryNotFound = errors.New("query in query history not found")
+)
+
 type QueryHistory struct {
-	Id            int64            `json:"id"`
+	ID            int64  `xorm:"pk autoincr 'id'"`
-	Uid           string           `json:"uid"`
+	UID           string `xorm:"uid"`
-	DatasourceUid string           `json:"datasourceUid"`
+	DatasourceUID string `xorm:"datasource_uid"`
-	OrgId         int64            `json:"orgId"`
+	OrgID         int64  `xorm:"org_id"`
+	CreatedBy     int64
+	CreatedAt     int64
+	Comment       string
+	Queries       *simplejson.Json
+}
+
+type CreateQueryInQueryHistoryCommand struct {
+	DatasourceUID string           `json:"datasourceUid"`
+	Queries       *simplejson.Json `json:"queries"`
+}
+
+type QueryHistoryDTO struct {
+	UID           string           `json:"uid"`
+	DatasourceUID string           `json:"datasourceUid"`
 	CreatedBy     int64            `json:"createdBy"`
 	CreatedAt     int64            `json:"createdAt"`
 	Comment       string           `json:"comment"`
 	Queries       *simplejson.Json `json:"queries"`
+	Starred       bool             `json:"starred"`
 }
 
-type CreateQueryInQueryHistoryCommand struct {
+// QueryHistoryResponse is a response struct for QueryHistoryDTO
-	DatasourceUid string           `json:"datasourceUid"`
+type QueryHistoryResponse struct {
-	Queries       *simplejson.Json `json:"queries"`
+	Result QueryHistoryDTO `json:"result"`
+}
+
+// DeleteQueryFromQueryHistoryResponse is the response struct for deleting a query from query history
+type DeleteQueryFromQueryHistoryResponse struct {
+	ID      int64  `json:"id"`
+	Message string `json:"message"`
 }

pkg/services/queryhistory/queryhistory.go

@@ -27,7 +27,8 @@ func ProvideService(cfg *setting.Cfg, sqlStore *sqlstore.SQLStore, routeRegister
 }
 
 type Service interface {
-	CreateQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, cmd CreateQueryInQueryHistoryCommand) error
+	CreateQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, cmd CreateQueryInQueryHistoryCommand) (QueryHistoryDTO, error)
+	DeleteQueryFromQueryHistory(ctx context.Context, user *models.SignedInUser, UID string) (int64, error)
 }
 
 type QueryHistoryService struct {
@@ -37,6 +38,10 @@ type QueryHistoryService struct {
 	log           log.Logger
 }
 
-func (s QueryHistoryService) CreateQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, cmd CreateQueryInQueryHistoryCommand) error {
+func (s QueryHistoryService) CreateQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, cmd CreateQueryInQueryHistoryCommand) (QueryHistoryDTO, error) {
 	return s.createQuery(ctx, user, cmd)
 }
+
+func (s QueryHistoryService) DeleteQueryFromQueryHistory(ctx context.Context, user *models.SignedInUser, UID string) (int64, error) {
+	return s.deleteQuery(ctx, user, UID)
+}

pkg/services/queryhistory/queryhistory_create_test.go

@@ -11,7 +11,7 @@ func TestCreateQueryInQueryHistory(t *testing.T) {
 	testScenario(t, "When users tries to create query in query history it should succeed",
 		func(t *testing.T, sc scenarioContext) {
 			command := CreateQueryInQueryHistoryCommand{
-				DatasourceUid: "NCzh67i",
+				DatasourceUID: "NCzh67i",
 				Queries: simplejson.NewFromAny(map[string]interface{}{
 					"expr": "test",
 				}),

pkg/services/queryhistory/queryhistory_delete_test.go

@@ -0,0 +1,23 @@
+package queryhistory
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana/pkg/web"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDeleteQueryFromQueryHistory(t *testing.T) {
+	testScenarioWithQueryInQueryHistory(t, "When users tries to delete query in query history that does not exist, it should fail",
+		func(t *testing.T, sc scenarioContext) {
+			resp := sc.service.deleteHandler(sc.reqContext)
+			require.Equal(t, 404, resp.Status())
+		})
+
+	testScenarioWithQueryInQueryHistory(t, "When users tries to delete query in query history that exists, it should succeed",
+		func(t *testing.T, sc scenarioContext) {
+			sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": sc.initialResult.Result.UID})
+			resp := sc.service.deleteHandler(sc.reqContext)
+			require.Equal(t, 200, resp.Status())
+		})
+}

pkg/services/queryhistory/queryhistory_test.go

@@ -9,6 +9,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/grafana/grafana/pkg/api/response"
+	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
@@ -22,10 +24,11 @@ var (
 )
 
 type scenarioContext struct {
-	ctx        *web.Context
+	ctx           *web.Context
-	service    *QueryHistoryService
+	service       *QueryHistoryService
-	reqContext *models.ReqContext
+	reqContext    *models.ReqContext
-	sqlStore   *sqlstore.SQLStore
+	sqlStore      *sqlstore.SQLStore
+	initialResult QueryHistoryResponse
 }
 
 func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioContext)) {
@@ -71,7 +74,36 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
 	})
 }
 
+func testScenarioWithQueryInQueryHistory(t *testing.T, desc string, fn func(t *testing.T, sc scenarioContext)) {
+	t.Helper()
+
+	testScenario(t, desc, func(t *testing.T, sc scenarioContext) {
+		command := CreateQueryInQueryHistoryCommand{
+			DatasourceUID: "NCzh67i",
+			Queries: simplejson.NewFromAny(map[string]interface{}{
+				"expr": "test",
+			}),
+		}
+		sc.reqContext.Req.Body = mockRequestBody(command)
+		resp := sc.service.createHandler(sc.reqContext)
+		sc.initialResult = validateAndUnMarshalResponse(t, resp)
+		fn(t, sc)
+	})
+}
+
 func mockRequestBody(v interface{}) io.ReadCloser {
 	b, _ := json.Marshal(v)
 	return io.NopCloser(bytes.NewReader(b))
 }
+
+func validateAndUnMarshalResponse(t *testing.T, resp response.Response) QueryHistoryResponse {
+	t.Helper()
+
+	require.Equal(t, 200, resp.Status())
+
+	var result = QueryHistoryResponse{}
+	err := json.Unmarshal(resp.Body(), &result)
+	require.NoError(t, err)
+
+	return result
+}