Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[API] Fix 9544 | return 200 when reaction already exist #9550

Merged
merged 15 commits into from
Dec 31, 2019
Merged

[API] Fix 9544 | return 200 when reaction already exist #9550

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
1a19846
add ErrReactionAlreadyExist
6543 Dec 30, 2019
93067d9
extend CreateReaction
6543 Dec 30, 2019
73935a6
reaction already exist = 200
6543 Dec 30, 2019
82557ce
extend FindReactionsOptions
6543 Dec 30, 2019
4ad86e2
refactor swagger options/definitions
6543 Dec 30, 2019
67767b4
fix swagger-validate
6543 Dec 30, 2019
7148d99
Update models/error.go
6543 Dec 30, 2019
e7bd9f2
fix test PART1
6543 Dec 30, 2019
fadb79a
extend FindReactionsOptions with UserID option
6543 Dec 30, 2019
6c331e0
catch error on test
6543 Dec 30, 2019
bef7e3e
Merge branch 'master' into fix-9544_duplicate-reaction-return-200
6543 Dec 30, 2019
b83bbd1
fix test PART2
6543 Dec 30, 2019
90d8768
format ...
6543 Dec 31, 2019
2802e70
Merge branch 'master' into fix-9544_duplicate-reaction-return-200
6543 Dec 31, 2019
f367800
Merge branch 'master' into fix-9544_duplicate-reaction-return-200
techknowlogick Dec 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions models/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,21 @@ func (err ErrForbiddenIssueReaction) Error() string {
return fmt.Sprintf("'%s' is not an allowed reaction", err.Reaction)
}

// ErrReactionAlreadyExist is used when a existing reaction was try to created
type ErrReactionAlreadyExist struct {
Reaction string
}

// IsErrReactionAlreadyExist checks if an error is a ErrReactionAlreadyExist.
func IsErrReactionAlreadyExist(err error) bool {
_, ok := err.(ErrReactionAlreadyExist)
return ok
}

func (err ErrReactionAlreadyExist) Error() string {
return fmt.Sprintf("reaction '%s' already exist", err.Reaction)
6543 marked this conversation as resolved.
Show resolved Hide resolved
}

