Include thread related headers in issue/coment mail (#7484)

mrsdizzie · techknowlogick · commit 944d904980f7 · 2019-07-17T15:02:42.000-04:00
* Include thread related headers in issue/coment mail

Make it so mail programs will group comments from an issue into the same
thread by setting Message-ID on initial issue and then using In-Reply-To
and References headers to reference that later on.

* Add tests

* more tests

* fix typo
diff --git a/models/issue.go b/models/issue.go
@@ -472,6 +472,18 @@ func (issue *Issue) sendLabelUpdatedWebhook(doer *User) {
 	}
 }
 
+// ReplyReference returns tokenized address to use for email reply headers
+func (issue *Issue) ReplyReference() string {
+	var path string
+	if issue.IsPull {
+		path = "pulls"
+	} else {
+		path = "issues"
+	}
+
+	return fmt.Sprintf("%s/%s/%d@%s", issue.Repo.FullName(), path, issue.Index, setting.Domain)
+}
+
 func (issue *Issue) addLabel(e *xorm.Session, label *Label, doer *User) error {
 	return newIssueLabel(e, issue, label, doer)
 }
diff --git a/models/mail.go b/models/mail.go
@@ -156,7 +156,13 @@ func composeTplData(subject, body, link string) map[string]interface{} {
 }
 
 func composeIssueCommentMessage(issue *Issue, doer *User, content string, comment *Comment, tplName base.TplName, tos []string, info string) *mailer.Message {
-	subject := issue.mailSubject()
+	var subject string
+	if comment != nil {
+		subject = "Re: " + issue.mailSubject()
+	} else {
+		subject = issue.mailSubject()
+	}
+
 	err := issue.LoadRepo()
 	if err != nil {
 		log.Error("LoadRepo: %v", err)
@@ -179,6 +185,15 @@ func composeIssueCommentMessage(issue *Issue, doer *User, content string, commen
 
 	msg := mailer.NewMessageFrom(tos, doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String())
 	msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info)
+
+	// Set Message-ID on first message so replies know what to reference
+	if comment == nil {
+		msg.SetHeader("Message-ID", "<"+issue.ReplyReference()+">")
+	} else {
+		msg.SetHeader("In-Reply-To", "<"+issue.ReplyReference()+">")
+		msg.SetHeader("References", "<"+issue.ReplyReference()+">")
+	}
+
 	return msg
 }
 
diff --git a/models/mail_test.go b/models/mail_test.go
@@ -0,0 +1,87 @@
+// Copyright 2019 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package models
+
+import (
+	"html/template"
+	"testing"
+
+	"code.gitea.io/gitea/modules/setting"
+
+	"github.com/stretchr/testify/assert"
+)
+
+const tmpl = `
+<!DOCTYPE html>
+<html>
+<head>
+	<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+	<title>{{.Subject}}</title>
+</head>
+
+<body>
+	<p>{{.Body}}</p>
+	<p>
+		---
+		<br>
+		<a href="{{.Link}}">View it on Gitea</a>.
+	</p>
+</body>
+</html>
+`
+
+func TestComposeIssueCommentMessage(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	var MailService setting.Mailer
+
+	MailService.From = "test@gitea.com"
+	setting.MailService = &MailService
+
+	doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1, Owner: doer}).(*Repository)
+	issue := AssertExistsAndLoadBean(t, &Issue{ID: 1, Repo: repo, Poster: doer}).(*Issue)
+	comment := AssertExistsAndLoadBean(t, &Comment{ID: 2, Issue: issue}).(*Comment)
+
+	email := template.Must(template.New("issue/comment").Parse(tmpl))
+	InitMailRender(email)
+
+	tos := []string{"test@gitea.com", "test2@gitea.com"}
+	msg := composeIssueCommentMessage(issue, doer, "test body", comment, mailIssueComment, tos, "issue comment")
+
+	subject := msg.GetHeader("Subject")
+	inreplyTo := msg.GetHeader("In-Reply-To")
+	references := msg.GetHeader("References")
+
+	assert.Equal(t, subject[0], "Re: "+issue.mailSubject(), "Comment reply subject should contain Re:")
+	assert.Equal(t, inreplyTo[0], "<user2/repo1/issues/1@localhost>", "In-Reply-To header doesn't match")
+	assert.Equal(t, references[0], "<user2/repo1/issues/1@localhost>", "References header doesn't match")
+
+}
+
+func TestComposeIssueMessage(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	var MailService setting.Mailer
+
+	MailService.From = "test@gitea.com"
+	setting.MailService = &MailService
+
+	doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1, Owner: doer}).(*Repository)
+	issue := AssertExistsAndLoadBean(t, &Issue{ID: 1, Repo: repo, Poster: doer}).(*Issue)
+
+	email := template.Must(template.New("issue/comment").Parse(tmpl))
+	InitMailRender(email)
+
+	tos := []string{"test@gitea.com", "test2@gitea.com"}
+	msg := composeIssueCommentMessage(issue, doer, "test body", nil, mailIssueComment, tos, "issue create")
+
+	subject := msg.GetHeader("Subject")
+	messageID := msg.GetHeader("Message-ID")
+
+	assert.Equal(t, subject[0], issue.mailSubject(), "Subject not equal to issue.mailSubject()")
+	assert.Nil(t, msg.GetHeader("In-Reply-To"))
+	assert.Nil(t, msg.GetHeader("References"))
+	assert.Equal(t, messageID[0], "<user2/repo1/issues/1@localhost>", "Message-ID header doesn't match")
+}

Original file line number	Diff line number	Diff line change
`@@ -472,6 +472,18 @@ func (issue Issue) sendLabelUpdatedWebhook(doer User) {`
`472`	`472`	`}`
`473`	`473`	`}`
`474`	`474`
	`475`	`+// ReplyReference returns tokenized address to use for email reply headers`
	`476`	`+func (issue *Issue) ReplyReference() string {`
	`477`	`+ var path string`
	`478`	`+ if issue.IsPull {`
	`479`	`+ path = "pulls"`
	`480`	`+ } else {`
	`481`	`+ path = "issues"`
	`482`	`+ }`
	`483`	`+`
	`484`	`+ return fmt.Sprintf("%s/%s/%d@%s", issue.Repo.FullName(), path, issue.Index, setting.Domain)`
	`485`	`+}`
	`486`	`+`
`475`	`487`	`func (issue Issue) addLabel(e xorm.Session, label Label, doer User) error {`
`476`	`488`	`return newIssueLabel(e, issue, label, doer)`
`477`	`489`	`}`