Last Comment Bug 379787 - Flag request mail has a blank requestee when requestee doesn't get the mail
: Flag request mail has a blank requestee when requestee doesn't get the mail
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Email Notifications (show other bugs)
: 3.0
: All All
: -- normal (vote)
: Bugzilla 3.0
Assigned To: Frédéric Buclin
: default-qa
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-04 16:01 PDT by Teemu Mannermaa (:wicked)
Modified: 2007-05-14 12:27 PDT (History)
1 user (show)
LpSolit: approval+
LpSolit: approval3.0+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch, v1 (1.92 KB, patch)
2007-05-08 10:11 PDT, Frédéric Buclin
wicked: review+
myk: review+
Details | Diff | Splinter Review

Description Teemu Mannermaa (:wicked) 2007-05-04 16:01:35 PDT
Request mails sent to flags CC List addresses can have a blank requestee even if the request has one. This happens when requestee of a flag request doesn't have "Email me when someone asks me to set a flag" email preference enabled and thus doesn't get the request mail.

For example, this is what first line of request mail looks like when requestee gets the notification:
"Test User <testuser@example.net> has asked Bugmaster <bugmaster@example.net> for test-cclist:"

And this is what it looks like when requestee doesn't get the notification:
"Test User <testuser@example.net> has asked  for test-cclist:"
Comment 1 Frédéric Buclin 2007-05-08 10:11:40 PDT
Created attachment 264123 [details] [diff] [review]
patch, v1

Requesting review for the 3.0 branch.
Comment 2 Teemu Mannermaa (:wicked) 2007-05-14 11:19:51 PDT
Comment on attachment 264123 [details] [diff] [review]
patch, v1

Seems to fix the problem and not break other notify mails.
Comment 3 Myk Melez [:myk] [@mykmelez] 2007-05-14 11:31:40 PDT
Comment on attachment 264123 [details] [diff] [review]
patch, v1

>             my $requester;
>             if ($flag->status eq '?') {
>                 $requester = $flag->setter;
>+                $flag->{'requester'} = $requester;
>             }

Nit: couldn't this be simply:

            if ($flag->status eq '?') {
                $flag->{'requester'} = $flag->setter;
            }

And then use $flag->{'requester'} instead of the $requester temporary variable later in the methods.

Otherwise this looks good, although the "addressee" and "to_identity" names are unfortunate, since the "addressee" isn't the only person to receive the message (and in fact might not receive it at all if not in the group to which the message is restricted), and the person identified by "to_identity" isn't necessarily the person to whom the message is being sent.

r=myk
Comment 4 Frédéric Buclin 2007-05-14 12:27:37 PDT
tip:

Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.84; previous revision: 1.83
done
Checking in template/en/default/request/email.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/request/email.txt.tmpl,v  <--  email.txt.tmpl
new revision: 1.17; previous revision: 1.16
done


3.0:

Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.83.2.1; previous revision: 1.83
done
Checking in template/en/default/request/email.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/request/email.txt.tmpl,v  <--  email.txt.tmpl
new revision: 1.14.2.2; previous revision: 1.14.2.1
done

Note You need to log in before you can comment on or make changes to this bug.