Query history: Create API to star and unstar query in query history (#45077)

* Query history: Add starring and unstarring API * Return dto with starred info when commenting * Add documentation for starring and unstarring of query * Return dto when starring/unstarring * Update documentation * Update deleting with unstarring * Check queryUID length in queryhistory * Fix linting issues * Update docs/sources/http_api/query_history.md Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com> * 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>
2025-01-27 16:57:14 -06:00 · 2022-02-23 17:03:04 +01:00 · 2022-02-23 17:03:04 +01:00 · a3a852be81
commit a3a852be81
parent 3cfbbbdbf2
10 changed files with 369 additions and 9 deletions
--- a/docs/sources/http_api/query_history.md
+++ b/docs/sources/http_api/query_history.md
@ -77,7 +77,7 @@ Status codes:
 - **400** - Errors (invalid JSON, missing or invalid fields)
 - **500** – Unable to add query to the database

-### Delete query from Query history by UID
+## Delete query from Query history by UID

 `DELETE /api/query-history/:uid`

@ -107,10 +107,9 @@ Content-Type: application/json
 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
+## Update comment of query in Query history by UID

 `PATCH /api/query-history/:uid`

@ -165,3 +164,99 @@ Status codes:
 - **200** – OK
 - **400** - Errors (invalid JSON, missing or invalid fields)
 - **500** – Unable to update comment of query in the database
+
+## Star query in Query history
+
+`POST /api/query-history/star/:uid`
+
+Stars query in query history.
+
+**Example request:**
+
+```http
+POST /api/query-history/star/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
+{
+  "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
+- **500** – Unable to star query in the database
+
+## Unstar query in Query history
+
+`DELETE /api/query-history/star/:uid`
+
+Removes stars from query in query history.
+
+**Example request:**
+
+```http
+DELETE /api/query-history/star/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
+{
+  "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
+- **500** – Unable to unstar query in the database
--- a/pkg/services/queryhistory/api.go
+++ b/pkg/services/queryhistory/api.go
@ -15,6 +15,8 @@ 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.Post("/star/:uid", middleware.ReqSignedIn, routing.Wrap(s.starHandler))
+		entities.Delete("/star/:uid", middleware.ReqSignedIn, routing.Wrap(s.unstarHandler))
 		entities.Patch("/:uid", middleware.ReqSignedIn, routing.Wrap(s.patchCommentHandler))
 	})
 }
