Bug 108870: Bugzilla does not set email prefs for new user until user visits userprefs.cgi

Bugzilla/BugMail.pm

@@ -615,15 +615,6 @@ sub filterEmailGroup ($$$) {
         SendSQL("SELECT emailflags FROM profiles WHERE userid = $userid");
         my $prefs = FetchOneColumn();
         
-        # If the user's preferences are empty, it means the user has not set
-        # their mail preferences after the installation upgraded from a
-        # version of Bugzilla without email preferences to one with them. In
-        # this case, assume they want to receive all mail.
-        if (!defined($prefs) || $prefs !~ /email/) {
-            push(@recipients, $user);
-            next;
-        }
-        
         # Write the user's preferences into a Perl record indexed by 
         # preference name.  We pass the value "255" to the split function 
         # because otherwise split will trim trailing null fields, causing 
@@ -665,10 +656,7 @@ sub filterEmailGroup ($$$) {
         }
         
         # If the user prefers to be included in mail about this change,
-        # or they haven't specified a preference for it (because they
-        # haven't visited the email preferences page since the preference
-        # was added, in which case we include them by default), add them
-        # to the list of recipients.
+        # add them to the list of recipients.
         foreach my $reason (@$reasons) {
             my $pref = "email$role$reason";
             if (!exists($prefs{$pref}) || $prefs{$pref} eq 'on') {

Bugzilla/Constants.pm

@@ -25,7 +25,7 @@
 #                 J. Paul Reed <preed@sigkill.com>
 #                 Bradley Baetz <bbaetz@student.usyd.edu.au>
 #                 Christopher Aillon <christopher@aillon.com>
-
+#                 Shane H. W. Travis <travis@sedsystems.ca>
 
 package Bugzilla::Constants;
 use strict;
@@ -51,6 +51,9 @@ use base qw(Exporter);
     LOGOUT_CURRENT
     LOGOUT_KEEP_CURRENT
 
+    DEFAULT_FLAG_EMAIL_SETTINGS
+    DEFAULT_EMAIL_SETTINGS
+
     GRANT_DIRECT
     GRANT_DERIVED
     GRANT_REGEXP
@@ -122,6 +125,72 @@ use constant contenttypes =>
    "ics" => "text/calendar" ,
   };
 
+use constant DEFAULT_FLAG_EMAIL_SETTINGS =>
+      "~FlagRequestee~on" .
+      "~FlagRequester~on";
+
+# By default, almost all bugmail is turned on, with the exception
+# of CC list additions for anyone except the Assignee/Owner.
+# If you want to customize the default settings for new users at
+# your own site, ensure that each of the lines ends with either
+# "~on" or just "~" (for off).
+
+use constant DEFAULT_EMAIL_SETTINGS => 
+      "ExcludeSelf~on" .
+
+      "~FlagRequestee~on" .
+      "~FlagRequester~on" .
+
+      "~emailOwnerRemoveme~on" .
+      "~emailOwnerComments~on" .
+      "~emailOwnerAttachments~on" .
+      "~emailOwnerStatus~on" .
+      "~emailOwnerResolved~on" .
+      "~emailOwnerKeywords~on" .
+      "~emailOwnerCC~on" .
+      "~emailOwnerOther~on" .
+      "~emailOwnerUnconfirmed~on" .
+  
+      "~emailReporterRemoveme~on" .
+      "~emailReporterComments~on" .
+      "~emailReporterAttachments~on" .
+      "~emailReporterStatus~on" .
+      "~emailReporterResolved~on" .
+      "~emailReporterKeywords~on" .
+      "~emailReporterCC~" .
+      "~emailReporterOther~on" .
+      "~emailReporterUnconfirmed~on" .
+  
+      "~emailQAcontactRemoveme~on" .
+      "~emailQAcontactComments~on" .
+      "~emailQAcontactAttachments~on" .
+      "~emailQAcontactStatus~on" .
+      "~emailQAcontactResolved~on" .
+      "~emailQAcontactKeywords~on" .
+      "~emailQAcontactCC~" .
+      "~emailQAcontactOther~on" .
+      "~emailQAcontactUnconfirmed~on" .
+  
+      "~emailCClistRemoveme~on" .
+      "~emailCClistComments~on" .
+      "~emailCClistAttachments~on" .
+      "~emailCClistStatus~on" .
+      "~emailCClistResolved~on" .
+      "~emailCClistKeywords~on" .
+      "~emailCClistCC~" .
+      "~emailCClistOther~on" .
+      "~emailCClistUnconfirmed~on" .
+  
+      "~emailVoterRemoveme~on" .
+      "~emailVoterComments~on" .
+      "~emailVoterAttachments~on" .
+      "~emailVoterStatus~on" .
+      "~emailVoterResolved~on" .
+      "~emailVoterKeywords~on" .
+      "~emailVoterCC~" .
+      "~emailVoterOther~on" .
+      "~emailVoterUnconfirmed~on";
+
 use constant GRANT_DIRECT => 0;
 use constant GRANT_DERIVED => 1;
 use constant GRANT_REGEXP => 2;

Bugzilla/User.pm

@@ -858,20 +858,6 @@ sub email_prefs {
     my @reasons = qw(Removeme Comments Attachments Status Resolved Keywords 
                      CC Other Unconfirmed);
 
-    # If the prefs are empty, this user hasn't visited the email pane
-    # of userprefs.cgi since before the change to use the "emailflags" 
-    # column, so initialize that field with the default prefs.
-    if (!$flags) {
-        # Create a default prefs string that causes the user to get all email.
-        $flags = "ExcludeSelf~on~FlagRequestee~on~FlagRequester~on~";
-        foreach my $role (@roles) {
-            foreach my $reason (@reasons) {
-                $flags .= "email$role$reason~on~";
-            }
-        }
-        chop $flags;
-    }
-
     # Convert the prefs from the flags string from the database into
     # a Perl record.  The 255 param is here because split will trim 
     # any trailing null fields without a third param, which causes Perl 

checksetup.pl

@@ -26,6 +26,7 @@
 #                 Jacob Steenhagen <jake@bugzilla.org>
 #                 Bradley Baetz <bbaetz@student.usyd.edu.au>
 #                 Tobias Burnus <burnus@net-b.de>
+#                 Shane H. W. Travis <travis@sedsystems.ca>
 #                 Gervase Markham <gerv@gerv.net>
 #                 Erik Stambaugh <erik@dasbistro.com>
 #
@@ -4457,8 +4458,11 @@ if ($sth->rows == 0) {
     $realname = $dbh->quote($realname);
     $cryptedpassword = $dbh->quote($cryptedpassword);
 
-    $dbh->do("INSERT INTO profiles (login_name, realname, cryptpassword)" .
-            " VALUES ($login, $realname, $cryptedpassword)");
+    # Set default email flags for the Admin, same as for users
+    my $defaultflagstring = $dbh->quote(Bugzilla::Constants::DEFAULT_EMAIL_SETTINGS);
+
+    $dbh->do("INSERT INTO profiles (login_name, realname, cryptpassword, emailflags) " .
+             "VALUES ($login, $realname, $cryptedpassword, $defaultflagstring)");
   }
     # Put the admin in each group if not already    
     my $query = "select userid from profiles where login_name = $login";    
@@ -4567,6 +4571,47 @@ if (GetFieldDef('bugs', 'short_desc')->[2]) { # if it allows nulls
 # Make sure groups get rederived
 $dbh->do("UPDATE groups SET last_changed = NOW() WHERE name = 'admin'");
 
+# 2004-12-29 - Flag email code is broke somewhere, and doesn't treat a lack
+# of FlagRequestee/er emailflags as 'on' like it's supposed to. Easiest way
+# to fix this is to make sure that everyone has these set. (bug 275599).
+# While we're at it, let's make sure everyone has some emailprefs set,
+# whether or not they've ever visited userprefs.cgi (bug 108870). In fact,
+# do this first so that the second check gets fewer hits.
+# 
+my $emailflags_count = 0;
+$sth = $dbh->prepare("SELECT userid FROM profiles " .
+                     "WHERE emailflags LIKE '' " .
+                     "OR emailflags IS NULL");
+$sth->execute();
+while (my ($userid) = $sth->fetchrow_array()) {
+    $dbh->do("UPDATE profiles SET emailflags = " .
+             $dbh->quote(Bugzilla::Constants::DEFAULT_EMAIL_SETTINGS) .
+             "WHERE userid = $userid");
+    $emailflags_count++;
+}
+
+if ($emailflags_count) {
+  print "Added default email prefs to $emailflags_count users who had none.\n" unless $silent;
+  $emailflags_count = 0;
+}
+
+
+$sth = $dbh->prepare("SELECT userid, emailflags FROM profiles " .
+                     "WHERE emailflags NOT LIKE '%Flagrequeste%' ");
+$sth->execute();
+while (my ($userid, $emailflags) = $sth->fetchrow_array()) {
+    $emailflags .= Bugzilla::Constants::DEFAULT_FLAG_EMAIL_SETTINGS;
+    $emailflags = $dbh->quote($emailflags);
+    $dbh->do("UPDATE profiles SET emailflags = $emailflags " .
+             "WHERE userid = $userid");
+    $emailflags_count++;
+}
+
+if ($emailflags_count) {
+  print "Added default Flagrequester/ee email prefs to $emailflags_count users who had none.\n" unless $silent;
+  $emailflags_count = 0;
+}
+
 
 # 2003-10-24 - alt@sonic.net, bug 224208
 # Support classification level and make sure there is a default classification

editusers.cgi

@@ -460,11 +460,12 @@ if ($action eq 'new') {
     # Add the new user
     SendSQL("INSERT INTO profiles ( " .
             "login_name, cryptpassword, realname,  " .
-            "disabledtext" .
+            "emailflags, disabledtext" .
             " ) VALUES ( " .
             SqlQuote($user) . "," .
             SqlQuote(Crypt($password)) . "," .
             SqlQuote($realname) . "," .
+            SqlQuote(Bugzilla::Constants::DEFAULT_EMAIL_SETTINGS) . "," .
             SqlQuote($disabledtext) . ")" );
 
     #+++ send e-mail away

globals.pl

@@ -435,13 +435,15 @@ sub InsertNewUser {
     my $cryptpassword = Crypt($password);
 
 
+    my $defaultflagstring = SqlQuote(Bugzilla::Constants::DEFAULT_EMAIL_SETTINGS); 
+
     # Insert the new user record into the database.            
     $username = SqlQuote($username);
     $realname = SqlQuote($realname);
     $cryptpassword = SqlQuote($cryptpassword);
     PushGlobalSQLState();
-    SendSQL("INSERT INTO profiles (login_name, realname, cryptpassword) 
-             VALUES ($username, $realname, $cryptpassword)");
+    SendSQL("INSERT INTO profiles (login_name, realname, cryptpassword, emailflags) 
+             VALUES ($username, $realname, $cryptpassword, $defaultflagstring)");
     PopGlobalSQLState();
 
     # Return the password to the calling code so it can be included 

userprefs.cgi

@@ -36,22 +36,10 @@ use Bugzilla::RelationSet;
 # Use global template variables.
 use vars qw($template $vars $userid);
 
-# The default email flags leave all email on.
-my $defaultflagstring = "ExcludeSelf~on~";
-
 my @roles = ("Owner", "Reporter", "QAcontact", "CClist", "Voter");
 my @reasons = ("Removeme", "Comments", "Attachments", "Status", "Resolved", 
                "Keywords", "CC", "Other", "Unconfirmed");
 
-foreach my $role (@roles) {
-    foreach my $reason (@reasons) {
-        $defaultflagstring .= "email$role$reason~on~";
-    }
-}
-
-# Remove final "~".
-chop $defaultflagstring;
-
 ###############################################################################
 # Each panel has two functions - panel Foo has a DoFoo, to get the data 
 # necessary for displaying the panel, and a SaveFoo, to save the panel's 
@@ -161,16 +149,6 @@ sub DoEmail {
 
     my ($flagstring) = FetchSQLData();
 
-    # If the emailflags haven't been set before, that means that this user 
-    # hasn't been to the email pane of userprefs.cgi since the change to 
-    # use emailflags. Create a default flagset for them, based on
-    # static defaults. 
-    if (!$flagstring) {
-        $flagstring = $defaultflagstring;
-        SendSQL("UPDATE profiles SET emailflags = " .
-                SqlQuote($flagstring) . " WHERE userid = $userid");
-    }
-
     # The 255 param is here, because without a third param, split will
     # trim any trailing null fields, which causes Perl to eject lots of
     # warnings. Any suitably large number would do.