diff options
| -rw-r--r-- | Bugzilla/Constants.pm | 10 | ||||
| -rw-r--r-- | Bugzilla/Milestone.pm | 265 | ||||
| -rw-r--r-- | Bugzilla/Object.pm | 1 | ||||
| -rwxr-xr-x | editmilestones.cgi | 183 | ||||
| -rw-r--r-- | template/en/default/admin/milestones/updated.html.tmpl | 11 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 12 |
6 files changed, 280 insertions, 202 deletions
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 7ac6048c4..99edb2f50 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -142,7 +142,11 @@ use File::Basename; SAFE_PROTOCOLS + MIN_SMALLINT + MAX_SMALLINT + MAX_LEN_QUERY_NAME + MAX_MILESTONE_SIZE ); @Bugzilla::Constants::EXPORT_OK = qw(contenttypes); @@ -397,9 +401,15 @@ use constant ROOT_USER => $^O =~ /MSWin32/i ? 'Administrator' : 'root'; # True if we're on Win32. use constant ON_WINDOWS => ($^O =~ /MSWin32/i); +use constant MIN_SMALLINT => -32768; +use constant MAX_SMALLINT => 32767; + # The longest that a saved search name can be. use constant MAX_LEN_QUERY_NAME => 64; +# The longest milestone name allowed. +use constant MAX_MILESTONE_SIZE => 20; + sub bz_locations { # We know that Bugzilla/Constants.pm must be in %INC at this point. # So the only question is, what's the name of the directory diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm index 596c34bd8..7a0a81a42 100644 --- a/Bugzilla/Milestone.pm +++ b/Bugzilla/Milestone.pm @@ -14,6 +14,7 @@ # # Contributor(s): Tiago R. Mello <timello@async.com.br> # Max Kanat-Alexander <mkanat@bugzilla.org> +# Frédéric Buclin <LpSolit@gmail.com> use strict; @@ -21,6 +22,7 @@ package Bugzilla::Milestone; use base qw(Bugzilla::Object); +use Bugzilla::Constants; use Bugzilla::Util; use Bugzilla::Error; @@ -31,6 +33,8 @@ use Bugzilla::Error; use constant DEFAULT_SORTKEY => 0; use constant DB_TABLE => 'milestones'; +use constant NAME_FIELD => 'value'; +use constant LIST_ORDER => 'sortkey, value'; use constant DB_COLUMNS => qw( id @@ -39,8 +43,26 @@ use constant DB_COLUMNS => qw( sortkey ); -use constant NAME_FIELD => 'value'; -use constant LIST_ORDER => 'sortkey, value'; +use constant REQUIRED_CREATE_FIELDS => qw( + name + product +); + +use constant UPDATE_COLUMNS => qw( + value + sortkey +); + +use constant VALIDATORS => { + product => \&_check_product, + sortkey => \&_check_sortkey, +}; + +use constant UPDATE_VALIDATORS => { + value => \&_check_value, +}; + +################################ sub new { my $class = shift; @@ -71,6 +93,118 @@ sub new { return $class->SUPER::new(@_); } +sub run_create_validators { + my $class = shift; + my $params = $class->SUPER::run_create_validators(@_); + + my $product = delete $params->{product}; + $params->{product_id} = $product->id; + $params->{value} = $class->_check_value($params->{name}, $product); + delete $params->{name}; + + return $params; +} + +sub update { + my $self = shift; + my $changes = $self->SUPER::update(@_); + + if (exists $changes->{value}) { + my $dbh = Bugzilla->dbh; + # The milestone value is stored in the bugs table instead of its ID. + $dbh->do('UPDATE bugs SET target_milestone = ? + WHERE target_milestone = ? AND product_id = ?', + undef, ($self->name, $changes->{value}->[0], $self->product_id)); + + # The default milestone also stores the value instead of the ID. + $dbh->do('UPDATE products SET defaultmilestone = ? + WHERE id = ? AND defaultmilestone = ?', + undef, ($self->name, $self->product_id, $changes->{value}->[0])); + } + return $changes; +} + +sub remove_from_db { + my $self = shift; + my $dbh = Bugzilla->dbh; + + # The default milestone cannot be deleted. + if ($self->name eq $self->product->default_milestone) { + ThrowUserError('milestone_is_default', { milestone => $self }); + } + + if ($self->bug_count) { + # We don't want to delete bugs when deleting a milestone. + # Bugs concerned are reassigned to the default milestone. + my $bug_ids = + $dbh->selectcol_arrayref('SELECT bug_id FROM bugs + WHERE product_id = ? AND target_milestone = ?', + undef, ($self->product->id, $self->name)); + + my $timestamp = $dbh->selectrow_array('SELECT NOW()'); + + $dbh->do('UPDATE bugs SET target_milestone = ?, delta_ts = ? + WHERE bug_id IN (' . join(', ', @$bug_ids) . ')', + undef, ($self->product->default_milestone, $timestamp)); + + require Bugzilla::Bug; + import Bugzilla::Bug qw(LogActivityEntry); + foreach my $bug_id (@$bug_ids) { + LogActivityEntry($bug_id, 'target_milestone', + $self->name, + $self->product->default_milestone, + Bugzilla->user->id, $timestamp); + } + } + + $dbh->do('DELETE FROM milestones WHERE id = ?', undef, $self->id); +} + +################################ +# Validators +################################ + +sub _check_value { + my ($invocant, $name, $product) = @_; + + trim($name) || ThrowUserError('milestone_blank_name'); + if (length($name) > MAX_MILESTONE_SIZE) { + ThrowUserError('milestone_name_too_long', {name => $name}); + } + + $product = $invocant->product if (ref $invocant); + my $milestone = new Bugzilla::Milestone({product => $product, name => $name}); + if ($milestone && (!ref $invocant || $milestone->id != $invocant->id)) { + ThrowUserError('milestone_already_exists', { name => $milestone->name, + product => $product->name }); + } + return $name; +} + +sub _check_sortkey { + my ($invocant, $sortkey) = @_; + + # Keep a copy in case detaint_signed() clears the sortkey + my $stored_sortkey = $sortkey; + + if (!detaint_signed($sortkey) || $sortkey < MIN_SMALLINT || $sortkey > MAX_SMALLINT) { + ThrowUserError('milestone_sortkey_invalid', {sortkey => $stored_sortkey}); + } + return $sortkey; +} + +sub _check_product { + my ($invocant, $product) = @_; + return Bugzilla->user->check_can_admin_product($product->name); +} + +################################ +# Methods +################################ + +sub set_name { $_[0]->set('value', $_[1]); } +sub set_sortkey { $_[0]->set('sortkey', $_[1]); } + sub bug_count { my $self = shift; my $dbh = Bugzilla->dbh; @@ -92,22 +226,12 @@ sub name { return $_[0]->{'value'}; } sub product_id { return $_[0]->{'product_id'}; } sub sortkey { return $_[0]->{'sortkey'}; } -################################ -##### Subroutines ##### -################################ - -sub check_sort_key { - my ($milestone_name, $sortkey) = @_; - # Keep a copy in case detaint_signed() clears the sortkey - my $stored_sortkey = $sortkey; +sub product { + my $self = shift; - if (!detaint_signed($sortkey) || $sortkey < -32768 - || $sortkey > 32767) { - ThrowUserError('milestone_sortkey_invalid', - {'name' => $milestone_name, - 'sortkey' => $stored_sortkey}); - } - return $sortkey; + require Bugzilla::Product; + $self->{'product'} ||= new Bugzilla::Product($self->product_id); + return $self->{'product'}; } 1; @@ -122,13 +246,21 @@ Bugzilla::Milestone - Bugzilla product milestone class. use Bugzilla::Milestone; - my $milestone = new Bugzilla::Milestone( - { product => $product, name => 'milestone_value' }); + my $milestone = new Bugzilla::Milestone({ name => $name, product => $product }); + my $name = $milestone->name; my $product_id = $milestone->product_id; - my $value = $milestone->value; + my $product = $milestone->product; + my $sortkey = $milestone->sortkey; + + my $milestone = Bugzilla::Milestone->create( + { name => $name, product => $product, sortkey => $sortkey }); - my $milestone = $hash_ref->{'milestone_value'}; + $milestone->set_name($new_name); + $milestone->set_sortkey($new_sortkey); + $milestone->update(); + + $milestone->remove_from_db; =head1 DESCRIPTION @@ -138,16 +270,48 @@ Milestone.pm represents a Product Milestone object. =over -=item C<new($product_id, $value)> +=item C<new({name => $name, product => $product})> Description: The constructor is used to load an existing milestone - by passing a product id and a milestone value. + by passing a product object and a milestone name. - Params: $product_id - Integer with a Bugzilla product id. - $value - String with a milestone value. + Params: $product - a Bugzilla::Product object. + $name - the name of a milestone (string). Returns: A Bugzilla::Milestone object. +=item C<name()> + + Description: Name (value) of the milestone. + + Params: none. + + Returns: The name of the milestone. + +=item C<product_id()> + + Description: ID of the product the milestone belongs to. + + Params: none. + + Returns: The ID of a product. + +=item C<product()> + + Description: The product object of the product the milestone belongs to. + + Params: none. + + Returns: A Bugzilla::Product object. + +=item C<sortkey()> + + Description: Sortkey of the milestone. + + Params: none. + + Returns: The sortkey of the milestone. + =item C<bug_count()> Description: Returns the total of bugs that belong to the milestone. @@ -156,4 +320,55 @@ Milestone.pm represents a Product Milestone object. Returns: Integer with the number of bugs. +=item C<set_name($new_name)> + + Description: Changes the name of the milestone. + + Params: $new_name - new name of the milestone (string). This name + must be unique within the product. + + Returns: Nothing. + +=item C<set_sortkey($new_sortkey)> + + Description: Changes the sortkey of the milestone. + + Params: $new_sortkey - new sortkey of the milestone (signed integer). + + Returns: Nothing. + +=item C<update()> + + Description: Writes the new name and/or the new sortkey into the DB. + + Params: none. + + Returns: A hashref with changes made to the milestone object. + +=item C<remove_from_db()> + + Description: Deletes the current milestone from the DB. The object itself + is not destroyed. + + Params: none. + + Returns: Nothing. + +=back + +=head1 CLASS METHODS + +=over + +=item C<create({name => $name, product => $product, sortkey => $sortkey})> + + Description: Create a new milestone for the given product. + + Params: $name - name of the new milestone (string). This name + must be unique within the product. + $product - a Bugzilla::Product object. + $sortkey - the sortkey of the new milestone (signed integer) + + Returns: A Bugzilla::Milestone object. + =back diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 9afad7633..9e155bc10 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -215,6 +215,7 @@ sub set { if (exists $validators{$field}) { my $validator = $validators{$field}; $value = $self->$validator($value, $field); + trick_taint($value) if (defined $value && !ref($value)); if ($self->can('_set_global_validator')) { $self->_set_global_validator($value, $field); diff --git a/editmilestones.cgi b/editmilestones.cgi index 880e1d4a7..777625326 100755 --- a/editmilestones.cgi +++ b/editmilestones.cgi @@ -15,7 +15,6 @@ # Frédéric Buclin <LpSolit@gmail.com> # - use strict; use lib "."; @@ -24,7 +23,6 @@ use Bugzilla::Constants; use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::Milestone; -use Bugzilla::Bug; use Bugzilla::Token; my $cgi = Bugzilla->cgi; @@ -37,7 +35,6 @@ my $vars = {}; # my $user = Bugzilla->login(LOGIN_REQUIRED); -my $whoid = $user->id; print $cgi->header(); @@ -86,16 +83,11 @@ unless ($action) { $vars->{'showbugcounts'} = $showbugcounts; $vars->{'product'} = $product; - $template->process("admin/milestones/list.html.tmpl", - $vars) + $template->process("admin/milestones/list.html.tmpl", $vars) || ThrowTemplateError($template->error()); - exit; } - - - # # action='add' -> present form for parameters for new milestone # @@ -105,62 +97,29 @@ unless ($action) { if ($action eq 'add') { $vars->{'token'} = issue_session_token('add_milestone'); $vars->{'product'} = $product; - $template->process("admin/milestones/create.html.tmpl", - $vars) + $template->process("admin/milestones/create.html.tmpl", $vars) || ThrowTemplateError($template->error()); - exit; } - - # # action='new' -> add milestone entered in the 'action=add' screen # if ($action eq 'new') { check_token_data($token, 'add_milestone'); - $milestone_name || ThrowUserError('milestone_blank_name'); - - if (length($milestone_name) > 20) { - ThrowUserError('milestone_name_too_long', - {'name' => $milestone_name}); - } - - $sortkey = Bugzilla::Milestone::check_sort_key($milestone_name, - $sortkey); - - my $milestone = new Bugzilla::Milestone( - { product => $product, name => $milestone_name }); - - if ($milestone) { - ThrowUserError('milestone_already_exists', - {'name' => $milestone->name, - 'product' => $product->name}); - } - - # Add the new milestone - trick_taint($milestone_name); - $dbh->do('INSERT INTO milestones ( value, product_id, sortkey ) - VALUES ( ?, ?, ? )', - undef, $milestone_name, $product->id, $sortkey); - - $milestone = new Bugzilla::Milestone( - { product => $product, name => $milestone_name }); + my $milestone = Bugzilla::Milestone->create({ name => $milestone_name, + product => $product, + sortkey => $sortkey }); delete_token($token); $vars->{'milestone'} = $milestone; $vars->{'product'} = $product; - $template->process("admin/milestones/created.html.tmpl", - $vars) + $template->process("admin/milestones/created.html.tmpl", $vars) || ThrowTemplateError($template->error()); - exit; } - - - # # action='del' -> ask if user really wants to delete # @@ -176,7 +135,7 @@ if ($action eq 'del') { # The default milestone cannot be deleted. if ($product->default_milestone eq $milestone->name) { - ThrowUserError("milestone_is_default", $vars); + ThrowUserError("milestone_is_default", { milestone => $milestone }); } $vars->{'token'} = issue_session_token('delete_milestone'); @@ -185,8 +144,6 @@ if ($action eq 'del') { exit; } - - # # action='delete' -> really delete the milestone # @@ -195,47 +152,17 @@ if ($action eq 'delete') { check_token_data($token, 'delete_milestone'); my $milestone = Bugzilla::Milestone->check({ product => $product, name => $milestone_name }); + $milestone->remove_from_db; + delete_token($token); + $vars->{'milestone'} = $milestone; $vars->{'product'} = $product; - # The default milestone cannot be deleted. - if ($milestone->name eq $product->default_milestone) { - ThrowUserError("milestone_is_default", $vars); - } - - if ($milestone->bug_count) { - # We don't want to delete bugs when deleting a milestone. - # Bugs concerned are reassigned to the default milestone. - my $bug_ids = - $dbh->selectcol_arrayref("SELECT bug_id FROM bugs - WHERE product_id = ? AND target_milestone = ?", - undef, ($product->id, $milestone->name)); - my $timestamp = $dbh->selectrow_array("SELECT NOW()"); - foreach my $bug_id (@$bug_ids) { - $dbh->do("UPDATE bugs SET target_milestone = ?, - delta_ts = ? WHERE bug_id = ?", - undef, ($product->default_milestone, $timestamp, - $bug_id)); - # We have to update the 'bugs_activity' table too. - LogActivityEntry($bug_id, 'target_milestone', - $milestone->name, - $product->default_milestone, - $whoid, $timestamp); - } - } - - $dbh->do("DELETE FROM milestones WHERE product_id = ? AND value = ?", - undef, ($product->id, $milestone->name)); - - delete_token($token); - $template->process("admin/milestones/deleted.html.tmpl", $vars) || ThrowTemplateError($template->error()); exit; } - - # # action='edit' -> present the edit milestone form # @@ -251,15 +178,11 @@ if ($action eq 'edit') { $vars->{'product'} = $product; $vars->{'token'} = issue_session_token('edit_milestone'); - $template->process("admin/milestones/edit.html.tmpl", - $vars) + $template->process("admin/milestones/edit.html.tmpl", $vars) || ThrowTemplateError($template->error()); - exit; } - - # # action='update' -> update the milestone # @@ -267,93 +190,23 @@ if ($action eq 'edit') { if ($action eq 'update') { check_token_data($token, 'edit_milestone'); my $milestone_old_name = trim($cgi->param('milestoneold') || ''); - my $milestone_old = Bugzilla::Milestone->check( - { product => $product, name => $milestone_old_name }); - - if (length($milestone_name) > 20) { - ThrowUserError('milestone_name_too_long', - {'name' => $milestone_name}); - } - - $dbh->bz_lock_tables('bugs WRITE', - 'milestones WRITE', - 'products WRITE'); - - 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, - $milestone_old->name); - - $vars->{'updated_sortkey'} = 1; - } - - if ($milestone_name ne $milestone_old->name) { - unless ($milestone_name) { - ThrowUserError('milestone_blank_name'); - } - my $milestone = new Bugzilla::Milestone( - { product => $product, name => $milestone_name }); - if ($milestone) { - ThrowUserError('milestone_already_exists', - {'name' => $milestone->name, - 'product' => $product->name}); - } - - trick_taint($milestone_name); - - $dbh->do('UPDATE bugs - SET target_milestone = ? - WHERE target_milestone = ? - AND product_id = ?', - undef, - $milestone_name, - $milestone_old->name, - $product->id); - - $dbh->do("UPDATE milestones - SET value = ? - WHERE product_id = ? - AND value = ?", - undef, - $milestone_name, - $product->id, - $milestone_old->name); - - $dbh->do("UPDATE products - SET defaultmilestone = ? - WHERE id = ? - AND defaultmilestone = ?", - undef, - $milestone_name, - $product->id, - $milestone_old->name); - - $vars->{'updated_name'} = 1; - } + my $milestone = Bugzilla::Milestone->check({ product => $product, + name => $milestone_old_name }); - $dbh->bz_unlock_tables(); + $milestone->set_name($milestone_name); + $milestone->set_sortkey($sortkey); + my $changes = $milestone->update(); - my $milestone = Bugzilla::Milestone->check({ product => $product, - name => $milestone_name }); delete_token($token); $vars->{'milestone'} = $milestone; $vars->{'product'} = $product; - $template->process("admin/milestones/updated.html.tmpl", - $vars) + $vars->{'changes'} = $changes; + $template->process("admin/milestones/updated.html.tmpl", $vars) || ThrowTemplateError($template->error()); - exit; } - # # No valid action found # diff --git a/template/en/default/admin/milestones/updated.html.tmpl b/template/en/default/admin/milestones/updated.html.tmpl index 3f86e2870..daa6581dd 100644 --- a/template/en/default/admin/milestones/updated.html.tmpl +++ b/template/en/default/admin/milestones/updated.html.tmpl @@ -19,11 +19,10 @@ #%] [%# INTERFACE: + # milestone: object; the milestone being edited. # product: object; Bugzilla::Product object representing the product to # which the milestone belongs. - # - # 'updated_XXX' variables are booleans, and are defined if the - # 'XXX' field was updated during the edit just being handled. + # changes: hashref; contains changes made to the milestone. #%] [% title = BLOCK %]Updating Milestone '[% milestone.name FILTER html %]' of Product @@ -32,15 +31,15 @@ title = title %] -[% IF updated_name %] +[% IF changes.value.defined %] <p>Updated Milestone name to: '[% milestone.name FILTER html %]'.</p> [% END %] -[% IF updated_sortkey %] +[% IF changes.sortkey.defined %] <p>Updated Milestone sortkey to: '[% milestone.sortkey FILTER html %]'.</p> [% END %] -[% UNLESS updated_sortkey || updated_name %] +[% UNLESS changes.value.defined || changes.sortkey.defined %] <p>Nothing changed for milestone '[% milestone.name FILTER html %]'.</p> [% END %] diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index f72275bd5..4a5cd58d5 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -895,9 +895,9 @@ [% title = "Default milestone not deletable" %] [% admindocslinks = {'products.html' => 'Administering products', 'milestones.html' => 'About Milestones'} %] - Sorry, but [% milestone.name FILTER html %] is the default milestone - for product '[% product.name FILTER html %]', and so it can not be - deleted. + Sorry, but [% milestone.name FILTER html %] is the default milestone + for the '[% milestone.product.name FILTER html %]' product, and so + it cannot be deleted. [% ELSIF error == "milestone_name_too_long" %] [% title = "Milestone Name Is Too Long" %] @@ -913,9 +913,9 @@ [% ELSIF error == "milestone_sortkey_invalid" %] [% title = "Invalid Milestone Sortkey" %] - The sortkey '[% sortkey FILTER html %]' for milestone ' - [% name FILTER html %]' is not in the range -32768 ≤ sortkey - ≤ 32767. + The sortkey '[% sortkey FILTER html %]' is not in the range + [%+ constants.MIN_SMALLINT FILTER html %] ≤ sortkey ≤ + [%+ constants.MAX_SMALLINT FILTER html %]. [% ELSIF error == "misarranged_dates" %] [% title = "Misarranged Dates" %] |