@ -35,7 +37,7 @@ 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 !util.IsValidShortUID(queryUID) {
+	if len(queryUID) > 0 && !util.IsValidShortUID(queryUID) {
 		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
 	}

@ -52,7 +54,7 @@ func (s *QueryHistoryService) deleteHandler(c *models.ReqContext) response.Respo

 func (s *QueryHistoryService) patchCommentHandler(c *models.ReqContext) response.Response {
 	queryUID := web.Params(c.Req)[":uid"]
-	if !util.IsValidShortUID(queryUID) {
+	if len(queryUID) > 0 && !util.IsValidShortUID(queryUID) {
 		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
 	}

@ -68,3 +70,31 @@ func (s *QueryHistoryService) patchCommentHandler(c *models.ReqContext) response

 	return response.JSON(http.StatusOK, QueryHistoryResponse{Result: query})
 }
+
+func (s *QueryHistoryService) starHandler(c *models.ReqContext) response.Response {
+	queryUID := web.Params(c.Req)[":uid"]
+	if len(queryUID) > 0 && !util.IsValidShortUID(queryUID) {
+		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
+	}
+
+	query, err := s.StarQueryInQueryHistory(c.Req.Context(), c.SignedInUser, queryUID)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Failed to star query in query history", err)
+	}
+
+	return response.JSON(http.StatusOK, QueryHistoryResponse{Result: query})
+}
+
+func (s *QueryHistoryService) unstarHandler(c *models.ReqContext) response.Response {
+	queryUID := web.Params(c.Req)[":uid"]
+	if len(queryUID) > 0 && !util.IsValidShortUID(queryUID) {
+		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
+	}
+
+	query, err := s.UnstarQueryInQueryHistory(c.Req.Context(), c.SignedInUser, queryUID)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Failed to unstar query in query history", err)
+	}
+
+	return response.JSON(http.StatusOK, QueryHistoryResponse{Result: query})
+}
--- a/pkg/services/queryhistory/database.go
+++ b/pkg/services/queryhistory/database.go
@ -43,13 +43,24 @@ func (s QueryHistoryService) createQuery(ctx context.Context, user *models.Signe

 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 {
+	err := s.SQLStore.WithTransactionalDbSession(ctx, func(session *sqlstore.DBSession) error {
+		// Try to unstar the query first
+		_, err := session.Table("query_history_star").Where("user_id = ? AND query_uid = ?", user.UserId, UID).Delete(QueryHistoryStar{})
+		if err != nil {
+			s.log.Error("Failed to unstar query while deleting it from query history", "query", UID, "user", user.UserId, "error", err)
+		}
+
+		// Then delete it
 		id, err := session.Where("org_id = ? AND created_by = ? AND uid = ?", user.OrgId, user.UserId, UID).Delete(QueryHistory{})
+		if err != nil {
+			return err
+		}
 		if id == 0 {
 			return ErrQueryNotFound
 		}
+
 		queryID = id
-		return err
+		return nil
 	})

 	return queryID, err
@ -57,6 +68,8 @@ func (s QueryHistoryService) deleteQuery(ctx context.Context, user *models.Signe

 func (s QueryHistoryService) patchQueryComment(ctx context.Context, user *models.SignedInUser, UID string, cmd PatchQueryCommentInQueryHistoryCommand) (QueryHistoryDTO, error) {
 	var queryHistory QueryHistory
+	var isStarred bool
+
 	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 {
@ -72,6 +85,11 @@ func (s QueryHistoryService) patchQueryComment(ctx context.Context, user *models
 			return err
 		}

+		starred, err := session.Table("query_history_star").Where("user_id = ? AND query_uid = ?", user.UserId, UID).Exist()
+		if err != nil {
+			return err
+		}
+		isStarred = starred
 		return nil
 	})

@ -86,7 +104,98 @@ func (s QueryHistoryService) patchQueryComment(ctx context.Context, user *models
 		CreatedAt:     queryHistory.CreatedAt,
 		Comment:       queryHistory.Comment,
 		Queries:       queryHistory.Queries,
-		Starred:       false,
+		Starred:       isStarred,
+	}
+
+	return dto, nil
+}
+
+func (s QueryHistoryService) starQuery(ctx context.Context, user *models.SignedInUser, UID string) (QueryHistoryDTO, error) {
+	var queryHistory QueryHistory
+	var isStarred bool
+
+	err := s.SQLStore.WithTransactionalDbSession(ctx, func(session *sqlstore.DBSession) error {
+		// Check if query exists as we want to star only existing queries
+		exists, err := session.Table("query_history").Where("org_id = ? AND created_by = ? AND uid = ?", user.OrgId, user.UserId, UID).Get(&queryHistory)
+		if err != nil {
+			return err
+		}
+		if !exists {
+			return ErrQueryNotFound
+		}
+
+		// If query exists then star it
+		queryHistoryStar := QueryHistoryStar{
+			UserID:   user.UserId,
+			QueryUID: UID,
+		}
+
+		_, err = session.Insert(&queryHistoryStar)
+		if err != nil {
+			if s.SQLStore.Dialect.IsUniqueConstraintViolation(err) {
+				return ErrQueryAlreadyStarred
+			}
+			return err
+		}
+
+		isStarred = true
+		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:       isStarred,
+	}
+
+	return dto, nil
+}
+
+func (s QueryHistoryService) unstarQuery(ctx context.Context, user *models.SignedInUser, UID string) (QueryHistoryDTO, error) {
+	var queryHistory QueryHistory
+	var isStarred bool
+
+	err := s.SQLStore.WithTransactionalDbSession(ctx, func(session *sqlstore.DBSession) error {
+		exists, err := session.Table("query_history").Where("org_id = ? AND created_by = ? AND uid = ?", user.OrgId, user.UserId, UID).Get(&queryHistory)
+		if err != nil {
+			return err
+		}
+		if !exists {
+			return ErrQueryNotFound
+		}
+
+		id, err := session.Table("query_history_star").Where("user_id = ? AND query_uid = ?", user.UserId, UID).Delete(QueryHistoryStar{})
+		if id == 0 {
+			return ErrStarredQueryNotFound
+		}
+		if err != nil {
+			return err
+		}
+
+		isStarred = false
+		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:       isStarred,
 	}

 	return dto, nil
--- a/pkg/services/queryhistory/models.go
+++ b/pkg/services/queryhistory/models.go
@ -7,7 +7,9 @@ import (
 )

 var (
-	ErrQueryNotFound = errors.New("query in query history not found")
+	ErrQueryNotFound        = errors.New("query in query history not found")
+	ErrStarredQueryNotFound = errors.New("starred query not found")
+	ErrQueryAlreadyStarred  = errors.New("query was already starred")
 )

 type QueryHistory struct {
@ -21,6 +23,12 @@ type QueryHistory struct {
 	Queries       *simplejson.Json
 }

+type QueryHistoryStar struct {
+	ID       int64  `xorm:"pk autoincr 'id'"`
+	QueryUID string `xorm:"query_uid"`
+	UserID   int64  `xorm:"user_id"`
+}
+
 type CreateQueryInQueryHistoryCommand struct {
 	DatasourceUID string           `json:"datasourceUid"`
 	Queries       *simplejson.Json `json:"queries"`
--- a/pkg/services/queryhistory/queryhistory.go
+++ b/pkg/services/queryhistory/queryhistory.go
@ -30,6 +30,8 @@ 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)
+	StarQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, UID string) (QueryHistoryDTO, error)
+	UnstarQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, UID string) (QueryHistoryDTO, error)
 }

 type QueryHistoryService struct {
@ -50,3 +52,11 @@ func (s QueryHistoryService) DeleteQueryFromQueryHistory(ctx context.Context, us
 func (s QueryHistoryService) PatchQueryCommentInQueryHistory(ctx context.Context, user *models.SignedInUser, UID string, cmd PatchQueryCommentInQueryHistoryCommand) (QueryHistoryDTO, error) {
 	return s.patchQueryComment(ctx, user, UID, cmd)
 }
+
+func (s QueryHistoryService) StarQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, UID string) (QueryHistoryDTO, error) {
+	return s.starQuery(ctx, user, UID)
+}
+
+func (s QueryHistoryService) UnstarQueryInQueryHistory(ctx context.Context, user *models.SignedInUser, UID string) (QueryHistoryDTO, error) {
+	return s.unstarQuery(ctx, user, UID)
+}
--- a/pkg/services/queryhistory/queryhistory_delete_test.go
+++ b/pkg/services/queryhistory/queryhistory_delete_test.go
@ -1,8 +1,10 @@
 package queryhistory

 import (
+	"context"
 	"testing"

+	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/web"
 	"github.com/stretchr/testify/require"
 )
@ -20,4 +22,22 @@ func TestDeleteQueryFromQueryHistory(t *testing.T) {
 			resp := sc.service.deleteHandler(sc.reqContext)
 			require.Equal(t, 200, resp.Status())
 		})
+
+	testScenarioWithQueryInQueryHistory(t, "When users tries to delete query in query history that exists, it should also unstar it and succeed",
+		func(t *testing.T, sc scenarioContext) {
+			sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": sc.initialResult.Result.UID})
+			// Star added query
+			sc.service.starHandler(sc.reqContext)
+			// Then delete it
+			resp := sc.service.deleteHandler(sc.reqContext)
+			// Check if query is still in query_history_star table
+			err := sc.sqlStore.WithDbSession(context.Background(), func(dbSession *sqlstore.DBSession) error {
+				exists, err := dbSession.Table("query_history_star").Where("user_id = ? AND query_uid = ?", sc.reqContext.SignedInUser.UserId, sc.initialResult.Result.UID).Exist()
+				require.NoError(t, err)
+				require.Equal(t, false, exists)
+				return err
+			})
+			require.NoError(t, err)
+			require.Equal(t, 200, resp.Status())
+		})
 }
--- a/pkg/services/queryhistory/queryhistory_star_test.go
+++ b/pkg/services/queryhistory/queryhistory_star_test.go
@ -0,0 +1,31 @@
+package queryhistory
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana/pkg/web"
+	"github.com/stretchr/testify/require"
+)
+
+func TestStarQueryInQueryHistory(t *testing.T) {
+	testScenarioWithQueryInQueryHistory(t, "When users tries to star query in query history that does not exists, it should fail",
+		func(t *testing.T, sc scenarioContext) {
+			resp := sc.service.starHandler(sc.reqContext)
+			require.Equal(t, 500, resp.Status())
+		})
+
+	testScenarioWithQueryInQueryHistory(t, "When users tries to star 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.starHandler(sc.reqContext)
+			require.Equal(t, 200, resp.Status())
+		})
+
+	testScenarioWithQueryInQueryHistory(t, "When users tries to star query that is already starred, it should fail",
+		func(t *testing.T, sc scenarioContext) {
+			sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": sc.initialResult.Result.UID})
+			sc.service.starHandler(sc.reqContext)
+			resp := sc.service.starHandler(sc.reqContext)
+			require.Equal(t, 500, resp.Status())
+		})
+}
--- a/pkg/services/queryhistory/queryhistory_unstar_test.go
+++ b/pkg/services/queryhistory/queryhistory_unstar_test.go
@ -0,0 +1,33 @@
+package queryhistory
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/web"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUnstarQueryInQueryHistory(t *testing.T) {
+	testScenarioWithQueryInQueryHistory(t, "When users tries to unstar query in query history that does not exists, it should fail",
+		func(t *testing.T, sc scenarioContext) {
+			resp := sc.service.starHandler(sc.reqContext)
+			require.Equal(t, 500, resp.Status())
+		})
+
+	testScenarioWithQueryInQueryHistory(t, "When users tries to unstar starred query in query history, 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})
+			sc.service.starHandler(sc.reqContext)
+			resp := sc.service.unstarHandler(sc.reqContext)
+			fmt.Println(resp)
+			require.Equal(t, 200, resp.Status())
+		})
+
+	testScenarioWithQueryInQueryHistory(t, "When users tries to unstar query in query history that is not starred, it should fail",
+		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.unstarHandler(sc.reqContext)
+			require.Equal(t, 500, resp.Status())
+		})
+}
--- a/pkg/services/sqlstore/migrations/migrations.go
+++ b/pkg/services/sqlstore/migrations/migrations.go
@ -78,6 +78,7 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
 			accesscontrol.AddTeamMembershipMigrations(mg)
 		}
 	}
