Bug 286158: Remove GetSelectableProducts() from globals.pl and use Bugzilla::User::get_selectable_products() instead - Patch by Frédéric Buclin <LpSolit@gmail.com> r=joel,kiko a=justdave

3 files changed, 46 insertions, 57 deletions

diff --git a/Bugzilla/Component.pm b/Bugzilla/Component.pm
index a3278dea9..dfbcf00a8 100644
--- a/Bugzilla/Component.pm
+++ b/Bugzilla/Component.pm
@@ -121,11 +121,11 @@ sub get_components_by_product {
         SELECT id FROM components
         WHERE product_id = ?}, undef, $product_id);
 
-    my $components;
+    my @components;
     foreach my $id (@$ids) {
-        $components->{$id} = new Bugzilla::Component($id);
+        push @components, new Bugzilla::Component($id);
     }
-    return $components;
+    return @components;
 }
 
 1;
@@ -151,8 +151,7 @@ Bugzilla::Component - Bugzilla product component class.
     my $default_assignee   = $component->default_assignee;
     my $default_qa_contact = $component->default_qa_contact;
 
-    my $hash_ref = Bugzilla::Component::get_components_by_product(1);
-    my $component = $hash_ref->{1};
+    my @components = Bugzilla::Component::get_components_by_product($id);
 
 =head1 DESCRIPTION
 
@@ -184,13 +183,11 @@ Component.pm represents a Product Component object.
 
 =item C<get_components_by_product($product_id)>
 
- Description: Returns all Bugzilla components that belong to the
-              supplied product.
+ Description: Returns all components that belong to the supplied product.
 
  Params:      $product_id - Integer with a Bugzilla product id.
 
- Returns:     A hash with component id as key and Bugzilla::Component
-              object as value.
+ Returns:     An array of Bugzilla::Component objects.
 
 =back
 
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index c257bd4ce..514620258 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -98,8 +98,9 @@ sub components {
     my $self = shift;
 
     if (!defined $self->{components}) {
-        $self->{components} =
+        my @components =
             Bugzilla::Component::get_components_by_product($self->id);
+        $self->{components} = \@components;
     }
     return $self->{components};
 }
@@ -247,11 +248,11 @@ Bugzilla::Product - Bugzilla product class.
     my $product = new Bugzilla::Product(1);
     my $product = new Bugzilla::Product('AcmeProduct');
 
-    my $components      = $product->components();
+    my @components      = $product->components();
     my $classification  = $product->classification();
-    my $hash_ref        = $product->group_controls();
-    my @array_ref       = $product->milestones();
-    my @array_ref       = $product->versions();
+    my $groups_controls = $product->group_controls();
+    my @milestones      = $product->milestones();
+    my @versions        = $product->versions();
     my $bugcount        = $product->bug_count();
 
     my $id               = $product->id;
@@ -290,12 +291,12 @@ Product.pm represents a product object.
 
 =item C<components()>
 
- Description: Returns a hash with all product components.
+ Description: Returns an array of component objects belonging to
+              the product.
 
  Params:      none.
 
- Returns:     A hash where component id is the hash key and
-              Bugzilla::Component object is the hash value.
+ Returns:     An array of Bugzilla::Component object.
 
 =item C<classification()>
 
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 87f894752..2c6c6b0b5 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -61,8 +61,6 @@ use constant USER_MATCH_SUCCESS  => 1;
 
 use constant MATCH_SKIP_CONFIRM  => 1;
 
-use constant GET_PRODUCTS_BY_ID => 1;
-
 ################################################################################
 # Functions
 ################################################################################
@@ -420,13 +418,12 @@ sub can_see_bug {
 sub get_selectable_products {
     my ($self, $by_id) = @_;
 
-    if (defined $self->{SelectableProducts}) {
-        my %list = @{$self->{SelectableProducts}};
-        return \%list if $by_id;
-        return values(%list);
+    if (defined $self->{selectable_products}) {
+        return $self->{selectable_products};
     }
 
-    my $query = "SELECT id, name " .
+    my $dbh = Bugzilla->dbh;
+    my $query = "SELECT id " .
                 "FROM products " .
                 "LEFT JOIN group_control_map " .
                 "ON group_control_map.product_id = products.id ";
@@ -439,38 +436,31 @@ sub get_selectable_products {
     $query .= "AND group_id NOT IN(" . 
                $self->groups_as_string . ") " .
               "WHERE group_id IS NULL ORDER BY name";
-    my $dbh = Bugzilla->dbh;
-    my $sth = $dbh->prepare($query);
-    $sth->execute();
-    my @products = ();
-    while (my @row = $sth->fetchrow_array) {
-        push(@products, @row);
+
+    my $prod_ids = $dbh->selectcol_arrayref($query);
+    my @products;
+    foreach my $prod_id (@$prod_ids) {
+        push(@products, new Bugzilla::Product($prod_id));
     }
-    $self->{SelectableProducts} = \@products;
-    my %list = @products;
-    return \%list if $by_id;
-    return values(%list);
+    $self->{selectable_products} = \@products;
+    return $self->{selectable_products};
 }
 
-sub get_selectable_classifications ($) {
+sub get_selectable_classifications {
     my ($self) = @_;
 
     if (defined $self->{selectable_classifications}) {
         return $self->{selectable_classifications};
     }
- 
-    my $products = $self->get_selectable_products(GET_PRODUCTS_BY_ID);
-    
-    my $selectable_classifications;
-   
-    foreach my $prod_id (keys %$products) {
-        my $product = new Bugzilla::Product($prod_id);
-        
-        $selectable_classifications->{$product->classification_id} =
-            $product->classification;
+
+    my $products = $self->get_selectable_products;
+
+    my $class;
+    foreach my $product (@$products) {
+        $class->{$product->classification_id} ||= $product->classification;
     }
-    $self->{selectable_classifications} = 
-        [values %$selectable_classifications];
+    my @sorted_class = sort {lc($a->name) cmp lc($b->name)} (values %$class);
+    $self->{selectable_classifications} = \@sorted_class;
     return $self->{selectable_classifications};
 }
 
@@ -1450,22 +1440,23 @@ care of by the constructor. However, when updating the email address, the
 user may be placed into different groups, based on a new email regexp. This
 method should be called in such a case to force reresolution of these groups.
 
-=item C<get_selectable_products(by_id)>
+=item C<get_selectable_products>
+
+ Description: Returns all products the user is allowed to access.
+
+ Params:      none
 
-Returns an alphabetical list of product names from which
-the user can select bugs.  If the $by_id parameter is true, it returns
-a hash where the keys are the product ids and the values are the
-product names.
+ Returns:     An array of product objects, sorted by the product name.
 
 =item C<get_selectable_classifications>
 
- Description: Returns the classifications that a user, according his
-              groups ownership, can select to entering, serch, view or
-              edit a bug.
+ Description: Returns all classifications containing at least one product
+              the user is allowed to view.
 
- Params:      none.
+ Params:      none
 
- Returns:     Bugzilla::Classification objects values.
+ Returns:     An array of Bugzilla::Classification objects, sorted by
+              the classification name.
 
 =item C<get_userlist>