From: lpsolit%gmail.com <> Date: Wed, 14 Dec 2005 05:16:16 +0000 (+0000) Subject: Bug 266147: Internal error when Flag::notify() ends up with an invalid or empty CC... X-Git-Tag: bugzilla-2.20.1~61 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ecd2b22fbe5ad03085a200df4efc4fb999ca6518;p=thirdparty%2Fbugzilla.git Bug 266147: Internal error when Flag::notify() ends up with an invalid or empty CC: list and no requestee in restricted bugs - Patch by Frédéric Buclin r=myk a=justdave --- diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 9c8a857f70..760731b281 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -19,7 +19,7 @@ # # Contributor(s): Myk Melez # Jouni Heikniemi -# Frédéric Buclin +# Frédéric Buclin =head1 NAME @@ -538,12 +538,13 @@ sub create { $timestamp)"); # Send an email notifying the relevant parties about the flag creation. - if (($flag->{'requestee'} - && $flag->{'requestee'}->wants_mail([EVT_FLAG_REQUESTED])) - || $flag->{'type'}->{'cc_list'}) + if ($flag->{'requestee'} + && $flag->{'requestee'}->wants_mail([EVT_FLAG_REQUESTED])) { - notify($flag, "request/email.txt.tmpl"); + $flag->{'addressee'} = $flag->{'requestee'}; } + + notify($flag, "request/email.txt.tmpl"); } =pod @@ -645,14 +646,25 @@ sub modify { modification_date = $timestamp , is_active = 1 WHERE id = $flag->{'id'}"); - - # Send an email notifying the relevant parties about the fulfillment. - if ($flag->{'setter'}->wants_mail([EVT_REQUESTED_FLAG]) - || $flag->{'type'}->{'cc_list'}) - { - $flag->{'status'} = $status; - notify($flag, "request/email.txt.tmpl"); + + # If the status of the flag was "?", we have to notify + # the requester (if he wants to). + my $requester; + if ($flag->{'status'} eq '?') { + $requester = $flag->{'setter'}; } + # Now update the flag object with its new values. + $flag->{'setter'} = Bugzilla->user; + $flag->{'requestee'} = undef; + $flag->{'status'} = $status; + + # Send an email notifying the relevant parties about the fulfillment, + # including the requester. + if ($requester && $requester->wants_mail([EVT_REQUESTED_FLAG])) { + $flag->{'addressee'} = $requester; + } + + notify($flag, "request/email.txt.tmpl"); } elsif ($status eq '?') { # Get the requestee, if any. @@ -661,6 +673,11 @@ sub modify { $requestee_id = login_to_id($requestee_email); $flag->{'requestee'} = new Bugzilla::User($requestee_id); } + else { + # If the status didn't change but we only removed the + # requestee, we have to clear the requestee field. + $flag->{'requestee'} = undef; + } # Update the database with the changes. &::SendSQL("UPDATE flags @@ -670,14 +687,19 @@ sub modify { modification_date = $timestamp , is_active = 1 WHERE id = $flag->{'id'}"); - + + # Now update the flag object with its new values. + $flag->{'setter'} = Bugzilla->user; + $flag->{'status'} = $status; + # Send an email notifying the relevant parties about the request. - if ($flag->{'requestee'} - && ($flag->{'requestee'}->wants_mail([EVT_FLAG_REQUESTED]) - || $flag->{'type'}->{'cc_list'})) + if ($flag->{'requestee'} + && $flag->{'requestee'}->wants_mail([EVT_FLAG_REQUESTED])) { - notify($flag, "request/email.txt.tmpl"); + $flag->{'addressee'} = $flag->{'requestee'}; } + + notify($flag, "request/email.txt.tmpl"); } # The user unset the flag; set is_active = 0 elsif ($status eq 'X') { @@ -711,12 +733,24 @@ sub clear { &::SendSQL("UPDATE flags SET is_active = 0 WHERE id = $id"); &::PopGlobalSQLState(); + # If we cancel a pending request, we have to notify the requester + # (if he wants to). + my $requester; + if ($flag->{'status'} eq '?') { + $requester = $flag->{'setter'}; + } + + # Now update the flag object to its new values. The last + # requester/setter and requestee are kept untouched (for the + # record). Else we could as well delete the flag completely. $flag->{'exists'} = 0; - # Set the flag's status to "cleared" so the email template - # knows why email is being sent about the request. $flag->{'status'} = "X"; - - notify($flag, "request/email.txt.tmpl") if $flag->{'requestee'}; + + if ($requester && $requester->wants_mail([EVT_REQUESTED_FLAG])) { + $flag->{'addressee'} = $requester; + } + + notify($flag, "request/email.txt.tmpl"); } @@ -891,7 +925,8 @@ sub GetTarget { =item C -Sends an email notification about a flag being created or fulfilled. +Sends an email notification about a flag being created, fulfilled +or deleted. =back @@ -900,6 +935,9 @@ Sends an email notification about a flag being created or fulfilled. sub notify { my ($flag, $template_file) = @_; + # There is nobody to notify. + return unless ($flag->{'addressee'} || $flag->{'type'}->{'cc_list'}); + # If the target bug is restricted to one or more groups, then we need # to make sure we don't send email about it to unauthorized users # on the request type's CC: list, so we have to trawl the list for users @@ -923,10 +961,11 @@ sub notify { $flag->{'type'}->{'cc_list'} = join(", ", @new_cc_list); } - $flag->{'requestee'}->{'email'} .= Param('emailsuffix'); - $flag->{'setter'}->{'email'} .= Param('emailsuffix'); + # If there is nobody left to notify, return. + return unless ($flag->{'addressee'} || $flag->{'type'}->{'cc_list'}); + $::vars->{'flag'} = $flag; - + my $message; my $rv = $::template->process($template_file, $::vars, \$message); diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl index d59cad0ec5..9c3be33875 100644 --- a/template/en/default/request/email.txt.tmpl +++ b/template/en/default/request/email.txt.tmpl @@ -18,6 +18,7 @@ # # Contributor(s): Myk Melez # Jeff Hedlund + # Frédéric Buclin #%] [% PROCESS global/variables.none.tmpl %] @@ -28,18 +29,16 @@ [% statuses = { '+' => "granted" , '-' => 'denied' , 'X' => "cancelled" , '?' => "asked" } %] [% IF flag.status == '?' %] - [% to_email = flag.requestee.email - IF flag.requestee.wants_mail([constants.EVT_FLAG_REQUESTED]) %] - [% to_identity = flag.requestee.identity %] - [% subject_status = "requested" %] + [% to_identity = flag.addressee.identity _ " for" %] + [% subject_status = "requested" %] [% ELSE %] - [% to_email = flag.setter.email - IF flag.setter.wants_mail([constants.EVT_REQUESTED_FLAG]) %] - [% to_identity = flag.setter.identity _ "'s request" %] - [% subject_status = statuses.${flag.status} %] + [% IF flag.addressee %] + [% to_identity = flag.addressee.identity _ "'s request for" %] + [% END %] + [% subject_status = statuses.${flag.status} %] [% END %] From: bugzilla-request-daemon -To: [% to_email %] +To: [% flag.addressee.email %] CC: [% flag.type.cc_list %] Subject: [% flag.type.name %] [%+ subject_status %]: [[% terms.Bug %] [%+ flag.target.bug.id %]] [% flag.target.bug.summary %] [%- IF flag.target.attachment.exists %] : @@ -48,7 +47,7 @@ Subject: [% flag.type.name %] [%+ subject_status %]: [[% terms.Bug %] [%+ flag.t [%+ USE wrap -%] [%- FILTER bullet = wrap(80) -%] -[% user.identity %] has [% statuses.${flag.status} %] [%+ to_identity %] for [% flag.type.name %]: +[% user.identity %] has [% statuses.${flag.status} %] [%+ to_identity %] [%+ flag.type.name %]: [% terms.Bug %] [%+ bugidsummary %] [% END %]