Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add review comments to mail notifications #8996

Merged
merged 5 commits into from
Nov 15, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 27 additions & 23 deletions docs/content/doc/advanced/mail-templates-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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:

Expand Down
4 changes: 0 additions & 4 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -1016,10 +1016,6 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
return nil, err
}

if err := CommentList(comments).loadPosters(e); err != nil {
guillep2k marked this conversation as resolved.
Show resolved Hide resolved
return nil, err
}

if err := issue.loadRepo(e); err != nil {
return nil, err
}
Expand Down
4 changes: 3 additions & 1 deletion models/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions modules/notification/mail/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down
32 changes: 28 additions & 4 deletions services/mailer/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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:
Expand Down
14 changes: 13 additions & 1 deletion templates/mail/issue/default.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
---
Expand Down