Bug 330521: Remove @::legal_product, @::legal_components and @::legal_target_milestone - Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=justdave

8 files changed, 62 insertions, 123 deletions

diff --git a/Bugzilla/Version.pm b/Bugzilla/Version.pm
index 53bf9a678..9492e8135 100644
--- a/Bugzilla/Version.pm
+++ b/Bugzilla/Version.pm
@@ -109,13 +109,6 @@ sub check_version {
     return $version;
 }
 
-sub distinct_names {
-    my $dbh = Bugzilla->dbh;
-    my $names = $dbh->selectcol_arrayref(
-        'SELECT DISTINCT value FROM versions ORDER BY value');
-    return @$names;
-}
-
 1;
 
 __END__
@@ -179,17 +172,6 @@ Version.pm represents a Product Version object.
 
  Returns:     Bugzilla::Version object.
 
-=item C<distinct_names()>
-
- Description: A utility function for getting all the
-              possible version values from the database,
-              regardless of what product they're in.
-              Returns a list with no duplicate versions.
-
- Params:      none
-
- Returns:     A list of strings (versions).
-
 =back
 
 =cut
diff --git a/collectstats.pl b/collectstats.pl
index 2d7d3d258..f6bfbdae1 100755
--- a/collectstats.pl
+++ b/collectstats.pl
@@ -32,15 +32,14 @@
 use AnyDBM_File;
 use strict;
 use IO::Handle;
-use vars @::legal_product;
 
 use lib ".";
 require "globals.pl";
-use Bugzilla::Search;
-use Bugzilla::User;
-
 use Bugzilla;
 use Bugzilla::Config qw(:DEFAULT $datadir);
+use Bugzilla::Search;
+use Bugzilla::User;
+use Bugzilla::Product;
 
 # Turn off output buffering (probably needed when displaying output feedback
 # in the regenerate mode.)
@@ -64,8 +63,8 @@ if ($#ARGV >= 0 && $ARGV[0] eq "--regenerate") {
     $regenerate = 1;
 }
 
-my @myproducts;
-push( @myproducts, "-All-", @::legal_product );
+my @myproducts = map {$_->name} Bugzilla::Product::get_all_products();
+unshift(@myproducts, "-All-");
 
 my $tstart = time;
 foreach (@myproducts) {
diff --git a/config.cgi b/config.cgi
index f3471e14f..fe1c5936b 100755
--- a/config.cgi
+++ b/config.cgi
@@ -44,9 +44,6 @@ use vars
     @legal_platform 
     @legal_opsys 
     @legal_resolution 
-
-    @legal_components 
-    @legal_target_milestone 
   );
 
 # Use the global template variables defined in globals.pl 
diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index 1851e8b13..29a7467ad 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -38,6 +38,7 @@ use Bugzilla::Flag;
 use Bugzilla::FlagType;
 use Bugzilla::Group;
 use Bugzilla::Util;
+use Bugzilla::Product;
 
 my $template = Bugzilla->template;
 my $vars = {};
@@ -49,9 +50,6 @@ $user->in_group('editcomponents')
                                      action => "edit",
                                      object => "flagtypes"});
 
-# Suppress "used only once" warnings.
-use vars qw(@legal_product @legal_components %components);
-
 my $cgi = Bugzilla->cgi;
 my $product_id;
 my $component_id;
