Bug 512648 - Make Bugzilla::Bug centrally control available statuses in

enter_bug.cgi
r=mkanat, a=mkanat

2 files changed, 76 insertions, 34 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index b2afe6972..ed77cc50d 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1283,10 +1283,7 @@ sub _check_bug_status {
     else {
         $product = $params->{product};
         $comment = $params->{comment};
-        @valid_statuses = @{Bugzilla::Status->can_change_to()};
-        if (!$product->allows_unconfirmed) {
-            @valid_statuses = grep {$_->name ne 'UNCONFIRMED'} @valid_statuses;
-        }
+        @valid_statuses = @{ Bugzilla::Bug->statuses_available($product) };
     }
 
     # Check permissions for users filing new bugs.
@@ -3226,6 +3223,24 @@ sub classification {
     return $self->{classification};
 }
 
+sub default_bug_status {
+    my $class = shift;
+    # XXX This should just call new_bug_statuses when the UI accepts closed
+    # bug statuses instead of accepting them as a parameter.
+    my @statuses = @_;
+
+    my $status;
+    if (scalar(@statuses) == 1) {
+        $status = $statuses[0]->name;
+    }
+    else {
+        $status = ($statuses[0]->name ne 'UNCONFIRMED')
+                  ? $statuses[0]->name : $statuses[1]->name;
+    }
+
+    return $status;
+}
+
 sub dependson {
     my ($self) = @_;
     return $self->{'dependson'} if exists $self->{'dependson'};
@@ -3342,6 +3357,28 @@ sub comments {
     return \@comments;
 }
 
+sub new_bug_statuses {
+    my ($class, $product) = @_;
+    my $user = Bugzilla->user;
+
+    # Construct the list of allowable statuses.
+    my @statuses = @{ Bugzilla::Bug->statuses_available($product) };
+
+    # If the user has no privs...
+    unless ($user->in_group('editbugs', $product->id)
+            || $user->in_group('canconfirm', $product->id))
+    {
+        # ... use UNCONFIRMED if available, else use the first status of the list.
+        my ($unconfirmed) = grep { $_->name eq 'UNCONFIRMED' } @statuses;
+    
+        # Because of an apparent Perl bug, "$unconfirmed || $statuses[0]" doesn't
+        # work, so we're using an "?:" operator. See bug 603314 for details.
+        @statuses = ($unconfirmed ? $unconfirmed : $statuses[0]);
+    }
+
+    return \@statuses;
+}
+
 # This is needed by xt/search.t.
 sub percentage_complete {
     my $self = shift;
@@ -3422,18 +3459,40 @@ sub status {
 }
 
 sub statuses_available {
-    my $self = shift;
-    return [] if $self->{'error'};
-    return $self->{'statuses_available'}
-        if defined $self->{'statuses_available'};
+    my ($invocant, $product) = @_;
+
+    my @statuses;
+
+    if (ref $invocant) {
+      return [] if $invocant->{'error'};
 
-    my @statuses = @{ $self->status->can_change_to };
+      return $invocant->{'statuses_available'}
+          if defined $invocant->{'statuses_available'};
+
+        @statuses = @{ $invocant->status->can_change_to };
+        $product = $invocant->product_obj;
+    } else {
+        @statuses = @{ Bugzilla::Status->can_change_to };
+    }
 
     # UNCONFIRMED is only a valid status if it is enabled in this product.
-    if (!$self->product_obj->allows_unconfirmed) {
+    if (!$product->allows_unconfirmed) {
         @statuses = grep { $_->name ne 'UNCONFIRMED' } @statuses;
     }
 
+    if (ref $invocant) {
+        my $available = $invocant->_refine_available_statuses(@statuses);
+        $invocant->{'statuses_available'} = $available;
+        return $available;
+    }
+
+    return \@statuses;
+}
+
+sub _refine_available_statuses {
+    my $self = shift;
+    my @statuses = @_;
+
     my @available;
     foreach my $status (@statuses) {
         # Make sure this is a legal status transition
@@ -3446,9 +3505,8 @@ sub statuses_available {
     if (!grep($_->name eq $self->status->name, @available)) {
         unshift(@available, $self->status);
     }
-
-    $self->{'statuses_available'} = \@available;
-    return $self->{'statuses_available'};
+    
+    return \@available;
 }
 
 sub show_attachment_flags {
diff --git a/enter_bug.cgi b/enter_bug.cgi
index 497209df4..4ef886741 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -342,27 +342,15 @@ if ( Bugzilla->params->{'usetargetmilestone'} ) {
 }
 
 # Construct the list of allowable statuses.
-my @statuses = @{ Bugzilla::Status->can_change_to() };
+my @statuses = @{ Bugzilla::Bug->new_bug_statuses($product) };
 # Exclude closed states from the UI, even if the workflow allows them.
 # The back-end code will still accept them, though.
+# XXX We should remove this when the UI accepts closed statuses and update
+# Bugzilla::Bug->default_bug_status.
 @statuses = grep { $_->is_open } @statuses;
 
-# UNCONFIRMED is illegal if allows_unconfirmed is false.
-if (!$product->allows_unconfirmed) {
-    @statuses = grep { $_->name ne 'UNCONFIRMED' } @statuses;
-}
 scalar(@statuses) || 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 ($unconfirmed) = grep { $_->name eq 'UNCONFIRMED' } @statuses;
-
-    # Because of an apparent Perl bug, "$unconfirmed || $statuses[0]" doesn't
-    # work, so we're using an "?:" operator. See bug 603314 for details.
-    @statuses = ($unconfirmed ? $unconfirmed : $statuses[0]);
-}
-
 $vars->{'bug_status'} = \@statuses;
 
 # Get the default from a template value if it is legitimate.
@@ -372,12 +360,8 @@ $vars->{'bug_status'} = \@statuses;
 my $picked_status = formvalue('bug_status');
 if ($picked_status and grep($_->name eq $picked_status, @statuses)) {
     $default{'bug_status'} = formvalue('bug_status');
-} elsif (scalar @statuses == 1) {
-    $default{'bug_status'} = $statuses[0]->name;
-}
-else {
-    $default{'bug_status'} = ($statuses[0]->name ne 'UNCONFIRMED') 
-                             ? $statuses[0]->name : $statuses[1]->name;
+} else {
+    $default{'bug_status'} = Bugzilla::Bug->default_bug_status(@statuses);
 }
 
 my @groups = $cgi->param('groups');