diff --git a/webtools/bugzilla/Bugzilla/Classification.pm b/webtools/bugzilla/Bugzilla/Classification.pm index fd011e6fe004..c35c0ba9b6d2 100644 --- a/webtools/bugzilla/Bugzilla/Classification.pm +++ b/webtools/bugzilla/Bugzilla/Classification.pm @@ -21,6 +21,7 @@ package Bugzilla::Classification; use Bugzilla; use Bugzilla::Util; +use Bugzilla::Error; ############################### #### Initialization #### @@ -87,7 +88,7 @@ sub product_count { if (!defined $self->{'product_count'}) { $self->{'product_count'} = $dbh->selectrow_array(q{ SELECT COUNT(*) FROM products - WHERE classification_id = ?}, undef, $self->id); + WHERE classification_id = ?}, undef, $self->id) || 0; } return $self->{'product_count'}; } @@ -108,13 +109,31 @@ sub get_all_classifications () { my $dbh = Bugzilla->dbh; my $ids = $dbh->selectcol_arrayref(q{ - SELECT id FROM classifications}); + SELECT id FROM classifications ORDER BY name}); - my $classifications; + my @classifications; foreach my $id (@$ids) { - $classifications->{$id} = new Bugzilla::Classification($id); + push @classifications, new Bugzilla::Classification($id); } - return $classifications; + return @classifications; +} + +sub check_classification ($) { + my ($class_name) = @_; + + unless ($class_name) { + ThrowUserError("classification_not_specified"); + } + + my $classification = + new Bugzilla::Classification({name => $class_name}); + + unless ($classification) { + ThrowUserError("classification_doesnt_exist", + { name => $class_name }); + } + + return $classification; } 1; @@ -140,11 +159,14 @@ Bugzilla::Classification - Bugzilla classification class. my $hash_ref = Bugzilla::Classification::get_all_classifications(); my $classification = $hash_ref->{1}; + my $classification = + Bugzilla::Classification::check_classification('AcmeClass'); + =head1 DESCRIPTION Classification.pm represents a Classification object. -A Classification is a higher-level grouping of Bugzilla Products. +A Classification is a higher-level grouping of Products. =head1 METHODS @@ -181,12 +203,20 @@ A Classification is a higher-level grouping of Bugzilla Products. =item C - Description: Returns all Bugzilla classifications. + Description: Returns all classifications. Params: none. - Returns: A hash with classification id as key and - Bugzilla::Classification object as value. + Returns: Bugzilla::Classification object list. + +=item C + + Description: Checks if the classification name passed in is a + valid classification. + + Params: $classification_name - String with a classification name. + + Returns: Bugzilla::Classification object. =back diff --git a/webtools/bugzilla/Bugzilla/Product.pm b/webtools/bugzilla/Bugzilla/Product.pm index 37ef86a621ea..26c80103f8bf 100644 --- a/webtools/bugzilla/Bugzilla/Product.pm +++ b/webtools/bugzilla/Bugzilla/Product.pm @@ -196,13 +196,13 @@ sub get_products_by_classification ($) { my $ids = $dbh->selectcol_arrayref(q{ SELECT id FROM products - WHERE classification_id = ?}, undef, $class_id); + WHERE classification_id = ? ORDER by name}, undef, $class_id); - my $products; + my @products; foreach my $id (@$ids) { - $products->{$id} = new Bugzilla::Product($id); + push @products, new Bugzilla::Product($id); } - return $products; + return @products; } sub get_all_products () { @@ -265,8 +265,7 @@ Bugzilla::Product - Bugzilla product class. my $defaultmilestone = $product->default_milestone; my $classificationid = $product->classification_id; - my $hash_ref = Bugzilla::Product::get_products_by_classification(1); - my $product = $hash_ref->{1}; + my @products = Bugzilla::Product::get_products_by_classification(1); =head1 DESCRIPTION @@ -355,8 +354,7 @@ Product.pm represents a product object. Params: $class_id - Integer with classification id. - Returns: A hash with product id as key and a Bugzilla::Product - object as value. + Returns: Bugzilla::Product object list. =item C diff --git a/webtools/bugzilla/editclassifications.cgi b/webtools/bugzilla/editclassifications.cgi index 737db21d5b3c..5e49d8336921 100755 --- a/webtools/bugzilla/editclassifications.cgi +++ b/webtools/bugzilla/editclassifications.cgi @@ -28,6 +28,8 @@ use Bugzilla::Constants; use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::Config qw($datadir); +use Bugzilla::Classification; +use Bugzilla::Product; require "globals.pl"; @@ -36,34 +38,6 @@ my $dbh = Bugzilla->dbh; my $template = Bugzilla->template; my $vars = {}; -# TestClassification: just returns if the specified classification does exist -# CheckClassification: same check, optionally emit an error text - -sub TestClassification ($) { - my $cl = shift; - my $dbh = Bugzilla->dbh; - - trick_taint($cl); - # does the classification exist? - my $sth = $dbh->prepare("SELECT name - FROM classifications - WHERE name=?"); - $sth->execute($cl); - my @row = $sth->fetchrow_array(); - return $row[0]; -} - -sub CheckClassification ($) { - my $cl = shift; - - unless ($cl) { - ThrowUserError("classification_not_specified"); - } - if (! TestClassification($cl)) { - ThrowUserError("classification_doesnt_exist", { name => $cl }); - } -} - sub LoadTemplate ($) { my $action = shift; @@ -93,44 +67,16 @@ ThrowUserError("auth_classification_not_enabled") unless Param("useclassificatio # # often used variables # -my $action = trim($cgi->param('action') || ''); -my $classification = trim($cgi->param('classification') || ''); -trick_taint($classification); -$vars->{'classification'} = $classification; - +my $action = trim($cgi->param('action') || ''); +my $class_name = trim($cgi->param('classification') || ''); + # # action='' -> Show nice list of classifications # unless ($action) { - my @classifications; - # left join is tricky - # - must select "classifications" fields if you want a REAL value - # - must use "count(products.classification_id)" if you want a true - # count. If you use count(classifications.id), it will return 1 for NULL - # - must use "group by classifications.id" instead of - # products.classification_id. Otherwise it won't look for all - # classification ids, just the ones used by the products. - my $sth = $dbh->prepare("SELECT classifications.id, classifications.name, - classifications.description, - COUNT(classification_id) AS total - FROM classifications - LEFT JOIN products - ON classifications.id = products.classification_id - " . $dbh->sql_group_by('classifications.id', - 'classifications.name, - classifications.description') . " - ORDER BY name"); - $sth->execute(); - while (my ($id,$classification,$description,$total) = $sth->fetchrow_array()) { - my $cl = {}; - $cl->{'id'} = $id; - $cl->{'classification'} = $classification; - $cl->{'description'} = $description if (defined $description); - $cl->{'total'} = $total; - - push(@classifications, $cl); - } + my @classifications = + Bugzilla::Classification::get_all_classifications(); $vars->{'classifications'} = \@classifications; LoadTemplate("select"); @@ -151,19 +97,24 @@ if ($action eq 'add') { # if ($action eq 'new') { - unless ($classification) { - ThrowUserError("classification_not_specified"); - } - if (TestClassification($classification)) { - ThrowUserError("classification_already_exists", { name => $classification }); + + $class_name || ThrowUserError("classification_not_specified"); + + my $classification = + new Bugzilla::Classification({name => $class_name}); + + if ($classification) { + ThrowUserError("classification_already_exists", + { name => $classification->name }); } + my $description = trim($cgi->param('description') || ''); trick_taint($description); + trick_taint($class_name); # Add the new classification. - my $sth = $dbh->prepare("INSERT INTO classifications (name,description) - VALUES (?,?)"); - $sth->execute($classification,$description); + $dbh->do("INSERT INTO classifications (name, description) + VALUES (?, ?)", undef, ($class_name, $description)); # Make versioncache flush unlink "$datadir/versioncache"; @@ -178,25 +129,20 @@ if ($action eq 'new') { # if ($action eq 'del') { - CheckClassification($classification); - my $sth; - # display some data about the classification - $sth = $dbh->prepare("SELECT id, description - FROM classifications - WHERE name=?"); - $sth->execute($classification); - my ($classification_id, $description) = $sth->fetchrow_array(); + my $classification = + Bugzilla::Classification::check_classification($class_name); - ThrowUserError("classification_not_deletable") if ($classification_id eq "1"); + if ($classification->id == 1) { + ThrowUserError("classification_not_deletable"); + } - $sth = $dbh->prepare("SELECT name - FROM products - WHERE classification_id=$classification_id"); - $sth->execute(); - ThrowUserError("classification_has_products") if ($sth->fetchrow_array()); + if ($classification->product_count()) { + ThrowUserError("classification_has_products"); + } - $vars->{'description'} = $description if (defined $description); + $vars->{'description'} = $classification->description; + $vars->{'classification'} = $classification->name; LoadTemplate($action); } @@ -206,32 +152,31 @@ if ($action eq 'del') { # if ($action eq 'delete') { - CheckClassification($classification); - my $sth; - my $classification_id = get_classification_id($classification); + my $classification = + Bugzilla::Classification::check_classification($class_name); - if ($classification_id == 1) { - ThrowUserError("cant_delete_default_classification", { name => $classification }); + if ($classification->id == 1) { + ThrowUserError("classification_not_deletable"); } # lock the tables before we start to change everything: $dbh->bz_lock_tables('classifications WRITE', 'products WRITE'); # delete - $sth = $dbh->prepare("DELETE FROM classifications WHERE id=?"); - $sth->execute($classification_id); + $dbh->do("DELETE FROM classifications WHERE id = ?", undef, + $classification->id); # update products just in case - $sth = $dbh->prepare("UPDATE products - SET classification_id=1 - WHERE classification_id=?"); - $sth->execute($classification_id); + $dbh->do("UPDATE products SET classification_id = 1 + WHERE classification_id = ?", undef, $classification->id); $dbh->bz_unlock_tables(); unlink "$datadir/versioncache"; + $vars->{'classification'} = $classification->name; + LoadTemplate($action); } @@ -242,34 +187,17 @@ if ($action eq 'delete') { # if ($action eq 'edit') { - CheckClassification($classification); - my @products = (); - my $has_products = 0; - my $sth; - + my $classification = + Bugzilla::Classification::check_classification($class_name); - # get data of classification - $sth = $dbh->prepare("SELECT id,description - FROM classifications - WHERE name=?"); - $sth->execute($classification); - my ($classification_id,$description) = $sth->fetchrow_array(); - $vars->{'description'} = $description if (defined $description); + my @products = + Bugzilla::Product::get_products_by_classification( + $classification->id); - $sth = $dbh->prepare("SELECT name,description - FROM products - WHERE classification_id=? - ORDER BY name"); - $sth->execute($classification_id); - while ( my ($product, $prod_description) = $sth->fetchrow_array()) { - my $prod = {}; - $has_products = 1; - $prod->{'name'} = $product; - $prod->{'description'} = $prod_description if (defined $prod_description); - push(@products, $prod); - } - $vars->{'products'} = \@products if ($has_products); + $vars->{'description'} = $classification->description; + $vars->{'classification'} = $classification->name; + $vars->{'products'} = \@products; LoadTemplate($action); } @@ -279,44 +207,39 @@ if ($action eq 'edit') { # if ($action eq 'update') { - my $classificationold = trim($cgi->param('classificationold') || ''); - my $description = trim($cgi->param('description') || ''); - my $descriptionold = trim($cgi->param('descriptionold') || ''); - my $checkvotes = 0; - my $sth; - CheckClassification($classificationold); + $class_name || ThrowUserError("classification_not_specified"); - my $classification_id = get_classification_id($classificationold); - trick_taint($description); + my $class_old_name = trim($cgi->param('classificationold') || ''); + my $description = trim($cgi->param('description') || ''); - # Note that we got the $classification_id using $classificationold - # above so it will remain static even after we rename the - # classification in the database. + my $class_old = + Bugzilla::Classification::check_classification($class_old_name); $dbh->bz_lock_tables('classifications WRITE'); - if ($classification ne $classificationold) { - unless ($classification) { - ThrowUserError("classification_not_specified"); + if ($class_name ne $class_old->name) { + + my $class = new Bugzilla::Classification({name => $class_name}); + if ($class) { + ThrowUserError("classification_already_exists", + { name => $class->name }); } + trick_taint($class_name); + $dbh->do("UPDATE classifications SET name = ? WHERE id = ?", + undef, ($class_name, $class_old->id)); - if (TestClassification($classification)) { - ThrowUserError("classification_already_exists", { name => $classification }); - } - $sth = $dbh->prepare("UPDATE classifications - SET name=? WHERE id=?"); - $sth->execute($classification,$classification_id); $vars->{'updated_classification'} = 1; unlink "$datadir/versioncache"; } - if ($description ne $descriptionold) { - $sth = $dbh->prepare("UPDATE classifications - SET description=? - WHERE id=?"); - $sth->execute($description,$classification_id); + if ($description ne $class_old->description) { + trick_taint($description); + $dbh->do("UPDATE classifications SET description = ? + WHERE id = ?", undef, + ($description, $class_old->id)); + $vars->{'updated_description'} = 1; unlink "$datadir/versioncache"; @@ -332,26 +255,20 @@ if ($action eq 'update') { # if ($action eq 'reclassify') { - CheckClassification($classification); - my $sth; - # display some data about the classification - $sth = $dbh->prepare("SELECT id, description - FROM classifications - WHERE name=?"); - $sth->execute($classification); - my ($classification_id, $description) = $sth->fetchrow_array(); + my $classification = + Bugzilla::Classification::check_classification($class_name); + + $vars->{'description'} = $classification->description; - $vars->{'description'} = $description if (defined $description); + my $sth = $dbh->prepare("UPDATE products SET classification_id = ? + WHERE name = ?"); - $sth = $dbh->prepare("UPDATE products - SET classification_id=? - WHERE name=?"); if (defined $cgi->param('add_products')) { if (defined $cgi->param('prodlist')) { foreach my $prod ($cgi->param("prodlist")) { trick_taint($prod); - $sth->execute($classification_id,$prod); + $sth->execute($classification->id, $prod); } } } elsif (defined $cgi->param('remove_products')) { @@ -361,44 +278,24 @@ if ($action eq 'reclassify') { $sth->execute(1,$prod); } } - } elsif (defined $cgi->param('migrate_products')) { - if (defined $cgi->param('clprodlist')) { - foreach my $prod ($cgi->param("clprodlist")) { - trick_taint($prod); - $sth->execute($classification_id,$prod); - } - } } my @selected_products = (); - my @class_products = (); + my @unselected_products = (); - $sth = $dbh->prepare("SELECT classifications.id, - products.name, - classifications.name, - classifications.id > 1 as unknown - FROM products - INNER JOIN classifications - ON classifications.id = products.classification_id - ORDER BY unknown, products.name, - classifications.name"); - $sth->execute(); - while ( my ($clid, $name, $clname) = $sth->fetchrow_array() ) { - if ($clid == $classification_id) { - push(@selected_products,$name); + my @products = Bugzilla::Product::get_all_products(); + + foreach my $product (@products) { + if ($product->classification_id == $classification->id) { + push @selected_products, $product; } else { - my $cl = {}; - if ($clid == 1) { - $cl->{'name'} = "[$clname] $name"; - } else { - $cl->{'name'} = "$name [$clname]"; - } - $cl->{'value'} = $name; - push(@class_products,$cl); + push @unselected_products, $product; } } - $vars->{'selected_products'} = \@selected_products; - $vars->{'class_products'} = \@class_products; + + $vars->{'selected_products'} = \@selected_products; + $vars->{'unselected_products'} = \@unselected_products; + $vars->{'classification'} = $classification->name; LoadTemplate($action); } @@ -407,4 +304,4 @@ if ($action eq 'reclassify') { # No valid action found # -ThrowCodeError("action_unrecognized", $vars); +ThrowCodeError("action_unrecognized", {action => $action}); diff --git a/webtools/bugzilla/template/en/default/admin/classifications/edit.html.tmpl b/webtools/bugzilla/template/en/default/admin/classifications/edit.html.tmpl index fba32ca30201..65299df2270f 100644 --- a/webtools/bugzilla/template/en/default/admin/classifications/edit.html.tmpl +++ b/webtools/bugzilla/template/en/default/admin/classifications/edit.html.tmpl @@ -59,7 +59,6 @@ - diff --git a/webtools/bugzilla/template/en/default/admin/classifications/reclassify.html.tmpl b/webtools/bugzilla/template/en/default/admin/classifications/reclassify.html.tmpl index 5d4cb73e4a1b..3f7982bb3214 100644 --- a/webtools/bugzilla/template/en/default/admin/classifications/reclassify.html.tmpl +++ b/webtools/bugzilla/template/en/default/admin/classifications/reclassify.html.tmpl @@ -51,9 +51,9 @@ @@ -66,8 +66,8 @@ diff --git a/webtools/bugzilla/template/en/default/admin/classifications/select.html.tmpl b/webtools/bugzilla/template/en/default/admin/classifications/select.html.tmpl index 3cfa3fcaaaba..60ae81216636 100644 --- a/webtools/bugzilla/template/en/default/admin/classifications/select.html.tmpl +++ b/webtools/bugzilla/template/en/default/admin/classifications/select.html.tmpl @@ -23,8 +23,6 @@ title = "Select classification" %] -[% filt_classification = classification FILTER html %] - @@ -35,7 +33,7 @@ [% FOREACH cl = classifications %] - + [% IF (cl.id == 1) %] - + [% ELSE %] - + [% END %] [%# don't allow user to delete the default id. %] [% IF (cl.id == 1) %] [% ELSE %] - + [% END %] [% END %] diff --git a/webtools/bugzilla/template/en/default/global/user-error.html.tmpl b/webtools/bugzilla/template/en/default/global/user-error.html.tmpl index 6fd58ee6684b..59bbe1d874c3 100644 --- a/webtools/bugzilla/template/en/default/global/user-error.html.tmpl +++ b/webtools/bugzilla/template/en/default/global/user-error.html.tmpl @@ -248,10 +248,6 @@ must reassign those products to another classification before you can delete this one. - [% ELSIF error == "cant_delete_default_classification" %] - Sorry, but you can not delete the default classification, - '[% name FILTER html %]'. - [% ELSIF error == "component_already_exists" %] [% title = "Component Already Exists" %] A component with the name '[% name FILTER html %]' already exists.
Edit Classification ...
[% cl.classification FILTER html %][% cl.name FILTER html %] [% IF cl.description %] [% cl.description %] @@ -44,16 +42,16 @@ [% END %] [% cl.total FILTER html %][% cl.product_count FILTER html %]reclassify ([% cl.total FILTER html %])reclassify ([% cl.product_count FILTER html %]) deletedelete