Bug 344965: Fix process_bug.cgi and bug/* templates to work with custom bug status workflow - Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=LpSolit

4 files changed, 308 insertions, 48 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index dcaef8004..ce4423a27 100755
--- a/Bugzilla/Bug.pm
+++ b/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();
 
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 0e2635895..d811796d0 100644
--- a/Bugzilla/Constants.pm
+++ b/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;
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index f1a148353..7dd5ad269 100644
--- a/Bugzilla/Install/DB.pm
+++ b/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);
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
new file mode 100644
index 000000000..e83fd3533
--- /dev/null
+++ b/Bugzilla/Status.pm
@@ -0,0 +1,116 @@
+# -*- 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