@@ -128,17 +126,18 @@ sub list {
 sub edit {
     $action eq 'enter' ? validateTargetType() : (my $id = validateID());
     my $dbh = Bugzilla->dbh;
-    
-    # Get this installation's products and components.
-    GetVersionTable();
-
-    # products and components and the function used to modify the components
-    # menu when the products menu changes; used by the template to populate
-    # the menus and keep the components menu consistent with the products menu
-    $vars->{'products'} = \@::legal_product;
-    $vars->{'components'} = \@::legal_components;
-    $vars->{'components_by_product'} = \%::components;
-    
+
+    my @products = Bugzilla::Product::get_all_products();
+    # We require all unique component names.
+    my %components;
+    foreach my $product (@products) {
+        foreach my $component (@{$product->components}) {
+            $components{$component->name} = 1;
+        }
+    }
+    $vars->{'products'} = \@products;
+    $vars->{'components'} = [sort(keys %components)];
+
     $vars->{'last_action'} = $cgi->param('action');
     if ($cgi->param('action') eq 'enter' || $cgi->param('action') eq 'copy') {
         $vars->{'action'} = "insert";
@@ -216,17 +215,21 @@ sub processCategoryChange {
     my %inclusions = clusion_array_to_hash(\@inclusions);
     my %exclusions = clusion_array_to_hash(\@exclusions);
 
-    # Get this installation's products and components.
-    GetVersionTable();
+    my @products = Bugzilla::Product::get_all_products();
+    # We require all unique component names.
+    my %components;
+    foreach my $product (@products) {
+        foreach my $component (@{$product->components}) {
+            $components{$component->name} = 1;
+        }
+    }
+    $vars->{'products'} = \@products;
+    $vars->{'components'} = [sort(keys %components)];
 
-    # products and components; used by the template to populate the menus 
-    # and keep the components menu consistent with the products menu
-    $vars->{'products'} = \@::legal_product;
-    $vars->{'components'} = \@::legal_components;
-    $vars->{'components_by_product'} = \%::components;
     my @groups = Bugzilla::Group::get_all_groups();
     $vars->{'groups'} = \@groups;
     $vars->{'action'} = $cgi->param('action');
+
     my $type = {};
     foreach my $key ($cgi->param()) { $type->{$key} = $cgi->param($key) }
     $type->{'inclusions'} = \%inclusions;
diff --git a/globals.pl b/globals.pl
index 018260959..8ac08846a 100644
--- a/globals.pl
+++ b/globals.pl
@@ -38,6 +38,7 @@ use Bugzilla::Util;
 use Bugzilla::Config qw(:DEFAULT ChmodDataFile $localconfig $datadir);
 use Bugzilla::User;
 use Bugzilla::Error;
+use Bugzilla::Product;
 
 # Shut up misguided -w warnings about "used only once".  For some reason,
 # "use vars" chokes on me when I try it here.
@@ -46,13 +47,10 @@ sub globals_pl_sillyness {
     my $zz;
     $zz = @main::enterable_products;
     $zz = @main::legal_bug_status;
-    $zz = @main::legal_components;
     $zz = @main::legal_opsys;
     $zz = @main::legal_platform;
     $zz = @main::legal_priority;
-    $zz = @main::legal_product;
     $zz = @main::legal_severity;
-    $zz = @main::legal_target_milestone;
     $zz = @main::milestoneurl;
     $zz = @main::prodmaxvotes;
 }
@@ -104,7 +102,7 @@ $::SIG{PIPE} = 'IGNORE';
 sub GenerateVersionTable {
     my $dbh = Bugzilla->dbh;
 
-    my (@line, %carray);
+    my @line;
     SendSQL("SELECT components.name, products.name " .
             "FROM components, products " .
             "WHERE products.id = components.product_id " .
@@ -116,7 +114,6 @@ sub GenerateVersionTable {
         }
         my $ref = $::components{$p};
         push @$ref, $c;
-        $carray{$c} = 1;
     }
 
     SendSQL("SELECT products.name, classifications.name " .
@@ -185,8 +182,6 @@ sub GenerateVersionTable {
         splice(@::settable_resolution, $z, 1);
     }
 
-    @::legal_product = map($_->name, Bugzilla::Product::get_all_products());
-
     require File::Temp;
     my ($fh, $tmpname) = File::Temp::tempfile("versioncache.XXXXX",
                                               DIR => "$datadir");
@@ -202,21 +197,19 @@ sub GenerateVersionTable {
     print $fh (Data::Dumper->Dump([\@::log_columns],
                                   ['*::log_columns']));
 
-    foreach my $i (@::legal_product) {
+    my @legal_products = map($_->name, Bugzilla::Product::get_all_products());
+    foreach my $i (@legal_products) {
         if (!defined $::components{$i}) {
             $::components{$i} = [];
         }
     }
     print $fh (Data::Dumper->Dump([\%::components],
                                   ['*::components']));
-    @::legal_components = sort {uc($a) cmp uc($b)} keys(%carray);
 
-    print $fh (Data::Dumper->Dump([\@::legal_components, \@::legal_product,
-                                   \@::legal_priority, \@::legal_severity,
+    print $fh (Data::Dumper->Dump([\@::legal_priority, \@::legal_severity,
                                    \@::legal_platform, \@::legal_opsys,
                                    \@::legal_bug_status, \@::legal_resolution],
-                                  ['*::legal_components', '*::legal_product',
-                                   '*::legal_priority', '*::legal_severity',
+                                  ['*::legal_priority', '*::legal_severity',
                                    '*::legal_platform', '*::legal_opsys',
                                    '*::legal_bug_status', '*::legal_resolution']));
 
@@ -234,25 +227,17 @@ sub GenerateVersionTable {
                 "WHERE products.id = milestones.product_id " .
                 "ORDER BY milestones.sortkey, milestones.value");
         my @line;
-        my %tmarray;
-        @::legal_target_milestone = ();
         while(@line = FetchSQLData()) {
             my ($tm, $pr) = (@line);
             if (!defined $::target_milestone{$pr}) {
                 $::target_milestone{$pr} = [];
             }
             push @{$::target_milestone{$pr}}, $tm;
-            if (!exists $tmarray{$tm}) {
-                $tmarray{$tm} = 1;
-                push(@::legal_target_milestone, $tm);
-            }
         }
 
         print $fh (Data::Dumper->Dump([\%::target_milestone,
-                                       \@::legal_target_milestone,
                                        \%::milestoneurl],
                                       ['*::target_milestone',
-                                       '*::legal_target_milestone',
                                        '*::milestoneurl']));
     }
 
diff --git a/post_bug.cgi b/post_bug.cgi
index de8063543..dd699e668 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -44,7 +44,6 @@ sub sillyness {
     $zz = @::legal_opsys;
     $zz = @::legal_platform;
     $zz = @::legal_priority;
-    $zz = @::legal_product;
     $zz = @::legal_severity;
     $zz = %::target_milestone;
 }
@@ -219,7 +218,8 @@ if (!Param('letsubmitterchoosepriority')) {
 GetVersionTable();
 
 # Some more sanity checking
-check_field('product',      scalar $cgi->param('product'),      \@::legal_product);
+check_field('product',      scalar $cgi->param('product'),
+            [map($_->name, Bugzilla::Product::get_all_products())]);
 check_field('rep_platform', scalar $cgi->param('rep_platform'), \@::legal_platform);
 check_field('bug_severity', scalar $cgi->param('bug_severity'), \@::legal_severity);
 check_field('priority',     scalar $cgi->param('priority'),     \@::legal_priority);
diff --git a/query.cgi b/query.cgi
index 9bb90e782..bda05d7aa 100755
--- a/query.cgi
+++ b/query.cgi
@@ -35,21 +35,16 @@ use Bugzilla::Search;
 use Bugzilla::User;
 use Bugzilla::Util;
 use Bugzilla::Product;
-use Bugzilla::Version;
 use Bugzilla::Keyword;
 
 use vars qw(
     @legal_resolution
     @legal_bug_status
-    @legal_components
     @legal_opsys
     @legal_platform
     @legal_priority
-    @legal_product
     @legal_severity
-    @legal_target_milestone
     @log_columns
-    %components
 );
 
 my $cgi = Bugzilla->cgi;
@@ -215,50 +210,26 @@ if ($default{'chfieldto'}->[0] eq "") {
     $default{'chfieldto'} = ["Now"];
 }
 
-GetVersionTable();
-
 # if using groups for entry, then we don't want people to see products they 
 # don't have access to. Remove them from the list.
-
 my @selectable_products = sort {lc($a->name) cmp lc($b->name)} 
                                @{$user->get_selectable_products};
 
-my %component_set;
-my %version_set;
-my %milestone_set;
-
-foreach my $prod_obj (@selectable_products) {
-    # We build up boolean hashes in the "-set" hashes for each of these things 
-    # before making a list because there may be duplicates names across products.
-    my @component_names = map($_->name, @{$prod_obj->components});
-    my @version_names   = map($_->name, @{$prod_obj->versions});
-    my @milestone_names = map($_->name, @{$prod_obj->milestones});
-    $component_set{$_} = 1 foreach (@component_names);
-    $version_set{$_}   = 1 foreach (@version_names);
-    $milestone_set{$_} = 1 foreach (@milestone_names);
-}
-
 # Create the component, version and milestone lists.
-my @components = ();
-my @versions = ();
-my @milestones = ();
-foreach my $c (@::legal_components) {
-    if ($component_set{$c}) {
-        push @components, $c;
-    }
-}
-my @all_versions = Bugzilla::Version::distinct_names();
-foreach my $v (@all_versions) {
-    if ($version_set{$v}) {
-        push @versions, $v;
-    }
-}
-foreach my $m (@::legal_target_milestone) {
-    if ($milestone_set{$m}) {
-        push @milestones, $m;
-    }
+my %components;
+my %versions;
+my %milestones;
+
+foreach my $product (@selectable_products) {
+    $components{$_->name} = 1 foreach (@{$product->components});
+    $versions{$_->name}   = 1 foreach (@{$product->versions});
+    $milestones{$_->name} = 1 foreach (@{$product->milestones});
 }
 
+my @components = sort(keys %components);
+my @versions = sort(keys %versions);
+my @milestones = sort(keys %milestones);
+
 $vars->{'product'} = \@selectable_products;
 
 # Create data structures representing each classification
@@ -277,6 +248,8 @@ if (Param('usetargetmilestone')) {
 
 $vars->{'have_keywords'} = Bugzilla::Keyword::keyword_count();
 
+GetVersionTable();
+
 push @::legal_resolution, "---"; # Oy, what a hack.
 shift @::legal_resolution; 
       # Another hack - this array contains "" for some reason. See bug 106589.
diff --git a/template/en/default/admin/flag-type/edit.html.tmpl b/template/en/default/admin/flag-type/edit.html.tmpl
index dc49890b7..cb3ce9e17 100644
--- a/template/en/default/admin/flag-type/edit.html.tmpl
+++ b/template/en/default/admin/flag-type/edit.html.tmpl
@@ -28,9 +28,9 @@
   var first_load = 1; // is this the first time we load the page?
   var last_sel = []; // caches last selection
   var cpts = new Array();
-  [% FOREACH p = products %]
-    cpts['[% p FILTER js %]'] = [
-      [%- FOREACH item = components_by_product.$p %]'[% item FILTER js %]'[% ", " UNLESS loop.last %] [%- END -%] ];
+  [% FOREACH prod = products %]
+    cpts['[% prod.name FILTER js %]'] = [
+      [%- FOREACH comp = prod.components %]'[% comp.name FILTER js %]'[% ", " UNLESS loop.last %] [%- END -%] ];
   [% END %]
 [% END %]
 
@@ -107,18 +107,18 @@
               <b>Product/Component:</b><br>
               <select name="product" onchange="selectProduct(this.form, 'product', 'component', '__Any__');">
                 <option value="">__Any__</option>
-                [% FOREACH item = products %]
-                  <option value="[% item FILTER html %]" 
-                          [% "selected" IF type.product.name == item %]>
-                          [% item FILTER html %]</option>
+                [% FOREACH prod = products %]
+                  <option value="[% prod.name FILTER html %]" 
+                          [% "selected" IF type.product.name == prod.name %]>
+                          [% prod.name FILTER html %]</option>
                 [% END %]
               </select><br>
               <select name="component">
                 <option value="">__Any__</option>
-                [% FOREACH item = components %]
-                  <option value="[% item FILTER html %]" 
-                          [% "selected" IF type.component.name == item %]>
-                          [% item FILTER html %]</option>
+                [% FOREACH comp = components %]
+                  <option value="[% comp FILTER html %]" 
+                          [% "selected" IF type.component.name == comp %]>
+                          [% comp FILTER html %]</option>
                 [% END %]
               </select><br>
               <input type="submit" name="categoryAction-include" value="Include">