Fix for bug 80289: Group restrictions are now identified with checkboxes instead of select widgets. Also, product groups are now only offered if they are either already set, or match the current product (i.e. the option to set a product bit for a product other than the one the bug is in is no longer available) This results in much less clutter for the admin folks on sites with lots of products.

Patch by Joe Robbins <jmrobins@tgix.com>
r= justdave@syndicomm.com

5 files changed, 105 insertions, 51 deletions

diff --git a/bug_form.pl b/bug_form.pl
index 7801f2754..8e5d251ae 100644
--- a/bug_form.pl
+++ b/bug_form.pl
@@ -368,23 +368,34 @@ print "
 
 
 if ($::usergroupset ne '0') {
-    SendSQL("select bit, description, (bit & $bug{'groupset'} != 0) " . 
-            "from groups where bit & $::usergroupset != 0 " . 
-            "and isbuggroup != 0 " . 
+    SendSQL("select bit, name, description, (bit & $bug{'groupset'} != 0) " .
+	    "from groups where bit & $::usergroupset != 0 " .
+	    "and isbuggroup != 0 " . 
             # Include active groups as well as inactive groups to which
             # the bug already belongs.  This way the bug can be removed
             # from an inactive group but can only be added to active ones.
             "and (isactive = 1 or (bit & $bug{'groupset'} != 0)) " . 
-            "order by bit");
+	    "order by description");
+    # We only print out a header bit for this section if there are any
+    # results.
+    if(MoreSQLData()) {
+      print "<br><b>Only users in the selected groups can view this bug:</b><br>\n";
+    }
     while (MoreSQLData()) {
-        my ($bit, $description, $ison) = (FetchSQLData());
-        my $check0 = !$ison ? " SELECTED" : "";
-        my $check1 = $ison ? " SELECTED" : "";
-        print "<select name=bit-$bit><option value=0$check0>\n";
-        print "People not in the \"$description\" group can see this bug\n";
-        print "<option value=1$check1>\n";
-        print "Only people in the \"$description\" group can see this bug\n";
-        print "</select><br>\n";
+      my ($bit, $name, $description, $ison) = (FetchSQLData());
+      # For product groups, we only want to display the checkbox if either
+      # (1) The bit is already set, or
+      # (2) It's the group for this product.
+      # All other product groups will be skipped.  Non-product bug groups
+      # will still be displayed.
+      if($ison || ($name eq $bug{'product'}) || (!defined $::proddesc{$name})) {
+        # Modifying this to use checkboxes instead
+        my $checked = $ison ? " CHECKED" : "";
+        # indent these a bit
+        print "&nbsp;&nbsp;&nbsp;&nbsp;";
+        print "<input type=checkbox name=\"bit-$bit\" value=1$checked>\n";
+        print "$description<br>\n";
+      }
     }
 }
 
diff --git a/buglist.cgi b/buglist.cgi
index 58279a1c7..6c0ff1e7f 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -1450,17 +1450,21 @@ document.write(\" <input type=button value=\\\"Uncheck All\\\" onclick=\\\"SetCh
 <TEXTAREA WRAP=HARD NAME=comment ROWS=5 COLS=80></TEXTAREA><BR>";
 
 if ($::usergroupset ne '0' && $buggroupset =~ /^\d+$/) {
-    SendSQL("select bit, description, (bit & $buggroupset != 0) from groups where bit & $::usergroupset != 0 and isbuggroup != 0 order by bit");
+    SendSQL("select bit, description, (bit & $buggroupset != 0) from groups where bit & $::usergroupset != 0 and isbuggroup != 0 order by description");
+    # We only print out a header bit for this section if there are any
+    # results.
+    if(MoreSQLData()) {
+      print "<br><b>Only users in the selected groups can view this bug:</b><br>\n";
+    }
     while (MoreSQLData()) {
         my ($bit, $description, $ison) = (FetchSQLData());
-        my $check0 = !$ison ? " SELECTED" : "";
-        my $check1 = $ison ? " SELECTED" : "";
-        print "<select name=bit-$bit><option value=0$check0>\n";
-        print "People not in the \"$description\" group can see these bugs\n";
-        print "<option value=1$check1>\n";
-        print "Only people in the \"$description\" group can see these bugs\n";
-        print "</select><br>\n";
-    }
+        # Modifying this to use checkboxes instead
+        my $checked = $ison ? " CHECKED" : "";
+        # indent these a bit
+        print "&nbsp;&nbsp;&nbsp;&nbsp;";
+        print "<input type=checkbox name=\"bit-$bit\" value=1$checked>\n";
+        print "$description<br>\n";
+      }
 }
 
 
