Bug 881348: Do not display the Excluded list when sending bugmails

4 files changed, 68 insertions, 103 deletions

diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index e35ca35a1..a2501a99c 100644
--- a/Bugzilla/BugMail.pm
+++ b/Bugzilla/BugMail.pm
@@ -152,6 +152,9 @@ sub Send {
         push @referenced_bugs, @{ $bug->blocked };
     }
 
+    # If no changes have been made, there is no need to process further.
+    return {'sent' => []} unless scalar(@diffs) || scalar(@$comments);
+
     ###########################################################################
     # Start of email filtering code
     ###########################################################################
@@ -245,7 +248,6 @@ sub Send {
     # the bug in question. However, we are not necessarily going to mail them
     # all - there are preferences, permissions checks and all sorts to do yet.
     my @sent;
-    my @excluded;
 
     # The email client will display the Date: header in the desired timezone,
     # so we can always use UTC here.
@@ -257,18 +259,13 @@ sub Send {
 
     foreach my $user_id (keys %recipients) {
         my %rels_which_want;
-        my $sent_mail = 0;
-        $user_cache{$user_id} ||= new Bugzilla::User($user_id);
-        my $user = $user_cache{$user_id};
+        my $user = $user_cache{$user_id} ||= new Bugzilla::User($user_id);
         # Deleted users must be excluded.
         next unless $user;
 
         # If email notifications are disabled for this account, or the bug
         # is ignored, there is no need to do additional checks.
-        if ($user->email_disabled || $user->is_bug_ignored($id)) {
-            push(@excluded, $user->login);
-            next;
-        }
+        next if ($user->email_disabled || $user->is_bug_ignored($id));
 
         if ($user->can_see_bug($id)) {
             # Go through each role the user has and see if they want mail in
@@ -303,10 +300,7 @@ sub Send {
             # BMO: never send emails to bugs or .tld addresses.  this check needs to
             # happen after the bugmail_recipients hook.
             if ($user->email_enabled && $dep_ok &&
-                ($user->login !~ /bugs$/) &&
-                # sync-1@bugzilla.tld here is a temporary hack, see bug 844724
-                ($user->login eq 'sync-1@bugzilla.tld' || $user->login !~ /\.tld$/))
-
+                ($user->login !~ /bugs$/) && ($user->login !~ /\.tld$/))
             {
                 # Don't show summaries for bugs the user can't access, and
                 # provide a hook for extensions such as SecureMail to filter
@@ -327,8 +321,8 @@ sub Send {
                                         { updated_bug     => $bug,
                                           referenced_bugs => $referenced_bugs });
 
-                $sent_mail = sendMail(
-                    { to       => $user, 
+                my $sent_mail = sendMail(
+                    { to       => $user,
                       bug      => $bug,
                       comments => $comments,
                       date     => $date,
@@ -339,15 +333,9 @@ sub Send {
                       rels_which_want => \%rels_which_want,
                       referenced_bugs => $referenced_bugs,
                     });
+                push(@sent, $user->login) if $sent_mail;
             }
         }
-
-        if ($sent_mail) {
-            push(@sent, $user->login); 
-        } 
-        else {
-            push(@excluded, $user->login); 
-        } 
     }
 
     # When sending bugmail about a blocker being reopened or resolved,
@@ -359,7 +347,7 @@ sub Send {
         $bug->{lastdiffed} = $end;
     }
 
-    return {'sent' => \@sent, 'excluded' => \@excluded};
+    return {'sent' => \@sent};
 }
 
 sub sendMail {
diff --git a/contrib/sendbugmail.pl b/contrib/sendbugmail.pl
index 51de9407d..d0c7d63b7 100755
--- a/contrib/sendbugmail.pl
+++ b/contrib/sendbugmail.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl -w
+#!/usr/bin/perl -wT
 #
 #                    sendbugmail.pl
 #
@@ -12,6 +12,8 @@
 # 
 # Usage: perl -T contrib/sendbugmail.pl bug_id user_email
 
+use 5.10.1;
+use strict;
 use lib qw(. lib);
 
 use Bugzilla;
@@ -22,7 +24,7 @@ use Bugzilla::User;
 my $dbh = Bugzilla->dbh;
 
 sub usage {
-    print STDERR "Usage: $0 bug_id user_email\n";
+    say STDERR "Usage: $0 bug_id user_email";
     exit;
 }
 
@@ -36,7 +38,7 @@ my $changer = $ARGV[1];
 
 # Validate the bug number.
 if (!($bugnum =~ /^(\d+)$/)) {
-  print STDERR "Bug number \"$bugnum\" not numeric.\n";
+  say STDERR "Bug number \"$bugnum\" not numeric.";
   usage();
 }
 
@@ -46,19 +48,19 @@ my ($id) = $dbh->selectrow_array("SELECT bug_id FROM bugs WHERE bug_id = ?",
                                  undef, $bugnum);
 
 if (!$id) {
-  print STDERR "Bug number $bugnum does not exist.\n";
+  say STDERR "Bug number $bugnum does not exist.";
   usage();
 }
 
 # Validate the changer address.
 my $match = Bugzilla->params->{'emailregexp'};
 if ($changer !~ /$match/) {
-    print STDERR "Changer \"$changer\" doesn't match email regular expression.\n";
+    say STDERR "Changer \"$changer\" doesn't match email regular expression.";
     usage();
 }
 my $changer_user = new Bugzilla::User({ name => $changer });
 unless ($changer_user) {
-    print STDERR "\"$changer\" is not a valid user.\n";
+    say STDERR "\"$changer\" is not a valid user.";
     usage();
 }
 
@@ -67,26 +69,15 @@ my $outputref = Bugzilla::BugMail::Send($bugnum, {'changer' => $changer_user });
 
 # Report the results.
 my $sent = scalar(@{$outputref->{sent}});
-my $excluded = scalar(@{$outputref->{excluded}});
 
 if ($sent) {
-    print "email sent to $sent recipients:\n";
+    say "email sent to $sent recipients:";
 } else {
-    print "No email sent.\n";
+    say "No email sent.";
 }
 
 foreach my $sent (@{$outputref->{sent}}) {
-  print "  $sent\n";
-}
-
-if ($excluded) {
-    print "$excluded recipients excluded:\n";
-} else {
-    print "No recipients excluded.\n";
-}
-
-foreach my $excluded (@{$outputref->{excluded}}) {
-  print "  $excluded\n";
+  say "  $sent";
 }
 
 # This document is copyright (C) 2004 Perforce Software, Inc.  All rights
diff --git a/contrib/sendunsentbugmail.pl b/contrib/sendunsentbugmail.pl
index 6ddbd2e4c..7771cfeff 100755
--- a/contrib/sendunsentbugmail.pl
+++ b/contrib/sendunsentbugmail.pl
@@ -21,8 +21,8 @@
 # Contributor(s): Dave Miller <justdave@bugzilla.org>
 #                 Myk Melez <myk@mozilla.org>
 
+use 5.10.1;
 use strict;
-
 use lib qw(. lib);
 
 use Bugzilla;
@@ -40,28 +40,21 @@ my $list = $dbh->selectcol_arrayref(
      ' ORDER BY bug_id');
 
 if (scalar(@$list) > 0) {
-    print "OK, now attempting to send unsent mail\n";
-    print scalar(@$list) . " bugs found with possibly unsent mail.\n\n";
+    say "OK, now attempting to send unsent mail";
+    say scalar(@$list) . " bugs found with possibly unsent mail.\n";
     foreach my $bugid (@$list) {
         my $start_time = time;
-        print "Sending mail for bug $bugid...\n";
+        say "Sending mail for bug $bugid...";
         my $outputref = Bugzilla::BugMail::Send($bugid);
         if ($ARGV[0] && $ARGV[0] eq "--report") {
-          print "Mail sent to:\n";
-          foreach (sort @{$outputref->{sent}}) {
-              print $_ . "\n";
-          }
-          
-          print "Excluded:\n";
-          foreach (sort @{$outputref->{excluded}}) {
-              print $_ . "\n";
-          }
+          say "Mail sent to:";
+          say $_ foreach (sort @{$outputref->{sent}});
         }
         else {
-            my ($sent, $excluded) = (scalar(@{$outputref->{sent}}),scalar(@{$outputref->{excluded}}));
-            print "$sent mails sent, $excluded people excluded.\n";
-            print "Took " . (time - $start_time) . " seconds.\n\n";
-        }    
+            my $sent = scalar @{$outputref->{sent}};
+            say "$sent mails sent.";
+            say "Took " . (time - $start_time) . " seconds.\n";
+        }
     }
-    print "Unsent mail has been sent.\n";
+    say "Unsent mail has been sent.";
 }
diff --git a/template/en/default/bug/process/bugmail.html.tmpl b/template/en/default/bug/process/bugmail.html.tmpl
index 50f6e7aa8..8053ba525 100644
--- a/template/en/default/bug/process/bugmail.html.tmpl
+++ b/template/en/default/bug/process/bugmail.html.tmpl
@@ -24,66 +24,59 @@
   # sent_bugmail: The results of Bugzilla::BugMail::Send().
   #%]
 
+[% USE CGI %]
 [% PROCESS global/variables.none.tmpl %]
 
 [%# hide the recipient list by default from new users %]
 [% show_recipients =
      user.settings.post_bug_submit_action.value == 'nothing'
-     || user.in_group('canconfirm')
+     || CGI.cookie('show_bugmail_recipients')
      || !user.can_see_bug(mailing_bugid)
 %]
+[% recipient_count = sent_bugmail.sent.size %]
 
-<dl id="bugmail_summary_[% mailing_bugid FILTER none %]"
-    [%~ ' class="bz_default_hidden"' UNLESS show_recipients %]>
-[% PROCESS emails
-   description = "Email sent to"
-   names = sent_bugmail.sent
-%]
-
-[% PROCESS emails
-   description = "Excluding"
-   names = sent_bugmail.excluded
-%]
-</dl>
-
-[% IF !show_recipients %]
-  [% recipient_count = sent_bugmail.sent.size %]
-  <div id="bugmail_summary_placeholder_[% mailing_bugid FILTER none %]"
-       [%~ ' class="bz_default_hidden"' IF show_recipients %]>
-    [% IF recipient_count > 0 %]
-      Email sent to [% recipient_count FILTER html %]
-      recipient[% 's' UNLESS recipient_count == 1 %].
-    [% ELSE %]
-      No emails were sent.
-    [% END %]
-    (<a href="#" onclick="
-        YAHOO.util.Dom.removeClass(
-          'bugmail_summary_[% mailing_bugid FILTER none %]',
-          'bz_default_hidden');
-        YAHOO.util.Dom.addClass(
-          'bugmail_summary_placeholder_[% mailing_bugid FILTER none %]',
-          'bz_default_hidden');
-        return false;">show</a>)
-  </div>
-[% END %]
+<script>
+function toggleBugmailRecipients(bug_id, show) {
+  if (show) {
+    YAHOO.util.Dom.removeClass('bugmail_summary_' + bug_id, 'bz_default_hidden');
+    YAHOO.util.Dom.addClass('bugmail_summary_' + bug_id + '_short', 'bz_default_hidden');
+  } else {
+    YAHOO.util.Dom.addClass('bugmail_summary_' + bug_id, 'bz_default_hidden');
+    YAHOO.util.Dom.removeClass('bugmail_summary_' + bug_id + '_short', 'bz_default_hidden');
+  }
+  YAHOO.util.Cookie.set('show_bugmail_recipients', (show ? 1 : 0), {
+    expires: new Date("Janurary 12, 2025")
+  });
+  return false;
+}
+</script>
 
-[%############################################################################%]
-[%# Block for a set of email addresses                                       #%]
-[%############################################################################%]
-
-[% BLOCK emails %]
-  <dt>[% description FILTER html %]:</dt>
+<dl id="bugmail_summary_[% mailing_bugid FILTER none %]"
+    class="[% show_recipients ? "" : "bz_default_hidden" %]">
+  <dt>Email sent to:</dt>
   <dd>
   [% IF user.can_see_bug(mailing_bugid) %]
-    [% IF names.size > 0 %]
-      [%+ FOREACH name = names %]
+    [% IF sent_bugmail.sent.size > 0 %]
+      [%+ FOREACH name = sent_bugmail.sent %]
         <code>[% name FILTER html %]</code>[% ", " UNLESS loop.last() %]
       [% END %]
     [% ELSE %]
       no one
     [% END %]
+    (<a href="#" onclick="return toggleBugmailRecipients([% mailing_bugid FILTER none %], false)">hide</a>)
   [% ELSE %]
     (list of e-mails not available)
   [% END %]
   </dd>
-[% END %]
+</dl>
+
+<div id="bugmail_summary_[% mailing_bugid FILTER none %]_short"
+     class="[% show_recipients ? "bz_default_hidden" : "" %]">
+  [% IF recipient_count > 0 %]
+    Email sent to [% recipient_count FILTER html %] recipient[% 's' UNLESS recipient_count == 1 %].
+    (<a href="#" onclick="return toggleBugmailRecipients([% mailing_bugid FILTER none %], true)">show</a>)
+  [% ELSE %]
+    No emails were sent.
+  [% END %]
+</div>
+