From 12170d26a7fc75beef46311996db3798d7ad7c1c Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Wed, 6 Nov 2019 21:39:29 +0800
Subject: [PATCH] Split sendCreateCommentAction as two parts, one for update
 comment related informations, another for actions (#8784)

* Split sendCreateCommentAction as two parts, one for update comment related informations, another for actions

* fix lint
---
 models/issue_comment.go | 70 ++++++++++++++++++++++++-----------------
 models/review.go        |  8 +++--
 2 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/models/issue_comment.go b/models/issue_comment.go
index 90bb8c53a..63f5f6b77 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -535,6 +535,10 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 		return nil, err
 	}
 
+	if err = updateCommentInfos(e, opts, comment); err != nil {
+		return nil, err
+	}
+
 	if err = sendCreateCommentAction(e, opts, comment); err != nil {
 		return nil, err
 	}
@@ -546,19 +550,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 	return comment, nil
 }
 
-func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, comment *Comment) (err error) {
-	// Compose comment action, could be plain comment, close or reopen issue/pull request.
-	// This object will be used to notify watchers in the end of function.
-	act := &Action{
-		ActUserID: opts.Doer.ID,
-		ActUser:   opts.Doer,
-		Content:   fmt.Sprintf("%d|%s", opts.Issue.Index, strings.Split(opts.Content, "\n")[0]),
-		RepoID:    opts.Repo.ID,
-		Repo:      opts.Repo,
-		Comment:   comment,
-		CommentID: comment.ID,
-		IsPrivate: opts.Repo.IsPrivate,
-	}
+func updateCommentInfos(e *xorm.Session, opts *CreateCommentOptions, comment *Comment) (err error) {
 	// Check comment type.
 	switch opts.Type {
 	case CommentTypeCode:
@@ -574,8 +566,6 @@ func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, commen
 		}
 		fallthrough
 	case CommentTypeComment:
-		act.OpType = ActionCommentIssue
-
 		if _, err = e.Exec("UPDATE `issue` SET num_comments=num_comments+1 WHERE id=?", opts.Issue.ID); err != nil {
 			return err
 		}
@@ -601,30 +591,54 @@ func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, commen
 				return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err)
 			}
 		}
+	case CommentTypeReopen, CommentTypeClose:
+		if err = opts.Issue.updateClosedNum(e); err != nil {
+			return err
+		}
+	}
+	// update the issue's updated_unix column
+	return updateIssueCols(e, opts.Issue, "updated_unix")
+}
 
+func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, comment *Comment) (err error) {
+	// Compose comment action, could be plain comment, close or reopen issue/pull request.
+	// This object will be used to notify watchers in the end of function.
+	act := &Action{
+		ActUserID: opts.Doer.ID,
+		ActUser:   opts.Doer,
+		Content:   fmt.Sprintf("%d|%s", opts.Issue.Index, strings.Split(opts.Content, "\n")[0]),
+		RepoID:    opts.Repo.ID,
+		Repo:      opts.Repo,
+		Comment:   comment,
+		CommentID: comment.ID,
+		IsPrivate: opts.Repo.IsPrivate,
+	}
+	// Check comment type.
+	switch opts.Type {
+	case CommentTypeCode:
+		if comment.ReviewID != 0 {
+			if comment.Review == nil {
+				if err := comment.loadReview(e); err != nil {
+					return err
+				}
+			}
+			if comment.Review.Type <= ReviewTypePending {
+				return nil
+			}
+		}
+		fallthrough
+	case CommentTypeComment:
+		act.OpType = ActionCommentIssue
 	case CommentTypeReopen:
 		act.OpType = ActionReopenIssue
 		if opts.Issue.IsPull {
 			act.OpType = ActionReopenPullRequest
 		}
-
-		if err = opts.Issue.updateClosedNum(e); err != nil {
-			return err
-		}
-
 	case CommentTypeClose:
 		act.OpType = ActionCloseIssue
 		if opts.Issue.IsPull {
 			act.OpType = ActionClosePullRequest
 		}
-
-		if err = opts.Issue.updateClosedNum(e); err != nil {
-			return err
-		}
-	}
-	// update the issue's updated_unix column
-	if err = updateIssueCols(e, opts.Issue, "updated_unix"); err != nil {
-		return err
 	}
 	// Notify watchers for whatever action comes in, ignore if no action type.
 	if act.OpType > 0 {
diff --git a/models/review.go b/models/review.go
index 58660b2e3..89a26d6fd 100644
--- a/models/review.go
+++ b/models/review.go
@@ -129,13 +129,17 @@ func (r *Review) publish(e *xorm.Engine) error {
 				go func(en *xorm.Engine, review *Review, comm *Comment) {
 					sess := en.NewSession()
 					defer sess.Close()
-					if err := sendCreateCommentAction(sess, &CreateCommentOptions{
+					opts := &CreateCommentOptions{
 						Doer:    comm.Poster,
 						Issue:   review.Issue,
 						Repo:    review.Issue.Repo,
 						Type:    comm.Type,
 						Content: comm.Content,
-					}, comm); err != nil {
+					}
+					if err := updateCommentInfos(sess, opts, comm); err != nil {
+						log.Warn("updateCommentInfos: %v", err)
+					}
+					if err := sendCreateCommentAction(sess, opts, comm); err != nil {
 						log.Warn("sendCreateCommentAction: %v", err)
 					}
 				}(e, r, comment)