Bug 73665 - migrate email preferences to their own table, and rearchitect email…

Bugzilla/Constants.pm

@@ -51,9 +51,6 @@ use base qw(Exporter);
     LOGOUT_CURRENT
     LOGOUT_KEEP_CURRENT
 
-    DEFAULT_FLAG_EMAIL_SETTINGS
-    DEFAULT_EMAIL_SETTINGS
-
     GRANT_DIRECT
     GRANT_DERIVED
     GRANT_REGEXP
@@ -71,6 +68,20 @@ use base qw(Exporter);
     COMMENT_COLS
 
     UNLOCK_ABORT
+    
+    RELATIONSHIPS
+    REL_ASSIGNEE REL_QA REL_REPORTER REL_CC REL_VOTER 
+    REL_ANY
+    
+    POS_EVENTS
+    EVT_OTHER EVT_ADDED_REMOVED EVT_COMMENT EVT_ATTACHMENT EVT_ATTACHMENT_DATA
+    EVT_PROJ_MANAGEMENT EVT_OPENED_CLOSED EVT_KEYWORD EVT_CC 
+    
+    NEG_EVENTS
+    EVT_UNCONFIRMED EVT_CHANGED_BY_ME 
+        
+    GLOBAL_EVENTS
+    EVT_FLAG_REQUESTED EVT_REQUESTED_FLAG
 );
 
 @Bugzilla::Constants::EXPORT_OK = qw(contenttypes);
@@ -136,72 +147,6 @@ 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~" .
-      "~emailVoterAttachments~" .
-      "~emailVoterStatus~" .
-      "~emailVoterResolved~on" .
-      "~emailVoterKeywords~" .
-      "~emailVoterCC~" .
-      "~emailVoterOther~" .
-      "~emailVoterUnconfirmed~";
-
 use constant GRANT_DIRECT => 0;
 use constant GRANT_DERIVED => 1;
 use constant GRANT_REGEXP => 2;
@@ -230,4 +175,50 @@ use constant COMMENT_COLS => 80;
 # because of error
 use constant UNLOCK_ABORT => 1;
 
+use constant REL_ASSIGNEE           => 0;
+use constant REL_QA                 => 1;
+use constant REL_REPORTER           => 2;
+use constant REL_CC                 => 3;
+use constant REL_VOTER              => 4;
+
+use constant RELATIONSHIPS => REL_ASSIGNEE, REL_QA, REL_REPORTER, REL_CC, 
+                              REL_VOTER;
+                              
+# Used for global events like EVT_FLAG_REQUESTED
+use constant REL_ANY                => 100;
+
+# There are two sorts of event - positive and negative. Positive events are
+# those for which the user says "I want mail if this happens." Negative events
+# are those for which the user says "I don't want mail if this happens."
+#
+# Exactly when each event fires is defined in wants_bug_mail() in User.pm; I'm
+# not commenting them here in case the comments and the code get out of sync.
+use constant EVT_OTHER              => 0;
+use constant EVT_ADDED_REMOVED      => 1;
+use constant EVT_COMMENT            => 2;
+use constant EVT_ATTACHMENT         => 3;
+use constant EVT_ATTACHMENT_DATA    => 4;
+use constant EVT_PROJ_MANAGEMENT    => 5;
+use constant EVT_OPENED_CLOSED      => 6;
+use constant EVT_KEYWORD            => 7;
+use constant EVT_CC                 => 8;
+
+use constant POS_EVENTS => EVT_OTHER, EVT_ADDED_REMOVED, EVT_COMMENT, 
+                           EVT_ATTACHMENT, EVT_ATTACHMENT_DATA, 
+                           EVT_PROJ_MANAGEMENT, EVT_OPENED_CLOSED, EVT_KEYWORD,
+                           EVT_CC;
+
+use constant EVT_UNCONFIRMED        => 50;
+use constant EVT_CHANGED_BY_ME      => 51;
+
+use constant NEG_EVENTS => EVT_UNCONFIRMED, EVT_CHANGED_BY_ME;
+
+# These are the "global" flags, which aren't tied to a particular relationship.
+# and so use REL_ANY.
+use constant EVT_FLAG_REQUESTED     => 100; # Flag has been requested of me
+use constant EVT_REQUESTED_FLAG     => 101; # I have requested a flag
+
+use constant GLOBAL_EVENTS => EVT_FLAG_REQUESTED, EVT_REQUESTED_FLAG;
+
+
 1;

Bugzilla/DB/Schema.pm

