From 10b02af4fa2bd908adc442b39ac5880b573b011c Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Thu, 28 Jul 2005 07:35:50 +0000 Subject: Bug 300952: Change the editmilestones.cgi code to use Milestone.pm and Product.pm - Patch by Tiago R. Mello r=LpSolit a=justdave --- Bugzilla/Milestone.pm | 78 ++++- editmilestones.cgi | 357 +++++---------------- .../admin/milestones/confirm-delete.html.tmpl | 2 - .../en/default/admin/milestones/edit.html.tmpl | 1 - 4 files changed, 158 insertions(+), 280 deletions(-) diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm index dad8b6c11..6956d5313 100644 --- a/Bugzilla/Milestone.pm +++ b/Bugzilla/Milestone.pm @@ -74,11 +74,24 @@ sub _init { return $self; } +sub bug_count { + my $self = shift; + my $dbh = Bugzilla->dbh; + + if (!defined $self->{'bug_count'}) { + $self->{'bug_count'} = $dbh->selectrow_array(q{ + SELECT COUNT(*) FROM bugs + WHERE product_id = ? AND target_milestone = ?}, + undef, $self->product_id, $self->name) || 0; + } + return $self->{'bug_count'}; +} + ################################ ##### Accessors ###### ################################ -sub value { return $_[0]->{'value'}; } +sub name { return $_[0]->{'value'}; } sub product_id { return $_[0]->{'product_id'}; } sub sortkey { return $_[0]->{'sortkey'}; } @@ -105,12 +118,42 @@ sub get_milestones_by_product ($) { SELECT value FROM milestones WHERE product_id = ?}, undef, $product_id); - my $milestones; + my @milestones; foreach my $value (@$values) { - $milestones->{$value} = new Bugzilla::Milestone($product_id, - $value); + push @milestones, new Bugzilla::Milestone($product_id, $value); } - return $milestones; + return @milestones; +} + +sub check_milestone ($$) { + my ($product, $milestone_name) = @_; + + unless ($milestone_name) { + ThrowUserError('milestone_not_specified'); + } + + my $milestone = new Bugzilla::Milestone($product->id, + $milestone_name); + unless ($milestone) { + ThrowUserError('milestone_not_valid', + {'product' => $product->name, + 'milestone' => $milestone_name}); + } + return $milestone; +} + +sub check_sort_key ($$) { + my ($milestone_name, $sortkey) = @_; + # Keep a copy in case detaint_signed() clears the sortkey + my $stored_sortkey = $sortkey; + + if (!detaint_signed($sortkey) || $sortkey < -32768 + || $sortkey > 32767) { + ThrowUserError('milestone_sortkey_invalid', + {'name' => $milestone_name, + 'sortkey' => $stored_sortkey}); + } + return $sortkey; } 1; @@ -151,6 +194,14 @@ Milestone.pm represents a Product Milestone object. Returns: A Bugzilla::Milestone object. +=item C + + Description: Returns the total of bugs that belong to the milestone. + + Params: none. + + Returns: Integer with the number of bugs. + =back =head1 SUBROUTINES @@ -159,13 +210,22 @@ Milestone.pm represents a Product Milestone object. =item C - Description: Returns all Bugzilla product milestones that belong + Description: Returns all product milestones that belong to the supplied product. - Params: $product_id - Integer with a Bugzilla product id. + Params: $product_id - Integer with a product id. + + Returns: Bugzilla::Milestone object list. + +=item C + + Description: Checks if a milestone name was passed in + and if it is a valid milestone. + + Params: $product - Bugzilla::Product object. + $milestone_name - String with a milestone name. - Returns: A hash with milestone value as key and a - Bugzilla::Milestone object as hash value. + Returns: Bugzilla::Milestone object. =back diff --git a/editmilestones.cgi b/editmilestones.cgi index a2d4d4f5d..4fbcab046 100755 --- a/editmilestones.cgi +++ b/editmilestones.cgi @@ -25,112 +25,14 @@ require "globals.pl"; use Bugzilla::Constants; use Bugzilla::Config qw(:DEFAULT $datadir); use Bugzilla::User; +use Bugzilla::Product; +use Bugzilla::Milestone; use Bugzilla::Bug; use vars qw($template $vars); my $cgi = Bugzilla->cgi; - -# TestProduct: just returns if the specified product does exists -# CheckProduct: same check, optionally emit an error text -# TestMilestone: just returns if the specified product/version combination exists -# CheckMilestone: same check, optionally emit an error text - -sub TestProduct ($) -{ - my $product = shift; - - trick_taint($product); - - # does the product exist? - my $dbh = Bugzilla->dbh; - my $sth = $dbh->prepare_cached("SELECT name - FROM products - WHERE name = ?"); - $sth->execute($product); - - my ($row) = $sth->fetchrow_array; - - $sth->finish; - - return $row; -} - -sub CheckProduct ($) -{ - my $product = shift; - - # do we have a product? - unless ($product) { - ThrowUserError('product_not_specified'); - } - - # Does it exist in the DB? - unless (TestProduct $product) { - ThrowUserError('product_doesnt_exist', - {'product' => $product}); - } -} - -sub TestMilestone ($$) -{ - my ($product, $milestone) = @_; - - my $dbh = Bugzilla->dbh; - - # does the product exist? - my $sth = $dbh->prepare_cached(" - SELECT products.name, value - FROM milestones - INNER JOIN products - ON milestones.product_id = products.id - WHERE products.name = ? - AND value = ?"); - - trick_taint($product); - trick_taint($milestone); - - $sth->execute($product, $milestone); - - my ($db_milestone) = $sth->fetchrow_array(); - - $sth->finish(); - - return $db_milestone; -} - -sub CheckMilestone ($$) -{ - my ($product, $milestone) = @_; - - # do we have the milestone and product combination? - unless ($milestone) { - ThrowUserError('milestone_not_specified'); - } - - CheckProduct($product); - - unless (TestMilestone $product, $milestone) { - ThrowUserError('milestone_not_valid', - {'product' => $product, - 'milestone' => $milestone}); - } -} - -sub CheckSortkey ($$) -{ - my ($milestone, $sortkey) = @_; - # Keep a copy in case detaint_signed() clears the sortkey - my $stored_sortkey = $sortkey; - - if (!detaint_signed($sortkey) || $sortkey < -32768 || $sortkey > 32767) { - ThrowUserError('milestone_sortkey_invalid', - {'name' => $milestone, - 'sortkey' => $stored_sortkey}); - } - - return $sortkey; -} +my $dbh = Bugzilla->dbh; # # Preliminary checks: @@ -139,7 +41,7 @@ sub CheckSortkey ($$) my $user = Bugzilla->login(LOGIN_REQUIRED); my $whoid = $user->id; -print Bugzilla->cgi->header(); +print $cgi->header(); UserInGroup("editcomponents") || ThrowUserError("auth_failure", {group => "editcomponents", @@ -149,38 +51,18 @@ UserInGroup("editcomponents") # # often used variables # -my $product = trim($cgi->param('product') || ''); -my $milestone = trim($cgi->param('milestone') || ''); -my $sortkey = trim($cgi->param('sortkey') || '0'); -my $action = trim($cgi->param('action') || ''); +my $product_name = trim($cgi->param('product') || ''); +my $milestone_name = trim($cgi->param('milestone') || ''); +my $sortkey = trim($cgi->param('sortkey') || 0); +my $action = trim($cgi->param('action') || ''); # -# product = '' -> Show nice list of milestones +# product = '' -> Show nice list of products # -unless ($product) { - - my @products = (); - - my $dbh = Bugzilla->dbh; - - my $sth = $dbh->prepare_cached('SELECT products.name, products.description - FROM products - ORDER BY products.name'); - - my $data = $dbh->selectall_arrayref($sth); - - foreach my $aref (@$data) { - - my $prod = {}; - - my ($name, $description) = @$aref; - - $prod->{'name'} = $name; - $prod->{'description'} = $description; - - push(@products, $prod); - } +unless ($product_name) { + + my @products = Bugzilla::Product::get_all_products(); $vars->{'products'} = \@products; $template->process("admin/milestones/select-product.html.tmpl", @@ -190,7 +72,7 @@ unless ($product) { exit; } - +my $product = Bugzilla::Product::check_product($product_name); # # action='' -> Show nice list of milestones @@ -198,33 +80,10 @@ unless ($product) { unless ($action) { - CheckProduct($product); - my $product_id = get_product_id($product); - my @milestones = (); - - my $dbh = Bugzilla->dbh; - - my $sth = $dbh->prepare_cached('SELECT value, sortkey - FROM milestones - WHERE product_id = ? - ORDER BY sortkey, value'); - - my $data = $dbh->selectall_arrayref($sth, - undef, - $product_id); - - foreach my $aref (@$data) { - - my $milestone = {}; - my ($name, $sortkey) = @$aref; + my @milestones = + Bugzilla::Milestone::get_milestones_by_product($product->id); - $milestone->{'name'} = $name; - $milestone->{'sortkey'} = $sortkey; - - push(@milestones, $milestone); - } - - $vars->{'product'} = $product; + $vars->{'product'} = $product->name; $vars->{'milestones'} = \@milestones; $template->process("admin/milestones/list.html.tmpl", $vars) @@ -244,10 +103,7 @@ unless ($action) { if ($action eq 'add') { - CheckProduct($product); - my $product_id = get_product_id($product); - - $vars->{'product'} = $product; + $vars->{'product'} = $product->name; $template->process("admin/milestones/create.html.tmpl", $vars) || ThrowTemplateError($template->error()); @@ -263,43 +119,36 @@ if ($action eq 'add') { if ($action eq 'new') { - CheckProduct($product); - my $product_id = get_product_id($product); + $milestone_name || ThrowUserError('milestone_blank_name'); - # Cleanups and valididy checks - unless ($milestone) { - ThrowUserError('milestone_blank_name', - {'name' => $milestone}); - } - - if (length($milestone) > 20) { + if (length($milestone_name) > 20) { ThrowUserError('milestone_name_too_long', - {'name' => $milestone}); + {'name' => $milestone_name}); } - $sortkey = CheckSortkey($milestone, $sortkey); + $sortkey = Bugzilla::Milestone::check_sort_key($milestone_name, + $sortkey); + + my $milestone = new Bugzilla::Milestone($product->id, + $milestone_name); - if (TestMilestone($product, $milestone)) { + if ($milestone) { ThrowUserError('milestone_already_exists', - {'name' => $milestone, - 'product' => $product}); + {'name' => $milestone->name, + 'product' => $product->name}); } # Add the new milestone - my $dbh = Bugzilla->dbh; - trick_taint($milestone); + trick_taint($milestone_name); $dbh->do('INSERT INTO milestones ( value, product_id, sortkey ) VALUES ( ?, ?, ? )', - undef, - $milestone, - $product_id, - $sortkey); + undef, $milestone_name, $product->id, $sortkey); # Make versioncache flush unlink "$datadir/versioncache"; - $vars->{'name'} = $milestone; - $vars->{'product'} = $product; + $vars->{'name'} = $milestone_name; + $vars->{'product'} = $product->name; $template->process("admin/milestones/created.html.tmpl", $vars) || ThrowTemplateError($template->error()); @@ -317,28 +166,18 @@ if ($action eq 'new') { # if ($action eq 'del') { - CheckMilestone($product, $milestone); - my $product_id = get_product_id($product); - my $dbh = Bugzilla->dbh; - - $vars->{'default_milestone'} = - $dbh->selectrow_array('SELECT defaultmilestone - FROM products WHERE id = ?', - undef, $product_id); - - trick_taint($milestone); - $vars->{'name'} = $milestone; - $vars->{'product'} = $product; + my $milestone = Bugzilla::Milestone::check_milestone($product, + $milestone_name); + + $vars->{'name'} = $milestone->name; + $vars->{'product'} = $product->name; # The default milestone cannot be deleted. - if ($vars->{'default_milestone'} eq $milestone) { + if ($product->default_milestone eq $milestone->name) { ThrowUserError("milestone_is_default", $vars); } - $vars->{'bug_count'} = - $dbh->selectrow_array("SELECT COUNT(bug_id) FROM bugs - WHERE product_id = ? AND target_milestone = ?", - undef, ($product_id, $milestone)) || 0; + $vars->{'bug_count'} = $milestone->bug_count; $template->process("admin/milestones/confirm-delete.html.tmpl", $vars) || ThrowTemplateError($template->error()); @@ -352,21 +191,15 @@ if ($action eq 'del') { # if ($action eq 'delete') { - CheckMilestone($product, $milestone); - my $product_id = get_product_id($product); - my $dbh = Bugzilla->dbh; - my $default_milestone = - $dbh->selectrow_array("SELECT defaultmilestone - FROM products WHERE id = ?", - undef, $product_id); - - trick_taint($milestone); - $vars->{'name'} = $milestone; - $vars->{'product'} = $product; + my $milestone = + Bugzilla::Milestone::check_milestone($product, + $milestone_name); + $vars->{'name'} = $milestone->name; + $vars->{'product'} = $product->name; # The default milestone cannot be deleted. - if ($milestone eq $default_milestone) { + if ($milestone->name eq $product->default_milestone) { ThrowUserError("milestone_is_default", $vars); } @@ -375,7 +208,7 @@ if ($action eq 'delete') { my $bug_ids = $dbh->selectcol_arrayref("SELECT bug_id FROM bugs WHERE product_id = ? AND target_milestone = ?", - undef, ($product_id, $milestone)); + undef, ($product->id, $milestone->name)); my $nb_bugs = scalar(@$bug_ids); if ($nb_bugs) { @@ -383,17 +216,20 @@ if ($action eq 'delete') { foreach my $bug_id (@$bug_ids) { $dbh->do("UPDATE bugs SET target_milestone = ?, delta_ts = ? WHERE bug_id = ?", - undef, ($default_milestone, $timestamp, $bug_id)); + undef, ($product->default_milestone, $timestamp, + $bug_id)); # We have to update the 'bugs_activity' table too. - LogActivityEntry($bug_id, 'target_milestone', $milestone, - $default_milestone, $whoid, $timestamp); + LogActivityEntry($bug_id, 'target_milestone', + $milestone->name, + $product->default_milestone, + $whoid, $timestamp); } } $vars->{'bug_count'} = $nb_bugs; $dbh->do("DELETE FROM milestones WHERE product_id = ? AND value = ?", - undef, ($product_id, $milestone)); + undef, ($product->id, $milestone->name)); unlink "$datadir/versioncache"; @@ -412,25 +248,13 @@ if ($action eq 'delete') { if ($action eq 'edit') { - CheckMilestone($product, $milestone); - my $product_id = get_product_id($product); - - my $dbh = Bugzilla->dbh; - - my $sth = $dbh->prepare_cached('SELECT sortkey - FROM milestones - WHERE product_id = ? - AND value = ?'); - - trick_taint($milestone); + my $milestone = + Bugzilla::Milestone::check_milestone($product, + $milestone_name); - $vars->{'sortkey'} = $dbh->selectrow_array($sth, - undef, - $product_id, - $milestone) || 0; - - $vars->{'name'} = $milestone; - $vars->{'product'} = $product; + $vars->{'sortkey'} = $milestone->sortkey; + $vars->{'name'} = $milestone->name; + $vars->{'product'} = $product->name; $template->process("admin/milestones/edit.html.tmpl", $vars) @@ -447,80 +271,77 @@ if ($action eq 'edit') { if ($action eq 'update') { - my $milestoneold = trim($cgi->param('milestoneold') || ''); - my $sortkeyold = trim($cgi->param('sortkeyold') || '0'); - - CheckMilestone($product, $milestoneold); - my $product_id = get_product_id($product); + my $milestone_old_name = trim($cgi->param('milestoneold') || ''); + my $milestone_old = + Bugzilla::Milestone::check_milestone($product, + $milestone_old_name); - if (length($milestone) > 20) { + if (length($milestone_name) > 20) { ThrowUserError('milestone_name_too_long', - {'name' => $milestone}); + {'name' => $milestone_name}); } - my $dbh = Bugzilla->dbh; - $dbh->bz_lock_tables('bugs WRITE', 'milestones WRITE', 'products WRITE'); - if ($sortkey ne $sortkeyold) { - $sortkey = CheckSortkey($milestone, $sortkey); - - trick_taint($milestoneold); + if ($sortkey ne $milestone_old->sortkey) { + $sortkey = Bugzilla::Milestone::check_sort_key($milestone_name, + $sortkey); $dbh->do('UPDATE milestones SET sortkey = ? WHERE product_id = ? AND value = ?', undef, $sortkey, - $product_id, - $milestoneold); + $product->id, + $milestone_old->name); unlink "$datadir/versioncache"; $vars->{'updated_sortkey'} = 1; $vars->{'sortkey'} = $sortkey; } - if ($milestone ne $milestoneold) { - unless ($milestone) { + if ($milestone_name ne $milestone_old->name) { + unless ($milestone_name) { ThrowUserError('milestone_blank_name'); } - if (TestMilestone($product, $milestone)) { + my $milestone = + new Bugzilla::Milestone($product->id, $milestone_name); + if ($milestone) { ThrowUserError('milestone_already_exists', - {'name' => $milestone, - 'product' => $product}); + {'name' => $milestone->name, + 'product' => $product->name}); } - trick_taint($milestone); - trick_taint($milestoneold); + trick_taint($milestone_name); $dbh->do('UPDATE bugs SET target_milestone = ? WHERE target_milestone = ? AND product_id = ?', undef, - $milestone, - $milestoneold, - $product_id); + $milestone_name, + $milestone_old->name, + $product->id); $dbh->do("UPDATE milestones SET value = ? WHERE product_id = ? AND value = ?", undef, - $milestone, - $product_id, - $milestoneold); + $milestone_name, + $product->id, + $milestone_old->name); $dbh->do("UPDATE products SET defaultmilestone = ? WHERE id = ? AND defaultmilestone = ?", undef, - $milestone, - $product_id, - $milestoneold); + $milestone_name, + $product->id, + $milestone_old->name); unlink "$datadir/versioncache"; @@ -529,8 +350,8 @@ if ($action eq 'update') { $dbh->bz_unlock_tables(); - $vars->{'name'} = $milestone; - $vars->{'product'} = $product; + $vars->{'name'} = $milestone_name; + $vars->{'product'} = $product->name; $template->process("admin/milestones/updated.html.tmpl", $vars) || ThrowTemplateError($template->error()); diff --git a/template/en/default/admin/milestones/confirm-delete.html.tmpl b/template/en/default/admin/milestones/confirm-delete.html.tmpl index eda5add36..61601d185 100644 --- a/template/en/default/admin/milestones/confirm-delete.html.tmpl +++ b/template/en/default/admin/milestones/confirm-delete.html.tmpl @@ -23,8 +23,6 @@ [%# INTERFACE: # name: string; The name of the milestone # - # default_milestone: string; The default milestone for the product - # # bug_count: number; The number of bugs targetted at the milestone # # product: string; The name of the product diff --git a/template/en/default/admin/milestones/edit.html.tmpl b/template/en/default/admin/milestones/edit.html.tmpl index 6b5ec8fb0..417a69276 100644 --- a/template/en/default/admin/milestones/edit.html.tmpl +++ b/template/en/default/admin/milestones/edit.html.tmpl @@ -52,7 +52,6 @@ - -- cgit v1.2.3-24-g4f1b