Bug 330698: Remove $::components, @::enterable_products and $::classifications (and some others) - Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=justdave

8 files changed, 34 insertions, 141 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index c314b8ee1..60779b29f 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -32,8 +32,7 @@ use strict;
 
 use vars qw(@legal_platform
             @legal_priority @legal_severity @legal_opsys @legal_bug_status
-            @settable_resolution %components %target_milestone
-            @enterable_products %milestoneurl %prodmaxvotes);
+            @settable_resolution %prodmaxvotes);
 
 use CGI::Carp qw(fatalsToBrowser);
 
@@ -469,7 +468,9 @@ sub milestoneurl {
     my ($self) = @_;
     return $self->{'milestoneurl'} if exists $self->{'milestoneurl'};
     return '' if $self->{'error'};
-    $self->{'milestoneurl'} = $::milestoneurl{$self->{product}};
+
+    $self->{'prod_obj'} ||= new Bugzilla::Product({name => $self->{'product'}});
+    $self->{'milestoneurl'} = $self->{'prod_obj'}->milestone_url;
     return $self->{'milestoneurl'};
 }
 
@@ -640,34 +641,11 @@ sub choices {
     $self->{'choices'} = {};
     $self->{prod_obj} ||= new Bugzilla::Product({name => $self->{product}});
 
-    # Fiddle the product list.
-    my $seen_curr_prod;
-    my @prodlist;
-
-    foreach my $product (@::enterable_products) {
-        if ($product eq $self->{'product'}) {
-            # if it's the product the bug is already in, it's ALWAYS in
-            # the popup, period, whether the user can see it or not, and
-            # regardless of the disallownew setting.
-            $seen_curr_prod = 1;
-            push(@prodlist, $product);
-            next;
-        }
-
-        if (!Bugzilla->user->can_enter_product($product)) {
-            # If we're using bug groups to restrict entry on products, and
-            # this product has an entry group, and the user is not in that
-            # group, we don't want to include that product in this list.
-            next;
-        }
-
-        push(@prodlist, $product);
-    }
-
+    my @prodlist = map {$_->name} @{Bugzilla->user->get_enterable_products};
     # The current product is part of the popup, even if new bugs are no longer
     # allowed for that product
-    if (!$seen_curr_prod) {
-        push (@prodlist, $self->{'product'});
+    if (lsearch(\@prodlist, $self->{'product'}) < 0) {
+        push(@prodlist, $self->{'product'});
         @prodlist = sort @prodlist;
     }
 
@@ -683,9 +661,9 @@ sub choices {
        'op_sys' => \@::legal_opsys,
        'bug_status' => \@::legal_bug_status,
        'resolution' => \@res,
-       'component' => $::components{$self->{product}},
+       'component' => [map($_->name, @{$self->{prod_obj}->components})],
        'version' => [map($_->name, @{$self->{prod_obj}->versions})],
-       'target_milestone' => $::target_milestone{$self->{product}},
+       'target_milestone' => [map($_->name, @{$self->{prod_obj}->milestones})],
       };
 
     return $self->{'choices'};
diff --git a/buglist.cgi b/buglist.cgi
index 18b4658aa..9796d05d1 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -46,13 +46,10 @@ use Bugzilla::Keyword;
 # Include the Bugzilla CGI and general utility library.
 require "globals.pl";
 
-use vars qw(@components
-            @legal_platform
+use vars qw(@legal_platform
             @legal_priority
-            @legal_product
             @legal_severity
-            @settable_resolution
-            @target_milestone);
+            @settable_resolution);
 
 my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
diff --git a/duplicates.cgi b/duplicates.cgi
index 10ba5bf2b..8039e6b1d 100755
--- a/duplicates.cgi
+++ b/duplicates.cgi
@@ -67,8 +67,6 @@ else {
 
 my $dbh = Bugzilla->switch_to_shadow_db();
 
-use vars qw (@legal_product);
-
 my %dbmcount;
 my %count;
 my %before;
diff --git a/enter_bug.cgi b/enter_bug.cgi
index 529c4ce73..068e4fb0e 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -52,7 +52,6 @@ use vars qw(
   @legal_platform
   @legal_priority
   @legal_severity
-  %target_milestone
 );
 
 # If we're using bug groups to restrict bug entry, we need to know who the 
@@ -317,9 +316,9 @@ GetVersionTable();
 
 my $product_id = get_product_id($product);
 
-if (1 == @{$::components{$product}}) {
+if (scalar(@{$prod_obj->components}) == 1) {
     # Only one component; just pick it.
-    $cgi->param('component', $::components{$product}->[0]);
+    $cgi->param('component', $prod_obj->components->[0]->name);
 }
 
 my @components;
@@ -464,7 +463,7 @@ trick_taint($product);
 
 # Get list of milestones.
 if ( Param('usetargetmilestone') ) {
-    $vars->{'target_milestone'} = $::target_milestone{$product};
+    $vars->{'target_milestone'} = [map($_->name, @{$prod_obj->milestones})];
     if (formvalue('target_milestone')) {
        $default{'target_milestone'} = formvalue('target_milestone');
     } else {
diff --git a/globals.pl b/globals.pl
index 8ac08846a..5d7c9e250 100644
--- a/globals.pl
+++ b/globals.pl
@@ -38,20 +38,17 @@ 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.
 
 sub globals_pl_sillyness {
     my $zz;
-    $zz = @main::enterable_products;
     $zz = @main::legal_bug_status;
     $zz = @main::legal_opsys;
     $zz = @main::legal_platform;
     $zz = @main::legal_priority;
     $zz = @main::legal_severity;
-    $zz = @main::milestoneurl;
     $zz = @main::prodmaxvotes;
 }
 
@@ -103,50 +100,10 @@ sub GenerateVersionTable {
     my $dbh = Bugzilla->dbh;
 
     my @line;
-    SendSQL("SELECT components.name, products.name " .
-            "FROM components, products " .
-            "WHERE products.id = components.product_id " .
-            "ORDER BY components.name");
-    while (@line = FetchSQLData()) {
-        my ($c,$p) = (@line);
-        if (!defined $::components{$p}) {
-            $::components{$p} = [];
-        }
-        my $ref = $::components{$p};
-        push @$ref, $c;
-    }
-
-    SendSQL("SELECT products.name, classifications.name " .
-            "FROM products, classifications " .
-            "WHERE classifications.id = products.classification_id " .
-            "ORDER BY classifications.name");
-    while (@line = FetchSQLData()) {
-        my ($p,$c) = (@line);
-        if (!defined $::classifications{$c}) {
-            $::classifications{$c} = [];
-        }
-        my $ref = $::classifications{$c};
-        push @$ref, $p;
-    }
-
-    my $dotargetmilestone = 1;  # This used to check the param, but there's
-                                # enough code that wants to pretend we're using
-                                # target milestones, even if they don't get
-                                # shown to the user.  So we cache all the data
-                                # about them anyway.
-
-    my $mpart = $dotargetmilestone ? ", milestoneurl" : "";
-
-    SendSQL("SELECT name, votesperuser, disallownew$mpart " .
+    SendSQL("SELECT name, votesperuser " .
             "FROM products ORDER BY name");
     while (@line = FetchSQLData()) {
-        my ($p, $votesperuser, $dis, $u) = (@line);
-        if (!$dis && scalar($::components{$p})) {
-            push @::enterable_products, $p;
-        }
-        if ($dotargetmilestone) {
-            $::milestoneurl{$p} = $u;
-        }
+        my ($p, $votesperuser) = (@line);
         $::prodmaxvotes{$p} = $votesperuser;
     }
             
@@ -197,15 +154,6 @@ sub GenerateVersionTable {
     print $fh (Data::Dumper->Dump([\@::log_columns],
                                   ['*::log_columns']));
 
-    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']));
-
     print $fh (Data::Dumper->Dump([\@::legal_priority, \@::legal_severity,
                                    \@::legal_platform, \@::legal_opsys,
                                    \@::legal_bug_status, \@::legal_resolution],
@@ -213,33 +161,8 @@ sub GenerateVersionTable {
                                    '*::legal_platform', '*::legal_opsys',
                                    '*::legal_bug_status', '*::legal_resolution']));
 
-    print $fh (Data::Dumper->Dump([\@::settable_resolution,
-                                   \%::classifications,
-                                   \@::enterable_products, \%::prodmaxvotes],
-                                  ['*::settable_resolution',
-                                   '*::classifications',
-                                   '*::enterable_products', '*::prodmaxvotes']));
-
-    if ($dotargetmilestone) {
-        # reading target milestones in from the database - matthew@zeroknowledge.com
-        SendSQL("SELECT milestones.value, products.name " .
-                "FROM milestones, products " .
-                "WHERE products.id = milestones.product_id " .
-                "ORDER BY milestones.sortkey, milestones.value");
-        my @line;
-        while(@line = FetchSQLData()) {
-            my ($tm, $pr) = (@line);
-            if (!defined $::target_milestone{$pr}) {
-                $::target_milestone{$pr} = [];
-            }
-            push @{$::target_milestone{$pr}}, $tm;
-        }
-
-        print $fh (Data::Dumper->Dump([\%::target_milestone,
-                                       \%::milestoneurl],
-                                      ['*::target_milestone',
-                                       '*::milestoneurl']));
-    }
+    print $fh (Data::Dumper->Dump([\@::settable_resolution, \%::prodmaxvotes],
+                                  ['*::settable_resolution', '*::prodmaxvotes']));
 
     print $fh "1;\n";
     close $fh;
diff --git a/post_bug.cgi b/post_bug.cgi
index dd699e668..4fb66d316 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -40,12 +40,10 @@ use Bugzilla::Keyword;
 # "use vars" chokes on me when I try it here.
 sub sillyness {
     my $zz;
-    $zz = %::components;
     $zz = @::legal_opsys;
     $zz = @::legal_platform;
     $zz = @::legal_priority;
     $zz = @::legal_severity;
-    $zz = %::target_milestone;
 }
 
 my $user = Bugzilla->login(LOGIN_REQUIRED);
@@ -227,9 +225,10 @@ check_field('op_sys',       scalar $cgi->param('op_sys'),       \@::legal_opsys)
 check_field('bug_status',   scalar $cgi->param('bug_status'),   ['UNCONFIRMED', 'NEW']);
 check_field('version',      scalar $cgi->param('version'),
             [map($_->name, @{$prod_obj->versions})]);
-check_field('component',    scalar $cgi->param('component'),    $::components{$product});
+check_field('component',    scalar $cgi->param('component'),
+            [map($_->name, @{$prod_obj->components})]);
 check_field('target_milestone', scalar $cgi->param('target_milestone'),
-            $::target_milestone{$product});
+            [map($_->name, @{$prod_obj->milestones})]);
 
 foreach my $field_name ('assigned_to', 'bug_file_loc', 'comment') {
     defined($cgi->param($field_name))
diff --git a/process_bug.cgi b/process_bug.cgi
index c1f3b9b90..01b45e969 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -65,13 +65,10 @@ use Bugzilla::FlagType;
 
 # Shut up misguided -w warnings about "used only once":
 
-use vars qw(@legal_product
-          %components
-          %legal_opsys
+use vars qw(%legal_opsys
           %legal_platform
           %legal_priority
           %settable_resolution
-          %target_milestone
           %legal_severity
            );
 
@@ -339,16 +336,18 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct)
     # worthy of a comment, perhaps.
     #
     my @version_names = map($_->name, @{$prod_obj->versions});
+    my @component_names = map($_->name, @{$prod_obj->components});
     my $vok = lsearch(\@version_names, $cgi->param('version')) >= 0;
-    my $cok = lsearch($::components{$prod}, $cgi->param('component')) >= 0;
+    my $cok = lsearch(\@component_names, $cgi->param('component')) >= 0;
 
     my $mok = 1;   # so it won't affect the 'if' statement if milestones aren't used
+    my @milestone_names = ();
     if ( Param("usetargetmilestone") ) {
        defined($cgi->param('target_milestone'))
          || ThrowCodeError('undefined_field', { field => 'target_milestone' });
 
-       $mok = lsearch($::target_milestone{$prod},
-                      $cgi->param('target_milestone')) >= 0;
+       @milestone_names = map($_->name, @{$prod_obj->milestones});
+       $mok = lsearch(\@milestone_names, $cgi->param('target_milestone')) >= 0;
     }
 
     # If the product-specific fields need to be verified, or we need to verify
@@ -367,13 +366,13 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct)
             if ($vok) {
                 $defaults{'version'} = $cgi->param('version');
             }
-            $vars->{'components'} = $::components{$prod};
+            $vars->{'components'} = \@component_names;
             if ($cok) {
                 $defaults{'component'} = $cgi->param('component');
             }
             if (Param("usetargetmilestone")) {
                 $vars->{'use_target_milestone'} = 1;
-                $vars->{'milestones'} = $::target_milestone{$prod};
+                $vars->{'milestones'} = \@milestone_names;
                 if ($mok) {
                     $defaults{'target_milestone'} = $cgi->param('target_milestone');
                 } else {
@@ -611,12 +610,12 @@ if (defined $cgi->param('id')) {
     # is more work in the current architecture of this script...)
     my $prod_obj = Bugzilla::Product::check_product($cgi->param('product'));
     check_field('component', scalar $cgi->param('component'), 
-                \@{$::components{$cgi->param('product')}});
+                [map($_->name, @{$prod_obj->components})]);
     check_field('version', scalar $cgi->param('version'),
                 [map($_->name, @{$prod_obj->versions})]);
     if ( Param("usetargetmilestone") ) {
         check_field('target_milestone', scalar $cgi->param('target_milestone'), 
-                    \@{$::target_milestone{$cgi->param('product')}});
+                    [map($_->name, @{$prod_obj->milestones})]);
     }
     check_field('rep_platform', scalar $cgi->param('rep_platform'), \@::legal_platform);
     check_field('op_sys',       scalar $cgi->param('op_sys'),       \@::legal_opsys);
@@ -1473,7 +1472,7 @@ foreach my $id (@idlist) {
                                 # whether we do LOW_PRIORITY ...
     $dbh->bz_lock_tables("bugs $write", "bugs_activity $write", "cc $write",
             "profiles READ", "dependencies $write", "votes $write",
-            "products READ", "components READ",
+            "products READ", "components READ", "milestones READ",
             "keywords $write", "longdescs $write", "fielddefs READ",
             "bug_group_map $write", "flags $write", "duplicates $write",
             "user_group_map READ", "group_group_map READ", "flagtypes READ",
@@ -1568,7 +1567,8 @@ foreach my $id (@idlist) {
     if ($requiremilestone) {
         # musthavemilestoneonaccept applies only if at least two
         # target milestones are defined for the current product.
-        my $nb_milestones = scalar(@{$::target_milestone{$oldhash{'product'}}});
+        my $prod_obj = new Bugzilla::Product({'name' => $oldhash{'product'}});
+        my $nb_milestones = scalar(@{$prod_obj->milestones});
         if ($nb_milestones > 1) {
             my $value = $cgi->param('target_milestone');
             if (!defined $value || $value eq $cgi->param('dontchange')) {
diff --git a/reports.cgi b/reports.cgi
index 5553f2c97..947993a56 100755
--- a/reports.cgi
+++ b/reports.cgi
@@ -40,7 +40,6 @@ use lib qw(.);
 use Bugzilla::Config qw(:DEFAULT $datadir);
 
 require "globals.pl";
-use vars qw(@legal_product); # globals from er, globals.pl
 
 eval "use GD";
 $@ && ThrowCodeError("gd_not_installed");