Attachment 296768 Details for Bug 433744 – Bugzilla::Bug::set_all($args) + Bugzilla::WebService::Bug::update() + process_bug.cgi + new user errors

[patch] Bugzilla::Bug::set_all($args) + Bugzilla::WebService::Bug::update() + process_bug.cgi + new user errors

patch.diff (text/plain), 33.80 KB, created by Noura El hawary on 2008-03-04 17:23:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Noura El hawary

Created: 2008-03-04 17:23:37 UTC

Size: 33.80 KB

patch

obsolete

>Index: Bugzilla/Bug.pm
>===================================================================
>RCS file: /cvs/qa/rh_bugzilla_3/Bugzilla/Bug.pm,v
>retrieving revision 1.26
>diff -u -r1.26 Bug.pm
>--- Bugzilla/Bug.pm	27 Feb 2008 21:50:21 -0000	1.26
>+++ Bugzilla/Bug.pm	4 Mar 2008 17:13:48 -0000
>@@ -553,6 +553,119 @@
>     return $params;
> }
> 
>+sub set_all {
>+    my ($self, $args) = @_;
>+
>+    # For security purposes, and because lots of other checks depend on it,
>+    # we set the product first before anything else.
>+    my $product_change; # Used only for strict_isolation checks, right now.
>+    if ($args->{product}) {
>+        my $changed = $self->set_product($args->{product},
>+                                        { component => $args->{component},
>+                                          version   => $args->{version},
>+                                          target_milestone => $args->{target_milestone},
>+                                          change_confirmed => $args->{confirm_product_change},
>+                                          other_bugs => $args->{other_bugs},
>+                                        });
>+        $product_change = $changed;                                                                                   
>+    }
>+
>+    # add/remove groups
>+    $self->remove_group($_) foreach @{$args->{remove_group}};
>+    $self->add_group($_) foreach @{$args->{add_group}};
>+   
>+    # set dependson and blocked
>+    if (defined $args->{dependson} || defined $args->{blocked}) {
>+        $self->set_dependencies($args->{dependson}, $args->{blocked});
>+    }
>+
>+    # set keywords , the hash key for the keywords can either be 
>+    # add_keyword, delete_keyword, or makeexact_keyword
>+    # get the action from the hash key name
>+    foreach my $key_name (keys %$args){
>+        if ($key_name =~ /([a-z]*)_keyword$/){
>+            my $keyword_action = $1;
>+            $self->modify_keywords($args->{$key_name}, $keyword_action);
>+        }
>+    }
>+
>+    # 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.
>+    # REDHAT EXTENSION START 406151
>+    # Added devel_whiteboard, qa_whiteboard, internal_whiteboard and fixed_in
>+    my @set_fields = qw(op_sys platform priority severity
>+                        component target_milestone version
>+                        url status_whiteboard devel_whiteboard
>+                        qa_whiteboard internal_whiteboard fixed_in summary
>+                        deadline remaining_time estimated_time assigned_to qa_contact);
>+    # REDHAT EXTENSION END 406151
>+
>+    my @custom_fields = Bugzilla->active_custom_fields;
>+
>+    # adding comment to a bug
>+    if ($args->{comment} || $args->{work_time}) {
>+        # Add a comment as needed to the bug. This is done early because
>+        # there are lots of things that want to check if we added a comment.
>+        $self->add_comment($args->{comment},
>+            { isprivate => $args->{commentprivacy},
>+              work_time => $args->{work_time} });
>+    }
>+    foreach my $field_name (@set_fields) {
>+        if ($args->{$field_name}) {
>+            my $method = "set_" . $field_name;
>+            $self->$method($args->{$field_name});
>+        }
>+    }
>+    $self->reset_assigned_to if $args->{set_default_assignee};
>+    $self->reset_qa_contact  if $args->{set_default_qa_contact};
>+
>+    # And set custom fields.
>+    foreach my $field (@custom_fields) {
>+        my $fname = $field->name;
>+        if (defined $args->{$fname}) {
>+            $self->set_custom_field($field, [$args->{$fname}]);
>+        }
>+    }
>+
>+    $self->set_alias($args->{alias}) if defined $args->{alias};
>+
>+    # set reporter_accessible and cclist_accessible
>+    $self->set_cclist_accessible($args->{cclist_accessible}) if $args->{cclist_accessible};
>+    $self->set_reporter_accessible($args->{reporter_accessible}) if $args->{reporter_accessible};
>+
>+    # mark an existing comment as private by its id. note the user
>+    # must be in the insidergroup or his change will be ignored
>+    $self->set_comment_is_private($_, 1) foreach @{$args->{private_comment}};
>+    $self->set_comment_is_private($_, 0) foreach @{$args->{non_private_comment}};
>+
>+
>+    # add/remove cc  
>+    $self->remove_cc($_) foreach @{$args->{remove_cc}};
>+    $self->add_cc($_) foreach @{$args->{add_cc}};
>+
>+    if ($product_change) {
>+        # 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.
>+        $self->_check_strict_isolation();
>+    }
>+    
>+    # Set the status, resolution, and dupe_of (if needed). This has to be done
>+    # down here, because the validity of status changes depends on other fields,
>+    # such as Target Milestone.
>+    # the hash key for
>+    # NOTE: not sure if knob here is the right thing to do as knob can 
>+    #       be the actual bug status or the action to be performed on 
>+    #       the bug status and resolution  
>+    $args->{knob} ||= 'none';
>+    $self->process_knob($args->{knob}, $args->{resolution}, $args->{dupe_id});
>+    
>+}
>+
> sub update {
>     my $self = shift;
> 
>@@ -1744,6 +1857,7 @@
> sub set_deadline { $_[0]->set('deadline', $_[1]); }
> sub set_dependencies {
>     my ($self, $dependson, $blocked) = @_;
>+
>     ($dependson, $blocked) = $self->_check_dependencies($dependson, $blocked);
>     # These may already be detainted, but all setters are supposed to
>     # detaint their input if they've run a validator (just as though
>Index: Bugzilla/WebService/Bug.pm
>===================================================================
>RCS file: /cvs/qa/rh_bugzilla_3/Bugzilla/WebService/Bug.pm,v
>retrieving revision 1.6
>diff -u -r1.6 Bug.pm
>--- Bugzilla/WebService/Bug.pm	29 Feb 2008 04:01:24 -0000	1.6
>+++ Bugzilla/WebService/Bug.pm	4 Mar 2008 17:14:32 -0000
>@@ -30,6 +30,7 @@
> use Bugzilla::Bug;
> use Bugzilla::BugMail;
> use Bugzilla::Constants;
>+use Bugzilla::Status;
> 
> #############
> # Constants #
>@@ -241,6 +242,229 @@
>     return undef;
> }
> 
>+# this is a function that updates a bug:
>+# it can be called as the following:
>+# $call = $rpc->call( 'Bug.update', {ids => [1,2], update => {bug_status => 'NEW', priority => 'P3'}}
>+sub update {
>+
>+    my ($self, $params) = @_;      
>+    my $user = Bugzilla->login(LOGIN_REQUIRED);
>+
>+    $params->{ids} ||  ThrowCodeError('param_required', { param => 'ids' });
>+    my $updates = $params->{update} ||  ThrowCodeError('param_required', { param => 'update' });
>+
>+    my @ids = ref($params->{ids}) ? @{$params->{ids}} : ($params->{ids});
>+
>+    my @valid_ids;
>+    foreach my $bug_id (@ids) {
>+        ValidateBugID($bug_id);
>+        push(@valid_ids, $bug_id);
>+    }
>+
>+    my @bug_objects;
>+    @bug_objects = @{Bugzilla::Bug->new_from_list(\@valid_ids)};
>+    scalar(@bug_objects) || ThrowUserError("no_bugs_chosen", {action => 'modify'}); 
>+
>+    # For each bug, we have to check if the user can edit the bug the product
>+    # is currently in, before we allow them to change anything.
>+    foreach my $bug (@bug_objects) {
>+        if (!Bugzilla->user->can_edit_product($bug->product_obj->id) ) {
>+            ThrowUserError("product_edit_denied",
>+                          { product => $bug->product });
>+        }
>+    }
>+    
>+    foreach my $bug (@bug_objects) {
>+        my $args;
>+
>+        if ($updates->{product}) {
>+            foreach my $product_field qw(component version target_milestone) {
>+                if (!$updates->{$product_field}){
>+                    ThrowUserError("product_update_not_allowed");
>+                }
>+            }  
>+            $args->{product} = $updates->{product};
>+            $args->{component} = $updates->{component};
>+            $args->{version} = $updates->{version};
>+            $args->{target_milestone} = $updates->{target_milestone};
>+            $args->{confirm_product_change} =  1;
>+            $args->{other_bugs} = \@bug_objects;
>+        }
>+        
>+        foreach my $group (@{$bug->product_obj->groups_valid}) {
>+            my $gid = $group->id;
>+            my $gname = $group->name;
>+            if (grep {$_ =~ /$gname/ } @{$updates->{remove_group}}) {
>+                push (@{$args->{remove_group}}, $gid);
>+            }
>+            elsif (grep {$_ =~ /$gname/ } @{$updates->{add_group}}) {
>+                push (@{$args->{add_group}}, $gid);
>+            }
>+        }
>+     
>+
>+        # dependson and blocked lists should be passed as string of bug ids 
>+        # separated by commas to set_all
>+        if (scalar @bug_objects > 1  && (defined $updates->{dependson} || defined $updates->{blocked})) {
>+            ThrowUserError("dependencies_not_allowed");
>+        }
>+        else {
>+            my %dependson_ids;
>+            map { $dependson_ids{$_} = 1 } @{$bug->dependson};
>+            delete $dependson_ids{$_} foreach @{$updates->{delete_dependson}};
>+
>+            map { $dependson_ids{$_} = 1 } @{$updates->{add_dependson}};
>+
>+            $args->{dependson} = ($updates->{delete_dependson} || $updates->{add_dependson}) ? 
>+                                 join(',', keys %dependson_ids) : '';  
>+
>+            my %blocked_ids;
>+            map { $blocked_ids{$_} = 1 } @{$bug->blocked};
>+            delete $blocked_ids{$_} foreach @{$updates->{delete_blocked}};
>+
>+            map { $blocked_ids{$_} = 1 } @{$updates->{add_blocked}};
>+
>+            $args->{blocked} = ($updates->{delete_blocked} || $updates->{add_blocked}) ? 
>+                                 join(',', keys %blocked_ids) : ''; 
>+
>+        }
>+
>+        foreach my $key_name (keys %{$params->{update}}){
>+            if ($key_name =~ /keyword$/){
>+                $args->{$key_name} = join(',', @{$updates->{$key_name}}) if defined $updates->{$key_name};
>+            }
>+        }
>+
>+        my @custom_fields = Bugzilla->active_custom_fields;
>+
>+        if ($updates->{comment} || $updates->{work_time}) {
>+            $args->{comment}        = $updates->{comment};
>+            $args->{work_time}      = $updates->{work_time};
>+            $args->{commentprivacy} = $updates->{private};
>+        }
>+
>+        # REDHAT EXTENSION START 406151
>+        # Added devel_whiteboard, qa_whiteboard, internal_whiteboard and fixed_in
>+        my @set_fields = qw(op_sys rep_platform priority bug_severity
>+                            component target_milestone version
>+                            bug_file_loc status_whiteboard devel_whiteboard
>+                            qa_whiteboard internal_whiteboard fixed_in short_desc
>+                            deadline remaining_time estimated_time);
>+        # REDHAT EXTENSION END 406151
>+
>+        push(@set_fields, 'assigned_to') if !$updates->{set_default_assignee};
>+        push(@set_fields, 'qa_contact')  if !$updates->{set_default_qa_contact};
>+
>+        my %field_mapping = (
>+            bug_severity => 'severity',
>+            rep_platform => 'platform',
>+            short_desc   => 'summary',
>+            bug_file_loc => 'url',
>+        );
>+
>+        foreach my $field (@set_fields) {
>+            if ($updates->{$field}) {
>+                my $mapped_field     = $field_mapping{$field};
>+                my $field_name       = $mapped_field || $field;
>+                $args->{$field_name} = $updates->{$field};
>+            }
>+        }
>+
>+        $args->{set_default_assignee}   = $updates->{set_default_assignee};
>+        $args->{set_default_qa_contact} = $updates->{set_default_qa_contact};
>+
>+        foreach my $field (@custom_fields) {
>+            my $fname = $field->name;
>+            $args->{$fname} = $updates->{$fname} if defined $updates->{$fname};
>+        }
>+    
>+        # Certain changes can only happen on individual bugs, never on mass-changes.
>+        if (scalar @bug_objects == 1) {
>+            if (Bugzilla->params->{"usebugaliases"} && defined $updates->{alias}) {
>+                $args->{alias} = $updates->{alias};
>+            }
>+
>+            $args->{cclist_accessible}   = $updates->{cclist_accessible};
>+            $args->{reporter_accessible} = $updates->{reporter_accessible};
>+
>+            # this part of the code is not really user friendly as the user needs 
>+            # to know the actual comment id as how it is in the longdescs table
>+            # to be able to mark/unmark privacy of the comment
>+            $args->{private_comment}     = $updates->{private_comment};
>+            $args->{non_private_comment} = $updates->{non_private_comment};
>+        }
>+
>+        $args->{add_cc}    = $updates->{add_cc};
>+        $args->{remove_cc} = $updates->{delete_cc};
>+
>+        # You cannot mark bugs as duplicates when changing several bugs at once
>+        # (because currently there is no way to check for duplicate loops in that
>+        # situation).
>+        if (scalar @bug_objects > 1 && $updates->{dupe_id}) {
>+            ThrowUserError('dupe_not_allowed');
>+        }
>+        
>+        if (scalar @bug_objects > 1 && $updates->{alias}) {
>+            ThrowUserError('alias_not_allowed');
>+        }
>+
>+        # Set the status, resolution, and dupe_of (if needed). This has to be done
>+        # down here, because the validity of status changes depends on other fields,
>+        # such as Target Milestone.
>+        if ( defined $updates->{bug_status} && ($bug->status->name ne $updates->{bug_status}) 
>+                                            && (is_open_state($updates->{bug_status})) ) {
>+            $args->{knob} = $updates->{bug_status};
>+        } 
>+        elsif ( defined $updates->{bug_status} && ($bug->status->name ne $updates->{bug_status}) 
>+                                               &&  !(is_open_state($updates->{bug_status})) ) {
>+            $args->{knob} = $updates->{bug_status};
>+            $args->{resolution} = $updates->{resolution};
>+        }
>+        elsif ( defined $updates->{resolution} && ($bug->resolution ne $updates->{resolution})
>+                                               && !(is_open_state($bug->status->name)) ) {
>+            $args->{knob} = 'change_resolution';
>+            $args->{resolution} = $updates->{resolution};
>+        }
>+        elsif ( defined $updates->{dupe_id} ) {
>+            $args->{knob}    = 'duplicate';
>+            $args->{dupe_id} = $updates->{dupe_id};
>+        }
>+        else {
>+            $args->{knob} = 'none';
>+        }
>+
>+        $bug->set_all($args);
>+    }
>+
>+    my $dbh = Bugzilla->dbh;
>+    my %changes;
>+    # Do Actual Database Updates #
>+    foreach my $bug (@bug_objects) {     
>+        $dbh->bz_start_transaction();
>+    
>+        my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
>+        my $bug_changes = $bug->update($timestamp);
>+        $changes{$bug->bug_id} = [$bug_changes];
>+
>+        my $keyword_changes= $bug->update_keywords($timestamp);
>+        push @{$changes{$bug->bug_id}}, {keywords => $keyword_changes} if $keyword_changes;       
>+
>+        my ($cc_removed) = $bug->update_cc($timestamp);
>+        $cc_removed = [map {$_->login} @$cc_removed];
>+        push  @{$changes{$bug->bug_id}}, {cc => $cc_removed} if $cc_removed;
>+
>+        my ($dep_changes) = $bug->update_dependencies($timestamp);
>+        push  @{$changes{$bug->bug_id}}, {dependencies => $dep_changes} if $dep_changes;
>+
>+        my $mail_results = Bugzilla::BugMail::Send($bug->bug_id, { changer => Bugzilla->user->login });
>+        push  @{$changes{$bug->bug_id}}, $mail_results;
>+   
>+        $dbh->bz_commit_transaction();
>+    }
>+
>+    return { bug_updates => \%changes };   
>+}
>+
> 1;
> 
> __END__
>Index: process_bug.cgi
>===================================================================
>RCS file: /cvs/qa/rh_bugzilla_3/process_bug.cgi,v
>retrieving revision 1.17
>diff -u -r1.17 process_bug.cgi
>--- process_bug.cgi	27 Feb 2008 21:50:21 -0000	1.17
>+++ process_bug.cgi	4 Mar 2008 17:15:17 -0000
>@@ -65,6 +65,7 @@
> my $user = Bugzilla->login(LOGIN_REQUIRED);
> 
> my $cgi = Bugzilla->cgi;
>+
> my $dbh = Bugzilla->dbh;
> my $template = Bugzilla->template;
> my $vars = {};
>@@ -241,185 +242,198 @@
>     }
> }
> 
>-# For security purposes, and because lots of other checks depend on it,
>-# we set the product first before anything else.
>-my $product_change; # Used only for strict_isolation checks, right now.
>-if (should_set('product')) {
>-    foreach my $b (@bug_objects) {
>-        my $changed = $b->set_product(scalar $cgi->param('product'),
>-            { component        => scalar $cgi->param('component'),
>-              version          => scalar $cgi->param('version'),
>-              target_milestone => scalar $cgi->param('target_milestone'),
>-              change_confirmed => scalar $cgi->param('confirm_product_change'),
>-              other_bugs => \@bug_objects,
>-            });
>-        $product_change ||= $changed;
>-    }
>-}
>-        
>-# strict_isolation checks mean that we should set the groups
>-# immediately after changing the product.
>-foreach my $b (@bug_objects) {
>-    foreach my $group (@{$b->product_obj->groups_valid}) {
>-        my $gid = $group->id;
>-        if (should_set("bit-$gid", 1)) {
>-            # Check ! first to avoid having to check defined below.
>-            if (!$cgi->param("bit-$gid")) {
>-                $b->remove_group($gid);
>-            }
>-            # "== 1" is important because mass-change uses -1 to mean
>-            # "don't change this restriction"
>-            elsif ($cgi->param("bit-$gid") == 1) {
>-                $b->add_group($gid);
>-            }
>-        }
>-    }
>-}
>+# Noura adds this
>+foreach my $bug (@bug_objects) {
> 
>-if ($cgi->param('id') && (defined $cgi->param('dependson')
>-                          || defined $cgi->param('blocked')) )
>-{
>-    $first_bug->set_dependencies(scalar $cgi->param('dependson'),
>-                                 scalar $cgi->param('blocked'));
>-}
>-# Right now, you can't modify dependencies on a mass change.
>-else {
>-    $cgi->delete('dependson');
>-    $cgi->delete('blocked');
>-}
>+    my $args;
> 
>-my $any_keyword_changes;
>-if (defined $cgi->param('keywords')) {
>-    foreach my $b (@bug_objects) {
>-        my $return =
>-            $b->modify_keywords(scalar $cgi->param('keywords'),
>-                                scalar $cgi->param('keywordaction'));
>-        $any_keyword_changes ||= $return;
>+    if (should_set('product')) {
>+        $args->{product} = scalar $cgi->param('product');
>+        $args->{component} = scalar $cgi->param('component');
>+        $args->{version} = scalar $cgi->param('version');
>+        $args->{target_milestone} = scalar $cgi->param('target_milestone');
>+        $args->{confirm_product_change} =  scalar $cgi->param('confirm_product_change');        
>+        $args->{other_bugs} = \@bug_objects;
>+    }
>+
>+     foreach my $group (@{$bug->product_obj->groups_valid}) {
>+         my $gid = $group->id;
>+         if (should_set("bit-$gid", 1)) {
>+             # Check ! first to avoid having to check defined below.
>+             if (!$cgi->param("bit-$gid")) {
>+                push (@{$args->{remove_group}}, $gid);
>+             }
>+             # "== 1" is important because mass-change uses -1 to mean
>+             # "don't change this restriction"
>+             elsif ($cgi->param("bit-$gid") == 1) {
>+                push (@{$args->{add_group}}, $gid);
>+             }
>+         }
>+     }
>+ 
>+    if ($cgi->param('id') && (defined $cgi->param('dependson')
>+                           || defined $cgi->param('blocked')) )
>+    {
>+        $args->{dependson} = scalar $cgi->param('dependson');
>+        $args->{blocked} = scalar $cgi->param('blocked');
>+    }
>+    # Right now, you can't modify dependencies on a mass change.
>+    else {
>+        $cgi->delete('dependson');
>+        $cgi->delete('blocked');
>+     }
>+    if (defined $cgi->param('keywords')) {
>+        my $keyword_arg_name = scalar $cgi->param('keywordaction') . "_keyword";
>+        $args->{$keyword_arg_name}  = scalar $cgi->param('keywords');
>     }
>-}
> 
>-# 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.
>-# REDHAT EXTENSION START 406151
>-# Added devel_whiteboard, qa_whiteboard, internal_whiteboard and fixed_in
>-my @set_fields = qw(op_sys rep_platform priority bug_severity
>-                    component target_milestone version
>-                    bug_file_loc status_whiteboard devel_whiteboard
>-                    qa_whiteboard internal_whiteboard fixed_in short_desc
>-                    deadline remaining_time estimated_time);
>-# REDHAT EXTENSION END 406151
>-push(@set_fields, 'assigned_to') if !$cgi->param('set_default_assignee');
>-push(@set_fields, 'qa_contact')  if !$cgi->param('set_default_qa_contact');
>-my @custom_fields = Bugzilla->active_custom_fields;
>-
>-my %methods = (
>-    bug_severity => 'set_severity',
>-    rep_platform => 'set_platform',
>-    short_desc   => 'set_summary',
>-    bug_file_loc => 'set_url',
>-);
>-foreach my $b (@bug_objects) {
>+    my @custom_fields = Bugzilla->active_custom_fields;
>+ 
>     if (should_set('comment') || $cgi->param('work_time')) {
>-        # Add a comment as needed to each bug. This is done early because
>-        # there are lots of things that want to check if we added a comment.
>-        $b->add_comment(scalar($cgi->param('comment')),
>-            { isprivate => scalar $cgi->param('commentprivacy'),
>-              work_time => scalar $cgi->param('work_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));
>+        $args->{comment} = scalar $cgi->param('comment');
>+        $args->{work_time} = $cgi->param('work_time');
>+        $args->{commentprivacy} = scalar $cgi->param('commentprivacy');
>+    }
>+
>+    # 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.
>+    # REDHAT EXTENSION START 406151
>+    # Added devel_whiteboard, qa_whiteboard, internal_whiteboard and fixed_in
>+    my @set_fields = qw(op_sys rep_platform priority bug_severity
>+                        component target_milestone version
>+                        bug_file_loc status_whiteboard devel_whiteboard
>+                        qa_whiteboard internal_whiteboard fixed_in short_desc
>+                        deadline remaining_time estimated_time);
>+    # REDHAT EXTENSION END 406151
>+
>+    push(@set_fields, 'assigned_to') if !$cgi->param('set_default_assignee');
>+    push(@set_fields, 'qa_contact')  if !$cgi->param('set_default_qa_contact');
>+
>+    my %field_mapping = (
>+        bug_severity => 'severity',
>+        rep_platform => 'platform',
>+        short_desc   => 'summary',
>+        bug_file_loc => 'url',
>+    );
>+
>+    foreach my $field (@set_fields) {
>+        if (should_set($field)) {
>+            my $mapped_field = $field_mapping{$field};
>+            my $field_name = $mapped_field || $field;
>+            $args->{$field_name} = $cgi->param($field);
>+            
>+
>         }
>     }
>-    $b->reset_assigned_to if $cgi->param('set_default_assignee');
>-    $b->reset_qa_contact  if $cgi->param('set_default_qa_contact');
>+ 
>+    $args->{set_default_assignee} = $cgi->param('set_default_assignee');
>+    $args->{set_default_qa_contact} = $cgi->param('set_default_qa_contact');
> 
>-    # And set custom fields.
>     foreach my $field (@custom_fields) {
>         my $fname = $field->name;
>         if (should_set($fname, 1)) {
>-            $b->set_custom_field($field, [$cgi->param($fname)]);
>+            $args->{$fname} = $cgi->param($fname);
>         }
>     }
>-}
>+ 
>+    # Certain changes can only happen on individual bugs, never on mass-changes.
>+    if (defined $cgi->param('id')) {
>+        if (Bugzilla->params->{"usebugaliases"} && defined $cgi->param('alias')) {
>+            $args->{alias} = $cgi->param('alias');
>+        }
>+ 
>+        # reporter_accessible and cclist_accessible--these are only set if
>+        # the user can change them and they appear on the page.
>+        if (should_set('cclist_accessible', 1)) {
>+            $args->{cclist_accessible} = $cgi->param('cclist_accessible');
>+        }
>+        if (should_set('reporter_accessible', 1)) {
>+            $args->{reporter_accessible} = $cgi->param('reporter_accessible');
>+        }
>+
>+        # You can only mark/unmark comments as private on single bugs. If
>+        # you're not in the insider group, this code won't do anything.
>+        foreach my $cgi_field (grep(/^defined_isprivate/, $cgi->param())) {
>+            $cgi_field =~ /(\d+)$/;
>+            my $comment_id = $1;
>+            if ($cgi->param("isprivate_$comment_id")){
>+                push (@{$args->{private_comment}}, $comment_id); 
>+            }
>+            else {
>+                push (@{$args->{non_private_comment}}, $comment_id);
>+            }
>+        }       
>+    }   
> 
>-# Certain changes can only happen on individual bugs, never on mass-changes.
>-if (defined $cgi->param('id')) {
>-    # Since aliases are unique (like bug numbers), they can only be changed
>-    # for one bug at a time.
>-    if (Bugzilla->params->{"usebugaliases"} && defined $cgi->param('alias')) {
>-        $first_bug->set_alias($cgi->param('alias'));
>-    }
>+    # We need to check the addresses involved in a CC change before we touch 
>+    # any bugs. What we'll do here is formulate the CC data into two arrays of
>+    # users involved in this CC change.  Then those arrays can be used later 
>+    # on for the actual change.
>+    if (defined $cgi->param('newcc')
>+        || defined $cgi->param('addselfcc')
>+        || defined $cgi->param('removecc')
>+        || defined $cgi->param('masscc')) {
>+
>+        # If masscc is defined, then we came from buglist and need to either add or
>+        # remove cc's... otherwise, we came from bugform and may need to do both.
>+        my ($cc_add, $cc_remove) = "";
>+        if (defined $cgi->param('masscc')) {
>+            if ($cgi->param('ccaction') eq 'add') {
>+                $cc_add = join(' ',$cgi->param('masscc'));
>+            } elsif ($cgi->param('ccaction') eq 'remove') {
>+                $cc_remove = join(' ',$cgi->param('masscc'));
>+            }
>+        } else {
>+            $cc_add = join(' ',$cgi->param('newcc'));
>+            # We came from bug_form which uses a select box to determine what cc's
>+            # need to be removed...
>+            if (defined $cgi->param('removecc') && $cgi->param('cc')) {
>+                $cc_remove = join (",", $cgi->param('cc'));
>+            }
>+        }
> 
>-    # reporter_accessible and cclist_accessible--these are only set if
>-    # the user can change them and they appear on the page.
>-    if (should_set('cclist_accessible', 1)) {
>-        $first_bug->set_cclist_accessible($cgi->param('cclist_accessible'))
>-    }
>-    if (should_set('reporter_accessible', 1)) {
>-        $first_bug->set_reporter_accessible($cgi->param('reporter_accessible'))
>+        push(@{$args->{add_cc}}, split(/[\s,]+/, $cc_add)) if $cc_add;
>+        push(@{$args->{add_cc}}, Bugzilla->user) if $cgi->param('addselfcc');
>+
>+        push(@{$args->{remove_cc}}, split(/[\s,]+/, $cc_remove)) if $cc_remove;
>     }
>-    
>-    # You can only mark/unmark comments as private on single bugs. If
>-    # you're not in the insider group, this code won't do anything.
>-    foreach my $field (grep(/^defined_isprivate/, $cgi->param())) {
>-        $field =~ /(\d+)$/;
>-        my $comment_id = $1;
>-        $first_bug->set_comment_is_private($comment_id,
>-                                           $cgi->param("isprivate_$comment_id"));
>+ 
>+    # You cannot mark bugs as duplicates when changing several bugs at once
>+    # (because currently there is no way to check for duplicate loops in that
>+    # situation).
>+    if (!$cgi->param('id') && $cgi->param('dup_id')) {
>+        ThrowUserError('dupe_not_allowed');
>     }
>-}
>-
>-# We need to check the addresses involved in a CC change before we touch 
>-# any bugs. What we'll do here is formulate the CC data into two arrays of
>-# users involved in this CC change.  Then those arrays can be used later 
>-# on for the actual change.
>-my (@cc_add, @cc_remove);
>-if (defined $cgi->param('newcc')
>-    || defined $cgi->param('addselfcc')
>-    || defined $cgi->param('removecc')
>-    || defined $cgi->param('masscc')) {
>-        
>-    # If masscc is defined, then we came from buglist and need to either add or
>-    # remove cc's... otherwise, we came from bugform and may need to do both.
>-    my ($cc_add, $cc_remove) = "";
>-    if (defined $cgi->param('masscc')) {
>-        if ($cgi->param('ccaction') eq 'add') {
>-            $cc_add = join(' ',$cgi->param('masscc'));
>-        } elsif ($cgi->param('ccaction') eq 'remove') {
>-            $cc_remove = join(' ',$cgi->param('masscc'));
>-        }
>-    } else {
>-        $cc_add = join(' ',$cgi->param('newcc'));
>-        # We came from bug_form which uses a select box to determine what cc's
>-        # need to be removed...
>-        if (defined $cgi->param('removecc') && $cgi->param('cc')) {
>-            $cc_remove = join (",", $cgi->param('cc'));
>+ 
>+    # Set the status, resolution, and dupe_of (if needed). This has to be done
>+    # down here, because the validity of status changes depends on other fields,
>+    # such as Target Milestone.
>+   
>+    if (should_set('knob')) {
>+        # First, get the correct resolution <select>, in case there is more
>+        # than one open -> closed transition allowed. Allow to fallback to
>+        # 'resolution' (useful when called from email_in.pl).
>+        my $knob = $cgi->param('knob');
>+        my $status = new Bugzilla::Status({name => $knob});
>+        my $resolution;
>+        if ($status) {
>+            $resolution = $cgi->param('resolution_knob_' . $status->id)
>+                          || $cgi->param('resolution');
>         }
>+        else {
>+            $resolution = $cgi->param('resolution_knob_change_resolution');
>+        }
>+        $args->{knob} = $knob;
>+        $args->{resolution} = $resolution;
>+        $args->{dupe_id} = scalar $cgi->param('dup_id');
>+ 
>     }
>-
>-    push(@cc_add, split(/[\s,]+/, $cc_add)) if $cc_add;
>-    push(@cc_add, Bugzilla->user) if $cgi->param('addselfcc');
>-
>-    push(@cc_remove, split(/[\s,]+/, $cc_remove)) if $cc_remove;
>-}
>-
>-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();
>-    }
>+   
>+    $bug->set_all($args); 
> }
>+    
>+ 
> 
> my $move_action = $cgi->param('action') || '';
> if ($move_action eq Bugzilla->params->{'move-button-text'}) {
>@@ -487,38 +501,6 @@
>     exit;
> }
> 
>-
>-# You cannot mark bugs as duplicates when changing several bugs at once
>-# (because currently there is no way to check for duplicate loops in that
>-# situation).
>-if (!$cgi->param('id') && $cgi->param('dup_id')) {
>-    ThrowUserError('dupe_not_allowed');
>-}
>-
>-# Set the status, resolution, and dupe_of (if needed). This has to be done
>-# down here, because the validity of status changes depends on other fields,
>-# such as Target Milestone.
>-foreach my $b (@bug_objects) {
>-    if (should_set('knob')) {
>-        # First, get the correct resolution <select>, in case there is more
>-        # than one open -> closed transition allowed. Allow to fallback to
>-        # 'resolution' (useful when called from email_in.pl).
>-        my $knob = $cgi->param('knob');
>-        my $status = new Bugzilla::Status({name => $knob});
>-        my $resolution;
>-        if ($status) {
>-            $resolution = $cgi->param('resolution_knob_' . $status->id)
>-                          || $cgi->param('resolution');
>-        }
>-        else {
>-            $resolution = $cgi->param('resolution_knob_change_resolution');
>-        }
>-        
>-        # Translate the knob values into new status and resolution values.
>-        $b->process_knob($knob, $resolution, scalar $cgi->param('dup_id'));
>-    }
>-}
>-
> ##############################
> # Do Actual Database Updates #
> ##############################
>Index: template/en/default/global/user-error.html.tmpl
>===================================================================
>RCS file: /cvs/qa/rh_bugzilla_3/template/en/default/global/user-error.html.tmpl,v
>retrieving revision 1.15
>diff -u -r1.15 user-error.html.tmpl
>--- template/en/default/global/user-error.html.tmpl	29 Feb 2008 05:45:48 -0000	1.15
>+++ template/en/default/global/user-error.html.tmpl	4 Mar 2008 17:15:40 -0000
>@@ -344,6 +344,17 @@
>     The custom field '[% name FILTER html %]' cannot be deleted because
>     at least one [% terms.bug %] has a non empty value for this field.
> 
>+  [% ELSIF error == "product_update_not_allowed" %]
>+    [% title = "Product Update Not Allowed" %]
>+    Missing required information to change a product of a bug.
>+    You must specify all the following:
>+    component name, version, and target milestone.
>+
>+  [% ELSIF error == "alias_not_allowed" %]
>+    [% title = "Alias Update Not Allowed" %]
>+    You cannot update the alias when changing several
>+    bugs at once.
>+
>   [% ELSIF error == "dependency_loop_multi" %]
>     [% title = "Dependency Loop Detected" %]
>     The following [% terms.bug %](s) would appear on both the "depends on"
>@@ -358,6 +369,11 @@
>     [% title = "Dependency Loop Detected" %]
>     You can't make [% terms.abug %] block itself or depend on itself.
> 
>+  [% ELSIF error == "dependencies_not_allowed" %]
>+    [% title = "Cannot mark $terms.Bug as dependson/blocked" %]
>+    You cannot mark [% terms.abug %] as dependson or blocked when
>+    changing several [% terms.bugs %] at once.
>+
>   [% ELSIF error == "dupe_id_required" %]
>     [% title = "Duplicate $terms.Bug Id Required" %]
>     You must specify [% terms.abug %] id to mark this [% terms.bug %]

Flags:

dkl: review-

Actions: View | Diff

Attachments on bug 433744: 295483 | 296081 | 296620 | 296768 | 297132 | 297564 | 297565