+	addQueryHistoryStarMigrations(mg)

 	if mg.Cfg != nil && mg.Cfg.IsFeatureToggleEnabled != nil {
 		if mg.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagDashboardComments) || mg.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAnnotationComments) {
--- a/pkg/services/sqlstore/migrations/query_history_star_mig.go
+++ b/pkg/services/sqlstore/migrations/query_history_star_mig.go
@ -0,0 +1,23 @@
+package migrations
+
+import (
+	. "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
+)
+
+func addQueryHistoryStarMigrations(mg *Migrator) {
+	queryHistoryStarV1 := Table{
+		Name: "query_history_star",
+		Columns: []*Column{
+			{Name: "id", Type: DB_BigInt, Nullable: false, IsPrimaryKey: true, IsAutoIncrement: true},
+			{Name: "query_uid", Type: DB_NVarchar, Length: 40, Nullable: false},
+			{Name: "user_id", Type: DB_Int, Nullable: false},
+		},
+		Indices: []*Index{
+			{Cols: []string{"user_id", "query_uid"}, Type: UniqueIndex},
+		},
+	}
+
+	mg.AddMigration("create query_history_star table v1", NewAddTableMigration(queryHistoryStarV1))
+
+	mg.AddMigration("add index query_history.user_id-query_uid", NewAddIndexMigration(queryHistoryStarV1, queryHistoryStarV1.Indices[0]))
+}