From c9349805da679da8136ae030d887c5e2aae73aa2 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Thu, 17 Jun 2010 14:44:54 -0700 Subject: Bug 413215: Move the sending of email notifications from process_bug.cgi to Bugzilla::Bug r=dkl, a=mkanat --- Bugzilla/Bug.pm | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++ process_bug.cgi | 91 +++------------------------------------------------------ 2 files changed, 90 insertions(+), 87 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 378e219ff..3fb1dcf98 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -660,6 +660,8 @@ sub update { # inside this function. my $delta_ts = shift || $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); + $dbh->bz_start_transaction(); + my ($changes, $old_bug) = $self->SUPER::update(@_); # Certain items in $changes have to be fixed so that they hold @@ -889,6 +891,8 @@ sub update { $self->{delta_ts} = $delta_ts; } + $dbh->bz_commit_transaction(); + # The only problem with this here is that update() is often called # in the middle of a transaction, and if that transaction is rolled # back, this change will *not* be rolled back. As we expect rollbacks @@ -1016,6 +1020,88 @@ sub remove_from_db { return $self; } +##################################################################### +# Sending Email After Bug Update +##################################################################### + +sub send_changes { + my ($self, $changes, $vars) = @_; + + my $user = Bugzilla->user; + + my $old_qa = $changes->{'qa_contact'} + ? $changes->{'qa_contact'}->[0] : ''; + my $old_own = $changes->{'assigned_to'} + ? $changes->{'assigned_to'}->[0] : ''; + my $old_cc = $changes->{cc} + ? $changes->{cc}->[0] : ''; + + my %forced = ( + cc => [split(/[\s,]+/, $old_cc)], + owner => $old_own, + qacontact => $old_qa, + changer => $user, + ); + + _send_bugmail({ id => $self->id, type => 'bug', forced => \%forced }, + $vars); + + # If the bug was marked as a duplicate, we need to notify users on the + # other bug of any changes to that bug. + my $new_dup_id = $changes->{'dup_id'} ? $changes->{'dup_id'}->[1] : undef; + if ($new_dup_id) { + _send_bugmail({ forced => { changer => $user }, type => "dupe", + id => $new_dup_id }, $vars); + } + + # If there were changes in dependencies, we need to notify those + # dependencies. + my %notify_deps; + if ($changes->{'bug_status'}) { + my ($old_status, $new_status) = @{ $changes->{'bug_status'} }; + + # If this bug has changed from opened to closed or vice-versa, + # then all of the bugs we block need to be notified. + if (is_open_state($old_status) ne is_open_state($new_status)) { + $notify_deps{$_} = 1 foreach (@{ $self->blocked }); + } + } + + # To get a list of all changed dependencies, convert the "changes" arrays + # into a long string, then collapse that string into unique numbers in + # a hash. + my $all_changed_deps = join(', ', @{ $changes->{'dependson'} || [] }); + $all_changed_deps = join(', ', @{ $changes->{'blocked'} || [] }, + $all_changed_deps); + my %changed_deps = map { $_ => 1 } split(', ', $all_changed_deps); + # When clearning one field (say, blocks) and filling in the other + # (say, dependson), an empty string can get into the hash and cause + # an error later. + delete $changed_deps{''}; + + my %all_dep_changes = (%notify_deps, %changed_deps); + foreach my $id (sort { $a <=> $b } (keys %all_dep_changes)) { + _send_bugmail({ forced => { changer => $user }, type => "dep", + id => $id }, $vars); + } +} + +sub _send_bugmail { + my ($params, $vars) = @_; + + my $results = + Bugzilla::BugMail::Send($params->{'id'}, $params->{'forced'}); + + if (Bugzilla->usage_mode == USAGE_MODE_BROWSER) { + my $template = Bugzilla->template; + $vars->{$_} = $params->{$_} foreach keys %$params; + $vars->{'sent_bugmail'} = $results; + $template->process("bug/process/results.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + $vars->{'header_done'} = 1; + } +} + ##################################################################### # Validators ##################################################################### diff --git a/process_bug.cgi b/process_bug.cgi index bb4a9f653..f915a0893 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -75,19 +75,6 @@ my $vars = {}; # Subroutines ###################################################################### -# Used to send email when an update is done. -sub send_results { - my ($bug_id, $vars) = @_; - my $template = Bugzilla->template; - $vars->{'sent_bugmail'} = - Bugzilla::BugMail::Send($bug_id, $vars->{'mailrecipients'}); - if (Bugzilla->usage_mode != USAGE_MODE_EMAIL) { - $template->process("bug/process/results.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - } - $vars->{'header_done'} = 1; -} - # Tells us whether or not a field should be changed by process_bug. sub should_set { # check_defined is used for fields where there's another field @@ -214,7 +201,7 @@ if (defined $cgi->param('id')) { my $next_bug_id = $bug_list[$cur + 1]; detaint_natural($next_bug_id); if ($next_bug_id and $user->can_see_bug($next_bug_id)) { - # We create an object here so that send_results can use it + # We create an object here so that $bug->send_changes can use it # when displaying the header. $vars->{'bug'} = new Bugzilla::Bug($next_bug_id); } @@ -459,21 +446,10 @@ if ($move_action eq Bugzilla->params->{'move-button-text'}) { # Do Actual Database Updates # ############################## foreach my $bug (@bug_objects) { - $dbh->bz_start_transaction(); - - my $timestamp = $dbh->selectrow_array(q{SELECT LOCALTIMESTAMP(0)}); - my $changes = $bug->update($timestamp); + my $changes = $bug->update(); - my %notify_deps; if ($changes->{'bug_status'}) { - my ($old_status, $new_status) = @{ $changes->{'bug_status'} }; - - # If this bug has changed from opened to closed or vice-versa, - # then all of the bugs we block need to be notified. - if (is_open_state($old_status) ne is_open_state($new_status)) { - $notify_deps{$_} = 1 foreach (@{$bug->blocked}); - } - + my $new_status = $changes->{'bug_status'}->[1]; # We may have zeroed the remaining time, if we moved into a closed # status, so we should inform the user about that. if (!is_open_state($new_status) && $changes->{'remaining_time'}) { @@ -482,66 +458,7 @@ foreach my $bug (@bug_objects) { } } - # To get a list of all changed dependencies, convert the "changes" arrays - # into a long string, then collapse that string into unique numbers in - # a hash. - my $all_changed_deps = join(', ', @{ $changes->{'dependson'} || [] }); - $all_changed_deps = join(', ', @{ $changes->{'blocked'} || [] }, - $all_changed_deps); - my %changed_deps = map { $_ => 1 } split(', ', $all_changed_deps); - # When clearning one field (say, blocks) and filling in the other - # (say, dependson), an empty string can get into the hash and cause - # an error later. - delete $changed_deps{''}; - - $dbh->bz_commit_transaction(); - - ############### - # Send Emails # - ############### - - my $old_qa = $changes->{'qa_contact'} ? $changes->{'qa_contact'}->[0] : ''; - my $old_own = $changes->{'assigned_to'} ? $changes->{'assigned_to'}->[0] : ''; - my $old_cc = $changes->{cc} ? $changes->{cc}->[0] : ''; - $vars->{'mailrecipients'} = { - cc => [split(/[\s,]+/, $old_cc)], - owner => $old_own, - qacontact => $old_qa, - changer => Bugzilla->user }; - - $vars->{'id'} = $bug->id; - $vars->{'type'} = "bug"; - - # Let the user know the bug was changed and who did and didn't - # receive email about the change. - send_results($bug->id, $vars); - - # If the bug was marked as a duplicate, we need to notify users on the - # other bug of any changes to that bug. - my $new_dup_id = $changes->{'dup_id'} ? $changes->{'dup_id'}->[1] : undef; - if ($new_dup_id) { - $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user }; - - $vars->{'id'} = $new_dup_id; - $vars->{'type'} = "dupe"; - - # Let the user know a duplication notation was added to the - # original bug. - send_results($new_dup_id, $vars); - } - - my %all_dep_changes = (%notify_deps, %changed_deps); - foreach my $id (sort { $a <=> $b } (keys %all_dep_changes)) { - $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user }; - $vars->{'id'} = $id; - $vars->{'type'} = "dep"; - - # Let the user (if he is able to see the bug) know we checked to - # see if we should email notice of this change to users with a - # relationship to the dependent bug and who did and didn't - # receive email about it. - send_results($id, $vars); - } + $bug->send_changes($changes, $vars); } if (Bugzilla->usage_mode == USAGE_MODE_EMAIL) { -- cgit v1.2.1