Bug 100953: Move data/nomail into the DB and implement a UI to edit it

Patch by A. Karl Kornel <karl@kornel.name> r=wurblzap a=justdave

9 files changed, 108 insertions, 35 deletions

diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index 1b2fb5429..dd92cd3b4 100644
--- a/Bugzilla/BugMail.pm
+++ b/Bugzilla/BugMail.pm
@@ -57,20 +57,6 @@ use constant REL_NAMES => {
     REL_VOTER   , "Voter"
 };
 
-sub _read_nomail {
-    my $nomail = Bugzilla->request_cache->{bugmail_nomail};
-    return $nomail if $nomail;
-    if (open(NOMAIL, '<', bz_locations->{'datadir'} . "/nomail")) {
-        while (<NOMAIL>) {
-            $nomail->{trim($_)} = 1;
-        }
-        close(NOMAIL);
-    }
-    Bugzilla->request_cache->{bugmail_nomail} = $nomail;
-    return $nomail;
-}
-
-
 sub FormatTriple {
     my ($a, $b, $c) = (@_);
     $^A = "";
@@ -465,8 +451,7 @@ sub ProcessOneBug {
 
             # Make sure the user isn't in the nomail list, and the insider and 
             # dep checks passed.
-            my $nomail = _read_nomail();
-            if ((!$nomail->{$user->login}) &&
+            if ($user->email_enabled &&
                 $insider_ok &&
                 $dep_ok)
             {
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 5396a3d20..9f4670845 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -614,6 +614,8 @@ use constant ABSTRACT_SCHEMA => {
             cryptpassword  => {TYPE => 'varchar(128)'},
             realname       => {TYPE => 'varchar(255)'},
             disabledtext   => {TYPE => 'MEDIUMTEXT', NOTNULL => 1},
+            disable_mail   => {TYPE => 'BOOLEAN', NOTNULL => 1,
+                               DEFAULT => 'FALSE'},
             mybugslink     => {TYPE => 'BOOLEAN', NOTNULL => 1,
                                DEFAULT => 'TRUE'},
             refreshed_when => {TYPE => 'DATETIME', NOTNULL => 1},
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 48502326e..924cb0511 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -122,6 +122,7 @@ sub _create {
         'showmybugslink' => 0,
         'disabledtext'   => '',
         'flags'          => {},
+        'disable_mail'   => 0,
     };
     bless ($self, $class);
     return $self unless $cond && $val;
@@ -131,9 +132,9 @@ sub _create {
 
     my $dbh = Bugzilla->dbh;
 
-    my ($id, $login, $name, $disabledtext, $mybugslink) =
+    my ($id, $login, $name, $disabledtext, $mybugslink, $disable_mail) =
         $dbh->selectrow_array(qq{SELECT userid, login_name, realname,
-                                        disabledtext, mybugslink
+                                        disabledtext, mybugslink, disable_mail 
                                  FROM profiles WHERE $cond},
                                  undef, $val);
 
@@ -144,6 +145,7 @@ sub _create {
     $self->{'login'}          = $login;
     $self->{'disabledtext'}   = $disabledtext;
     $self->{'showmybugslink'} = $mybugslink;
+    $self->{'disable_mail'}   = $disable_mail;
 
     return $self;
 }
@@ -156,6 +158,8 @@ sub name { $_[0]->{name}; }
 sub disabledtext { $_[0]->{'disabledtext'}; }
 sub is_disabled { $_[0]->disabledtext ? 1 : 0; }
 sub showmybugslink { $_[0]->{showmybugslink}; }
+sub email_disabled { $_[0]->{disable_mail}; }
+sub email_enabled { !($_[0]->{disable_mail}); }
 
 sub set_authorizer {
     my ($self, $authorizer) = @_;
@@ -1339,10 +1343,11 @@ sub get_userlist {
 }
 
 sub insert_new_user {
-    my ($username, $realname, $password, $disabledtext) = (@_);
+    my ($username, $realname, $password, $disabledtext, $disable_mail) = (@_);
     my $dbh = Bugzilla->dbh;
 
     $disabledtext ||= '';
+    $disable_mail ||= 0;
 
     # If not specified, generate a new random password for the user.
     # If the password is '*', do not encrypt it; we are creating a user
@@ -1358,10 +1363,11 @@ sub insert_new_user {
     # Insert the new user record into the database.
     $dbh->do("INSERT INTO profiles 
                           (login_name, realname, cryptpassword, disabledtext,
-                           refreshed_when) 
-                   VALUES (?, ?, ?, ?, '1901-01-01 00:00:00')",
+                           refreshed_when, disable_mail) 
+                   VALUES (?, ?, ?, ?, '1901-01-01 00:00:00', ?)",
              undef, 
-             ($username, $realname, $cryptpassword, $disabledtext));
+             ($username, $realname, $cryptpassword, $disabledtext, 
+              $disable_mail));
 
     # Turn on all email for the new user
     my $new_userid = $dbh->bz_last_key('profiles', 'userid');
@@ -1868,6 +1874,11 @@ Params: $username (scalar, string) - The login name for the new user.
                                          If given, the user will be disabled,
                                          meaning the account will be
                                          unavailable for login.
+        $disable_mail (scalar, boolean) - Optional, defaults to 0.
+                                          If 1, bug-related mail will not be 
+                                          sent to this user; if 0, mail will
+                                          be sent depending on the user's 
+                                          email preferences.
 
 Returns: The password for this user, in plain text, so it can be included
          in an e-mail sent to the user.
diff --git a/checksetup.pl b/checksetup.pl
index 8f1fdd167..485173fb2 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -3358,6 +3358,54 @@ if (!$dbh->bz_column_info('classifications', 'sortkey')) {
 $dbh->bz_add_column('fielddefs', 'enter_bug',
     {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'});
 
+# 2006-07-14 karl@kornel.name - Bug 100953
+# If a nomail file exists, move its contents into the DB
+$dbh->bz_add_column('profiles', 'disable_mail',
+                    { TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE' });
+if (-e "$datadir/nomail") {
+    # We have a data/nomail file, read it in and delete it
+    my %nomail;
+    print "Found a data/nomail file.  Moving nomail entries into DB...\n";
+    open NOMAIL, '<', "$datadir/nomail";
+    while (<NOMAIL>) {
+        $nomail{trim($_)} = 1;
+    }
+    close NOMAIL;
+
+    # Go through each entry read.  If a user exists, set disable_mail.
+    my $query = $dbh->prepare('UPDATE profiles
+                                  SET disable_mail = 1
+                                WHERE userid = ?');
+    foreach my $user_to_check (keys %nomail) {
+        my $uid;
+        if ($uid = Bugzilla::User::login_to_id($user_to_check)) {
+            my $user = new Bugzilla::User($uid);
+            print "\tDisabling email for user ", $user->email, "\n";
+            $query->execute($user->id);
+            delete $nomail{$user->email};
+        }
+    }
+
+    # If there are any nomail entries remaining, move them to nomail.bad
+    # and say something to the user.
+    if (scalar(keys %nomail)) {
+        print 'The following users were listed in data/nomail, but do not ' .
+              'have an account here.  The unmatched entries have been moved ' .
+              "to $datadir/nomail.bad\n";
+        open NOMAIL_BAD, '>>', "$datadir/nomail.bad";
+        foreach my $unknown_user (keys %nomail) {
+            print "\t$unknown_user\n";
+            print NOMAIL_BAD "$unknown_user\n";
+            delete $nomail{$unknown_user};
+        }
+        close NOMAIL_BAD;
+    }
+
+    # Now that we don't need it, get rid of the nomail file.
+    unlink "$datadir/nomail";
+}
+
+
 # If you had to change the --TABLE-- definition in any way, then add your
 # differential change code *** A B O V E *** this comment.
 #
diff --git a/editusers.cgi b/editusers.cgi
index 4d7292391..1809101d6 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -195,6 +195,7 @@ if ($action eq 'search') {
     my $password     = $cgi->param('password');
     my $realname     = trim($cgi->param('name')         || '');
     my $disabledtext = trim($cgi->param('disabledtext') || '');
+    my $disable_mail = $cgi->param('disable_mail') =~ /^(0|1)$/ ? $1 : 0;
 
     # Lock tables during the check+creation session.
     $dbh->bz_lock_tables('profiles WRITE', 'profiles_activity WRITE',
@@ -216,7 +217,7 @@ if ($action eq 'search') {
     trick_taint($password);
     trick_taint($disabledtext);
 
-    insert_new_user($login, $realname, $password, $disabledtext);
+    insert_new_user($login, $realname, $password, $disabledtext, $disable_mail);
     my $new_user_id = $dbh->bz_last_key('profiles', 'userid');
     $dbh->bz_unlock_tables();
     userDataToVars($new_user_id);
@@ -234,6 +235,7 @@ if ($action eq 'search') {
 } elsif ($action eq 'update') {
     my $otherUser = check_user($otherUserID, $otherUserLogin);
     $otherUserID = $otherUser->id;
+    my $oldprofile = new Bugzilla::User($otherUserID);
 
     my $logoutNeeded = 0;
     my @changedFields;
@@ -255,14 +257,17 @@ if ($action eq 'search') {
                                            object => "user"});
 
     # Cleanups
-    my $loginold        = $cgi->param('loginold')        || '';
-    my $realnameold     = $cgi->param('nameold')         || '';
-    my $disabledtextold = $cgi->param('disabledtextold') || '';
+    my $loginold         = $cgi->param('loginold')         || '';
+    my $realnameold      = $cgi->param('nameold')          || '';
+    my $disabledtextold  = $cgi->param('disabledtextold')  || '';
+    my $disable_mail_old = $cgi->param('disable_mail_old') =~ /^(0|1)$/ ? 
+                           $1 : $oldprofile->email_disabled;
 
     my $login        = $cgi->param('login');
     my $password     = $cgi->param('password');
     my $realname     = trim($cgi->param('name')         || '');
     my $disabledtext = trim($cgi->param('disabledtext') || '');
+    my $disable_mail = $cgi->param('disable_mail') =~ /^(0|1)$/ ? $1 : 0;
 
     # Update profiles table entry; silently skip doing this if the user
     # is not authorized.
@@ -308,6 +313,11 @@ if ($action eq 'search') {
             push(@values, $disabledtext);
             $logoutNeeded = 1;
         }
+        if ($disable_mail != $disable_mail_old) {
+            trick_taint($disable_mail);
+            push(@changedFields, 'disable_mail');
+            push(@values, $disable_mail);
+        }
         if (@changedFields) {
             push (@values, $otherUserID);
             $logoutNeeded && Bugzilla->logout_user($otherUser);
diff --git a/template/en/default/admin/users/userdata.html.tmpl b/template/en/default/admin/users/userdata.html.tmpl
index afb402554..be29a1b4b 100644
--- a/template/en/default/admin/users/userdata.html.tmpl
+++ b/template/en/default/admin/users/userdata.html.tmpl
@@ -70,6 +70,24 @@
     </td>
   </tr>
   <tr>
+    <th><label for="disable_mail">Bugmail Disabled:</label></th>
+    <td>
+      <input type="checkbox" name="disable_mail" id="disable_mail" value="1" 
+      [% IF otheruser.email_disabled %] checked="checked" [% END %] />
+      (This affects bugmail and whinemail, not password-reset or other 
+      non-bug-related emails)
+      [% IF editform %]
+        <input type="hidden" name="disable_mail_old"
+          [% IF otheruser.email_disabled %]
+            value="1"
+          [% ELSE %]
+            value="0" 
+          [% END %]
+        />
+      [% END %]
+    </td>
+  </tr>
+  <tr>
     <th><label for="disabledtext">Disable text:</label></th>
     <td>
       [% INCLUDE global/textarea.html.tmpl
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index 45b584bea..08321ed2c 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -57,6 +57,12 @@
               A new password has been set.
             [% ELSIF field == 'disabledtext' %]
               The disable text has been modified.
+            [% ELSIF field == 'disable_mail' %]
+              [% IF otheruser.email_disabled %]
+                Bugmail has been disabled.
+              [% ELSE %]
+                Bugmail has been enabled.
+              [% END %]
             [% END %]
           </li>
         [% END %]
diff --git a/whine.pl b/whine.pl
index 8e60370a3..12b03fb62 100755
--- a/whine.pl
+++ b/whine.pl
@@ -102,14 +102,6 @@ if ($fromaddress !~ Bugzilla->params->{'emailregexp'}) {
         "The maintainer email address has not been properly set!\n";
 }
 
-# Check the nomail file for users who should not receive mail
-my %nomail;
-if (open(NOMAIL, '<', bz_locations()->{'datadir'} . "/nomail")) {
-    while (<NOMAIL>) {
-        $nomail{trim($_)} = 1;
-    }
-}
-
 # get the current date and time
 my ($now_sec, $now_minute, $now_hour, $now_day, $now_month, $now_year, 
     $now_weekday) = localtime;
@@ -373,7 +365,7 @@ sub mail {
     my $args = shift;
 
     # Don't send mail to someone on the nomail list.
-    return if $nomail{$args->{'recipient'}->{'login'}};
+    return if $args->{recipient}->email_disabled;
 
     my $msg = ''; # it's a temporary variable to hold the template output
     $args->{'alternatives'} ||= [];
diff --git a/whineatnews.pl b/whineatnews.pl
index 580990769..42df7be55 100755
--- a/whineatnews.pl
+++ b/whineatnews.pl
@@ -43,6 +43,7 @@ my $query = q{SELECT bug_id, short_desc, login_name
           INNER JOIN profiles
                   ON userid = assigned_to
                WHERE (bug_status = ? OR bug_status = ?)
+                 AND disable_mail = 0
                  AND } . $dbh->sql_to_days('NOW()') . " - " .
                        $dbh->sql_to_days('delta_ts') . " > " .
                        Bugzilla->params->{'whinedays'} .