Bug 385415: Bugs marked as duplicate or moved to another installation always go to the RESOLVED state, even if the workflow has RESOLVED excluded from it (or if this bug status has been removed or renamed). Some major problems related to the workflow when upgrading or installing 3.1 are also fixed here - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=mkanat

5 files changed, 176 insertions, 74 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 3ace277dd..da9e9a6b3 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1986,10 +1986,10 @@ sub get_new_status_and_resolution {
         # Leaving the status unchanged doesn't need more investigation.
         return ($self->bug_status, $self->resolution, $self->everconfirmed);
     }
-    elsif ($action eq 'duplicate') {
-        # Only alter the bug status if the bug is currently open.
-        $status = is_open_state($self->bug_status) ? 'RESOLVED' : $self->bug_status;
-        $resolution = 'DUPLICATE';
+    elsif ($action eq 'duplicate' || $action eq 'move') {
+        # Always change the bug status, even if the bug was already "closed".
+        $status = Bugzilla->params->{'duplicate_or_move_bug_status'};
+        $resolution = ($action eq 'duplicate') ? 'DUPLICATE' : 'MOVED';
     }
     elsif ($action eq 'change_resolution') {
         $status = $self->bug_status;
diff --git a/Bugzilla/Config/BugChange.pm b/Bugzilla/Config/BugChange.pm
index 6941f0046..65b2aec96 100644
--- a/Bugzilla/Config/BugChange.pm
+++ b/Bugzilla/Config/BugChange.pm
@@ -34,13 +34,35 @@ package Bugzilla::Config::BugChange;
 use strict;
 
 use Bugzilla::Config::Common;
+use Bugzilla::Status;
 
 $Bugzilla::Config::BugChange::sortkey = "03";
 
 sub get_param_list {
   my $class = shift;
+
+  # Hardcoded bug statuses which existed before Bugzilla 3.1.
+  my @closed_bug_statuses = ('RESOLVED', 'VERIFIED', 'CLOSED');
+
+  # If we are upgrading from 3.0 or older, bug statuses are not customisable
+  # and bug_status.is_open is not yet defined (hence the eval), so we use
+  # the bug statuses above as they are still hardcoded.
+  eval {
+      my @current_closed_states = map {$_->name} Bugzilla::Status::closed_bug_statuses();
+      # If no closed state was found, use the default list above.
+      @closed_bug_statuses = @current_closed_states if scalar(@current_closed_states);
+  };
+
   my @param_list = (
   {
+   name => 'duplicate_or_move_bug_status',
+   type => 's',
+   choices => \@closed_bug_statuses,
+   default => $closed_bug_statuses[0],
+   checker => \&check_bug_status
+  },
+
+  {
    name => 'letsubmitterchoosepriority',
    type => 'b',
    default => 1
diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm
index 0d6db5257..188ef0c90 100644
--- a/Bugzilla/Config/Common.pm
+++ b/Bugzilla/Config/Common.pm
@@ -40,6 +40,7 @@ use Bugzilla::Util;
 use Bugzilla::Constants;
 use Bugzilla::Field;
 use Bugzilla::Group;
+use Bugzilla::Status;
 
 use base qw(Exporter);
 @Bugzilla::Config::Common::EXPORT =
@@ -48,7 +49,7 @@ use base qw(Exporter);
        check_opsys check_shadowdb check_urlbase check_webdotbase
        check_netmask check_user_verify_class check_image_converter
        check_languages check_mail_delivery_method check_notification
-       check_timezone check_utf8
+       check_timezone check_utf8 check_bug_status
 );
 
 # Checking functions for the various values
@@ -166,6 +167,15 @@ sub check_opsys {
     return "";
 }
 
+sub check_bug_status {
+    my $bug_status = shift;
+    my @closed_bug_statuses = map {$_->name} Bugzilla::Status::closed_bug_statuses();
+    if (lsearch(\@closed_bug_statuses, $bug_status) < 0) {
+        return "Must be a valid closed status: one of " . join(', ', @closed_bug_statuses);
+    }
+    return "";
+}
+
 sub check_group {
     my $group_name = shift;
     return "" unless $group_name;
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 3991cd453..4b4e7a068 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -2783,85 +2783,102 @@ sub _initialize_workflow {
     my $old_params = shift;
     my $dbh = Bugzilla->dbh;
 
-    if (!$dbh->bz_column_info('bug_status', 'is_open')) {
-        $dbh->bz_add_column('bug_status', 'is_open',
-                            {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'TRUE'});
-
-        # Till now, bug statuses were not customizable. Nevertheless, local
-        # changes are possible and so we will try to respect these changes.
-        # This means: get the status of bugs having a resolution different from ''
-        # and mark these statuses as 'closed', even if some of these statuses are
-        # expected to be open statuses. Bug statuses we have no information about
-        # are left as 'open'.
-        my @statuses =
-          @{$dbh->selectcol_arrayref('SELECT DISTINCT bug_status FROM bugs
-                                      WHERE resolution != ?', undef, '')};
+    $dbh->bz_add_column('bug_status', 'is_open',
+                        {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'TRUE'});
 
-        # Append the default list of closed statuses. Duplicated statuses don't hurt.
-        @statuses = map {$dbh->quote($_)} (@statuses, qw(RESOLVED VERIFIED CLOSED));
+    # Till now, bug statuses were not customizable. Nevertheless, local
+    # changes are possible and so we will try to respect these changes.
+    # This means: get the status of bugs having a resolution different from ''
+    # and mark these statuses as 'closed', even if some of these statuses are
+    # expected to be open statuses. Bug statuses we have no information about
+    # are left as 'open'.
+    my @closed_statuses =
+      @{$dbh->selectcol_arrayref('SELECT DISTINCT bug_status FROM bugs
+                                  WHERE resolution != ?', undef, '')};
+
+    # Append the default list of closed statuses *unless* we detect at least
+    # one closed state in the DB (i.e. with is_open = 0). This would mean that
+    # the DB has already been updated at least once and maybe the admin decided
+    # that e.g. 'RESOLVED' is now an open state, in which case we don't want to
+    # override this attribute. At least one bug status has to be a closed state
+    # anyway (due to the 'duplicate_or_move_bug_status' parameter) so it's safe
+    # to use this criteria.
+    my $num_closed_states = $dbh->selectrow_array('SELECT COUNT(*) FROM bug_status
+                                                   WHERE is_open = 0');
+
+    if (!$num_closed_states) {
+        @closed_statuses =
+          map {$dbh->quote($_)} (@closed_statuses, qw(RESOLVED VERIFIED CLOSED));
 
         print "Marking closed bug statuses as such...\n";
         $dbh->do('UPDATE bug_status SET is_open = 0 WHERE value IN (' .
-                  join(', ', @statuses) . ')');
+                  join(', ', @closed_statuses) . ')');
     }
 
     # Populate the status_workflow table. We do nothing if the table already
     # has entries. If all bug status transitions have been deleted, the
     # workflow will be restored to its default schema.
     my $count = $dbh->selectrow_array('SELECT COUNT(*) FROM status_workflow');
-    return if $count;
-
-    my $create = $old_params->{'commentoncreate'};
-    my $confirm = $old_params->{'commentonconfirm'};
-    my $accept = $old_params->{'commentonaccept'};
-    my $resolve = $old_params->{'commentonresolve'};
-    my $verify = $old_params->{'commentonverify'};
-    my $close = $old_params->{'commentonclose'};
-    my $reopen = $old_params->{'commentonreopen'};
-    # This was till recently the only way to get back to NEW for
-    # confirmed bugs, so we use this parameter here.
-    my $reassign = $old_params->{'commentonreassign'};
-
-    # This is the default workflow.
-    my @workflow = ([undef, 'UNCONFIRMED', $create],
-                    [undef, 'NEW', $create],
-                    [undef, 'ASSIGNED', $create],
-                    ['UNCONFIRMED', 'NEW', $confirm],
-                    ['UNCONFIRMED', 'ASSIGNED', $accept],
-                    ['UNCONFIRMED', 'RESOLVED', $resolve],
-                    ['NEW', 'ASSIGNED', $accept],
-                    ['NEW', 'RESOLVED', $resolve],
-                    ['ASSIGNED', 'NEW', $reassign],
-                    ['ASSIGNED', 'RESOLVED', $resolve],
-                    ['REOPENED', 'NEW', $reassign],
-                    ['REOPENED', 'ASSIGNED', $accept],
-                    ['REOPENED', 'RESOLVED', $resolve],
-                    ['RESOLVED', 'UNCONFIRMED', $reopen],
-                    ['RESOLVED', 'REOPENED', $reopen],
-                    ['RESOLVED', 'VERIFIED', $verify],
-                    ['RESOLVED', 'CLOSED', $close],
-                    ['VERIFIED', 'UNCONFIRMED', $reopen],
-                    ['VERIFIED', 'REOPENED', $reopen],
-                    ['VERIFIED', 'CLOSED', $close],
-                    ['CLOSED', 'UNCONFIRMED', $reopen],
-                    ['CLOSED', 'REOPENED', $reopen]);
-
-    print "Now filling the 'status_workflow' table with valid bug status transitions...\n";
-    my $sth_select = $dbh->prepare('SELECT id FROM bug_status WHERE value = ?');
-    my $sth = $dbh->prepare('INSERT INTO status_workflow (old_status, new_status,
-                                         require_comment) VALUES (?, ?, ?)');
-
-    foreach my $transition (@workflow) {
-        my ($from, $to);
-        # If it's an initial state, there is no "old" value.
-        $from = $dbh->selectrow_array($sth_select, undef, $transition->[0])
-          if $transition->[0];
-        $to = $dbh->selectrow_array($sth_select, undef, $transition->[1]);
-        # If one of the bug statuses doesn't exist, the transition is invalid.
-        next if (($transition->[0] && !$from) || !$to);
-
-        $sth->execute($from, $to, $transition->[2] ? 1 : 0);
+
+    if (!$count) {
+        # Make sure the variables below are defined as
+        # status_workflow.require_comment cannot be NULL.
+        my $create = $old_params->{'commentoncreate'} || 0;
+        my $confirm = $old_params->{'commentonconfirm'} || 0;
+        my $accept = $old_params->{'commentonaccept'} || 0;
+        my $resolve = $old_params->{'commentonresolve'} || 0;
+        my $verify = $old_params->{'commentonverify'} || 0;
+        my $close = $old_params->{'commentonclose'} || 0;
+        my $reopen = $old_params->{'commentonreopen'} || 0;
+        # This was till recently the only way to get back to NEW for
+        # confirmed bugs, so we use this parameter here.
+        my $reassign = $old_params->{'commentonreassign'} || 0;
+
+        # This is the default workflow.
+        my @workflow = ([undef, 'UNCONFIRMED', $create],
+                        [undef, 'NEW', $create],
+                        [undef, 'ASSIGNED', $create],
+                        ['UNCONFIRMED', 'NEW', $confirm],
+                        ['UNCONFIRMED', 'ASSIGNED', $accept],
+                        ['UNCONFIRMED', 'RESOLVED', $resolve],
+                        ['NEW', 'ASSIGNED', $accept],
+                        ['NEW', 'RESOLVED', $resolve],
+                        ['ASSIGNED', 'NEW', $reassign],
+                        ['ASSIGNED', 'RESOLVED', $resolve],
+                        ['REOPENED', 'NEW', $reassign],
+                        ['REOPENED', 'ASSIGNED', $accept],
+                        ['REOPENED', 'RESOLVED', $resolve],
+                        ['RESOLVED', 'UNCONFIRMED', $reopen],
+                        ['RESOLVED', 'REOPENED', $reopen],
+                        ['RESOLVED', 'VERIFIED', $verify],
+                        ['RESOLVED', 'CLOSED', $close],
+                        ['VERIFIED', 'UNCONFIRMED', $reopen],
+                        ['VERIFIED', 'REOPENED', $reopen],
+                        ['VERIFIED', 'CLOSED', $close],
+                        ['CLOSED', 'UNCONFIRMED', $reopen],
+                        ['CLOSED', 'REOPENED', $reopen]);
+
+        print "Now filling the 'status_workflow' table with valid bug status transitions...\n";
+        my $sth_select = $dbh->prepare('SELECT id FROM bug_status WHERE value = ?');
+        my $sth = $dbh->prepare('INSERT INTO status_workflow (old_status, new_status,
+                                             require_comment) VALUES (?, ?, ?)');
+
+        foreach my $transition (@workflow) {
+            my ($from, $to);
+            # If it's an initial state, there is no "old" value.
+            $from = $dbh->selectrow_array($sth_select, undef, $transition->[0])
+              if $transition->[0];
+            $to = $dbh->selectrow_array($sth_select, undef, $transition->[1]);
+            # If one of the bug statuses doesn't exist, the transition is invalid.
+            next if (($transition->[0] && !$from) || !$to);
+
+            $sth->execute($from, $to, $transition->[2] ? 1 : 0);
+        }
     }
+
+    # Make sure the bug status used by the 'duplicate_or_move_bug_status'
+    # parameter has all the required transitions set.
+    Bugzilla::Status::add_missing_bug_status_transitions();
 }
 
 1;
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index e91f83871..cf8f98efa 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -54,15 +54,22 @@ sub is_open   { return $_[0]->{'is_open'};  }
 #####       Methods        ####
 ###############################
 
+sub closed_bug_statuses {
+    my @bug_statuses = Bugzilla::Status->get_all;
+    @bug_statuses = grep { !$_->is_open } @bug_statuses;
+    return @bug_statuses;
+}
+
 sub can_change_to {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
 
     if (!ref($self) || !defined $self->{'can_change_to'}) {
-        my ($cond, @args);
+        my ($cond, @args, $self_exists);
         if (ref($self)) {
             $cond = '= ?';
             push(@args, $self->id);
+            $self_exists = 1;
         }
         else {
             $cond = 'IS NULL';
@@ -78,12 +85,37 @@ sub can_change_to {
                                                           AND old_status $cond",
                                                         undef, @args);
 
+        # Allow the bug status to remain unchanged.
+        push(@$new_status_ids, $self->id) if $self_exists;
         $self->{'can_change_to'} = Bugzilla::Status->new_from_list($new_status_ids);
     }
 
     return $self->{'can_change_to'};
 }
 
+sub add_missing_bug_status_transitions {
+    my $bug_status = shift || Bugzilla->params->{'duplicate_or_move_bug_status'};
+    my $dbh = Bugzilla->dbh;
+    my $new_status = new Bugzilla::Status({name => $bug_status});
+    # Silently discard invalid bug statuses.
+    $new_status || return;
+
+    my $missing_statuses = $dbh->selectcol_arrayref('SELECT id
+                                                       FROM bug_status
+                                                  LEFT JOIN status_workflow
+                                                         ON old_status = id
+                                                        AND new_status = ?
+                                                      WHERE old_status IS NULL',
+                                                      undef, $new_status->id);
+
+    my $sth = $dbh->prepare('INSERT INTO status_workflow
+                             (old_status, new_status) VALUES (?, ?)');
+
+    foreach my $old_status_id (@$missing_statuses) {
+        next if ($old_status_id == $new_status->id);
+        $sth->execute($old_status_id, $new_status->id);
+    }
+}
 
 1;
 
@@ -100,6 +132,10 @@ Bugzilla::Status - Bug status class.
     my $bug_status = new Bugzilla::Status({name => 'ASSIGNED'});
     my $bug_status = new Bugzilla::Status(4);
 
+    my @closed_bug_statuses = Bugzilla::Status::closed_bug_statuses();
+
+    Bugzilla::Status::add_missing_bug_status_transitions($bug_status);
+
 =head1 DESCRIPTION
 
 Status.pm represents a bug status object. It is an implementation
@@ -113,6 +149,15 @@ below.
 
 =over
 
+=item C<closed_bug_statuses>
+
+ Description: Returns a list of C<Bugzilla::Status> objects which can have
+              a resolution associated with them ("closed" bug statuses).
+
+ Params:      none.
+
+ Returns:     A list of Bugzilla::Status objects.
+
 =item C<can_change_to>
 
  Description: Returns the list of active statuses a bug can be changed to
@@ -122,6 +167,14 @@ below.
 
  Returns:     A list of Bugzilla::Status objects.
 
+=item C<add_missing_bug_status_transitions>
+
+ Description: Insert all missing transitions to a given bug status.
+
+ Params:      $bug_status - The value (name) of a bug status.
+
+ Returns:     nothing.
+
 =back
 
 =cut