diff options
| -rw-r--r-- | Bugzilla/Bug.pm | 52 | ||||
| -rw-r--r-- | Bugzilla/Object.pm | 15 | ||||
| -rwxr-xr-x | attachment.cgi | 2 | ||||
| -rw-r--r-- | extensions/Voting/Extension.pm | 2 | ||||
| -rwxr-xr-x | process_bug.cgi | 3 |
5 files changed, 44 insertions, 30 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 8705d3805..378e219ff 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -192,11 +192,13 @@ sub VALIDATOR_DEPENDENCIES { my %deps = ( assigned_to => ['component'], - bug_status => ['product', 'comment'], + bug_status => ['product', 'comment', 'target_milestone'], cc => ['component'], component => ['product'], + dup_id => ['bug_status', 'resolution'], groups => ['product'], keywords => ['product'], + resolution => ['bug_status'], qa_contact => ['component'], target_milestone => ['product'], version => ['product'], @@ -1971,7 +1973,6 @@ sub set_all { my %normal_set_all; foreach my $name (keys %$params) { # These are handled separately below. - next if grep($_ eq $name, qw(status resolution dup_id)); if ($self->can("set_$name")) { $normal_set_all{$name} = $params->{$name}; } @@ -2010,22 +2011,6 @@ sub set_all { { ThrowUserError('dupe_not_allowed'); } - - # Seting the status, resolution, and dupe_of has to be done - # down here, because the validity of status changes depends on - # other fields, such as Target Milestone. - if (exists $params->{'status'}) { - $self->set_status($params->{'status'}, - { resolution => $params->{'resolution'}, - dupe_of => $params->{'dup_id'} }); - } - elsif (exists $params->{'resolution'}) { - $self->set_resolution($params->{'resolution'}, - { dupe_of => $params->{'dup_id'} }); - } - elsif (exists $params->{'dup_id'}) { - $self->set_dup_id($params->{'dup_id'}); - } } # Helper for set_all that helps with fields that have an "add/remove" @@ -2120,6 +2105,21 @@ sub set_dup_id { $self->set('dup_id', $dup_id); my $new = $self->dup_id; return if $old == $new; + + # Make sure that we have the DUPLICATE resolution. This is needed + # if somebody calls set_dup_id without calling set_bug_status or + # set_resolution. + if ($self->resolution ne 'DUPLICATE') { + # Even if the current status is VERIFIED, we change it back to + # RESOLVED (or whatever the duplicate_or_move_bug_status is) here, + # because that's the same thing the UI does when you click on the + # "Mark as Duplicate" link. If people really want to retain their + # current status, they can use set_bug_status and set the DUPLICATE + # resolution before getting here. + $self->set_bug_status( + Bugzilla->params->{'duplicate_or_move_bug_status'}, + { resolution => 'DUPLICATE' }); + } # Update the other bug. my $dupe_of = new Bugzilla::Bug($self->dup_id); @@ -2343,13 +2343,17 @@ sub set_resolution { # of another, theoretically. Note that this code block will also run # when going between different closed states. if ($self->resolution eq 'DUPLICATE') { - if ($params->{dupe_of}) { - $self->set_dup_id($params->{dupe_of}); + if (my $dup_id = $params->{dup_id}) { + $self->set_dup_id($dup_id); } elsif (!$self->dup_id) { ThrowUserError('dupe_id_required'); } } + + # This method has handled dup_id, so set_all doesn't have to worry + # about it now. + delete $params->{dup_id}; } sub clear_resolution { my $self = shift; @@ -2360,7 +2364,7 @@ sub clear_resolution { $self->_clear_dup_id; } sub set_severity { $_[0]->set('bug_severity', $_[1]); } -sub set_status { +sub set_bug_status { my ($self, $status, $params) = @_; my $old_status = $self->status; $self->set('bug_status', $status); @@ -2368,11 +2372,15 @@ sub set_status { delete $self->{'statuses_available'}; delete $self->{'choices'}; my $new_status = $self->status; - + if ($new_status->is_open) { # Check for the everconfirmed transition $self->_set_everconfirmed($new_status->name eq 'UNCONFIRMED' ? 0 : 1); $self->clear_resolution(); + # Calling clear_resolution handled the "resolution" and "dup_id" + # setting, so set_all doesn't have to worry about them. + delete $params->{resolution}; + delete $params->{dup_id}; } else { # We do this here so that we can make sure closed statuses have diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index ba5b82f9f..67517c405 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -316,13 +316,20 @@ sub set { sub set_all { my ($self, $params) = @_; - my @sorted_names = $self->_sort_by_dep(keys %$params); + # Don't let setters modify the values in $params for the caller. + my %field_values = %$params; + + my @sorted_names = $self->_sort_by_dep(keys %field_values); foreach my $key (@sorted_names) { + # It's possible for one set_ method to delete a key from $params + # for another set method, so if that's happened, we don't call the + # other set method. + next if !exists $field_values{$key}; my $method = "set_$key"; - $self->$method($params->{$key}); + $self->$method($field_values{$key}, \%field_values); } - Bugzilla::Hook::process('object_end_of_set_all', { object => $self, - params => $params }); + Bugzilla::Hook::process('object_end_of_set_all', + { object => $self, params => \%field_values }); } sub update { diff --git a/attachment.cgi b/attachment.cgi index b650a3522..cdfcc6bf7 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -515,7 +515,7 @@ sub insert { && ($bug_status->name ne 'UNCONFIRMED' || $bug->product_obj->allows_unconfirmed)) { - $bug->set_status($bug_status->name); + $bug->set_bug_status($bug_status->name); $bug->clear_resolution(); } # Make sure the person we are taking the bug from gets mail. diff --git a/extensions/Voting/Extension.pm b/extensions/Voting/Extension.pm index 97e061313..24ac4fdb5 100644 --- a/extensions/Voting/Extension.pm +++ b/extensions/Voting/Extension.pm @@ -844,7 +844,7 @@ sub _confirm_if_vote_confirmed { } ThrowCodeError('voting_no_open_bug_status') unless $new_status; - # We cannot call $bug->set_status() here, because a user without + # We cannot call $bug->set_bug_status() here, because a user without # canconfirm privs should still be able to confirm a bug by # popular vote. We already know the new status is valid, so it's safe. $bug->{bug_status} = $new_status; diff --git a/process_bug.cgi b/process_bug.cgi index e71c7ef4d..bb4a9f653 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -262,7 +262,6 @@ my %field_translation = ( set_default_qa_contact => 'reset_qa_contact', keywordaction => 'keywords_action', confirm_product_change => 'product_change_confirmed', - bug_status => 'status', ); my %set_all_fields = ( other_bugs => \@bug_objects ); @@ -409,7 +408,7 @@ if ($move_action eq Bugzilla->params->{'move-button-text'}) { my $new_status = Bugzilla->params->{'duplicate_or_move_bug_status'}; foreach my $bug (@bug_objects) { - $bug->set_status($new_status, {resolution => 'MOVED', moving => 1}); + $bug->set_bug_status($new_status, {resolution => 'MOVED', moving => 1}); } $_->update() foreach @bug_objects; $dbh->bz_commit_transaction(); |