From a21f7c5492e0e54709a73014367acf8707fd3a15 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 22 Nov 2019 21:09:15 +0800 Subject: [PATCH 1/3] Extract createComment --- models/issue.go | 44 +++++++++++++++++++----- models/issue_assignees.go | 10 ++++-- models/issue_comment.go | 71 ++++++++++++++++++++++++--------------- models/issue_label.go | 18 +++++++--- models/issue_lock.go | 9 +++-- models/issue_milestone.go | 15 ++++++++- models/issue_xref.go | 9 +++-- models/review.go | 3 +- services/pull/review.go | 3 +- 9 files changed, 131 insertions(+), 51 deletions(-) diff --git a/models/issue.go b/models/issue.go index e28e7c01c3025..27135d1024016 100644 --- a/models/issue.go +++ b/models/issue.go @@ -656,12 +656,18 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (er if !issue.IsClosed { cmtType = CommentTypeReopen } - if _, err := createComment(e, &CreateCommentOptions{ + + var opts = &CreateCommentOptions{ Type: cmtType, Doer: doer, Repo: issue.Repo, Issue: issue, - }); err != nil { + } + comment, err := createCommentWithNoAction(e, opts) + if err != nil { + return err + } + if err = sendCreateCommentAction(e, opts, comment); err != nil { return err } @@ -711,17 +717,21 @@ func (issue *Issue) ChangeTitle(doer *User, oldTitle string) (err error) { return fmt.Errorf("loadRepo: %v", err) } - if _, err = createComment(sess, &CreateCommentOptions{ + var opts = &CreateCommentOptions{ Type: CommentTypeChangeTitle, Doer: doer, Repo: issue.Repo, Issue: issue, OldTitle: oldTitle, NewTitle: issue.Title, - }); err != nil { + } + comment, err := createCommentWithNoAction(sess, opts) + if err != nil { return fmt.Errorf("createComment: %v", err) } - + if err = sendCreateCommentAction(sess, opts, comment); err != nil { + return err + } if err = issue.addCrossReferences(sess, doer, true); err != nil { return err } @@ -740,13 +750,18 @@ func AddDeletePRBranchComment(doer *User, repo *Repository, issueID int64, branc if err := sess.Begin(); err != nil { return err } - if _, err := createComment(sess, &CreateCommentOptions{ + var opts = &CreateCommentOptions{ Type: CommentTypeDeleteBranch, Doer: doer, Repo: repo, Issue: issue, CommitSHA: branchName, - }); err != nil { + } + comment, err := createCommentWithNoAction(sess, opts) + if err != nil { + return err + } + if err = sendCreateCommentAction(sess, opts, comment); err != nil { return err } @@ -880,7 +895,20 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { return err } - if _, err = createMilestoneComment(e, doer, opts.Repo, opts.Issue, 0, opts.Issue.MilestoneID); err != nil { + var opts = &CreateCommentOptions{ + Type: CommentTypeMilestone, + Doer: doer, + Repo: opts.Repo, + Issue: opts.Issue, + OldMilestoneID: 0, + MilestoneID: opts.Issue.MilestoneID, + } + comment, err := createCommentWithNoAction(e, opts) + if err != nil { + return err + } + + if err = sendCreateCommentAction(e, opts, comment); err != nil { return err } } diff --git a/models/issue_assignees.go b/models/issue_assignees.go index 08a567c4ebef4..c78132db5de70 100644 --- a/models/issue_assignees.go +++ b/models/issue_assignees.go @@ -131,18 +131,22 @@ func (issue *Issue) toggleAssignee(sess *xorm.Session, doer *User, assigneeID in return false, nil, fmt.Errorf("loadRepo: %v", err) } - // Comment - comment, err = createComment(sess, &CreateCommentOptions{ + var opts = &CreateCommentOptions{ Type: CommentTypeAssignees, Doer: doer, Repo: issue.Repo, Issue: issue, RemovedAssignee: removed, AssigneeID: assigneeID, - }) + } + // Comment + comment, err = createCommentWithNoAction(sess, opts) if err != nil { return false, nil, fmt.Errorf("createComment: %v", err) } + if err = sendCreateCommentAction(sess, opts, comment); err != nil { + return false, nil, err + } // if pull request is in the middle of creation - don't call webhook if isCreate { diff --git a/models/issue_comment.go b/models/issue_comment.go index 73fd9c8c83eeb..0a0c3d3a8f87f 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -495,7 +495,7 @@ func (c *Comment) CodeCommentURL() string { return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag()) } -func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) { +func createCommentWithNoAction(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) { var LabelID int64 if opts.Label != nil { LabelID = opts.Label.ID @@ -539,12 +539,6 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err return nil, err } - if !opts.NoAction { - if err = sendCreateCommentAction(e, opts, comment); err != nil { - return nil, err - } - } - if err = comment.addCrossReferences(e, opts.Doer, false); err != nil { return nil, err } @@ -651,19 +645,7 @@ func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, commen return nil } -func createMilestoneComment(e *xorm.Session, doer *User, repo *Repository, issue *Issue, oldMilestoneID, milestoneID int64) (*Comment, error) { - return createComment(e, &CreateCommentOptions{ - Type: CommentTypeMilestone, - Doer: doer, - Repo: repo, - Issue: issue, - OldMilestoneID: oldMilestoneID, - MilestoneID: milestoneID, - }) -} - func createDeadlineComment(e *xorm.Session, doer *User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) { - var content string var commentType CommentType @@ -685,13 +667,18 @@ func createDeadlineComment(e *xorm.Session, doer *User, issue *Issue, newDeadlin return nil, err } - return createComment(e, &CreateCommentOptions{ + var opts = &CreateCommentOptions{ Type: commentType, Doer: doer, Repo: issue.Repo, Issue: issue, Content: content, - }) + } + comment, err := createCommentWithNoAction(e, opts) + if err != nil { + return nil, err + } + return comment, sendCreateCommentAction(e, opts, comment) } // Creates issue dependency comment @@ -705,27 +692,35 @@ func createIssueDependencyComment(e *xorm.Session, doer *User, issue *Issue, dep } // Make two comments, one in each issue - _, err = createComment(e, &CreateCommentOptions{ + var opts = &CreateCommentOptions{ Type: cType, Doer: doer, Repo: issue.Repo, Issue: issue, DependentIssueID: dependentIssue.ID, - }) + } + comment, err := createCommentWithNoAction(e, opts) if err != nil { return } + if err = sendCreateCommentAction(e, opts, comment); err != nil { + return err + } - _, err = createComment(e, &CreateCommentOptions{ + opts = &CreateCommentOptions{ Type: cType, Doer: doer, Repo: issue.Repo, Issue: dependentIssue, DependentIssueID: issue.ID, - }) + } + comment, err = createCommentWithNoAction(e, opts) if err != nil { return } + if err = sendCreateCommentAction(e, opts, comment); err != nil { + return err + } return } @@ -758,7 +753,6 @@ type CreateCommentOptions struct { RefCommentID int64 RefAction references.XRefAction RefIsPull bool - NoAction bool } // CreateComment creates comment of issue or commit. @@ -769,7 +763,30 @@ func CreateComment(opts *CreateCommentOptions) (comment *Comment, err error) { return nil, err } - comment, err = createComment(sess, opts) + comment, err = createCommentWithNoAction(sess, opts) + if err != nil { + return nil, err + } + + if err = sendCreateCommentAction(sess, opts, comment); err != nil { + return nil, err + } + + if err = sess.Commit(); err != nil { + return nil, err + } + + return comment, nil +} + +func CreateCommentWithNoAction(opts *CreateCommentOptions) (comment *Comment, err error) { + sess := x.NewSession() + defer sess.Close() + if err = sess.Begin(); err != nil { + return nil, err + } + + comment, err = createCommentWithNoAction(sess, opts) if err != nil { return nil, err } diff --git a/models/issue_label.go b/models/issue_label.go index 77281e9bd4a65..1915708289d7c 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -402,14 +402,19 @@ func newIssueLabel(e *xorm.Session, issue *Issue, label *Label, doer *User) (err return } - if _, err = createComment(e, &CreateCommentOptions{ + var opts = &CreateCommentOptions{ Type: CommentTypeLabel, Doer: doer, Repo: issue.Repo, Issue: issue, Label: label, Content: "1", - }); err != nil { + } + comment, err := createCommentWithNoAction(e, opts) + if err != nil { + return err + } + if err = sendCreateCommentAction(e, opts, comment); err != nil { return err } @@ -478,13 +483,18 @@ func deleteIssueLabel(e *xorm.Session, issue *Issue, label *Label, doer *User) ( return } - if _, err = createComment(e, &CreateCommentOptions{ + var opts = &CreateCommentOptions{ Type: CommentTypeLabel, Doer: doer, Repo: issue.Repo, Issue: issue, Label: label, - }); err != nil { + } + comment, err := createCommentWithNoAction(e, opts) + if err != nil { + return err + } + if err = sendCreateCommentAction(e, opts, comment); err != nil { return err } diff --git a/models/issue_lock.go b/models/issue_lock.go index dc6655ad3b51d..05eca27ad6c1c 100644 --- a/models/issue_lock.go +++ b/models/issue_lock.go @@ -45,16 +45,21 @@ func updateIssueLock(opts *IssueLockOptions, lock bool) error { return err } - _, err := createComment(sess, &CreateCommentOptions{ + var opt = &CreateCommentOptions{ Doer: opts.Doer, Issue: opts.Issue, Repo: opts.Issue.Repo, Type: commentType, Content: opts.Reason, - }) + } + comment, err := createCommentWithNoAction(sess, opt) if err != nil { return err } + if err = sendCreateCommentAction(sess, opt, comment); err != nil { + return err + } + return sess.Commit() } diff --git a/models/issue_milestone.go b/models/issue_milestone.go index a4c2b4b0622c7..e3cbb90624060 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -386,7 +386,20 @@ func changeMilestoneAssign(e *xorm.Session, doer *User, issue *Issue, oldMilesto return err } - if _, err := createMilestoneComment(e, doer, issue.Repo, issue, oldMilestoneID, issue.MilestoneID); err != nil { + var opts = &CreateCommentOptions{ + Type: CommentTypeMilestone, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + OldMilestoneID: oldMilestoneID, + MilestoneID: issue.MilestoneID, + } + comment, err := createCommentWithNoAction(e, opts) + if err != nil { + return err + } + + if err := sendCreateCommentAction(e, opts, comment); err != nil { return err } } diff --git a/models/issue_xref.go b/models/issue_xref.go index 9f30aba259178..5cf8f58a0f256 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -116,7 +116,7 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC if ctx.OrigComment != nil { refCommentID = ctx.OrigComment.ID } - if _, err := createComment(e, &CreateCommentOptions{ + var opts = &CreateCommentOptions{ Type: ctx.Type, Doer: ctx.Doer, Repo: xref.Issue.Repo, @@ -126,7 +126,12 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC RefCommentID: refCommentID, RefAction: xref.Action, RefIsPull: ctx.OrigIssue.IsPull, - }); err != nil { + } + comment, err := createCommentWithNoAction(e, opts) + if err != nil { + return err + } + if err = sendCreateCommentAction(e, opts, comment); err != nil { return err } } diff --git a/models/review.go b/models/review.go index 0dc44a2a4cb3e..ab22cbde38f12 100644 --- a/models/review.go +++ b/models/review.go @@ -286,14 +286,13 @@ func SubmitReview(doer *User, issue *Issue, reviewType ReviewType, content strin } } - comm, err := createComment(sess, &CreateCommentOptions{ + comm, err := createCommentWithNoAction(sess, &CreateCommentOptions{ Type: CommentTypeReview, Doer: doer, Content: review.Content, Issue: issue, Repo: issue.Repo, ReviewID: review.ID, - NoAction: true, }) if err != nil || comm == nil { return nil, nil, err diff --git a/services/pull/review.go b/services/pull/review.go index 880647c6b57ee..fedb7cdbcdd18 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -117,7 +117,7 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models } patch = gitdiff.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) } - return models.CreateComment(&models.CreateCommentOptions{ + return models.CreateCommentWithNoAction(&models.CreateCommentOptions{ Type: models.CommentTypeCode, Doer: doer, Repo: repo, @@ -128,7 +128,6 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models CommitSHA: commitID, ReviewID: reviewID, Patch: patch, - NoAction: true, }) } From cb92b1782067645129e10a1390bbe38722e40059 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 22 Nov 2019 21:16:03 +0800 Subject: [PATCH 2/3] fix lint --- models/issue_comment.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/issue_comment.go b/models/issue_comment.go index 0a0c3d3a8f87f..1fad141d9273a 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -779,6 +779,7 @@ func CreateComment(opts *CreateCommentOptions) (comment *Comment, err error) { return comment, nil } +// CreateCommentWithNoAction creates comment of issue or commit with no action created func CreateCommentWithNoAction(opts *CreateCommentOptions) (comment *Comment, err error) { sess := x.NewSession() defer sess.Close() From b1dcd6351f8e51ab58fc5bd0dd09c4fe84ad3eae Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 23 Nov 2019 16:39:26 +0800 Subject: [PATCH 3/3] fix lint --- models/issue.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/models/issue.go b/models/issue.go index 27135d1024016..310971c45f0db 100644 --- a/models/issue.go +++ b/models/issue.go @@ -667,11 +667,7 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (er if err != nil { return err } - if err = sendCreateCommentAction(e, opts, comment); err != nil { - return err - } - - return nil + return sendCreateCommentAction(e, opts, comment) } // ChangeStatus changes issue status to open or closed.