diff options
| -rwxr-xr-x | Bugzilla/Bug.pm | 533 | ||||
| -rw-r--r-- | Bugzilla/Status.pm | 98 | ||||
| -rwxr-xr-x | process_bug.cgi | 257 | ||||
| -rw-r--r-- | template/en/default/global/code-error.html.tmpl | 4 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 37 |
5 files changed, 488 insertions, 441 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 28752da46..20e87c25c 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -164,6 +164,7 @@ use constant UPDATE_VALIDATORS => { assigned_to => \&_check_assigned_to, bug_status => \&_check_bug_status, cclist_accessible => \&Bugzilla::Object::check_boolean, + dup_id => \&_check_dup_id, qa_contact => \&_check_qa_contact, reporter_accessible => \&Bugzilla::Object::check_boolean, resolution => \&_check_resolution, @@ -178,14 +179,14 @@ sub UPDATE_COLUMNS { my @columns = qw( alias assigned_to + bug_file_loc + bug_severity + bug_status cclist_accessible component_id deadline estimated_time everconfirmed - bug_file_loc - bug_severity - bug_status op_sys priority product_id @@ -437,12 +438,8 @@ sub run_create_validators { delete $params->{product}; ($params->{bug_status}, $params->{everconfirmed}) - = $class->_check_bug_status($params->{bug_status}, $product); - - # Check whether a comment is required on bug creation. - my $vars = {}; - $vars->{comment_exists} = ($params->{comment} =~ /\S+/) ? 1 : 0; - Bugzilla::Bug->check_status_change_triggers($params->{bug_status}, [], $vars); + = $class->_check_bug_status($params->{bug_status}, $product, + $params->{comment}); $params->{target_milestone} = $class->_check_target_milestone( $params->{target_milestone}, $product); @@ -582,14 +579,19 @@ sub update { $delta_ts); } - # If this bug is no longer a duplicate, it no longer belongs in the - # dup table. - if (exists $changes->{'resolution'} - && $changes->{'resolution'}->[0] eq 'DUPLICATE') - { - my $dup_id = $self->dup_id; + # Check if we have to update the duplicates table and the other bug. + my ($old_dup, $cur_dup) = ($old_bug->dup_id || 0, $self->dup_id || 0); + if ($old_dup != $cur_dup) { $dbh->do("DELETE FROM duplicates WHERE dupe = ?", undef, $self->id); - $changes->{'dupe_of'} = [$dup_id, undef]; + if ($cur_dup) { + $dbh->do('INSERT INTO duplicates (dupe, dupe_of) VALUES (?,?)', + undef, $self->id, $cur_dup); + if (my $update_dup = delete $self->{_dup_for_update}) { + $update_dup->update(); + } + } + + $changes->{'dup_id'} = [$old_dup || undef, $cur_dup || undef]; } # If any change occurred, refresh the timestamp of the bug. @@ -893,41 +895,62 @@ sub _check_bug_severity { } sub _check_bug_status { - my ($invocant, $status, $product) = @_; + my ($invocant, $status, $product, $comment) = @_; my $user = Bugzilla->user; - my @valid_statuses; + # Make sure this is a valid status. + my $new_status = ref $status ? $status : Bugzilla::Status->check($status); + + my $old_status; # Note that this is undef for new bugs. if (ref $invocant) { $product = $invocant->product_obj; - @valid_statuses = map { $_->name } @{$invocant->status->can_change_to}; - } - else { - @valid_statuses = map { $_->name } @{Bugzilla::Status->can_change_to()}; - } - - if (!$product->votes_to_confirm) { - # UNCONFIRMED becomes an invalid status if votes_to_confirm is 0, - # even if you are in editbugs. - @valid_statuses = grep {$_ ne 'UNCONFIRMED'} @valid_statuses; + $old_status = $invocant->status; + my $comments = $invocant->{added_comments} || []; + $comment = $comments->[-1]; } - - if (!ref($invocant)) { + + # Check permissions for users filing new bugs. + if (!ref $invocant) { + my $default_status = Bugzilla::Status->can_change_to->[0]; + if ($user->in_group('editbugs', $product->id) || $user->in_group('canconfirm', $product->id)) { # If the user with privs hasn't selected another status, # select the first one of the list. - $status ||= $valid_statuses[0]; + $new_status ||= $default_status; } else { # A user with no privs cannot choose the initial status. - $status = $valid_statuses[0]; + $new_status = $default_status; } } - # This check already takes the workflow into account. - check_field('bug_status', $status, \@valid_statuses); + # Make sure this is a valid transition. + if (!$new_status->allow_change_from($old_status, $product)) { + ThrowUserError('illegal_bug_status_transition', + { old => $old_status, new => $new_status }); + } - return $status if ref $invocant; + # Check if a comment is required for this change. + if ($new_status->comment_required_on_change_from($old_status) && !$comment) + { + ThrowUserError('comment_required', { old => $old_status, + new => $new_status }); + + } + + if (ref $invocant && $new_status->name eq 'ASSIGNED' + && Bugzilla->params->{"usetargetmilestone"} + && Bugzilla->params->{"musthavemilestoneonaccept"} + # musthavemilestoneonaccept applies only if at least two + # target milestones are defined for the product. + && scalar(@{ $product->milestones }) > 1 + && $invocant->target_milestone eq $product->default_milestone) + { + ThrowUserError("milestone_required", { bug => $invocant }); + } + + return $new_status->name if ref $invocant; return ($status, $status eq 'UNCONFIRMED' ? 0 : 1); } @@ -1073,6 +1096,85 @@ sub _check_dependencies { return ($deps{'dependson'}, $deps{'blocked'}); } +sub _check_dup_id { + my ($self, $dupe_of) = @_; + my $dbh = Bugzilla->dbh; + + $dupe_of = trim($dupe_of); + $dupe_of || ThrowCodeError('undefined_field', { field => 'dup_id' }); + # Make sure we can change the original bug (issue A on bug 96085) + ValidateBugID($dupe_of, 'dup_id'); + + # Make sure a loop isn't created when marking this bug + # as duplicate. + my %dupes; + my $this_dup = $dupe_of; + my $sth = $dbh->prepare('SELECT dupe_of FROM duplicates WHERE dupe = ?'); + + while ($this_dup) { + if ($this_dup == $self->id) { + ThrowUserError('dupe_loop_detected', { bug_id => $self->id, + dupe_of => $dupe_of }); + } + # If $dupes{$this_dup} is already set to 1, then a loop + # already exists which does not involve this bug. + # As the user is not responsible for this loop, do not + # prevent him from marking this bug as a duplicate. + last if exists $dupes{$this_dup}; + $dupes{$this_dup} = 1; + $this_dup = $dbh->selectrow_array($sth, undef, $this_dup); + } + + my $cur_dup = $self->dup_id || 0; + if ($cur_dup != $dupe_of && Bugzilla->params->{'commentonduplicate'} + && !$self->{added_comments}) + { + ThrowUserError('comment_required'); + } + + # Should we add the reporter to the CC list of the new bug? + # If he can see the bug... + if ($self->reporter->can_see_bug($dupe_of)) { + my $dupe_of_bug = new Bugzilla::Bug($dupe_of); + # We only add him if he's not the reporter of the other bug. + $self->{_add_dup_cc} = 1 + if $dupe_of_bug->reporter->id != $self->reporter->id; + } + # What if the reporter currently can't see the new bug? In the browser + # interface, we prompt the user. In other interfaces, we default to + # not adding the user, as the safest option. + elsif (Bugzilla->params->usage_mode == USAGE_MODE_BROWSER) { + # If we've already confirmed whether the user should be added... + my $cgi = Bugzilla->cgi; + my $add_confirmed = $cgi->param('confirm_add_duplicate'); + if (defined $add_confirmed) { + $self->{_add_dup_cc} = $add_confirmed; + } + else { + # Note that here we don't check if he user is already the reporter + # of the dupe_of bug, since we already checked if he can *see* + # the bug, above. People might have reporter_accessible turned + # off, but cclist_accessible turned on, so they might want to + # add the reporter even though he's already the reporter of the + # dup_of bug. + my $vars = {}; + my $template = Bugzilla->template; + # Ask the user what they want to do about the reporter. + $vars->{'cclist_accessible'} = $dbh->selectrow_array( + q{SELECT cclist_accessible FROM bugs WHERE bug_id = ?}, + undef, $dupe_of); + $vars->{'original_bug_id'} = $dupe_of; + $vars->{'duplicate_bug_id'} = $self->id; + print $cgi->header(); + $template->process("bug/process/confirm-duplicate.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + exit; + } + } + + return $dupe_of; +} + sub _check_estimated_time { return $_[0]->_check_time($_[1], 'estimated_time'); } @@ -1221,9 +1323,45 @@ sub _check_rep_platform { } sub _check_resolution { - my ($invocant, $resolution) = @_; + my ($self, $resolution) = @_; $resolution = trim($resolution); + + # Throw a special error for resolving bugs without a resolution + # (or trying to change the resolution to '' on a closed bug without + # using clear_resolution). + ThrowUserError('missing_resolution', { status => $self->status->name }) + if !$resolution && !$self->status->is_open; + + # Make sure this is a valid resolution. check_field('resolution', $resolution); + + # The moving code doesn't use set_resolution. This check prevents + # people from hacking the URL variables (or using some other interface) + # and setting a bug to MOVED without moving it. + ThrowCodeError('no_manual_moved') if $resolution eq 'MOVED'; + + # Don't allow open bugs to have resolutions. + ThrowUserError('resolution_not_allowed') if $self->status->is_open; + + # Check noresolveonopenblockers. + if (Bugzilla->params->{"noresolveonopenblockers"} && $resolution eq 'FIXED') + { + my @dependencies = CountOpenDependencies($self->id); + if (@dependencies) { + ThrowUserError("still_unresolved_bugs", + { dependencies => \@dependencies, + dependency_count => scalar @dependencies }); + } + } + + # Check if they're changing the resolution and need to comment. + if (Bugzilla->params->{'commentonchange_resolution'} + && $self->resolution && $resolution ne $self->resolution + && !$self->{added_comments}) + { + ThrowUserError('comment_required'); + } + return $resolution; } @@ -1476,6 +1614,11 @@ sub set_assigned_to { } sub reset_assigned_to { my $self = shift; + if (Bugzilla->params->{'commentonreassignbycomponent'} + && !$self->{added_comments}) + { + ThrowUserError('comment_required'); + } my $comp = $self->component_obj; $self->set_assigned_to($comp->default_assignee); } @@ -1528,6 +1671,38 @@ sub set_dependencies { $self->{'dependson'} = $dependson; $self->{'blocked'} = $blocked; } +sub _clear_dup_id { $_[0]->{dup_id} = undef; } +sub set_dup_id { + my ($self, $dup_id) = @_; + my $old = $self->dup_id || 0; + $self->set('dup_id', $dup_id); + my $new = $self->dup_id || 0; + return if $old == $new; + + # Update the other bug. + my $dupe_of = new Bugzilla::Bug($self->dup_id); + if (delete $self->{_add_dup_cc}) { + $dupe_of->add_cc($self->reporter); + } + $dupe_of->add_comment("", { type => CMT_HAS_DUPE, + extra_data => $self->id }); + $self->{_dup_for_update} = $dupe_of; + + # Now make sure that we add a duplicate comment on *this* bug. + # (Change an existing comment into a dup comment, if there is one, + # or add an empty dup comment.) + if ($self->{added_comments}) { + my @normal = grep { !defined $_->{type} || $_->{type} == CMT_NORMAL } + @{ $self->{added_comments} }; + # Turn the last one into a dup comment. + $normal[-1]->{type} = CMT_DUPE_OF; + $normal[-1]->{extra_data} = $self->dup_id; + } + else { + $self->add_comment('', { type => CMT_DUPE_OF, + extra_data => $self->dup_id }); + } +} sub set_estimated_time { $_[0]->set('estimated_time', $_[1]); } sub _set_everconfirmed { $_[0]->set('everconfirmed', $_[1]); } sub set_op_sys { $_[0]->set('op_sys', $_[1]); } @@ -1665,6 +1840,11 @@ sub set_qa_contact { } sub reset_qa_contact { my $self = shift; + if (Bugzilla->params->{'commentonreassignbycomponent'} + && !$self->{added_comments}) + { + ThrowUserError('comment_required'); + } my $comp = $self->component_obj; $self->set_qa_contact($comp->default_qa_contact); } @@ -1672,14 +1852,73 @@ 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; } sub set_reporter_accessible { $_[0]->set('reporter_accessible', $_[1]); } -sub set_resolution { $_[0]->set('resolution', $_[1]); } -sub clear_resolution { $_[0]->{'resolution'} = '' } +sub set_resolution { + my ($self, $value, $dupe_of) = @_; + + my $old_res = $self->resolution; + $self->set('resolution', $value); + my $new_res = $self->resolution; + + if ($new_res ne $old_res) { + # Clear the dup_id if we're leaving the dup resolution. + if ($old_res eq 'DUPLICATE') { + $self->_clear_dup_id(); + } + # Duplicates should have no remaining time left. + elsif ($new_res eq 'DUPLICATE' && $self->remaining_time != 0) { + $self->_zero_remaining_time(); + } + } + + # We don't check if we're entering or leaving the dup resolution here, + # because we could be moving from being a dup of one bug to being a dup + # of another, theoretically. Note that this code block will also run + # when going between different closed states. + if ($self->resolution eq 'DUPLICATE') { + if ($dupe_of) { + $self->set_dup_id($dupe_of); + } + elsif (!$self->dup_id) { + ThrowUserError('dupe_id_required'); + } + } +} +sub clear_resolution { + my $self = shift; + if (!$self->status->is_open) { + ThrowUserError('resolution_cant_clear', { bug_id => $self->id }); + } + if (Bugzilla->params->{'commentonclearresolution'} + && $self->resolution && !$self->{added_comments}) + { + ThrowUserError('comment_required'); + } + $self->{'resolution'} = ''; + $self->_clear_dup_id; +} sub set_severity { $_[0]->set('bug_severity', $_[1]); } sub set_status { - my ($self, $status) = @_; + my ($self, $status, $resolution, $dupe_of) = @_; + my $old_status = $self->status; $self->set('bug_status', $status); - # Check for the everconfirmed transition - $self->_set_everconfirmed(1) if (is_open_state($status) && $status ne 'UNCONFIRMED'); + delete $self->{'status'}; + my $new_status = $self->status; + + if ($new_status->is_open) { + # Check for the everconfirmed transition + $self->_set_everconfirmed(1) if $new_status->name ne 'UNCONFIRMED'; + $self->clear_resolution(); + } + else { + # We do this here so that we can make sure closed statuses have + # resolutions. + $self->set_resolution($resolution || $self->resolution, $dupe_of); + + # Changing between closed statuses zeros the remaining time. + if ($new_status->id != $old_status->id && $self->remaining_time != 0) { + $self->_zero_remaining_time(); + } + } } sub set_status_whiteboard { $_[0]->set('status_whiteboard', $_[1]); } sub set_summary { $_[0]->set('short_desc', $_[1]); } @@ -2389,217 +2628,29 @@ sub bug_alias_to_id { # Workflow Control routines ##################################################################### -# Make sure that the new status is allowed by the status workflow. -sub check_status_transition { - my ($self, $new_status) = @_; - - if (!grep { $_->name eq $self->bug_status } @{$new_status->can_change_from}) { - ThrowUserError('illegal_bug_status_transition', {old => $self->bug_status, - new => $new_status->name}) - } -} - -# Make sure all checks triggered by the workflow are successful. -# Some are hardcoded and come from older versions of Bugzilla. -sub check_status_change_triggers { - my ($self, $action, $bugs, $vars) = @_; +sub process_knob { + my ($self, $action, $to_resolution, $dupe_of) = @_; my $dbh = Bugzilla->dbh; - $vars ||= {}; - - my @bug_ids = map {$_->id} @$bugs; - # First, make sure no comment is required if there is none. - # If a comment is given, then this check is useless. - if (!$vars->{comment_exists}) { - if (grep { $action eq $_ } SPECIAL_STATUS_WORKFLOW_ACTIONS) { - # 'commentonnone' doesn't exist, so this is safe. - ThrowUserError('comment_required') if Bugzilla->params->{"commenton$action"}; - } - elsif (!scalar @bug_ids) { - # The bug is being created; that's why @bug_ids is undefined. - my $comment_required = - $dbh->selectrow_array('SELECT require_comment - FROM status_workflow - INNER JOIN bug_status - ON id = new_status - WHERE old_status IS NULL - AND value = ?', - undef, $action); - - ThrowUserError('description_required') if $comment_required; - } - else { - my $required_for_transitions = - $dbh->selectcol_arrayref('SELECT DISTINCT bug_status.value - FROM bug_status - INNER JOIN bugs - ON bugs.bug_status = bug_status.value - INNER JOIN status_workflow - ON bug_status.id = old_status - INNER JOIN bug_status b_s - ON b_s.id = new_status - WHERE bug_id IN (' . join (',', @bug_ids). ') - AND b_s.value = ? - AND require_comment = 1', - undef, $action); - - if (scalar(@$required_for_transitions)) { - ThrowUserError('comment_required', {old => $required_for_transitions, - new => $action}); - } - } - } - - # Now run hardcoded checks. - # There is no checks for these actions. - return if ($action eq 'none' || $action eq 'clearresolution'); - - # Also leave now if we are creating a new bug (we only want to check - # if a comment is required on bug creation). - return unless scalar @bug_ids; + return if $action eq 'none'; + if ($action eq 'duplicate') { - # You cannot mark bugs as duplicates when changing - # several bugs at once. - $vars->{bug_id} || ThrowUserError('dupe_not_allowed'); - - # Make sure we can change the original bug (issue A on bug 96085) - $vars->{dup_id} || ThrowCodeError('undefined_field', { field => 'dup_id' }); - ValidateBugID($vars->{dup_id}, 'dup_id'); - - # Make sure a loop isn't created when marking this bug - # as duplicate. - my %dupes; - my $dupe_of = $vars->{dup_id}; - my $sth = $dbh->prepare('SELECT dupe_of FROM duplicates - WHERE dupe = ?'); - - while ($dupe_of) { - if ($dupe_of == $vars->{bug_id}) { - ThrowUserError('dupe_loop_detected', { bug_id => $vars->{bug_id}, - dupe_of => $vars->{dup_id} }); - } - # If $dupes{$dupe_of} is already set to 1, then a loop - # already exists which does not involve this bug. - # As the user is not responsible for this loop, do not - # prevent him from marking this bug as a duplicate. - last if exists $dupes{"$dupe_of"}; - $dupes{"$dupe_of"} = 1; - $sth->execute($dupe_of); - $dupe_of = $sth->fetchrow_array; - } - - # Also, let's see if the reporter has authorization to see - # the bug to which we are duping. If not we need to prompt. - $vars->{DuplicateUserConfirm} = 1; - - # DUPLICATE bugs should have no time remaining. - foreach my $bug (@$bugs) { - # Note that 0.00 is *true* for Perl! - next unless ($bug->remaining_time > 0); - $bug->_zero_remaining_time; - $vars->{'message'} = "remaining_time_zeroed" - if Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'}); - } - } - elsif ($action eq 'change_resolution' || !is_open_state($action)) { - # don't resolve as fixed while still unresolved blocking bugs - if (Bugzilla->params->{"noresolveonopenblockers"} - && $vars->{resolution} eq 'FIXED') - { - my @dependencies = Bugzilla::Bug::CountOpenDependencies(@bug_ids); - if (scalar @dependencies > 0) { - ThrowUserError("still_unresolved_bugs", - { dependencies => \@dependencies, - dependency_count => scalar @dependencies }); - } - } - - # You cannot use change_resolution if there is at least one open bug - # nor can you close open bugs if no resolution is given. - my $open_states = join(',', map {$dbh->quote($_)} BUG_STATE_OPEN); - my $idlist = join(',', @bug_ids); - my $is_open = - $dbh->selectrow_array("SELECT 1 FROM bugs WHERE bug_id IN ($idlist) - AND bug_status IN ($open_states)"); - - if ($is_open) { - ThrowUserError('resolution_not_allowed') if ($action eq 'change_resolution'); - ThrowUserError('missing_resolution', {status => $action}) if !$vars->{resolution}; - } - # Now is good time to validate the resolution, if any. - check_field('resolution', $vars->{resolution}, - Bugzilla::Bug->settable_resolutions) if $vars->{resolution}; - - if ($action ne 'change_resolution') { - foreach my $b (@$bugs) { - if ($b->bug_status ne $action) { - # Note that 0.00 is *true* for Perl! - next unless ($b->remaining_time > 0); - $b->_zero_remaining_time; - $vars->{'message'} = "remaining_time_zeroed" - if Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'}); - } - } - } + $self->set_status(Bugzilla->params->{'duplicate_or_move_bug_status'}, + 'DUPLICATE', $dupe_of); } - elsif ($action eq 'ASSIGNED' - && Bugzilla->params->{"usetargetmilestone"} - && Bugzilla->params->{"musthavemilestoneonaccept"}) - { - $vars->{requiremilestone} = 1; - } -} - -sub get_new_status_and_resolution { - my ($self, $action, $resolution) = @_; - my $dbh = Bugzilla->dbh; - - my $status; - my $everconfirmed = $self->everconfirmed; - if ($action eq 'none') { - # Leaving the status unchanged doesn't need more investigation. - return ($self->bug_status, $self->resolution, $self->everconfirmed); - } - elsif ($action eq 'duplicate' || $action eq 'move') { - # Always change the bug status, even if the bug was already "closed". - $status = Bugzilla->params->{'duplicate_or_move_bug_status'}; - $resolution = ($action eq 'duplicate') ? 'DUPLICATE' : 'MOVED'; + elsif ($action eq 'move') { + $self->set_status(Bugzilla->params->{'duplicate_or_move_bug_status'}, + 'MOVED'); } elsif ($action eq 'change_resolution') { - $status = $self->bug_status; - # You cannot change the resolution of an open bug. - ThrowUserError('resolution_not_allowed') if is_open_state($status); - $resolution || ThrowUserError('missing_resolution', {status => $status}); + $self->set_resolution($to_resolution); } elsif ($action eq 'clearresolution') { - $status = $self->bug_status; - is_open_state($status) || ThrowUserError('missing_resolution', {status => $status}); - $resolution = ''; + $self->clear_resolution(); } else { - $status = $action; - if (is_open_state($status)) { - # Open bugs have no resolution. - $resolution = ''; - $everconfirmed = ($status eq 'UNCONFIRMED') ? 0 : 1; - } - elsif (is_open_state($self->bug_status)) { - # A resolution is required to close bugs. - $resolution || ThrowUserError('missing_resolution', {status => $status}); - } - else { - # Already closed bugs can only change their resolution - # using the change_resolution action. - $resolution = $self->resolution - } + $self->set_status($action, $to_resolution); } - # Now it's time to validate the bug resolution. - # Bug resolutions have no workflow specific rules, so any valid - # resolution will do it. - check_field('resolution', $resolution) if ($resolution ne ''); - trick_taint($resolution); - - return ($status, $resolution, $everconfirmed); } ##################################################################### diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm index 65baf04b8..5573fa836 100644 --- a/Bugzilla/Status.pm +++ b/Bugzilla/Status.pm @@ -106,6 +106,24 @@ sub can_change_to { return $self->{'can_change_to'}; } +sub allow_change_from { + my ($self, $old_status, $product) = @_; + + # Always allow transitions from a status to itself. + return 1 if ($old_status && $old_status->id == $self->id); + + if ($self->name eq 'UNCONFIRMED' && !$product->votes_to_confirm) { + # UNCONFIRMED is an invalid status transition if votes_to_confirm is 0 + # in this product. + return 0; + } + + my ($cond, $values) = $self->_status_condition($old_status); + my ($transition_allowed) = Bugzilla->dbh->selectrow_array( + "SELECT 1 FROM status_workflow WHERE $cond", undef, @$values); + return $transition_allowed ? 1 : 0; +} + sub can_change_from { my $self = shift; my $dbh = Bugzilla->dbh; @@ -128,6 +146,32 @@ sub can_change_from { return $self->{'can_change_from'}; } +sub comment_required_on_change_from { + my ($self, $old_status) = @_; + my ($cond, $values) = $self->_status_condition($old_status); + + my ($require_comment) = Bugzilla->dbh->selectrow_array( + "SELECT require_comment FROM status_workflow + WHERE $cond", undef, @$values); + return $require_comment; +} + +# Used as a helper for various functions that have to deal with old_status +# sometimes being NULL and sometimes having a value. +sub _status_condition { + my ($self, $old_status) = @_; + my @values; + my $cond = 'old_status IS NULL'; + # For newly-filed bugs + if ($old_status) { + $cond = 'old_status = ?'; + push(@values, $old_status->id); + } + $cond .= " AND new_status = ?"; + push(@values, $self->id); + return ($cond, \@values); +} + sub add_missing_bug_status_transitions { my $bug_status = shift || Bugzilla->params->{'duplicate_or_move_bug_status'}; my $dbh = Bugzilla->dbh; @@ -215,6 +259,60 @@ below. Returns: A list of Bugzilla::Status objects. +=item C<allow_change_from> + +=over + +=item B<Description> + +Tells you whether or not a change to this status from another status is +allowed. + +=item B<Params> + +=over + +=item C<$old_status> - The Bugzilla::Status you're changing from. + +=item C<$product> - A L<Bugzilla::Product> representing the product of +the bug you're changing. Needed to check product-specific workflow +issues (such as whether or not the C<UNCONFIRMED> status is enabled +in this product). + +=back + +=item B<Returns> + +C<1> if you are allowed to change to this status from that status, or +C<0> if you aren't allowed. + +Note that changing from a status to itself is always allowed. + +=back + +=item C<comment_required_on_change_from> + +=over + +=item B<Description> + +Checks if a comment is required to change to this status from another +status, according to the current settings in the workflow. + +Note that this doesn't implement the checks enforced by the various +C<commenton> parameters--those are checked by internal checks in +L<Bugzilla::Bug>. + +=item B<Params> + +C<$old_status> - The status you're changing from. + +=item B<Returns> + +C<1> if a comment is required on this change, C<0> if not. + +=back + =item C<add_missing_bug_status_transitions> Description: Insert all missing transitions to a given bug status. diff --git a/process_bug.cgi b/process_bug.cgi index dcea4ffba..71be09168 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -75,7 +75,6 @@ $vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); my @editable_bug_fields = editable_bug_fields(); -my $requiremilestone = 0; local our $PrivilegesRequired = 0; ###################################################################### @@ -268,48 +267,9 @@ if (should_set('product')) { } } -# Confirm that the reporter of the current bug can access the bug we are duping to. -sub DuplicateUserConfirm { - my ($dupe, $original) = @_; - my $cgi = Bugzilla->cgi; - my $dbh = Bugzilla->dbh; - my $template = Bugzilla->template; - - # if we've already been through here, then exit - if (defined $cgi->param('confirm_add_duplicate')) { - return; - } - - if ($dupe->reporter->can_see_bug($original)) { - $cgi->param('confirm_add_duplicate', '1'); - return; - } - elsif (Bugzilla->usage_mode == USAGE_MODE_EMAIL) { - # The email interface defaults to the safe alternative, which is - # not CC'ing the user. - $cgi->param('confirm_add_duplicate', 0); - return; - } - - $vars->{'cclist_accessible'} = $dbh->selectrow_array( - q{SELECT cclist_accessible FROM bugs WHERE bug_id = ?}, - undef, $original); - - # Once in this part of the subroutine, the user has not been auto-validated - # and the duper has not chosen whether or not to add to CC list, so let's - # ask the duper what he/she wants to do. - - $vars->{'original_bug_id'} = $original; - $vars->{'duplicate_bug_id'} = $dupe->bug_id; - - # Confirm whether or not to add the reporter to the cc: list - # of the original bug (the one this bug is being duped against). - print Bugzilla->cgi->header(); - $template->process("bug/process/confirm-duplicate.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - exit; -} - +# 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. my @set_fields = qw(op_sys rep_platform priority bug_severity component target_milestone version bug_file_loc status_whiteboard short_desc @@ -324,9 +284,13 @@ 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. + 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}; @@ -480,7 +444,13 @@ if ($action eq Bugzilla->params->{'move-button-text'}) { foreach my $bug (@bug_objects) { my ($status, $resolution) = $bug->get_new_status_and_resolution('move'); $bug->set_status($status); - $bug->set_resolution($resolution); + # We don't use set_resolution here because the MOVED resolution is + # special and is normally rejected by set_resolution. + $bug->{resolution} = $resolution; + # That means that we need to clear dups manually. Eventually this + # bug-moving code will all be inside Bugzilla::Bug, so it's OK + # to call an internal function here. + $bug->_clear_dup_id; } $_->update() foreach @bug_objects; $dbh->bz_unlock_tables(); @@ -527,56 +497,34 @@ if ($action eq Bugzilla->params->{'move-button-text'}) { } -if (($cgi->param('set_default_assignee') || $cgi->param('set_default_qa_contact')) - && Bugzilla->params->{'commentonreassignbycomponent'} && !comment_exists()) -{ - ThrowUserError('comment_required'); +# 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'); } -my $duplicate; # It will store the ID of the bug we are pointing to, if any. - -# Make sure the bug status transition is legal for all bugs. -my $knob = scalar $cgi->param('knob'); -# Special actions (duplicate, change_resolution and clearresolution) are outside -# the workflow. -if (!grep { $knob eq $_ } SPECIAL_STATUS_WORKFLOW_ACTIONS) { - # Make sure the bug status exists and is active. - check_field('bug_status', $knob); - my $bug_status = new Bugzilla::Status({name => $knob}); - $_->check_status_transition($bug_status) foreach @bug_objects; - - # Fill the resolution field with the correct value (e.g. in case the - # workflow allows several open -> closed transitions). - if ($bug_status->is_open) { - $cgi->delete('resolution'); - } - else { - $cgi->param('resolution', $cgi->param('resolution_knob_' . $bug_status->id)); +# 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. + my $knob = $cgi->param('knob'); + my $status = new Bugzilla::Status({name => $knob}); + my $resolution; + if ($status) { + $resolution = $cgi->param('resolution_knob_' . $status->id); + } + 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')); } } -elsif ($knob eq 'change_resolution') { - # Fill the resolution field with the correct value. - $cgi->param('resolution', $cgi->param('resolution_knob_change_resolution')); -} -else { - # The resolution field is not in use. - $cgi->delete('resolution'); -} - -# The action is a valid one. -trick_taint($knob); -# Some information is required for checks. -$vars->{comment_exists} = comment_exists(); -$vars->{bug_id} = $cgi->param('id'); -$vars->{dup_id} = $cgi->param('dup_id'); -$vars->{resolution} = $cgi->param('resolution') || ''; -Bugzilla::Bug->check_status_change_triggers($knob, \@bug_objects, $vars); - -# Some triggers require extra actions. -$duplicate = $vars->{dup_id} if ($knob eq 'duplicate'); -$requiremilestone = $vars->{requiremilestone}; -# $vars->{DuplicateUserConfirm} is true only if a single bug is being edited. -DuplicateUserConfirm($bug, $duplicate) if $vars->{DuplicateUserConfirm}; my $any_keyword_changes; if (defined $cgi->param('keywords')) { @@ -627,32 +575,6 @@ foreach my $id (@idlist) { my $comma = $::comma; my $old_bug_obj = new Bugzilla::Bug($id); - my ($status, $everconfirmed); - my $resolution = $old_bug_obj->resolution; - # We only care about the resolution field if the user explicitly edits it - # or if he closes the bug. - if ($knob eq 'change_resolution' || $cgi->param('resolution')) { - $resolution = $cgi->param('resolution'); - } - ($status, $resolution, $everconfirmed) = - $old_bug_obj->get_new_status_and_resolution($knob, $resolution); - - if ($status ne $old_bug_obj->bug_status) { - $query .= "$comma bug_status = ?"; - push(@bug_values, $status); - $comma = ','; - } - if ($resolution ne $old_bug_obj->resolution) { - $query .= "$comma resolution = ?"; - push(@bug_values, $resolution); - $comma = ','; - } - if ($everconfirmed ne $old_bug_obj->everconfirmed) { - $query .= "$comma everconfirmed = ?"; - push(@bug_values, $everconfirmed); - $comma = ','; - } - my $bug_changed = 0; my $write = "WRITE"; # Might want to make a param to control # whether we do LOW_PRIORITY ... @@ -695,9 +617,6 @@ foreach my $id (@idlist) { $formhash{$col} = $cgi->param($col) if defined $cgi->param($col); $i++; } - # The status and resolution are defined by the workflow. - $formhash{'bug_status'} = $status; - $formhash{'resolution'} = $resolution; # This hash is required by Bug::check_can_change_field(). my $cgi_hash = {'dontchange' => scalar $cgi->param('dontchange')}; @@ -731,15 +650,7 @@ foreach my $id (@idlist) { } 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'}) = @@ -763,23 +674,32 @@ foreach my $id (@idlist) { exit; } - if ($cgi->param('comment') || $cgi->param('work_time') || $duplicate) { - my $type = $duplicate ? CMT_DUPE_OF : CMT_NORMAL; - - $bug_objects{$id}->add_comment(scalar($cgi->param('comment')), - { isprivate => scalar($cgi->param('commentprivacy')), - work_time => scalar $cgi->param('work_time'), type => $type, - extra_data => $duplicate}); - $bug_changed = 1; - } - + ################################# # Start Actual Database Updates # ################################# $timestamp = $dbh->selectrow_array(q{SELECT NOW()}); - $bug_objects{$id}->update($timestamp); + my $changes = $bug_objects{$id}->update($timestamp); + + my %notify_deps; + if ($changes->{'bug_status'}) { + my ($old_status, $new_status) = @{ $changes->{'bug_status'} }; + + # If this bug has changed from opened to closed or vice-versa, + # then all of the bugs we block need to be notified. + if (is_open_state($old_status) ne is_open_state($new_status)) { + $notify_deps{$_} = 1 foreach (@{$bug_objects{$id}->blocked}); + } + + # We may have zeroed the remaining time, if we moved into a closed + # status, so we should inform the user about that. + if (!is_open_state($new_status) && $changes->{'remaining_time'}) { + $vars->{'message'} = "remaining_time_zeroed" + if Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'}); + } + } $bug_objects{$id}->update_keywords($timestamp); @@ -790,11 +710,6 @@ foreach my $id (@idlist) { $dbh->do($query, undef, @bug_values); } - # Check for duplicates if the bug is [re]open or its resolution is changed. - if ($resolution ne 'DUPLICATE') { - $dbh->do(q{DELETE FROM duplicates WHERE dupe = ?}, undef, $id); - } - my ($cc_removed) = $bug_objects{$id}->update_cc($timestamp); $cc_removed = [map {$_->login} @$cc_removed]; @@ -828,7 +743,6 @@ foreach my $id (@idlist) { # $msgs will store emails which have to be sent to voters, if any. my $msgs; - my %notify_deps; foreach my $c (@editable_bug_fields) { my $col = $c; # We modify it, don't want to modify array @@ -844,6 +758,7 @@ foreach my $id (@idlist) { bug_severity short_desc alias deadline estimated_time remaining_time reporter_accessible cclist_accessible + bug_status resolution status_whiteboard bug_file_loc), Bugzilla->custom_field_names); @@ -857,14 +772,6 @@ foreach my $id (@idlist) { CheckIfVotedConfirmed($id, $whoid); } - # If this bug has changed from opened to closed or vice-versa, - # then all of the bugs we block need to be notified. - if ($col eq 'bug_status' - && is_open_state($old) ne is_open_state($new)) - { - $notify_deps{$_} = 1 foreach (@{$bug_objects{$id}->blocked}); - } - LogActivityEntry($id,$col,$old,$new,$whoid,$timestamp); $bug_changed = 1; } @@ -883,35 +790,6 @@ foreach my $id (@idlist) { MessageToMTA($msg); } - if ($duplicate) { - # If the bug was already marked as a duplicate, remove - # the existing entry. - $dbh->do('DELETE FROM duplicates WHERE dupe = ?', - undef, $cgi->param('id')); - - my $dup = new Bugzilla::Bug($duplicate); - my $reporter = $new_bug_obj->reporter; - my $isoncc = $dbh->selectrow_array(q{SELECT who FROM cc - WHERE bug_id = ? AND who = ?}, - undef, $duplicate, $reporter->id); - unless (($reporter->id == $dup->reporter->id) || $isoncc - || !$cgi->param('confirm_add_duplicate')) { - # The reporter is oblivious to the existence of the original bug - # and is permitted access. Add him to the cc (and record activity). - LogActivityEntry($duplicate,"cc","",$reporter->name, - $whoid,$timestamp); - $dbh->do(q{INSERT INTO cc (who, bug_id) VALUES (?, ?)}, - undef, $reporter->id, $duplicate); - } - # Bug 171639 - Duplicate notifications do not need to be private. - $dup->add_comment("", { type => CMT_HAS_DUPE, - extra_data => $new_bug_obj->bug_id }); - $dup->update($timestamp); - - $dbh->do(q{INSERT INTO duplicates VALUES (?, ?)}, undef, - $duplicate, $cgi->param('id')); - } - # Now all changes to the DB have been made. It's time to email # all concerned users, including the bug itself, but also the # duplicated bug and dependent bugs, if any. @@ -930,15 +808,18 @@ foreach my $id (@idlist) { # receive email about the change. send_results($id, $vars); - if ($duplicate) { + # If the bug was marked as a duplicate, we need to notify users on the + # other bug of any changes to that bug. + my $new_dup_id = $changes->{'dup_id'} ? $changes->{'dup_id'}->[1] : undef; + if ($new_dup_id) { $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login }; - $vars->{'id'} = $duplicate; + $vars->{'id'} = $new_dup_id; $vars->{'type'} = "dupe"; # Let the user know a duplication notation was added to the # original bug. - send_results($duplicate, $vars); + send_results($new_dup_id, $vars); } my %all_dep_changes = (%notify_deps, %changed_deps); diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index dce7947d6..1e5581e13 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -309,6 +309,10 @@ [% ELSIF error == "need_quipid" %] A valid quipid is needed. + [% ELSIF error == "no_manual_moved" %] + You cannot set the resolution of [% terms.abug %] to MOVED without + moving the [% terms.bug %]. + [% ELSIF error == "param_must_be_numeric" %] [% title = "Invalid Parameter" %] Invalid parameter passed to [% function FILTER html %]. diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 7bce4ad98..ca9bf0cdb 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -252,14 +252,15 @@ [% ELSIF error == "comment_required" %] [% title = "Comment Required" %] - You have to specify a <b>comment</b> - [% IF old.size && new %] - to change the [% terms.bug %] status from [% old.join(", ") FILTER html %] - to [% new FILTER html %]. + You have to specify a + [% IF old && new %] + <b>comment</b> when changing the status of [% terms.abug %] from + [%+ old.name FILTER html %] to [% new.name FILTER html %]. + [% ELSIF new %] + description for this [% terms.bug %]. [% ELSE %] - on this change. + <b>comment</b> on this change. [% END %] - Please explain your change. [% ELSIF error == "comment_too_long" %] [% title = "Comment Too Long" %] @@ -346,9 +347,10 @@ [% title = "Dependency Loop Detected" %] You can't make [% terms.abug %] block itself or depend on itself. - [% ELSIF error == "description_required" %] - [% title = "Description Required" %] - You must provide a description of the [% terms.bug %]. + [% ELSIF error == "dupe_id_required" %] + [% title = "Duplicate Bug Id Required" %] + You must specify [% terms.abug %] id to mark this [% terms.bug %] + as a duplicate of. [% ELSIF error == "dupe_not_allowed" %] [% title = "Cannot mark $terms.bugs as duplicates" %] @@ -679,8 +681,13 @@ [% ELSIF error == "illegal_bug_status_transition" %] [% title = "Illegal $terms.Bug Status Change" %] - You are not allowed to change the [% terms.bug %] status from - [%+ old FILTER html %] to [% new FILTER html %]. + [% IF old.defined %] + You are not allowed to change the [% terms.bug %] status from + [%+ old.name FILTER html %] to [% new.name FILTER html %]. + [% ELSE %] + You are not allowed to file new [% terms.bugs %] with the + [%+ new.name FILTER html %] status. + [% END %] [% ELSIF error == "illegal_change" %] [% title = "Not allowed" %] @@ -940,7 +947,7 @@ [% ELSIF error == "milestone_required" %] [% title = "Milestone Required" %] You must determine a target milestone for [% terms.bug %] - [%+ bug_id FILTER html %] + [%+ bug.id FILTER html %] if you are going to accept it. Part of accepting [%+ terms.abug %] is giving an estimate of when it will be fixed. @@ -1379,6 +1386,10 @@ [% title = "Summary Needed" %] You must enter a summary for this [% terms.bug %]. + [% ELSIF error == "resolution_cant_clear" %] + [% terms.Bug %] [% bug_id FILTER bug_link(bug_id) FILTER none %] is + closed, so you cannot clear its resolution. + [% ELSIF error == "resolution_not_allowed" %] [% title = "Resolution Not Allowed" %] You cannot set a resolution for open [% terms.bugs %]. @@ -1659,5 +1670,7 @@ version [% ELSIF class == "Bugzilla::Milestone" %] milestone + [% ELSIF class == "Bugzilla::Status" %] + status [% END %] [% END %] |