Bug 389835: Various issues when changing several bugs at once - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=mkanat

6 files changed, 73 insertions, 34 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index de1884b7f..4793c87da 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1899,30 +1899,13 @@ sub bug_alias_to_id {
 # Workflow Control routines
 #####################################################################
 
-# Make sure that the new status is valid for ALL bugs.
+# Make sure that the new status is allowed by the status workflow.
 sub check_status_transition {
-    my ($self, $new_status, $bug_ids) = @_;
-    my $dbh = Bugzilla->dbh;
+    my ($self, $new_status) = @_;
 
-    check_field('bug_status', $new_status);
-    trick_taint($new_status);
-
-    my $illegal_statuses =
-      $dbh->selectcol_arrayref('SELECT DISTINCT bug_status.value
-                                  FROM bug_status
-                            INNER JOIN bugs
-                                    ON bugs.bug_status = bug_status.value
-                                 WHERE bug_id IN (' . join (',', @$bug_ids). ')
-                                   AND bug_status.id NOT IN (SELECT old_status
-                                                               FROM status_workflow
-                                                         INNER JOIN bug_status b_s
-                                                                 ON b_s.id = status_workflow.new_status
-                                                              WHERE b_s.value = ?)',
-                                 undef, $new_status);
-
-    if (scalar(@$illegal_statuses)) {
-        ThrowUserError('illegal_bug_status_transition', {old => $illegal_statuses,
-                                                         new => $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})
     }
 }
 
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index cf8f98efa..9af0f043c 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -93,6 +93,28 @@ sub can_change_to {
     return $self->{'can_change_to'};
 }
 
+sub can_change_from {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+
+    if (!defined $self->{'can_change_from'}) {
+        my $old_status_ids = $dbh->selectcol_arrayref('SELECT old_status
+                                                         FROM status_workflow
+                                                   INNER JOIN bug_status
+                                                           ON id = old_status
+                                                        WHERE isactive = 1
+                                                          AND new_status = ?
+                                                          AND old_status IS NOT NULL',
+                                                        undef, $self->id);
+
+        # Allow the bug status to remain unchanged.
+        push(@$old_status_ids, $self->id);
+        $self->{'can_change_from'} = Bugzilla::Status->new_from_list($old_status_ids);
+    }
+
+    return $self->{'can_change_from'};
+}
+
 sub add_missing_bug_status_transitions {
     my $bug_status = shift || Bugzilla->params->{'duplicate_or_move_bug_status'};
     my $dbh = Bugzilla->dbh;
@@ -161,7 +183,20 @@ below.
 =item C<can_change_to>
 
  Description: Returns the list of active statuses a bug can be changed to
-              given the current bug status.
+              given the current bug status. If this method is called as a
+              class method, then it returns all bug statuses available on
+              bug creation.
+
+ Params:      none.
+
+ Returns:     A list of Bugzilla::Status objects.
+
+=item C<can_change_from>
+
+ Description: Returns the list of active statuses a bug can be changed from
+              given the new bug status. If the bug status is available on
+              bug creation, this method doesn't return this information.
+              You have to call C<can_change_to> instead.
 
  Params:      none.
 
diff --git a/buglist.cgi b/buglist.cgi
index f9c9875c4..8aa9249df 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -1099,9 +1099,8 @@ $vars->{'buglist_joined'} = join(',', @bugidlist);
 $vars->{'columns'} = $columns;
 $vars->{'displaycolumns'} = \@displaycolumns;
 
-my @openstates = BUG_STATE_OPEN;
-$vars->{'openstates'} = \@openstates;
-$vars->{'closedstates'} = ['CLOSED', 'VERIFIED', 'RESOLVED'];
+$vars->{'openstates'} = [BUG_STATE_OPEN];
+$vars->{'closedstates'} = [map {$_->name} Bugzilla::Status::closed_bug_statuses()];
 
 # The list of query fields in URL query string format, used when creating
 # URLs to the same query results page with different parameters (such as
diff --git a/process_bug.cgi b/process_bug.cgi
index f8b5201b3..14d6c4ce7 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -814,8 +814,11 @@ 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) {
-    Bugzilla::Bug->check_status_transition($knob, \@idlist);
+    # 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) {
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index f7fe1d652..f92399f00 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -655,7 +655,7 @@
   [% ELSIF error == "illegal_bug_status_transition" %]
     [% title = "Illegal $terms.Bug Status Change" %]
     You are not allowed to change the [% terms.bug %] status from
-    [%+ old.join(", ") FILTER html %] to [%+ new FILTER html %].
+    [%+ old FILTER html %] to [% new FILTER html %].
 
   [% ELSIF error == "illegal_change" %]
     [% title = "Not allowed" %]
diff --git a/template/en/default/list/edit-multiple.html.tmpl b/template/en/default/list/edit-multiple.html.tmpl
index 28e513e7b..0fd206797 100644
--- a/template/en/default/list/edit-multiple.html.tmpl
+++ b/template/en/default/list/edit-multiple.html.tmpl
@@ -300,7 +300,9 @@
 
 [% END %]
 
-
+[% all_open_bugs = !current_bug_statuses.containsany(closedstates) %]
+[% all_closed_bugs = !current_bug_statuses.containsany(openstates) %]
+[% display_warning = 0 %]
 
 <input id="knob-none" type="radio" name="knob" value="none" checked="checked">
 <label for="knob-none">Do nothing else</label><br>
@@ -311,18 +313,35 @@
     <label for="knob_[% bug_status.id FILTER html %]">
       Change status to <b>[% get_status(bug_status.name) FILTER html %]</b>
     </label>
-    [% IF !bug_status.is_open %]
-      and set the resolution to [% PROCESS select_resolution field = "knob_${bug_status.id}" %]
+    [%# Closed bugs cannot have their resolution changed this way. %]
+    [% IF !bug_status.is_open && !all_closed_bugs %]
+      and set the resolution to [% PROCESS select_resolution id = bug_status.id %]
+      [%+ "(*)" UNLESS all_open_bugs %]
+      [% display_warning = 1 UNLESS all_open_bugs %]
     [% END %]
     <br>
 [% END %]
 
 [%# If all the bugs being changed are open, allow the user to clear their resolution. %]
-[% IF !current_bug_statuses.containsany(closedstates) %]
+[% IF all_open_bugs %]
   <input id="knob-clearresolution" type="radio" name="knob" value="clearresolution">
   <label for="knob-clearresolution">Clear the resolution</label><br>
 [% END %]
 
+[%# If all the bugs being changed are closed, allow the user to change their resolution. %]
+[% IF all_closed_bugs %]
+  <input type="radio" id="knob_change_resolution" name="knob" value="change_resolution">
+  <label for="knob_change_resolution">Change resolution to</label>
+  [%+ PROCESS select_resolution id = "change_resolution" %]<br>
+[% END %]
+
+[% IF display_warning %]
+  <p class="box">
+    (*) Note that the resolution will only be applied to open [% terms.bugs %].
+    Already closed [% terms.bugs %] will keep their resolution unchanged.
+  </p>
+[% END %]
+
 <input type="submit" id="commit" value="Commit">
 
 [% IF Param('move-enabled') && user.is_mover %]
@@ -346,8 +365,8 @@
 [% END %]
 
 [% BLOCK select_resolution %]
-  <select name="resolution"
-          onchange="document.forms['changeform'].[% field FILTER html %].checked=true">
+  <select id="resolution_knob_[% id FILTER html %]" name="resolution_knob_[% id FILTER html %]"
+          onchange="document.forms['changeform'].[% "knob_$id" FILTER html %].checked=true">
     [% FOREACH r = resolutions %]
       [% NEXT IF !r %]
       <option value="[% r FILTER html %]">[% get_resolution(r) FILTER html %]</option>