From c63a80138a29aa9a8164d3eddd1eddaaf414ddc6 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 31 Dec 2019 18:00:17 +0100 Subject: [PATCH] backport part of #9550 (#9564) * add ErrReactionAlreadyExist * extend FindReactionsOptions * createReaction check if exit before create --- models/error.go | 15 +++++++++++++++ models/issue_reaction.go | 35 ++++++++++++++++++++++++++++++----- models/issue_reaction_test.go | 13 ++++++------- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/models/error.go b/models/error.go index 995617e83..be23b47b8 100644 --- a/models/error.go +++ b/models/error.go @@ -1104,6 +1104,21 @@ func (err ErrNewIssueInsert) Error() string { return err.OriginalError.Error() } +// 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 exists", err.Reaction) +} + // __________ .__ .__ __________ __ // \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_ // | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\ diff --git a/models/issue_reaction.go b/models/issue_reaction.go index ab644b4b3..a0dc72851 100644 --- a/models/issue_reaction.go +++ b/models/issue_reaction.go @@ -30,6 +30,8 @@ type Reaction struct { type FindReactionsOptions struct { IssueID int64 CommentID int64 + UserID int64 + Reaction string } func (opts *FindReactionsOptions) toConds() builder.Cond { @@ -40,6 +42,13 @@ func (opts *FindReactionsOptions) toConds() builder.Cond { if opts.CommentID > 0 { cond = cond.And(builder.Eq{"reaction.comment_id": opts.CommentID}) } + if opts.UserID > 0 { + cond = cond.And(builder.Eq{"reaction.user_id": opts.UserID}) + } + if opts.Reaction != "" { + cond = cond.And(builder.Eq{"reaction.type": opts.Reaction}) + } + return cond } @@ -57,9 +66,25 @@ 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 to issue only + Reaction: opts.Type, + UserID: opts.Doer.ID, + } 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 } @@ -76,19 +101,19 @@ type ReactionOptions struct { } // CreateReaction creates reaction for issue or comment. -func CreateReaction(opts *ReactionOptions) (reaction *Reaction, err error) { +func CreateReaction(opts *ReactionOptions) (*Reaction, error) { sess := x.NewSession() defer sess.Close() - if err = sess.Begin(); err != nil { + if err := sess.Begin(); err != nil { return nil, err } - reaction, err = createReaction(sess, opts) + reaction, err := createReaction(sess, opts) if err != nil { - return nil, err + return reaction, err } - if err = sess.Commit(); err != nil { + if err := sess.Commit(); err != nil { return nil, err } return reaction, nil diff --git a/models/issue_reaction_test.go b/models/issue_reaction_test.go index bbd8cf29f..a2f1c025f 100644 --- a/models/issue_reaction_test.go +++ b/models/issue_reaction_test.go @@ -50,9 +50,10 @@ func TestIssueAddDuplicateReaction(t *testing.T) { Type: "heart", }) assert.Error(t, err) - assert.Nil(t, reaction) + assert.Equal(t, ErrReactionAlreadyExist{Reaction: "heart"}, err) - AssertExistsAndLoadBean(t, &Reaction{Type: "heart", UserID: user1.ID, IssueID: issue1.ID}) + existingR := AssertExistsAndLoadBean(t, &Reaction{Type: "heart", UserID: user1.ID, IssueID: issue1.ID}).(*Reaction) + assert.Equal(t, existingR.ID, reaction.ID) } func TestIssueDeleteReaction(t *testing.T) { @@ -129,7 +130,6 @@ func TestIssueCommentDeleteReaction(t *testing.T) { user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) user3 := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) user4 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User) - ghost := NewGhostUser() issue1 := AssertExistsAndLoadBean(t, &Issue{ID: 1}).(*Issue) @@ -139,14 +139,13 @@ func TestIssueCommentDeleteReaction(t *testing.T) { addReaction(t, user2, issue1, comment1, "heart") addReaction(t, user3, issue1, comment1, "heart") addReaction(t, user4, issue1, comment1, "+1") - addReaction(t, ghost, issue1, comment1, "heart") err := comment1.LoadReactions() assert.NoError(t, err) - assert.Len(t, comment1.Reactions, 5) + assert.Len(t, comment1.Reactions, 4) reactions := comment1.Reactions.GroupByType() - assert.Len(t, reactions["heart"], 4) + assert.Len(t, reactions["heart"], 3) assert.Len(t, reactions["+1"], 1) } @@ -160,7 +159,7 @@ func TestIssueCommentReactionCount(t *testing.T) { comment1 := AssertExistsAndLoadBean(t, &Comment{ID: 1}).(*Comment) addReaction(t, user1, issue1, comment1, "heart") - DeleteCommentReaction(user1, issue1, comment1, "heart") + assert.NoError(t, DeleteCommentReaction(user1, issue1, comment1, "heart")) AssertNotExistsBean(t, &Reaction{Type: "heart", UserID: user1.ID, IssueID: issue1.ID, CommentID: comment1.ID}) }