diff --git a/enter_bug.cgi b/enter_bug.cgi
index 62c7fc49c..c78628a6c 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -408,11 +408,21 @@ print "
 ";
 
 if ($::usergroupset ne '0') {
-    SendSQL("SELECT bit, description FROM groups " .
+    SendSQL("SELECT bit, name, description FROM groups " .
             "WHERE bit & $::usergroupset != 0 " .
             "  AND isbuggroup != 0 AND isactive = 1 ORDER BY description");
-     while (MoreSQLData()) {
-        my ($bit, $description) = (FetchSQLData());
+    # We only print out a header bit for this section if there are any
+    # results.
+    if(MoreSQLData()) {
+      print "<br><b>Only users in the selected groups can view this bug:</b><br>\n";
+      print "<font size=\"-1\">(Leave all boxes unchecked to make this a public bug.)</font><br><br>\n";
+    }
+    while (MoreSQLData()) {
+        my ($bit, $prodname, $description) = (FetchSQLData());
+        # Don't want to include product groups other than this product.
+        unless(($prodname eq $product) || (!defined($::proddesc{$prodname}))) {
+            next;
+        }
         # Rather than waste time with another Param check and another database
         # access, $group_bit will only have a non-zero value if we're using
         # bug groups and have  one for this product, so I'll check on that
@@ -421,21 +431,20 @@ if ($::usergroupset ne '0') {
         # select-box patch.  Also, if $group_bit is 0, it won't match the
         # current group, either, so I'll compare it to the current bit
         # instead of checking for non-zero. -DDM, 3/11/00
-        my $check = 0; # default selection
-        if($group_bit == $bit) {
-            # In addition, we need to handle the possibility that we're coming
-            # from a bookmark template.  We'll simply check if we've got a
-            # parameter called bit-# passed.  If so, then we're coming from a
-            # template, and we'll use the template value.
-            $check = formvalue("bit-$bit","1");
+        # Modifying this to use checkboxes instead of a select list.
+        # -JMR, 5/11/01
+        # If this is the group for this product, make it checked.
+        my $check = ($group_bit == $bit);
+        # If this is a bookmarked template, then we only want to set the bit
+        # for those bits set in the template.
+        if(formvalue("maketemplate","") eq "Remember values as bookmarkable template") {
+          $check = formvalue("bit-$bit",0);
         }
-        print BuildPulldown("bit-$bit",
-                            [["0",
-                             "People not in the \"$description\" group can see this bug"],
-                             ["1",
-                              "Only people in the \"$description\" group can see this bug"]],
-                            $check);
-        print "<BR>\n";
+        my $checked = $check ? " CHECKED" : "";
+        # indent these a bit
+        print "&nbsp;&nbsp;&nbsp;&nbsp;";
+        print "<input type=checkbox name=\"bit-$bit\" value=1$checked>\n";
+        print "$description<br>\n";
     }
 }
 
diff --git a/post_bug.cgi b/post_bug.cgi
index 68842b646..e83690897 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -202,7 +202,7 @@ $comment = trim($comment);
 # OK except for the fact that it causes e-mail to be suppressed.
 $comment = $comment ? $comment : " ";
 
-$query .= "now(), 0";
+$query .= "now(), (0";
 
 foreach my $b (grep(/^bit-\d*$/, keys %::FORM)) {
     if ($::FORM{$b}) {
@@ -226,7 +226,7 @@ foreach my $b (grep(/^bit-\d*$/, keys %::FORM)) {
 
 
 
-$query .= ")\n";
+$query .= ") & $::usergroupset)\n";
 
 
 my %ccids;
diff --git a/process_bug.cgi b/process_bug.cgi
index 1b02b7b0c..26ea412c1 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -382,20 +382,50 @@ sub CheckonComment( $ ) {
     return( ! $ret ); # Return val has to be inverted
 }
 
-
-my $foundbit = 0;
-foreach my $b (grep(/^bit-\d*$/, keys %::FORM)) {
-    if (!$foundbit) {
-        $foundbit = 1;
-        DoComma();
-        $::query .= "groupset = 0";
-    }
-    if ($::FORM{$b}) {
-        my $v = substr($b, 4);
-        $::query .= "+ $v";     # Carefully written so that the math is
+# Changing this so that it will process groups from checkboxes instead of
+# select lists.  This means that instead of looking for the bit-X values in
+# the form, we need to loop through all the bug groups this user has access
+# to, and for each one, see if it's selected.
+# In addition, adding a little extra work so that we don't clobber groupsets
+# for bugs where the user doesn't have access to the group, but does to the
+# bug (as with the proposed reporter access patch.)
+if($::usergroupset ne '0') {
+  # We want to start from zero and build up, since if all boxes have been
+  # unchecked, we want to revert to 0.
+  DoComma();
+  $::query .= "groupset = 0";
+  SendSQL("select bit from groups ".
+          "where bit & $::usergroupset != 0 ".
+          "and isbuggroup != 0 ".
+          "order by bit");
+  while(my $b = FetchSQLData()) {
+    if($::FORM{"bit-$b"}) {
+      $::query .= " + $b";      # Carefully written so that the math is
                                 # done by MySQL, which can handle 64-bit math,
                                 # and not by Perl, which I *think* can not.
     }
+  }
+  # If we're changing the groupset, then we want to check for any bits
+  # that may have been excluded because the user wasn't in that group, but
+  # that were set previously.
+  my $tmpbugid = 0;
+  if(defined $::FORM{'id'}) {
+    $tmpbugid = $::FORM{'id'};
+  } else {
+    $tmpbugid = (grep(/^id_/, (keys %::FORM)))[0];
+    $tmpbugid =~ s/^id_//;
+  }
+  SendSQL("select sum(bit) from groups ".
+          "LEFT JOIN bugs ON bugs.groupset & bit != 0 ".
+          "where bugs.bug_id = $tmpbugid ".
+          "and bit & $::usergroupset = 0 ".
+          "and isbuggroup != 0");
+  if(MoreSQLData()) {
+    my ($bitsum) = FetchSQLData();
+    if($bitsum =~ /^\d+$/) {
+      $::query .= " + $bitsum";
+    }
+  }
 }
 
 foreach my $field ("rep_platform", "priority", "bug_severity",