diff --git a/docs/sources/http_api/query_history.md b/docs/sources/http_api/query_history.md index f360f90ea37..2b5833fa2fb 100644 --- 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 diff --git a/pkg/services/queryhistory/api.go b/pkg/services/queryhistory/api.go index 76c76cd0094..2a1104c75ee 100644 --- 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}) +} diff --git a/pkg/services/queryhistory/database.go b/pkg/services/queryhistory/database.go index c8d6f4dc3d9..c926286a3f7 100644 --- 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 diff --git a/pkg/services/queryhistory/models.go b/pkg/services/queryhistory/models.go index f313a8bae1a..dd42fe8218e 100644 --- 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"` diff --git a/pkg/services/queryhistory/queryhistory.go b/pkg/services/queryhistory/queryhistory.go index 71332ae2bd1..40b31836259 100644 --- 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) +} diff --git a/pkg/services/queryhistory/queryhistory_delete_test.go b/pkg/services/queryhistory/queryhistory_delete_test.go index 181cf1f3430..4e1ac3dc60b 100644 --- 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()) + }) } diff --git a/pkg/services/queryhistory/queryhistory_star_test.go b/pkg/services/queryhistory/queryhistory_star_test.go new file mode 100644 index 00000000000..f74fb120a7f --- /dev/null +++ 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()) + }) +} diff --git a/pkg/services/queryhistory/queryhistory_unstar_test.go b/pkg/services/queryhistory/queryhistory_unstar_test.go new file mode 100644 index 00000000000..6ef52c03eec --- /dev/null +++ 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()) + }) +} diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index d65076973ea..bad451879d8 100644 --- 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) { diff --git a/pkg/services/sqlstore/migrations/query_history_star_mig.go b/pkg/services/sqlstore/migrations/query_history_star_mig.go new file mode 100644 index 00000000000..e4ebe662c2a --- /dev/null +++ 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])) +}