Add review comments to mail notifications (#8996)

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 ffe2d4a27b..d14159467f 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 c7e9e7cdfa..37d4d8c115 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 e1674e885d..0dc44a2a4c 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 0900c6dcdf..24f68bd642 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 fc892f6076..27767be688 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 ee15d6d8e1..223ff7ffdc 100644
--- a/templates/mail/issue/default.tmpl
+++ b/templates/mail/issue/default.tmpl
@@ -3,6 +3,12 @@
 <head>
 	<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
 	<title>{{.Subject}}</title>
+	{{if .ReviewComments}}
+	<style>
+		.review { padding-left: 1em; margin: 1em 0; }
+		.review > pre { padding: 1em; border-left: 1px solid grey; }
+	</style>
+	{{end}}
 </head>
 
 <body>
@@ -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}}
+			<div class="review">
+				<pre>{{.Patch}}</pre>
+				<div>{{.RenderedContent | Safe}}</div>
+			</div>
+		{{end -}}		
 	</p>
 	<p>
 		---