100089 - Templatise 'passwordmail' email

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, enhancement, P1)

Description

[Matthew Tuck [:CodeMachine]]

We should aim to get rid of the "template" parameters now we have templates. 
These include things like bannerhtml, footerhtml & newchangedmail.

Comment 1

[Matthew Tuck [:CodeMachine]]

Whoops, wrong component.

Comment 2

[Dave Miller [:justdave]]

templates can include other templates, right?  If so this would probably work.

We'd need some sort of routine to compare the existing contents of those
parameters with the defaults and if they're different, create a copy of the
template with those contents in the custom folder...

Comment 3

[Matthew Tuck [:CodeMachine]]

Yes - currently there are footer and header templates.  However they currently
take the existing parameters.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

It makes sense to do this after we finish templatizing the entire application,
since then we can make the change in one place and have it take effect for every
script.  Making this bug depend on bug 86168.

Comment 5

[Dave Miller [:justdave]]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 6

[Matthew Tuck [:CodeMachine]]

The HTML templates were removed over at bug #140437.  That just leaves the mail
templates.

Comment 7

[J. Paul Reed [:preed]]

->Email notifications, since this is now a part of the bigger
making-mail-notifications-not-suck picture, and will have to be addressed as
part of bug 84876.

Comment 8

[Max Kanat-Alexander]

Let's break this down. It should be really easy to templatize passwordmail. I'll
file bugs for whinemail, newchangedmail, and voteremovedmail.

Comment 9

[Max Kanat-Alexander]

As preed is no longer actively working on Bugzilla (from what I know), I somehow
doubt that this bug will make it in for 2.20. :-) If I'm wrong, feel free to
take it back and re-target it. :-)

Comment 10

[Emmanuel Seyman]

Attachment: Patch to defparams.pl and CGI.pl to use a template instead of Param(passwordmail)

This modifies MailPassword() to use a template file instead of a parameter.

Comment 11

[Emmanuel Seyman]

Attachment: template for account creation mail

Template to be used for creating the mail.
Should be in template/en/default/email/passwordmail.txt.tmpl

Comment 12

[Max Kanat-Alexander]

Comment on attachment 182624 [details]
template for account creation mail

Kiko is listed as a template reviewer in
http://www.bugzilla.org/docs/reviewer-list.html#templates so he can probably
look over this. (Normally bugmail would probably be Jake, but I think he's not
around much lately.)

Comment 13

[Max Kanat-Alexander]

Comment on attachment 182623 [details] [diff] [review]
Patch to defparams.pl and CGI.pl to use a template instead of Param(passwordmail)

Hey Erik, I think this is mostly a param change, want to review it?

Comment 14

[Frédéric Buclin]

Comment on attachment 182623 [details] [diff] [review]
Patch to defparams.pl and CGI.pl to use a template instead of Param(passwordmail)

CGI.pl no longer exists.

Comment 15

[Emmanuel Seyman]

Attachment: removing the passwordmail param from Bugzilla, docs and all

Comment 16

[Emmanuel Seyman]

Attachment: template for account creation mail

Comment 17

[Frédéric Buclin]

Attachment: patch, v2

merging the two patches from manu in a single one, per his request on IRC.

Comment 18

[Frédéric Buclin]

Comment on attachment 201614 [details] [diff] [review]
patch, v2

>Index: Bugzilla/BugMail.pm

>+    my $vars = {
>+      urlbase => Param("urlbase") };

Param("urlbase") is already available from templates directly.


>+    my $template = Bugzilla->template;
>+    $template->process("email/password.mail.tmpl", $vars, \$msg)
>+      or die($template->error());

It should be "|| ThrowTemplateError($template->error());" instead of die().



>Index: template/en/default/email/password.mail.tmpl

The template name must be password.txt.tmpl.


>+To: [% mailaddress FILTER none %]

No need to filter directives as it's a txt template, not a html one.


>+ [%+ urlbase FILTER none %]userprefs.cgi

Write [% Param("urlbase") %] directly.

Comment 19

[Emmanuel Seyman]

Attachment: patch, v3

Comment 20

[Frédéric Buclin]

Comment on attachment 201624 [details] [diff] [review]
patch, v3

Nit: move Bugzilla->template at the beginning of MailPassword(). Else it works fine. r=LpSolit

Unrelated comment: BugMail::MailPassword() should be removed and its code moved to createaccount.cgi.

Comment 21

[Myk Melez [:myk] [@mykmelez]]

Err, this should wait for 2.24.

Comment 22

[Colin Ogilvie [:cso]]

Checked in....

Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.64; previous revision: 1.63
done
Checking in Bugzilla/Config/MTA.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/MTA.pm,v  <--  MTA.pm
new revision: 1.5; previous revision: 1.4
done
Checking in docs/xml/administration.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/administration.xml,v  <--  administration.xml
new revision: 1.58; previous revision: 1.57
done
Checking in template/en/default/admin/params/mta.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/params/mta.html.tmpl,v  <--  mta.html.tmpl
new revision: 1.3; previous revision: 1.2
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/email/password.txt.tmpl,v
done
Checking in template/en/default/email/password.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/email/password.txt.tmpl,v  <--  password.txt.tmpl
initial revision: 1.1
done

Comment 23

[Max Kanat-Alexander]

Added to relnotes in bug 349423. Please let me know if I missed any critical information about this bug in the release notes.