Add review comments to mail notifications (#8996)

This commit is contained in:
guillep2k 2019-11-15 09:59:21 -03:00 committed by zeripath
parent 97dc314652
commit 9930d47be2
6 changed files with 73 additions and 35 deletions

View file

@ -32,6 +32,8 @@ Currently, the following notification events make use of templates:
| `close` | An issue or pull request was closed. | | `close` | An issue or pull request was closed. |
| `reopen` | An issue or pull request was reopened. | | `reopen` | An issue or pull request was reopened. |
| `review` | The head comment of a review in a pull request. | | `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. | | `code` | A single comment on the code of a pull request. |
| `assigned` | Used was assigned to an issue or 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. | | `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 _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: are provided with a _metadata context_ assembled for each notification. The context contains the following elements:
| Name | Type | Available | Usage | | Name | Type | Available | Usage |
|--------------------|----------------|---------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| |--------------------|------------------|---------------|------------------------------------------------------------------------------------------------------------------------------------------------------|
| `.FallbackSubject` | string | Always | A default subject line. See Below. | | `.FallbackSubject` | string | Always | A default subject line. See Below. |
| `.Subject` | string | Only in body | The _subject_, once resolved. | | `.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_ | | `.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. | | `.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. | | `.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. | | `.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`). | | `.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`) | | `.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. | | `.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. | | `.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. | | `.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. | | `.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. | | `.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. | | `.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. 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 template system contains several functions that can be used to further process and format
the messages. Here's a list of some of them: the messages. Here's a list of some of them:
| Name | Parameters | Available | Usage | | Name | Parameters | Available | Usage |
|----------------------|-------------|-----------|---------------------------------------------------------------------| |----------------------|-------------|-----------|------------------------------------------------------------------------------|
| `AppUrl` | - | Any | Gitea's URL | | `AppUrl` | - | Any | Gitea's URL |
| `AppName` | - | Any | Set from `app.ini`, usually "Gitea" | | `AppName` | - | Any | Set from `app.ini`, usually "Gitea" |
| `AppDomain` | - | Any | Gitea's host name | | `AppDomain` | - | Any | Gitea's host name |
| `EllipsisString` | string, int | Any | Truncates a string to the specified length; adds ellipsis as needed | | `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. | | `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: These are _functions_, not metadata, so they have to be used:

View file

@ -1016,10 +1016,6 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
return nil, err return nil, err
} }
if err := CommentList(comments).loadPosters(e); err != nil {
return nil, err
}
if err := issue.loadRepo(e); err != nil { if err := issue.loadRepo(e); err != nil {
return nil, err return nil, err
} }

View file

@ -62,7 +62,9 @@ type Review struct {
} }
func (r *Review) loadCodeComments(e Engine) (err error) { 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 return
} }

View file

@ -40,7 +40,7 @@ func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.
} }
if err := mailer.MailParticipantsComment(comment, act, issue); err != nil { 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 act = models.ActionCommentIssue
} }
if err := mailer.MailParticipantsComment(comment, act, pr.Issue); err != nil { if err := mailer.MailParticipantsComment(comment, act, pr.Issue); err != nil {
log.Error("MailParticipants: %v", err) log.Error("MailParticipantsComment: %v", err)
} }
} }

View file

@ -177,7 +177,8 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy
link string link string
prefix string prefix string
// Fall back subject for bad templates, make sure subject is never empty // Fall back subject for bad templates, make sure subject is never empty
fallback string fallback string
reviewComments []*models.Comment
) )
commentType := models.CommentTypeComment commentType := models.CommentTypeComment
@ -189,12 +190,26 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy
link = issue.HTMLURL() link = issue.HTMLURL()
} }
reviewType := models.ReviewTypeComment
if comment != nil && comment.Review != nil {
reviewType = comment.Review.Type
}
fallback = prefix + fallbackMailSubject(issue) fallback = prefix + fallbackMailSubject(issue)
// This is the body of the new issue or comment, not the mail body // 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())) 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{}{ mailMeta := map[string]interface{}{
"FallbackSubject": fallback, "FallbackSubject": fallback,
@ -210,6 +225,7 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy
"SubjectPrefix": prefix, "SubjectPrefix": prefix,
"ActionType": actType, "ActionType": actType,
"ActionName": actName, "ActionName": actName,
"ReviewComments": reviewComments,
} }
var mailSubject bytes.Buffer 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 // 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) // (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 { if issue.IsPull {
typeName = "pull" typeName = "pull"
} else { } else {
@ -292,7 +309,14 @@ func actionToTemplate(issue *models.Issue, actionType models.ActionType, comment
default: default:
switch commentType { switch commentType {
case models.CommentTypeReview: case models.CommentTypeReview:
name = "review" switch reviewType {
case models.ReviewTypeApprove:
name = "approve"
case models.ReviewTypeReject:
name = "reject"
default:
name = "review"
}
case models.CommentTypeCode: case models.CommentTypeCode:
name = "code" name = "code"
case models.CommentTypeAssignees: case models.CommentTypeAssignees:

View file

@ -3,6 +3,12 @@
<head> <head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>{{.Subject}}</title> <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> </head>
<body> <body>
@ -15,12 +21,18 @@
Closed #{{.Issue.Index}}. Closed #{{.Issue.Index}}.
{{else if eq .ActionName "reopen"}} {{else if eq .ActionName "reopen"}}
Reopened #{{.Issue.Index}}. Reopened #{{.Issue.Index}}.
{{else}} {{else if ne .ReviewComments}}
Empty comment on #{{.Issue.Index}}. Empty comment on #{{.Issue.Index}}.
{{end}} {{end}}
{{else}} {{else}}
{{.Body | Str2html}} {{.Body | Str2html}}
{{end -}} {{end -}}
{{- range .ReviewComments}}
<div class="review">
<pre>{{.Patch}}</pre>
<div>{{.RenderedContent | Safe}}</div>
</div>
{{end -}}
</p> </p>
<p> <p>
--- ---