Bug 372700: Make Bugzilla::Bug do bug updating for moving in process_bug.cgi

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

4 files changed, 114 insertions, 62 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 9137a082d..78c939b98 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -140,7 +140,16 @@ sub VALIDATORS {
     return $validators;
 };
 
-use constant UPDATE_COLUMNS => qw();
+use constant UPDATE_VALIDATORS => {
+    bug_status => \&_check_bug_status,
+    resolution => \&_check_resolution,
+};
+
+use constant UPDATE_COLUMNS => qw(
+    everconfirmed
+    bug_status
+    resolution
+);
 
 # This is used by add_comment to know what we validate before putting in
 # the DB.
@@ -363,7 +372,7 @@ sub run_create_validators {
     delete $params->{product};
 
     ($params->{bug_status}, $params->{everconfirmed})
-        = $class->_check_bug_status($product, $params->{bug_status});
+        = $class->_check_bug_status($params->{bug_status}, $product);
 
     $params->{target_milestone} = $class->_check_target_milestone($product,
         $params->{target_milestone});
@@ -412,13 +421,15 @@ sub run_create_validators {
 
 sub update {
     my $self = shift;
-    my $changes = $self->SUPER::update(@_);
 
+    my $dbh = Bugzilla->dbh;
     # XXX This is just a temporary hack until all updating happens
     # inside this function.
-    my $delta_ts = shift;
+    my $delta_ts = shift || $dbh->selectrow_array("SELECT NOW()");
+    $self->{delta_ts} = $delta_ts;
+
+    my $changes = $self->SUPER::update(@_);
 
-    my $dbh = Bugzilla->dbh;
     foreach my $comment (@{$self->{added_comments} || []}) {
         my $columns = join(',', keys %$comment);
         my @values  = values %$comment;
@@ -428,6 +439,25 @@ sub update {
                  $self->bug_id, Bugzilla->user->id, $delta_ts, @values);
     }
 
+    # Log bugs_activity items
+    # XXX Eventually, when bugs_activity is able to track the dupe_id,
+    # this code should go below the duplicates-table-updating code below.
+    foreach my $field (keys %$changes) {
+        my $change = $changes->{$field};
+        LogActivityEntry($self->id, $field, $change->[0], $change->[1], 
+                         Bugzilla->user->id, $delta_ts);
+    }
+
+    # If this bug is no longer a duplicate, it no longer belongs in the
+    # dup table.
+    if (exists $changes->{'resolution'} 
+        && $changes->{'resolution'}->[0] eq 'DUPLICATE') 
+    {
+        my $dup_id = $self->dup_id;
+        $dbh->do("DELETE FROM duplicates WHERE dupe = ?", undef, $self->id);
+        $changes->{'dupe_of'} = [$dup_id, undef];
+    }
+
     return $changes;
 }
 
@@ -547,30 +577,43 @@ sub _check_bug_severity {
 }
 
 sub _check_bug_status {
-    my ($invocant, $product, $status) = @_;
+    my ($invocant, $status, $product) = @_;
     my $user = Bugzilla->user;
 
-    my @valid_statuses = VALID_ENTRY_STATUS;
-
-    if ($user->in_group('editbugs', $product->id)
-        || $user->in_group('canconfirm', $product->id)) {
-       # Default to NEW if the user with privs hasn't selected another status.
-       $status ||= 'NEW';
-    }
-    elsif (!$product->votes_to_confirm) {
-        # Without privs, products that don't support UNCONFIRMED default to
-        # NEW.
-        $status = 'NEW';
+    my %valid_statuses;
+    if (ref $invocant) {
+        $invocant->{'prod_obj'} ||= 
+            new Bugzilla::Product({name => $invocant->product});
+        $product = $invocant->{'prod_obj'};
+        my $field = new Bugzilla::Field({ name => 'bug_status' });
+        %valid_statuses = map { $_ => 1 } @{$field->legal_values};
     }
     else {
-        $status = 'UNCONFIRMED';
+        %valid_statuses = map { $_ => 1 } VALID_ENTRY_STATUS;
+
+        if ($user->in_group('editbugs', $product->id)
+            || $user->in_group('canconfirm', $product->id)) {
+           # Default to NEW if the user with privs hasn't selected another
+           # status.
+           $status ||= 'NEW';
+        }
+        elsif (!$product->votes_to_confirm) {
+            # Without privs, products that don't support UNCONFIRMED default 
+            # to NEW.
+            $status = 'NEW';
+        }
+        else {
+            $status = 'UNCONFIRMED';
+        }
     }
 
     # UNCONFIRMED becomes an invalid status if votes_to_confirm is 0,
     # even if you are in editbugs.
-    shift @valid_statuses if !$product->votes_to_confirm;
+    delete $valid_statuses{'UNCONFIRMED'} if !$product->votes_to_confirm;
 
-    check_field('bug_status', $status, \@valid_statuses);
+    check_field('bug_status', $status, [keys %valid_statuses]);
+
+    return $status if ref $invocant;
     return ($status, $status eq 'UNCONFIRMED' ? 0 : 1);
 }
 
@@ -794,6 +837,13 @@ sub _check_rep_platform {
     return $platform;
 }
 
+sub _check_resolution {
+    my ($invocant, $resolution) = @_;
+    $resolution = trim($resolution);
+    check_field('resolution', $resolution);
+    return $resolution;
+}
+
 sub _check_short_desc {
     my ($invocant, $short_desc) = @_;
     # Set the parameter to itself, but cleaned up
@@ -930,6 +980,24 @@ sub fields {
 # Mutators 
 #####################################################################
 
+#################
+# "Set" Methods #
+#################
+
+sub _set_everconfirmed { $_[0]->set('everconfirmed', $_[1]); }
+sub set_resolution     { $_[0]->set('resolution',    $_[1]); }
+sub set_status { 
+    my ($self, $status) = @_;
+    $self->set('bug_status', $status); 
+    # Check for the everconfirmed transition
+    $self->_set_everconfirmed(1) if ($status eq 'NEW'
+                                     || $status eq 'ASSIGNED');
+}
+
+########################
+# "Add/Remove" Methods #
+########################
+
 # $bug->add_comment("comment", {isprivate => 1, work_time => 10.5,
 #                               type => CMT_NORMAL, extra_data => $data});
 sub add_comment {
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index ae4fbeebf..6775d4719 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -30,6 +30,8 @@ use constant NAME_FIELD => 'name';
 use constant ID_FIELD   => 'id';
 use constant LIST_ORDER => NAME_FIELD;
 
+use constant UPDATE_VALIDATORS => {};
+
 ###############################
 ####    Initialization     ####
 ###############################
@@ -136,8 +138,8 @@ sub new_from_list {
 ####      Accessors      ######
 ###############################
 
-sub id                { return $_[0]->{'id'};          }
-sub name              { return $_[0]->{'name'};        }
+sub id   { return $_[0]->{$_[0]->ID_FIELD};   }
+sub name { return $_[0]->{$_[0]->NAME_FIELD}; }
 
 ###############################
 ####        Methods        ####
@@ -153,9 +155,9 @@ sub set {
                             superclass => __PACKAGE__,
                             function   => 'Bugzilla::Object->set' });
 
-    my $validators = $self->VALIDATORS;
-    if (exists $validators->{$field}) {
-        my $validator = $validators->{$field};
+    my %validators = (%{$self->VALIDATORS}, %{$self->UPDATE_VALIDATORS});
+    if (exists $validators{$field}) {
+        my $validator = $validators{$field};
         $value = $self->$validator($value, $field);
     }
 
@@ -374,6 +376,14 @@ These functions should call L<Bugzilla::Error/ThrowUserError> if they fail.
 
 The validator must return the validated value.
 
+=item C<UPDATE_VALIDATORS>
+
+This is just like L</VALIDATORS>, but these validators are called only
+when updating an object, not when creating it. Any validator that appears
+here must not appear in L</VALIDATORS>.
+
+L<Bugzilla::Bug> has good examples in its code of when to use this.
+
 =item C<UPDATE_COLUMNS>
 
 A list of columns to update when L</update> is called.
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 3e952e56d..df97682f4 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -82,7 +82,7 @@ use constant DB_TABLE => 'profiles';
 # Bugzilla::User used "name" for the realname field. This should be
 # fixed one day.
 use constant DB_COLUMNS => (
-    'profiles.userid     AS id',
+    'profiles.userid',
     'profiles.login_name',
     'profiles.realname',
     'profiles.mybugslink AS showmybugslink',
@@ -368,7 +368,7 @@ sub groups {
                                                AND user_id=?
                                                AND isbless=0},
                                           { Columns=>[1,2] },
-                                          $self->{id});
+                                          $self->id);
 
     # The above gives us an arrayref [name, id, name, id, ...]
     # Convert that into a hashref
@@ -563,7 +563,7 @@ sub can_edit_product {
     my ($self, $prod_id) = @_;
     my $dbh = Bugzilla->dbh;
     my $sth = $self->{sthCanEditProductId};
-    my $userid = $self->{id};
+    my $userid = $self->id;
     my $query = q{SELECT group_id FROM group_control_map 
                    WHERE product_id =? 
                      AND canedit != 0 };
@@ -583,7 +583,7 @@ sub can_see_bug {
     my ($self, $bugid) = @_;
     my $dbh = Bugzilla->dbh;
     my $sth  = $self->{sthCanSeeBug};
-    my $userid  = $self->{id};
+    my $userid  = $self->id;
     # Get fields from bug, presence of user on cclist, and determine if
     # the user is missing any groups required by the bug. The prepared query
     # is cached because this may be called for every row in buglists or
diff --git a/process_bug.cgi b/process_bug.cgi
index 18097d084..a8603dc44 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -554,52 +554,26 @@ if ($action eq Bugzilla->params->{'move-button-text'}) {
     $user->is_mover || ThrowUserError("auth_failure", {action => 'move',
                                                        object => 'bugs'});
 
-    # Moved bugs are marked as RESOLVED MOVED.
-    my $sth = $dbh->prepare("UPDATE bugs
-                                SET bug_status = 'RESOLVED',
-                                    resolution = 'MOVED',
-                                    delta_ts = ?
-                              WHERE bug_id = ?");
-    # Bugs cannot be a dupe and moved at the same time.
-    my $sth2 = $dbh->prepare("DELETE FROM duplicates WHERE dupe = ?");
-
-    my $comment = "";
-    if (defined $cgi->param('comment') && $cgi->param('comment') !~ /^\s*$/) {
-        $comment = $cgi->param('comment');
-    }
-
     $dbh->bz_lock_tables('bugs WRITE', 'bugs_activity WRITE', 'duplicates WRITE',
                          'longdescs WRITE', 'profiles READ', 'groups READ',
                          'bug_group_map READ', 'group_group_map READ',
                          'user_group_map READ', 'classifications READ',
                          'products READ', 'components READ', 'votes READ',
-                         'cc READ', 'fielddefs READ');
+                         'cc READ', 'fielddefs READ', 'bug_status READ',
+                         'resolution READ');
 
-    my $timestamp = $dbh->selectrow_array("SELECT NOW()");
     my @bugs;
     # First update all moved bugs.
     foreach my $id (@idlist) {
         my $bug = new Bugzilla::Bug($id);
         push(@bugs, $bug);
-        $bug->add_comment($comment, { type       => CMT_MOVED_TO, 
-                                      extra_data => $user->login });
+        $bug->add_comment(scalar $cgi->param('comment'), 
+                          { type => CMT_MOVED_TO, extra_data => $user->login });
+        $bug->set_status('RESOLVED');
+        $bug->set_resolution('MOVED');
     }
-    foreach my $bug (@bugs) {
-        $bug->update($timestamp);
-        my $id = $bug->bug_id;
-
-        $sth->execute($timestamp, $id);
-        $sth2->execute($id);
 
-        if ($bug->bug_status ne 'RESOLVED') {
-            LogActivityEntry($id, 'bug_status', $bug->bug_status,
-                             'RESOLVED', $whoid, $timestamp);
-        }
-        if ($bug->resolution ne 'MOVED') {
-            LogActivityEntry($id, 'resolution', $bug->resolution,
-                             'MOVED', $whoid, $timestamp);
-        }
-    }
+    $_->update() foreach @bugs;
     $dbh->bz_unlock_tables();
 
     # Now send emails.