// __________ .__ .__ __________ __
// \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_
// | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\
Expand Down
19 changes: 17 additions & 2 deletions models/issue_reaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type Reaction struct {
type FindReactionsOptions struct {
IssueID int64
CommentID int64
Reaction string
}

func (opts *FindReactionsOptions) toConds() builder.Cond {
Expand All @@ -46,6 +47,9 @@ func (opts *FindReactionsOptions) toConds() builder.Cond {
} else if opts.CommentID == -1 {
cond = cond.And(builder.Eq{"reaction.comment_id": 0})
}
if opts.Reaction != "" {
cond = cond.And(builder.Eq{"reaction.type": opts.Reaction})
}

return cond
}
Expand Down Expand Up @@ -80,9 +84,20 @@ func createReaction(e *xorm.Session, opts *ReactionOptions) (*Reaction, error) {
UserID: opts.Doer.ID,
IssueID: opts.Issue.ID,
}
findOpts := FindReactionsOptions{IssueID: opts.Issue.ID, CommentID: -1, Reaction: opts.Type}
if opts.Comment != nil {
reaction.CommentID = opts.Comment.ID
findOpts.CommentID = opts.Comment.ID
}

existingR, err := findReactions(e, findOpts)
if err != nil {
return nil, err
}
if len(existingR) > 0 {
return existingR[0], ErrReactionAlreadyExist{Reaction: opts.Type}
}

if _, err := e.Insert(reaction); err != nil {
return nil, err
}
Expand Down Expand Up @@ -112,13 +127,13 @@ func CreateReaction(opts *ReactionOptions) (reaction *Reaction, err error) {

reaction, err = createReaction(sess, opts)
if err != nil {
return nil, err
return
6543 marked this conversation as resolved.
Show resolved Hide resolved
}

if err = sess.Commit(); err != nil {
return nil, err
}
return reaction, nil
return
}

// CreateIssueReaction creates a reaction on issue.
Expand Down
4 changes: 2 additions & 2 deletions modules/structs/issue_reaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ type EditReactionOption struct {
Reaction string `json:"content"`
}

// ReactionResponse contain one reaction
type ReactionResponse struct {
// Reaction contain one reaction
type Reaction struct {
User *User `json:"user"`
Reaction string `json:"content"`
// swagger:strfmt date-time
Expand Down
50 changes: 28 additions & 22 deletions routers/api/v1/repo/issue_reaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
// required: true
// responses:
// "200":
// "$ref": "#/responses/ReactionResponseList"
// "$ref": "#/responses/ReactionList"
// "403":
// "$ref": "#/responses/forbidden"

Expand Down Expand Up @@ -71,9 +71,9 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
return
}

var result []api.ReactionResponse
var result []api.Reaction
for _, r := range reactions {
result = append(result, api.ReactionResponse{
result = append(result, api.Reaction{
User: r.User.APIFormat(),
Reaction: r.Type,
Created: r.CreatedUnix.AsTime(),
Expand Down Expand Up @@ -114,8 +114,10 @@ func PostIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOpti
// schema:
// "$ref": "#/definitions/EditReactionOption"
// responses:
// "200":
// "$ref": "#/responses/Reaction"
// "201":
// "$ref": "#/responses/ReactionResponse"
// "$ref": "#/responses/Reaction"
6543 marked this conversation as resolved.
Show resolved Hide resolved
// "403":
// "$ref": "#/responses/forbidden"

Expand Down Expand Up @@ -188,19 +190,20 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp
if err != nil {
if models.IsErrForbiddenIssueReaction(err) {
ctx.Error(http.StatusForbidden, err.Error(), err)
} else if models.IsErrReactionAlreadyExist(err) {
ctx.JSON(http.StatusOK, api.Reaction{
User: ctx.User.APIFormat(),
zeripath marked this conversation as resolved.
Show resolved Hide resolved
Reaction: reaction.Type,
Created: reaction.CreatedUnix.AsTime(),
})
} else {
ctx.Error(http.StatusInternalServerError, "CreateCommentReaction", err)
}
return
}
_, err = reaction.LoadUser()
if err != nil {
ctx.Error(http.StatusInternalServerError, "Reaction.LoadUser()", err)
return
}

ctx.JSON(http.StatusCreated, api.ReactionResponse{
User: reaction.User.APIFormat(),
ctx.JSON(http.StatusCreated, api.Reaction{
User: ctx.User.APIFormat(),
Reaction: reaction.Type,
Created: reaction.CreatedUnix.AsTime(),
})
Expand Down Expand Up @@ -244,7 +247,7 @@ func GetIssueReactions(ctx *context.APIContext) {
// required: true
// responses:
// "200":
// "$ref": "#/responses/ReactionResponseList"
// "$ref": "#/responses/ReactionList"
// "403":
// "$ref": "#/responses/forbidden"

Expand Down Expand Up @@ -274,9 +277,9 @@ func GetIssueReactions(ctx *context.APIContext) {
return
}

var result []api.ReactionResponse
var result []api.Reaction
for _, r := range reactions {
result = append(result, api.ReactionResponse{
result = append(result, api.Reaction{
User: r.User.APIFormat(),
Reaction: r.Type,
Created: r.CreatedUnix.AsTime(),
Expand Down Expand Up @@ -317,8 +320,10 @@ func PostIssueReaction(ctx *context.APIContext, form api.EditReactionOption) {
// schema:
// "$ref": "#/definitions/EditReactionOption"
// responses:
// "200":
// "$ref": "#/responses/Reaction"
// "201":
// "$ref": "#/responses/ReactionResponse"
// "$ref": "#/responses/Reaction"
// "403":
// "$ref": "#/responses/forbidden"

Expand Down Expand Up @@ -386,19 +391,20 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i
if err != nil {
if models.IsErrForbiddenIssueReaction(err) {
ctx.Error(http.StatusForbidden, err.Error(), err)
} else if models.IsErrReactionAlreadyExist(err) {
ctx.JSON(http.StatusOK, api.Reaction{
User: ctx.User.APIFormat(),
Reaction: reaction.Type,
Created: reaction.CreatedUnix.AsTime(),
})
} else {
ctx.Error(http.StatusInternalServerError, "CreateCommentReaction", err)
}
return
}
_, err = reaction.LoadUser()
if err != nil {
ctx.Error(http.StatusInternalServerError, "Reaction.LoadUser()", err)
return
}

ctx.JSON(http.StatusCreated, api.ReactionResponse{
User: reaction.User.APIFormat(),
ctx.JSON(http.StatusCreated, api.Reaction{
User: ctx.User.APIFormat(),
Reaction: reaction.Type,
Created: reaction.CreatedUnix.AsTime(),
})
Expand Down
23 changes: 8 additions & 15 deletions routers/api/v1/swagger/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,23 +99,16 @@ type swaggerResponseStopWatchList struct {
Body []api.StopWatch `json:"body"`
}

// EditReactionOption
// swagger:response EditReactionOption
type swaggerEditReactionOption struct {
// Reaction
// swagger:response Reaction
type swaggerReaction struct {
// in:body
Body api.EditReactionOption `json:"body"`
Body api.Reaction `json:"body"`
}

// ReactionResponse
// swagger:response ReactionResponse
type swaggerReactionResponse struct {
// ReactionList
// swagger:response ReactionList
type swaggerReactionList struct {
// in:body
Body api.ReactionResponse `json:"body"`
}

// ReactionResponseList
// swagger:response ReactionResponseList
type swaggerReactionResponseList struct {
// in:body
Body []api.ReactionResponse `json:"body"`
Body []api.Reaction `json:"body"`
}
3 changes: 3 additions & 0 deletions routers/api/v1/swagger/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,7 @@ type swaggerParameterBodies struct {

// in:body
RepoTopicOptions api.RepoTopicOptions

// in:body
EditReactionOption api.EditReactionOption
}
38 changes: 19 additions & 19 deletions templates/swagger/v1_json.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3130,7 +3130,7 @@
],
"responses": {
"200": {
"$ref": "#/responses/ReactionResponseList"
"$ref": "#/responses/ReactionList"
},
"403": {
"$ref": "#/responses/forbidden"
Expand Down Expand Up @@ -3181,8 +3181,11 @@
}
],
"responses": {
"200": {
"$ref": "#/responses/Reaction"
},
"201": {
"$ref": "#/responses/ReactionResponse"
"$ref": "#/responses/Reaction"
},
"403": {
"$ref": "#/responses/forbidden"
Expand Down Expand Up @@ -3896,7 +3899,7 @@
],
"responses": {
"200": {
"$ref": "#/responses/ReactionResponseList"
"$ref": "#/responses/ReactionList"
},
"403": {
"$ref": "#/responses/forbidden"
Expand Down Expand Up @@ -3947,8 +3950,11 @@
}
],
"responses": {
"200": {
"$ref": "#/responses/Reaction"
},
"201": {
"$ref": "#/responses/ReactionResponse"
"$ref": "#/responses/Reaction"
},
"403": {
"$ref": "#/responses/forbidden"
Expand Down Expand Up @@ -10815,8 +10821,8 @@
},
"x-go-package": "code.gitea.io/gitea/modules/structs"
},
"ReactionResponse": {
"description": "ReactionResponse contain one reaction",
"Reaction": {
"description": "Reaction contain one reaction",
"type": "object",
"properties": {
"content": {
Expand Down Expand Up @@ -11728,12 +11734,6 @@
}
}
},
"EditReactionOption": {
"description": "EditReactionOption",
"schema": {
"$ref": "#/definitions/EditReactionOption"
}
},
"EmailList": {
"description": "EmailList",
"schema": {
Expand Down Expand Up @@ -11920,18 +11920,18 @@
}
}
},
"ReactionResponse": {
"description": "ReactionResponse",
"Reaction": {
"description": "Reaction",
"schema": {
"$ref": "#/definitions/ReactionResponse"
"$ref": "#/definitions/Reaction"
}
},
"ReactionResponseList": {
"description": "ReactionResponseList",
"ReactionList": {
"description": "ReactionList",
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/ReactionResponse"
"$ref": "#/definitions/Reaction"
}
}
},
Expand Down Expand Up @@ -12157,7 +12157,7 @@
"parameterBodies": {
"description": "parameterBodies",
"schema": {
"$ref": "#/definitions/RepoTopicOptions"
"$ref": "#/definitions/EditReactionOption"
}
},
"redirect": {
Expand Down