Checkin for Bug 42851 'Use listbox with input for CC management on bug form'

contributed by dave@intrec.com (Dave Miller)

4 files changed, 134 insertions, 31 deletions

diff --git a/Bugzilla/RelationSet.pm b/Bugzilla/RelationSet.pm
index 92e2158f2..b5cae289c 100644
--- a/Bugzilla/RelationSet.pm
+++ b/Bugzilla/RelationSet.pm
@@ -18,6 +18,7 @@
 # 
 # Contributor(s): Dan Mosedale <dmose@mozilla.org>
 #                 Terry Weissman <terry@mozilla.org>
+#                 Dave Miller <dave@intrec.com>
 
 # This object models a set of relations between one item and a group
 # of other items.  An example is the set of relations between one bug
@@ -179,6 +180,42 @@ sub mergeFromString {
   }
 }
 
+# remove a set in string form from this set
+#
+sub removeItemsInString {
+  ($#_ == 1) || confess("invalid number of arguments");
+  my $self = shift();
+
+  # do the merge
+  #
+  foreach my $person (split(/[ ,]/, shift())) {
+    if ($person ne "") {
+      my $dbid = &::DBNameToIdAndCheck($person);
+      if (exists $$self{$dbid}) {
+        delete $$self{$dbid};
+      }
+    }
+  }
+}
+
+# remove a set in array form from this set
+#
+sub removeItemsInArray {
+  ($#_ > 0) || confess("invalid number of arguments");
+  my $self = shift();
+
+  # do the merge
+  #
+  while (my $person = shift()) {
+    if ($person ne "") {
+      my $dbid = &::DBNameToIdAndCheck($person);
+      if (exists $$self{$dbid}) {
+        delete $$self{$dbid};
+      }
+    }
+  }
+}
+
 # return the number of elements in this set
 #
 sub size {
diff --git a/RelationSet.pm b/RelationSet.pm
index 92e2158f2..b5cae289c 100644
--- a/RelationSet.pm
+++ b/RelationSet.pm
@@ -18,6 +18,7 @@
 # 
 # Contributor(s): Dan Mosedale <dmose@mozilla.org>
 #                 Terry Weissman <terry@mozilla.org>
+#                 Dave Miller <dave@intrec.com>
 
 # This object models a set of relations between one item and a group
 # of other items.  An example is the set of relations between one bug
@@ -179,6 +180,42 @@ sub mergeFromString {
   }
 }
 
+# remove a set in string form from this set
+#
+sub removeItemsInString {
+  ($#_ == 1) || confess("invalid number of arguments");
+  my $self = shift();
+
+  # do the merge
+  #
+  foreach my $person (split(/[ ,]/, shift())) {
+    if ($person ne "") {
+      my $dbid = &::DBNameToIdAndCheck($person);
+      if (exists $$self{$dbid}) {
+        delete $$self{$dbid};
+      }
+    }
+  }
+}
+
+# remove a set in array form from this set
+#
+sub removeItemsInArray {
+  ($#_ > 0) || confess("invalid number of arguments");
+  my $self = shift();
+
+  # do the merge
+  #
+  while (my $person = shift()) {
+    if ($person ne "") {
+      my $dbid = &::DBNameToIdAndCheck($person);
+      if (exists $$self{$dbid}) {
+        delete $$self{$dbid};
+      }
+    }
+  }
+}
+
 # return the number of elements in this set
 #
 sub size {
diff --git a/bug_form.pl b/bug_form.pl
index 1263957c7..5f6a77cea 100644
--- a/bug_form.pl
+++ b/bug_form.pl
@@ -18,6 +18,7 @@
 # Rights Reserved.
 #
 # Contributor(s): Terry Weissman <terry@mozilla.org>
+#                 Dave Miller <dave@intrec.com>
 
 use diagnostics;
 use strict;
@@ -151,9 +152,16 @@ my $component_popup = make_options($::components{$bug{'product'}},
 
 my $ccSet = new RelationSet;
 $ccSet->mergeFromDB("select who from cc where bug_id=$id");
-my $cc_element = '<INPUT NAME=cc SIZE=30 VALUE="' .
-  $ccSet->toString() . '">';
-
+my @ccList = $ccSet->toArrayOfStrings();
+my $cc_element = "<INPUT TYPE=HIDDEN NAME=cc VALUE=\"\">";
+if (scalar(@ccList) > 0) {
+  $cc_element = "<SELECT NAME=cc MULTIPLE SIZE=5>\n";
+  foreach my $ccName ( @ccList ) {
+    $cc_element .= "<OPTION VALUE=\"$ccName\">$ccName\n";
+  }
+  $cc_element .= "</SELECT><BR>\n" .
+        "<INPUT TYPE=CHECKBOX NAME=removecc>Remove selected CCs<br>\n";
+}
 
 my $URL = $bug{'bug_file_loc'};
 
@@ -169,40 +177,55 @@ print "
 <INPUT TYPE=HIDDEN NAME=\"id\" VALUE=$id>
   <TABLE CELLSPACING=0 CELLPADDING=0 BORDER=0><TR>
     <TD ALIGN=RIGHT><B>Bug#:</B></TD><TD><A HREF=\"show_bug.cgi?id=$bug{'bug_id'}\">$bug{'bug_id'}</A></TD>
+  <TD>&nbsp;</TD>
     <TD ALIGN=RIGHT><B><A HREF=\"bug_status.html#rep_platform\">Platform:</A></B></TD>
     <TD><SELECT NAME=rep_platform>$platform_popup</SELECT></TD>
-    <TD ALIGN=RIGHT><B>Version:</B></TD>
-    <TD><SELECT NAME=version>" .
-    make_options($::versions{$bug{'product'}}, $bug{'version'}) .
-    "</SELECT></TD>
-  </TR><TR>
+  <TD>&nbsp;</TD>
+    <TD ALIGN=RIGHT><B>Reporter:</B></TD><TD>$bug{'reporter'}</TD>
+</TR><TR>
     <TD ALIGN=RIGHT><B>Product:</B></TD>
     <TD><SELECT NAME=product>" .
     make_options(\@::legal_product, $bug{'product'}) .
     "</SELECT></TD>
+  <TD>&nbsp;</TD>
     <TD ALIGN=RIGHT><B>OS:</B></TD>
     <TD><SELECT NAME=op_sys>" .
     make_options(\@::legal_opsys, $bug{'op_sys'}) .
-    "</SELECT><TD ALIGN=RIGHT><B>Reporter:</B></TD><TD>$bug{'reporter'}</TD>
-     </TDTR><TR>
+    "</SELECT></TD>
+  <TD>&nbsp;</TD>
+    <TD ALIGN=RIGHT NOWRAP><b>Add CC:</b></TD>
+    <TD><INPUT NAME=newcc SIZE=30 VALUE=\"\"></TD>
+</TR><TR>
+    <TD ALIGN=RIGHT><B><A HREF=\"describecomponents.cgi?product=" .
+    url_quote($bug{'product'}) . "\">Component:</A></B></TD>
+      <TD><SELECT NAME=component>$component_popup</SELECT></TD>
+  <TD>&nbsp;</TD>
+    <TD ALIGN=RIGHT><B>Version:</B></TD>
+    <TD><SELECT NAME=version>" .
+    make_options($::versions{$bug{'product'}}, $bug{'version'}) .
+    "</SELECT></TD>
+  <TD>&nbsp;</TD>
+    <TD ROWSPAN=4 ALIGN=RIGHT VALIGN=TOP><B>Cc:</B></TD>
+    <TD ROWSPAN=4 VALIGN=TOP> $cc_element </TD>
+</TR><TR>
     <TD ALIGN=RIGHT><B><A HREF=\"bug_status.html\">Status:</A></B></TD>
       <TD>$bug{'bug_status'}</TD>
+  <TD>&nbsp;</TD>
     <TD ALIGN=RIGHT><B><A HREF=\"bug_status.html#priority\">Priority:</A></B></TD>
       <TD><SELECT NAME=priority>$priority_popup</SELECT></TD>
-    <TD ALIGN=RIGHT><B>Cc:</B></TD>
-      <TD> $cc_element </TD>
-  </TR><TR>
+  <TD>&nbsp;</TD>
+</TR><TR>
     <TD ALIGN=RIGHT><B><A HREF=\"bug_status.html\">Resolution:</A></B></TD>
       <TD>$bug{'resolution'}</TD>
+  <TD>&nbsp;</TD>
     <TD ALIGN=RIGHT><B><A HREF=\"bug_status.html#severity\">Severity:</A></B></TD>
       <TD><SELECT NAME=bug_severity>$sev_popup</SELECT></TD>
-    <TD ALIGN=RIGHT><B><A HREF=\"describecomponents.cgi?product=" .
-    url_quote($bug{'product'}) . "\">Component:</A></B></TD>
-      <TD><SELECT NAME=component>$component_popup</SELECT></TD>
-  </TR><TR>
+  <TD>&nbsp;</TD>
+</TR><TR>
     <TD ALIGN=RIGHT><B><A HREF=\"bug_status.html#assigned_to\">Assigned&nbsp;To:
         </A></B></TD>
-      <TD>$bug{'assigned_to'}</TD>";
+      <TD>$bug{'assigned_to'}</TD>
+  <TD>&nbsp;</TD>";
 
 if (Param("usetargetmilestone")) {
     my $url = "";
@@ -220,8 +243,9 @@ if (Param("usetargetmilestone")) {
 <TD><SELECT NAME=target_milestone>" .
     make_options($::target_milestone{$bug{'product'}},
                  $bug{'target_milestone'}) .
-                     "</SELECT></TD>";
-}
+                     "</SELECT></TD>
+  <TD>&nbsp;</TD>";
+} else { print "<TD></TD><TD></TD><TD>&nbsp;</TD>"; }
 
 print "
 </TR>";
@@ -234,7 +258,7 @@ if (Param("useqacontact")) {
     <TD COLSPAN=6>
       <INPUT NAME=qa_contact VALUE=\"" .
     value_quote($name) .
-    "\" SIZE=60></
+    "\" SIZE=60></TD>
   </TR>";
 }
 
@@ -242,11 +266,11 @@ if (Param("useqacontact")) {
 print "
   <TR>
     <TD ALIGN=\"RIGHT\">$URL
-    <TD COLSPAN=6>
+    <TD COLSPAN=8>
       <INPUT NAME=bug_file_loc VALUE=\"$bug{'bug_file_loc'}\" SIZE=60></TD>
   </TR><TR>
     <TD ALIGN=\"RIGHT\"><B>Summary:</B>
-    <TD COLSPAN=6>
+    <TD COLSPAN=8>
       <INPUT NAME=short_desc VALUE=\"" .
     value_quote($bug{'short_desc'}) .
     "\" SIZE=60></TD>
@@ -256,10 +280,10 @@ if (Param("usestatuswhiteboard")) {
     print "
   <TR>
     <TD ALIGN=\"RIGHT\"><B>Status Whiteboard:</B>
-    <TD COLSPAN=6>
+    <TD COLSPAN=8>
       <INPUT NAME=status_whiteboard VALUE=\"" .
     value_quote($bug{'status_whiteboard'}) .
-    "\" SIZE=60></
+    "\" SIZE=60></TD>
   </TR>";
 }
 
@@ -276,7 +300,7 @@ if (@::legal_keywords) {
     print qq{
 <TR>
 <TD ALIGN=right><B><A HREF="describekeywords.cgi">Keywords</A>:</B>
-<TD COLSPAN=6><INPUT NAME="keywords" VALUE="$value" SIZE=60></TD>
+<TD COLSPAN=8><INPUT NAME="keywords" VALUE="$value" SIZE=60></TD>
 </TR>
 };
 }
@@ -292,7 +316,7 @@ while (MoreSQLData()) {
     $desc = value_quote($desc);
     print qq{<td><a href="$link">$date</a></td><td colspan=4>$desc</td></tr><tr><td></td>};
 }
-print "<td colspan=6><a href=\"createattachment.cgi?id=$id\">Create a new attachment</a> (proposed patch, testcase, etc.)</td></tr></table>\n";
+print "<td colspan=8><a href=\"createattachment.cgi?id=$id\">Create a new attachment</a> (proposed patch, testcase, etc.)</td></tr></table>\n";
 
 
 sub EmitDependList {
diff --git a/process_bug.cgi b/process_bug.cgi
index 8b1226b3a..7665c1eda 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -20,6 +20,7 @@
 #
 # Contributor(s): Terry Weissman <terry@mozilla.org>
 #                 Dan Mosedale <dmose@mozilla.org>
+#                 Dave Miller <dave@intrec.com>
 
 use diagnostics;
 use strict;
@@ -391,10 +392,14 @@ my $origCcString;
 # form of the data it gets from $::FORM{'cc'}, so anything bogus from a 
 # security standpoint should trigger an abort there.
 #
-if (defined $::FORM{'cc'} && defined $::FORM{'id'}) {
+if (defined $::FORM{'newcc'} && defined $::FORM{'id'}) {
     $origCcSet->mergeFromDB("select who from cc where bug_id = $::FORM{'id'}");
+    $formCcSet->mergeFromDB("select who from cc where bug_id = $::FORM{'id'}");
     $origCcString = $origCcSet->toString();  # cache a copy of the string vers
-    $formCcSet->mergeFromString($::FORM{'cc'});
+    if ((exists $::FORM{'removecc'}) && (exists $::FORM{'cc'})) {
+      $formCcSet->removeItemsInArray(@{$::MFORM{'cc'}});
+    }
+    $formCcSet->mergeFromString($::FORM{'newcc'});
 }
 
 if ( Param('strictvaluechecks') ) {
@@ -789,7 +794,7 @@ The changes made were:
         AppendComment($id, $::FORM{'who'}, $::FORM{'comment'});
     }
     
-    if (defined $::FORM{'cc'} && defined $::FORM{'id'}
+    if (defined $::FORM{'newcc'} && defined $::FORM{'id'}
         && ! $origCcSet->isEqual($formCcSet) ) {
 
         # update the database to look like the form
@@ -802,7 +807,7 @@ The changes made were:
 
         my $col = GetFieldID('cc');
         my $origq = SqlQuote($origCcString);
-        my $newq = SqlQuote($::FORM{'cc'});
+        my $newq = SqlQuote($formCcSet->toString());
         SendSQL("INSERT INTO bugs_activity " . 
                 "(bug_id,who,bug_when,fieldid,oldvalue,newvalue) VALUES " . 
                 "($id,$whoid,'$timestamp',$col,$origq,$newq)");