583287 - Some fields should not be displayed in bugmail for new bugs

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, defect)

Description

[Frédéric Buclin]

Creation date: 2010.07.30 18:55
 Last changed date: 2010-07-30 18:55:38
          Deadline: 1970-01-01

These fields should not be displayed in bugmail we get for new bugs. The creation and last changed dates are totally useless as the bug just got created, and the deadline should not be displayed either when not set (no idea why it falls back to 1970 when left empty).

This is a regression in 4.0. Bugzilla 3.6.1 doesn't have these problems. Note that in_new_bugmail => 1 has been added by bug 289357 in Bugzilla 3.0 (for some obscure reason).

No idea what changed between 3.6 and 4.0 in the email notification area which triggered these regressions.

Comment 1

[Max Kanat-Alexander]

The Deadline thing is almost certainly Pg-specific.

But the other fields should not appear, I agree.

Comment 2

[Frédéric Buclin]

(In reply to comment #1)
> The Deadline thing is almost certainly Pg-specific.

No, this happens on MySQL too. I found the culprit. In BugMail.pm, we have:

 elsif ($name eq 'deadline') {
     $value = time2str("%Y-%m-%d", str2time($value));

If $value is empty, time2str() returns 1970-01-01. It is also responsible for all these warnings in the web server error log:

Use of uninitialized value $time in localtime at /opt/perl/lib/site_perl/5.12.1/Date/Format.pm line 121.

Comment 3

[Frédéric Buclin]

Attachment: patch, v1

This fixes 2 things:
- creation_ts and delta_ts are no longer in_new_bugmail
- don't format estimated_time nor deadline if not set.

$bug->estimated_time returns "0.00", and so "return unless $value" sees $value as being true instead of wrong, which is why I force it to be 0 when not set, so that this field is correctly skipped.

Comment 4

[Frédéric Buclin]

At least for deadline, this is a regression due to bug 395451.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 461980 [details] [diff] [review]
patch, v1

Ah, very cool. :-) Looks good. :-)

It would be nice to reverse things to "($value == 0)" instead, though, on checkin. (I think it's usually clearer to have positive booleans.)

Comment 6

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/BugMail.pm
modified Bugzilla/Field.pm
Committed revision 7419.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified Bugzilla/BugMail.pm
modified Bugzilla/Field.pm
Committed revision 7362.