Gmail sorting problem with PR emails in 1.16.0

[parnic-sks]

Gitea Version

1.16.0

Git Version

2.35.0

Operating System

Ubuntu 20.04.3, aarch64/arm64

How are you running Gitea?

Built myself from tag v1.16.0
Also reproducible on https://try.gitea.io/

Database

PostgreSQL

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Description

This PR on try.gitea.io has the "merged" notification sorted above all other emails in the PR thread (see screenshot below). It also happens on our private instance. PR email threads are confusing to read as a result.

This was not a problem in 1.15.10; messages were previously sorted in chronological order.

Possibly related: #17206, #17900 ?

Repro was:

• Create initialized repo with an account whose email address uses Gmail
• Add collaborator with write access
• Collaborator makes one commit in a branch
• Collaborator creates a PR and assigns it to the repo owner
• Collaborator merges PR

Screenshots

Despite the "merged" email having a later timestamp, it is displayed as the first message in the thread on Gmail:

[zeripath]

I think it would be useful to see the Message-IDs and the In-Reply-To here. I suspect that the problem is that the merged message doesn't have the correct In-Reply-To or even Message-ID

[parnic-sks]

Certainly.

Headers for the creation email
Headers for the comment
Headers for the merged notification

[zeripath]

yup it looks like the message-id for the merged notification is:

Message-ID: parnic-sks/pr-email-test/pulls/1@try.gitea.io

[zeripath]

Whereas for the comments it looks like:

Message-ID: parnic-sks/pr-email-test/pulls/1/comment/113263@try.gitea.io
In-Reply-To: parnic-sks/pr-email-test/pulls/1@try.gitea.io

[zeripath]

So Gmail will call the comments as replies to the the merged notification and will reorder things...

Clearly this is not ideal.

Anything that comes through:

// MailParticipants sends new issue thread created emails to repository watchers
// and mentioned people.
func MailParticipants(issue *models.Issue, doer *user_model.User, opType models.ActionType, mentions []*user_model.User) error {
	if setting.MailService == nil {
		// No mail service configured
		return nil
	}

	content := issue.Content
	if opType == models.ActionCloseIssue || opType == models.ActionClosePullRequest ||
		opType == models.ActionReopenIssue || opType == models.ActionReopenPullRequest ||
		opType == models.ActionMergePullRequest {
		content = ""
	}
	if err := mailIssueCommentToParticipants(
		&mailCommentContext{
			Issue:      issue,
			Doer:       doer,
			ActionType: opType,
			Content:    content,
			Comment:    nil,
		}, mentions); err != nil {
		log.Error("mailIssueCommentToParticipants: %v", err)
	}
	return nil
}

will get the same id.

so we need to change:

func createReference(issue *models.Issue, comment *models.Comment) string {
	var path string
	if issue.IsPull {
		path = "pulls"
	} else {
		path = "issues"
	}

	var extra string
	if comment != nil {
		extra = fmt.Sprintf("/comment/%d", comment.ID)
	}

	return fmt.Sprintf("%s/%s/%d%s@%s", issue.Repo.FullName(), path, issue.Index, extra, setting.Domain)
}

to take account of these different actiontypes