Query history: Create API to patch comment of query in query history (#44981)

* Query history: Patch comment

* Update docs/sources/http_api/query_history.md

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

* Update docs/sources/http_api/query_history.md

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

* Remove redundant check

* Use WithTransactionalDbSession to update comment

* Fix status code in test

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

docs/sources/http_api/query_history.md

@@ -109,3 +109,59 @@ Status codes:
 - **200** – OK
 - **404** - Query in query history not found
 - **500** – Unable to delete query from the database
+
+### Update comment of query in Query history by UID
+
+`PATCH /api/query-history/:uid`
+
+Updates comment of a query with a specific uid that is stored in the query history.
+
+Query parameters:
+
+- **comment** – New comment that will be added to the specified query.
+
+**Example Request**:
+
+```http
+PATCH /api/query-history/P8zM2I1nz HTTP/1.1
+Accept: application/json
+Content-Type: application/json
+Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
+{
+  "comment": "Debugging query",
+}
+```
+
+**Example Response**:
+
+```http
+HTTP/1.1 200
+Content-Type: application/json
+{
+  "result": {
+    "uid": "P8zM2I1nz",
+    "datasourceUid": "PE1C5CBDA0504A6A3",
+    "createdBy": 1,
+    "createdAt": 1643630762,
+    "starred": false,
+    "comment": "Debugging query",
+    "queries": [
+      {
+        "refId": "A",
+        "key": "Q-87fed8e3-62ba-4eb2-8d2a-4129979bb4de-0",
+        "scenarioId": "csv_content",
+        "datasource": {
+            "type": "testdata",
+            "uid": "PD8C576611E62080A"
+        }
+      }
+    ]
+  }
+}
+```
+
+Status codes:
+
+- **200** – OK
+- **400** - Errors (invalid JSON, missing or invalid fields)
+- **500** – Unable to update comment of query in the database

pkg/services/queryhistory/api.go

@@ -15,6 +15,7 @@ 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))
+		entities.Patch("/:uid", middleware.ReqSignedIn, routing.Wrap(s.patchCommentHandler))
 	})
 }
 
@@ -34,10 +35,6 @@ func (s *QueryHistoryService) createHandler(c *models.ReqContext) response.Respo
 
 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)
 	}
@@ -52,3 +49,22 @@ func (s *QueryHistoryService) deleteHandler(c *models.ReqContext) response.Respo
 		ID:      id,
 	})
 }
+
+func (s *QueryHistoryService) patchCommentHandler(c *models.ReqContext) response.Response {
+	queryUID := web.Params(c.Req)[":uid"]
+	if !util.IsValidShortUID(queryUID) {
+		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
+	}
+
+	cmd := PatchQueryCommentInQueryHistoryCommand{}
+	if err := web.Bind(c.Req, &cmd); err != nil {
+		return response.Error(http.StatusBadRequest, "bad request data", err)
+	}
+
+	query, err := s.PatchQueryCommentInQueryHistory(c.Req.Context(), c.SignedInUser, queryUID, cmd)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Failed to update comment of query in query history", err)
+	}
+
+	return response.JSON(http.StatusOK, QueryHistoryResponse{Result: query})
+}

pkg/services/queryhistory/database.go

@@ -54,3 +54,40 @@ func (s QueryHistoryService) deleteQuery(ctx context.Context, user *models.Signe
 
 	return queryID, err
 }
+
+func (s QueryHistoryService) patchQueryComment(ctx context.Context, user *models.SignedInUser, UID string, cmd PatchQueryCommentInQueryHistoryCommand) (QueryHistoryDTO, error) {
+	var queryHistory QueryHistory
+	err := s.SQLStore.WithTransactionalDbSession(ctx, func(session *sqlstore.DBSession) error {
+		exists, err := session.Where("org_id = ? AND created_by = ? AND uid = ?", user.OrgId, user.UserId, UID).Get(&queryHistory)
+		if err != nil {
+			return err
+		}
+		if !exists {
+			return ErrQueryNotFound
+		}
+
+		queryHistory.Comment = cmd.Comment
+		_, err = session.ID(queryHistory.ID).Update(queryHistory)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	})
+
+	if err != nil {
+		return QueryHistoryDTO{}, err
+	}
+
+	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
+}

pkg/services/queryhistory/models.go

@@ -26,6 +26,10 @@ type CreateQueryInQueryHistoryCommand struct {
 	Queries       *simplejson.Json `json:"queries"`
 }
 
+type PatchQueryCommentInQueryHistoryCommand struct {
+	Comment string `json:"comment"`
+}
+
 type QueryHistoryDTO struct {
 	UID           string           `json:"uid"`
 	DatasourceUID string           `json:"datasourceUid"`

pkg/services/queryhistory/queryhistory.go

@@ -29,6 +29,7 @@ func ProvideService(cfg *setting.Cfg, sqlStore *sqlstore.SQLStore, routeRegister
 type Service interface {
 	CreateQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, cmd CreateQueryInQueryHistoryCommand) (QueryHistoryDTO, error)
 	DeleteQueryFromQueryHistory(ctx context.Context, user *models.SignedInUser, UID string) (int64, error)
+	PatchQueryCommentInQueryHistory(ctx context.Context, user *models.SignedInUser, UID string, cmd PatchQueryCommentInQueryHistoryCommand) (QueryHistoryDTO, error)
 }
 
 type QueryHistoryService struct {
@@ -45,3 +46,7 @@ func (s QueryHistoryService) CreateQueryInQueryHistory(ctx context.Context, user
 func (s QueryHistoryService) DeleteQueryFromQueryHistory(ctx context.Context, user *models.SignedInUser, UID string) (int64, error) {
 	return s.deleteQuery(ctx, user, UID)
 }
+
+func (s QueryHistoryService) PatchQueryCommentInQueryHistory(ctx context.Context, user *models.SignedInUser, UID string, cmd PatchQueryCommentInQueryHistoryCommand) (QueryHistoryDTO, error) {
+	return s.patchQueryComment(ctx, user, UID, cmd)
+}

pkg/services/queryhistory/queryhistory_delete_test.go

@@ -11,7 +11,7 @@ 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())
+			require.Equal(t, 500, resp.Status())
 		})
 
 	testScenarioWithQueryInQueryHistory(t, "When users tries to delete query in query history that exists, it should succeed",

pkg/services/queryhistory/queryhistory_patch_test.go

@@ -0,0 +1,25 @@
+package queryhistory
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana/pkg/web"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPatchQueryCommentInQueryHistory(t *testing.T) {
+	testScenarioWithQueryInQueryHistory(t, "When user tries to patch comment of query in query history that does not exist, it should fail",
+		func(t *testing.T, sc scenarioContext) {
+			resp := sc.service.patchCommentHandler(sc.reqContext)
+			require.Equal(t, 500, resp.Status())
+		})
+
+	testScenarioWithQueryInQueryHistory(t, "When user tries to patch comment of query in query history that exists, it should succeed",
+		func(t *testing.T, sc scenarioContext) {
+			cmd := PatchQueryCommentInQueryHistoryCommand{Comment: "test comment"}
+			sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": sc.initialResult.Result.UID})
+			sc.reqContext.Req.Body = mockRequestBody(cmd)
+			resp := sc.service.patchCommentHandler(sc.reqContext)
+			require.Equal(t, 200, resp.Status())
+		})
+}