From 6f941b54201302b8be4d20da1763b671078625a9 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 01:07:27 +0100 Subject: [PATCH 01/18] reject reactions wich ar not allowed --- models/error.go | 15 +++++++++++++++ models/issue_reaction.go | 5 +++++ 2 files changed, 20 insertions(+) diff --git a/models/error.go b/models/error.go index 313c36354df30..d2c678355cfe3 100644 --- a/models/error.go +++ b/models/error.go @@ -1121,6 +1121,21 @@ func (err ErrNewIssueInsert) Error() string { return err.OriginalError.Error() } +// ErrNewIssueReaction is used when a forbidden Issue was try to created +type ErrForbiddenIssueReaction struct { + Reaction string +} + +// IsErrForbiddenIssueReaction checks if an error is a ErrForbiddenIssueReaction. +func IsErrForbiddenIssueReaction(err error) bool { + _, ok := err.(ErrForbiddenIssueReaction) + return ok +} + +func (err ErrForbiddenIssueReaction) Error() string { + return fmt.Sprintf("'%s' is not an allowed reaction", err.Reaction) +} + // __________ .__ .__ __________ __ // \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_ // | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\ diff --git a/models/issue_reaction.go b/models/issue_reaction.go index 4596d32d06a79..b04865157f7e7 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" "xorm.io/xorm" @@ -77,6 +78,10 @@ type ReactionOptions struct { // CreateReaction creates reaction for issue or comment. func CreateReaction(opts *ReactionOptions) (reaction *Reaction, err error) { + if !util.IsStringInSlice(opts.Type, setting.UI.Reactions) { + return nil, ErrForbiddenIssueReaction{opts.Type} + } + sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { From d430a766478ecbea6bbb6da8ed5e1336b975cf33 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 01:23:45 +0100 Subject: [PATCH 02/18] dont duble check CreateReaction now throw ErrForbiddenIssueReaction --- routers/repo/issue.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index c79ea02e86f37..32d8749acb46d 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1463,12 +1463,6 @@ func ChangeIssueReaction(ctx *context.Context, form auth.ReactionForm) { switch ctx.Params(":action") { case "react": - if !util.IsStringInSlice(form.Content, setting.UI.Reactions) { - err := fmt.Errorf("ChangeIssueReaction: '%s' is not an allowed reaction", form.Content) - ctx.ServerError(err.Error(), err) - return - } - reaction, err := models.CreateIssueReaction(ctx.User, issue, form.Content) if err != nil { log.Info("CreateIssueReaction: %s", err) @@ -1564,12 +1558,6 @@ func ChangeCommentReaction(ctx *context.Context, form auth.ReactionForm) { switch ctx.Params(":action") { case "react": - if !util.IsStringInSlice(form.Content, setting.UI.Reactions) { - err := fmt.Errorf("ChangeIssueReaction: '%s' is not an allowed reaction", form.Content) - ctx.ServerError(err.Error(), err) - return - } - reaction, err := models.CreateCommentReaction(ctx.User, comment.Issue, comment, form.Content) if err != nil { log.Info("CreateCommentReaction: %s", err) From 7c5739e99c9472c02e7764f06173f83b4aa8d92d Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 01:54:29 +0100 Subject: [PATCH 03/18] add /repos/{owner}/{repo}/issues/comments/{id}/reactions endpoint --- modules/structs/issue_reaction.go | 20 +++ routers/api/v1/api.go | 12 +- routers/api/v1/repo/issue_reaction.go | 197 ++++++++++++++++++++++++++ routers/api/v1/swagger/issue.go | 21 +++ templates/swagger/v1_json.tmpl | 191 +++++++++++++++++++++++++ 5 files changed, 438 insertions(+), 3 deletions(-) create mode 100644 modules/structs/issue_reaction.go create mode 100644 routers/api/v1/repo/issue_reaction.go diff --git a/modules/structs/issue_reaction.go b/modules/structs/issue_reaction.go new file mode 100644 index 0000000000000..af0ae7aaa80df --- /dev/null +++ b/modules/structs/issue_reaction.go @@ -0,0 +1,20 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package structs + +import ( + "time" +) + +type EditReactionOption struct { + Reaction string `json:"content"` +} + +type ReactionResponse struct { + User *User `json:"user"` + Reaction string `json:"content"` + // swagger:strfmt date-time + Created time.Time `json:"created_at"` +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 47c9c95c7fe00..a11af5391bae3 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -657,9 +657,15 @@ func RegisterRoutes(m *macaron.Macaron) { Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueOption{}), repo.CreateIssue) m.Group("/comments", func() { m.Get("", repo.ListRepoIssueComments) - m.Combo("/:id", reqToken()). - Patch(mustNotBeArchived, bind(api.EditIssueCommentOption{}), repo.EditIssueComment). - Delete(repo.DeleteIssueComment) + m.Group("/:id", func() { + m.Combo("", reqToken()). + Patch(mustNotBeArchived, bind(api.EditIssueCommentOption{}), repo.EditIssueComment). + Delete(repo.DeleteIssueComment) + m.Combo("/reactions", reqToken()). + Get(repo.GetIssueCommentReactions). + Post(bind(api.EditReactionOption{}), repo.PostIssueCommentReaction). + Delete(bind(api.EditReactionOption{}), repo.DeleteIssueCommentReaction) + }) }) m.Group("/:index", func() { m.Combo("").Get(repo.GetIssue). diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go new file mode 100644 index 0000000000000..b287e827b70fb --- /dev/null +++ b/routers/api/v1/repo/issue_reaction.go @@ -0,0 +1,197 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "errors" + "time" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/context" + api "code.gitea.io/gitea/modules/structs" +) + +// PostIssueCommentReaction add a reaction to a comment of a issue +func PostIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOption) { + // swagger:operation POST /repos/{owner}/{repo}/issues/comments/{id}/reactions issue issuePostCommentReaction + // --- + // summary: Add a reaction to a comment of a issue + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: id of the comment to edit + // type: integer + // format: int64 + // required: true + // - name: content + // in: body + // schema: + // "$ref": "#/definitions/EditReactionOption" + // responses: + // "201": + // "$ref": "#/responses/ReactionResponse" + changeIssueCommentReaction(ctx, form, true) +} + +// DeleteIssueCommentReaction list reactions of a issue comment +func DeleteIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOption) { + // swagger:operation DELETE /repos/{owner}/{repo}/issues/comments/{id}/reactions issue issueDeleteCommentReaction + // --- + // summary: Remove a reaction from a comment of a issue + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: id of the comment to edit + // type: integer + // format: int64 + // required: true + // - name: content + // in: body + // schema: + // "$ref": "#/definitions/EditReactionOption" + // responses: + // "200": + // "$ref": "#/responses/empty" + changeIssueCommentReaction(ctx, form, false) +} + +func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOption, isCreateType bool) { + comment, err := models.GetCommentByID(ctx.ParamsInt64(":id")) + if err != nil { + if models.IsErrCommentNotExist(err) { + ctx.NotFound(err) + } else { + ctx.Error(500, "GetCommentByID", err) + } + return + } + + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + ctx.Error(500, "GetIssueByIndex", err) + return + } + + if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin { + ctx.Error(403, "CreateIssueComment", errors.New(ctx.Tr("repo.issues.comment_on_locked"))) + return + } + + if isCreateType { + // PostIssueCommentReaction part + reaction, err := models.CreateCommentReaction(ctx.User, comment.Issue, comment, form.Reaction) + if err != nil { + if models.IsErrForbiddenIssueReaction(err) { + ctx.Error(403, err.Error(), err) + } else { + ctx.Error(500, "CreateCommentReaction", err) + } + return + } + + ctx.JSON(201, api.ReactionResponse{ + User: reaction.User.APIFormat(), + Reaction: reaction.Type, + Created: reaction.CreatedUnix.AsTime(), + }) + } else { + // DeleteIssueCommentReaction part + err = models.DeleteReaction( + &models.ReactionOptions{ + Type: form.Reaction, + Issue: comment.Issue, + Comment: comment, + Doer: ctx.User, + }) + if err != nil { + ctx.Error(500, "DeleteReaction", err) + return + } + ctx.Status(200) + } +} + +// GetIssueCommentReactions list reactions of a issue comment +func GetIssueCommentReactions(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/issues/comments/{id}/reactions issue issueGetCommentReactions + // --- + // summary: Get a list reactions of a issue comment + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: id of the comment to edit + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/ReactionResponseList" + + r1 := api.ReactionResponse{ + Reaction: "heart", + Created: time.Now(), + User: &api.User{ + UserName: "aaaaa", + Language: "not DE", + }, + } + + r2 := api.ReactionResponse{ + Reaction: "rocket", + Created: time.Now(), + User: &api.User{ + UserName: "user2", + Language: "not EN", + }, + } + + var r []api.ReactionResponse + r = append(r, r1) + r = append(r, r2) + + ctx.JSON(200, r) +} diff --git a/routers/api/v1/swagger/issue.go b/routers/api/v1/swagger/issue.go index c06186bf6cd72..a78c2982fd706 100644 --- a/routers/api/v1/swagger/issue.go +++ b/routers/api/v1/swagger/issue.go @@ -84,3 +84,24 @@ type swaggerIssueDeadline struct { // in:body Body api.IssueDeadline `json:"body"` } + +// EditReactionOption +// swagger:response EditReactionOption +type swaggerEditReactionOption struct { + // in:body + Body api.EditReactionOption `json:"body"` +} + +// ReactionResponse +// swagger:response ReactionResponse +type swaggerReactionResponse struct { + // in:body + Body api.ReactionResponse `json:"body"` +} + +// ReactionResponseList +// swagger:response ReactionResponseList +type swaggerReactionResponseList struct { + // in:body + Body []api.ReactionResponse `json:"body"` +} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 1d8cd3685876f..400622084766c 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -3016,6 +3016,148 @@ } } }, + "/repos/{owner}/{repo}/issues/comments/{id}/reactions": { + "get": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Get a list reactions of a issue comment", + "operationId": "issueGetCommentReactions", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the comment to edit", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/ReactionResponseList" + } + } + }, + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Add a reaction to a comment of a issue", + "operationId": "issuePostCommentReaction", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the comment to edit", + "name": "id", + "in": "path", + "required": true + }, + { + "name": "content", + "in": "body", + "schema": { + "$ref": "#/definitions/EditReactionOption" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/ReactionResponse" + } + } + }, + "delete": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Remove a reaction from a comment of a issue", + "operationId": "issueDeleteCommentReaction", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the comment to edit", + "name": "id", + "in": "path", + "required": true + }, + { + "name": "content", + "in": "body", + "schema": { + "$ref": "#/definitions/EditReactionOption" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/empty" + } + } + } + }, "/repos/{owner}/{repo}/issues/{id}/times": { "get": { "produces": [ @@ -8721,6 +8863,16 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "EditReactionOption": { + "type": "object", + "properties": { + "content": { + "type": "string", + "x-go-name": "Reaction" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "EditReleaseOption": { "description": "EditReleaseOption options when editing a release", "type": "object", @@ -10095,6 +10247,24 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "ReactionResponse": { + "type": "object", + "properties": { + "content": { + "type": "string", + "x-go-name": "Reaction" + }, + "created_at": { + "type": "string", + "format": "date-time", + "x-go-name": "Created" + }, + "user": { + "$ref": "#/definitions/User" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "Reference": { "type": "object", "title": "Reference represents a Git reference.", @@ -10960,6 +11130,12 @@ } } }, + "EditReactionOption": { + "description": "EditReactionOption", + "schema": { + "$ref": "#/definitions/EditReactionOption" + } + }, "EmailList": { "description": "EmailList", "schema": { @@ -11146,6 +11322,21 @@ } } }, + "ReactionResponse": { + "description": "ReactionResponse", + "schema": { + "$ref": "#/definitions/ReactionResponse" + } + }, + "ReactionResponseList": { + "description": "ReactionResponseList", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/ReactionResponse" + } + } + }, "Reference": { "description": "Reference", "schema": { From 7a041cae6d09aa190c2ffdf092f507f0cb7b224c Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 03:00:03 +0100 Subject: [PATCH 04/18] add Find Functions --- models/issue_reaction.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/models/issue_reaction.go b/models/issue_reaction.go index b04865157f7e7..009e1f9276ab6 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -44,6 +44,21 @@ func (opts *FindReactionsOptions) toConds() builder.Cond { return cond } +//FindIssueReactions returns a ReactionList of all reactions from an comment +func FindCommentReactions(comment *Comment) (ReactionList, error) { + return findReactions(x, FindReactionsOptions{ + IssueID: comment.IssueID, + CommentID: comment.ID}) +} + +//FindIssueReactions returns a ReactionList of all reactions from an issue +func FindIssueReactions(issue *Issue) (ReactionList, error) { + return findReactions(x, FindReactionsOptions{ + IssueID: issue.ID, + CommentID: 0, + }) +} + func findReactions(e Engine, opts FindReactionsOptions) ([]*Reaction, error) { reactions := make([]*Reaction, 0, 10) sess := e.Where(opts.toConds()) From e8cd6e2b48b8d5ba854c6a68092bf7c02a82bd64 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 03:17:56 +0100 Subject: [PATCH 05/18] fix some swagger stuff + add issue reaction endpoints + GET ReactionList now use FindReactions... --- routers/api/v1/api.go | 8 +- routers/api/v1/repo/issue_reaction.go | 259 ++++++++++++++++++++++---- templates/swagger/v1_json.tmpl | 146 ++++++++++++++- 3 files changed, 372 insertions(+), 41 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index a11af5391bae3..cd5fc1f3eb27d 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -670,14 +670,12 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/:index", func() { m.Combo("").Get(repo.GetIssue). Patch(reqToken(), bind(api.EditIssueOption{}), repo.EditIssue) - m.Group("/comments", func() { m.Combo("").Get(repo.ListIssueComments). Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueCommentOption{}), repo.CreateIssueComment) m.Combo("/:id", reqToken()).Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueCommentDeprecated). Delete(repo.DeleteIssueCommentDeprecated) }) - m.Group("/labels", func() { m.Combo("").Get(repo.ListIssueLabels). Post(reqToken(), bind(api.IssueLabelsOption{}), repo.AddIssueLabels). @@ -685,12 +683,10 @@ func RegisterRoutes(m *macaron.Macaron) { Delete(reqToken(), repo.ClearIssueLabels) m.Delete("/:id", reqToken(), repo.DeleteIssueLabel) }) - m.Group("/times", func() { m.Combo("").Get(repo.ListTrackedTimes). Post(reqToken(), bind(api.AddTimeOption{}), repo.AddTime) }) - m.Combo("/deadline").Post(reqToken(), bind(api.EditDeadlineOption{}), repo.UpdateIssueDeadline) m.Group("/stopwatch", func() { m.Post("/start", reqToken(), repo.StartIssueStopwatch) @@ -701,6 +697,10 @@ func RegisterRoutes(m *macaron.Macaron) { m.Put("/:user", reqToken(), repo.AddIssueSubscription) m.Delete("/:user", reqToken(), repo.DelIssueSubscription) }) + m.Combo("/reactions", reqToken()). + Get(repo.GetIssueReactions). + Post(bind(api.EditReactionOption{}), repo.PostIssueReaction). + Delete(bind(api.EditReactionOption{}), repo.DeleteIssueReaction) }) }, mustEnableIssuesOrPulls) m.Group("/labels", func() { diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go index b287e827b70fb..462c9c01330be 100644 --- a/routers/api/v1/repo/issue_reaction.go +++ b/routers/api/v1/repo/issue_reaction.go @@ -13,11 +13,78 @@ import ( api "code.gitea.io/gitea/modules/structs" ) +// GetIssueCommentReactions list reactions of a issue comment +func GetIssueCommentReactions(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/issues/comments/{id}/reactions issue issueGetCommentReactions + // --- + // summary: Get a list reactions of a issue comment + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: id of the comment to edit + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/ReactionResponseList" + comment, err := models.GetCommentByID(ctx.ParamsInt64(":id")) + if err != nil { + if models.IsErrCommentNotExist(err) { + ctx.NotFound(err) + } else { + ctx.Error(500, "GetCommentByID", err) + } + return + } + + if !ctx.Repo.CanRead(models.UnitTypeIssues) && !ctx.User.IsAdmin { + ctx.Error(403, "GetIssueCommentReactions", errors.New("no permission to get reactions")) + return + } + + reactions, err := models.FindCommentReactions(comment) + if err != nil { + ctx.Error(500, "FindIssueReactions", err) + return + } + _, err = reactions.LoadUsers() + if err != nil { + ctx.Error(500, "ReactionList.LoadUsers()", err) + return + } + + var result []api.ReactionResponse + for _, r := range reactions { + result = append(result, api.ReactionResponse{ + User: r.User.APIFormat(), + Reaction: r.Type, + Created: r.CreatedUnix.AsTime(), + }) + } + + ctx.JSON(200, result) +} + // PostIssueCommentReaction add a reaction to a comment of a issue func PostIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOption) { // swagger:operation POST /repos/{owner}/{repo}/issues/comments/{id}/reactions issue issuePostCommentReaction // --- - // summary: Add a reaction to a comment of a issue + // summary: Add a reaction to a comment of a issue comment // consumes: // - application/json // produces: @@ -53,7 +120,7 @@ func PostIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOpti func DeleteIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOption) { // swagger:operation DELETE /repos/{owner}/{repo}/issues/comments/{id}/reactions issue issueDeleteCommentReaction // --- - // summary: Remove a reaction from a comment of a issue + // summary: Remove a reaction from a comment of a issue comment // consumes: // - application/json // produces: @@ -103,7 +170,7 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp } if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin { - ctx.Error(403, "CreateIssueComment", errors.New(ctx.Tr("repo.issues.comment_on_locked"))) + ctx.Error(403, "ChangeIssueCommentReaction", errors.New("no permission to change reaction")) return } @@ -126,26 +193,20 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp }) } else { // DeleteIssueCommentReaction part - err = models.DeleteReaction( - &models.ReactionOptions{ - Type: form.Reaction, - Issue: comment.Issue, - Comment: comment, - Doer: ctx.User, - }) + err = models.DeleteCommentReaction(ctx.User, comment.Issue, comment, form.Reaction) if err != nil { - ctx.Error(500, "DeleteReaction", err) + ctx.Error(500, "DeleteCommentReaction", err) return } ctx.Status(200) } } -// GetIssueCommentReactions list reactions of a issue comment -func GetIssueCommentReactions(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/issues/comments/{id}/reactions issue issueGetCommentReactions +// GetIssueReactions list reactions of a issue comment +func GetIssueReactions(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/issues/{index}/reactions issue issueGetIssueReactions // --- - // summary: Get a list reactions of a issue comment + // summary: Get a list reactions of a issue // consumes: // - application/json // produces: @@ -161,37 +222,165 @@ func GetIssueCommentReactions(ctx *context.APIContext) { // description: name of the repo // type: string // required: true - // - name: id + // - name: index // in: path - // description: id of the comment to edit + // description: index of the issue // type: integer // format: int64 // required: true // responses: // "200": // "$ref": "#/responses/ReactionResponseList" + issue, err := models.GetIssueWithAttrsByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + if models.IsErrIssueNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(500, "GetIssueByIndex", err) + } + return + } + + if !ctx.Repo.CanRead(models.UnitTypeIssues) && !ctx.User.IsAdmin { + ctx.Error(403, "GetIssueReactions", errors.New("no permission to get reactions")) + return + } + + reactions, err := models.FindIssueReactions(issue) + if err != nil { + ctx.Error(500, "FindIssueReactions", err) + return + } + _, err = reactions.LoadUsers() + if err != nil { + ctx.Error(500, "ReactionList.LoadUsers()", err) + return + } + + var result []api.ReactionResponse + for _, r := range reactions { + result = append(result, api.ReactionResponse{ + User: r.User.APIFormat(), + Reaction: r.Type, + Created: r.CreatedUnix.AsTime(), + }) + } + + ctx.JSON(200, result) +} + +// PostIssueReaction add a reaction to a comment of a issue +func PostIssueReaction(ctx *context.APIContext, form api.EditReactionOption) { + // swagger:operation POST /repos/{owner}/{repo}/issues/{index}/reactions issue issuePostIssueReaction + // --- + // summary: Add a reaction to a comment of a issue + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: index + // in: path + // description: index of the issue + // type: integer + // format: int64 + // required: true + // - name: content + // in: body + // schema: + // "$ref": "#/definitions/EditReactionOption" + // responses: + // "201": + // "$ref": "#/responses/ReactionResponse" + changeIssueReaction(ctx, form, true) +} + +// DeleteIssueReaction list reactions of a issue comment +func DeleteIssueReaction(ctx *context.APIContext, form api.EditReactionOption) { + // swagger:operation DELETE /repos/{owner}/{repo}/issues/{index}/reactions issue issueDeleteIssueReaction + // --- + // summary: Remove a reaction from a comment of a issue + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: index + // in: path + // description: index of the issue + // type: integer + // format: int64 + // required: true + // - name: content + // in: body + // schema: + // "$ref": "#/definitions/EditReactionOption" + // responses: + // "200": + // "$ref": "#/responses/empty" + changeIssueReaction(ctx, form, false) +} - r1 := api.ReactionResponse{ - Reaction: "heart", - Created: time.Now(), - User: &api.User{ - UserName: "aaaaa", - Language: "not DE", - }, +func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, isCreateType bool) { + issue, err := models.GetIssueWithAttrsByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + if models.IsErrIssueNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(500, "GetIssueByIndex", err) + } + return } - r2 := api.ReactionResponse{ - Reaction: "rocket", - Created: time.Now(), - User: &api.User{ - UserName: "user2", - Language: "not EN", - }, + if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin { + ctx.Error(403, "ChangeIssueCommentReaction", errors.New("no permission to change reaction")) + return } - var r []api.ReactionResponse - r = append(r, r1) - r = append(r, r2) + if isCreateType { + // PostIssueReaction part + reaction, err := models.CreateIssueReaction(ctx.User, issue, form.Reaction) + if err != nil { + if models.IsErrForbiddenIssueReaction(err) { + ctx.Error(403, err.Error(), err) + } else { + ctx.Error(500, "CreateCommentReaction", err) + } + return + } - ctx.JSON(200, r) + ctx.JSON(201, api.ReactionResponse{ + User: reaction.User.APIFormat(), + Reaction: reaction.Type, + Created: reaction.CreatedUnix.AsTime(), + }) + } else { + // DeleteIssueReaction part + err = models.DeleteIssueReaction(ctx.User, issue, form.Reaction) + if err != nil { + ctx.Error(500, "DeleteIssueReaction", err) + return + } + ctx.Status(200) + } } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 400622084766c..13375f0d3a0bf 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -3069,7 +3069,7 @@ "tags": [ "issue" ], - "summary": "Add a reaction to a comment of a issue", + "summary": "Add a reaction to a comment of a issue comment", "operationId": "issuePostCommentReaction", "parameters": [ { @@ -3118,7 +3118,7 @@ "tags": [ "issue" ], - "summary": "Remove a reaction from a comment of a issue", + "summary": "Remove a reaction from a comment of a issue comment", "operationId": "issueDeleteCommentReaction", "parameters": [ { @@ -3830,6 +3830,148 @@ } } }, + "/repos/{owner}/{repo}/issues/{index}/reactions": { + "get": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Get a list reactions of a issue", + "operationId": "issueGetIssueReactions", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/ReactionResponseList" + } + } + }, + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Add a reaction to a comment of a issue", + "operationId": "issuePostIssueReaction", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + }, + { + "name": "content", + "in": "body", + "schema": { + "$ref": "#/definitions/EditReactionOption" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/ReactionResponse" + } + } + }, + "delete": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Remove a reaction from a comment of a issue", + "operationId": "issueDeleteIssueReaction", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + }, + { + "name": "content", + "in": "body", + "schema": { + "$ref": "#/definitions/EditReactionOption" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/empty" + } + } + } + }, "/repos/{owner}/{repo}/issues/{index}/stopwatch/start": { "post": { "consumes": [ From 492c7d97166c1fb6af6535f7aee8e0f44b803286 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 03:51:43 +0100 Subject: [PATCH 06/18] explicite Issue Only Reaction for FindReactionsOptions with "-1" commentID --- models/issue_reaction.go | 6 +++++- routers/api/v1/repo/issue_reaction.go | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/models/issue_reaction.go b/models/issue_reaction.go index 009e1f9276ab6..3c62f928f2882 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -41,6 +41,10 @@ func (opts *FindReactionsOptions) toConds() builder.Cond { if opts.CommentID > 0 { cond = cond.And(builder.Eq{"reaction.comment_id": opts.CommentID}) } + if opts.CommentID == -1 { + cond = cond.And(builder.Eq{"reaction.comment_id": 0}) + } + return cond } @@ -55,7 +59,7 @@ func FindCommentReactions(comment *Comment) (ReactionList, error) { func FindIssueReactions(issue *Issue) (ReactionList, error) { return findReactions(x, FindReactionsOptions{ IssueID: issue.ID, - CommentID: 0, + CommentID: -1, }) } diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go index 462c9c01330be..9264a0935c367 100644 --- a/routers/api/v1/repo/issue_reaction.go +++ b/routers/api/v1/repo/issue_reaction.go @@ -6,7 +6,6 @@ package repo import ( "errors" - "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" From e3c9dbd93416f1332c57750374aa88364caadd4d Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 04:15:02 +0100 Subject: [PATCH 07/18] load issue; load user ... --- models/issue_reaction.go | 10 ++++++++++ routers/api/v1/repo/issue_reaction.go | 17 +++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/models/issue_reaction.go b/models/issue_reaction.go index 3c62f928f2882..2b5468c137154 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -184,6 +184,16 @@ func DeleteCommentReaction(doer *User, issue *Issue, comment *Comment, content s }) } +// LoadUsers loads reactions' all users +func (r *Reaction) LoadUser() (*User, error) { + user, err := getUserByID(x, r.UserID) + if err != nil { + return nil, err + } + r.User = user + return user, nil +} + // ReactionList represents list of reactions type ReactionList []*Reaction diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go index 9264a0935c367..56e12ccdcc3ca 100644 --- a/routers/api/v1/repo/issue_reaction.go +++ b/routers/api/v1/repo/issue_reaction.go @@ -162,13 +162,12 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp return } - issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + err = comment.LoadIssue() if err != nil { - ctx.Error(500, "GetIssueByIndex", err) - return + ctx.Error(500, "comment.LoadIssue() failed", err) } - if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin { + if comment.Issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin { ctx.Error(403, "ChangeIssueCommentReaction", errors.New("no permission to change reaction")) return } @@ -184,6 +183,11 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp } return } + _, err = reaction.LoadUser() + if err != nil { + ctx.Error(500, "Reaction.LoadUser()", err) + return + } ctx.JSON(201, api.ReactionResponse{ User: reaction.User.APIFormat(), @@ -367,6 +371,11 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i } return } + _, err = reaction.LoadUser() + if err != nil { + ctx.Error(500, "Reaction.LoadUser()", err) + return + } ctx.JSON(201, api.ReactionResponse{ User: reaction.User.APIFormat(), From b4650b55d78d8e4bf22a64f27f186645d033e198 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 04:19:13 +0100 Subject: [PATCH 08/18] return error again --- models/error.go | 2 +- models/issue_reaction.go | 6 +++--- modules/structs/issue_reaction.go | 2 ++ routers/repo/issue.go | 8 ++++++++ 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/models/error.go b/models/error.go index d2c678355cfe3..16be5121390af 100644 --- a/models/error.go +++ b/models/error.go @@ -1121,7 +1121,7 @@ func (err ErrNewIssueInsert) Error() string { return err.OriginalError.Error() } -// ErrNewIssueReaction is used when a forbidden Issue was try to created +// ErrForbiddenIssueReaction is used when a forbidden reaction was try to created type ErrForbiddenIssueReaction struct { Reaction string } diff --git a/models/issue_reaction.go b/models/issue_reaction.go index 2b5468c137154..95ca76f5bc7e6 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -48,14 +48,14 @@ func (opts *FindReactionsOptions) toConds() builder.Cond { return cond } -//FindIssueReactions returns a ReactionList of all reactions from an comment +// FindCommentReactions returns a ReactionList of all reactions from an comment func FindCommentReactions(comment *Comment) (ReactionList, error) { return findReactions(x, FindReactionsOptions{ IssueID: comment.IssueID, CommentID: comment.ID}) } -//FindIssueReactions returns a ReactionList of all reactions from an issue +// FindIssueReactions returns a ReactionList of all reactions from an issue func FindIssueReactions(issue *Issue) (ReactionList, error) { return findReactions(x, FindReactionsOptions{ IssueID: issue.ID, @@ -184,7 +184,7 @@ func DeleteCommentReaction(doer *User, issue *Issue, comment *Comment, content s }) } -// LoadUsers loads reactions' all users +// LoadUser load user of reaction func (r *Reaction) LoadUser() (*User, error) { user, err := getUserByID(x, r.UserID) if err != nil { diff --git a/modules/structs/issue_reaction.go b/modules/structs/issue_reaction.go index af0ae7aaa80df..9d71740052001 100644 --- a/modules/structs/issue_reaction.go +++ b/modules/structs/issue_reaction.go @@ -8,10 +8,12 @@ import ( "time" ) +// EditReactionOption contain the reaction type type EditReactionOption struct { Reaction string `json:"content"` } +// ReactionResponse contain one reaction type ReactionResponse struct { User *User `json:"user"` Reaction string `json:"content"` diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 32d8749acb46d..5d5aaca253f25 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1465,6 +1465,10 @@ func ChangeIssueReaction(ctx *context.Context, form auth.ReactionForm) { case "react": reaction, err := models.CreateIssueReaction(ctx.User, issue, form.Content) if err != nil { + if models.IsErrForbiddenIssueReaction(err) { + ctx.ServerError("ChangeIssueReaction", err) + return + } log.Info("CreateIssueReaction: %s", err) break } @@ -1560,6 +1564,10 @@ func ChangeCommentReaction(ctx *context.Context, form auth.ReactionForm) { case "react": reaction, err := models.CreateCommentReaction(ctx.User, comment.Issue, comment, form.Content) if err != nil { + if models.IsErrForbiddenIssueReaction(err) { + ctx.ServerError("ChangeIssueReaction", err) + return + } log.Info("CreateCommentReaction: %s", err) break } From 895455242815f22058a1bd63242c449d4f316dcb Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 04:36:53 +0100 Subject: [PATCH 09/18] swagger def canged after LINT --- templates/swagger/v1_json.tmpl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 13375f0d3a0bf..9c8db28817aba 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -9006,6 +9006,7 @@ "x-go-package": "code.gitea.io/gitea/modules/structs" }, "EditReactionOption": { + "description": "EditReactionOption contain the reaction type", "type": "object", "properties": { "content": { @@ -10390,6 +10391,7 @@ "x-go-package": "code.gitea.io/gitea/modules/structs" }, "ReactionResponse": { + "description": "ReactionResponse contain one reaction", "type": "object", "properties": { "content": { From b2780620b717a3b36729ba94976853b33e1eabc3 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 16:40:17 +0100 Subject: [PATCH 10/18] check if user has ben loaded --- models/issue_reaction.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/issue_reaction.go b/models/issue_reaction.go index 95ca76f5bc7e6..2548457b225e3 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -186,6 +186,9 @@ func DeleteCommentReaction(doer *User, issue *Issue, comment *Comment, content s // LoadUser load user of reaction func (r *Reaction) LoadUser() (*User, error) { + if r.User != nil { + return r.User, nil + } user, err := getUserByID(x, r.UserID) if err != nil { return nil, err From 6d6d4edd2115c50ffe3d39cb39d68ec8ed3c0715 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 4 Dec 2019 15:42:03 +0100 Subject: [PATCH 11/18] add Tests --- integrations/api_issue_reaction_test.go | 129 ++++++++++++++++++++++++ models/fixtures/reaction.yml | 40 +++++++- 2 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 integrations/api_issue_reaction_test.go diff --git a/integrations/api_issue_reaction_test.go b/integrations/api_issue_reaction_test.go new file mode 100644 index 0000000000000..7a7cf60efd91c --- /dev/null +++ b/integrations/api_issue_reaction_test.go @@ -0,0 +1,129 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "fmt" + "net/http" + "testing" + "time" + + "code.gitea.io/gitea/models" + api "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" +) + +func TestAPIIssuesReactions(t *testing.T) { + defer prepareTestEnv(t)() + + issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1}).(*models.Issue) + _ = issue.LoadRepo() + owner := models.AssertExistsAndLoadBean(t, &models.User{ID: issue.Repo.OwnerID}).(*models.User) + + session := loginUser(t, owner.Name) + token := getTokenForLoggedInUser(t, session) + + user1 := models.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User) + user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/reactions?token=%s", + owner.Name, issue.Repo.Name, issue.Index, token) + + //Try to add not allowed reaction + req := NewRequestWithJSON(t, "POST", urlStr, &api.EditReactionOption{ + Reaction: "wrong", + }) + resp := session.MakeRequest(t, req, http.StatusForbidden) + + //Delete not allowed reaction + req = NewRequestWithJSON(t, "DELETE", urlStr, &api.EditReactionOption{ + Reaction: "zzz", + }) + resp = session.MakeRequest(t, req, http.StatusOK) + + //Add allowed reaction + req = NewRequestWithJSON(t, "POST", urlStr, &api.EditReactionOption{ + Reaction: "rocket", + }) + resp = session.MakeRequest(t, req, http.StatusCreated) + var apiNewReaction api.ReactionResponse + DecodeJSON(t, resp, &apiNewReaction) + + //Get end result of reaction list of issue #1 + req = NewRequestf(t, "GET", urlStr) + resp = session.MakeRequest(t, req, http.StatusOK) + var apiReactions []*api.ReactionResponse + DecodeJSON(t, resp, &apiReactions) + var expectResponse []*api.ReactionResponse + expectResponse = append(expectResponse, &api.ReactionResponse{ + User: user1.APIFormat(), + Reaction: "zzz", + Created: time.Unix(1573248002, 0), + }) + expectResponse = append(expectResponse, &api.ReactionResponse{ + User: user2.APIFormat(), + Reaction: "eyes", + Created: time.Unix(1573248003, 0), + }) + expectResponse = append(expectResponse, &apiNewReaction) + assert.Equal(t, expectResponse, apiReactions) +} + +func TestAPICommentReactions(t *testing.T) { + defer prepareTestEnv(t)() + + comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2}).(*models.Comment) + _ = comment.LoadIssue() + issue := comment.Issue + _ = issue.LoadRepo() + owner := models.AssertExistsAndLoadBean(t, &models.User{ID: issue.Repo.OwnerID}).(*models.User) + + session := loginUser(t, owner.Name) + token := getTokenForLoggedInUser(t, session) + + user1 := models.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User) + user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d/reactions?token=%s", + owner.Name, issue.Repo.Name, comment.ID, token) + + //Try to add not allowed reaction + req := NewRequestWithJSON(t, "POST", urlStr, &api.EditReactionOption{ + Reaction: "wrong", + }) + resp := session.MakeRequest(t, req, http.StatusForbidden) + + //Delete none existing reaction + req = NewRequestWithJSON(t, "DELETE", urlStr, &api.EditReactionOption{ + Reaction: "eyes", + }) + resp = session.MakeRequest(t, req, http.StatusOK) + + //Add allowed reaction + req = NewRequestWithJSON(t, "POST", urlStr, &api.EditReactionOption{ + Reaction: "+1", + }) + resp = session.MakeRequest(t, req, http.StatusCreated) + var apiNewReaction api.ReactionResponse + DecodeJSON(t, resp, &apiNewReaction) + + //Get end result of reaction list of issue #1 + req = NewRequestf(t, "GET", urlStr) + resp = session.MakeRequest(t, req, http.StatusOK) + var apiReactions []*api.ReactionResponse + DecodeJSON(t, resp, &apiReactions) + var expectResponse []*api.ReactionResponse + expectResponse = append(expectResponse, &api.ReactionResponse{ + User: user2.APIFormat(), + Reaction: "laugh", + Created: time.Unix(1573248004, 0), + }) + expectResponse = append(expectResponse, &api.ReactionResponse{ + User: user1.APIFormat(), + Reaction: "laugh", + Created: time.Unix(1573248005, 0), + }) + expectResponse = append(expectResponse, &apiNewReaction) + assert.Equal(t, expectResponse, apiReactions) +} diff --git a/models/fixtures/reaction.yml b/models/fixtures/reaction.yml index ca780a73aa0c1..4925935fe6fe6 100644 --- a/models/fixtures/reaction.yml +++ b/models/fixtures/reaction.yml @@ -1 +1,39 @@ -[] # empty +- + id: 1 #issue reaction + type: zzz # not allowed reaction (added before allowed reaction list has changed) + issue_id: 1 + comment_id: 0 + user_id: 2 + created_unix: 1573248001 + +- + id: 2 #issue reaction + type: zzz # not allowed reaction (added before allowed reaction list has changed) + issue_id: 1 + comment_id: 0 + user_id: 1 + created_unix: 1573248002 + +- + id: 3 #issue reaction + type: eyes # allowed reaction + issue_id: 1 + comment_id: 0 + user_id: 2 + created_unix: 1573248003 + +- + id: 4 #comment reaction + type: laugh # allowed reaction + issue_id: 1 + comment_id: 2 + user_id: 2 + created_unix: 1573248004 + +- + id: 5 #comment reaction + type: laugh # allowed reaction + issue_id: 1 + comment_id: 2 + user_id: 1 + created_unix: 1573248005 From 682c6ffd2d50b0585309bb8b270b729f4e97e987 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 4 Dec 2019 16:17:28 +0100 Subject: [PATCH 12/18] better way of comparing results --- integrations/api_issue_reaction_test.go | 38 ++++++++++++++++--------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/integrations/api_issue_reaction_test.go b/integrations/api_issue_reaction_test.go index 7a7cf60efd91c..ef20199a47fa7 100644 --- a/integrations/api_issue_reaction_test.go +++ b/integrations/api_issue_reaction_test.go @@ -56,19 +56,24 @@ func TestAPIIssuesReactions(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusOK) var apiReactions []*api.ReactionResponse DecodeJSON(t, resp, &apiReactions) - var expectResponse []*api.ReactionResponse - expectResponse = append(expectResponse, &api.ReactionResponse{ + expectResponse := make(map[int]api.ReactionResponse) + expectResponse[0] = api.ReactionResponse{ User: user1.APIFormat(), Reaction: "zzz", Created: time.Unix(1573248002, 0), - }) - expectResponse = append(expectResponse, &api.ReactionResponse{ + } + expectResponse[1] = api.ReactionResponse{ User: user2.APIFormat(), Reaction: "eyes", Created: time.Unix(1573248003, 0), - }) - expectResponse = append(expectResponse, &apiNewReaction) - assert.Equal(t, expectResponse, apiReactions) + } + expectResponse[2] = apiNewReaction + assert.Len(t, apiReactions, 3) + for i, r := range apiReactions { + assert.Equal(t, expectResponse[i].Reaction, r.Reaction) + assert.Equal(t, expectResponse[i].Created, r.Created) + assert.Equal(t, expectResponse[i].User.ID, r.User.ID) + } } func TestAPICommentReactions(t *testing.T) { @@ -113,17 +118,22 @@ func TestAPICommentReactions(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusOK) var apiReactions []*api.ReactionResponse DecodeJSON(t, resp, &apiReactions) - var expectResponse []*api.ReactionResponse - expectResponse = append(expectResponse, &api.ReactionResponse{ + expectResponse := make(map[int]api.ReactionResponse) + expectResponse[0] = api.ReactionResponse{ User: user2.APIFormat(), Reaction: "laugh", Created: time.Unix(1573248004, 0), - }) - expectResponse = append(expectResponse, &api.ReactionResponse{ + } + expectResponse[1] = api.ReactionResponse{ User: user1.APIFormat(), Reaction: "laugh", Created: time.Unix(1573248005, 0), - }) - expectResponse = append(expectResponse, &apiNewReaction) - assert.Equal(t, expectResponse, apiReactions) + } + expectResponse[2] = apiNewReaction + assert.Len(t, apiReactions, 3) + for i, r := range apiReactions { + assert.Equal(t, expectResponse[i].Reaction, r.Reaction) + assert.Equal(t, expectResponse[i].Created, r.Created) + assert.Equal(t, expectResponse[i].User.ID, r.User.ID) + } } From 1b4279254ced3730e9fbe9538551c8f8ac354e82 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 5 Dec 2019 18:03:21 +0100 Subject: [PATCH 13/18] add suggestion --- models/issue_reaction.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/models/issue_reaction.go b/models/issue_reaction.go index 2548457b225e3..605095b0d7b24 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -40,8 +40,7 @@ func (opts *FindReactionsOptions) toConds() builder.Cond { } if opts.CommentID > 0 { cond = cond.And(builder.Eq{"reaction.comment_id": opts.CommentID}) - } - if opts.CommentID == -1 { + } else if opts.CommentID == -1 { cond = cond.And(builder.Eq{"reaction.comment_id": 0}) } From 5fadb7bfdfc78ecc6309112ec69ab89581f2dadc Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 5 Dec 2019 18:04:09 +0100 Subject: [PATCH 14/18] use different issue for test (dont interfear with integration test) --- models/issue_reaction_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/models/issue_reaction_test.go b/models/issue_reaction_test.go index bbd8cf29fec12..1189b389e9a9c 100644 --- a/models/issue_reaction_test.go +++ b/models/issue_reaction_test.go @@ -81,22 +81,22 @@ func TestIssueReactionCount(t *testing.T) { user4 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User) ghost := NewGhostUser() - issue1 := AssertExistsAndLoadBean(t, &Issue{ID: 1}).(*Issue) + issue := AssertExistsAndLoadBean(t, &Issue{ID: 2}).(*Issue) - addReaction(t, user1, issue1, nil, "heart") - addReaction(t, user2, issue1, nil, "heart") - addReaction(t, user3, issue1, nil, "heart") - addReaction(t, user3, issue1, nil, "+1") - addReaction(t, user4, issue1, nil, "+1") - addReaction(t, user4, issue1, nil, "heart") - addReaction(t, ghost, issue1, nil, "-1") - - err := issue1.loadReactions(x) + addReaction(t, user1, issue, nil, "heart") + addReaction(t, user2, issue, nil, "heart") + addReaction(t, user3, issue, nil, "heart") + addReaction(t, user3, issue, nil, "+1") + addReaction(t, user4, issue, nil, "+1") + addReaction(t, user4, issue, nil, "heart") + addReaction(t, ghost, issue, nil, "-1") + + err := issue.loadReactions(x) assert.NoError(t, err) - assert.Len(t, issue1.Reactions, 7) + assert.Len(t, issue.Reactions, 7) - reactions := issue1.Reactions.GroupByType() + reactions := issue.Reactions.GroupByType() assert.Len(t, reactions["heart"], 4) assert.Equal(t, 2, reactions["heart"].GetMoreUserCount()) assert.Equal(t, user1.DisplayName()+", "+user2.DisplayName(), reactions["heart"].GetFirstUsers()) From f4303c2d690054400bb319f44f9ae96cbd7df1fd Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 5 Dec 2019 18:26:01 +0100 Subject: [PATCH 15/18] test dont compare Location on timeCompare --- integrations/api_issue_reaction_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integrations/api_issue_reaction_test.go b/integrations/api_issue_reaction_test.go index ef20199a47fa7..753298670ef89 100644 --- a/integrations/api_issue_reaction_test.go +++ b/integrations/api_issue_reaction_test.go @@ -71,7 +71,7 @@ func TestAPIIssuesReactions(t *testing.T) { assert.Len(t, apiReactions, 3) for i, r := range apiReactions { assert.Equal(t, expectResponse[i].Reaction, r.Reaction) - assert.Equal(t, expectResponse[i].Created, r.Created) + assert.Equal(t, expectResponse[i].Created.Unix(), r.Created.Unix()) assert.Equal(t, expectResponse[i].User.ID, r.User.ID) } } @@ -133,7 +133,7 @@ func TestAPICommentReactions(t *testing.T) { assert.Len(t, apiReactions, 3) for i, r := range apiReactions { assert.Equal(t, expectResponse[i].Reaction, r.Reaction) - assert.Equal(t, expectResponse[i].Created, r.Created) + assert.Equal(t, expectResponse[i].Created.Unix(), r.Created.Unix()) assert.Equal(t, expectResponse[i].User.ID, r.User.ID) } } From 6a25109e118a2f432812b8733fe529c45ca71d0e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 6 Dec 2019 02:37:23 +0100 Subject: [PATCH 16/18] TEST: add forbidden dubble add --- integrations/api_issue_reaction_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/integrations/api_issue_reaction_test.go b/integrations/api_issue_reaction_test.go index 753298670ef89..f3fcf3946fb47 100644 --- a/integrations/api_issue_reaction_test.go +++ b/integrations/api_issue_reaction_test.go @@ -51,6 +51,9 @@ func TestAPIIssuesReactions(t *testing.T) { var apiNewReaction api.ReactionResponse DecodeJSON(t, resp, &apiNewReaction) + //Add existing reaction + resp = session.MakeRequest(t, req, http.StatusForbidden) + //Get end result of reaction list of issue #1 req = NewRequestf(t, "GET", urlStr) resp = session.MakeRequest(t, req, http.StatusOK) @@ -113,6 +116,9 @@ func TestAPICommentReactions(t *testing.T) { var apiNewReaction api.ReactionResponse DecodeJSON(t, resp, &apiNewReaction) + //Add existing reaction + resp = session.MakeRequest(t, req, http.StatusForbidden) + //Get end result of reaction list of issue #1 req = NewRequestf(t, "GET", urlStr) resp = session.MakeRequest(t, req, http.StatusOK) From 76c175825f07db79e0388410215f7c21550eea25 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 6 Dec 2019 09:08:32 +0100 Subject: [PATCH 17/18] add comments in code to explain --- models/issue_reaction.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/issue_reaction.go b/models/issue_reaction.go index 605095b0d7b24..911dec0185333 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -34,10 +34,14 @@ type FindReactionsOptions struct { } func (opts *FindReactionsOptions) toConds() builder.Cond { + //If Issue ID is set add to Query var cond = builder.NewCond() if opts.IssueID > 0 { cond = cond.And(builder.Eq{"reaction.issue_id": opts.IssueID}) } + //If CommentID is > 0 add to Query + //If it is 0 Query ignore CommentID to select + //If it is -1 it explicit search of Issue Reactions where CommentID = 0 if opts.CommentID > 0 { cond = cond.And(builder.Eq{"reaction.comment_id": opts.CommentID}) } else if opts.CommentID == -1 { From 567af8415725da7a22261d4821bd90614abd1c4e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 Dec 2019 19:44:01 +0100 Subject: [PATCH 18/18] add settings.UI.ReactionsMap so if !setting.UI.ReactionsMap[opts.Type] works --- models/issue_reaction.go | 3 +-- modules/setting/setting.go | 6 ++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/models/issue_reaction.go b/models/issue_reaction.go index 911dec0185333..b4f332a08969e 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -10,7 +10,6 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/modules/util" "xorm.io/builder" "xorm.io/xorm" @@ -100,7 +99,7 @@ type ReactionOptions struct { // CreateReaction creates reaction for issue or comment. func CreateReaction(opts *ReactionOptions) (reaction *Reaction, err error) { - if !util.IsStringInSlice(opts.Type, setting.UI.Reactions) { + if !setting.UI.ReactionsMap[opts.Type] { return nil, ErrForbiddenIssueReaction{opts.Type} } diff --git a/modules/setting/setting.go b/modules/setting/setting.go index c08621df581a7..f55833a0e05fb 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -171,6 +171,7 @@ var ( DefaultTheme string Themes []string Reactions []string + ReactionsMap map[string]bool SearchRepoDescription bool UseServiceWorker bool @@ -985,6 +986,11 @@ func NewContext() { U2F.AppID = sec.Key("APP_ID").MustString(strings.TrimRight(AppURL, "/")) zip.Verbose = false + + UI.ReactionsMap = make(map[string]bool) + for _, reaction := range UI.Reactions { + UI.ReactionsMap[reaction] = true + } } func loadInternalToken(sec *ini.Section) string {