Commit 814b24fd authored by Frédéric Buclin's avatar Frédéric Buclin

Bug 519835: Remove Bugzilla::Product::check_product() in favor of Bugzilla::Product->check()

r=mkanat a=LpSolit
parent ef414f2b
...@@ -1649,10 +1649,8 @@ sub _check_product { ...@@ -1649,10 +1649,8 @@ sub _check_product {
} }
# Check that the product exists and that the user # Check that the product exists and that the user
# is allowed to enter bugs into this product. # is allowed to enter bugs into this product.
Bugzilla->user->can_enter_product($name, THROW_ERROR); my $product = Bugzilla->user->can_enter_product($name, THROW_ERROR);
# can_enter_product already does everything that check_product return $product;
# would do for us, so we don't need to use it.
return new Bugzilla::Product({ name => $name });
} }
sub _check_priority { sub _check_priority {
......
...@@ -812,26 +812,17 @@ sub classification_id { return $_[0]->{'classification_id'}; } ...@@ -812,26 +812,17 @@ sub classification_id { return $_[0]->{'classification_id'}; }
#### Subroutines ###### #### Subroutines ######
############################### ###############################
sub check_product {
my ($product_name) = @_;
unless ($product_name) {
ThrowUserError('product_not_specified');
}
my $product = new Bugzilla::Product({name => $product_name});
unless ($product) {
ThrowUserError('product_doesnt_exist',
{'product' => $product_name});
}
return $product;
}
sub check { sub check {
my ($class, $params) = @_; my ($class, $params) = @_;
$params = { name => $params } if !ref $params; $params = { name => $params } if !ref $params;
$params->{_error} = 'product_access_denied'; if (!$params->{allow_inaccessible}) {
$params->{_error} = 'product_access_denied';
}
my $product = $class->SUPER::check($params); my $product = $class->SUPER::check($params);
if (!Bugzilla->user->can_access_product($product)) {
if (!$params->{allow_inaccessible}
&& !Bugzilla->user->can_access_product($product))
{
ThrowUserError('product_access_denied', $params); ThrowUserError('product_access_denied', $params);
} }
return $product; return $product;
...@@ -1052,15 +1043,6 @@ than calling those accessors on every item in the array individually. ...@@ -1052,15 +1043,6 @@ than calling those accessors on every item in the array individually.
This function is not exported, so must be called like This function is not exported, so must be called like
C<Bugzilla::Product::preload($products)>. C<Bugzilla::Product::preload($products)>.
=item C<check_product($product_name)>
Description: Checks if the product name was passed in and if is a valid
product.
Params: $product_name - String with a product name.
Returns: Bugzilla::Product object.
=back =back
=head1 SEE ALSO =head1 SEE ALSO
......
...@@ -985,7 +985,7 @@ sub check_can_admin_product { ...@@ -985,7 +985,7 @@ sub check_can_admin_product {
my ($self, $product_name) = @_; my ($self, $product_name) = @_;
# First make sure the product name is valid. # First make sure the product name is valid.
my $product = Bugzilla::Product::check_product($product_name); my $product = Bugzilla::Product->check($product_name);
($self->in_group('editcomponents', $product->id) ($self->in_group('editcomponents', $product->id)
&& $self->can_see_product($product->name)) && $self->can_see_product($product->name))
......
...@@ -70,9 +70,6 @@ if ($#ARGV >= 0 && $ARGV[0] eq "--regenerate") { ...@@ -70,9 +70,6 @@ if ($#ARGV >= 0 && $ARGV[0] eq "--regenerate") {
my $datadir = bz_locations()->{'datadir'}; my $datadir = bz_locations()->{'datadir'};
my @myproducts = map {$_->name} Bugzilla::Product->get_all;
unshift(@myproducts, "-All-");
# As we can now customize statuses and resolutions, looking at the current list # As we can now customize statuses and resolutions, looking at the current list
# of legal values only is not enough as some now removed statuses and resolutions # of legal values only is not enough as some now removed statuses and resolutions
# may have existed in the past, or have been renamed. We want them all. # may have existed in the past, or have been renamed. We want them all.
...@@ -142,6 +139,10 @@ if ($regenerate) { ...@@ -142,6 +139,10 @@ if ($regenerate) {
} }
my $tstart = time; my $tstart = time;
my @myproducts = Bugzilla::Product->get_all;
unshift(@myproducts, "-All-");
foreach (@myproducts) { foreach (@myproducts) {
my $dir = "$datadir/mining"; my $dir = "$datadir/mining";
...@@ -173,11 +174,11 @@ sub collect_stats { ...@@ -173,11 +174,11 @@ sub collect_stats {
my $product = shift; my $product = shift;
my $when = localtime (time); my $when = localtime (time);
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
my $product_id; my $product_id;
if ($product ne '-All-') {
my $prod = Bugzilla::Product::check_product($product); if (ref $product) {
$product_id = $prod->id; $product_id = $product->id;
$product = $product->name;
} }
# NB: Need to mangle the product for the filename, but use the real # NB: Need to mangle the product for the filename, but use the real
...@@ -312,6 +313,9 @@ sub regenerate_stats { ...@@ -312,6 +313,9 @@ sub regenerate_stats {
# NB: Need to mangle the product for the filename, but use the real # NB: Need to mangle the product for the filename, but use the real
# product name in the query # product name in the query
if (ref $product) {
$product = $product->name;
}
my $file_product = $product; my $file_product = $product;
$file_product =~ s/\//-/gs; $file_product =~ s/\//-/gs;
my $file = join '/', $dir, $file_product; my $file = join '/', $dir, $file_product;
......
...@@ -69,7 +69,7 @@ $vars->{'custom_fields'} = ...@@ -69,7 +69,7 @@ $vars->{'custom_fields'} =
if ($cgi->param('product')) { if ($cgi->param('product')) {
my @products = $cgi->param('product'); my @products = $cgi->param('product');
foreach my $product_name (@products) { foreach my $product_name (@products) {
# We don't use check_product because config.cgi outputs mostly # We don't use check() because config.cgi outputs mostly
# in XML and JS and we don't want to display an HTML error # in XML and JS and we don't want to display an HTML error
# instead of that. # instead of that.
my $product = new Bugzilla::Product({ name => $product_name }); my $product = new Bugzilla::Product({ name => $product_name });
......
...@@ -595,7 +595,8 @@ sub validateProduct { ...@@ -595,7 +595,8 @@ sub validateProduct {
my $product_name = shift; my $product_name = shift;
return unless $product_name; return unless $product_name;
my $product = Bugzilla::Product::check_product($product_name); my $product = Bugzilla::Product->check({ name => $product_name,
allow_inaccessible => 1 });
return $product; return $product;
} }
......
...@@ -153,15 +153,10 @@ if ($product_name eq '') { ...@@ -153,15 +153,10 @@ if ($product_name eq '') {
$product = $enterable_products[0]; $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 # We need to check and make sure that the user has permission
# to enter a bug against this product. # to enter a bug against this product.
$user->can_enter_product($product ? $product->name : $product_name, THROW_ERROR); $product = $user->can_enter_product($product || $product_name, THROW_ERROR);
############################################################################## ##############################################################################
# Useful Subroutines # Useful Subroutines
......
...@@ -208,7 +208,7 @@ sub queue { ...@@ -208,7 +208,7 @@ sub queue {
# Filter results by exact product or component. # Filter results by exact product or component.
if (defined $cgi->param('product') && $cgi->param('product') ne "") { if (defined $cgi->param('product') && $cgi->param('product') ne "") {
my $product = Bugzilla::Product::check_product(scalar $cgi->param('product')); my $product = Bugzilla::Product->check(scalar $cgi->param('product'));
push(@criteria, "bugs.product_id = " . $product->id); push(@criteria, "bugs.product_id = " . $product->id);
push(@excluded_columns, 'product') unless $cgi->param('do_union'); push(@excluded_columns, 'product') unless $cgi->param('do_union');
if (defined $cgi->param('component') && $cgi->param('component') ne "") { if (defined $cgi->param('component') && $cgi->param('component') ne "") {
......
...@@ -1328,10 +1328,6 @@ ...@@ -1328,10 +1328,6 @@
[% END %] [% END %]
does not exist or you don't have access to it. does not exist or you don't have access to it.
[% ELSIF error == "product_doesnt_exist" %]
[% title = "Specified Product Does Not Exist" %]
The product '[% product FILTER html %]' does not exist.
[% ELSIF error == "product_illegal_group" %] [% ELSIF error == "product_illegal_group" %]
[% title = "Illegal Group" %] [% title = "Illegal Group" %]
[% group.name FILTER html %] is not an active [% terms.bug %] group [% group.name FILTER html %] is not an active [% terms.bug %] group
...@@ -1401,15 +1397,6 @@ ...@@ -1401,15 +1397,6 @@
'versions.html' => 'Administering versions'} %] 'versions.html' => 'Administering versions'} %]
You must enter a valid version to create a new product. You must enter a valid version to create a new product.
[% ELSIF error == "product_not_specified" %]
[% title = "No Product Specified" %]
[% admindocslinks = {'products.html' => 'Administering products',
'components.html' => 'Administering components',
'milestones.html' => 'Administering milestones',
'versions.html' => 'Administering versions'} %]
No product specified when trying to edit components, milestones, versions
or product.
[% ELSIF error == "query_name_exists" %] [% ELSIF error == "query_name_exists" %]
[% title = "Search Name Already In Use" %] [% title = "Search Name Already In Use" %]
The name <em>[% name FILTER html %]</em> is already used by another The name <em>[% name FILTER html %]</em> is already used by another
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment