Bug 344964: enter_bug.cgi should look at the custom status workflow to get the valid initial bug statuses - Patch by Frédéric Buclin <LpSolit@gmail.com> r=gerv a=justdave

4 files changed, 55 insertions, 51 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index ce4423a27..5ca806092 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -170,14 +170,6 @@ use constant MAX_LINE_LENGTH => 254;
 # Used in ValidateComment(). Gives the max length allowed for a comment.
 use constant MAX_COMMENT_LENGTH => 65535;
 
-# The statuses that are valid on enter_bug.cgi and post_bug.cgi.
-# The order is important--see _check_bug_status
-use constant VALID_ENTRY_STATUS => qw(
-    UNCONFIRMED
-    NEW
-    ASSIGNED
-);
-
 use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw(
     none
     duplicate
@@ -593,38 +585,38 @@ sub _check_bug_status {
     my ($invocant, $status, $product) = @_;
     my $user = Bugzilla->user;
 
-    my %valid_statuses;
+    my @valid_statuses;
     if (ref $invocant) {
         $invocant->{'prod_obj'} ||= 
             new Bugzilla::Product({name => $invocant->product});
         $product = $invocant->{'prod_obj'};
-        my $field = new Bugzilla::Field({ name => 'bug_status' });
-        %valid_statuses = map { $_ => 1 } @{$field->legal_values};
+        @valid_statuses = map { $_->name } @{$invocant->status->can_change_to};
     }
     else {
-        %valid_statuses = map { $_ => 1 } VALID_ENTRY_STATUS;
+        @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;
+    }
+
+    if (!ref($invocant)) {
         if ($user->in_group('editbugs', $product->id)
             || $user->in_group('canconfirm', $product->id)) {
-           # Default to NEW if the user with privs hasn't selected another
-           # status.
-           $status ||= 'NEW';
-        }
-        elsif (!$product->votes_to_confirm) {
-            # Without privs, products that don't support UNCONFIRMED default 
-            # to NEW.
-            $status = 'NEW';
+           # If the user with privs hasn't selected another status,
+           # select the first one of the list.
+           $status ||= $valid_statuses[0];
         }
         else {
-            $status = 'UNCONFIRMED';
+            # A user with no privs cannot choose the initial status.
+            $status = $valid_statuses[0];
         }
     }
 
-    # UNCONFIRMED becomes an invalid status if votes_to_confirm is 0,
-    # even if you are in editbugs.
-    delete $valid_statuses{'UNCONFIRMED'} if !$product->votes_to_confirm;
-
-    check_field('bug_status', $status, [keys %valid_statuses]);
+    # This check already takes the workflow into account.
+    check_field('bug_status', $status, \@valid_statuses);
 
     return $status if ref $invocant;
     return ($status, $status eq 'UNCONFIRMED' ? 0 : 1);
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index e83fd3533..e91f83871 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -58,14 +58,25 @@ 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
+    if (!ref($self) || !defined $self->{'can_change_to'}) {
+        my ($cond, @args);
+        if (ref($self)) {
+            $cond = '= ?';
+            push(@args, $self->id);
+        }
+        else {
+            $cond = 'IS NULL';
+            # Let's do it so that the code below works in all cases.
+            $self = {};
+        }
+
+        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);
+                                                          AND old_status $cond",
+                                                        undef, @args);
 
         $self->{'can_change_to'} = Bugzilla::Status->new_from_list($new_status_ids);
     }
diff --git a/enter_bug.cgi b/enter_bug.cgi
index c802c0096..d4ee73bfb 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -49,6 +49,7 @@ use Bugzilla::Classification;
 use Bugzilla::Keyword;
 use Bugzilla::Token;
 use Bugzilla::Field;
+use Bugzilla::Status;
 
 my $user = Bugzilla->login(LOGIN_REQUIRED);
 
@@ -494,29 +495,24 @@ if ( Bugzilla->params->{'usetargetmilestone'} ) {
 }
 
 # Construct the list of allowable statuses.
-#
-# * If the product requires votes to confirm:
-#   users with privs   : NEW + ASSI + UNCO
-#   users with no privs: UNCO
-#
-# * If the product doesn't require votes to confirm:
-#   users with privs   : NEW + ASSI
-#   users with no privs: NEW (as these users cannot reassign
-#                             bugs to them, it doesn't make sense
-#                             to let them mark bugs as ASSIGNED)
-
-my @status;
-if ($has_editbugs || $has_canconfirm) {
-    @status = ('NEW', 'ASSIGNED');
-}
-elsif (!$product->votes_to_confirm) {
-    @status = ('NEW');
-}
-if ($product->votes_to_confirm) {
-    push(@status, 'UNCONFIRMED');
+my $initial_statuses = Bugzilla::Status->can_change_to();
+# Exclude closed states from the UI, even if the workflow allows them.
+# The back-end code will still accept them, though.
+@$initial_statuses = grep { $_->is_open } @$initial_statuses;
+
+my @status = map { $_->name } @$initial_statuses;
+# UNCONFIRMED is illegal if votes_to_confirm = 0.
+@status = grep {$_ ne 'UNCONFIRMED'} @status unless $product->votes_to_confirm;
+scalar(@status) || ThrowUserError('no_initial_bug_status');
+
+# If the user has no privs...
+unless ($has_editbugs || $has_canconfirm) {
+    # ... use UNCONFIRMED if available, else use the first status of the list.
+    my $bug_status = (grep {$_ eq 'UNCONFIRMED'} @status) ? 'UNCONFIRMED' : $status[0];
+    @status = ($bug_status);
 }
 
-$vars->{'bug_status'} = \@status; 
+$vars->{'bug_status'} = \@status;
 
 # Get the default from a template value if it is legitimate.
 # Otherwise, set the default to the first item on the list.
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 4b676ab4e..e21e54948 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1084,6 +1084,11 @@
     and an error
     occurred opening yesterday's dupes file: [% error_msg FILTER html %].
 
+  [% ELSIF error == "no_initial_bug_status" %]
+    [% title = "No Initial $terms.Bug Status" %]
+    No [% terms.bug %] status is available on [% terms.bug %] creation.
+    Please report the problem to [% Param("maintainer") %].
+
   [% ELSIF error == "no_new_quips" %]
     [% title = "No New Quips" %]
     [% admindocslinks = {'quips.html' => 'Controlling quip usage'} %]