Bug 306325: Move CanEnterProduct() and CanEnterProductOrWarn() out of globals.pl - Patch by Frédéric Buclin <LpSolit@gmail.com> r=joel r=wicked a=justdave

9 files changed, 117 insertions, 106 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index e050d2211..526f002b0 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -650,7 +650,7 @@ sub choices {
             next;
         }
 
-        if (!&::CanEnterProduct($product)) {
+        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.
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 3fca325b6..a20f2e338 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -428,7 +428,7 @@ sub can_see_product {
 }
 
 sub get_selectable_products {
-    my ($self, $by_id) = @_;
+    my $self = shift;
 
     if (defined $self->{selectable_products}) {
         return $self->{selectable_products};
@@ -476,6 +476,81 @@ sub get_selectable_classifications {
     return $self->{selectable_classifications};
 }
 
+sub can_enter_product {
+    my ($self, $product_name, $warn) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    if (!defined($product_name)) {
+        return unless $warn;
+        ThrowUserError('no_products');
+    }
+    trick_taint($product_name);
+
+    # Checks whether the user has access to the product.
+    my $has_access = $dbh->selectrow_array('SELECT group_id IS NULL
+                                              FROM products
+                                         LEFT JOIN group_control_map
+                                                ON group_control_map.product_id = products.id
+                                               AND group_control_map.entry != 0
+                                               AND group_id NOT IN (' . $self->groups_as_string . ')
+                                             WHERE products.name = ? ' .
+                                             $dbh->sql_limit(1),
+                                            undef, $product_name);
+
+    if (!$has_access) {
+        return unless $warn;
+        ThrowUserError('entry_access_denied', { product => $product_name });
+    }
+
+    # Checks whether the product is open for new bugs and
+    # has at least one component and one version.
+    my ($is_open, $has_version) = 
+        $dbh->selectrow_array('SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0 END,
+                                      versions.value IS NOT NULL
+                                 FROM products
+                           INNER JOIN components
+                                   ON components.product_id = products.id
+                            LEFT JOIN versions
+                                   ON versions.product_id = products.id
+                                WHERE products.name = ? ' .
+                               $dbh->sql_limit(1), undef, $product_name);
+
+    # Returns undef if the product has no components
+    # Returns 0 if the product has no versions, or is closed for bug entry
+    # Returns 1 if the user can enter bugs into the product
+    return ($is_open && $has_version) unless $warn;
+
+    # (undef, undef): the product has no components,
+    # (0,     ?)    : the product is closed for new bug entry,
+    # (?,     0)    : the product has no versions,
+    # (1,     1)    : the user can enter bugs into the product,
+    if (!defined $is_open) {
+        ThrowUserError('missing_component', { product => $product_name });
+    } elsif (!$is_open) {
+        ThrowUserError('product_disabled', { product => $product_name });
+    } elsif (!$has_version) {
+        ThrowUserError('missing_version', { product => $product_name });
+    }
+    return 1;
+}
+
+sub get_enterable_products {
+    my $self = shift;
+
+    if (defined $self->{enterable_products}) {
+        return $self->{enterable_products};
+    }
+
+    my @products;
+    foreach my $product (Bugzilla::Product::get_all_products()) {
+        if ($self->can_enter_product($product->name)) {
+            push(@products, $product);
+        }
+    }
+    $self->{enterable_products} = \@products;
+    return $self->{enterable_products};
+}
+
 # visible_groups_inherited returns a reference to a list of all the groups
 # whose members are visible to this user.
 sub visible_groups_inherited {
@@ -1493,6 +1568,32 @@ method should be called in such a case to force reresolution of these groups.
  Returns:     An array of Bugzilla::Classification objects, sorted by
               the classification name.
 
+=item C<can_enter_product($product_name, $warn)>
+
+ Description: Returns 1 if the user can enter bugs into the specified product.
+              If the user cannot enter bugs into the product, the behavior of
+              this method depends on the value of $warn:
+              - if $warn is false (or not given), a 'false' value is returned;
+              - if $warn is true, an error is thrown.
+
+ Params:      $product_name - a product name.
+              $warn         - optional parameter, indicating whether an error
+                              must be thrown if the user cannot enter bugs
+                              into the specified product.
+
+ Returns:     1 if the user can enter bugs into the product,
+              0 if the user cannot enter bugs into the product and if $warn
+              is false (an error is thrown if $warn is true).
+
+=item C<get_enterable_products>
+
+ Description: Returns an array of product objects into which the user is
+              allowed to enter bugs.
+
+ Params:      none
+
+ Returns:     an array of product objects.
+
 =item C<get_userlist>
 
 Returns a reference to an array of users.  The array is populated with hashrefs
diff --git a/buglist.cgi b/buglist.cgi
index b832d82c7..11835c3a6 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -988,8 +988,7 @@ if ($dotweak) {
     $vars->{'dotweak'} = 1;
     $vars->{'use_keywords'} = 1 if @::legal_keywords;
 
-    my @enterable_products = GetEnterableProducts();
-    $vars->{'products'} = \@enterable_products;
+    $vars->{'products'} = Bugzilla->user->get_enterable_products;
     $vars->{'platforms'} = \@::legal_platform;
     $vars->{'op_sys'} = \@::legal_opsys;
     $vars->{'priorities'} = \@::legal_priority;
diff --git a/describecomponents.cgi b/describecomponents.cgi
index 9602c0fe0..8fec1ec26 100755
--- a/describecomponents.cgi
+++ b/describecomponents.cgi
@@ -30,7 +30,7 @@ require "globals.pl";
 
 use vars qw($vars @legal_product);
 
-Bugzilla->login();
+my $user = Bugzilla->login();
 
 GetVersionTable();
 
@@ -39,7 +39,7 @@ my $template = Bugzilla->template;
 my $product = trim($cgi->param('product') || '');
 my $product_id = get_product_id($product);
 
-if (!$product_id || !CanEnterProduct($product)) {
+if (!$product_id || !$user->can_enter_product($product)) {
     # Reference to a subset of %::proddesc, which the user is allowed to see
     my %products;
 
@@ -47,7 +47,7 @@ if (!$product_id || !CanEnterProduct($product)) {
         # OK, now only add products the user can see
         Bugzilla->login(LOGIN_REQUIRED);
         foreach my $p (@::legal_product) {
-            if (CanEnterProduct($p)) {
+            if ($user->can_enter_product($p)) {
                 $products{$p} = $::proddesc{$p};
             }
         }
diff --git a/enter_bug.cgi b/enter_bug.cgi
index 1540209fe..e2772ccec 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -84,7 +84,7 @@ if (!defined $product || $product eq "") {
         foreach my $classification (@$classifications) {
             my $found = 0;
             foreach my $p (@enterable_products) {
-               if (CanEnterProduct($p)
+               if (Bugzilla->user->can_enter_product($p)
                    && IsInClassification($classification->{name},$p)) {
                        $found = 1; 
                }
@@ -116,7 +116,7 @@ if (!defined $product || $product eq "") {
 
     my %products;
     foreach my $p (@enterable_products) {
-        if (CanEnterProduct($p)) {
+        if (Bugzilla->user->can_enter_product($p)) {
             if (IsInClassification(scalar $cgi->param('classification'),$p) ||
                 $cgi->param('classification') eq "__all") {
                 $products{$p} = $::proddesc{$p};
@@ -328,7 +328,7 @@ if ($cloned_bug_id) {
 
 # We need to check and make sure
 # that the user has permission to enter a bug against this product.
-CanEnterProductOrWarn($product);
+Bugzilla->user->can_enter_product($product, 1);
 
 GetVersionTable();
 
diff --git a/globals.pl b/globals.pl
index 4d21b088b..07d12a984 100644
--- a/globals.pl
+++ b/globals.pl
@@ -411,98 +411,6 @@ sub IsInClassification {
     }
 }
 
-# This function determines whether or not a user can enter
-# bugs into the named product.
-sub CanEnterProduct {
-    my ($productname, $verbose) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    return unless defined($productname);
-    trick_taint($productname);
-
-    # First check whether or not the user has access to that product.
-    my $query = "SELECT group_id IS NULL " .
-                "FROM products " .
-                "LEFT JOIN group_control_map " .
-                "ON group_control_map.product_id = products.id " .
-                "AND group_control_map.entry != 0 ";
-    if (%{Bugzilla->user->groups}) {
-        $query .= "AND group_id NOT IN(" . 
-                   join(',', values(%{Bugzilla->user->groups})) . ") ";
-    }
-    $query .= "WHERE products.name = ? " .
-              $dbh->sql_limit(1);
-
-    my $has_access = $dbh->selectrow_array($query, undef, $productname);
-    if (!$has_access) {
-        # Do we require the exact reason why we cannot enter
-        # bugs into that product? Returning -1 explicitely
-        # means the user has no access to the product or the
-        # product does not exist.
-        return (defined($verbose)) ? -1 : 0;
-    }
-
-    # Check if the product is open for new bugs and has
-    # at least one component and has at least one version.
-    my ($allow_new_bugs, $has_version) = 
-        $dbh->selectrow_array('SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0 END, ' .
-                              'versions.value IS NOT NULL ' .
-                              'FROM products INNER JOIN components ' .
-                              'ON components.product_id = products.id ' .
-                              'LEFT JOIN versions ' .
-                              'ON versions.product_id = products.id ' .
-                              'WHERE products.name = ? ' .
-                              $dbh->sql_limit(1), undef, $productname);
-
-
-    if (defined $verbose) {
-        # Return (undef, undef) if the product has no components,
-        # Return (?,     0)     if the product has no versions,
-        # Return (0,     ?)     if the product is closed for new bug entry,
-        # Return (1,     1)     if the user can enter bugs into the product,
-        return ($allow_new_bugs, $has_version);
-    } else {
-        # Return undef if the product has no components
-        # Return 0 if the product has no versions, or is closed for bug entry
-        # Return 1 if the user can enter bugs into the product
-        return ($allow_new_bugs && $has_version);
-    }
-}
-
-# Call CanEnterProduct() and display an error message
-# if the user cannot enter bugs into that product.
-sub CanEnterProductOrWarn {
-    my ($product) = @_;
-
-    if (!defined($product)) {
-        ThrowUserError("no_products");
-    }
-    my ($allow_new_bugs, $has_version) = CanEnterProduct($product, 1);
-    trick_taint($product);
-
-    if (!defined $allow_new_bugs) {
-        ThrowUserError("missing_component", { product => $product });
-    } elsif (!$allow_new_bugs) {
-        ThrowUserError("product_disabled", { product => $product});
-    } elsif ($allow_new_bugs < 0) {
-        ThrowUserError("entry_access_denied", { product => $product});
-    } elsif (!$has_version) {
-        ThrowUserError("missing_version", { product => $product });
-    }
-    return 1;
-}
-
-sub GetEnterableProducts {
-    my @products;
-    # XXX rewrite into pure SQL instead of relying on legal_products?
-    foreach my $p (@::legal_product) {
-        if (CanEnterProduct($p)) {
-            push @products, $p;
-        }
-    }
-    return (@products);
-}
-
 sub ValidatePassword {
     # Determines whether or not a password is valid (i.e. meets Bugzilla's
     # requirements for length and content).    
diff --git a/post_bug.cgi b/post_bug.cgi
index f32ef9b14..176b42d71 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -78,7 +78,7 @@ ValidateComment($comment);
 # Check that the product exists and that the user
 # is allowed to enter bugs into this product.
 my $product = $cgi->param('product');
-CanEnterProductOrWarn($product);
+$user->can_enter_product($product, 1);
 
 my $product_id = get_product_id($product);
 
diff --git a/process_bug.cgi b/process_bug.cgi
index af3ebc977..9362af4a8 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -279,7 +279,7 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct)
                                $dbh->sql_limit(1),
                                undef, $prod);
 
-    if ($check_can_enter) { CanEnterProductOrWarn($prod) }
+    if ($check_can_enter) { $user->can_enter_product($prod, 1) }
 
     # note that when this script is called from buglist.cgi (rather
     # than show_bug.cgi), it's possible that the product will be changed
diff --git a/template/en/default/list/edit-multiple.html.tmpl b/template/en/default/list/edit-multiple.html.tmpl
index 92754387d..d55a4e2db 100644
--- a/template/en/default/list/edit-multiple.html.tmpl
+++ b/template/en/default/list/edit-multiple.html.tmpl
@@ -55,13 +55,15 @@
     <th><label for="product">Product:</label></th>
     <td>
       [% PROCESS selectmenu menuname = "product"
-                            menuitems = products %]
+                            menuitems = products
+                            property = "name" %]
     </td>
 
     <th><label for="version">Version:</label></th>
     <td>
       [% PROCESS selectmenu menuname = "version"
-                            menuitems = versions %]
+                            menuitems = versions
+                            property = "" %]
     </td>
 
   </tr>
@@ -358,6 +360,7 @@
       [% dontchange FILTER html %]
     </option>
     [% FOREACH menuitem = menuitems %]
+      [% IF property %][% menuitem = menuitem.$property %][% END %]
       <option value="[% menuitem FILTER html %]">[% menuitem FILTER html %]</option>
     [% END %]
   </select>