Bug 388147: Move assigned_to and qa_contact updating into Bugzilla::Bug

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

2 files changed, 260 insertions, 338 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 7c0cc191f..466ceb988 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -74,6 +74,7 @@ sub DB_COLUMNS {
     my @custom_names = map {$_->name} @custom;
     return qw(
         alias
+        assigned_to
         bug_file_loc
         bug_id
         bug_severity
@@ -86,6 +87,7 @@ sub DB_COLUMNS {
         op_sys
         priority
         product_id
+        qa_contact
         remaining_time
         rep_platform
         reporter_accessible
@@ -95,9 +97,7 @@ sub DB_COLUMNS {
         target_milestone
         version
     ),
-    'assigned_to AS assigned_to_id',
     'reporter    AS reporter_id',
-    'qa_contact  AS qa_contact_id',
     $dbh->sql_date_format('creation_ts', '%Y.%m.%d %H:%i') . ' AS creation_ts',
     $dbh->sql_date_format('deadline', '%Y-%m-%d') . ' AS deadline',
     @custom_names;
@@ -158,8 +158,10 @@ sub VALIDATORS {
 };
 
 use constant UPDATE_VALIDATORS => {
+    assigned_to         => \&_check_assigned_to,
     bug_status          => \&_check_bug_status,
     cclist_accessible   => \&Bugzilla::Object::check_boolean,
+    qa_contact          => \&_check_qa_contact,
     reporter_accessible => \&Bugzilla::Object::check_boolean,
     resolution          => \&_check_resolution,
     target_milestone    => \&_check_target_milestone,
@@ -172,6 +174,7 @@ sub UPDATE_COLUMNS {
     my @custom_names = map {$_->name} @custom;
     my @columns = qw(
         alias
+        assigned_to
         cclist_accessible
         component_id
         deadline
@@ -183,6 +186,7 @@ sub UPDATE_COLUMNS {
         op_sys
         priority
         product_id
+        qa_contact
         remaining_time
         rep_platform
         reporter_accessible
@@ -452,9 +456,9 @@ sub run_create_validators {
     delete $params->{component};
 
     $params->{assigned_to} = 
-        $class->_check_assigned_to($component, $params->{assigned_to});
+        $class->_check_assigned_to($params->{assigned_to}, $component);
     $params->{qa_contact} =
-        $class->_check_qa_contact($component, $params->{qa_contact});
+        $class->_check_qa_contact($params->{qa_contact}, $component);
     $params->{cc} = $class->_check_cc($component, $params->{cc});
 
     # Callers cannot set Reporter, currently.
@@ -467,8 +471,8 @@ sub run_create_validators {
         $params->{remaining_time} = $params->{estimated_time};
     }
 
-    $class->_check_strict_isolation($product, $params->{cc},
-                                    $params->{assigned_to}, $params->{qa_contact});
+    $class->_check_strict_isolation($params->{cc}, $params->{assigned_to},
+                                    $params->{qa_contact}, $product);
 
     ($params->{dependson}, $params->{blocked}) = 
         $class->_check_dependencies($params->{dependson}, $params->{blocked},
@@ -545,6 +549,10 @@ sub update {
             $from  = $self->{"_old_${field}_name"};
             $to    = $self->$field;
         }
+        if (grep ($_ eq $field, qw(qa_contact assigned_to))) {
+            $from = $old_bug->$field->login if $from;
+            $to   = $self->$field->login    if $to;
+        }
         LogActivityEntry($self->id, $field, $from, $to, Bugzilla->user->id,
                          $delta_ts);
     }
@@ -814,16 +822,28 @@ sub _check_alias {
 }
 
 sub _check_assigned_to {
-    my ($invocant, $component, $name) = @_;
+    my ($invocant, $assignee, $component) = @_;
     my $user = Bugzilla->user;
 
-    $name = trim($name);
     # Default assignee is the component owner.
     my $id;
-    if (!$user->in_group('editbugs', $component->product_id) || !$name) {
+    # If this is a new bug, you can only set the assignee if you have editbugs.
+    # If you didn't specify the assignee, we use the default assignee.
+    if (!ref $invocant
+        && (!$user->in_group('editbugs', $component->product_id) || !$assignee))
+    {
         $id = $component->default_assignee->id;
     } else {
-        $id = login_to_id($name, THROW_ERROR);
+        if (!ref $assignee) {
+            $assignee = trim($assignee);
+            # When updating a bug, assigned_to can't be empty.
+            ThrowUserError("reassign_to_empty") if ref $invocant && !$assignee;
+            $assignee = Bugzilla::User->check($assignee);
+        }
+        $id = $assignee->id;
+        # create() checks this another way, so we don't have to run this
+        # check during create().
+        $invocant->_check_strict_isolation_for_user($assignee) if ref $invocant;
     }
     return $id;
 }
@@ -1134,6 +1154,38 @@ sub _check_priority {
     return $priority;
 }
 
+sub _check_qa_contact {
+    my ($invocant, $qa_contact, $component) = @_;
+    $qa_contact = trim($qa_contact) if !ref $qa_contact;
+    
+    my $id;
+    if (!ref $invocant) {
+        # Bugs get no QA Contact on creation if useqacontact is off.
+        return undef if !Bugzilla->params->{useqacontact};
+        # Set the default QA Contact if one isn't specified or if the
+        # user doesn't have editbugs.
+        if (!Bugzilla->user->in_group('editbugs', $component->product_id)
+            || !$qa_contact)
+        {
+            $id = $component->default_qa_contact->id;
+        }
+    }
+    
+    # If a QA Contact was specified or if we're updating, check
+    # the QA Contact for validity.
+    if (!defined $id && $qa_contact) {
+        $qa_contact = Bugzilla::User->check($qa_contact) if !ref $qa_contact;
+        $id = $qa_contact->id;
+        # create() checks this another way, so we don't have to run this
+        # check during create().
+        $invocant->_check_strict_isolation_for_user($qa_contact)
+            if ref $invocant;
+    }
+
+    # "0" always means "undef", for QA Contact.
+    return $id || undef;
+}
+
 sub _check_remaining_time {
     return $_[0]->_check_time($_[1], 'remaining_time');
 }
@@ -1167,33 +1219,69 @@ sub _check_status_whiteboard { return defined $_[1] ? $_[1] : ''; }
 
 # Unlike other checkers, this one doesn't return anything.
 sub _check_strict_isolation {
-    my ($invocant, $product, $cc_ids, $assignee_id, $qa_contact_id) = @_;
-
+    my ($invocant, $ccs, $assignee, $qa_contact, $product) = @_;
     return unless Bugzilla->params->{'strict_isolation'};
 
-    my @related_users = @$cc_ids;
-    push(@related_users, $assignee_id);
+    if (ref $invocant) {
+        my $original = $invocant->new($invocant->id);
+
+        # We only check people if they've been added. This way, if
+        # strict_isolation is turned on when there are invalid users
+        # on bugs, people can still add comments and so on.
+        my @old_cc = map { $_->id } @{$original->cc_users};
+        my @new_cc = map { $_->id } @{$invocant->cc_users};
+        my ($removed, $added) = diff_arrays(\@old_cc, \@new_cc);
+        $ccs = $added;
+        $assignee = $invocant->assigned_to
+            if $invocant->assigned_to->id != $original->assigned_to->id;
+        $qa_contact = $invocant->qa_contact
+            if $invocant->qa_contact->id != $original->qa_contact->id;
+        $product = $invocant->product;
+    }
+
+    my @related_users = @$ccs;
+    push(@related_users, $assignee) if $assignee;
 
-    if (Bugzilla->params->{'useqacontact'} && $qa_contact_id) {
-        push(@related_users, $qa_contact_id);
+    if (Bugzilla->params->{'useqacontact'} && $qa_contact) {
+        push(@related_users, $qa_contact);
     }
 
+    @related_users = @{Bugzilla::User->new_from_list(\@related_users)}
+        if !ref $invocant;
+
     # For each unique user in @related_users...(assignee and qa_contact
     # could be duplicates of users in the CC list)
-    my %unique_users = map {$_ => 1} @related_users;
+    my %unique_users = map {$_->id => $_} @related_users;
     my @blocked_users;
-    foreach my $pid (keys %unique_users) {
-        my $related_user = Bugzilla::User->new($pid);
+    foreach my $id (keys %unique_users) {
+        my $related_user = $unique_users{$id};
         if (!$related_user->can_edit_product($product->id) ||
             !$related_user->can_see_product($product->id)) {
             push (@blocked_users, $related_user->login);
         }
     }
     if (scalar(@blocked_users)) {
-        ThrowUserError("invalid_user_group",
-            {'users' => \@blocked_users,
-             'new' => 1,
-             'product' => $product->name});
+        my %vars = ( users   => \@blocked_users,
+                     product => $product->name );
+        if (ref $invocant) {
+            $vars{'bug_id'} = $invocant->id;
+        }
+        else {
+            $vars{'new'} = 1;
+        }
+        ThrowUserError("invalid_user_group", \%vars);
+    }
+}
+
+# This is used by various set_ checkers, to make their code simpler.
+sub _check_strict_isolation_for_user {
+    my ($self, $user) = @_;
+    return unless Bugzilla->params->{"strict_isolation"};
+    if (!$user->can_edit_product($self->{product_id})) {
+        ThrowUserError('invalid_user_group',
+                       { users   => $user->login,
+                         product => $self->product,
+                         bug_id  => $self->id });
     }
 }
 
@@ -1223,25 +1311,6 @@ sub _check_time {
     return $time;
 }
 
-sub _check_qa_contact {
-    my ($invocant, $component, $name) = @_;
-    my $user = Bugzilla->user;
-
-    return undef unless Bugzilla->params->{'useqacontact'};
-
-    $name = trim($name);
-
-    my $id;
-    if (!$user->in_group('editbugs', $component->product_id) || !$name) {
-        # We want to insert NULL into the database if we get a 0.
-        $id = $component->default_qa_contact->id || undef;
-    } else {
-        $id = login_to_id($name, THROW_ERROR);
-    }
-
-    return $id;
-}
-
 sub _check_version {
     my ($invocant, $version, $product) = @_;
     $version = trim($version);
@@ -1331,13 +1400,21 @@ sub fields {
 # To run check_can_change_field.
 sub _set_global_validator {
     my ($self, $value, $field) = @_;
-    my $current_value = $self->$field;
+    my $current = $self->$field;
     my $privs;
-    $self->check_can_change_field($field, $current_value, $value, \$privs)
-        || ThrowUserError('illegal_change', { field    => $field,
-                                              oldvalue => $current_value,
-                                              newvalue => $value,
-                                              privs    => $privs });
+    $current = $current->id if ref $current && $current->isa('Bugzilla::Object');
+    $value   = $value->id   if ref $value   && $value->isa('Bugzilla::Object');
+    my $can = $self->check_can_change_field($field, $current, $value, \$privs);
+    if (!$can) {
+        if ($field eq 'assigned_to' || $field eq 'qa_contact') {
+            $value   = user_id_to_login($value);
+            $current = user_id_to_login($current);
+        }
+        ThrowUserError('illegal_change', { field    => $field,
+                                           oldvalue => $current,
+                                           newvalue => $value,
+                                           privs    => $privs });
+    }
 }
 
 
@@ -1346,6 +1423,16 @@ sub _set_global_validator {
 #################
 
 sub set_alias { $_[0]->set('alias', $_[1]); }
+sub set_assigned_to {
+    my ($self, $value) = @_;
+    $self->set('assigned_to', $value);
+    delete $self->{'assigned_to_obj'};
+}
+sub reset_assigned_to {
+    my $self = shift;
+    my $comp = $self->component_obj;
+    $self->set_assigned_to($comp->default_assignee);
+}
 sub set_cclist_accessible { $_[0]->set('cclist_accessible', $_[1]); }
 sub set_comment_is_private {
     my ($self, $comment_id, $isprivate) = @_;
@@ -1505,6 +1592,16 @@ sub set_product {
     return $product_changed;
 }
 
+sub set_qa_contact {
+    my ($self, $value) = @_;
+    $self->set('qa_contact', $value);
+    delete $self->{'qa_contact_obj'};
+}
+sub reset_qa_contact {
+    my $self = shift;
+    my $comp = $self->component_obj;
+    $self->set_qa_contact($comp->default_qa_contact);
+}
 sub set_remaining_time { $_[0]->set('remaining_time', $_[1]); }
 # Used only when closing a bug or moving between closed states.
 sub _zero_remaining_time { $_[0]->{'remaining_time'} = 0; }
@@ -1512,9 +1609,9 @@ sub set_reporter_accessible { $_[0]->set('reporter_accessible', $_[1]); }
 sub set_resolution     { $_[0]->set('resolution',    $_[1]); }
 sub clear_resolution   { $_[0]->{'resolution'} = '' }
 sub set_severity       { $_[0]->set('bug_severity',  $_[1]); }
-sub set_status { 
+sub set_status {
     my ($self, $status) = @_;
-    $self->set('bug_status', $status); 
+    $self->set('bug_status', $status);
     # Check for the everconfirmed transition
     $self->_set_everconfirmed(1) if (is_open_state($status) && $status ne 'UNCONFIRMED');
 }
@@ -1537,14 +1634,7 @@ sub add_cc {
     return if !$user_or_name;
     my $user = ref $user_or_name ? $user_or_name
                                  : Bugzilla::User->check($user_or_name);
-
-    if (Bugzilla->params->{strict_isolation}
-        && !$user->can_edit_product($self->product_obj->id))
-    {
-        ThrowUserError('invalid_user_group', { users  => $user->login,
-                                               bug_id => $self->id });
-    }
-
+    $self->_check_strict_isolation_for_user($user);
     my $cc_users = $self->cc_users;
     push(@$cc_users, $user) if !grep($_->id == $user->id, @$cc_users);
 }
@@ -1729,10 +1819,10 @@ sub attachments {
 
 sub assigned_to {
     my ($self) = @_;
-    return $self->{'assigned_to'} if exists $self->{'assigned_to'};
-    $self->{'assigned_to_id'} = 0 if $self->{'error'};
-    $self->{'assigned_to'} = new Bugzilla::User($self->{'assigned_to_id'});
-    return $self->{'assigned_to'};
+    return $self->{'assigned_to_obj'} if exists $self->{'assigned_to_obj'};
+    $self->{'assigned_to'} = 0 if $self->{'error'};
+    $self->{'assigned_to_obj'} ||= new Bugzilla::User($self->{'assigned_to'});
+    return $self->{'assigned_to_obj'};
 }
 
 sub blocked {
@@ -1914,18 +2004,18 @@ sub product_obj {
 
 sub qa_contact {
     my ($self) = @_;
-    return $self->{'qa_contact'} if exists $self->{'qa_contact'};
+    return $self->{'qa_contact_obj'} if exists $self->{'qa_contact_obj'};
     return undef if $self->{'error'};
 
-    if (Bugzilla->params->{'useqacontact'} && $self->{'qa_contact_id'}) {
-        $self->{'qa_contact'} = new Bugzilla::User($self->{'qa_contact_id'});
+    if (Bugzilla->params->{'useqacontact'} && $self->{'qa_contact'}) {
+        $self->{'qa_contact_obj'} = new Bugzilla::User($self->{'qa_contact'});
     } else {
         # XXX - This is somewhat inconsistent with the assignee/reporter 
         # methods, which will return an empty User if they get a 0. 
         # However, we're keeping it this way now, for backwards-compatibility.
-        $self->{'qa_contact'} = undef;
+        $self->{'qa_contact_obj'} = undef;
     }
-    return $self->{'qa_contact'};
+    return $self->{'qa_contact_obj'};
 }
 
 sub reporter {
@@ -2065,10 +2155,10 @@ sub user {
 
     my $unknown_privileges = $user->in_group('editbugs', $prod_id);
     my $canedit = $unknown_privileges
-                  || $user->id == $self->{assigned_to_id}
+                  || $user->id == $self->{'assigned_to'}
                   || (Bugzilla->params->{'useqacontact'}
-                      && $self->{'qa_contact_id'}
-                      && $user->id == $self->{qa_contact_id});
+                      && $self->{'qa_contact'}
+                      && $user->id == $self->{'qa_contact'});
     my $canconfirm = $unknown_privileges
                      || $user->in_group('canconfirm', $prod_id);
     my $isreporter = $user->id
@@ -2991,14 +3081,14 @@ sub check_can_change_field {
     # Make sure that a valid bug ID has been given.
     if (!$self->{'error'}) {
         # Allow the assignee to change anything else.
-        if ($self->{'assigned_to_id'} == $user->id) {
+        if ($self->{'assigned_to'} == $user->id) {
             return 1;
         }
 
         # Allow the QA contact to change anything else.
         if (Bugzilla->params->{'useqacontact'}
-            && $self->{'qa_contact_id'}
-            && ($self->{'qa_contact_id'} == $user->id))
+            && $self->{'qa_contact'}
+            && ($self->{'qa_contact'} == $user->id))
         {
             return 1;
         }
diff --git a/process_bug.cgi b/process_bug.cgi
index a8df416cd..91d430aa9 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -293,6 +293,13 @@ sub DuplicateUserConfirm {
     exit;
 }
 
+my @set_fields = qw(op_sys rep_platform priority bug_severity
+                    component target_milestone version
+                    bug_file_loc status_whiteboard short_desc
+                    deadline remaining_time estimated_time);
+push(@set_fields, 'assigned_to') if !$cgi->param('set_default_assignee');
+push(@set_fields, 'qa_contact')  if !$cgi->param('set_default_qa_contact');
+
 my %methods = (
     bug_severity => 'set_severity',
     rep_platform => 'set_platform',
@@ -303,97 +310,19 @@ foreach my $b (@bug_objects) {
     # Component, target_milestone, and version are in here just in case
     # the 'product' field wasn't defined in the CGI. It doesn't hurt to set
     # them twice.
-    foreach my $field_name (qw(op_sys rep_platform priority bug_severity
-                               component target_milestone version
-                               bug_file_loc status_whiteboard short_desc
-                               deadline remaining_time estimated_time))
-    {
+    foreach my $field_name (@set_fields) {
         if (should_set($field_name)) {
             my $method = $methods{$field_name};
             $method ||= "set_" . $field_name;
             $b->$method($cgi->param($field_name));
         }
     }
+    $b->reset_assigned_to if $cgi->param('set_default_assignee');
+    $b->reset_qa_contact  if $cgi->param('set_default_qa_contact');
 }
 
 my $action = trim($cgi->param('action') || '');
 
-if ($action eq Bugzilla->params->{'move-button-text'}) {
-    Bugzilla->params->{'move-enabled'} || ThrowUserError("move_bugs_disabled");
-
-    $user->is_mover || ThrowUserError("auth_failure", {action => 'move',
-                                                       object => 'bugs'});
-
-    my @multi_select_locks  = map {'bug_' . $_->name . " WRITE"}
-        Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT,
-                               obsolete => 0 });
-
-    $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', 'bug_status READ',
-                         'status_workflow READ', 'resolution READ', @multi_select_locks);
-
-    # First update all moved bugs.
-    foreach my $bug (@bug_objects) {
-        $bug->add_comment(scalar $cgi->param('comment'),
-                          { type => CMT_MOVED_TO, extra_data => $user->login });
-    }
-    # Don't export the new status and resolution. We want the current ones.
-    local $Storable::forgive_me = 1;
-    my $bugs = dclone(\@bug_objects);
-    foreach my $bug (@bug_objects) {
-        my ($status, $resolution) = $bug->get_new_status_and_resolution('move');
-        $bug->set_status($status);
-        $bug->set_resolution($resolution);
-    }
-    $_->update() foreach @bug_objects;
-    $dbh->bz_unlock_tables();
-
-    # Now send emails.
-    foreach my $id (@idlist) {
-        $vars->{'mailrecipients'} = { 'changer' => $user->login };
-        $vars->{'id'} = $id;
-        $vars->{'type'} = "move";
-        send_results($id, $vars);
-    }
-    # Prepare and send all data about these bugs to the new database
-    my $to = Bugzilla->params->{'move-to-address'};
-    $to =~ s/@/\@/;
-    my $from = Bugzilla->params->{'moved-from-address'};
-    $from =~ s/@/\@/;
-    my $msg = "To: $to\n";
-    $msg .= "From: Bugzilla <" . $from . ">\n";
-    $msg .= "Subject: Moving bug(s) " . join(', ', @idlist) . "\n\n";
-
-    my @fieldlist = (Bugzilla::Bug->fields, 'group', 'long_desc',
-                     'attachment', 'attachmentdata');
-    my %displayfields;
-    foreach (@fieldlist) {
-        $displayfields{$_} = 1;
-    }
-
-    $template->process("bug/show.xml.tmpl", { bugs => $bugs,
-                                              displayfields => \%displayfields,
-                                            }, \$msg)
-      || ThrowTemplateError($template->error());
-
-    $msg .= "\n";
-    MessageToMTA($msg);
-
-    # End the response page.
-    unless (Bugzilla->usage_mode == USAGE_MODE_EMAIL) {
-        $template->process("bug/navigate.html.tmpl", $vars)
-            || ThrowTemplateError($template->error());
-        $template->process("global/footer.html.tmpl", $vars)
-            || ThrowTemplateError($template->error());
-    }
-    exit;
-}
-
-
 $::query = "UPDATE bugs SET";
 $::comma = "";
 local our @values;
@@ -498,83 +427,93 @@ if (defined $cgi->param('newcc')
 foreach my $b (@bug_objects) {
     $b->remove_cc($_) foreach @cc_remove;
     $b->add_cc($_) foreach @cc_add;
+    # Theoretically you could move a product without ever specifying
+    # a new assignee or qa_contact, or adding/removing any CCs. So,
+    # we have to check that the current assignee, qa, and CCs are still
+    # valid if we've switched products, under strict_isolation. We can only
+    # do that here. There ought to be some better way to do this,
+    # architecturally, but I haven't come up with it.
+    if ($product_change) {
+        $b->_check_strict_isolation();
+    }
 }
 
-# Store the new assignee and QA contact IDs (if any). This is the
-# only way to keep these informations when bugs are reassigned by
-# component as $cgi->param('assigned_to') and $cgi->param('qa_contact')
-# are not the right fields to look at.
-# If the assignee or qacontact is changed, the new one is checked when
-# changed information is validated.  If not, then the unchanged assignee
-# or qacontact may have to be validated later.
-
-my $assignee;
-my $qacontact;
-my $qacontact_checked = 0;
-my $assignee_checked = 0;
-
-my %usercache = ();
-
-if (should_set('assigned_to') && !$cgi->param('set_default_assignee')) {
-    my $name = trim($cgi->param('assigned_to'));
-    if ($name ne "") {
-        $assignee = login_to_id($name, THROW_ERROR);
-        if (Bugzilla->params->{"strict_isolation"}) {
-            $usercache{$assignee} ||= Bugzilla::User->new($assignee);
-            my $assign_user = $usercache{$assignee};
-            foreach my $product_id (@newprod_ids) {
-                if (!$assign_user->can_edit_product($product_id)) {
-                    my $product_name = Bugzilla::Product->new($product_id)->name;
-                    ThrowUserError('invalid_user_group',
-                                      {'users'   => $assign_user->login,
-                                       'product' => $product_name,
-                                       'bug_id' => (scalar(@idlist) > 1)
-                                                     ? undef : $idlist[0]
-                                      });
-                }
-            }
-        }
-    } else {
-        ThrowUserError("reassign_to_empty");
+if ($action eq Bugzilla->params->{'move-button-text'}) {
+    Bugzilla->params->{'move-enabled'} || ThrowUserError("move_bugs_disabled");
+
+    $user->is_mover || ThrowUserError("auth_failure", {action => 'move',
+                                                       object => 'bugs'});
+
+    my @multi_select_locks  = map {'bug_' . $_->name . " WRITE"}
+        Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT,
+                               obsolete => 0 });
+
+    $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', 'bug_status READ',
+                         'status_workflow READ', 'resolution READ', @multi_select_locks);
+
+    # First update all moved bugs.
+    foreach my $bug (@bug_objects) {
+        $bug->add_comment(scalar $cgi->param('comment'),
+                          { type => CMT_MOVED_TO, extra_data => $user->login });
     }
-    DoComma();
-    $::query .= "assigned_to = ?";
-    push(@values, $assignee);
-    $assignee_checked = 1;
-};
+    # Don't export the new status and resolution. We want the current ones.
+    local $Storable::forgive_me = 1;
+    my $bugs = dclone(\@bug_objects);
+    foreach my $bug (@bug_objects) {
+        my ($status, $resolution) = $bug->get_new_status_and_resolution('move');
+        $bug->set_status($status);
+        $bug->set_resolution($resolution);
+    }
+    $_->update() foreach @bug_objects;
+    $dbh->bz_unlock_tables();
 
-if (should_set('qa_contact') && !$cgi->param('set_default_qa_contact')) {
-    my $name = trim($cgi->param('qa_contact'));
-    $qacontact = login_to_id($name, THROW_ERROR) if ($name ne "");
-    if ($qacontact && Bugzilla->params->{"strict_isolation"}
-        && !(defined $cgi->param('id') && $bug->qa_contact
-             && $qacontact == $bug->qa_contact->id))
-    {
-            $usercache{$qacontact} ||= Bugzilla::User->new($qacontact);
-            my $qa_user = $usercache{$qacontact};
-            foreach my $product_id (@newprod_ids) {
-                if (!$qa_user->can_edit_product($product_id)) {
-                    my $product_name = Bugzilla::Product->new($product_id)->name;
-                    ThrowUserError('invalid_user_group',
-                                      {'users'   => $qa_user->login,
-                                       'product' => $product_name,
-                                       'bug_id' => (scalar(@idlist) > 1)
-                                                     ? undef : $idlist[0]
-                                      });
-                }
-            }
+    # Now send emails.
+    foreach my $id (@idlist) {
+        $vars->{'mailrecipients'} = { 'changer' => $user->login };
+        $vars->{'id'} = $id;
+        $vars->{'type'} = "move";
+        send_results($id, $vars);
     }
-    $qacontact_checked = 1;
-    DoComma();
-    if($qacontact) {
-        $::query .= "qa_contact = ?";
-        push(@values, $qacontact);
+    # Prepare and send all data about these bugs to the new database
+    my $to = Bugzilla->params->{'move-to-address'};
+    $to =~ s/@/\@/;
+    my $from = Bugzilla->params->{'moved-from-address'};
+    $from =~ s/@/\@/;
+    my $msg = "To: $to\n";
+    $msg .= "From: Bugzilla <" . $from . ">\n";
+    $msg .= "Subject: Moving bug(s) " . join(', ', @idlist) . "\n\n";
+
+    my @fieldlist = (Bugzilla::Bug->fields, 'group', 'long_desc',
+                     'attachment', 'attachmentdata');
+    my %displayfields;
+    foreach (@fieldlist) {
+        $displayfields{$_} = 1;
     }
-    else {
-        $::query .= "qa_contact = NULL";
+
+    $template->process("bug/show.xml.tmpl", { bugs => $bugs,
+                                              displayfields => \%displayfields,
+                                            }, \$msg)
+      || ThrowTemplateError($template->error());
+
+    $msg .= "\n";
+    MessageToMTA($msg);
+
+    # End the response page.
+    unless (Bugzilla->usage_mode == USAGE_MODE_EMAIL) {
+        $template->process("bug/navigate.html.tmpl", $vars)
+            || ThrowTemplateError($template->error());
+        $template->process("global/footer.html.tmpl", $vars)
+            || ThrowTemplateError($template->error());
     }
+    exit;
 }
 
+
 if (($cgi->param('set_default_assignee') || $cgi->param('set_default_qa_contact'))
     && Bugzilla->params->{'commentonreassignbycomponent'} && !comment_exists())
 {
@@ -661,56 +600,6 @@ sub SnapShotBug {
 
 my $timestamp;
 
-if ($product_change && Bugzilla->params->{"strict_isolation"}) {
-    my $sth_cc = $dbh->prepare("SELECT who
-                                FROM cc
-                                WHERE bug_id = ?");
-    my $sth_bug = $dbh->prepare("SELECT assigned_to, qa_contact
-                                 FROM bugs
-                                 WHERE bug_id = ?");
-
-    foreach my $id (@idlist) {
-        $sth_cc->execute($id);
-        my @blocked_cc = ();
-        while (my ($pid) = $sth_cc->fetchrow_array) {
-            # Ignore deleted accounts. They will never get notification.
-            $usercache{$pid} ||= Bugzilla::User->new($pid) || next;
-            my $cc_user = $usercache{$pid};
-            if (!$cc_user->can_edit_product($product->id)) {
-                push (@blocked_cc, $cc_user->login);
-            }
-        }
-        if (scalar(@blocked_cc)) {
-            ThrowUserError('invalid_user_group',
-                              {'users'   => \@blocked_cc,
-                               'bug_id' => $id,
-                               'product' => $product->name});
-        }
-        $sth_bug->execute($id);
-        my ($assignee, $qacontact) = $sth_bug->fetchrow_array;
-        if (!$assignee_checked) {
-            $usercache{$assignee} ||= Bugzilla::User->new($assignee) || next;
-            my $assign_user = $usercache{$assignee};
-            if (!$assign_user->can_edit_product($product->id)) {
-                    ThrowUserError('invalid_user_group',
-                                      {'users'   => $assign_user->login,
-                                       'bug_id' => $id,
-                                       'product' => $product->name});
-            }
-        }
-        if (!$qacontact_checked && $qacontact) {
-            $usercache{$qacontact} ||= Bugzilla::User->new($qacontact) || next;
-            my $qa_user = $usercache{$qacontact};
-            if (!$qa_user->can_edit_product($product->id)) {
-                    ThrowUserError('invalid_user_group',
-                                      {'users'   => $qa_user->login,
-                                       'bug_id' => $id,
-                                       'product' => $product->name});
-            }
-        }
-    }
-}
-
 my %bug_objects = map {$_->id => $_} @bug_objects;
 
 # This loop iterates once for each bug to be processed (i.e. all the
@@ -751,35 +640,6 @@ foreach my $id (@idlist) {
         $comma = ',';
     }
 
-    # We have to check whether the bug is moved to another product
-    # and/or component before reassigning.
-    if ($cgi->param('set_default_assignee')) {
-        my $new_comp_id = $bug_objects{$id}->component_id;
-        $assignee = $dbh->selectrow_array('SELECT initialowner
-                                           FROM components
-                                           WHERE components.id = ?',
-                                           undef, $new_comp_id);
-        $query .= "$comma assigned_to = ?";
-        push(@bug_values, $assignee);
-        $comma = ',';
-    }
-
-    if (Bugzilla->params->{'useqacontact'} && $cgi->param('set_default_qa_contact')) {
-        my $new_comp_id = $bug_objects{$id}->component_id;
-        $qacontact = $dbh->selectrow_array('SELECT initialqacontact
-                                            FROM components
-                                            WHERE components.id = ?',
-                                            undef, $new_comp_id);
-        if ($qacontact) {
-            $query .= "$comma qa_contact = ?";
-            push(@bug_values, $qacontact);
-        }
-        else {
-            $query .= "$comma qa_contact = NULL";
-        }
-        $comma = ',';
-    }
-
     my $bug_changed = 0;
     my $write = "WRITE";        # Might want to make a param to control
                                 # whether we do LOW_PRIORITY ...
@@ -826,14 +686,11 @@ foreach my $id (@idlist) {
     $formhash{'bug_status'} = $status;
     $formhash{'resolution'} = $resolution;
 
-    # We need to convert $newhash{'assigned_to'} and $newhash{'qa_contact'}
-    # email addresses into their corresponding IDs;
-    $formhash{'qa_contact'} = $qacontact if Bugzilla->params->{'useqacontact'};
-    $formhash{'assigned_to'} = $assignee;
-
     # This hash is required by Bug::check_can_change_field().
     my $cgi_hash = {'dontchange' => scalar $cgi->param('dontchange')};
     foreach my $col (@editable_bug_fields) {
+        # XXX - Ugly workaround which has to go away before 3.1.3.
+        next if ($col eq 'assigned_to' || $col eq 'qa_contact');
         if (exists $formhash{$col}
             && !$old_bug_obj->check_can_change_field($col, $oldhash{$col}, $formhash{$col},
                                                      \$PrivilegesRequired, $cgi_hash))
@@ -844,11 +701,6 @@ foreach my $id (@idlist) {
                 $vars->{'oldvalue'} = $old_bug_obj->component;
                 $vars->{'newvalue'} = $cgi->param('component');
                 $vars->{'field'} = 'component';
-            } elsif ($col eq 'assigned_to' || $col eq 'qa_contact') {
-                # Display the assignee or QA contact email address
-                $vars->{'oldvalue'} = user_id_to_login($oldhash{$col});
-                $vars->{'newvalue'} = user_id_to_login($formhash{$col});
-                $vars->{'field'} = $col;
             } else {
                 $vars->{'oldvalue'} = $oldhash{$col};
                 $vars->{'newvalue'} = $formhash{$col};
@@ -1034,11 +886,6 @@ foreach my $id (@idlist) {
         $newhash{$col} = $newvalues[$i];
         $i++;
     }
-    # for passing to Bugzilla::BugMail to ensure that when someone is removed
-    # from one of these fields, they get notified of that fact (if desired)
-    #
-    my $origOwner = "";
-    my $origQaContact = "";
 
     # $msgs will store emails which have to be sent to voters, if any.
     my $msgs;
@@ -1051,27 +898,10 @@ foreach my $id (@idlist) {
         my $new = shift @newvalues;
         if ($old ne $new) {
 
-            # save off the old value for passing to Bugzilla::BugMail so
-            # the old assignee can be notified
-            #
-            if ($col eq 'assigned_to') {
-                $old = ($old) ? user_id_to_login($old) : "";
-                $new = ($new) ? user_id_to_login($new) : "";
-                $origOwner = $old;
-            }
-
-            # ditto for the old qa contact
-            #
-            if ($col eq 'qa_contact') {
-                $old = ($old) ? user_id_to_login($old) : "";
-                $new = ($new) ? user_id_to_login($new) : "";
-                $origQaContact = $old;
-            }
-
             # Bugzilla::Bug does these for us already.
             next if grep($_ eq $col, qw(keywords op_sys rep_platform priority
                                         product_id component_id version
-                                        target_milestone
+                                        target_milestone assigned_to qa_contact
                                         bug_severity short_desc alias
                                         deadline estimated_time remaining_time
                                         reporter_accessible cclist_accessible
@@ -1147,10 +977,12 @@ foreach my $id (@idlist) {
     # all concerned users, including the bug itself, but also the
     # duplicated bug and dependent bugs, if any.
 
-    $vars->{'mailrecipients'} = { 'cc' => $cc_removed,
-                                  'owner' => $origOwner,
-                                  'qacontact' => $origQaContact,
-                                  'changer' => Bugzilla->user->login };
+    my $orig_qa = $old_bug_obj->qa_contact;
+    $vars->{'mailrecipients'} = {
+        cc        => $cc_removed,
+        owner     => $old_bug_obj->assigned_to->login,
+        qacontact => $orig_qa ? $orig_qa->login : '',
+        changer   => Bugzilla->user->login };
 
     $vars->{'id'} = $id;
     $vars->{'type'} = "bug";