Bug 344965: Fix process_bug.cgi and bug/* templates to work with custom bug…

Bugzilla/Bug.pm

@@ -42,6 +42,7 @@ use Bugzilla::Error;
 use Bugzilla::Product;
 use Bugzilla::Component;
 use Bugzilla::Group;
+use Bugzilla::Status;
 
 use List::Util qw(min);
 use Storable qw(dclone);
@@ -52,8 +53,9 @@ use base qw(Bugzilla::Object Exporter);
     bug_alias_to_id ValidateBugAlias ValidateBugID
     RemoveVotes CheckIfVotedConfirmed
     LogActivityEntry
-    is_open_state
+    BUG_STATE_OPEN is_open_state
     editable_bug_fields
+    SPECIAL_STATUS_WORKFLOW_ACTIONS
 );
 
 #####################################################################
@@ -176,6 +178,19 @@ use constant VALID_ENTRY_STATUS => qw(
     ASSIGNED
 );
 
+use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw(
+    none
+    duplicate
+    change_resolution
+    clearresolution
+);
+
+sub BUG_STATE_OPEN {
+    # XXX - We should cache this list.
+    my $dbh = Bugzilla->dbh;
+    return @{$dbh->selectcol_arrayref('SELECT value FROM bug_status WHERE is_open = 1')};
+}
+
 #####################################################################
 
 sub new {
@@ -213,12 +228,6 @@ sub new {
         return $error_self;
     }
 
-    # XXX At some point these should be moved into accessors.
-    # They only are here because this is how Bugzilla::Bug
-    # originally did things, before it was a Bugzilla::Object.
-    $self->{'isunconfirmed'} = ($self->{bug_status} eq 'UNCONFIRMED');
-    $self->{'isopened'}      = is_open_state($self->{bug_status});
-
     return $self;
 }
 
@@ -1025,8 +1034,7 @@ sub set_status {
     my ($self, $status) = @_;
     $self->set('bug_status', $status); 
     # Check for the everconfirmed transition
-    $self->_set_everconfirmed(1) if ($status eq 'NEW'
-                                     || $status eq 'ASSIGNED');
+    $self->_set_everconfirmed(1) if (is_open_state($status) && $status ne 'UNCONFIRMED');
 }
 
 ########################
@@ -1247,6 +1255,16 @@ sub flag_types {
     return $self->{'flag_types'};
 }
 
+sub isopened {
+    my $self = shift;
+    return is_open_state($self->{bug_status}) ? 1 : 0;
+}
+
+sub isunconfirmed {
+    my $self = shift;
+    return ($self->bug_status eq 'UNCONFIRMED') ? 1 : 0;
+}
+
 sub keywords {
     my ($self) = @_;
     return $self->{'keywords'} if exists $self->{'keywords'};
@@ -1317,6 +1335,13 @@ sub reporter {
     return $self->{'reporter'};
 }
 
+sub status {
+    my $self = shift;
+    return undef if $self->{'error'};
+
+    $self->{'status'} ||= new Bugzilla::Status({name => $self->{'bug_status'}});
+    return $self->{'status'};
+}
 
 sub show_attachment_flags {
     my ($self) = @_;
@@ -1530,18 +1555,159 @@ sub bug_alias_to_id {
 # Workflow Control routines
 #####################################################################
 
+# Make sure that the new status is valid for ALL bugs.
+sub check_status_transition {
+    my ($self, $new_status, $bug_ids) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    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})
+    }
+}
+
+# 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, $bug_ids, $vars) = @_;
+    my $dbh = Bugzilla->dbh;
+    $vars ||= {};
+
+    # 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"};
+        }
+        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');
+
+    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.
+        $vars->{remove_remaining_time} = 1;
+    }
+    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};
+
+        $vars->{remove_remaining_time} = 1 if ($action ne 'change_resolution');
+    }
+    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);
-    }
-    elsif ($action eq 'reopen') {
-        $status = $self->everconfirmed ? 'REOPENED' : 'UNCONFIRMED';
-        $resolution = '';
+        return ($self->bug_status, $self->resolution, $self->everconfirmed);
     }
     elsif ($action eq 'duplicate') {
         # Only alter the bug status if the bug is currently open.
@@ -1560,37 +1726,21 @@ sub get_new_status_and_resolution {
         $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 });
-        }
-
+        $status = $action;
         if (is_open_state($status)) {
             # Open bugs have no resolution.
             $resolution = '';
+            $everconfirmed = ($status eq 'UNCONFIRMED') ? 0 : 1;
         }
-        else {
-            # All non-open statuses must have a resolution.
+        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
+        }
     }
     # Now it's time to validate the bug resolution.
     # Bug resolutions have no workflow specific rules, so any valid
@@ -1598,7 +1748,7 @@ sub get_new_status_and_resolution {
     check_field('resolution', $resolution) if ($resolution ne '');
     trick_taint($resolution);
 
-    return ($status, $resolution);
+    return ($status, $resolution, $everconfirmed);
 }
 
 #####################################################################
@@ -1968,7 +2118,7 @@ sub CountOpenDependencies {
             "FROM bugs, dependencies " .
            "WHERE blocked IN (" . (join "," , @bug_list) . ") " .
              "AND bug_id = dependson " .
-             "AND bug_status IN ('" . (join "','", BUG_STATE_OPEN)  . "') " .
+             "AND bug_status IN (" . join(', ', map {$dbh->quote($_)} BUG_STATE_OPEN)  . ") " .
           $dbh->sql_group_by('blocked'));
     $sth->execute();
 

Bugzilla/Constants.pm

@@ -120,8 +120,6 @@ use File::Basename;
     FIELD_TYPE_FREETEXT
     FIELD_TYPE_SINGLE_SELECT
 
-    BUG_STATE_OPEN
-
     USAGE_MODE_BROWSER
     USAGE_MODE_CMDLINE
     USAGE_MODE_WEBSERVICE
@@ -351,10 +349,6 @@ use constant SAFE_PROTOCOLS => ('afs', 'cid', 'ftp', 'gopher', 'http', 'https',
                                 'irc', 'mid', 'news', 'nntp', 'prospero', 'telnet',
                                 'view-source', 'wais');
 
-# States that are considered to be "open" for bugs.
-use constant BUG_STATE_OPEN => ('NEW', 'REOPENED', 'ASSIGNED', 
-                                'UNCONFIRMED');
-
 # Usage modes. Default USAGE_MODE_BROWSER. Use with Bugzilla->usage_mode.
 use constant USAGE_MODE_BROWSER    => 0;
 use constant USAGE_MODE_CMDLINE    => 1;

Bugzilla/Install/DB.pm

@@ -22,7 +22,7 @@ package Bugzilla::Install::DB;
 
 use strict;
 
-use Bugzilla::Bug qw(is_open_state);
+use Bugzilla::Bug qw(BUG_STATE_OPEN is_open_state);
 use Bugzilla::Constants;
 use Bugzilla::Hook;
 use Bugzilla::Install::Util qw(indicate_progress);

Bugzilla/Status.pm

+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Bug Tracking System.
+#
+# The Initial Developer of the Original Code is Frédéric Buclin.
+# Portions created by Frédéric Buclin are Copyright (C) 2007
+# Frédéric Buclin. All Rights Reserved.
+#
+# Contributor(s): Frédéric Buclin <LpSolit@gmail.com>
+
+use strict;
+
+package Bugzilla::Status;
+
+use base qw(Bugzilla::Object);
+
+################################
+#####   Initialization     #####
+################################
+
+use constant DB_TABLE => 'bug_status';
+
+use constant DB_COLUMNS => qw(
+    id
+    value
+    sortkey
+    isactive
+    is_open
+);
+
+use constant NAME_FIELD => 'value';
+use constant LIST_ORDER => 'sortkey, value';
+
+###############################
+#####     Accessors        ####
+###############################
+
+sub name      { return $_[0]->{'value'};    }
+sub sortkey   { return $_[0]->{'sortkey'};  }
+sub is_active { return $_[0]->{'isactive'}; }
+sub is_open   { return $_[0]->{'is_open'};  }
+
+###############################
+#####       Methods        ####
+###############################
+
+sub can_change_to {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+
+    if (!defined $self->{'can_change_to'}) {
+        my $new_status_ids = $dbh->selectcol_arrayref('SELECT new_status
+                                                         FROM status_workflow
+                                                   INNER JOIN bug_status
+                                                           ON id = new_status
+                                                        WHERE isactive = 1
+                                                          AND old_status = ?',
+                                                        undef, $self->id);
+
+        $self->{'can_change_to'} = Bugzilla::Status->new_from_list($new_status_ids);
+    }
+
+    return $self->{'can_change_to'};
+}
+
+
+1;
+
+__END__
+
+=head1 NAME
+
+Bugzilla::Status - Bug status class.
+
+=head1 SYNOPSIS
+
+    use Bugzilla::Status;
+
+    my $bug_status = new Bugzilla::Status({name => 'ASSIGNED'});
+    my $bug_status = new Bugzilla::Status(4);
+
+=head1 DESCRIPTION
+
+Status.pm represents a bug status object. It is an implementation
+of L<Bugzilla::Object>, and thus provides all methods that
+L<Bugzilla::Object> provides.
+
+The methods that are specific to C<Bugzilla::Status> are listed
+below.
+
+=head1 METHODS
+
+=over
+
+=item C<can_change_to>
+
+ Description: Returns the list of active statuses a bug can be changed to
+              given the current bug status.
+
+ Params:      none.
+
+ Returns:     A list of Bugzilla::Status objects.
+
+=back
+
+=cut

buglist.cgi

@@ -1139,8 +1139,26 @@ if ($dotweak) {
 
     $vars->{'unconfirmedstate'} = 'UNCONFIRMED';
 
-    $vars->{'bugstatuses'} = [ keys %$bugstatuses ];
-
+    # Convert bug statuses to their ID.
+    my @bug_statuses = map {$dbh->quote($_)} keys %$bugstatuses;
+    my $bug_status_ids =
+      $dbh->selectcol_arrayref('SELECT id FROM bug_status
+                                WHERE value IN (' . join(', ', @bug_statuses) .')');
+
+    # This query collects new statuses which are common to all current bug statuses.
+    # It also accepts transitions where the bug status doesn't change.
+    $bug_status_ids =
+      $dbh->selectcol_arrayref('SELECT DISTINCT new_status
+                                FROM status_workflow sw1
+                                WHERE NOT EXISTS (SELECT * FROM status_workflow sw2
+                                                  WHERE sw2.old_status != sw1.new_status
+                                                  AND sw2.old_status IN (' . join(', ', @$bug_status_ids) . ')
+                                                  AND NOT EXISTS (SELECT * FROM status_workflow sw3
+                                                                  WHERE sw3.new_status = sw1.new_status
+                                                                  AND sw3.old_status = sw2.old_status))');
+
+    $vars->{'current_bug_statuses'} = [keys %$bugstatuses];
+    $vars->{'new_bug_statuses'} = Bugzilla::Status->new_from_list($bug_status_ids);
     # The groups to which the user belongs.
     $vars->{'groups'} = GetGroups();
 

editworkflow.cgi

@@ -27,6 +27,7 @@ use Bugzilla;
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Token;
+use Bugzilla::Status;
 
 my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
@@ -42,12 +43,6 @@ $user->in_group('admin')
 my $action = $cgi->param('action') || 'edit';
 my $token = $cgi->param('token');
 
-sub get_statuses {
-    my $statuses = $dbh->selectall_arrayref('SELECT id, value, is_open FROM bug_status
-                                             ORDER BY sortkey, value', { Slice => {} });
-    return $statuses;
-}
-
 sub get_workflow {
     my $workflow = $dbh->selectall_arrayref('SELECT old_status, new_status, require_comment
                                              FROM status_workflow');
@@ -64,7 +59,7 @@ sub load_template {
     my $template = Bugzilla->template;
     my $vars = {};
 
-    $vars->{'statuses'} = get_statuses();
+    $vars->{'statuses'} = [Bugzilla::Status->get_all];
     $vars->{'workflow'} = get_workflow();
     $vars->{'token'} = issue_session_token("workflow_$filename");
     $vars->{'message'} = $message;
@@ -79,9 +74,8 @@ if ($action eq 'edit') {
 }
 elsif ($action eq 'update') {
     check_token_data($token, 'workflow_edit');
-    my $statuses = get_statuses;
+    my $statuses = [Bugzilla::Status->get_all];
     my $workflow = get_workflow();
-    my $initial_state = {id => 0};
 
     my $sth_insert = $dbh->prepare('INSERT INTO status_workflow (old_status, new_status)
                                     VALUES (?, ?)');
@@ -90,22 +84,28 @@ elsif ($action eq 'update') {
     my $sth_delnul = $dbh->prepare('DELETE FROM status_workflow
                                     WHERE old_status IS NULL AND new_status = ?');
 
-    foreach my $old ($initial_state, @$statuses) {
-        # Hashes cannot have undef as a key, so we use 0. But the DB
-        # must store undef, for referential integrity.
-        my $old_id_for_db = $old->{'id'} || undef;
+    # Part 1: Initial bug statuses.
+    foreach my $new (@$statuses) {
+        if ($cgi->param('w_0_' . $new->id)) {
+            $sth_insert->execute(undef, $new->id)
+              unless defined $workflow->{0}->{$new->id};
+        }
+        else {
+            $sth_delnul->execute($new->id);
+        }
+    }
+
+    # Part 2: Bug status changes.
+    foreach my $old (@$statuses) {
         foreach my $new (@$statuses) {
-            next if $old->{'id'} == $new->{'id'};
+            next if $old->id == $new->id;
 
-            if ($cgi->param('w_' . $old->{'id'} . '_' . $new->{'id'})) {
-                $sth_insert->execute($old_id_for_db, $new->{'id'})
-                  unless defined $workflow->{$old->{'id'}}->{$new->{'id'}};
-            }
-            elsif ($old_id_for_db) {
-                $sth_delete->execute($old_id_for_db, $new->{'id'});
+            if ($cgi->param('w_' . $old->id . '_' . $new->id)) {
+                $sth_insert->execute($old->id, $new->id)
+                  unless defined $workflow->{$old->id}->{$new->id};
             }
             else {
-                $sth_delnul->execute($new->{'id'});
+                $sth_delete->execute($old->id, $new->id);
             }
         }
     }

process_bug.cgi

@@ -120,6 +120,11 @@ sub send_results {
     $vars->{'header_done'} = 1;
 }
 
+sub comment_exists {
+    my $cgi = Bugzilla->cgi;
+    return ($cgi->param('comment') && $cgi->param('comment') =~ /\S+/) ? 1 : 0;
+}
+
 ######################################################################
 # Begin Data/Security Validation
 ######################################################################
@@ -244,32 +249,6 @@ if ($cgi->cookie("BUGLIST") && defined $cgi->param('id')) {
     $vars->{'bug_list'} = \@bug_list;
 }
 
-# This function checks if there is a comment required for a specific
-# function and tests, if the comment was given.
-# If comments are required for functions is defined by params.
-#
-sub CheckonComment {
-    my ($function) = (@_);
-    my $cgi = Bugzilla->cgi;
-    
-    # Param is 1 if comment should be added !
-    my $ret = Bugzilla->params->{ "commenton" . $function };
-
-    # Allow without comment in case of undefined Params.
-    $ret = 0 unless ( defined( $ret ));
-
-    if( $ret ) {
-        if (!defined $cgi->param('comment')
-            || $cgi->param('comment') =~ /^\s*$/) {
-            # No comment - sorry, action not allowed !
-            ThrowUserError("comment_required");
-        } else {
-            $ret = 0;
-        }
-    }
-    return( ! $ret ); # Return val has to be inverted
-}
-
 # Figure out whether or not the user is trying to change the product
 # (either the "product" variable is not set to "don't change" or the
 # user is changing a single bug and has changed the bug's product),
@@ -287,11 +266,13 @@ if (defined $cgi->param('id')) {
 defined($cgi->param('product'))
   || ThrowCodeError('undefined_field', { field => 'product' });
 
-if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct) 
+if ((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct)
      || (!$cgi->param('id')
          && $cgi->param('product') ne $cgi->param('dontchange')))
-    && CheckonComment( "reassignbycomponent" ))
 {
+    if (Bugzilla->params->{'commentonreassignbycomponent'} && !comment_exists()) {
+        ThrowUserError('comment_required');
+    }
     # Check to make sure they actually have the right to change the product
     if (!$bug->check_can_change_field('product', $oldproduct, $cgi->param('product'),
                                       \$PrivilegesRequired))
@@ -439,6 +420,7 @@ defined($cgi->param('component'))
 
 # 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;
@@ -448,11 +430,6 @@ sub DuplicateUserConfirm {
         return;
     }
 
-    # Remember that we validated both these ids earlier, so we know
-    # they are both valid bug ids
-    my $dupe = $cgi->param('id');
-    my $original = $cgi->param('dup_id');
-    
     my $reporter = $dbh->selectrow_array(
         q{SELECT reporter FROM bugs WHERE bug_id = ?}, undef, $dupe);
     my $rep_user = Bugzilla::User->new($reporter);
@@ -618,16 +595,6 @@ sub DoComma {
     $::comma = ",";
 }
 
-sub DoConfirm {
-    my $bug = shift;
-    if ($bug->check_can_change_field("canconfirm", 0, 1, 
-                                     \$PrivilegesRequired)) 
-    {
-        DoComma();
-        $::query .= "everconfirmed = 1";
-    }
-}
-
 # Changing this so that it will process groups from checkboxes instead of
 # select lists.  This means that instead of looking for the bit-X values in
 # the form, we need to loop through all the bug groups this user has access
@@ -941,127 +908,54 @@ if (defined $cgi->param('qa_contact') && !$cgi->param('set_default_qa_contact'))
     }
 }
 
-if ($cgi->param('set_default_assignee') || $cgi->param('set_default_qa_contact')) {
-    CheckonComment('reassignbycomponent');
+if (($cgi->param('set_default_assignee') || $cgi->param('set_default_qa_contact'))
+    && Bugzilla->params->{'commentonreassignbycomponent'} && !comment_exists())
+{
+        ThrowUserError('comment_required');
 }
 
 my $duplicate; # It will store the ID of the bug we are pointing to, if any.
 
-SWITCH: for ($cgi->param('knob')) {
-    /^none$/ && do {
-        last SWITCH;
-    };
-    /^confirm$/ && CheckonComment( "confirm" ) && do {
-        DoConfirm($bug);
-        last SWITCH;
-    };
-    /^accept$/ && CheckonComment( "accept" ) && do {
-        DoConfirm($bug);
-        if (Bugzilla->params->{"usetargetmilestone"}
-            && Bugzilla->params->{"musthavemilestoneonaccept"}) 
-        {
-            $requiremilestone = 1;
-        }
-        last SWITCH;
-    };
-    /^clearresolution$/ && CheckonComment( "clearresolution" ) && do {
-        last SWITCH;
-    };
-    /^(resolve|change_resolution)$/ && CheckonComment( "resolve" ) && do {
-        # Check here, because it's the only place we require the resolution
-        check_field('resolution', scalar $cgi->param('resolution'),
-                    Bugzilla::Bug->settable_resolutions);
-
-        # don't resolve as fixed while still unresolved blocking bugs
-        if (Bugzilla->params->{"noresolveonopenblockers"}
-            && $cgi->param('resolution') eq 'FIXED')
-        {
-            my @dependencies = Bugzilla::Bug::CountOpenDependencies(@idlist);
-            if (scalar @dependencies > 0) {
-                ThrowUserError("still_unresolved_bugs",
-                               { dependencies     => \@dependencies,
-                                 dependency_count => scalar @dependencies });
-            }
-        }
-
-        if ($cgi->param('knob') eq 'resolve') {
-            # RESOLVED bugs should have no time remaining;
-            # more time can be added for the VERIFY step, if needed.
-            _remove_remaining_time();
-        }
-        else {
-            # You cannot use change_resolution if there is at least
-            # one open bug.
-            my $open_states = join(',', map {$dbh->quote($_)} BUG_STATE_OPEN);
-            my $idlist = join(',', @idlist);
-            my $is_open =
-              $dbh->selectrow_array("SELECT 1 FROM bugs WHERE bug_id IN ($idlist)
-                                     AND bug_status IN ($open_states)");
-
-            ThrowUserError('resolution_not_allowed') if $is_open;
-        }
-        last SWITCH;
-    };
-    /^reopen$/  && CheckonComment( "reopen" ) && do {
-        last SWITCH;
-    };
-    /^verify$/ && CheckonComment( "verify" ) && do {
-        last SWITCH;
-    };
-    /^close$/ && CheckonComment( "close" ) && do {
-        # CLOSED bugs should have no time remaining.
-        _remove_remaining_time();
-        last SWITCH;
-    };
-    /^duplicate$/ && CheckonComment( "duplicate" ) && do {
-        # You cannot mark bugs as duplicates when changing
-        # several bugs at once.
-        unless (defined $cgi->param('id')) {
-            ThrowUserError('dupe_not_allowed');
-        }
-
-        # Make sure we can change the original bug (issue A on bug 96085)
-        defined($cgi->param('dup_id'))
-          || ThrowCodeError('undefined_field', { field => 'dup_id' });
-
-        $duplicate = $cgi->param('dup_id');
-        ValidateBugID($duplicate, 'dup_id');
-        $cgi->param('dup_id', $duplicate);
-
-        # Make sure a loop isn't created when marking this bug
-        # as duplicate.
-        my %dupes;
-        my $dupe_of = $duplicate;
-        my $sth = $dbh->prepare('SELECT dupe_of FROM duplicates
-                                 WHERE dupe = ?');
-
-        while ($dupe_of) {
-            if ($dupe_of == $cgi->param('id')) {
-                ThrowUserError('dupe_loop_detected', { bug_id  => $cgi->param('id'),
-                                                       dupe_of => $duplicate });
-            }
-            # 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.
-        DuplicateUserConfirm();
-
-        # DUPLICATE bugs should have no time remaining.
-        _remove_remaining_time();
-        last SWITCH;
-    };
-
-    ThrowCodeError("unknown_action", { action => $cgi->param('knob') });
+# 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) {
+    Bugzilla::Bug->check_status_transition($knob, \@idlist);
+    my $bug_status = new Bugzilla::Status({name => $knob});
+    # 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));
+    }
+}
+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, \@idlist, $vars);
+
+# Some triggers require extra actions.
+$duplicate = $vars->{dup_id};
+$requiremilestone = $vars->{requiremilestone};
+DuplicateUserConfirm($vars->{bug_id}, $duplicate) if $vars->{DuplicateUserConfirm};
+_remove_remaining_time() if $vars->{remove_remaining_time};
+
 my @keywordlist;
 my %keywordseen;
 
@@ -1252,14 +1146,15 @@ foreach my $id (@idlist) {
     my $comma = $::comma;
     my $old_bug_obj = new Bugzilla::Bug($id);
 
-    my $status;
+    my ($status, $everconfirmed);
     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)$/) {
+    # 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) =
-      $old_bug_obj->get_new_status_and_resolution(scalar $cgi->param('knob'), $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 = ?";
@@ -1271,6 +1166,11 @@ foreach my $id (@idlist) {
         push(@bug_values, $resolution);
         $comma = ',';
     }
+    if ($everconfirmed ne $old_bug_obj->everconfirmed) {
+        $query .= "$comma everconfirmed = ?";
+        push(@bug_values, $everconfirmed);
+        $comma = ',';
+    }
 
     # We have to check whether the bug is moved to another product
     # and/or component before reassigning. If $component is defined,
@@ -1314,7 +1214,7 @@ foreach my $id (@idlist) {
             "user_group_map READ", "group_group_map READ", "flagtypes READ",
             "flaginclusions AS i READ", "flagexclusions AS e READ",
             "keyworddefs READ", "groups READ", "attachments READ",
-            "group_control_map AS oldcontrolmap READ",
+            "bug_status READ", "group_control_map AS oldcontrolmap READ",
             "group_control_map AS newcontrolmap READ",
             "group_control_map READ", "email_setting READ", "classifications READ");
 

reports.cgi

@@ -43,6 +43,7 @@ use Bugzilla;
 use Bugzilla::Constants;
 use Bugzilla::Util;
 use Bugzilla::Error;
+use Bugzilla::Bug;
 
 eval "use GD";
 $@ && ThrowCodeError("gd_not_installed");

sanitycheck.cgi

@@ -32,6 +32,7 @@ use Bugzilla;
 use Bugzilla::Constants;
 use Bugzilla::Util;
 use Bugzilla::Error;
+use Bugzilla::Bug;
 
 ###########################################################################
 # General subs

template/en/default/admin/workflow/comment.html.tmpl

@@ -50,24 +50,24 @@
     <th>&nbsp;</th>
     [% FOREACH status = statuses %]
       <th class="col-header[% status.is_open ? " open-status" : " closed-status" %]">
-        [% status.value FILTER html %]
+        [% status.name FILTER html %]
       </th>
     [% END %]
   </tr>
 
   [%# This defines the entry point in the workflow %]
-  [% p = [{id => 0, value => "{Start}", is_open => 1}] %]
+  [% p = [{id => 0, name => "{Start}", is_open => 1}] %]
   [% FOREACH status = p.merge(statuses) %]
     <tr class="highlight">
       <th align="right" class="[% status.is_open ? "open-status" : "closed-status" %]">
-        [% status.value FILTER html %]
+        [% status.name FILTER html %]
       </th>
 
       [% FOREACH new_status = statuses %]
         [% IF workflow.${status.id}.${new_status.id}.defined %]
           <td align="center" class="checkbox-cell
               [% " checked" IF workflow.${status.id}.${new_status.id} %]"
-              title="From [% status.value FILTER html %] to [% new_status.value FILTER html %]">
+              title="From [% status.name FILTER html %] to [% new_status.name FILTER html %]">
             <input type="checkbox" name="c_[% status.id %]_[% new_status.id %]"
                    id="c_[% status.id %]_[% new_status.id %]" onclick="toggle_cell(this)"
               [% " checked='checked'" IF workflow.${status.id}.${new_status.id} %]>

template/en/default/admin/workflow/edit.html.tmpl

@@ -34,8 +34,12 @@
 </script>
 
 <p>
-  This page allows you to define which status transitions are valid
-  in your workflow.
+  This page allows you to define which status transitions are valid in your workflow.
+  For compatibility with older versions of [% terms.Bugzilla %], reopening [% terms.abug %]
+  will only display either UNCONFIRMED or REOPENED (if allowed by your workflow) but not
+  both. The decision depends on whether the [% terms.bug %] has ever been confirmed or not.
+  So it is a good idea to allow both transitions and let [% terms.Bugzilla %] select the
+  correct one.
 </p>
 
 <form id="workflow_form" method="POST" action="editworkflow.cgi">
@@ -50,24 +54,24 @@
     <th>&nbsp;</th>
     [% FOREACH status = statuses %]
       <th class="col-header[% status.is_open ? " open-status" : " closed-status" %]">
-        [% status.value FILTER html %]
+        [% status.name FILTER html %]
       </th>
     [% END %]
   </tr>
 
   [%# This defines the entry point in the workflow %]
-  [% p = [{id => 0, value => "{Start}", is_open => 1}] %]
+  [% p = [{id => 0, name => "{Start}", is_open => 1}] %]
   [% FOREACH status = p.merge(statuses) %]
     <tr class="highlight">
       <th align="right" class="[% status.is_open ? "open-status" : "closed-status" %]">
-        [% status.value FILTER html %]
+        [% status.name FILTER html %]
       </th>
 
       [% FOREACH new_status = statuses %]
         [% IF status.id != new_status.id %]
           <td align="center" class="checkbox-cell
               [% " checked" IF workflow.${status.id}.${new_status.id}.defined %]"
-              title="From [% status.value FILTER html %] to [% new_status.value FILTER html %]">
+              title="From [% status.name FILTER html %] to [% new_status.name FILTER html %]">
             <input type="checkbox" name="w_[% status.id %]_[% new_status.id %]"
                    id="w_[% status.id %]_[% new_status.id %]" onclick="toggle_cell(this)"
               [% " checked='checked'" IF workflow.${status.id}.${new_status.id}.defined %]>

template/en/default/bug/edit.html.tmpl

@@ -595,7 +595,7 @@
       <td align="right">
         <b><a href="page.cgi?id=fields.html#status">Status</a></b>:
       </td>
-      <td>[% status_descs.${bug.bug_status} FILTER html %]</td>
+      <td>[% get_status(bug.bug_status) FILTER html %]</td>
     </tr>
 
     <tr>

template/en/default/bug/knob.html.tmpl

@@ -18,42 +18,39 @@
   #
   # Contributor(s): Gervase Markham <gerv@gerv.net>
   #                 Vaskin Kissoyan <vkissoyan@yahoo.com>
+  #                 Frédéric Buclin <LpSolit@gmail.com>
   #%]
 
 [% PROCESS global/variables.none.tmpl %]
 
-[%# *** Knob *** %]
-
 <br>
 <div id="knob">
   <div id="knob-options">
-
-  [% knum = 1 %]
   [% initial_action_shown = 0 %]
 
-  [% IF bug.isunconfirmed && bug.user.canconfirm %]
-    [% PROCESS initial_action %]
-    <input type="radio" id="knob-confirm" name="knob" value="confirm">
-    <label for="knob-confirm">
-      Confirm [% terms.bug %] (change status to <b>[% get_status("NEW") FILTER html %]</b>)
-    </label>
-    <br>
-    [% knum = knum + 1 %]
-  [% END %]
-
-  [% IF bug.isopened && bug.bug_status != "ASSIGNED" && bug.user.canedit
-        && (!bug.isunconfirmed || bug.user.canconfirm) %]
+  [%# These actions are based on the current custom workflow. %]
+  [% FOREACH bug_status = bug.status.can_change_to %]
+    [% NEXT IF bug.isunconfirmed && bug_status.is_open && !bug.user.canconfirm %]
+    [% NEXT IF bug.isopened && !bug.isunconfirmed && bug_status.is_open && !bug.user.canedit %]
+    [% NEXT IF !bug_status.is_open && !bug.user.canedit && !bug.user.isreporter %]
+    [% NEXT IF !bug_status.is_open && bug_status.is_open && !bug.user.canedit && !bug.user.isreporter %]
+    [%# Special hack to only display UNCO or REOP when reopening, but not both;
+      # for compatibility with older versions. %]
+    [% NEXT IF !bug.isopened && (bug.everconfirmed && bug_status.name == "UNCONFIRMED"
+                                 || !bug.everconfirmed && bug_status.name == "REOPENED") %]
     [% PROCESS initial_action %]
-    <input type="radio" id="knob-accept" name="knob" value="accept">
-    <label for="knob-accept">
-      Accept [% terms.bug %] (
-      [% IF bug.isunconfirmed %]confirm [% terms.bug %], and [% END %]change
-      status to <b>[% get_status("ASSIGNED") FILTER html %]</b>)
+    <input type="radio" id="knob_[% bug_status.id FILTER html %]" name="knob"
+           value="[% bug_status.name FILTER html %]">
+    <label for="knob_[% bug_status.id FILTER html %]">
+      Change status to <b>[% get_status(bug_status.name) FILTER html %]</b>
     </label>
+    [% IF bug.isopened && !bug_status.is_open %]
+      and set the resolution to [% PROCESS select_resolution field = "knob_${bug_status.id}" %]
+    [% END %]
     <br>
-    [% knum = knum + 1 %]
   [% END %]
 
+  [%# These actions are special and are independent of the workflow. %]
   [% IF bug.user.canedit || bug.user.isreporter %]
     [% IF bug.isopened %]
       [% IF bug.resolution %]
@@ -64,65 +61,27 @@
           <b>[% get_resolution(bug.resolution) FILTER html %]</b>)
         </label>
         <br>
-        [% knum = knum + 1 %]
       [% END %]
-
-      [% PROCESS initial_action %]
-      <input type="radio" id="knob-resolve" name="knob" value="resolve">
-      <label for="knob-resolve">
-        Resolve [% terms.bug %], changing 
-        <a href="page.cgi?id=fields.html#resolution">resolution</a> to
-      </label>  
-      [% PROCESS select_resolution %]
-
-      [% PROCESS duplicate %]
     [% ELSE %]
-      [% IF bug.resolution != "MOVED" ||
-           (bug.resolution == "MOVED" && bug.user.canmove) %]
+      [% IF bug.resolution != "MOVED" || bug.user.canmove %]
         [% PROCESS initial_action %]
-        <input type="radio" id="knob-change-resolution" name="knob" value="change_resolution">
-        <label for="knob-change-resolution">
+        <input type="radio" id="knob_change_resolution" name="knob" value="change_resolution">
+        <label for="knob_change_resolution">
           Change <a href="page.cgi?id=fields.html#resolution">resolution</a> to
         </label>
-        [% PROCESS select_resolution %]
-
-        [% PROCESS duplicate %]
-
-        <input type="radio" id="knob-reopen" name="knob" value="reopen">
-        <label for="knob-reopen">
-          Reopen [% terms.bug %]
-        </label>
-        <br>
-        [% knum = knum + 1 %]
-      [% END %]
-      [% IF bug.bug_status == "RESOLVED" %]
-        [% PROCESS initial_action %]
-        <input type="radio" id="knob-verify" name="knob" value="verify">
-        <label for="knob-verify">
-          Mark [% terms.bug %] as <b>[% get_status("VERIFIED") FILTER html %]</b>
-        </label>
+        [% PROCESS select_resolution field = "knob_change_resolution" %]
         <br>
-        [% knum = knum + 1 %]
-      [% END %]
-      [% IF bug.bug_status != "CLOSED" %]
-        [% PROCESS initial_action %]
-        <input type="radio" id="knob-close" name="knob" value="close">
-        <label for="knob-close">
-          Mark [% terms.bug %] as <b>[% get_status("CLOSED") FILTER html %]</b>
-        </label>
-        <br>
-        [% knum = knum + 1 %]
       [% END %]
     [% END %]
+
+    [% PROCESS duplicate %]
   [% END %]
   </div>
 
   <div id="knob-buttons">
-  <input type="submit" value="Commit" id="commit">
+    <input type="submit" value="Commit" id="commit">
     [% IF bug.user.canmove %]
-      &nbsp; <font size="+1"><b> | </b></font> &nbsp;
-      <input type="submit" name="action" id="action"
-             value="[% Param("move-button-text") %]">
+      <input type="submit" name="action" id="action" value="[% Param("move-button-text") %]">
     [% END %]
   </div>
 </div>
@@ -143,23 +102,20 @@
 [% END %]
 
 [% BLOCK select_resolution %]
-  <select name="resolution"
-          onchange="document.changeform.knob[[% knum %]].checked=true">
+  <select name="resolution_[% field FILTER html %]"
+          onchange="document.forms['changeform'].[% field FILTER html %].checked=true">
     [% FOREACH r = bug.choices.resolution %]
       <option value="[% r FILTER html %]">[% get_resolution(r) FILTER html %]</option>
     [% END %]
   </select>
-  <br>
-  [% knum = knum + 1 %]
 [% END %]
 
 [% BLOCK duplicate %]
-  <input type="radio" id="knob-duplicate" name="knob" value="duplicate">
-  <label for="knob-duplicate">
+  <input type="radio" id="knob_duplicate" name="knob" value="duplicate">
+  <label for="knob_duplicate">
     Mark the [% terms.bug %] as duplicate of [% terms.bug %] #
   </label>
   <input name="dup_id" size="6"
-         onchange="if (this.value != '') {document.changeform.knob[[% knum %]].checked=true}">
+         onchange="if (this.value != '') {document.forms['changeform'].knob_duplicate.checked=true}">
   <br>
-  [% knum = knum + 1 %]
 [% END %]

template/en/default/filterexceptions.pl

@@ -191,8 +191,7 @@
 
 'list/edit-multiple.html.tmpl' => [
   'group.id', 
-  'knum', 
-  'menuname', 
+  'menuname',
 ],
 
 'list/list.rdf.tmpl' => [
@@ -319,10 +318,6 @@
   'flag.status',
 ],
 
-'bug/knob.html.tmpl' => [
-  'knum', 
-],
-
 'bug/navigate.html.tmpl' => [
   'bug_list.first', 
   'bug_list.last', 

template/en/default/global/user-error.html.tmpl

@@ -243,7 +243,13 @@
 
   [% ELSIF error == "comment_required" %]
     [% title = "Comment Required" %]
-    You have to specify a <b>comment</b> on this change.
+    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 %].
+    [% ELSE %]
+      on this change.
+    [% END %]
     Please explain your change.
 
   [% ELSIF error == "comment_too_long" %]
@@ -633,7 +639,12 @@
     [% title = "Your Search Makes No Sense" %]
     The only legal values for the <em>Attachment is patch</em> field are
     0 and 1.
-         
+
+  [% 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 %].
+
   [% ELSIF error == "illegal_change" %]
     [% title = "Not allowed" %]
     You tried to change the 

template/en/default/list/edit-multiple.html.tmpl

@@ -18,6 +18,7 @@
   #
   # Contributor(s): Myk Melez <myk@mozilla.org>
   #                 Max Kanat-Alexander <mkanat@bugzilla.org>
+  #                 Frédéric Buclin <LpSolit@gmail.com>
   #%]
 
 [% PROCESS global/variables.none.tmpl %]
@@ -301,66 +302,25 @@
 
 
 
-[% knum = 0 %]
 <input id="knob-none" type="radio" name="knob" value="none" checked="checked">
 <label for="knob-none">Do nothing else</label><br>
 
-[% IF bugstatuses.size == 1 && bugstatuses.0 == unconfirmedstate %]
-  [% knum = knum + 1 %]
-  <input id="knob-confirm" type="radio" name="knob" value="confirm">
-  <label for="knob-confirm">
-    Confirm [% terms.bugs %] (change status to <b>[% get_status("NEW") FILTER html %]</b>)
-  </label><br>
+[% FOREACH bug_status = new_bug_statuses %]
+    <input type="radio" id="knob_[% bug_status.id FILTER html %]" name="knob"
+           value="[% bug_status.name FILTER html %]">
+    <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}" %]
+    [% END %]
+    <br>
 [% END %]
 
-[%# If all the bugs being changed are open, allow the user to accept them,
-    clear their resolution or resolve them. %]
-[% IF !bugstatuses.containsany(closedstates) %]
-  [% knum = knum + 1 %]
-  <input id="knob-accept" type="radio" name="knob" value="accept">
-  <label for="knob-accept">
-    Accept [% terms.bugs %] (change status to <b>[% get_status("ASSIGNED") FILTER html %]</b>)
-  </label><br>
-
-  [% knum = knum + 1 %]
+[%# If all the bugs being changed are open, allow the user to clear their resolution. %]
+[% IF !current_bug_statuses.containsany(closedstates) %]
   <input id="knob-clearresolution" type="radio" name="knob" value="clearresolution">
   <label for="knob-clearresolution">Clear the resolution</label><br>
-
-  [% knum = knum + 1 %]
-  <input id="knob-resolve" type="radio" name="knob" value="resolve">
-  <label for="knob-resolve">
-    Resolve [% terms.bugs %], changing <a href="page.cgi?id=fields.html#resolution">resolution</a> to
-  </label>
-  <select name="resolution" onchange="document.forms.changeform.knob[[% knum %]].checked=true">
-    [% FOREACH resolution = resolutions %]
-      [% NEXT IF !resolution %]
-      <option value="[% resolution FILTER html %]">
-        [% get_resolution(resolution) FILTER html %]
-      </option>
-    [% END %]
-  </select><br>
-
-[% END %]
-
-[%# If all the bugs are closed, allow the user to reopen them. %]
-[% IF !bugstatuses.containsany(openstates) %]
-  [% knum = knum + 1 %]
-  <input id="knob-reopen" type="radio" name="knob" value="reopen">
-  <label for="knob-reopen">Reopen [% terms.bugs %]</label><br>
-[% END %]
-
-[% IF bugstatuses.size == 1 %]
-  [% IF bugstatuses.contains('RESOLVED') %]
-    [% knum = knum + 1 %]
-    <input id="knob-verify" type="radio" name="knob" value="verify">
-    <label for="knob-verify">Mark [% terms.bugs %] as <b>[% get_status("VERIFIED") FILTER html %]</b></label><br>
-  [% END %]
-[% END %]
-
-[% IF !bugstatuses.containsany(openstates) AND !bugstatuses.contains('CLOSED') %]
-  [% knum = knum + 1 %]
-  <input id="knob-close" type="radio" name="knob" value="close">
-  <label for="knob-close">Mark [% terms.bugs %] as <b>[% get_status("CLOSED") FILTER html %]</b></label><br>
 [% END %]
 
 <input type="submit" id="commit" value="Commit">
@@ -384,3 +344,13 @@
     [% END %]
   </select>
 [% END %]
+
+[% BLOCK select_resolution %]
+  <select name="resolution"
+          onchange="document.forms['changeform'].[% field FILTER html %].checked=true">
+    [% FOREACH r = resolutions %]
+      [% NEXT IF !r %]
+      <option value="[% r FILTER html %]">[% get_resolution(r) FILTER html %]</option>
+    [% END %]
+  </select>
+[% END %]