452761 - TheSchwartz queuing appears to timestamp bugmail when it gets sent instead of when the bug change happened

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, defect)

Description

[Dave Miller [:justdave]]

Discovered this after our upgrade last night when we had bugmail for several bugs that was created during our testing phase before the site went live released from the queue a little late...  they all got timestamped when the mail was actually sent.  It probably would have made better sense to timestamp them when the bug change happened.

Comment 1

[Reed Loden [:reed]]

Attachment: patch - v1

Not yet tested, but this seems simple and correct enough from my reading of Bugzilla::BugMail, Bugzilla::Mailer, and Bugzilla::Job::Mailer.

Comment 2

[Max Kanat-Alexander]

Comment on attachment 455788 [details] [diff] [review]
patch - v1

I think you want FILTER time, not time2str. The Date header right now looks like:

Date: Wed, 31 Dec 1969 16:33:30 -0800

(And note that that timezone is wrong--it should be -0700 on my machine.)

Comment 3

[Reed Loden [:reed]]

Attachment: patch - v2

Also untested, but since you're in a review swing, throwing it in your queue. I'll test it as soon as I get back home tonight.

Comment 4

[Max Kanat-Alexander]

Comment on attachment 455991 [details] [diff] [review]
patch - v2

Hmm. This is technically still trunk...how hard would it be to make "to" an object in flagmail? Then you could just get the timezone.

Comment 5

[Reed Loden [:reed]]

(In reply to comment #4)
> (From update of attachment 455991 [details] [diff] [review])
> Hmm. This is technically still trunk...how hard would it be to make "to" an
> object in flagmail? Then you could just get the timezone.

That's basically what I started to do in bug 390586. I guess I just need to finish that patch.

Comment 6

[Max Kanat-Alexander]

Comment on attachment 455991 [details] [diff] [review]
patch - v2

Okay, this looks good, except that it adds an extra space between the weekday name and the day (probably because the day is only one digit), which, while technically fine per the RFC, the RFC also "recommends" that you don't add extra whitespace in this header.

Comment 7

[Reed Loden [:reed]]

Attachment: patch - v3

Use %d instead of %e. This zero-pads the field instead, which is permitted, as per the RFC.

Comment 8

[Max Kanat-Alexander]

Comment on attachment 456235 [details] [diff] [review]
patch - v3

Looks fine. :-)

Comment 9

[Max Kanat-Alexander]

I'm about to branch, so I checked this in myself. Thanks for the patch, reed! :-)

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Flag.pm
modified Bugzilla/Mailer.pm
modified template/en/default/email/newchangedmail.txt.tmpl
modified template/en/default/request/email.txt.tmpl
Committed revision 7294.

Comment 10

[Max Kanat-Alexander]

Added to the release notes in bug 604256.