556429 - Stop sending email notifications from inside the template

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, enhancement)

Description

[Max Kanat-Alexander]

Currently, template/en/default/bug/process/bugmail.html.tmpl actually calls SendBugMail inside of itself, which goes out to the Perl code to send email. Normally this is just silly, but unfortunately, a bug in Template Toolkit ( https://rt.cpan.org/Ticket/Display.html?id=47929 ) combined with recent versions of DateTime make it into a serious issue. When using edit-multiple to modify bugs, instead of getting a result, Template Toolkit causes Perl to exit immediately while the BugMail is being created, so you just get a blank white page.

  On HEAD, the same problem happens if a vote confirms a bug. It's pretty much impossible to say what's going to trigger it, but commenting out the SendBugMail line in the template always fixes it, so moving that code outside of the templates where it should be anyway will be our workaround for now.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

This was actually a lot easier than I expected it to be.

Comment 2

[miketosh]

(In reply to comment #1)
>   This was actually a lot easier than I expected it to be.

AFAICS, Looks like there is no functionality lost with this change.  The only impact would be extensions that make a similar change to this.  Those would have to accommodate, but that is normal for updating 3rd party extensions to work with the next version update.

Comment 3

[Max Kanat-Alexander]

(In reply to comment #2)
> AFAICS, Looks like there is no functionality lost with this change.  The only
> impact would be extensions that make a similar change to this.  Those would
> have to accommodate, but that is normal for updating 3rd party extensions to
> work with the next version update.

  Yeah, that's right. :-)

Comment 4

[Frédéric Buclin]

Which version(s) of DateTime are we talking about here? Editing several bugs at once is fine on my machine, using DateTime 0.50.

Comment 5

[Frédéric Buclin]

(In reply to comment #4)
> Which version(s) of DateTime are we talking about here? Editing several bugs at
> once is fine on my machine, using DateTime 0.50.

I tested editing several bugs at once with the latest DateTime and DateTime-TimeZone versions, and this is still working fine.

Comment 6

[Max Kanat-Alexander]

  What version of TT do you have installed? Are you using the XS Stash?

Comment 7

[Max Kanat-Alexander]

  On HEAD, I can always reproduce the bug by submitting a vote that confirms a bug, with the Voting extension turned on.

Comment 8

[Frédéric Buclin]

(In reply to comment #7)
>   On HEAD, I can always reproduce the bug by submitting a vote that confirms a
> bug, with the Voting extension turned on.

WFM. I'm using TT 2.22 (that's the only version we support) and yes, XS Stash is in use AFAIK.

Comment 9

[Frédéric Buclin]

Comment on attachment 436389 [details] [diff] [review]
v1

>=== modified file 'post_bug.cgi'

>+foreach my $dep (@{$bug->dependson || []}, @{$bug->blocked || []}) {
>+    my $dep_sent = Bugzilla::BugMail::Send($dep, $recipients);
>+    $dep_sent->{type} = 'dep';
>+    $dep_sent->{id}   = $id;

The last line must be: $dep_sent->{id}   = $dep; (not $id).


Also, please remove the following comments, which are no longer relevant:

Bugzilla/BugMail.pm line 110:
# This hash usually comes from the "mailrecipients" var in a template call.

template/en/default/bug/process/results.html.tmpl line 28:
# mailrecipients: hash; BugMail recipient params. Optional.

template/en/default/bug/create/created.html.tmpl lines 27-28:
# mailrecipients: hash; contains the BugMail recipients, for details on
#     this contents, see template bug/process/bugmail.html.tmpl


Otherwise looks good and works fine. r=LpSolit

Comment 10

[Max Kanat-Alexander]

Attachment: v1 (3.6)

Okay, the patch required a backport for 3.6, because voting is still part of core in 3.6. I haven't actually tested this, but I have checked that it compiles. The only differences are in votes.cgi, Bugzilla::Product, and template/en/default/admin/products/updated.html.tmpl.

Comment 11

[Max Kanat-Alexander]

  The backport also contains all the fixes you mentioned, by the way.

  Oh, and I just checked this in on trunk before realizing that it needed a 3.6 backport, so here's the trunk commit message:

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified attachment.cgi
modified post_bug.cgi
modified process_bug.cgi
modified Bugzilla/BugMail.pm
modified Bugzilla/Template.pm
modified extensions/Voting/Extension.pm                                        
modified extensions/Voting/template/en/default/hook/admin/products/updated-changes.html.tmpl
modified template/en/default/bug/create/created.html.tmpl
modified template/en/default/bug/process/bugmail.html.tmpl
modified template/en/default/bug/process/results.html.tmpl
modified template/en/default/pages/release-notes.html.tmpl
Committed revision 7113.

Comment 13

[Frédéric Buclin]

Comment on attachment 437163 [details] [diff] [review]
v1 (3.6)

Works fine. r=LpSolit

Comment 14

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/
modified attachment.cgi
modified post_bug.cgi
modified process_bug.cgi
modified votes.cgi
modified Bugzilla/BugMail.pm
modified Bugzilla/Product.pm
modified Bugzilla/Template.pm
modified template/en/default/admin/products/updated.html.tmpl
modified template/en/default/bug/create/created.html.tmpl
modified template/en/default/bug/process/bugmail.html.tmpl
modified template/en/default/bug/process/results.html.tmpl
modified template/en/default/pages/release-notes.html.tmpl
Committed revision 7075.