From: lpsolit%gmail.com <> Date: Wed, 15 Mar 2006 06:37:16 +0000 (+0000) Subject: Bug 311422: describecomponents.cgi and enter_bug.cgi need some cleanup - Patch by... X-Git-Tag: bugzilla-2.23.1~53 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=577a3b3540fa2331e41f51cc2a2201ce902df289;p=thirdparty%2Fbugzilla.git Bug 311422: describecomponents.cgi and enter_bug.cgi need some cleanup - Patch by Frédéric Buclin r=wicked a=myk --- diff --git a/describecomponents.cgi b/describecomponents.cgi index c00f39bed3..87a1eed32a 100755 --- a/describecomponents.cgi +++ b/describecomponents.cgi @@ -20,6 +20,7 @@ # # Contributor(s): Terry Weissman # Bradley Baetz +# Frédéric Buclin use strict; use lib qw(.); @@ -27,83 +28,59 @@ use lib qw(.); use Bugzilla; use Bugzilla::Constants; require "globals.pl"; - -use vars qw(@legal_product); +use Bugzilla::Product; my $user = Bugzilla->login(); -GetVersionTable(); - my $cgi = Bugzilla->cgi; my $dbh = Bugzilla->dbh; my $template = Bugzilla->template; my $vars = {}; -my $product = trim($cgi->param('product') || ''); -my $product_id = get_product_id($product); -if (!$product_id || !$user->can_enter_product($product)) { +print $cgi->header(); + +my $product_name = trim($cgi->param('product') || ''); +my $product = new Bugzilla::Product({'name' => $product_name}); + +unless ($product && $user->can_enter_product($product->name)) { # Products which the user is allowed to see. - my @products = @{$user->get_enterable_products()}; + my @products = @{$user->get_enterable_products}; if (scalar(@products) == 0) { ThrowUserError("no_products"); } - elsif (scalar(@products) > 1) { - # XXX - For backwards-compatibility with old template - # interfaces, we now create a proddesc hash. This can go away - # once we update the templates. - my %product_desc; - foreach my $product (@products) { - $product_desc{$product->name} = $product->description; - } - $vars->{'proddesc'} = \%product_desc; + # If there is only one product available but the user entered + # another product name, we display a list with this single + # product only, to not confuse the user with components of a + # product he didn't request. + elsif (scalar(@products) > 1 || $product_name) { + $vars->{'products'} = \@products; $vars->{'target'} = "describecomponents.cgi"; # If an invalid product name is given, or the user is not # allowed to access that product, a message is displayed # with a list of the products the user can choose from. - if ($product) { + if ($product_name) { $vars->{'message'} = "product_invalid"; - $vars->{'product'} = $product; + # Do not use $product->name here, else you could use + # this way to determine whether the product exists or not. + $vars->{'product'} = $product_name; } - print $cgi->header(); $template->process("global/choose-product.html.tmpl", $vars) || ThrowTemplateError($template->error()); exit; } - # Else, if there is only one product: - $product = $products[0]->name; - $product_id = $products[0]->id; + # If there is only one product available and the user didn't specify + # any product name, we show this product. + $product = $products[0]; } ###################################################################### # End Data/Security Validation ###################################################################### -my @components; -my $comps = $dbh->selectall_arrayref( - q{SELECT name, initialowner, initialqacontact, description - FROM components - WHERE product_id = ? - ORDER BY name}, undef, $product_id); -foreach my $comp (@$comps) { - my ($name, $initialowner, $initialqacontact, $description) = @$comp; - my %component; - - $component{'name'} = $name; - $component{'initialowner'} = $initialowner ? - DBID_to_name($initialowner) : ''; - $component{'initialqacontact'} = $initialqacontact ? - DBID_to_name($initialqacontact) : ''; - $component{'description'} = $description; - - push @components, \%component; -} - $vars->{'product'} = $product; -$vars->{'components'} = \@components; -print $cgi->header(); $template->process("reports/components.html.tmpl", $vars) || ThrowTemplateError($template->error()); diff --git a/enter_bug.cgi b/enter_bug.cgi index d1d0701482..529c4ce737 100755 --- a/enter_bug.cgi +++ b/enter_bug.cgi @@ -43,11 +43,11 @@ use Bugzilla::Bug; use Bugzilla::User; use Bugzilla::Hook; use Bugzilla::Product; +use Bugzilla::Classification; use Bugzilla::Keyword; require "globals.pl"; use vars qw( - @enterable_products @legal_opsys @legal_platform @legal_priority @@ -66,95 +66,74 @@ my $cgi = Bugzilla->cgi; my $template = Bugzilla->template; my $vars = {}; -my $product = $cgi->param('product'); +my $product = trim($cgi->param('product') || ''); -if (!defined $product || $product eq "") { - GetVersionTable(); - Bugzilla->login(); +if ($product eq '') { + my $user = Bugzilla->login(); - if ( ! Param('useclassification') ) { - # Just use a fake value for the Classification. - $cgi->param(-name => 'classification', - -value => '__all'); - } + # 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); - if (!$cgi->param('classification')) { - my $classifications = Bugzilla->user->get_selectable_classifications(); - foreach my $classification (@$classifications) { - my $found = 0; - foreach my $p (@enterable_products) { - if (Bugzilla->user->can_enter_product($p) - && IsInClassification($classification->{name},$p)) { - $found = 1; - } - } - if ($found == 0) { - @$classifications = grep($_->{name} ne $classification->{name}, - @$classifications); - } + my $classification = Param('useclassification') ? + scalar($cgi->param('classification')) : '__all'; + + unless ($classification) { + my $class; + # Get all classifications with at least one enterable product. + foreach $product (@enterable_products) { + $class->{$product->classification_id} ||= + new Bugzilla::Classification($product->classification_id); } + my @classifications = sort {lc($a->name) cmp lc($b->name)} (values %$class); - if (scalar(@$classifications) == 0) { - ThrowUserError("no_products"); - } - elsif (scalar(@$classifications) > 1) { - $vars->{'classifications'} = $classifications; + # We know there is at least one classification available, + # else we would have stopped earlier. + if (scalar(@classifications) > 1) { + $vars->{'classifications'} = \@classifications; $vars->{'target'} = "enter_bug.cgi"; $vars->{'format'} = $cgi->param('format'); - $vars->{'cloned_bug_id'} = $cgi->param('cloned_bug_id'); print $cgi->header(); $template->process("global/choose-classification.html.tmpl", $vars) || ThrowTemplateError($template->error()); - exit; + exit; } - $cgi->param(-name => 'classification', -value => @$classifications[0]->name); + # If we come here, then there is only one classification available. + $classification = $classifications[0]->name; } - my %products; - # XXX - This loop should work in some more sensible, efficient way. - foreach my $p (@enterable_products) { - if (Bugzilla->user->can_enter_product($p)) { - if (IsInClassification(scalar $cgi->param('classification'),$p) || - $cgi->param('classification') eq "__all") { - my $product_object = new Bugzilla::Product({name => $p}); - $products{$p} = $product_object->description; - } - } - } - - my $prodsize = scalar(keys %products); - if ($prodsize == 0) { - ThrowUserError("no_products"); - } - elsif ($prodsize > 1) { - my %classifications; - if ( ! Param('useclassification') ) { - @{$classifications{"all"}} = keys %products; + # Keep only enterable products which are in the specified classification. + if ($classification ne "__all") { + my $class = new Bugzilla::Classification({'name' => $classification}); + # If the classification doesn't exist, then there is no product in it. + if ($class) { + @enterable_products + = grep {$_->classification_id == $class->id} @enterable_products; } - elsif ($cgi->param('classification') eq "__all") { - %classifications = %::classifications; - } else { - $classifications{$cgi->param('classification')} = - $::classifications{$cgi->param('classification')}; + else { + @enterable_products = (); } - $vars->{'proddesc'} = \%products; - $vars->{'classifications'} = \%classifications; + } + if (scalar(@enterable_products) == 0) { + ThrowUserError('no_products'); + } + elsif (scalar(@enterable_products) > 1) { + $vars->{'products'} = \@enterable_products; $vars->{'target'} = "enter_bug.cgi"; $vars->{'format'} = $cgi->param('format'); - $vars->{'cloned_bug_id'} = $cgi->param('cloned_bug_id'); - + print $cgi->header(); $template->process("global/choose-product.html.tmpl", $vars) || ThrowTemplateError($template->error()); - exit; + exit; } else { - # Only one product exists - $product = (keys %products)[0]; + # Only one product exists. + $product = $enterable_products[0]->name; } } diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl index 3dd671b38e..b7e43c3c66 100644 --- a/template/en/default/filterexceptions.pl +++ b/template/en/default/filterexceptions.pl @@ -96,7 +96,6 @@ 'reports/components.html.tmpl' => [ 'numcols', - 'comp.description', ], 'reports/duplicates-table.html.tmpl' => [ @@ -240,7 +239,6 @@ 'global/choose-product.html.tmpl' => [ 'target', - 'proddesc.$p', ], # You are not permitted to add any values here. Everything in this file should diff --git a/template/en/default/global/choose-product.html.tmpl b/template/en/default/global/choose-product.html.tmpl index 078b9b7007..346a537516 100644 --- a/template/en/default/global/choose-product.html.tmpl +++ b/template/en/default/global/choose-product.html.tmpl @@ -20,8 +20,11 @@ #%] [%# INTERFACE: - # proddesc: hash. May be empty. The hash keys are the products, and the values - # are their descriptions. + # products: array of product objects. The list of products + # the user can enter bugs into. + # target: the script that displays this template. + # cloned_bug_id: ID of the bug being cloned. + # format: the desired format to display the target. #%] [% PROCESS global/variables.none.tmpl %] @@ -39,18 +42,16 @@ -[% FOREACH p = proddesc.keys.sort %] +[% FOREACH p = products %] - [% IF proddesc.$p %] - - [% END %] + [% END %] diff --git a/template/en/default/reports/components.html.tmpl b/template/en/default/reports/components.html.tmpl index 3578a86e41..3950932bd2 100644 --- a/template/en/default/reports/components.html.tmpl +++ b/template/en/default/reports/components.html.tmpl @@ -20,18 +20,14 @@ #%] [%# INTERFACE: - # product: string. The product this is the components list for. - # components: List of hashes. May be empty. Each hash has four members: - # name: string. Name of the component. - # description: string. Description of the component. May contain HTML. - # initialowner: string. Component's default assignee. - # initialqacontact: string. Component's default QA contact. + # product: object. The product for which we want to display component descriptions. #%] - -[% filtered_product = product FILTER html %] -[% PROCESS global/header.html.tmpl - title = "Components for $product" - h2 = filtered_product %] + +[% title = BLOCK %] + Components for [% product.name FILTER html %] +[% END %] + +[% PROCESS global/header.html.tmpl title = title %] [% IF Param("useqacontact") %] [% numcols = 3 %] @@ -39,28 +35,24 @@ [% numcols = 2 %] [% END %] -[% IF components.size == 0 %] - This product has no components. -[% ELSE %] -
- - [% p FILTER html %]:  + [% p.name FILTER html %]:  [% proddesc.$p %][% p.description FILTER none %]
- - - - [% IF Param("useqacontact") %] - - [% END %] - - - [% FOREACH comp = components %] - [% INCLUDE describe_comp %] +
ComponentDefault AssigneeDefault QA Contact
+ + + + [% IF Param("useqacontact") %] + [% END %] - - - -
ComponentDefault AssigneeDefault QA Contact
-
-
-[% END %] + + + [% FOREACH comp = product.components %] + [% INCLUDE describe_comp %] + [% END %] + + +
+ + + [% PROCESS global/footer.html.tmpl %] @@ -79,21 +71,19 @@ [% comp.name FILTER html %] - - [% comp.initialowner FILTER html %] + + [% comp.default_assignee.login FILTER html %] [% IF Param("useqacontact") %] - - [% comp.initialqacontact FILTER html %] + + [% comp.default_qa_contact.login FILTER html %] [% END %] - [% comp.description %] + [% comp.description FILTER none %] [% END %]