Skip to content

Gmail sorting problem with PR emails in 1.16.0 #18560

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

Closed
parnic-sks opened this issue Feb 2, 2022 · 5 comments · Fixed by #18566
Closed

Gmail sorting problem with PR emails in 1.16.0 #18560

parnic-sks opened this issue Feb 2, 2022 · 5 comments · Fixed by #18566
Labels
Milestone

Comments

@parnic-sks
Copy link

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:
image

@parnic-sks parnic-sks changed the title Regression in 1.16.0: Gmail sorting problem with PR emails Gmail sorting problem with PR emails in 1.16.0 Feb 2, 2022
@zeripath
Copy link
Contributor

zeripath commented Feb 2, 2022

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
Copy link
Author

@zeripath
Copy link
Contributor

zeripath commented Feb 2, 2022

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

Message-ID: parnic-sks/pr-email-test/pulls/[email protected]

@zeripath
Copy link
Contributor

zeripath commented Feb 2, 2022

Whereas for the comments it looks like:

Message-ID: parnic-sks/pr-email-test/pulls/1/comment/[email protected]
In-Reply-To: parnic-sks/pr-email-test/pulls/[email protected]

@zeripath
Copy link
Contributor

zeripath commented Feb 2, 2022

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

zeripath added a commit to zeripath/gitea that referenced this issue Feb 2, 2022
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 go-gitea#18560

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath added this to the 1.16.1 milestone Feb 2, 2022
techknowlogick pushed a commit that referenced this issue Feb 3, 2022
…18566)

* Prevent merge messages from being sorted to the top of email chains

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 <[email protected]>

* add test

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this issue Feb 3, 2022
…o-gitea#18566)

Backport go-gitea#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 go-gitea#18560

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit that referenced this issue Feb 4, 2022
…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 <[email protected]>
Chianina pushed a commit to Chianina/gitea that referenced this issue Mar 28, 2022
…o-gitea#18566)

* Prevent merge messages from being sorted to the top of email chains

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 go-gitea#18560

Signed-off-by: Andrew Thornton <[email protected]>

* add test

Signed-off-by: Andrew Thornton <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants