Bug 512606: Make statuses currently available for the user to change this bug to be controlled by Bugzilla::Bug, not the template

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

2 files changed, 57 insertions, 56 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 83e95aecd..2c3a11e8c 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1110,7 +1110,7 @@ sub _check_bug_status {
     my $old_status; # Note that this is undef for new bugs.
 
     if (ref $invocant) {
-        @valid_statuses = @{$invocant->status->can_change_to};
+        @valid_statuses = @{$invocant->statuses_available};
         $product = $invocant->product_obj;
         $old_status = $invocant->status;
         my $comments = $invocant->{added_comments} || [];
@@ -1118,12 +1118,11 @@ sub _check_bug_status {
     }
     else {
         @valid_statuses = @{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 {$_->name ne 'UNCONFIRMED'} @valid_statuses;
+        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 {$_->name ne 'UNCONFIRMED'} @valid_statuses;
+        }
     }
 
     # Check permissions for users filing new bugs.
@@ -2167,6 +2166,8 @@ sub set_status {
     my $old_status = $self->status;
     $self->set('bug_status', $status);
     delete $self->{'status'};
+    delete $self->{'statuses_available'};
+    delete $self->{'choices'};
     my $new_status = $self->status;
     
     if ($new_status->is_open) {
@@ -2680,11 +2681,6 @@ sub isopened {
     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 join(', ', (map { $_->name } @{$self->keyword_objects}));
@@ -2805,6 +2801,31 @@ sub status {
     return $self->{'status'};
 }
 
+sub statuses_available {
+    my $self = shift;
+    return [] if $self->{'error'};
+    return $self->{'statuses_available'}
+        if defined $self->{'statuses_available'};
+
+    my @statuses = @{ $self->status->can_change_to };
+
+    # UNCONFIRMED is only a valid status if it is enabled in this product.
+    if (!$self->product_obj->votes_to_confirm) {
+        @statuses = grep { $_->name ne 'UNCONFIRMED' } @statuses;
+    }
+
+    my @available;
+    foreach my $status (@statuses) {
+        # Make sure this is a legal status transition
+        next if !$self->check_can_change_field(
+                     'bug_status', $self->status->name, $status->name);
+        push(@available, $status);
+    }
+
+    $self->{'statuses_available'} = \@available;
+    return $self->{'statuses_available'};
+}
+
 sub show_attachment_flags {
     my ($self) = @_;
     return $self->{'show_attachment_flags'} 
@@ -2958,9 +2979,10 @@ sub choices {
     }
 
     my %choices = (
-        product   => \@products,
-        component => $self->product_obj->components,
-        version   => $self->product_obj->versions,
+        bug_status => $self->statuses_available,
+        product    => \@products,
+        component  => $self->product_obj->components,
+        version    => $self->product_obj->versions,
         target_milestone => $self->product_obj->milestones,
     );
 
@@ -3451,12 +3473,7 @@ sub check_can_change_field {
     }
 
     # *Only* users with (product-specific) "canconfirm" privs can confirm bugs.
-    if ($field eq 'canconfirm'
-        || ($field eq 'everconfirmed' && $newvalue)
-        || ($field eq 'bug_status'
-            && $oldvalue eq 'UNCONFIRMED'
-            && is_open_state($newvalue)))
-    {
+    if ($self->_changes_everconfirmed($field, $oldvalue, $newvalue)) {
         $$PrivilegesRequired = 3;
         return $user->in_group('canconfirm', $self->{'product_id'});
     }
@@ -3532,6 +3549,24 @@ sub check_can_change_field {
     return 0;
 }
 
+# A helper for check_can_change_field
+sub _changes_everconfirmed {
+    my ($self, $field, $old, $new) = @_;
+    return 1 if $field eq 'everconfirmed';
+    if ($field eq 'bug_status') {
+        if ($self->everconfirmed) {
+            # Moving a confirmed bug to UNCONFIRMED will change everconfirmed.
+            return 1 if $new eq 'UNCONFIRMED';
+        }
+        else {
+            # Moving an unconfirmed bug to an open state that isn't 
+            # UNCONFIRMED will confirm the bug.
+            return 1 if (is_open_state($new) and $new ne 'UNCONFIRMED');
+        }
+    }
+    return 0;
+}
+
 #
 # Field Validation
 #
@@ -3626,8 +3661,7 @@ sub _validate_attribute {
     my @valid_attributes = (
         # Miscellaneous properties and methods.
         qw(error groups product_id component_id
-           comments milestoneurl attachments
-           isopened isunconfirmed
+           comments milestoneurl attachments isopened
            flag_types num_attachment_flag_types
            show_attachment_flags any_flags_requesteeble),
 
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index 4720dc129..4d1281e7e 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -176,28 +176,6 @@ 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 comment_required_on_change_from {
     my ($self, $old_status) = @_;
     my ($cond, $values) = $self->_status_condition($old_status);
@@ -300,17 +278,6 @@ below.
 
  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.
-
- Returns:     A list of Bugzilla::Status objects.
-
 =item C<comment_required_on_change_from>
 
 =over