Add ActionCommentPull action (#9456)

* Add ActionCommentPull action

Adds ActionCommentPull action to distinguish between a comment on an
issue and on a pull request

* Update modules/notification/action/action.go

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

models/action.go

@@ -50,6 +50,7 @@ const (
 	ActionMirrorSyncDelete                         // 20
 	ActionApprovePullRequest                       // 21
 	ActionRejectPullRequest                        // 22
+	ActionCommentPull                              // 23
 )
 
 // Action represents user operation type and other information to

models/repo_watch.go

@@ -210,7 +210,7 @@ func notifyWatchers(e Engine, act *Action) error {
 			if !act.Repo.checkUnitUser(e, act.UserID, false, UnitTypeIssues) {
 				continue
 			}
-		case ActionCreatePullRequest, ActionMergePullRequest, ActionClosePullRequest, ActionReopenPullRequest:
+		case ActionCreatePullRequest, ActionCommentPull, ActionMergePullRequest, ActionClosePullRequest, ActionReopenPullRequest:
 			if !act.Repo.checkUnitUser(e, act.UserID, false, UnitTypePullRequests) {
 				continue
 			}

modules/notification/action/action.go

@@ -90,7 +90,6 @@ func (a *actionNotifier) NotifyIssueChangeStatus(doer *models.User, issue *model
 func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
 	issue *models.Issue, comment *models.Comment) {
 	act := &models.Action{
-		OpType:    models.ActionCommentIssue,
 		ActUserID: doer.ID,
 		ActUser:   doer,
 		Content:   fmt.Sprintf("%d|%s", issue.Index, comment.Content),
@@ -100,6 +99,11 @@ func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *model
 		CommentID: comment.ID,
 		IsPrivate: issue.Repo.IsPrivate,
 	}
+	if issue.IsPull {
+		act.OpType = models.ActionCommentPull
+	} else {
+		act.OpType = models.ActionCommentIssue
+	}
 
 	// Notify watchers for whatever action comes in, ignore if no action type.
 	if err := models.NotifyWatchers(act); err != nil {
@@ -208,7 +212,7 @@ func (a *actionNotifier) NotifyPullRequestReview(pr *models.PullRequest, review
 					ActUserID: review.Reviewer.ID,
 					ActUser:   review.Reviewer,
 					Content:   fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comm.Content, "\n")[0]),
-					OpType:    models.ActionCommentIssue,
+					OpType:    models.ActionCommentPull,
 					RepoID:    review.Issue.RepoID,
 					Repo:      review.Issue.Repo,
 					IsPrivate: review.Issue.Repo.IsPrivate,
@@ -237,7 +241,7 @@ func (a *actionNotifier) NotifyPullRequestReview(pr *models.PullRequest, review
 		case models.ReviewTypeReject:
 			action.OpType = models.ActionRejectPullRequest
 		default:
-			action.OpType = models.ActionCommentIssue
+			action.OpType = models.ActionCommentPull
 		}
 
 		actions = append(actions, action)

modules/notification/mail/mail.go

@@ -85,7 +85,7 @@ func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models
 	} else if comment.Type == models.CommentTypeReopen {
 		act = models.ActionReopenIssue
 	} else if comment.Type == models.CommentTypeComment {
-		act = models.ActionCommentIssue
+		act = models.ActionCommentPull
 	}
 	if err := mailer.MailParticipantsComment(comment, act, pr.Issue); err != nil {
 		log.Error("MailParticipantsComment: %v", err)

modules/templates/helper.go

@@ -548,7 +548,7 @@ func ActionIcon(opType models.ActionType) string {
 		return "issue-opened"
 	case models.ActionCreatePullRequest:
 		return "git-pull-request"
-	case models.ActionCommentIssue:
+	case models.ActionCommentIssue, models.ActionCommentPull:
 		return "comment-discussion"
 	case models.ActionMergePullRequest:
 		return "git-merge"

options/locale/locale_en-US.ini

@@ -2047,6 +2047,7 @@ create_pull_request = `created pull request <a href="%s/pulls/%s">%s#%[2]s</a>`
 close_pull_request = `closed pull request <a href="%s/pulls/%s">%s#%[2]s</a>`
 reopen_pull_request = `reopened pull request <a href="%s/pulls/%s">%s#%[2]s</a>`
 comment_issue = `commented on issue <a href="%s/issues/%s">%s#%[2]s</a>`
+comment_pull = `commented on pull request <a href="%s/pulls/%s">%s#%[2]s</a>`
 merge_pull_request = `merged pull request <a href="%s/pulls/%s">%s#%[2]s</a>`
 transfer_repo = transferred repository <code>%s</code> to <a href="%s">%s</a>
 push_tag = pushed tag <a href="%s/src/tag/%s">%[2]s</a> to <a href="%[1]s">%[3]s</a>

services/mailer/mail.go

@@ -291,7 +291,7 @@ func actionToTemplate(issue *models.Issue, actionType models.ActionType,
 	switch actionType {
 	case models.ActionCreateIssue, models.ActionCreatePullRequest:
 		name = "new"
-	case models.ActionCommentIssue:
+	case models.ActionCommentIssue, models.ActionCommentPull:
 		name = "comment"
 	case models.ActionCloseIssue, models.ActionClosePullRequest:
 		name = "close"

services/mailer/mail_test.go

@@ -153,7 +153,7 @@ func TestTemplateSelection(t *testing.T) {
 
 	pull := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 2, Repo: repo, Poster: doer}).(*models.Issue)
 	comment = models.AssertExistsAndLoadBean(t, &models.Comment{ID: 4, Issue: pull}).(*models.Comment)
-	msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: pull, Doer: doer, ActionType: models.ActionCommentIssue,
+	msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: pull, Doer: doer, ActionType: models.ActionCommentPull,
 		Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection")
 	expect(t, msg, "pull/comment/subject", "pull/comment/body")
 

templates/user/dashboard/feeds.tmpl

@@ -67,6 +67,9 @@
 						{{else if eq .GetOpType 22}}
 							{{ $index := index .GetIssueInfos 0}}
 							{{$.i18n.Tr "action.reject_pull_request" .GetRepoLink $index .ShortRepoPath | Str2html}}
+						{{else if eq .GetOpType 23}}
+							{{ $index := index .GetIssueInfos 0}}
+							{{$.i18n.Tr "action.comment_pull" .GetRepoLink $index .ShortRepoPath | Str2html}}
 						{{end}}
 					</p>
 					{{if or (eq .GetOpType 5) (eq .GetOpType 18)}}
@@ -86,7 +89,7 @@
 						<span class="text truncate issue title has-emoji">{{index .GetIssueInfos 1}}</span>
 					{{else if eq .GetOpType 7}}
 						<span class="text truncate issue title has-emoji">{{index .GetIssueInfos 1}}</span>
-					{{else if or (eq .GetOpType 10) (eq .GetOpType 21) (eq .GetOpType 22)}}
+					{{else if or (eq .GetOpType 10) (eq .GetOpType 21) (eq .GetOpType 22) (eq .GetOpType 23)}}
 						<a href="{{.GetCommentLink}}" class="text truncate issue title has-emoji">{{.GetIssueTitle}}</a>
 						<p class="text light grey has-emoji">{{index .GetIssueInfos 1}}</p>
 					{{else if eq .GetOpType 11}}