summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Bugzilla/Constants.pm10
-rw-r--r--Bugzilla/Milestone.pm265
-rw-r--r--Bugzilla/Object.pm1
-rwxr-xr-xeditmilestones.cgi183
-rw-r--r--template/en/default/admin/milestones/updated.html.tmpl11
-rw-r--r--template/en/default/global/user-error.html.tmpl12
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 &le; sortkey
- &le; 32767.
+ The sortkey '[% sortkey FILTER html %]' is not in the range
+ [%+ constants.MIN_SMALLINT FILTER html %] &le; sortkey &le;
+ [%+ constants.MAX_SMALLINT FILTER html %].
[% ELSIF error == "misarranged_dates" %]
[% title = "Misarranged Dates" %]