summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rwxr-xr-xBugzilla/Bug.pm84
-rwxr-xr-xprocess_bug.cgi177
-rw-r--r--template/en/default/global/user-error.html.tmpl5
3 files changed, 122 insertions, 144 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index daebaf701..777fca323 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1495,6 +1495,90 @@ sub bug_alias_to_id {
}
#####################################################################
+# Workflow Control routines
+#####################################################################
+
+sub get_new_status_and_resolution {
+ my ($self, $action, $resolution) = @_;
+ my $dbh = Bugzilla->dbh;
+
+ my $status;
+ if ($action eq 'none') {
+ # Leaving the status unchanged doesn't need more investigation.
+ return ($self->bug_status, $self->resolution);
+ }
+ elsif ($action eq 'reopen') {
+ $status = $self->everconfirmed ? 'REOPENED' : 'UNCONFIRMED';
+ $resolution = '';
+ }
+ elsif ($action =~ /^reassign(?:bycomponent)?$/) {
+ if (!is_open_state($self->bug_status) || $self->bug_status eq 'UNCONFIRMED') {
+ $status = $self->bug_status;
+ }
+ else {
+ $status = 'NEW';
+ }
+ $resolution = $self->resolution;
+ }
+ elsif ($action eq 'duplicate') {
+ # Only alter the bug status if the bug is currently open.
+ $status = is_open_state($self->bug_status) ? 'RESOLVED' : $self->bug_status;
+ $resolution = 'DUPLICATE';
+ }
+ 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});
+ }
+ elsif ($action eq 'clearresolution') {
+ $status = $self->bug_status;
+ is_open_state($status) || ThrowUserError('missing_resolution', {status => $status});
+ $resolution = '';
+ }
+ else {
+ # That's where actions not requiring any specific trigger (such as future
+ # custom statuses) come.
+ # XXX - This is hardcoded here for now, but will disappear soon when
+ # this routine will look at the DB directly to get the workflow.
+ if ($action eq 'confirm') {
+ $status = 'NEW';
+ }
+ elsif ($action eq 'accept') {
+ $status = 'ASSIGNED';
+ }
+ elsif ($action eq 'resolve') {
+ $status = 'RESOLVED';
+ }
+ elsif ($action eq 'verify') {
+ $status = 'VERIFIED';
+ }
+ elsif ($action eq 'close') {
+ $status = 'CLOSED';
+ }
+ else {
+ ThrowCodeError('unknown_action', { action => $action });
+ }
+
+ if (is_open_state($status)) {
+ # Open bugs have no resolution.
+ $resolution = '';
+ }
+ else {
+ # All non-open statuses must have a resolution.
+ $resolution || ThrowUserError('missing_resolution', {status => $status});
+ }
+ }
+ # 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);
+}
+
+#####################################################################
# Subroutines
#####################################################################
diff --git a/process_bug.cgi b/process_bug.cgi
index 6128bcf65..74468ada9 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -643,9 +643,6 @@ sub DoComma {
$::comma = ",";
}
-# $everconfirmed is used by ChangeStatus() to determine whether we are
-# confirming the bug or not.
-local our $everconfirmed;
sub DoConfirm {
my $bug = shift;
if ($bug->check_can_change_field("canconfirm", 0, 1,
@@ -653,111 +650,6 @@ sub DoConfirm {
{
DoComma();
$::query .= "everconfirmed = 1";
- $everconfirmed = 1;
- }
-}
-
-sub ChangeStatus {
- my ($str) = (@_);
- my $cgi = Bugzilla->cgi;
- my $dbh = Bugzilla->dbh;
-
- if (!$cgi->param('dontchange')
- || $str ne $cgi->param('dontchange')) {
- DoComma();
- if ($cgi->param('knob') eq 'reopen') {
- # When reopening, we need to check whether the bug was ever
- # confirmed or not
- $::query .= "bug_status = CASE WHEN everconfirmed = 1 THEN " .
- $dbh->quote($str) . " ELSE 'UNCONFIRMED' END";
- } elsif (is_open_state($str)) {
- # Note that we cannot combine this with the above branch - here we
- # need to check if bugs.bug_status is open, (since we don't want to
- # reopen closed bugs when reassigning), while above the whole point
- # is to reopen a closed bug.
- # Currently, the UI doesn't permit a user to reassign a closed bug
- # from the single bug page (only during a mass change), but they
- # could still hack the submit, so don't restrict this extended
- # check to the mass change page for safety/sanity/consistency
- # purposes.
-
- # The logic for this block is:
- # If the new state is open:
- # If the old state was open
- # If the bug was confirmed
- # - move it to the new state
- # Else
- # - Set the state to unconfirmed
- # Else
- # - leave it as it was
-
- # This is valid only because 'reopen' is the only thing which moves
- # from closed to open, and it's handled above
- # This also relies on the fact that confirming and accepting have
- # already called DoConfirm before this is called
-
- my @open_state = map($dbh->quote($_), BUG_STATE_OPEN);
- my $open_state = join(", ", @open_state);
-
- # If we are changing everconfirmed to 1, we have to take this change
- # into account and the new bug status is given by $str.
- my $cond = $dbh->quote($str);
- # If we are not setting everconfirmed, the new bug status depends on
- # the actual value of everconfirmed, which is bug-specific.
- unless ($everconfirmed) {
- $cond = "(CASE WHEN everconfirmed = 1 THEN " . $cond .
- " ELSE 'UNCONFIRMED' END)";
- }
- $::query .= "bug_status = CASE WHEN bug_status IN($open_state) THEN " .
- $cond . " ELSE bug_status END";
- } else {
- $::query .= "bug_status = ?";
- push(@values, $str);
- }
- # If bugs are reassigned and their status is "UNCONFIRMED", they
- # should keep this status instead of "NEW" as suggested here.
- # This point is checked for each bug later in the code.
- $cgi->param('bug_status', $str);
- }
-}
-
-sub ChangeResolution {
- my ($bug, $str) = (@_);
- my $dbh = Bugzilla->dbh;
- my $cgi = Bugzilla->cgi;
-
- if (!$cgi->param('dontchange')
- || $str ne $cgi->param('dontchange'))
- {
- # Make sure the user is allowed to change the resolution.
- # If the user is changing several bugs at once using the UI,
- # then he has enough privs to do so. In the case he is hacking
- # the URL, we don't care if he reads --UNKNOWN-- as a resolution
- # in the error message.
- my $old_resolution = '-- UNKNOWN --';
- my $bug_id = $cgi->param('id');
- if ($bug_id) {
- $old_resolution =
- $dbh->selectrow_array('SELECT resolution FROM bugs WHERE bug_id = ?',
- undef, $bug_id);
- }
- unless ($bug->check_can_change_field('resolution', $old_resolution, $str,
- \$PrivilegesRequired))
- {
- $vars->{'oldvalue'} = $old_resolution;
- $vars->{'newvalue'} = $str;
- $vars->{'field'} = 'resolution';
- $vars->{'privs'} = $PrivilegesRequired;
- ThrowUserError("illegal_change", $vars);
- }
-
- DoComma();
- $::query .= "resolution = ?";
- trick_taint($str);
- push(@values, $str);
- # We define this variable here so that customized installations
- # may set rules based on the resolution in Bug::check_can_change_field().
- $cgi->param('resolution', $str);
}
}
@@ -1042,13 +934,11 @@ SWITCH: for ($cgi->param('knob')) {
};
/^confirm$/ && CheckonComment( "confirm" ) && do {
DoConfirm($bug);
- ChangeStatus('NEW');
last SWITCH;
};
/^accept$/ && CheckonComment( "accept" ) && do {
DoConfirm($bug);
- ChangeStatus('ASSIGNED');
- if (Bugzilla->params->{"usetargetmilestone"}
+ if (Bugzilla->params->{"usetargetmilestone"}
&& Bugzilla->params->{"musthavemilestoneonaccept"})
{
$requiremilestone = 1;
@@ -1056,7 +946,6 @@ SWITCH: for ($cgi->param('knob')) {
last SWITCH;
};
/^clearresolution$/ && CheckonComment( "clearresolution" ) && do {
- ChangeResolution($bug, '');
last SWITCH;
};
/^(resolve|change_resolution)$/ && CheckonComment( "resolve" ) && do {
@@ -1080,8 +969,6 @@ SWITCH: for ($cgi->param('knob')) {
# RESOLVED bugs should have no time remaining;
# more time can be added for the VERIFY step, if needed.
_remove_remaining_time();
-
- ChangeStatus('RESOLVED');
}
else {
# You cannot use change_resolution if there is at least
@@ -1094,16 +981,12 @@ SWITCH: for ($cgi->param('knob')) {
ThrowUserError('resolution_not_allowed') if $is_open;
}
-
- ChangeResolution($bug, $cgi->param('resolution'));
last SWITCH;
};
/^reassign$/ && CheckonComment( "reassign" ) && do {
if ($cgi->param('andconfirm')) {
DoConfirm($bug);
}
- ChangeStatus('NEW');
- DoComma();
if (defined $cgi->param('assigned_to')
&& trim($cgi->param('assigned_to')) ne "") {
$assignee = login_to_id(trim($cgi->param('assigned_to')), THROW_ERROR);
@@ -1125,6 +1008,7 @@ SWITCH: for ($cgi->param('knob')) {
} else {
ThrowUserError("reassign_to_empty");
}
+ DoComma();
$::query .= "assigned_to = ?";
push(@values, $assignee);
$assignee_checked = 1;
@@ -1134,23 +1018,17 @@ SWITCH: for ($cgi->param('knob')) {
if ($cgi->param('compconfirm')) {
DoConfirm($bug);
}
- ChangeStatus('NEW');
last SWITCH;
};
/^reopen$/ && CheckonComment( "reopen" ) && do {
- ChangeStatus('REOPENED');
- ChangeResolution($bug, '');
last SWITCH;
};
/^verify$/ && CheckonComment( "verify" ) && do {
- ChangeStatus('VERIFIED');
last SWITCH;
};
/^close$/ && CheckonComment( "close" ) && do {
# CLOSED bugs should have no time remaining.
_remove_remaining_time();
-
- ChangeStatus('CLOSED');
last SWITCH;
};
/^duplicate$/ && CheckonComment( "duplicate" ) && do {
@@ -1196,9 +1074,6 @@ SWITCH: for ($cgi->param('knob')) {
# DUPLICATE bugs should have no time remaining.
_remove_remaining_time();
-
- ChangeStatus('RESOLVED');
- ChangeResolution($bug, 'DUPLICATE');
last SWITCH;
};
@@ -1391,8 +1266,30 @@ if ($prod_changed && Bugzilla->params->{"strict_isolation"}) {
foreach my $id (@idlist) {
my $query = $basequery;
my @bug_values = @values;
+ # XXX We really have to get rid of $::comma.
+ my $comma = $::comma;
my $old_bug_obj = new Bugzilla::Bug($id);
+ my $status;
+ my $resolution = $old_bug_obj->resolution;
+ # These are the only actions where we care about the resolution field.
+ if ($cgi->param('knob') =~ /^(?:resolve|change_resolution)$/) {
+ $resolution = $cgi->param('resolution');
+ }
+ ($status, $resolution) =
+ $old_bug_obj->get_new_status_and_resolution(scalar $cgi->param('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 ($cgi->param('knob') eq 'reassignbycomponent') {
# We have to check whether the bug is moved to another product
# and/or component before reassigning. If $component is defined,
@@ -1402,24 +1299,23 @@ foreach my $id (@idlist) {
FROM components
WHERE components.id = ?',
undef, $new_comp_id);
- $query .= ", assigned_to = ?";
+ $query .= "$comma assigned_to = ?";
push(@bug_values, $assignee);
+ $comma = ',';
if (Bugzilla->params->{"useqacontact"}) {
$qacontact = $dbh->selectrow_array('SELECT initialqacontact
FROM components
WHERE components.id = ?',
undef, $new_comp_id);
if ($qacontact) {
- $query .= ", qa_contact = ?";
+ $query .= "$comma qa_contact = ?";
push(@bug_values, $qacontact);
}
else {
- $query .= ", qa_contact = NULL";
+ $query .= "$comma qa_contact = NULL";
}
}
-
-
# And add in the Default CC for the Component.
my $comp_obj = $component || new Bugzilla::Component($new_comp_id);
my @new_init_cc = @{$comp_obj->initial_cc};
@@ -1466,20 +1362,18 @@ foreach my $id (@idlist) {
}
$oldhash{$col} = $oldvalues[$i];
$formhash{$col} = $cgi->param($col) if defined $cgi->param($col);
+ # The status and resolution are defined by the workflow.
+ $formhash{$col} = $status if $col eq 'bug_status';
+ $formhash{$col} = $resolution if $col eq 'resolution';
$i++;
}
# If the user is reassigning bugs, we need to:
# - convert $newhash{'assigned_to'} and $newhash{'qa_contact'}
# email addresses into their corresponding IDs;
- # - update $newhash{'bug_status'} to its real state if the bug
- # is in the unconfirmed state.
$formhash{'qa_contact'} = $qacontact if Bugzilla->params->{'useqacontact'};
if ($cgi->param('knob') eq 'reassignbycomponent'
|| $cgi->param('knob') eq 'reassign') {
$formhash{'assigned_to'} = $assignee;
- if ($oldhash{'bug_status'} eq 'UNCONFIRMED') {
- $formhash{'bug_status'} = $oldhash{'bug_status'};
- }
}
# This hash is required by Bug::check_can_change_field().
my $cgi_hash = {
@@ -1487,9 +1381,6 @@ foreach my $id (@idlist) {
'knob' => scalar $cgi->param('knob')
};
foreach my $col (@editable_bug_fields) {
- # The 'resolution' field is checked by ChangeResolution(),
- # i.e. only if we effectively use it.
- next if ($col eq 'resolution');
if (exists $formhash{$col}
&& !$old_bug_obj->check_can_change_field($col, $oldhash{$col}, $formhash{$col},
\$PrivilegesRequired, $cgi_hash))
@@ -1665,14 +1556,12 @@ foreach my $id (@idlist) {
}
$query .= " WHERE bug_id = ?";
push(@bug_values, $id);
-
- if ($::comma ne "") {
+
+ if ($comma ne '') {
$dbh->do($query, undef, @bug_values);
}
# Check for duplicates if the bug is [re]open or its resolution is changed.
- my $resolution = $dbh->selectrow_array(
- q{SELECT resolution FROM bugs WHERE bug_id = ?}, undef, $id);
if ($resolution ne 'DUPLICATE') {
$dbh->do(q{DELETE FROM duplicates WHERE dupe = ?}, undef, $id);
}
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 171eb9c20..4fa138206 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -977,6 +977,11 @@
does not exist.
[% END %]
+ [% ELSIF error == "missing_resolution" %]
+ [% title = "Resolution Required" %]
+ A valid resolution is required to mark [% terms.bugs %] as
+ [%+ status FILTER upper FILTER html %].
+
[% ELSIF error == "move_bugs_disabled" %]
[% title = BLOCK %][% terms.Bug %] Moving Disabled[% END %]
Sorry, [% terms.bug %] moving has been disabled. If you need
generated by cgit v1.2.3-24-g4f1b (git 2.32.0) at 2024-11-25 22:08:25 +0100