Bug 328449: Remove BugMail::MessageToMTA() from Bug::RemoveVotes() - Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=myk

3 files changed, 44 insertions, 14 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 302ac38b7..71b6bc7f4 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -38,7 +38,6 @@ use vars qw($legal_keywords @legal_platform
 use CGI::Carp qw(fatalsToBrowser);
 
 use Bugzilla::Attachment;
-use Bugzilla::BugMail;
 use Bugzilla::Config;
 use Bugzilla::Constants;
 use Bugzilla::Field;
@@ -1019,6 +1018,11 @@ sub RemoveVotes {
     while (my ($name, $userid, $oldvotes, $votesperuser, $maxvotesperbug) = $sth->fetchrow_array()) {
         push(@list, [$name, $userid, $oldvotes, $votesperuser, $maxvotesperbug]);
     }
+
+    # @messages stores all emails which have to be sent, if any.
+    # This array is passed to the caller which will send these emails itself.
+    my @messages = ();
+
     if (scalar(@list)) {
         foreach my $ref (@list) {
             my ($name, $userid, $oldvotes, $votesperuser, $maxvotesperbug) = (@$ref);
@@ -1079,7 +1083,7 @@ sub RemoveVotes {
             $substs{"count"} = $removedvotes . "\n    " . $newvotestext;
 
             my $msg = perform_substs(Param("voteremovedmail"), \%substs);
-            Bugzilla::BugMail::MessageToMTA($msg);
+            push(@messages, $msg);
         }
         my $votes = $dbh->selectrow_array("SELECT SUM(vote_count) " .
                                           "FROM votes WHERE bug_id = ?",
@@ -1087,6 +1091,8 @@ sub RemoveVotes {
         $dbh->do("UPDATE bugs SET votes = ? WHERE bug_id = ?",
                  undef, ($votes, $id));
     }
+    # Now return the array containing emails to be sent.
+    return \@messages;
 }
 
 # If a user votes for a bug, or the number of votes required to
diff --git a/editproducts.cgi b/editproducts.cgi
index d9ebcedd9..dc288a54b 100755
--- a/editproducts.cgi
+++ b/editproducts.cgi
@@ -23,7 +23,7 @@
 #               Dawn Endico <endico@mozilla.org>
 #               Joe Robins <jmrobins@tgix.com>
 #               Gavin Shelley <bugzilla@chimpychompy.org>
-#               Fr��ic Buclin <LpSolit@gmail.com>
+#               Frédéric Buclin <LpSolit@gmail.com>
 #               Greg Hendricks <ghendricks@novell.com>
 #               Lance Larsh <lance.larsh@oracle.com>
 #
@@ -39,6 +39,7 @@ require "globals.pl";
 use Bugzilla::Bug;
 use Bugzilla::Series;
 use Bugzilla::Config qw(:DEFAULT $datadir);
+use Bugzilla::BugMail;
 use Bugzilla::Product;
 use Bugzilla::Classification;
 use Bugzilla::Milestone;
@@ -899,9 +900,15 @@ if ($action eq 'update') {
 
             foreach my $vote (@$votes) {
                 my ($who, $id) = (@$vote);
-                RemoveVotes($id, $who, "The rules for voting on this product " .
-                                       "has changed;\nyou had too many votes " .
-                                       "for a single bug.");
+                # If some votes are removed, RemoveVotes() returns a list
+                # of messages to send to voters.
+                my $msgs =
+                    RemoveVotes($id, $who, "The rules for voting on this product " .
+                                           "has changed;\nyou had too many votes " .
+                                           "for a single bug.");
+                foreach my $msg (@$msgs) {
+                    Bugzilla::BugMail::MessageToMTA($msg);
+                }
                 my $name = DBID_to_name($who);
 
                 push(@toomanyvotes_list,
@@ -945,10 +952,16 @@ if ($action eq 'update') {
                                undef, ($product->id, $who));
 
                 foreach my $bug_id (@$bug_ids) {
-                    RemoveVotes($bug_id, $who, "The rules for voting on this " .
-                                               "product has changed; you had " .
-                                               "too many\ntotal votes, so all " .
-                                               "votes have been removed.");
+                    # RemoveVotes() returns a list of messages to send
+                    # in case some voters had too many votes.
+                    my $msgs =
+                        RemoveVotes($bug_id, $who, "The rules for voting on this " .
+                                                   "product has changed; you had " .
+                                                   "too many\ntotal votes, so all " .
+                                                   "votes have been removed.");
+                    foreach my $msg (@$msgs) {
+                        Bugzilla::BugMail::MessageToMTA($msg);
+                    }
                     my $name = DBID_to_name($who);
 
                     push(@toomanytotalvotes_list,
diff --git a/process_bug.cgi b/process_bug.cgi
index a489594f4..b3006565f 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -1960,7 +1960,10 @@ foreach my $id (@idlist) {
     #
     my $origOwner = "";
     my $origQaContact = "";
-    
+
+    # $msgs will store emails which have to be sent to voters, if any.
+    my $msgs;
+
     foreach my $c (@::log_columns) {
         my $col = $c;           # We modify it, don't want to modify array
                                 # values in place.
@@ -2004,10 +2007,13 @@ foreach my $id (@idlist) {
             }
 
             if ($col eq 'product') {
-                RemoveVotes($id, 0,
-                            "This bug has been moved to a different product");
+                # If some votes have been removed, RemoveVotes() returns
+                # a list of messages to send to voters.
+                # We delay the sending of these messages till tables are unlocked.
+                $msgs = RemoveVotes($id, 0,
+                          "This bug has been moved to a different product");
             }
-            
+
             if ($col eq 'bug_status' 
                 && IsOpenedState($old) ne IsOpenedState($new))
             {
@@ -2026,6 +2032,11 @@ foreach my $id (@idlist) {
     }
     $dbh->bz_unlock_tables();
 
+    # Now is a good time to send email to voters.
+    foreach my $msg (@$msgs) {
+        Bugzilla::BugMail::MessageToMTA($msg);
+    }
+
     if ($duplicate) {
         # Check to see if Reporter of this bug is reporter of Dupe 
         SendSQL("SELECT reporter FROM bugs WHERE bug_id = " .