Bug 1191706: When editing flag types, components do not match the selected product when classifications are enabled

r/a=dkl

7 files changed, 51 insertions, 86 deletions

diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index d848d250a..d0b9443b5 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -436,17 +436,30 @@ sub get_products_and_components {
 
     my @products;
     if ($user->in_group('editcomponents')) {
-        @products = Bugzilla::Product->get_all;
+        if (Bugzilla->params->{useclassification}) {
+            # We want products grouped by classifications.
+            @products = map { @{ $_->products } } Bugzilla::Classification->get_all;
+        }
+        else {
+            @products = Bugzilla::Product->get_all;
+        }
     }
     else {
         @products = @{$user->get_products_by_permission('editcomponents')};
+
+        if (Bugzilla->params->{useclassification}) {
+            my %class;
+            push(@{$class{$_->classification_id}}, $_) foreach @products;
+
+            # Let's sort the list by classifications.
+            @products = ();
+            push(@products, @{$class{$_->id}}) foreach Bugzilla::Classification->get_all;
+        }
     }
-    # We require all unique component names.
+
     my %components;
     foreach my $product (@products) {
-        foreach my $component (@{$product->components}) {
-            $components{$component->name} = 1;
-        }
+        $components{$_->name} = 1 foreach @{$product->components};
     }
     $vars->{'products'} = \@products;
     $vars->{'components'} = [sort(keys %components)];
diff --git a/js/productform.js b/js/productform.js
index d5e24d306..0f669aa26 100644
--- a/js/productform.js
+++ b/js/productform.js
@@ -10,16 +10,10 @@
 // collection of javascript arrays containing strings.
 
 /**
- * Reads the selected products and updates component, version and milestone
- * lists accordingly.
+ * Reads the selected products and updates the component list accordingly.
  *
  * @param  product    Select element that contains products.
- * @param  component  Select element that contains components. Can be null if
- *                    there is no such element to update.
- * @param  version    Select element that contains versions. Can be null if
- *                    there is no such element to update.
- * @param  milestone  Select element that contains milestones. Can be null if
- *                    there is no such element to update.
+ * @param  component  Select element that contains components.
  * @param  anyval     Value to use for a special "Any" list item. Can be null
  *                    to not use any. If used must and will be first item in
  *                    the select element.
@@ -27,21 +21,15 @@
  * @global cpts       Array of arrays, indexed by product name. The subarrays
  *                    contain a list of components to be fed to the respective
  *                    select element.
- * @global vers       Array of arrays, indexed by product name. The subarrays
- *                    contain a list of versions to be fed to the respective
- *                    select element.
- * @global tms        Array of arrays, indexed by product name. The subarrays
- *                    contain a list of milestones to be fed to the respective
- *                    select element.
  * @global first_load Boolean; true if this is the first time this page loads
  *                    or false if not.
  * @global last_sel   Array that contains last list of products so we know what
  *                    has changed, and optimize for additions.
  */
-function selectProduct(product, component, version, milestone, anyval) {
+function selectProduct(product, component, anyval) {
     // This is to avoid handling events that occur before the form
     // itself is ready, which could happen in buggy browsers.
-    if (!product)
+    if (!product || !component)
         return;
 
     // Do nothing if no products are defined. This is to avoid the
@@ -78,15 +66,8 @@ function selectProduct(product, component, version, milestone, anyval) {
     var findall = (product.selectedIndex == -1
                    || (anyval != null && product.options[0].selected));
 
-    if (useclassification) {
-        // Update index based on the complete product array.
-        sel = get_selection(product, findall, true, anyval);
-        for (var i=0; i<sel.length; i++)
-           sel[i] = prods[sel[i]];
-    }
-    else {
-        sel = get_selection(product, findall, false, anyval);
-    }
+    sel = get_selection(product, findall, false, anyval);
+
     if (!findall) {
         // Save sel for the next invocation of selectProduct().
         var tmp = sel;
@@ -103,23 +84,9 @@ function selectProduct(product, component, version, milestone, anyval) {
     }
 
     // Do the actual fill/update.
-    if (component) {
-        var saved_cpts = get_selection(component, false, true, null);
-        updateSelect(cpts, sel, component, merging, anyval);
-        restoreSelection(component, saved_cpts);
-    }
-
-    if (version) {
-        var saved_vers = get_selection(version, false, true, null);
-        updateSelect(vers, sel, version, merging, anyval);
-        restoreSelection(version, saved_vers);
-    }
-
-    if (milestone) {
-        var saved_tms = get_selection(milestone, false, true, null);
-        updateSelect(tms, sel, milestone, merging, anyval);
-        restoreSelection(milestone, saved_tms);
-    }
+    var saved_cpts = get_selection(component, false, true, null);
+    updateSelect(cpts, sel, component, merging, anyval);
+    restoreSelection(component, saved_cpts);
 }
 
 /**
diff --git a/request.cgi b/request.cgi
index 566781a0c..347cb7a44 100755
--- a/request.cgi
+++ b/request.cgi
@@ -305,13 +305,22 @@ sub queue {
     $vars->{'requests'} = \@requests;
     $vars->{'types'} = \@types;
 
-    my %components;
-    foreach my $prod (@{$user->get_selectable_products}) {
-        foreach my $comp (@{$prod->components}) {
-            $components{$comp->name} = 1;
+    # This code is needed to populate the Product and Component select fields.
+    my ($products, %components);
+    if (Bugzilla->params->{useclassification}) {
+        foreach my $class (@{$user->get_selectable_classifications}) {
+            push @$products, @{$user->get_selectable_products($class->id)};
         }
     }
-    $vars->{'components'} = [ sort { $a cmp $b } keys %components ];
+    else {
+        $products = $user->get_selectable_products;
+    }
+
+    foreach my $product (@$products) {
+        $components{$_->name} = 1 foreach @{$product->components};
+    }
+    $vars->{'products'} = $products;
+    $vars->{'components'} = [ sort keys %components ];
 
     $vars->{'urlquerypart'} = $cgi->canonicalise_query('ctype');
 
diff --git a/template/en/default/admin/flag-type/edit.html.tmpl b/template/en/default/admin/flag-type/edit.html.tmpl
index 6c0e0cb78..7505ebd8c 100644
--- a/template/en/default/admin/flag-type/edit.html.tmpl
+++ b/template/en/default/admin/flag-type/edit.html.tmpl
@@ -23,7 +23,7 @@
   title = title
   style_urls = ['skins/standard/admin.css']
   onload="var f = document.forms['flagtype_properties'];
-          selectProduct(f.product, f.component, null, null, '__Any__');"
+          selectProduct(f.product, f.component, '__Any__');"
   javascript_urls=["js/productform.js"]
   doc_section = "administering/flags.html"
 %]
@@ -92,7 +92,7 @@
                 id       => "product"
                 name     => "product"
                 add      => "__Any__"
-                onchange => "selectProduct(this, this.form.component, null, null, '__Any__');"
+                onchange => "selectProduct(this, this.form.component, '__Any__');"
                 products => products
               %]<br>
               <select name="component">
diff --git a/template/en/default/admin/flag-type/list.html.tmpl b/template/en/default/admin/flag-type/list.html.tmpl
index b91b49199..b1665c3b0 100644
--- a/template/en/default/admin/flag-type/list.html.tmpl
+++ b/template/en/default/admin/flag-type/list.html.tmpl
@@ -11,7 +11,7 @@
 [% PROCESS global/header.html.tmpl
   title = 'Administer Flag Types'
   style_urls = ['skins/standard/admin.css']
-  onload="var f = document.flagtype_form; selectProduct(f.product, f.component, null, null, '__All__');"
+  onload="var f = document.flagtype_form; selectProduct(f.product, f.component, '__All__');"
   javascript_urls=["js/productform.js"]
   doc_section = "administering/flags.html"
 %]
@@ -43,7 +43,7 @@
       id       => "product"
       name     => "product"
       add      => "__Any__"
-      onchange => "selectProduct(this, this.form.component, null, null, '__Any__');"
+      onchange => "selectProduct(this, this.form.component, '__Any__');"
       products => products
     %]
   </div>
diff --git a/template/en/default/global/js-products.html.tmpl b/template/en/default/global/js-products.html.tmpl
index e40a3e3ae..940a5e64d 100644
--- a/template/en/default/global/js-products.html.tmpl
+++ b/template/en/default/global/js-products.html.tmpl
@@ -8,14 +8,13 @@
 
 [%# The javascript block gets used in header.html.tmpl. %]
 [% javascript = BLOCK %]
-  var useclassification = false; // No classification level in use
   var first_load = true; // Is this the first time we load the page?
   var last_sel = []; // Caches last selection
   var cpts = new Array();
+
   [% n = 1 %]
   [% FOREACH prod = products %]
-    cpts['[% n %]'] = [
-      [%- FOREACH comp = prod.components %]'[% comp.name FILTER js %]'[% ", " UNLESS loop.last %] [%- END -%] ];
+    cpts['[% n %]'] = [[% FOREACH comp = prod.components %]'[% comp.name FILTER js %]'[% ", " UNLESS loop.last %] [%- END -%]];
     [% n = n+1 %]
   [% END %]
 [% END %]
diff --git a/template/en/default/request/queue.html.tmpl b/template/en/default/request/queue.html.tmpl
index 7e8c44c8b..101fdd046 100644
--- a/template/en/default/request/queue.html.tmpl
+++ b/template/en/default/request/queue.html.tmpl
@@ -9,40 +9,17 @@
 [% USE Bugzilla %]
 [% cgi = Bugzilla.cgi %]
 
+[% PROCESS "global/js-products.html.tmpl" %]
+
 [% PROCESS global/header.html.tmpl
   title="Request Queue"
   generate_api_token = 1
-  onload="var f = document.request_form; selectProduct(f.product, f.component, null, null, 'Any');"
+  onload="var f = document.request_form; selectProduct(f.product, f.component, 'Any');"
   javascript_urls=["js/productform.js", "js/field.js"]
   style_urls = ['skins/standard/buglist.css']
   yui = ['autocomplete']
 %]
 
-<script type="text/javascript">
-  var useclassification = false; // No classification level in use
-  var first_load = true; // Is this the first time we load the page?
-  var last_sel = []; // Caches last selection
-  var cpts = new Array();
-  [% n = 1 %]
-  [% IF Param('useclassification') %]
-    [% FOREACH clas = user.get_selectable_classifications %]
-      [% FOREACH prod = user.get_selectable_products(clas.id) %]
-        [%+ PROCESS js_comp %]
-      [% END %]
-    [% END %]
-  [% ELSE %]
-    [% FOREACH prod = user.get_selectable_products %]
-      [%+ PROCESS js_comp %]
-    [% END %]
-  [% END %]
-</script>
-
-[% BLOCK js_comp %]
-  cpts['[% n %]'] = [
-    [%- FOREACH comp = prod.components %]'[% comp.name FILTER js %]'[% ", " UNLESS loop.last %] [%- END -%]];
-  [% n = n+1 %]
-[% END %]
-
 <p>
 When you are logged in, only requests made by you or addressed to you
 are shown by default.  You can change the criteria using the form below.
@@ -72,7 +49,7 @@ to some group are shown by default.
            id       => "product"
            name     => "product"
            add      => "Any"
-           onchange => "selectProduct(this, this.form.component, null, null, 'Any');"
+           onchange => "selectProduct(this, this.form.component, 'Any');"
         %]
       </td>
       <th>Flag:</th>