@@ -584,7 +584,6 @@ use constant ABSTRACT_SCHEMA => {
             disabledtext   => {TYPE => 'MEDIUMTEXT', NOTNULL => 1},
             mybugslink     => {TYPE => 'BOOLEAN', NOTNULL => 1,
                                DEFAULT => 'TRUE'},
-            emailflags     => {TYPE => 'MEDIUMTEXT'},
             refreshed_when => {TYPE => 'DATETIME', NOTNULL => 1},
             extern_id      => {TYPE => 'varchar(64)'},
         ],
@@ -610,6 +609,20 @@ use constant ABSTRACT_SCHEMA => {
         ],
     },
 
+    email_setting => {
+        FIELDS => [
+            user_id      => {TYPE => 'INT3', NOTNULL => 1},
+            relationship => {TYPE => 'INT1', NOTNULL => 1},
+            event        => {TYPE => 'INT1', NOTNULL => 1},
+        ],
+        INDEXES => [
+            email_settings_user_id_idx => ['user_id'],
+            email_settings_unique_idx  =>
+                                    {FIELDS => [qw(user_id relationship event)],
+                                     TYPE => 'UNIQUE'},
+        ],
+    },
+
     watch => {
         FIELDS => [
             watcher => {TYPE => 'INT3', NOTNULL => 1},

Bugzilla/Flag.pm

@@ -72,6 +72,7 @@ use Bugzilla::Util;
 use Bugzilla::Error;
 use Bugzilla::Attachment;
 use Bugzilla::BugMail;
+use Bugzilla::Constants;
 
 use constant TABLES_ALREADY_LOCKED => 1;
 
@@ -482,7 +483,7 @@ sub create {
     
     # Send an email notifying the relevant parties about the flag creation.
     if (($flag->{'requestee'} 
-         && $flag->{'requestee'}->email_prefs->{'FlagRequestee'})
+         && $flag->{'requestee'}->wants_mail([EVT_FLAG_REQUESTED]))
          || $flag->{'type'}->{'cc_list'})
     {
         notify($flag, "request/email.txt.tmpl");
@@ -590,7 +591,7 @@ sub modify {
                         WHERE  id = $flag->{'id'}");
             
             # Send an email notifying the relevant parties about the fulfillment.
-            if ($flag->{'setter'}->email_prefs->{'FlagRequester'} 
+            if ($flag->{'setter'}->wants_mail([EVT_REQUESTED_FLAG]) 
                 || $flag->{'type'}->{'cc_list'})
             {
                 $flag->{'status'} = $status;
@@ -616,7 +617,7 @@ sub modify {
             
             # Send an email notifying the relevant parties about the request.
             if ($flag->{'requestee'} 
-                && ($flag->{'requestee'}->email_prefs->{'FlagRequestee'} 
+                && ($flag->{'requestee'}->wants_mail([EVT_FLAG_REQUESTED]) 
                     || $flag->{'type'}->{'cc_list'}))
             {
                 notify($flag, "request/email.txt.tmpl");

checksetup.pl

@@ -2485,7 +2485,8 @@ if (!($sth->fetchrow_arrayref()->[0])) {
 
 # 2000-12-18.  Added an 'emailflags' field for storing preferences about
 # when email gets sent on a per-user basis.
-if (!$dbh->bz_get_field_def('profiles', 'emailflags')) {
+if (!$dbh->bz_get_field_def('profiles', 'emailflags') && 
+    !$dbh->bz_get_field_def('email_setting', 'user_id')) {
     $dbh->bz_add_field('profiles', 'emailflags', 'mediumtext');
 }
 
@@ -3756,47 +3757,6 @@ if ($dbh->bz_get_field_def('bugs', 'short_desc')->[2]) { # if it allows nulls
     $dbh->bz_change_field_type('bugs', 'short_desc', 'mediumtext not null');
 }
 
-# 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
 $dbh->bz_add_field('products', 'classification_id', 'smallint DEFAULT 1');
@@ -3882,8 +3842,113 @@ if (!$dbh->bz_get_field_def('bugs', 'qa_contact')->[2]) { # if it's NOT NULL
                WHERE initialqacontact = 0");
 }
 
-} # END LEGACY CHECKS
+# 2005-03-29 - gerv@gerv.net - bug 73665.
+# Migrate email preferences to new email prefs table.
+if ($dbh->bz_get_field_def("profiles", "emailflags")) {    
+    print "Migrating email preferences to new table ...\n" unless $silent;
+    
+    # These are the "roles" and "reasons" from the original code, mapped to
+    # the new terminology of relationships and events.
+    my %relationships = ("Owner" => REL_ASSIGNEE, 
+                         "Reporter" => REL_REPORTER,
+                         "QAcontact" => REL_QA,
+                         "CClist" => REL_CC,
+                         "Voter" => REL_VOTER);
+                         
+    my %events = ("Removeme" => EVT_ADDED_REMOVED, 
+                  "Comments" => EVT_COMMENT, 
+                  "Attachments" => EVT_ATTACHMENT, 
+                  "Status" => EVT_PROJ_MANAGEMENT, 
+                  "Resolved" => EVT_OPENED_CLOSED,
+                  "Keywords" => EVT_KEYWORD, 
+                  "CC" => EVT_CC, 
+                  "Other" => EVT_OTHER,
+                  "Unconfirmed" => EVT_UNCONFIRMED);
+                                    
+    # Request preferences
+    my %requestprefs = ("FlagRequestee" => EVT_FLAG_REQUESTED,
+                        "FlagRequester" => EVT_REQUESTED_FLAG);
+
+    # Select all emailflags flag strings
+    my $sth = $dbh->prepare("SELECT userid, emailflags FROM profiles");
+    $sth->execute();
 
+    while (my ($userid, $flagstring) = $sth->fetchrow_array()) {
+        # If the user has never logged in since emailprefs arrived, and the
+        # temporary code to give them a default string never ran, then 
+        # $flagstring will be null. In this case, they just get all mail.
+        $flagstring ||= "";
+        
+        # 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.
+        my %emailflags = split(/~/, $flagstring, 255); # Appease my editor: /
+     
+        my $sth2 = $dbh->prepare("INSERT into email_setting " .
+                                 "(user_id, relationship, event) VALUES (" . 
+                                 "$userid, ?, ?)");
+        foreach my $relationship (keys %relationships) {
+            foreach my $event (keys %events) {
+                my $key = "email$relationship$event";
+                if (!exists($emailflags{$key}) || $emailflags{$key} eq 'on') {
+                    $sth2->execute($relationships{$relationship},
+                                   $events{$event});
+                }
+            }
+        }
+
+        # Note that in the old system, the value of "excludeself" is assumed to
+        # be off if the preference does not exist in the user's list, unlike 
+        # other preferences whose value is assumed to be on if they do not 
+        # exist.
+        #
+        # This preference has changed from global to per-relationship.
+        if (!exists($emailflags{'ExcludeSelf'}) 
+            || $emailflags{'ExcludeSelf'} ne 'on')
+        {
+            foreach my $relationship (keys %relationships) {
+                $dbh->do("INSERT into email_setting " .
+                         "(user_id, relationship, event) VALUES (" . 
+                         $userid . ", " .
+                         $relationships{$relationship}. ", " .
+                         EVT_CHANGED_BY_ME . ")");
+            }
+        }
+
+        foreach my $key (keys %requestprefs) {
+            if (!exists($emailflags{$key}) || $emailflags{$key} eq 'on') {
+              $dbh->do("INSERT into email_setting " . 
+                       "(user_id, relationship, event) VALUES (" . 
+                       $userid . ", " . REL_ANY . ", " . 
+                       $requestprefs{$key} . ")");            
+            }
+        }
+    }
+   
+    # EVT_ATTACHMENT_DATA should initially have identical settings to 
+    # EVT_ATTACHMENT. 
+    CloneEmailEvent(EVT_ATTACHMENT, EVT_ATTACHMENT_DATA); 
+       
+    $dbh->bz_drop_field("profiles", "emailflags");    
+}
+
+sub CloneEmailEvent {
+    my ($source, $target) = @_;
+
+    my $sth1 = $dbh->prepare("SELECT user_id, relationship FROM email_setting
+                              WHERE event = $source");
+    my $sth2 = $dbh->prepare("INSERT into email_setting " . 
+                             "(user_id, relationship, event) VALUES (" . 
+                             "?, ?, $target)");
+    
+    $sth1->execute();
+
+    while (my ($userid, $relationship) = $sth1->fetchrow_array()) {
+        $sth2->execute($userid, $relationship);            
+    }    
+} 
+
+} # END LEGACY CHECKS
 
 # If you had to change the --TABLE-- definition in any way, then add your
 # differential change code *** A B O V E *** this comment.
@@ -4156,10 +4221,10 @@ if ($sth->rows == 0) {
 
         $dbh->do(
           q{INSERT INTO profiles (login_name, realname, cryptpassword, 
-                                  emailflags, disabledtext, refreshed_when)
+                                  disabledtext, refreshed_when)
             VALUES (?, ?, ?, ?, ?, ?)},
             undef, $login, $realname, $cryptedpassword, 
-            Bugzilla::Constants::DEFAULT_EMAIL_SETTINGS, '', '1900-01-01 00:00:00');
+            '', '1900-01-01 00:00:00');
     }
 
     # Put the admin in each group if not already    

contrib/sendunsentbugmail.pl

@@ -47,9 +47,22 @@ if (scalar(@list) > 0) {
         my $start_time = time;
         print "Sending mail for bug $bugid...\n";
         my $outputref = Bugzilla::BugMail::Send($bugid);
-        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";
+        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";
+          }
+        }
+        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";
+        }    
     }
     print "Unsent mail has been sent.\n";
 }

post_bug.cgi

@@ -486,14 +486,7 @@ if (UserInGroup("editbugs")) {
 }
 
 # Email everyone the details of the new bug 
-$vars->{'mailrecipients'} = { 'cc' => \@cc,
-                              'owner' => DBID_to_name($::FORM{'assigned_to'}),
-                              'reporter' => Bugzilla->user->login,
-                              'changer' => Bugzilla->user->login };
-
-if (defined $::FORM{'qa_contact'}) {
-    $vars->{'mailrecipients'}->{'qacontact'} = DBID_to_name($::FORM{'qa_contact'});
-}
+$vars->{'mailrecipients'} = {'changer' => Bugzilla->user->login};
 
 $vars->{'id'} = $id;
 my $bug = new Bugzilla::Bug($id, $::userid);

template/en/default/filterexceptions.pl

@@ -645,9 +645,9 @@
 ],
 
 'account/prefs/email.html.tmpl' => [
-  'role', 
-  'reason.name', 
-  'reason.description',
+  'relationship.id',
+  'event.id',
+  'prefname',
 ],
 
 'account/prefs/permissions.html.tmpl' => [

template/en/default/request/email.txt.tmpl

@@ -28,11 +28,13 @@
 [% statuses = { '+' => "granted" , '-' => 'denied' , 'X' => "cancelled" ,
                 '?' => "asked" } %]
 [% IF flag.status == '?' %]
-   [% to_email = flag.requestee.email IF flag.requestee.email_prefs.FlagRequestee %]
+   [% to_email = flag.requestee.email 
+                   IF flag.requestee.wants_mail(constants.EVT_FLAG_REQUESTED) %]
    [% to_identity = flag.requestee.identity %]
    [% subject_status = "requested" %]
 [% ELSE %]
-   [% to_email = flag.setter.email IF flag.setter.email_prefs.FlagRequester %]
+   [% to_email = flag.setter.email 
+                      IF flag.setter.wants_mail(constants.EVT_REQUESTED_FLAG) %]
    [% to_identity = flag.setter.identity _ "'s request" %]
    [% subject_status = statuses.${flag.status} %]
 [% END %]

userprefs.cgi

@@ -37,10 +37,6 @@ require "CGI.pl";
 # Use global template variables.
 use vars qw($template $vars $userid);
 
-my @roles = ("Owner", "Reporter", "QAcontact", "CClist", "Voter");
-my @reasons = ("Removeme", "Comments", "Attachments", "Status", "Resolved", 
-               "Keywords", "CC", "Other", "Unconfirmed");
-
 ###############################################################################
 # 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 
@@ -175,7 +171,10 @@ sub SaveSettings {
 
 sub DoEmail {
     my $dbh = Bugzilla->dbh;
-
+    
+    ###########################################################################
+    # User watching
+    ###########################################################################
     if (Param("supportwatchers")) {
         my $watched_ref = $dbh->selectcol_arrayref(
             "SELECT profiles.login_name FROM watch, profiles"
@@ -197,81 +196,74 @@ sub DoEmail {
         $vars->{'watchers'} = \@watchers;
     }
 
-    SendSQL("SELECT emailflags FROM profiles WHERE userid = $userid");
-
-    my ($flagstring) = FetchSQLData();
+    ###########################################################################
+    # Role-based preferences
+    ###########################################################################
+    my $sth = Bugzilla->dbh->prepare("SELECT relationship, event " . 
+                                     "FROM email_setting " . 
+                                     "WHERE user_id = $userid");
+    $sth->execute();
+
+    my %mail;
+    while (my ($relationship, $event) = $sth->fetchrow_array()) {
+        $mail{$relationship}{$event} = 1;
+    }
 
-    # 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.
-    my %emailflags = split(/~/, $flagstring, 255);
+    $vars->{'mail'} = \%mail;      
+}
 
-    # Determine the value of the "excludeself" global email preference.
-    # Note that the value of "excludeself" is assumed to be off if the
-    # preference does not exist in the user's list, unlike other 
-    # preferences whose value is assumed to be on if they do not exist.
-    if (exists($emailflags{'ExcludeSelf'}) 
-        && $emailflags{'ExcludeSelf'} eq 'on')
-    {
-        $vars->{'excludeself'} = 1;
-    }
-    else {
-        $vars->{'excludeself'} = 0;
-    }
-    
-    foreach my $flag (qw(FlagRequestee FlagRequester)) {
-        $vars->{$flag} = 
-          !exists($emailflags{$flag}) || $emailflags{$flag} eq 'on';
-    }
+sub SaveEmail {
+    my $dbh = Bugzilla->dbh;
+    my $cgi = Bugzilla->cgi;
     
-    # Parse the info into a hash of hashes; the first hash keyed by role,
-    # the second by reason, and the value being 1 or 0 for (on or off).
-    # Preferences not existing in the user's list are assumed to be on.
-    foreach my $role (@roles) {
-        $vars->{$role} = {};
-        foreach my $reason (@reasons) {
-            my $key = "email$role$reason";
-            if (!exists($emailflags{$key}) || $emailflags{$key} eq 'on') {
-                $vars->{$role}{$reason} = 1;
+    ###########################################################################
+    # Role-based preferences
+    ###########################################################################
+    $dbh->bz_lock_tables("email_setting WRITE");
+
+    # Delete all the user's current preferences
+    $dbh->do("DELETE FROM email_setting WHERE user_id = $userid");
+
+    # Repopulate the table - first, with normal events in the 
+    # relationship/event matrix.
+    # Note: the database holds only "off" email preferences, as can be implied 
+    # from the name of the table - profiles_nomail.
+    foreach my $rel (RELATIONSHIPS) {
+        # Positive events: a ticked box means "send me mail."
+        foreach my $event (POS_EVENTS) {
+            if (1 == $cgi->param("email-$rel-$event")) {
+                $dbh->do("INSERT INTO email_setting " . 
+                         "(user_id, relationship, event) " . 
+                         "VALUES ($userid, $rel, $event)");
             }
-            else {
-                $vars->{$role}{$reason} = 0;
+        }
+        
+        # Negative events: a ticked box means "don't send me mail."
+        foreach my $event (NEG_EVENTS) {
+            if (!defined($cgi->param("neg-email-$rel-$event")) ||
+                $cgi->param("neg-email-$rel-$event") != 1) 
+            {
+                $dbh->do("INSERT INTO email_setting " . 
+                         "(user_id, relationship, event) " . 
+                         "VALUES ($userid, $rel, $event)");
             }
         }
     }
-}
-
-# Note: we no longer store "off" values in the database.
-sub SaveEmail {
-    my $updateString = "";
-    my $cgi = Bugzilla->cgi;
-    my $dbh = Bugzilla->dbh;
 
-    if (defined $cgi->param('ExcludeSelf')) {
-        $updateString .= 'ExcludeSelf~on';
-    } else {
-        $updateString .= 'ExcludeSelf~';
-    }
-    
-    foreach my $flag (qw(FlagRequestee FlagRequester)) {
-        $updateString .= "~$flag~" . (defined $cgi->param($flag) ? "on" : "");
-    }
-    
-    foreach my $role (@roles) {
-        foreach my $reason (@reasons) {
-            # Add this preference to the list without giving it a value,
-            # which is the equivalent of setting the value to "off."
-            $updateString .= "~email$role$reason~";
-            
-            # If the form field for this preference is defined, then we
-            # know the checkbox was checked, so set the value to "on".
-            $updateString .= "on" if defined $cgi->param("email$role$reason");
+    # Global positive events: a ticked box means "send me mail."
+    foreach my $event (GLOBAL_EVENTS) {
+        if (1 == $cgi->param("email-" . REL_ANY . "-$event")) {
+            $dbh->do("INSERT INTO email_setting " . 
+                     "(user_id, relationship, event) " . 
+                     "VALUES ($userid, " . REL_ANY . ", $event)");
         }
     }
-            
-    SendSQL("UPDATE profiles SET emailflags = " . SqlQuote($updateString) . 
-            " WHERE userid = $userid");
 
+    $dbh->bz_unlock_tables();
+
+    ###########################################################################
+    # User watching
+    ###########################################################################
     if (Param("supportwatchers") && defined $cgi->param('watchedusers')) {
         # Just in case.  Note that this much locking is actually overkill:
         # we don't really care if anyone reads the watch table.  So