Bug 339384: Make Bugzilla::Milestone use Bugzilla::Object - Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit a=justdave

7 files changed, 55 insertions, 58 deletions

diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 98a455e7b..661677319 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -490,6 +490,8 @@ use constant ABSTRACT_SCHEMA => {
 
     versions => {
         FIELDS => [
+            id         =>  {TYPE => 'MEDIUMSERIAL', NOTNULL => 1,
+                            PRIMARYKEY => 1},
             value      =>  {TYPE => 'varchar(64)', NOTNULL => 1},
             product_id =>  {TYPE => 'INT2', NOTNULL => 1},
         ],
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index d78db3f5d..aa8e52109 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -516,6 +516,8 @@ sub update_table_definitions {
 
     $dbh->bz_add_column('versions', 'id', 
         {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1});
+    $dbh->bz_add_column('milestones', 'id',
+        {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1});
 
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm
index 7b5d47d49..2e70b4e2d 100644
--- a/Bugzilla/Milestone.pm
+++ b/Bugzilla/Milestone.pm
@@ -13,11 +13,14 @@
 # The Original Code is the Bugzilla Bug Tracking System.
 #
 # Contributor(s): Tiago R. Mello <timello@async.com.br>
+#                 Max Kanat-Alexander <mkanat@bugzilla.org>
 
 use strict;
 
 package Bugzilla::Milestone;
 
+use base qw(Bugzilla::Object);
+
 use Bugzilla::Util;
 use Bugzilla::Error;
 
@@ -27,50 +30,45 @@ use Bugzilla::Error;
 
 use constant DEFAULT_SORTKEY => 0;
 
+use constant DB_TABLE => 'milestones';
+
 use constant DB_COLUMNS => qw(
-    milestones.value
-    milestones.product_id
-    milestones.sortkey
+    id
+    value
+    product_id
+    sortkey
 );
 
-my $columns = join(", ", DB_COLUMNS);
+use constant NAME_FIELD => 'value';
+use constant LIST_ORDER => 'sortkey, value';
 
 sub new {
-    my $invocant = shift;
-    my $class = ref($invocant) || $invocant;
-    my $self = {};
-    bless($self, $class);
-    return $self->_init(@_);
-}
-
-sub _init {
-    my $self = shift;
-    my ($product_id, $value) = (@_);
+    my $class = shift;
+    my $param = shift;
     my $dbh = Bugzilla->dbh;
 
-    my $milestone;
-
-    if (defined $product_id
-        && detaint_natural($product_id)
-        && defined $value) {
-
-        trick_taint($value);
-        $milestone = $dbh->selectrow_hashref(qq{
-            SELECT $columns FROM milestones
-            WHERE value = ?
-            AND product_id = ?}, undef, ($value, $product_id));
-    } else {
-        ThrowCodeError('bad_arg',
-            {argument => 'product_id/value',
-             function => 'Bugzilla::Milestone::_init'});
+    my $product;
+    if (ref $param) {
+        $product = $param->{product};
+        my $name = $param->{name};
+        if (!defined $product) {
+            ThrowCodeError('bad_arg',
+                {argument => 'product',
+                 function => "${class}::new"});
+        }
+        if (!defined $name) {
+            ThrowCodeError('bad_arg',
+                {argument => 'name',
+                 function => "${class}::new"});
+        }
+
+        my $condition = 'product_id = ? AND value = ?';
+        my @values = ($product->id, $name);
+        $param = { condition => $condition, values => \@values };
     }
 
-    return undef unless (defined $milestone);
-
-    foreach my $field (keys %$milestone) {
-        $self->{$field} = $milestone->{$field};
-    }
-    return $self;
+    unshift @_, $param;
+    return $class->SUPER::new(@_);
 }
 
 sub bug_count {
@@ -105,8 +103,8 @@ sub check_milestone {
         ThrowUserError('milestone_not_specified');
     }
 
-    my $milestone = new Bugzilla::Milestone($product->id,
-                                            $milestone_name);
+    my $milestone = new Bugzilla::Milestone({ product => $product,
+                                              name    => $milestone_name });
     unless ($milestone) {
         ThrowUserError('milestone_not_valid',
                        {'product' => $product->name,
@@ -141,7 +139,8 @@ Bugzilla::Milestone - Bugzilla product milestone class.
 
     use Bugzilla::Milestone;
 
-    my $milestone = new Bugzilla::Milestone(1, 'milestone_value');
+    my $milestone = new Bugzilla::Milestone(
+        { product => $product, name => 'milestone_value' });
 
     my $product_id = $milestone->product_id;
     my $value = $milestone->value;
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index c525efc11..728bd0652 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -120,16 +120,11 @@ sub milestones {
     my $dbh = Bugzilla->dbh;
 
     if (!defined $self->{milestones}) {
-        my $values = $dbh->selectcol_arrayref(q{
-            SELECT value FROM milestones
-            WHERE product_id = ?
-            ORDER BY sortkey}, undef, $self->id);
+        my $ids = $dbh->selectcol_arrayref(q{
+            SELECT id FROM milestones
+             WHERE product_id = ?}, undef, $self->id);
  
-        my @milestones;
-        foreach my $value (@$values) {
-            push @milestones, new Bugzilla::Milestone($self->id, $value);
-        }
-        $self->{milestones} = \@milestones;
+        $self->{milestones} = Bugzilla::Milestone->new_from_list($ids);
     }
     return $self->{milestones};
 }
diff --git a/editmilestones.cgi b/editmilestones.cgi
index 2df40451a..17733bdb1 100755
--- a/editmilestones.cgi
+++ b/editmilestones.cgi
@@ -130,8 +130,8 @@ if ($action eq 'new') {
     $sortkey = Bugzilla::Milestone::check_sort_key($milestone_name,
                                                    $sortkey);
 
-    my $milestone = new Bugzilla::Milestone($product->id,
-                                            $milestone_name);
+    my $milestone = new Bugzilla::Milestone(
+        { product => $product, name => $milestone_name });
 
     if ($milestone) {
         ThrowUserError('milestone_already_exists',
@@ -145,8 +145,8 @@ if ($action eq 'new') {
               VALUES ( ?, ?, ? )',
              undef, $milestone_name, $product->id, $sortkey);
 
-    $milestone = new Bugzilla::Milestone($product->id,
-                                         $milestone_name);
+    $milestone = new Bugzilla::Milestone(
+        { product => $product, name => $milestone_name });
     delete_token($token);
 
     $vars->{'milestone'} = $milestone;
@@ -301,8 +301,8 @@ if ($action eq 'update') {
         unless ($milestone_name) {
             ThrowUserError('milestone_blank_name');
         }
-        my $milestone = 
-            new Bugzilla::Milestone($product->id, $milestone_name);
+        my $milestone = new Bugzilla::Milestone(
+            { product => $product, name => $milestone_name });
         if ($milestone) {
             ThrowUserError('milestone_already_exists',
                            {'name' => $milestone->name,
diff --git a/editproducts.cgi b/editproducts.cgi
index 8e42130dc..b458f60bf 100755
--- a/editproducts.cgi
+++ b/editproducts.cgi
@@ -851,8 +851,8 @@ if ($action eq 'update') {
 
     # Only update milestone related stuff if 'usetargetmilestone' is on.
     if (Bugzilla->params->{'usetargetmilestone'}) {
-        my $milestone = new Bugzilla::Milestone($product_old->id,
-                                                $defaultmilestone);
+        my $milestone = new Bugzilla::Milestone(
+            { product => $product_old, name => $defaultmilestone });
 
         unless ($milestone) {
             ThrowUserError('product_must_define_defaultmilestone',
diff --git a/importxml.pl b/importxml.pl
index 5b14fc655..8f93ab46c 100755
--- a/importxml.pl
+++ b/importxml.pl
@@ -692,9 +692,8 @@ sub process_bug {
 
     # Milestone
     if ( $params->{"usetargetmilestone"} ) {
-        my $milestone =
-          new Bugzilla::Milestone( $product->id,
-                                   $bug_fields{'target_milestone'} );
+        my $milestone = new Bugzilla::Milestone(
+            { product => $product, name => $bug_fields{'target_milestone'} });
         if ($milestone) {
             push( @values, $milestone->name );
         }