Bug 561745: Impossible to uncheck boxes in the Email preferences

r/a=mkanat

3 files changed, 58 insertions, 73 deletions

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index d02dc947b..936cf36e4 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -1471,18 +1471,25 @@ sub wants_mail {
     # Skip DB query if relationship is explicit
     return 1 if $relationship == REL_GLOBAL_WATCHER;
 
+    my $wants_mail = grep { $self->mail_settings->{$relationship}{$_} } @$events;
+    return $wants_mail ? 1 : 0;
+}
+
+sub mail_settings {
+    my $self = shift;
     my $dbh = Bugzilla->dbh;
 
-    my $wants_mail = 
-        $dbh->selectrow_array('SELECT 1
-                                 FROM email_setting
-                                WHERE user_id = ?
-                                  AND relationship = ?
-                                  AND event IN (' . join(',', @$events) . ') ' .
-                                      $dbh->sql_limit(1),
-                              undef, ($self->id, $relationship));
+    if (!defined $self->{'mail_settings'}) {
+        my $data =
+          $dbh->selectall_arrayref('SELECT relationship, event FROM email_setting
+                                    WHERE user_id = ?', undef, $self->id);
+        my %mail;
+        # The hash is of the form $mail{$relationship}{$event} = 1.
+        $mail{$_->[0]}{$_->[1]} = 1 foreach @$data;
 
-    return defined($wants_mail) ? 1 : 0;
+        $self->{'mail_settings'} = \%mail;
+    }
+    return $self->{'mail_settings'};
 }
 
 sub is_mover {
diff --git a/template/en/default/account/prefs/email.html.tmpl b/template/en/default/account/prefs/email.html.tmpl
index 95acabdf4..291cd5dc3 100644
--- a/template/en/default/account/prefs/email.html.tmpl
+++ b/template/en/default/account/prefs/email.html.tmpl
@@ -77,8 +77,8 @@ document.write('<input type="button" value="Disable All Mail" onclick="SetCheckb
       [% prefname = "email-$constants.REL_ANY-$constants.EVT_FLAG_REQUESTED" %]
       <input type="checkbox" name="[% prefname %]" id="[% prefname %]" 
         value="1"
-        [% " checked" IF 
-                  mail.${constants.REL_ANY}.${constants.EVT_FLAG_REQUESTED} %]>
+        [% " checked"
+           IF user.mail_settings.${constants.REL_ANY}.${constants.EVT_FLAG_REQUESTED} %]>
       <label for="[% prefname %]">Email me when someone asks me to set a flag</label>
       <br>
     </td>
@@ -89,8 +89,8 @@ document.write('<input type="button" value="Disable All Mail" onclick="SetCheckb
       [% prefname = "email-$constants.REL_ANY-$constants.EVT_REQUESTED_FLAG" %]
       <input type="checkbox" name="[% prefname %]" id="[% prefname %]" 
         value="1"
-        [% " checked" IF 
-                  mail.${constants.REL_ANY}.${constants.EVT_REQUESTED_FLAG} %]>
+        [% " checked"
+           IF user.mail_settings.${constants.REL_ANY}.${constants.EVT_REQUESTED_FLAG} %]>
       <label for="[% prefname %]">Email me when someone sets a flag I asked for</label>
       <br>
     </td>
@@ -197,7 +197,7 @@ document.write('<input type="button" value="Disable All Mail" onclick="SetCheckb
                   no_added_removed.contains(relationship.id) 
             %]
                disabled
-            [% ELSIF mail.${relationship.id}.${event.id} %]
+            [% ELSIF user.mail_settings.${relationship.id}.${event.id} %]
                checked
             [% END %]>
         </td>
@@ -226,7 +226,7 @@ document.write('<input type="button" value="Disable All Mail" onclick="SetCheckb
           <input type="checkbox" 
             name="neg-email-[% relationship.id %]-[% event.id %]"
             value="1"
-            [% " checked" IF NOT mail.${relationship.id}.${event.id} %]>
+            [% " checked" IF NOT user.mail_settings.${relationship.id}.${event.id} %]>
         </td>
       [% END %]
       <td>
@@ -237,23 +237,6 @@ document.write('<input type="button" value="Disable All Mail" onclick="SetCheckb
   
 </table>
 
-[%# Add hidden form fields for fields not used %]
-[% FOREACH event = events %]  
-  [% FOREACH relationship = relationships %]
-    <input type="hidden" 
-           name="email-[% relationship.id %]-[% event.id %]"
-           value="[% mail.${relationship.id}.${event.id} ? "1" : "0" %]">
-  [% END %]
-[% END %]
-
-[% FOREACH event = neg_events %]  
-  [% FOREACH relationship = relationships %]
-    <input type="hidden" 
-           name="neg-email-[% relationship.id %]-[% event.id %]"
-          value="[% mail.${relationship.id}.${event.id} ? "0" : "1" %]">
-  [% END %]
-[% END %]
-
 <hr>
 <b>User Watching</b>
 
diff --git a/userprefs.cgi b/userprefs.cgi
index f95ddb3e3..d15bcd13a 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -219,21 +219,6 @@ sub DoEmail {
 
     @watchers = sort { lc($a) cmp lc($b) } @watchers;
     $vars->{'watchers'} = \@watchers;
-
-    ###########################################################################
-    # Role-based preferences
-    ###########################################################################
-    my $sth = $dbh->prepare("SELECT relationship, event " . 
-                            "FROM email_setting " . 
-                            "WHERE user_id = ?");
-    $sth->execute($user->id);
-
-    my %mail;
-    while (my ($relationship, $event) = $sth->fetchrow_array()) {
-        $mail{$relationship}{$event} = 1;
-    }
-
-    $vars->{'mail'} = \%mail;      
 }
 
 sub SaveEmail {
@@ -248,54 +233,64 @@ sub SaveEmail {
     ###########################################################################
     $dbh->bz_start_transaction();
 
-    # Delete all the user's current preferences
-    $dbh->do("DELETE FROM email_setting WHERE user_id = ?", undef, $user->id);
+    my $sth_insert = $dbh->prepare('INSERT INTO email_setting
+                                    (user_id, relationship, event) VALUES (?, ?, ?)');
 
-    # Repopulate the table - first, with normal events in the 
+    my $sth_delete = $dbh->prepare('DELETE FROM email_setting
+                                    WHERE user_id = ? AND relationship = ? AND event = ?');
+    # Load current email preferences into memory before updating them.
+    my $settings = $user->mail_settings;
+
+    # Update 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.
     my %relationships = Bugzilla::BugMail::relationships();
     foreach my $rel (keys %relationships) {
+        next if ($rel == REL_QA && !Bugzilla->params->{'useqacontact'});
         # Positive events: a ticked box means "send me mail."
         foreach my $event (POS_EVENTS) {
-            if (defined($cgi->param("email-$rel-$event"))
-                && $cgi->param("email-$rel-$event") == 1)
-            {
-                $dbh->do("INSERT INTO email_setting " . 
-                         "(user_id, relationship, event) " . 
-                         "VALUES (?, ?, ?)",
-                         undef, ($user->id, $rel, $event));
+            my $is_set = $cgi->param("email-$rel-$event");
+            if ($is_set xor $settings->{$rel}{$event}) {
+                if ($is_set) {
+                    $sth_insert->execute($user->id, $rel, $event);
+                }
+                else {
+                    $sth_delete->execute($user->id, $rel, $event);
+                }
             }
         }
         
         # 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 (?, ?, ?)",
-                         undef, ($user->id, $rel, $event));
+            my $is_set = $cgi->param("neg-email-$rel-$event");
+            if (!$is_set xor $settings->{$rel}{$event}) {
+                if (!$is_set) {
+                    $sth_insert->execute($user->id, $rel, $event);
+                }
+                else {
+                    $sth_delete->execute($user->id, $rel, $event);
+                }
             }
         }
     }
 
     # Global positive events: a ticked box means "send me mail."
     foreach my $event (GLOBAL_EVENTS) {
-        if (defined($cgi->param("email-" . REL_ANY . "-$event"))
-            && $cgi->param("email-" . REL_ANY . "-$event") == 1)
-        {
-            $dbh->do("INSERT INTO email_setting " . 
-                     "(user_id, relationship, event) " . 
-                     "VALUES (?, ?, ?)",
-                     undef, ($user->id, REL_ANY, $event));
+        my $is_set = $cgi->param("email-" . REL_ANY . "-$event");
+        if ($is_set xor $settings->{+REL_ANY}{$event}) {
+            if ($is_set) {
+                $sth_insert->execute($user->id, REL_ANY, $event);
+            }
+            else {
+                $sth_delete->execute($user->id, REL_ANY, $event);
+            }
         }
     }
 
     $dbh->bz_commit_transaction();
 
+    # We have to clear the cache about email preferences.
+    delete $user->{'mail_settings'};
+
     ###########################################################################
     # User watching
     ###########################################################################