From 9930d47be2516ba4dc33ddfc382f9a829628929d Mon Sep 17 00:00:00 2001
From: guillep2k <18600385+guillep2k@users.noreply.github.com>
Date: Fri, 15 Nov 2019 09:59:21 -0300
Subject: [PATCH] Add review comments to mail notifications (#8996)
---
.../content/doc/advanced/mail-templates-us.md | 50 ++++++++++---------
models/issue_comment.go | 4 --
models/review.go | 4 +-
modules/notification/mail/mail.go | 4 +-
services/mailer/mail.go | 32 ++++++++++--
templates/mail/issue/default.tmpl | 14 +++++-
6 files changed, 73 insertions(+), 35 deletions(-)
diff --git a/docs/content/doc/advanced/mail-templates-us.md b/docs/content/doc/advanced/mail-templates-us.md
index ffe2d4a27bc..d14159467f5 100644
--- a/docs/content/doc/advanced/mail-templates-us.md
+++ b/docs/content/doc/advanced/mail-templates-us.md
@@ -32,6 +32,8 @@ Currently, the following notification events make use of templates:
| `close` | An issue or pull request was closed. |
| `reopen` | An issue or pull request was reopened. |
| `review` | The head comment of a review in a pull request. |
+| `approve` | The head comment of a approving review for a pull request. |
+| `reject` | The head comment of a review requesting changes for a pull request. |
| `code` | A single comment on the code of a pull request. |
| `assigned` | Used was assigned to an issue or pull request. |
| `default` | Any action not included in the above categories, or when the corresponding category template is not present. |
@@ -84,22 +86,23 @@ _subject_ and _mail body_ templates requires at least three dashes; no other cha
_Subject_ and _mail body_ are parsed by [Golang's template engine](https://golang.org/pkg/text/template/) and
are provided with a _metadata context_ assembled for each notification. The context contains the following elements:
-| Name | Type | Available | Usage |
-|--------------------|----------------|---------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| `.FallbackSubject` | string | Always | A default subject line. See Below. |
-| `.Subject` | string | Only in body | The _subject_, once resolved. |
-| `.Body` | string | Always | The message of the issue, pull request or comment, parsed from Markdown into HTML and sanitized. Do not confuse with the _mail body_ |
-| `.Link` | string | Always | The address of the originating issue, pull request or comment. |
-| `.Issue` | models.Issue | Always | The issue (or pull request) originating the notification. To get data specific to a pull request (e.g. `HasMerged`), `.Issue.PullRequest` can be used, but care should be taken as this field will be `nil` if the issue is *not* a pull request. |
-| `.Comment` | models.Comment | If applicable | If the notification is from a comment added to an issue or pull request, this will contain the information about the comment. |
-| `.IsPull` | bool | Always | `true` if the mail notification is associated with a pull request (i.e. `.Issue.PullRequest` is not `nil`). |
-| `.Repo` | string | Always | Name of the repository, including owner name (e.g. `mike/stuff`) |
-| `.User` | models.User | Always | Owner of the repository from which the event originated. To get the user name (e.g. `mike`),`.User.Name` can be used. |
-| `.Doer` | models.User | Always | User that executed the action triggering the notification event. To get the user name (e.g. `rhonda`), `.Doer.Name` can be used. |
-| `.IsMention` | bool | Always | `true` if this notification was only generated because the user was mentioned in the comment, while not being subscribed to the source. It will be `false` if the recipient was subscribed to the issue or repository. |
-| `.SubjectPrefix` | string | Always | `Re: ` if the notification is about other than issue or pull request creation; otherwise an empty string. |
-| `.ActionType` | string | Always | `"issue"` or `"pull"`. Will correspond to the actual _action type_ independently of which template was selected. |
-| `.ActionName` | string | Always | It will be one of the action types described above (`new`, `comment`, etc.), and will correspond to the actual _action name_ independently of which template was selected. |
+| Name | Type | Available | Usage |
+|--------------------|------------------|---------------|------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `.FallbackSubject` | string | Always | A default subject line. See Below. |
+| `.Subject` | string | Only in body | The _subject_, once resolved. |
+| `.Body` | string | Always | The message of the issue, pull request or comment, parsed from Markdown into HTML and sanitized. Do not confuse with the _mail body_. |
+| `.Link` | string | Always | The address of the originating issue, pull request or comment. |
+| `.Issue` | models.Issue | Always | The issue (or pull request) originating the notification. To get data specific to a pull request (e.g. `HasMerged`), `.Issue.PullRequest` can be used, but care should be taken as this field will be `nil` if the issue is *not* a pull request. |
+| `.Comment` | models.Comment | If applicable | If the notification is from a comment added to an issue or pull request, this will contain the information about the comment. |
+| `.IsPull` | bool | Always | `true` if the mail notification is associated with a pull request (i.e. `.Issue.PullRequest` is not `nil`). |
+| `.Repo` | string | Always | Name of the repository, including owner name (e.g. `mike/stuff`) |
+| `.User` | models.User | Always | Owner of the repository from which the event originated. To get the user name (e.g. `mike`),`.User.Name` can be used. |
+| `.Doer` | models.User | Always | User that executed the action triggering the notification event. To get the user name (e.g. `rhonda`), `.Doer.Name` can be used. |
+| `.IsMention` | bool | Always | `true` if this notification was only generated because the user was mentioned in the comment, while not being subscribed to the source. It will be `false` if the recipient was subscribed to the issue or repository. |
+| `.SubjectPrefix` | string | Always | `Re: ` if the notification is about other than issue or pull request creation; otherwise an empty string. |
+| `.ActionType` | string | Always | `"issue"` or `"pull"`. Will correspond to the actual _action type_ independently of which template was selected. |
+| `.ActionName` | string | Always | It will be one of the action types described above (`new`, `comment`, etc.), and will correspond to the actual _action name_ independently of which template was selected. |
+| `.ReviewComments` | []models.Comment | Always | List of code comments in a review. The comment text will be in `.RenderedContent` and the referenced code will be in `.Patch`. |
All names are case sensitive.
@@ -254,13 +257,14 @@ This template produces something along these lines:
The template system contains several functions that can be used to further process and format
the messages. Here's a list of some of them:
-| Name | Parameters | Available | Usage |
-|----------------------|-------------|-----------|---------------------------------------------------------------------|
-| `AppUrl` | - | Any | Gitea's URL |
-| `AppName` | - | Any | Set from `app.ini`, usually "Gitea" |
-| `AppDomain` | - | Any | Gitea's host name |
-| `EllipsisString` | string, int | Any | Truncates a string to the specified length; adds ellipsis as needed |
-| `Str2html` | string | Body only | Sanitizes text by removing any HTML tags from it. |
+| Name | Parameters | Available | Usage |
+|----------------------|-------------|-----------|------------------------------------------------------------------------------|
+| `AppUrl` | - | Any | Gitea's URL |
+| `AppName` | - | Any | Set from `app.ini`, usually "Gitea" |
+| `AppDomain` | - | Any | Gitea's host name |
+| `EllipsisString` | string, int | Any | Truncates a string to the specified length; adds ellipsis as needed |
+| `Str2html` | string | Body only | Sanitizes text by removing any HTML tags from it. |
+| `Safe` | string | Body only | Takes the input as HTML; can be used for `.ReviewComments.RenderedContent`. |
These are _functions_, not metadata, so they have to be used:
diff --git a/models/issue_comment.go b/models/issue_comment.go
index c7e9e7cdfad..37d4d8c1156 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -1016,10 +1016,6 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
return nil, err
}
- if err := CommentList(comments).loadPosters(e); err != nil {
- return nil, err
- }
-
if err := issue.loadRepo(e); err != nil {
return nil, err
}
diff --git a/models/review.go b/models/review.go
index e1674e885d0..0dc44a2a4cb 100644
--- a/models/review.go
+++ b/models/review.go
@@ -62,7 +62,9 @@ type Review struct {
}
func (r *Review) loadCodeComments(e Engine) (err error) {
- r.CodeComments, err = fetchCodeCommentsByReview(e, r.Issue, nil, r)
+ if r.CodeComments == nil {
+ r.CodeComments, err = fetchCodeCommentsByReview(e, r.Issue, nil, r)
+ }
return
}
diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go
index 0900c6dcdf1..24f68bd6428 100644
--- a/modules/notification/mail/mail.go
+++ b/modules/notification/mail/mail.go
@@ -40,7 +40,7 @@ func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.
}
if err := mailer.MailParticipantsComment(comment, act, issue); err != nil {
- log.Error("MailParticipants: %v", err)
+ log.Error("MailParticipantsComment: %v", err)
}
}
@@ -87,7 +87,7 @@ func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models
act = models.ActionCommentIssue
}
if err := mailer.MailParticipantsComment(comment, act, pr.Issue); err != nil {
- log.Error("MailParticipants: %v", err)
+ log.Error("MailParticipantsComment: %v", err)
}
}
diff --git a/services/mailer/mail.go b/services/mailer/mail.go
index fc892f6076b..27767be6883 100644
--- a/services/mailer/mail.go
+++ b/services/mailer/mail.go
@@ -177,7 +177,8 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy
link string
prefix string
// Fall back subject for bad templates, make sure subject is never empty
- fallback string
+ fallback string
+ reviewComments []*models.Comment
)
commentType := models.CommentTypeComment
@@ -189,12 +190,26 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy
link = issue.HTMLURL()
}
+ reviewType := models.ReviewTypeComment
+ if comment != nil && comment.Review != nil {
+ reviewType = comment.Review.Type
+ }
+
fallback = prefix + fallbackMailSubject(issue)
// This is the body of the new issue or comment, not the mail body
body := string(markup.RenderByType(markdown.MarkupName, []byte(content), issue.Repo.HTMLURL(), issue.Repo.ComposeMetas()))
- actType, actName, tplName := actionToTemplate(issue, actionType, commentType)
+ actType, actName, tplName := actionToTemplate(issue, actionType, commentType, reviewType)
+
+ if comment != nil && comment.Review != nil {
+ reviewComments = make([]*models.Comment, 0, 10)
+ for _, lines := range comment.Review.CodeComments {
+ for _, comments := range lines {
+ reviewComments = append(reviewComments, comments...)
+ }
+ }
+ }
mailMeta := map[string]interface{}{
"FallbackSubject": fallback,
@@ -210,6 +225,7 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy
"SubjectPrefix": prefix,
"ActionType": actType,
"ActionName": actName,
+ "ReviewComments": reviewComments,
}
var mailSubject bytes.Buffer
@@ -272,7 +288,8 @@ func SendIssueMentionMail(issue *models.Issue, doer *models.User, actionType mod
// actionToTemplate returns the type and name of the action facing the user
// (slightly different from models.ActionType) and the name of the template to use (based on availability)
-func actionToTemplate(issue *models.Issue, actionType models.ActionType, commentType models.CommentType) (typeName, name, template string) {
+func actionToTemplate(issue *models.Issue, actionType models.ActionType,
+ commentType models.CommentType, reviewType models.ReviewType) (typeName, name, template string) {
if issue.IsPull {
typeName = "pull"
} else {
@@ -292,7 +309,14 @@ func actionToTemplate(issue *models.Issue, actionType models.ActionType, comment
default:
switch commentType {
case models.CommentTypeReview:
- name = "review"
+ switch reviewType {
+ case models.ReviewTypeApprove:
+ name = "approve"
+ case models.ReviewTypeReject:
+ name = "reject"
+ default:
+ name = "review"
+ }
case models.CommentTypeCode:
name = "code"
case models.CommentTypeAssignees:
diff --git a/templates/mail/issue/default.tmpl b/templates/mail/issue/default.tmpl
index ee15d6d8e1c..223ff7ffdc3 100644
--- a/templates/mail/issue/default.tmpl
+++ b/templates/mail/issue/default.tmpl
@@ -3,6 +3,12 @@
{{.Subject}}
+ {{if .ReviewComments}}
+
+ {{end}}
@@ -15,12 +21,18 @@
Closed #{{.Issue.Index}}.
{{else if eq .ActionName "reopen"}}
Reopened #{{.Issue.Index}}.
- {{else}}
+ {{else if ne .ReviewComments}}
Empty comment on #{{.Issue.Index}}.
{{end}}
{{else}}
{{.Body | Str2html}}
{{end -}}
+ {{- range .ReviewComments}}
+