Delay the creation of the reviewers change log message to avoid "Re:" subject on initial comment

Delay the creation of the reviewers change log message to avoid "Re:" subject on initial comment

[jrobbins (chromium), 2014-08-25 22:15:37]

A user pointed out that the initial messages were being sent in emails that had
a subject line starting with "Re:".  This change avoids even generating the
automatic log message until after the user's message is generated, so the "Re:"
logic works.  The autogenerated message is still saved first, so that it shows
up ahead of the user's message in the message list.

Example of the defect: on the email you get with this diff, there will be a
"Re:" even though it is the first email message in the thread to be sent out.

[jrobbins (corp), 2014-08-25 22:16:58]

Sigh, this email is not an example of the defect because it was from
codereview.appspot.com.  But, it does happen on codereview.chromium.org.


On Mon, Aug 25, 2014 at 3:15 PM, <jrobbins@chromium.org> wrote:

> Reviewers: rmistry,
>
> Message:
> A user pointed out that the initial messages were being sent in emails
> that had a subject line starting with "Re:".  This change avoids even
> generating the automatic log message until after the user's message is
> generated, so the "Re:" logic works.  The autogenerated message is still
> saved first, so that it shows up ahead of the user's message in the
> message list.
>
> Example of the defect: on the email you get with this diff, there will
> be a "Re:" even though it is the first email message in the thread to be
> sent out.
>
> Description:
> Delay the creation of the reviewers change log message to avoid "Re:"
> subject on initial comment
>
> Please review this at https://codereview.appspot.com/126680043/
>
> Affected files (+5, -2 lines):
>   M codereview/views.py
>
>
> Index: codereview/views.py
> ===================================================================
> --- a/codereview/views.py
> +++ b/codereview/views.py
> @@ -2839,8 +2839,7 @@
>    if not form.is_valid():
>      return respond(request, 'publish.html', {'form': form, 'issue':
> issue})
>
> -  log_msg = _log_reviewers_if_changed(
> -    request=request, orig_reviewers=issue.reviewers,
> new_reviewers=reviewers)
> +  orig_reviewers = issue.reviewers
>    issue.reviewers = reviewers
>    issue.cc = cc
>    if form.cleaned_data['commit'] and not issue.closed:
> @@ -2864,6 +2863,10 @@
>                       form.cleaned_data['send_mail'],
>                       draft=draft_message,
>                       in_reply_to=form.cleaned_data.get('in_reply_to'))
> +  # Do this step last so that the zeroth message can be treated specially,
> +  # but put() it before put()ing the user's message.
> +  log_msg = _log_reviewers_if_changed(
> +    request=request, orig_reviewers=orig_reviewers,
> new_reviewers=reviewers)
>    if log_msg:
>      tbd.append(log_msg)
>    tbd.append(msg)
>
>
>

[rmistry, 2014-08-26 13:19:08]

LGTM

[jrobbins (corp), 2014-08-26 16:10:32]

Committed as
https://code.google.com/p/rietveld/source/detail?r=af712bf0708b4ec1043c2783e1...