Bug 373689: Implement set_product: Move product changing from process_bug into Bugzilla::Bug

Bugzilla/Bug.pm

@@ -156,10 +156,12 @@ sub VALIDATORS {
 };
 
 use constant UPDATE_VALIDATORS => {
-    bug_status => \&_check_bug_status,
+    bug_status          => \&_check_bug_status,
     cclist_accessible   => \&Bugzilla::Object::check_boolean,
     reporter_accessible => \&Bugzilla::Object::check_boolean,
-    resolution => \&_check_resolution,
+    resolution          => \&_check_resolution,
+    target_milestone    => \&_check_target_milestone,
+    version             => \&_check_version,
 };
 
 sub UPDATE_COLUMNS {
@@ -169,6 +171,7 @@ sub UPDATE_COLUMNS {
     my @columns = qw(
         alias
         cclist_accessible
+        component_id
         deadline
         estimated_time
         everconfirmed
@@ -177,12 +180,15 @@ sub UPDATE_COLUMNS {
         bug_status
         op_sys
         priority
+        product_id
         remaining_time
         rep_platform
         reporter_accessible
         resolution
         short_desc
         status_whiteboard
+        target_milestone
+        version
     );
     push(@columns, @custom_names);
     return @columns;
@@ -429,17 +435,17 @@ sub run_create_validators {
     $vars->{comment_exists} = ($params->{comment} =~ /\S+/) ? 1 : 0;
     Bugzilla::Bug->check_status_change_triggers($params->{bug_status}, [], $vars);
 
-    $params->{target_milestone} = $class->_check_target_milestone($product,
-        $params->{target_milestone});
+    $params->{target_milestone} = $class->_check_target_milestone(
+        $params->{target_milestone}, $product);
 
-    $params->{version} = $class->_check_version($product, $params->{version});
+    $params->{version} = $class->_check_version($params->{version}, $product);
 
     $params->{keywords} = $class->_check_keywords($params->{keywords}, $product);
 
     $params->{groups} = $class->_check_groups($product,
         $params->{groups});
 
-    my $component = $class->_check_component($product, $params->{component});
+    my $component = $class->_check_component($params->{component}, $product);
     $params->{component_id} = $component->id;
     delete $params->{component};
 
@@ -521,6 +527,12 @@ sub update {
         my $change = $changes->{$field};
         my $from = defined $change->[0] ? $change->[0] : '';
         my $to   = defined $change->[1] ? $change->[1] : '';
+        # Certain fields have their name stored in bugs_activity, not their id.
+        if ( grep($_ eq $field, qw(product_id component_id)) ) {
+            $field =~ s/_id$//;
+            $from  = $self->{"_old_${field}_name"};
+            $to    = $self->$field;
+        }
         LogActivityEntry($self->id, $field, $from, $to, Bugzilla->user->id,
                          $delta_ts);
     }
@@ -912,9 +924,10 @@ sub _check_comment_type {
 }
 
 sub _check_component {
-    my ($invocant, $product, $name) = @_;
+    my ($invocant, $name, $product) = @_;
     $name = trim($name);
     $name || ThrowUserError("require_component");
+    ($product = $invocant->product_obj) if ref $invocant;
     my $obj = Bugzilla::Component::check_component($product, $name);
     return $obj;
 }
@@ -1071,20 +1084,18 @@ sub _check_keywords {
 
 sub _check_product {
     my ($invocant, $name) = @_;
-    my $obj;
-    if (ref $invocant) {
-         $obj = Bugzilla::Product::check_product($name);
-    }
-    else {
-        # Check that the product exists and that the user
-        # is allowed to enter bugs into this product.
-        Bugzilla->user->can_enter_product($name, THROW_ERROR);
-        # can_enter_product already does everything that check_product
-        # would do for us, so we don't need to use it.
-        $obj = new Bugzilla::Product({ name => $name });
+    $name = trim($name);
+    # If we're updating the bug and they haven't changed the product,
+    # always allow it.
+    if (ref $invocant && lc($invocant->product_obj->name) eq lc($name)) {
+        return $invocant->product_obj;
     }
-
-    return $obj;
+    # Check that the product exists and that the user
+    # is allowed to enter bugs into this product.
+    Bugzilla->user->can_enter_product($name, THROW_ERROR);
+    # can_enter_product already does everything that check_product
+    # would do for us, so we don't need to use it.
+    return new Bugzilla::Product({ name => $name });
 }
 
 sub _check_op_sys {
@@ -1168,11 +1179,10 @@ sub _check_strict_isolation {
 }
 
 sub _check_target_milestone {
-    my ($invocant, $product, $target) = @_;
+    my ($invocant, $target, $product) = @_;
+    $product = $invocant->product_obj if ref $invocant;
+
     $target = trim($target);
-    if (ref $invocant) {
-        return undef if !Bugzilla->params->{'usetargetmilestone'};
-    }
     $target = $product->default_milestone if !defined $target;
     check_field('target_milestone', $target,
             [map($_->name, @{$product->milestones})]);
@@ -1214,8 +1224,9 @@ sub _check_qa_contact {
 }
 
 sub _check_version {
-    my ($invocant, $product, $version) = @_;
+    my ($invocant, $version, $product) = @_;
     $version = trim($version);
+    ($product = $invocant->product_obj) if ref $invocant;
     check_field('version', $version, [map($_->name, @{$product->versions})]);
     return $version;
 }
@@ -1295,6 +1306,22 @@ sub _set_global_validator {
 
 sub set_alias { $_[0]->set('alias', $_[1]); }
 sub set_cclist_accessible { $_[0]->set('cclist_accessible', $_[1]); }
+sub set_component  {
+    my ($self, $name) = @_;
+    my $old_comp  = $self->component_obj;
+    my $component = $self->_check_component($name);
+    if ($old_comp->id != $component->id) {
+        $self->{component_id}  = $component->id;
+        $self->{component}     = $component->name;
+        $self->{component_obj} = $component;
+        # For update()
+        $self->{_old_component_name} = $old_comp->name;
+        # Add in the Default CC of the new Component;
+        foreach my $cc (@{$component->initial_cc}) {
+            $self->add_cc($cc);
+        }
+    }
+}
 sub set_custom_field {
     my ($self, $field, $value) = @_;
     if (ref $value eq 'ARRAY' && $field->type != FIELD_TYPE_MULTI_SELECT) {
@@ -1319,6 +1346,111 @@ sub _set_everconfirmed { $_[0]->set('everconfirmed', $_[1]); }
 sub set_op_sys         { $_[0]->set('op_sys',        $_[1]); }
 sub set_platform       { $_[0]->set('rep_platform',  $_[1]); }
 sub set_priority       { $_[0]->set('priority',      $_[1]); }
+sub set_product {
+    my ($self, $name, $params) = @_;
+    my $old_product = $self->product_obj;
+    my $product = $self->_check_product($name);
+    
+    my $product_changed = 0;
+    if ($old_product->id != $product->id) {
+        $self->{product_id}  = $product->id;
+        $self->{product}     = $product->name;
+        $self->{product_obj} = $product;
+        # For update()
+        $self->{_old_product_name} = $old_product->name;
+        # Delete fields that depend upon the old Product value.
+        delete $self->{choices};
+        delete $self->{milestoneurl};
+        $product_changed = 1;
+    }
+
+    $params ||= {};
+    my $comp_name = $params->{component} || $self->component;
+    my $vers_name = $params->{version}   || $self->version;
+    my $tm_name   = $params->{target_milestone};
+    # This way, if usetargetmilestone is off and we've changed products,
+    # set_target_milestone will reset our target_milestone to
+    # $product->default_milestone. But if we haven't changed products,
+    # we don't reset anything.
+    if (!defined $tm_name
+        && (Bugzilla->params->{'usetargetmilestone'} || !$product_changed))
+    {
+        $tm_name = $self->target_milestone;
+    }
+
+    if ($product_changed && Bugzilla->usage_mode == USAGE_MODE_BROWSER) {
+        # Try to set each value with the new product.
+        # Have to set error_mode because Throw*Error calls exit() otherwise.
+        my $old_error_mode = Bugzilla->error_mode;
+        Bugzilla->error_mode(ERROR_MODE_DIE);
+        my $component_ok = eval { $self->set_component($comp_name);      1; };
+        my $version_ok   = eval { $self->set_version($vers_name);        1; };
+        my $milestone_ok = eval { $self->set_target_milestone($tm_name); 1; };
+        # If there were any errors thrown, make sure we don't mess up any
+        # other part of Bugzilla that checks $@.
+        undef $@;
+        Bugzilla->error_mode($old_error_mode);
+        
+        my $verified = $params->{change_confirmed};
+        my %vars;
+        if (!$verified || !$component_ok || !$version_ok || !$milestone_ok) {
+            $vars{defaults} = {
+                # Note that because of the eval { set } above, these are
+                # already set correctly if they're valid, otherwise they're
+                # set to some invalid value which the template will ignore.
+                component => $self->component,
+                version   => $self->version,
+                milestone => $milestone_ok ? $self->target_milestone
+                                           : $product->default_milestone
+            };
+            $vars{components} = [map { $_->name } @{$product->components}];
+            $vars{milestones} = [map { $_->name } @{$product->milestones}];
+            $vars{versions}   = [map { $_->name } @{$product->versions}];
+        }
+
+        if (!$verified) {
+            $vars{verify_bug_groups} = 1;
+            my $dbh = Bugzilla->dbh;
+            my @idlist = ($self->id);
+            push(@idlist, map {$_->id} @{ $params->{other_bugs} })
+                if $params->{other_bugs};
+            # Get the ID of groups which are no longer valid in the new product.
+            my $gids = $dbh->selectcol_arrayref(
+                'SELECT bgm.group_id
+                   FROM bug_group_map AS bgm
+                  WHERE bgm.bug_id IN (' . join(',', ('?' x @idlist)) . ')
+                    AND bgm.group_id NOT IN
+                        (SELECT gcm.group_id
+                           FROM group_control_map AS gcm
+                           WHERE gcm.product_id = ?
+                                 AND ( (gcm.membercontrol != ?
+                                        AND gcm.group_id IN ('
+                                        . Bugzilla->user->groups_as_string . '))
+                                       OR gcm.othercontrol != ?) )',
+                undef, (@idlist, $product->id, CONTROLMAPNA, CONTROLMAPNA));
+            $vars{'old_groups'} = Bugzilla::Group->new_from_list($gids);            
+        }
+        
+        if (%vars) {
+            $vars{product} = $product;
+            my $template = Bugzilla->template;
+            $template->process("bug/process/verify-new-product.html.tmpl",
+                \%vars) || ThrowTemplateError($template->error());
+            exit;
+        }
+    }
+    else {
+        # When we're not in the browser (or we didn't change the product), we
+        # just die if any of these are invalid.
+        $self->set_component($comp_name);
+        $self->set_version($vers_name);
+        $self->set_target_milestone($tm_name);
+    }
+    
+    # XXX This is temporary until all of process_bug uses update();
+    return $product_changed;
+}
+
 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; }
@@ -1332,8 +1464,10 @@ sub set_status {
     $self->_set_everconfirmed(1) if (is_open_state($status) && $status ne 'UNCONFIRMED');
 }
 sub set_status_whiteboard { $_[0]->set('status_whiteboard', $_[1]); }
-sub set_summary        { $_[0]->set('short_desc',    $_[1]); }
-sub set_url            { $_[0]->set('bug_file_loc',  $_[1]); }
+sub set_summary           { $_[0]->set('short_desc',        $_[1]); }
+sub set_target_milestone  { $_[0]->set('target_milestone',  $_[1]); }
+sub set_url               { $_[0]->set('bug_file_loc',      $_[1]); }
+sub set_version           { $_[0]->set('version',           $_[1]); }
 
 ########################
 # "Add/Remove" Methods #
@@ -1344,16 +1478,13 @@ sub set_url            { $_[0]->set('bug_file_loc',  $_[1]); }
 # Accepts a User object or a username. Adds the user only if they
 # don't already exist as a CC on the bug.
 sub add_cc {
-    # XXX $product is a temporary hack until all of process_bug uses Bug
-    # objects for updating.
-    my ($self, $user_or_name, $product) = @_;
+    my ($self, $user_or_name) = @_;
     return if !$user_or_name;
     my $user = ref $user_or_name ? $user_or_name
                                  : Bugzilla::User->check($user_or_name);
 
-    my $product_id = $product ? $product->id : $self->{product_id};
     if (Bugzilla->params->{strict_isolation}
-        && !$user->can_edit_product($product_id))
+        && !$user->can_edit_product($self->product_obj->id))
     {
         ThrowUserError('invalid_user_group', { users  => $user->login,
                                                bug_id => $self->id });
@@ -1598,6 +1729,15 @@ sub component {
     return $self->{component};
 }
 
+# XXX Eventually this will replace component()
+sub component_obj {
+    my ($self) = @_;
+    return $self->{component_obj} if defined $self->{component_obj};
+    return {} if $self->{error};
+    $self->{component_obj} = new Bugzilla::Component($self->{component_id});
+    return $self->{component_obj};
+}
+
 sub classification_id {
     my ($self) = @_;
     return $self->{classification_id} if exists $self->{classification_id};
@@ -1710,8 +1850,8 @@ sub product {
 sub product_obj {
     my $self = shift;
     return {} if $self->{error};
-    $self->{prod_obj} ||= new Bugzilla::Product($self->{product_id});
-    return $self->{prod_obj};
+    $self->{product_obj} ||= new Bugzilla::Product($self->{product_id});
+    return $self->{product_obj};
 }
 
 sub qa_contact {

process_bug.cgi

@@ -95,6 +95,20 @@ sub send_results {
     $vars->{'header_done'} = 1;
 }
 
+# Tells us whether or not a field should be changed by process_bug, by
+# checking that it's defined and not set to dontchange.
+sub should_set {
+    my ($field) = @_;
+    my $cgi = Bugzilla->cgi;
+    if (defined $cgi->param($field)
+        && (!$cgi->param('dontchange') 
+            || $cgi->param($field) ne $cgi->param('dontchange')))
+    {
+        return 1;
+    }
+    return 0;
+}
+
 sub comment_exists {
     my $cgi = Bugzilla->cgi;
     return ($cgi->param('comment') && $cgi->param('comment') =~ /\S+/) ? 1 : 0;
@@ -223,167 +237,29 @@ if ($cgi->cookie("BUGLIST") && defined $cgi->param('id')) {
     $vars->{'bug_list'} = \@bug_list;
 }
 
-# Figure out whether or not the user is trying to change the product
-# (either the "product" variable is not set to "don't change" or the
-# user is changing a single bug and has changed the bug's product),
-# and make the user verify the version, component, target milestone,
-# and bug groups if so.
-# At this point, the product must be defined, even if set to "dontchange".
-defined($cgi->param('product'))
-  || ThrowCodeError('undefined_field', { field => 'product' });
-
-my $product_change = 0;
-if ((defined $cgi->param('id') && $cgi->param('product') ne $bug->product)
-     || (!$cgi->param('id')
-         && $cgi->param('product') ne $cgi->param('dontchange')))
-{
-    if (Bugzilla->params->{'commentonreassignbycomponent'} && !comment_exists()) {
-        ThrowUserError('comment_required');
-    }
-    # Check to make sure they actually have the right to change the product
-    my $oldproduct = (defined $cgi->param('id')) ? $bug->product : '';
-    if (!$bug->check_can_change_field('product', $oldproduct, $cgi->param('product'),
-                                      \$PrivilegesRequired))
-    {
-        $vars->{'oldvalue'} = $oldproduct;
-        $vars->{'newvalue'} = $cgi->param('product');
-        $vars->{'field'} = 'product';
-        $vars->{'privs'} = $PrivilegesRequired;
-        ThrowUserError("illegal_change", $vars);
-    }
-
-    my $product_name = $cgi->param('product');
-    my $product = new Bugzilla::Product({name => $product_name});
-
-    # If at least one bug does not belong to the product we are
-    # moving to, we have to check whether or not the user is
-    # allowed to enter bugs into that product.
-    # Note that this check must be done early to avoid the leakage
-    # of component, version and target milestone names.
-    my $check_can_enter = 1;
-    if ($product) {
-        $check_can_enter =
-          $dbh->selectrow_array("SELECT 1 FROM bugs
-                                 WHERE product_id != ?
-                                 AND bugs.bug_id IN
-                                 (" . join(',', @idlist) . ") " .
-                                 $dbh->sql_limit(1),
-                                 undef, $product->id);
-    }
-    if ($check_can_enter) { $user->can_enter_product($product_name, 1) }
-
-    # note that when this script is called from buglist.cgi (rather
-    # than show_bug.cgi), it's possible that the product will be changed
-    # but that the version and/or component will be set to 
-    # "--dont_change--" but still happen to be correct.  in this case,
-    # the if statement will incorrectly trigger anyway.  this is a 
-    # pretty weird case, and not terribly unreasonable behavior, but 
-    # worthy of a comment, perhaps.
-    #
-    my @version_names = map($_->name, @{$product->versions});
-    my @component_names = map($_->name, @{$product->components});
-    my $vok = 0;
-    if (defined $cgi->param('version')) {
-        $vok = lsearch(\@version_names, $cgi->param('version')) >= 0;
-    }
-    my $cok = 0;
-    if (defined $cgi->param('component')) {
-        $cok = lsearch(\@component_names, $cgi->param('component')) >= 0;
-    }
-
-    my $mok = 1;   # so it won't affect the 'if' statement if milestones aren't used
-    my @milestone_names = ();
-    if ( Bugzilla->params->{"usetargetmilestone"} ) {
-       @milestone_names = map($_->name, @{$product->milestones});
-       $mok = 0;
-       if (defined $cgi->param('target_milestone')) {
-           $mok = lsearch(\@milestone_names, $cgi->param('target_milestone')) >= 0;
-       }
-    }
-
-    # We cannot be sure if the component is the same by only checking $cok; the
-    # current component name could exist in the new product. So always display
-    # the form and use the confirm_product_change param to check if that was
-    # shown. Also show the verification form if the product-specific fields
-    # somehow still need to be verified, or if we need to verify whether or not
-    # to add the bugs to their new product's group.
-    if (!$vok || !$cok || !$mok || !defined $cgi->param('confirm_product_change')) {
-
-        if (Bugzilla->usage_mode == USAGE_MODE_EMAIL) {
-            if (!$vok) {
-                ThrowUserError('version_not_valid', {
-                    version => $cgi->param('version'),
-                    product => $product->name});
-            }
-            if (!$cok) {
-                ThrowUserError('component_not_valid', {
-                    product => $product->name,
-                    name    => $cgi->param('component')});
-            }
-            if (!$mok) {
-                ThrowUserError('milestone_not_valid', {
-                    product   => $product->name,
-                    milestone => $cgi->param('target_milestone')});
-            }
-        }
-
-        $vars->{'product'} = $product;
-        my %defaults;
-        # We set the defaults to these fields to the old value,
-        # if it's a valid option, otherwise we use the default where
-        # that's appropriate
-        $vars->{'versions'} = \@version_names;
-        if ($vok) {
-            $defaults{'version'} = $cgi->param('version');
-        }
-        elsif (scalar(@version_names) == 1) {
-            $defaults{'version'} = $version_names[0];
-        }
-
-        $vars->{'components'} = \@component_names;
-        if ($cok) {
-            $defaults{'component'} = $cgi->param('component');
-        }
-        elsif (scalar(@component_names) == 1) {
-            $defaults{'component'} = $component_names[0];
+my $product_change; # XXX Temporary until all of process_bug uses update()
+if (should_set('product')) {
+    # We only pass the fields if they're defined and not set to dontchange.
+    # This is because when you haven't changed the product, --do-not-change--
+    # isn't a valid component, version, or target_milestone. (When you're
+    # doing a mass-change, some bugs might already be in the new product.)
+    my %product_fields;
+    foreach my $field (qw(component version target_milestone)) {
+        if (should_set($field)) {
+            $product_fields{$field} = $cgi->param($field);
         }
+    }
 
-        if (Bugzilla->params->{"usetargetmilestone"}) {
-            $vars->{'milestones'} = \@milestone_names;
-            if ($mok) {
-                $defaults{'target_milestone'} = $cgi->param('target_milestone');
-            } else {
-                $defaults{'target_milestone'} = $product->default_milestone;
-            }
-        }
-        $vars->{'defaults'} = \%defaults;
-
-        # Get the ID of groups which are no longer valid in the new product.
-        my $gids =
-          $dbh->selectcol_arrayref('SELECT bgm.group_id
-                                      FROM bug_group_map AS bgm
-                                     WHERE bgm.bug_id IN (' . join(', ', @idlist) . ')
-                                       AND bgm.group_id NOT IN
-                                           (SELECT gcm.group_id
-                                              FROM group_control_map AS gcm
-                                             WHERE gcm.product_id = ?
-                                               AND ((gcm.membercontrol != ?
-                                                    AND gcm.group_id IN (' . $grouplist . '))
-                                                    OR gcm.othercontrol != ?))',
-                                     undef, ($product->id, CONTROLMAPNA, CONTROLMAPNA));
-        $vars->{'old_groups'} = Bugzilla::Group->new_from_list($gids);
-
-        $template->process("bug/process/verify-new-product.html.tmpl", $vars)
-          || ThrowTemplateError($template->error());
-        exit;
+    foreach my $b (@bug_objects) {
+        my $changed = $b->set_product(scalar $cgi->param('product'),
+            { %product_fields,
+              change_confirmed => scalar $cgi->param('confirm_product_change'),
+              other_bugs => \@bug_objects,
+            });
+        $product_change ||= $changed;
     }
-    $product_change = 1;
 }
 
-# At this point, the component must be defined, even if set to "dontchange".
-defined($cgi->param('component'))
-  || ThrowCodeError('undefined_field', { field => 'component' });
-
 # Confirm that the reporter of the current bug can access the bug we are duping to.
 sub DuplicateUserConfirm {
     my ($dupe, $original) = @_;
@@ -426,27 +302,6 @@ sub DuplicateUserConfirm {
     exit;
 }
 
-if (defined $cgi->param('id')) {
-    # since this means that we were called from show_bug.cgi, now is a good
-    # time to do a whole bunch of error checking that can't easily happen when
-    # we've been called from buglist.cgi, because buglist.cgi only tweaks
-    # values that have been changed instead of submitting all the new values.
-    # (XXX those error checks need to happen too, but implementing them 
-    # is more work in the current architecture of this script...)
-
-    my $prod = $bug->_check_product($cgi->param('product'));
-    $cgi->param('product', $prod->name);
-
-    my $comp = $bug->_check_component($prod, 
-                                      scalar $cgi->param('component'));
-    $cgi->param('component', $comp->name);
-
-    $cgi->param('version', $bug->_check_version($prod,
-                                                scalar $cgi->param('version')));
-    $cgi->param('target_milestone', $bug->_check_target_milestone($prod,
-        scalar $cgi->param('target_milestone')));
-}
-
 my %methods = (
     bug_severity => 'set_severity',
     rep_platform => 'set_platform',
@@ -454,7 +309,11 @@ my %methods = (
     bug_file_loc => 'set_url',
 );
 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))
     {
@@ -559,19 +418,6 @@ sub DoComma {
     $::comma = ",";
 }
 
-foreach my $field ("version", "target_milestone") {
-    if (defined $cgi->param($field)) {
-        if (!$cgi->param('dontchange')
-            || $cgi->param($field) ne $cgi->param('dontchange')) {
-            DoComma();
-            $::query .= "$field = ?";
-            my $value = trim($cgi->param($field));
-            trick_taint($value);
-            push(@values, $value);
-        }
-    }
-}
-
 # Add custom fields data to the query that will update the database.
 foreach my $field (Bugzilla->get_fields({custom => 1, obsolete => 0})) {
     my $fname = $field->name;
@@ -583,24 +429,10 @@ foreach my $field (Bugzilla->get_fields({custom => 1, obsolete => 0})) {
     }
 }
 
-my $product;
-my $prod_changed = 0;
-my @newprod_ids;
+my ($product, @newprod_ids);
 if ($cgi->param('product') ne $cgi->param('dontchange')) {
     $product = Bugzilla::Product::check_product(scalar $cgi->param('product'));
-
-    DoComma();
-    $::query .= "product_id = ?";
-    push(@values, $product->id);
     @newprod_ids = ($product->id);
-    # If the bug remains in the same product, leave $prod_changed set to 0.
-    # Even with 'strict_isolation' turned on, we ignore users who already
-    # play a role for the bug; else you would never be able to edit it.
-    # If you want to move the bug to another product, then you first have to
-    # remove these users from the bug.
-    unless (defined $cgi->param('id') && $bug->product_id == $product->id) {
-        $prod_changed = 1;
-    }
 } else {
     @newprod_ids = @{$dbh->selectcol_arrayref("SELECT DISTINCT product_id
                                                FROM bugs 
@@ -612,33 +444,8 @@ if ($cgi->param('product') ne $cgi->param('dontchange')) {
     }
 }
 
-my $component;
 my (@cc_add, @cc_remove);
 
-if ($cgi->param('component') ne $cgi->param('dontchange')) {
-    if (scalar(@newprod_ids) > 1) {
-        ThrowUserError("no_component_change_for_multiple_products");
-    }
-    $component =
-        Bugzilla::Component::check_component($product, scalar $cgi->param('component'));
-
-    # This parameter is required later when checking fields the user can change.
-    $cgi->param('component_id', $component->id);
-    DoComma();
-    $::query .= "component_id = ?";
-    push(@values, $component->id);
-
-    # Add in the default CC list for the component if we are moving bugs.
-    if (!$cgi->param('id') || $component->id != $bug->component_id) {
-        foreach my $cc (@{$component->initial_cc}) {
-            # NewCC must be defined or the code below won't insert
-            # any CCs.
-            $cgi->param('newcc') || $cgi->param('newcc', "");
-            push(@cc_add, $cc->login);
-        }
-    }
-}
-
 # Certain changes can only happen on individual bugs, never on mass-changes.
 if (defined $cgi->param('id')) {
     my $bug = $bug_objects[0];
@@ -721,7 +528,7 @@ if (defined $cgi->param('newcc')
 
 foreach my $b (@bug_objects) {
     $b->remove_cc($_) foreach @cc_remove;
-    $b->add_cc($_, $product) foreach @cc_add;
+    $b->add_cc($_) foreach @cc_add;
 }
 
 # Store the new assignee and QA contact IDs (if any). This is the
@@ -891,7 +698,7 @@ sub SnapShotBug {
 
 my $timestamp;
 
-if ($prod_changed && Bugzilla->params->{"strict_isolation"}) {
+if ($product_change && Bugzilla->params->{"strict_isolation"}) {
     my $sth_cc = $dbh->prepare("SELECT who
                                 FROM cc
                                 WHERE bug_id = ?");
@@ -982,10 +789,9 @@ foreach my $id (@idlist) {
     }
 
     # We have to check whether the bug is moved to another product
-    # and/or component before reassigning. If $component is defined,
-    # use it; else use the product/component the bug is already in.
+    # and/or component before reassigning.
     if ($cgi->param('set_default_assignee')) {
-        my $new_comp_id = $component ? $component->id : $old_bug_obj->component_id;
+        my $new_comp_id = $bug_objects{$id}->component_id;
         $assignee = $dbh->selectrow_array('SELECT initialowner
                                            FROM components
                                            WHERE components.id = ?',
@@ -996,7 +802,7 @@ foreach my $id (@idlist) {
     }
 
     if (Bugzilla->params->{'useqacontact'} && $cgi->param('set_default_qa_contact')) {
-        my $new_comp_id = $component ? $component->id : $old_bug_obj->component_id;
+        my $new_comp_id = $bug_objects{$id}->component_id;
         $qacontact = $dbh->selectrow_array('SELECT initialqacontact
                                             FROM components
                                             WHERE components.id = ?',
@@ -1096,25 +902,16 @@ foreach my $id (@idlist) {
                       { product => $oldhash{'product'} });
     }
 
-    # If we are editing a single bug or if bugs are being moved into
-    # a specific product, $product is defined. If $product is undefined,
-    # then we don't move bugs, so we can use their current product.
-    my $new_product = $product || new Bugzilla::Product({name => $oldhash{'product'}});
-    if ($requiremilestone) {
-        # musthavemilestoneonaccept applies only if at least two
-        # target milestones are defined for the product.
-        if (scalar(@{$new_product->milestones}) > 1) {
-            my $value = $cgi->param('target_milestone');
-            if (!defined $value || $value eq $cgi->param('dontchange')) {
-                $value = $oldhash{'target_milestone'};
-            }
-            # if musthavemilestoneonaccept == 1, then the target
-            # milestone must be different from the default one.
-            if ($value eq $new_product->default_milestone) {
-                ThrowUserError("milestone_required", { bug_id => $id });
-            }
-        }
-    }   
+    my $new_product = $bug_objects{$id}->product_obj;
+    # musthavemilestoneonaccept applies only if at least two
+    # target milestones are defined for the product.
+    if ($requiremilestone
+        && scalar(@{ $new_product->milestones }) > 1
+        && $bug_objects{$id}->target_milestone
+           eq $new_product->default_milestone)
+    {
+        ThrowUserError("milestone_required", { bug_id => $id });
+    }
     if (defined $cgi->param('delta_ts') && $cgi->param('delta_ts') ne $delta_ts)
     {
         ($vars->{'operations'}) =
@@ -1301,19 +1098,6 @@ foreach my $id (@idlist) {
         my $new = shift @newvalues;
         if ($old ne $new) {
 
-            # Products and components are now stored in the DB using ID's
-            # We need to translate this to English before logging it
-            if ($col eq 'product_id') {
-                $old = $old_bug_obj->product;
-                $new = $new_bug_obj->product;
-                $col = 'product';
-            }
-            if ($col eq 'component_id') {
-                $old = $old_bug_obj->component;
-                $new = $new_bug_obj->component;
-                $col = 'component';
-            }
-
             # save off the old value for passing to Bugzilla::BugMail so
             # the old assignee can be notified
             #
@@ -1333,6 +1117,8 @@ foreach my $id (@idlist) {
 
             # 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
                                         bug_severity short_desc alias
                                         deadline estimated_time remaining_time
                                         reporter_accessible cclist_accessible

template/en/default/bug/process/verify-new-product.html.tmpl

@@ -17,6 +17,7 @@
   #
   # Contributor(s): Myk Melez <myk@mozilla.org>
   #                 Frédéric Buclin <LpSolit@gmail.com>
+  #                 Max Kanat-Alexander <mkanat@bugzilla.org>
   #%]
 
 [%# INTERFACE:
@@ -26,12 +27,10 @@
   # milestones: array; milestones for the new product.
   # defaults: hash; keys are names of fields, values are defaults for
   #   those fields
+  #
+  # verify_bug_groups: If groups need to be confirmed in addition to fields.
   #%]
 
-[%# The global Bugzilla->cgi object is used to obtain form variable values. %]
-[% USE Bugzilla %]
-[% cgi = Bugzilla.cgi %]
-
 [% PROCESS global/variables.none.tmpl %]
 
 [% PROCESS global/header.html.tmpl
@@ -39,60 +38,77 @@
 
 <form action="process_bug.cgi" method="post">
 
+[% SET exclude_items = ['version', 'component', 'target_milestone'] %]
+[% IF verify_bug_groups %]
+  [% exclude_items.push('bit-\d+') %]
+[% END %]
+
 [% PROCESS "global/hidden-fields.html.tmpl"
-     exclude=("^version|component|target_milestone|bit-\\d+$") %]
+     exclude = '^' _ exclude_items.join('|') _ '$' %]
 
 <input type="hidden" name="confirm_product_change" value="1">
+    
 [%# Verify the version, component, and target milestone fields. %]
-  <h3>Verify Version, Component[% ", Target Milestone" IF Param("usetargetmilestone") %]</h3>
-
-  <p>
-  [% IF Param("usetargetmilestone") %]
-    You are moving the [% terms.bug %](s) to the product 
-    <b>[% cgi.param("product") FILTER html %]</b>,
-    and the version, component, and/or target milestone fields are no longer
-    correct.  Please set the correct version, component, and target milestone now:
-  [% ELSE %]
-    You are moving the [% terms.bug %](s) to the product 
-    <b>[% cgi.param("product") FILTER html %]</b>,
-    and the version and component fields are no longer correct.
-    Please set the correct version and component now:
-  [% END %]
-  </p>
-
-  <table>
-    <tr>
-      <td>
-        <b>Version:</b><br>
-        [% PROCESS "global/select-menu.html.tmpl" 
-                   name="version"
-                   options=versions
-                   default=defaults.version
-                   size=10 %]
-      </td>
+<h3>Verify Version, Component[% ", Target Milestone" IF Param("usetargetmilestone") %]</h3>
+
+<p>
+[% IF Param("usetargetmilestone") %]
+  You are moving the [% terms.bug %](s) to the product 
+  <b>[% product.name FILTER html %]</b>,
+  and the version, component, and/or target milestone fields are no longer
+  correct.  Please set the correct version, component, and target milestone now:
+[% ELSE %]
+  You are moving the [% terms.bug %](s) to the product 
+  <b>[% product.name FILTER html %]</b>,
+  and the version and component fields are no longer correct.
+  Please set the correct version and component now:
+[% END %]
+</p>
+
+<table>
+  <tr>
+    <td>
+      <b>Version:</b><br>
+      [% IF versions.size == 1 %]
+        [% SET default_version = versions.0 %]
+      [% ELSE %]
+        [% SET default_version = defaults.version %]
+      [% END %]
+      [% PROCESS "global/select-menu.html.tmpl" 
+                 name="version"
+                 options=versions
+                 default=default_version
+                 size=10 %]
+    </td>
+    <td>
+      <b>Component:</b><br>
+      [% IF components.size == 1 %]
+        [% SET default_component = components.0 %]
+      [% ELSE %]
+        [% SET default_component = defaults.component %]
+      [% END %]
+      [% PROCESS "global/select-menu.html.tmpl"
+                 name="component"
+                 options=components
+                 default=default_component
+                 size=10 %]
+    </td>
+    [% IF Param("usetargetmilestone") %]
       <td>
-        <b>Component:</b><br>
-        [% PROCESS "global/select-menu.html.tmpl"
-                   name="component"
-                   options=components
-                   default=defaults.component
-                   size=10 %]
+        <b>Target Milestone:</b><br>
+      [% PROCESS "global/select-menu.html.tmpl"
+                 name="target_milestone"
+                 options=milestones
+                 default=defaults.milestone
+                 size=10 %]
       </td>
-      [% IF Param("usetargetmilestone") %]
-        <td>
-          <b>Target Milestone:</b><br>
-        [% PROCESS "global/select-menu.html.tmpl"
-                   name="target_milestone"
-                   options=milestones
-                   default=defaults.target_milestone
-                   size=10 %]
-        </td>
-      [% END %]
-    </tr>
-  </table>
+    [% END %]
+  </tr>
+</table>
 
+[% IF verify_bug_groups %]
   <h3>Verify [% terms.Bug %] Group</h3>
-
+  
   [% IF old_groups.size %]
     <p>These groups are not legal for the '[% product.name FILTER html %]'
     product or you are not allowed to restrict [% terms.bugs %] to these groups.
@@ -157,6 +173,7 @@
     [% END %]
     </p>
   [% END %]
+[% END %]
 
 <input type="submit" id="change_product" value="Commit">