Closed Bug 1016273 Opened 7 years ago Closed 7 years ago

Bugzilla whine emails missing change to From header made by bug 1010751

Categories

(bugzilla.mozilla.org :: General, defect)

Production
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: emorley, Assigned: dkl)

References

Details

Attachments

(1 file)

Assignee: nobody → dkl
Status: NEW → ASSIGNED
Attached patch 1016273_1.patchSplinter Review
Attachment #8429418 - Flags: review?(glob)
Comment on attachment 8429418 [details] [diff] [review]
1016273_1.patch

Review of attachment 8429418 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob, with the following to be fixed on commit:

all of these are missing quotes around the BugzillaTitle term.
it needs to be:  "Bugzilla@Mozilla" <...>
Attachment #8429418 - Flags: review?(glob) → review+
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   24a16ae..775b7cc  master -> master
Closing.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
this causes whine.pl to throw warnings:

Use of uninitialized value $lang in hash element at Bugzilla/Util.pm line 753.
Use of uninitialized value $lang in hash element at Bugzilla/Util.pm line 766.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
i was poking around the code...  i think i made a mistake with my original patch.

i think it could be better to revert this patch and bug 1010751, and instead change the mailfrom parameter's value to:  "Bugzilla@Mozilla" <bugzilla-daemon@mozilla.org>

as far i can tell mailfrom is always inserted unfiltered/unmodified into an email, but that probably needs more checking if we take that path.

dkl, are you able to double-check if that would work?
(In reply to Byron Jones ‹:glob› from comment #6) 
> i think it could be better to revert this patch and bug 1010751, and instead
> change the mailfrom parameter's value to:  "Bugzilla@Mozilla"
> <bugzilla-daemon@mozilla.org>

That is an alternative way to do it yes. I liked the idea of an admin not having
to worry about From: formatting and it would just put the two values together
automatically (email address and site title). But I suppose an admin may want to
have the site title possible be different than what is in the email From: header
so would want to designate the value manually.
 
> as far i can tell mailfrom is always inserted unfiltered/unmodified into an
> email, but that probably needs more checking if we take that path.

It is. As for filtering we could leave it unfiltered as it is an admin param and
we have always assumed that the admin would know what they are doing.

> dkl, are you able to double-check if that would work?

1. Updated the data/params to:
   'mailfrom' => '"Bugzilla@Mozilla" <bugzilla-admin@mozilla.com>',

2. And performed the below git commands to roll back related commits:

$ git revert -n 1af952ceaa1c06a10e0eeb6f139e5a5652002e98
$ git revert -n 3f35dd081156d231710553099f01f40481aa187a
$ git revert -n 6c7add0ece060f9a945c7b18b4d03546a2220043
$ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#	modified:   email_in.pl
#	modified:   extensions/RequestNagger/template/en/default/email/request_nagging-requestee-header.txt.tmpl
#	modified:   extensions/RequestNagger/template/en/default/email/request_nagging-watching-header.txt.tmpl
#	modified:   extensions/SecureMail/template/en/default/account/email/securemail-test.txt.tmpl
#	modified:   extensions/Voting/template/en/default/voting/votes-removed.txt.tmpl
#	modified:   importxml.pl
#	modified:   template/en/default/account/cancel-token.txt.tmpl
#	modified:   template/en/default/account/email/change-new.txt.tmpl
#	modified:   template/en/default/account/email/change-old.txt.tmpl
#	modified:   template/en/default/account/email/request-new.txt.tmpl
#	modified:   template/en/default/account/password/forgotten-password.txt.tmpl
#	modified:   template/en/default/email/bugmail-header.txt.tmpl
#	modified:   template/en/default/email/lockout.txt.tmpl
#	modified:   template/en/default/email/sanitycheck.txt.tmpl
#	modified:   template/en/default/email/sudo.txt.tmpl
#	modified:   template/en/default/email/whine.txt.tmpl
#	modified:   template/en/default/request/email.txt.tmpl
#	modified:   whine.pl

3. All of the different email tests I performed properly filled in the From: header:

From: "Bugzilla@Mozilla" <bugzilla-admin@mozilla.com>

So I would say this would be fine to roll these changes back and instead update the params value if you concur.

dkl
Flags: needinfo?(glob)
(In reply to David Lawrence [:dkl] from comment #7)
> But I suppose an admin may want to have the site title possible be
> different than what is in the email From: header so would want to
> designate the value manually.

the site title is a bmo customisation, so there's no need to worry about any admins aside from us.

> So I would say this would be fine to roll these changes back and instead
> update the params value if you concur.

let's do that :)
Flags: needinfo?(glob)
(In reply to Byron Jones ‹:glob› from comment #8)
> let's do that :)

Committed. Make sure to update mailfrom param right after so people do not notice any difference.

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   ac9980a..685776f  master -> master

dkl
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
param changed to: "Bugzilla@Mozilla" <bugzilla-daemon@mozilla.org>
You need to log in before you can comment on or make changes to this bug.