5020: Gmail not threading emails correctly for review request with newly added reviewers

arma_otter 
What version are you running?


5.0.5


What's the URL of the page containing the problem?


N/A


What steps will reproduce the problem?


    

  1. Create a new review request with one reviewer (A) and publish draft with send email
      

    • owner and reviewer A will both get an email
      • 

    

    2. 

  2. Reviewer A adds a diff comment and publishes review
      

    • owner and reviewer A will both receive an email in the same thread
      • 

    

    3. 

  3. Owner adds a new reviewer B to the review request and publishes with send email
      

    • owner gets an email in the existing thread
      • 

    • reviewer B gets a new email
      • 

    • reviewer A gets no email
      • 

    

    4. 

  4. Reviewer B adds a diff comment and publishes review
      

    • owner gets an email in the existing thread
      • 

    • reviewer A gets an email in a new thread
      • 

    • reviewer B gets an email in existing thread
      • 

    

    5. 





What is the expected output? What do you see instead?


After step 5, reviewer A should also get an email in the existing thread rather than have a new email thread.


What operating system are you using? What browser?


macOS 13.6.3, Chrome Version 121.0.6167.85


Please provide any additional information below.


If multiple reviewers are added at different times for a review request (which does happen often for us), then it's possible for a reviewer to get many different email threads for the same review request.


I've only seen this in gmail, I have not tested other email services.


This seems to be due to the email_message_id for a review request (in the reviews_reviewrequest table) being updated when a new reviewer is added, so an earlier email thread will no longer have a matching email message id to the email  'reference' or 'in-reply-to' fields to link up the emails.


Here's the steps to reproduce with example details on the email field values and database values:

1. Create a new review request with one reviewer (A) and publish draft with send email

    a. owner and reviewer A will both get an email

    b. email message id = 1

    c. reviews_reviewrequest record has email_message_id is set to 1

3. Reviewer A adds a diff comment and publishes review

    a. owner and reviewer A will both receive an email in the same thread

    b. email message id = 2

    c. email fields in-reply-to and references = message id 1

    d. reviews_reviewrequest record email_message_id is still set to 1

    e. reviews_review has a new record where the email_message_id = 2

4. Owner adds a new reviewer B to the review request and publishes with send email

    a. owner gets an email in the existing thread

    b. reviewer B gets a new email

    c. reviewer A gets no email

    d. email message id = 3

    e. email fields in-reply-to and references = message id 1

    f. reviews_reviewrequest record email_message_id is now set to 3

5. Reviewer B adds a diff comment and publishes review

    a. owner gets an email in the existing thread

    b. reviewer A gets an email in a new thread

    c. reviewer B gets an email in existing thread

    d. email message id = 4

    e. email fields in-reply-to and references = message id 3

    f. reviews_reviewrequest record email_message_id is still set to 3

    g. reviews_review has a new record where the email_message_id = 4


Reviewer B doesn't get an email in 5.b. because the message ids of 3 and 4 do not match with any previous emails (which have message ids 1 and 2).


According to google https://support.google.com/mail/answer/5900?hl=en&co=GENIE.Platform%3DDesktop#zippy=%2Chow-gmail-groups-automated-emails:

Emails are grouped if each message meets the following:

- The same recipients, senders, or subject as a previous message

- A reference header with the same IDs as a previous message

- Sent within one week of a previous message


I think the second point is not being met, so a new thread is created for the same review request.
#1 arma_otter 
I've upgraded to version 6.0.2 and still running into this issue.
david
#2 david 
What SMTP server are you using to send the messages?


We only update the email_message_id when the SMTP server tells us that it changed it.
  • -New
    +NeedInfo
#3 arma_otter 
We are using Postfix.


I think we are not satisfying the Gmail threading condition: 'A reference header with the same IDs as a previous message'


In same cases, when replying to a review, the email only contains the message id of the review email it's replying to in the 'Reply-to' and 'References' headers. It doesn't know that this is linked with the initial review request email since it doesn't have its message id in those headers, causing split email threads to occur. I think appending the review request message id to the 'References' header should be able to satisfy this condition.
raymond.lam
#4 raymond.lam 
After poking around, I believe the issues arises from this section of code which filters out recipients based on whether the change involves only updates for target reviewers: (https://github.com/reviewboard/reviewboard/blob/master/reviewboard/notifications/email/message.py#L507)

If this is the case, then owners of the review_request shouldn't experience the separate threading, this is because the build_recipients function will always include the user of the review_request. (https://github.com/reviewboard/reviewboard/blob/master/reviewboard/notifications/email/utils.py#L48)

Adding the reference header doesn't seem to be enough, since once the reviewers are added and the review_request draft is published, the message_id attached to the original review_request is modified (https://github.com/reviewboard/reviewboard/blob/master/reviewboard/notifications/email/signal_handlers.py#L261)


Solutions might involve keeping track of all previous message_ids and adding those to the Headers["References"] field, but that might be too much, otherwise removal of the filtering will solve it (but that might cause too much noise)