Bug 338796: Remove get_product_* from globals.pl - Patch by Frédéric Buclin…

Bugzilla/BugMail.pm

@@ -38,6 +38,7 @@ use Bugzilla::Constants;
 use Bugzilla::Config qw(:DEFAULT $datadir);
 use Bugzilla::Util;
 use Bugzilla::Bug;
+use Bugzilla::Product;
 use Bugzilla::Component;
 use Bugzilla::Mailer;
 
@@ -135,8 +136,9 @@ sub ProcessOneBug {
                 lastdiffed AS start, LOCALTIMESTAMP(0) AS end
            FROM bugs WHERE bug_id = ?',
         undef, $id)};
-    
-    $values{product} = &::get_product_name($values{product_id});
+
+    my $product = new Bugzilla::Product($values{product_id});
+    $values{product} = $product->name;
     my $component = new Bugzilla::Component($values{component_id});
     $values{component} = $component->name;
 

collectstats.pl

@@ -54,6 +54,8 @@ if (chdir("graphs")) {
 
 GetVersionTable();
 
+# Let Throw*Error() work correctly outside a web browser.
+Bugzilla->batch(1);
 Bugzilla->switch_to_shadow_db();
 
 # To recreate the daily statistics,  run "collectstats.pl --regenerate" .
@@ -125,10 +127,13 @@ sub collect_stats {
     my $dir = shift;
     my $product = shift;
     my $when = localtime (time);
-    my $product_id = get_product_id($product) unless $product eq '-All-';
     my $dbh = Bugzilla->dbh;
 
-    die "Unknown product $product" unless ($product_id or $product eq '-All-');
+    my $product_id;
+    if ($product ne '-All-') {
+        my $prod = Bugzilla::Product::check_product($product);
+        $product_id = $prod->id;
+    }
 
     # NB: Need to mangle the product for the filename, but use the real
     # product name in the query

duplicates.cgi

@@ -35,6 +35,7 @@ use Bugzilla;
 use Bugzilla::Search;
 use Bugzilla::Config qw(:DEFAULT $datadir);
 use Bugzilla::Constants;
+use Bugzilla::Product;
 
 my $cgi = Bugzilla->cgi;
 
@@ -86,13 +87,9 @@ my @query_products = $cgi->param('product');
 my $sortvisible = formvalue("sortvisible");
 my @buglist = (split(/[:,]/, formvalue("bug_id")));
 
-my $product_id;
+# Make sure all products are valid.
 foreach my $p (@query_products) {
-    $product_id = get_product_id($p);
-    if (!$product_id) {
-        ThrowUserError("invalid_product_name",
-                       { product => $p });
-    }
+    Bugzilla::Product::check_product($p);
 }
 
 # Small backwards-compatibility hack, dated 2002-04-10.

editflagtypes.cgi

@@ -278,15 +278,20 @@ sub processCategoryChange {
 sub clusion_array_to_hash {
     my $array = shift;
     my %hash;
+    my %products;
     my %components;
     foreach my $ids (@$array) {
         trick_taint($ids);
         my ($product_id, $component_id) = split(":", $ids);
-        my $product_name = get_product_name($product_id) || "__Any__";
+        my $product_name = "__Any__";
+        if ($product_id) {
+            $products{$product_id} ||= new Bugzilla::Product($product_id);
+            $product_name = $products{$product_id}->name if $products{$product_id};
+        }
         my $component_name = "__Any__";
         if ($component_id) {
             $components{$component_id} ||= new Bugzilla::Component($component_id);
-            $component_name = $components{$component_id}->name;
+            $component_name = $components{$component_id}->name if $components{$component_id};
         }
         $hash{"$product_name:$component_name"} = $ids;
     }
@@ -656,6 +661,8 @@ sub validateAndSubmit {
     my ($id) = @_;
     my $dbh = Bugzilla->dbh;
 
+    # Cache product objects.
+    my %products;
     foreach my $category_type ("inclusions", "exclusions") {
         # Will be used several times below.
         my $sth = $dbh->prepare("INSERT INTO flag$category_type " .
@@ -666,15 +673,18 @@ sub validateAndSubmit {
         foreach my $category ($cgi->param($category_type)) {
             trick_taint($category);
             my ($product_id, $component_id) = split(":", $category);
-            # The product does not exist.
-            next if ($product_id && !get_product_name($product_id));
+            # Does the product exist?
+            if ($product_id) {
+                $products{$product_id} ||= new Bugzilla::Product($product_id);
+                next unless defined $products{$product_id};
+            }
             # A component was selected without a product being selected.
             next if (!$product_id && $component_id);
-            # The component does not belong to this product.
-            next if ($component_id
-                     && !$dbh->selectrow_array("SELECT id FROM components
-                                                WHERE id = ? AND product_id = ?",
-                                                undef, ($component_id, $product_id)));
+            # Does the component belong to this product?
+            if ($component_id) {
+                my @match = grep {$_->id == $component_id} @{$products{$product_id}->components};
+                next unless scalar(@match);
+            }
             $product_id ||= undef;
             $component_id ||= undef;
             $sth->execute($id, $product_id, $component_id);

editgroups.cgi

@@ -33,6 +33,7 @@ use Bugzilla;
 use Bugzilla::Constants;
 use Bugzilla::Config qw(:DEFAULT :admin);
 use Bugzilla::Group;
+use Bugzilla::Product;
 use Bugzilla::User;
 require "globals.pl";
 
@@ -364,7 +365,7 @@ if ($action eq 'del') {
     my $hasbugs = scalar(@$bug_ids) ? 1 : 0;
     my $buglist = join(',', @$bug_ids);
 
-    my $hasproduct = get_product_id($name) ? 1 : 0;
+    my $hasproduct = Bugzilla::Product->new({'name' => $name}) ? 1 : 0;
 
     my $hasflags = $dbh->selectrow_array('SELECT 1 FROM flagtypes 
                                            WHERE grant_group_id = ?
@@ -436,7 +437,9 @@ if ($action eq 'delete') {
         $cantdelete = 1;
     }
 
-    if (get_product_id($name) && !defined $cgi->param('unbind')) {
+    if (Bugzilla::Product->new({'name' => $name})
+        && !defined $cgi->param('unbind'))
+    {
         $cantdelete = 1;
     }
 

enter_bug.cgi

@@ -61,12 +61,15 @@ my $cloned_bug;
 my $cloned_bug_id;
 
 my $cgi = Bugzilla->cgi;
+my $dbh = Bugzilla->dbh;
 my $template = Bugzilla->template;
 my $vars = {};
 
-my $product = trim($cgi->param('product') || '');
+my $product_name = trim($cgi->param('product') || '');
+# Will contain the product object the bug is created in.
+my $product;
 
-if ($product eq '') {
+if ($product_name eq '') {
     # If the user cannot enter bugs in any product, stop here.
     my @enterable_products = @{$user->get_enterable_products};
     ThrowUserError('no_products') unless scalar(@enterable_products);
@@ -77,7 +80,7 @@ if ($product eq '') {
     unless ($classification) {
         my $class;
         # Get all classifications with at least one enterable product.
-        foreach $product (@enterable_products) {
+        foreach my $product (@enterable_products) {
             $class->{$product->classification_id} ||=
                 new Bugzilla::Classification($product->classification_id);
         }
@@ -129,9 +132,18 @@ if ($product eq '') {
         exit;
     } else {
         # Only one product exists.
-        $product = $enterable_products[0]->name;
+        $product = $enterable_products[0];
     }
 }
+else {
+    # Do not use Bugzilla::Product::check_product() here, else the user
+    # could know whether the product doesn't exist or is not accessible.
+    $product = new Bugzilla::Product({'name' => $product_name});
+}
+
+# We need to check and make sure that the user has permission
+# to enter a bug against this product.
+$user->can_enter_product($product ? $product->name : $product_name, THROW_ERROR);
 
 ##############################################################################
 # Useful Subroutines
@@ -300,48 +312,16 @@ if ($cloned_bug_id) {
     $cloned_bug = new Bugzilla::Bug($cloned_bug_id, $user->id);
 }
 
-# We need to check and make sure
-# that the user has permission to enter a bug against this product.
-my $prod_obj = new Bugzilla::Product({name => $product});
-# Update the product name to get the correct case.
-$product = $prod_obj->name if defined $prod_obj;
-$user->can_enter_product($product, 1);
-
 GetVersionTable();
 
-my $product_id = get_product_id($product);
-
-if (scalar(@{$prod_obj->components}) == 1) {
+if (scalar(@{$product->components}) == 1) {
     # Only one component; just pick it.
-    $cgi->param('component', $prod_obj->components->[0]->name);
-}
-
-my @components;
-
-my $dbh = Bugzilla->dbh;
-my $sth = $dbh->prepare(
-       q{SELECT name, description, p1.login_name, p2.login_name 
-           FROM components 
-      LEFT JOIN profiles p1 ON components.initialowner = p1.userid
-      LEFT JOIN profiles p2 ON components.initialqacontact = p2.userid
-          WHERE product_id = ?
-          ORDER BY name});
-
-$sth->execute($product_id);
-while (my ($name, $description, $owner, $qacontact)
-       = $sth->fetchrow_array()) {
-    push @components, {
-        name => $name,
-        description => $description,
-        initialowner => $owner,
-        initialqacontact => $qacontact || '',
-    };
+    $cgi->param('component', $product->components->[0]->name);
 }
 
 my %default;
 
 $vars->{'product'}               = $product;
-$vars->{'component_'}            = \@components;
 
 $vars->{'priority'}              = \@legal_priority;
 $vars->{'bug_severity'}          = \@legal_severity;
@@ -441,32 +421,27 @@ else {
 #
 # Eventually maybe each product should have a "current version"
 # parameter.
-$vars->{'version'} = [map($_->name, @{$prod_obj->versions})];
+$vars->{'version'} = [map($_->name, @{$product->versions})];
 
 if ( ($cloned_bug_id) &&
-     ("$product" eq "$cloned_bug->{'product'}" ) ) {
+     ($product->name eq $cloned_bug->{'product'} ) ) {
     $default{'version'} = $cloned_bug->{'version'};
 } elsif (formvalue('version')) {
     $default{'version'} = formvalue('version');
-} elsif (defined $cgi->cookie("VERSION-$product") &&
-    lsearch($vars->{'version'}, $cgi->cookie("VERSION-$product")) != -1) {
-    $default{'version'} = $cgi->cookie("VERSION-$product");
+} elsif (defined $cgi->cookie("VERSION-" . $product->name) &&
+    lsearch($vars->{'version'}, $cgi->cookie("VERSION-" . $product->name)) != -1) {
+    $default{'version'} = $cgi->cookie("VERSION-" . $product->name);
 } else {
     $default{'version'} = $vars->{'version'}->[$#{$vars->{'version'}}];
 }
 
-# Only used with placeholders below
-trick_taint($product);
-
 # Get list of milestones.
 if ( Param('usetargetmilestone') ) {
-    $vars->{'target_milestone'} = [map($_->name, @{$prod_obj->milestones})];
+    $vars->{'target_milestone'} = [map($_->name, @{$product->milestones})];
     if (formvalue('target_milestone')) {
        $default{'target_milestone'} = formvalue('target_milestone');
     } else {
-       $default{'target_milestone'} =
-                $dbh->selectrow_array('SELECT defaultmilestone FROM products
-                                       WHERE name = ?', undef, $product);
+       $default{'target_milestone'} = $product->default_milestone;
     }
 }
 
@@ -481,9 +456,7 @@ my @status;
 #  confirmation, user cannot confirm    UNCONFIRMED
 #  confirmation, user can confirm       NEW, UNCONFIRMED.
 
-my $votestoconfirm = $dbh->selectrow_array('SELECT votestoconfirm FROM products
-                                            WHERE name = ?', undef, $product);
-if ($votestoconfirm) {
+if ($product->votes_to_confirm) {
     if (UserInGroup("editbugs") || UserInGroup("canconfirm")) {
         push(@status, "NEW");
     }
@@ -510,7 +483,7 @@ my $grouplist = $dbh->selectall_arrayref(
                  LEFT JOIN group_control_map
                         ON group_id = id AND product_id = ?
                      WHERE isbuggroup != 0 AND isactive != 0
-                  ORDER BY description}, undef, $product_id);
+                  ORDER BY description}, undef, $product->id);
 
 my @groups;
 
@@ -535,7 +508,7 @@ foreach my $row (@$grouplist) {
     #   set a groups's checkbox based on the group control map
     #
     if ( ($cloned_bug_id) &&
-         ("$product" eq "$cloned_bug->{'product'}" ) ) {
+         ($product->name eq $cloned_bug->{'product'} ) ) {
         foreach my $i (0..(@{$cloned_bug->{'groups'}}-1) ) {
             if ($cloned_bug->{'groups'}->[$i]->{'bit'} == $id) {
                 $check = $cloned_bug->{'groups'}->[$i]->{'ison'};

globals.pl

@@ -160,26 +160,6 @@ sub DBID_to_name {
     return $::cachedNameArray{$id};
 }
 
-sub get_product_id {
-    my ($prod) = @_;
-    PushGlobalSQLState();
-    SendSQL("SELECT id FROM products WHERE name = " . SqlQuote($prod));
-    my ($prod_id) = FetchSQLData();
-    PopGlobalSQLState();
-    return $prod_id;
-}
-
-sub get_product_name {
-    my ($prod_id) = @_;
-    die "non-numeric prod_id '$prod_id' passed to get_product_name"
-      unless ($prod_id =~ /^\d+$/);
-    PushGlobalSQLState();
-    SendSQL("SELECT name FROM products WHERE id = $prod_id");
-    my ($prod) = FetchSQLData();
-    PopGlobalSQLState();
-    return $prod;
-}
-
 # Returns a list of all the legal values for a field that has a
 # list of legal values, like rep_platform or resolution.
 sub get_legal_field_values {

process_bug.cgi

@@ -1127,7 +1127,7 @@ if (defined $cgi->param('qa_contact')
                 my $qa_user = $usercache{$qacontact};
                 foreach my $product_id (@newprod_ids) {
                     if (!$qa_user->can_edit_product($product_id)) {
-                        my $product_name = get_product_name($product_id);
+                        my $product_name = Bugzilla::Product->new($product_id)->name;
                         ThrowUserError('invalid_user_group',
                                           {'users'   => $qa_user->login,
                                            'product' => $product_name,
@@ -1212,7 +1212,7 @@ SWITCH: for ($cgi->param('knob')) {
                 my $assign_user = $usercache{$assignee};
                 foreach my $product_id (@newprod_ids) {
                     if (!$assign_user->can_edit_product($product_id)) {
-                        my $product_name = get_product_name($product_id);
+                        my $product_name = Bugzilla::Product->new($product_id)->name;
                         ThrowUserError('invalid_user_group',
                                           {'users'   => $assign_user->login,
                                            'product' => $product_name,
@@ -1618,7 +1618,7 @@ foreach my $id (@idlist) {
         ThrowUserError("illegal_change", $vars);
     }
 
-    $oldhash{'product'} = get_product_name($oldhash{'product_id'});
+    $oldhash{'product'} = $old_bug_obj->product;
     if (!Bugzilla->user->can_edit_product($oldhash{'product_id'})) {
         ThrowUserError("product_edit_denied",
                       { product => $oldhash{'product'} });
@@ -1754,11 +1754,6 @@ foreach my $id (@idlist) {
         $dbh->do(q{DELETE FROM duplicates WHERE dupe = ?}, undef, $id);
     }
 
-    my $newproduct_id = $oldhash{'product_id'};
-    if ($cgi->param('product') ne $cgi->param('dontchange')) {
-        my $newproduct_id = get_product_id($cgi->param('product'));
-    }
-
     my %groupsrequired = ();
     my %groupsforbidden = ();
     my $group_controls =
@@ -1768,7 +1763,7 @@ foreach my $id (@idlist) {
                                        ON id = group_id
                                       AND product_id = ?
                                     WHERE isactive != 0},
-        undef, $newproduct_id);
+        undef, $oldhash{'product_id'});
     foreach my $group_control (@$group_controls) {
         my ($group, $control) = @$group_control;
         $control ||= 0;
@@ -1925,9 +1920,8 @@ foreach my $id (@idlist) {
     # about which can be found in comments within the conditionals below.
     # Check if the user has changed the product to which the bug belongs;
     if ($cgi->param('product') ne $cgi->param('dontchange')
-        && $cgi->param('product') ne $oldhash{'product'}
-    ) {
-        $newproduct_id = get_product_id($cgi->param('product'));
+        && $cgi->param('product') ne $oldhash{'product'})
+    {
         # Depending on the "addtonewgroup" variable, groups with
         # defaults will change.
         #
@@ -1954,7 +1948,7 @@ foreach my $id (@idlist) {
             LEFT JOIN bug_group_map
                    ON bug_group_map.group_id = groups.id
                   AND bug_group_map.bug_id = ?},
-            undef, $oldhash{'product_id'}, $newproduct_id, $id);
+            undef, $oldhash{'product_id'}, $product->id, $id);
         my @groupstoremove = ();
         my @groupstoadd = ();
         my @defaultstoremove = ();
@@ -2077,8 +2071,8 @@ foreach my $id (@idlist) {
             # Products and components are now stored in the DB using ID's
             # We need to translate this to English before logging it
             if ($col eq 'product_id') {
-                $old = get_product_name($old);
-                $new = get_product_name($new);
+                $old = $old_bug_obj->product;
+                $new = $new_bug_obj->product;
                 $col = 'product';
             }
             if ($col eq 'component_id') {

template/en/default/bug/create/create-guided.html.tmpl

@@ -51,7 +51,7 @@
 [%# This script displays the descriptions for selected components. %]
 <script type="text/javascript">
 var descriptions = [
-[% FOREACH c = component_ %]
+[% FOREACH c = product.components %]
   '[% c.description FILTER js %]',
 [% END %]
 ];
@@ -112,12 +112,12 @@ function PutDescription() {
 </p>
 
 [%# Include other products if sensible %]
-[% IF product == "Firefox" %]
+[% IF product.name == "Firefox" %]
   [% productstring = "product=Mozilla%20Application%20Suite&amp;product=Firefox" %]
-[% ELSIF product == "Thunderbird" %]
+[% ELSIF product.name == "Thunderbird" %]
   [% productstring = "product=Mozilla%20Application%20Suite&amp;product=Thunderbird" %]
 [% ELSE %]
-  [% productstring = BLOCK %]product=[% product FILTER url_quote %][% END %]
+  [% productstring = BLOCK %]product=[% product.name FILTER url_quote %][% END %]
 [% END %]
 
 <p>
@@ -142,11 +142,11 @@ function PutDescription() {
     <input type="hidden" name="format" value="simple">
     <input type="hidden" name="order" value="relevance desc">
     <input type="hidden" name="bug_status" value="__all__">
-    <input type="hidden" name="product" value="[% product FILTER html %]">
-    [% IF product == "Firefox" OR 
-          product == "Thunderbird" OR 
-          product == "Mozilla Application Suite" OR
-          product == "Camino" %]
+    <input type="hidden" name="product" value="[% product.name FILTER html %]">
+    [% IF product.name == "Firefox" OR 
+          product.name == "Thunderbird" OR 
+          product.name == "Mozilla Application Suite" OR
+          product.name == "Camino" %]
       <input type="hidden" name="product" value="Core">
       <input type="hidden" name="product" value="Toolkit">
       <input type="hidden" name="product" value="PSM">
@@ -194,8 +194,8 @@ function PutDescription() {
       <b>Product</b>
     </td>
     <td valign="top">
-      <input type="hidden" name="product" value="[% product FILTER html %]">
-      [% product FILTER html %]
+      <input type="hidden" name="product" value="[% product.name FILTER html %]">
+      [% product.name FILTER html %]
     </td>
   </tr>
 
@@ -209,7 +209,7 @@ function PutDescription() {
           <td valign="top">
             <select name="component" id="component"
                     size="5" onchange="PutDescription()">
-              [% FOREACH c = component_ %]
+              [% FOREACH c = product.components %]
                 <option value="[% c.name FILTER html %]"
                   [% " selected=\"selected\"" IF c == default.component_ %]>
                   [% c.name FILTER html %]
@@ -236,7 +236,7 @@ function PutDescription() {
         To pick the right component, you could use the same one as
         similar [% terms.bugs %] you found in your search, or read the full list of
         <a target="_blank" href="describecomponents.cgi?product=
-            [% product FILTER url_quote %]">component
+            [% product.name FILTER url_quote %]">component
         descriptions</a> (opens in new window) if you need more help.
       </p>
     </td>

template/en/default/bug/create/create.html.tmpl

@@ -21,32 +21,29 @@
   #                 Shane H. W. Travis <travis@sedsystems.ca>
   #%]
 
-[% PROCESS global/variables.none.tmpl %]
 [% PROCESS "global/field-descs.none.tmpl" %]
 
-[% product_name = product FILTER html %]
 [% PROCESS global/header.html.tmpl
-  title = "Enter $terms.Bug: $product_name"
+  title = "Enter $terms.Bug: $product.name"
   onload="set_assign_to();"
 %]
-[% USE Bugzilla %]
 
 <script type="text/javascript">
 <!--
 
-var initialowners = new Array([% component_.size %]);
+var initialowners = new Array([% product.components.size %]);
 var last_initialowner;
-var components = new Array([% component_.size %]);
+var components = new Array([% product.components.size %]);
 [% IF Param("useqacontact") %]
-    var initialqacontacts = new Array([% component_.size %]);
+    var initialqacontacts = new Array([% product.components.size %]);
     var last_initialqacontact;
 [% END %]
 [% count = 0 %]
-[%- FOREACH c = component_ %]
+[%- FOREACH c = product.components %]
     components[[% count %]] = "[% c.name FILTER js %]";
-    initialowners[[% count %]] = "[% c.initialowner FILTER js %]";
+    initialowners[[% count %]] = "[% c.default_assignee.login FILTER js %]";
     [% IF Param("useqacontact") %]
-        initialqacontacts[[% count %]] = "[% c.initialqacontact FILTER js %]";
+        initialqacontacts[[% count %]] = "[% c.default_qa_contact.login FILTER js %]";
     [% END %]
     [% count = count + 1 %]
 [%- END %]
@@ -93,7 +90,7 @@ function set_assign_to() {
 </script>
 
 <form name="Create" id="Create" method="post" action="post_bug.cgi">
-<input type="hidden" name="product" value="[% product FILTER html %]">
+<input type="hidden" name="product" value="[% product.name FILTER html %]">
 <input type="hidden" name="token" value="[% token FILTER html %]">
 
 <table cellspacing="2" cellpadding="0" border="0">
@@ -118,10 +115,10 @@ function set_assign_to() {
 
   <tr>
     <td align="right" valign="top"><strong>Reporter:</strong></td>
-    <td valign="top">[% Bugzilla.user.login FILTER html %]</td>
+    <td valign="top">[% user.login FILTER html %]</td>
 
     <td align="right" valign="top"><strong>Product:</strong></td>
-    <td valign="top">[% product FILTER html %]</td>
+    <td valign="top">[% product.name FILTER html %]</td>
   </tr>
 
   [%# We can't use the select block in these two cases for various reasons. %]
@@ -141,13 +138,13 @@ function set_assign_to() {
 
     <td align="right" valign="top">
       <strong>
-        <a href="describecomponents.cgi?product=[% product FILTER url_quote %]">
+        <a href="describecomponents.cgi?product=[% product.name FILTER url_quote %]">
           Component</a>:
       </strong>
     </td>
     <td>
       <select name="component" onchange="set_assign_to();" size="5">
-        [%- FOREACH c = component_ %]
+        [%- FOREACH c = product.components %]
           <option value="[% c.name FILTER html %]"
             [% " selected=\"selected\"" IF c.name == default.component_ %]>
             [% c.name FILTER html -%]