Prevent merge messages from being sorted to the top of email chains (#18566) (#18588)

Backport #18566

Gitea will currrently resend the same message-id for the closed/merged/reopened
messages for issues. This will cause the merged message to leap to the top of an
email chain and become out of sync.

This PR adds specific suffices for these actions.

Fix #18560

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2022-02-04 08:30:36 +00:00 committed by GitHub
parent 70ffec4509
commit 18c1edf15c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 131 additions and 4 deletions

View File

@ -14,6 +14,7 @@ import (
"strconv" "strconv"
"strings" "strings"
texttmpl "text/template" texttmpl "text/template"
"time"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
@ -297,13 +298,15 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
} }
// Make sure to compose independent messages to avoid leaking user emails // Make sure to compose independent messages to avoid leaking user emails
msgID := createReference(ctx.Issue, ctx.Comment, ctx.ActionType)
reference := createReference(ctx.Issue, nil, models.ActionType(0))
msgs := make([]*Message, 0, len(recipients)) msgs := make([]*Message, 0, len(recipients))
for _, recipient := range recipients { for _, recipient := range recipients {
msg := NewMessageFrom([]string{recipient.Email}, ctx.Doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String()) msg := NewMessageFrom([]string{recipient.Email}, ctx.Doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String())
msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info) msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info)
msg.SetHeader("Message-ID", "<"+createReference(ctx.Issue, ctx.Comment)+">") msg.SetHeader("Message-ID", "<"+msgID+">")
reference := createReference(ctx.Issue, nil)
msg.SetHeader("In-Reply-To", "<"+reference+">") msg.SetHeader("In-Reply-To", "<"+reference+">")
msg.SetHeader("References", "<"+reference+">") msg.SetHeader("References", "<"+reference+">")
@ -317,7 +320,7 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
return msgs, nil return msgs, nil
} }
func createReference(issue *models.Issue, comment *models.Comment) string { func createReference(issue *models.Issue, comment *models.Comment, actionType models.ActionType) string {
var path string var path string
if issue.IsPull { if issue.IsPull {
path = "pulls" path = "pulls"
@ -328,6 +331,17 @@ func createReference(issue *models.Issue, comment *models.Comment) string {
var extra string var extra string
if comment != nil { if comment != nil {
extra = fmt.Sprintf("/comment/%d", comment.ID) extra = fmt.Sprintf("/comment/%d", comment.ID)
} else {
switch actionType {
case models.ActionCloseIssue, models.ActionClosePullRequest:
extra = fmt.Sprintf("/close/%d", time.Now().UnixNano()/1e6)
case models.ActionReopenIssue, models.ActionReopenPullRequest:
extra = fmt.Sprintf("/reopen/%d", time.Now().UnixNano()/1e6)
case models.ActionMergePullRequest:
extra = fmt.Sprintf("/merge/%d", time.Now().UnixNano()/1e6)
case models.ActionPullRequestReadyForReview:
extra = fmt.Sprintf("/ready/%d", time.Now().UnixNano()/1e6)
}
} }
return fmt.Sprintf("%s/%s/%d%s@%s", issue.Repo.FullName(), path, issue.Index, extra, setting.Domain) return fmt.Sprintf("%s/%s/%d%s@%s", issue.Repo.FullName(), path, issue.Index, extra, setting.Domain)

View File

@ -6,7 +6,9 @@ package mailer
import ( import (
"bytes" "bytes"
"fmt"
"html/template" "html/template"
"strings"
"testing" "testing"
texttmpl "text/template" texttmpl "text/template"
@ -15,7 +17,6 @@ import (
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -236,3 +237,115 @@ func TestGenerateAdditionalHeaders(t *testing.T) {
} }
} }
} }
func Test_createReference(t *testing.T) {
_, _, issue, comment := prepareMailerTest(t)
_, _, pullIssue, _ := prepareMailerTest(t)
pullIssue.IsPull = true
type args struct {
issue *models.Issue
comment *models.Comment
actionType models.ActionType
}
tests := []struct {
name string
args args
prefix string
suffix string
}{
{
name: "Open Issue",
args: args{
issue: issue,
actionType: models.ActionCreateIssue,
},
prefix: fmt.Sprintf("%s/issues/%d@%s", issue.Repo.FullName(), issue.Index, setting.Domain),
},
{
name: "Open Pull",
args: args{
issue: pullIssue,
actionType: models.ActionCreatePullRequest,
},
prefix: fmt.Sprintf("%s/pulls/%d@%s", issue.Repo.FullName(), issue.Index, setting.Domain),
},
{
name: "Comment Issue",
args: args{
issue: issue,
comment: comment,
actionType: models.ActionCommentIssue,
},
prefix: fmt.Sprintf("%s/issues/%d/comment/%d@%s", issue.Repo.FullName(), issue.Index, comment.ID, setting.Domain),
},
{
name: "Comment Pull",
args: args{
issue: pullIssue,
comment: comment,
actionType: models.ActionCommentPull,
},
prefix: fmt.Sprintf("%s/pulls/%d/comment/%d@%s", issue.Repo.FullName(), issue.Index, comment.ID, setting.Domain),
},
{
name: "Close Issue",
args: args{
issue: issue,
actionType: models.ActionCloseIssue,
},
prefix: fmt.Sprintf("%s/issues/%d/close/", issue.Repo.FullName(), issue.Index),
},
{
name: "Close Pull",
args: args{
issue: pullIssue,
actionType: models.ActionClosePullRequest,
},
prefix: fmt.Sprintf("%s/pulls/%d/close/", issue.Repo.FullName(), issue.Index),
},
{
name: "Reopen Issue",
args: args{
issue: issue,
actionType: models.ActionReopenIssue,
},
prefix: fmt.Sprintf("%s/issues/%d/reopen/", issue.Repo.FullName(), issue.Index),
},
{
name: "Reopen Pull",
args: args{
issue: pullIssue,
actionType: models.ActionReopenPullRequest,
},
prefix: fmt.Sprintf("%s/pulls/%d/reopen/", issue.Repo.FullName(), issue.Index),
},
{
name: "Merge Pull",
args: args{
issue: pullIssue,
actionType: models.ActionMergePullRequest,
},
prefix: fmt.Sprintf("%s/pulls/%d/merge/", issue.Repo.FullName(), issue.Index),
},
{
name: "Ready Pull",
args: args{
issue: pullIssue,
actionType: models.ActionPullRequestReadyForReview,
},
prefix: fmt.Sprintf("%s/pulls/%d/ready/", issue.Repo.FullName(), issue.Index),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := createReference(tt.args.issue, tt.args.comment, tt.args.actionType)
if !strings.HasPrefix(got, tt.prefix) {
t.Errorf("createReference() = %v, want %v", got, tt.prefix)
}
if !strings.HasSuffix(got, tt.suffix) {
t.Errorf("createReference() = %v, want %v", got, tt.prefix)
}
})
}
}