Bug 313129: Implement $milestone->create and $milestone->update based on Object.pm - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=mkanat

3 files changed, 251 insertions, 25 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);