Bug 345100: Remove obsolete parameters and "officially" turn on custom bug status workflow - Patch by Frédéric Buclin <LpSolit@gmail.com> r=gerv a=justdave

4 files changed, 49 insertions, 66 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 5ca806092..c3be53e96 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -378,6 +378,11 @@ sub run_create_validators {
     ($params->{bug_status}, $params->{everconfirmed})
         = $class->_check_bug_status($params->{bug_status}, $product);
 
+    # Check whether a comment is required on bug creation.
+    my $vars = {};
+    $vars->{comment_exists} = ($params->{comment} =~ /\S+/) ? 1 : 0;
+    Bugzilla::Bug->check_status_change_triggers($params->{bug_status}, [], $vars);
+
     $params->{target_milestone} = $class->_check_target_milestone($product,
         $params->{target_milestone});
 
@@ -653,10 +658,6 @@ sub _check_comment {
 
     # Creation-only checks
     if (!ref $invocant) {
-        if (Bugzilla->params->{"commentoncreate"} && !$comment) {
-            ThrowUserError("description_required");
-        }
-
         # On creation only, there must be a single-space comment, or
         # email will be supressed.
         $comment = ' ' if $comment eq '';
@@ -1588,6 +1589,19 @@ sub check_status_change_triggers {
             # 'commentonnone' doesn't exist, so this is safe.
             ThrowUserError('comment_required') if Bugzilla->params->{"commenton$action"};
         }
+        elsif (!scalar(@$bug_ids)) {
+            # The bug is being created; that's why $bug_ids is undefined.
+            my $comment_required =
+              $dbh->selectrow_array('SELECT require_comment
+                                       FROM status_workflow
+                                 INNER JOIN bug_status
+                                         ON id = new_status
+                                      WHERE old_status IS NULL
+                                        AND value = ?',
+                                      undef, $action);
+
+            ThrowUserError('description_required') if $comment_required;
+        }
         else {
             my $required_for_transitions =
               $dbh->selectcol_arrayref('SELECT DISTINCT bug_status.value
@@ -1614,6 +1628,10 @@ sub check_status_change_triggers {
     # There is no checks for these actions.
     return if ($action eq 'none' || $action eq 'clearresolution');
 
+    # Also leave now if we are creating a new bug (we only want to check
+    # if a comment is required on bug creation).
+    return unless scalar(@$bug_ids);
+
     if ($action eq 'duplicate') {
         # You cannot mark bugs as duplicates when changing
         # several bugs at once.
diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm
index 85c9d6dec..fe58e0282 100644
--- a/Bugzilla/Config.pm
+++ b/Bugzilla/Config.pm
@@ -199,18 +199,16 @@ sub update_params {
 
     # --- REMOVE OLD PARAMS ---
 
-    my @oldparams;
+    my %oldparams;
     # Remove any old params, put them in old-params.txt
     foreach my $item (keys %$param) {
         if (!grep($_ eq $item, map ($_->{'name'}, @param_list))) {
-            local $Data::Dumper::Terse  = 1;
-            local $Data::Dumper::Indent = 0;
-            push (@oldparams, [$item, Data::Dumper->Dump([$param->{$item}])]);
+            $oldparams{$item} = $param->{$item};
             delete $param->{$item};
         }
     }
 
-    if (@oldparams) {
+    if (scalar(keys %oldparams)) {
         my $op_file = new IO::File('old-params.txt', '>>', 0600)
           || die "old-params.txt: $!";
 
@@ -218,11 +216,14 @@ sub update_params {
               " and so have been\nmoved from your parameters file into",
               " old-params.txt:\n";
 
-        foreach my $p (@oldparams) {
-            my ($item, $value) = @$p;
-            print $op_file "\n\n$item:\n$value\n";
-            print $item;
-            print ", " unless $item eq $oldparams[$#oldparams]->[0];
+        local $Data::Dumper::Terse  = 1;
+        local $Data::Dumper::Indent = 0;
+
+        my $comma = "";
+        foreach my $item (keys %oldparams) {
+            print $op_file "\n\n$item:\n" . Data::Dumper->Dump([$oldparams{$item}]) . "\n";
+            print "${comma}$item";
+            $comma = ", ";
         }
         print "\n";
         $op_file->close;
@@ -250,6 +251,10 @@ sub update_params {
     }
 
     write_params($param);
+
+    # Return deleted params and values so that checksetup.pl has a chance
+    # to convert old params to new data.
+    return %oldparams;
 }
 
 sub write_params {
diff --git a/Bugzilla/Config/BugChange.pm b/Bugzilla/Config/BugChange.pm
index bacacc082..6941f0046 100644
--- a/Bugzilla/Config/BugChange.pm
+++ b/Bugzilla/Config/BugChange.pm
@@ -59,37 +59,13 @@ sub get_param_list {
   },
 
   {
-   name => 'commentoncreate',
-   type => 'b',
-   default => 0
-  },
-
-  {
-   name => 'commentonaccept',
-   type => 'b',
-   default => 0
-  },
-
-  {
    name => 'commentonclearresolution',
    type => 'b',
    default => 0
   },
 
   {
-   name => 'commentonconfirm',
-   type => 'b',
-   default => 0
-  },
-
-  {
-   name => 'commentonresolve',
-   type => 'b',
-   default => 0
-  },
-
-  {
-   name => 'commentonreassign',
+   name => 'commentonchange_resolution',
    type => 'b',
    default => 0
   },
@@ -101,24 +77,6 @@ sub get_param_list {
   },
 
   {
-   name => 'commentonreopen',
-   type => 'b',
-   default => 0
-  },
-
-  {
-   name => 'commentonverify',
-   type => 'b',
-   default => 0
-  },
-
-  {
-   name => 'commentonclose',
-   type => 'b',
-   default => 0
-  },
-
-  {
    name => 'commentonduplicate',
    type => 'b',
    default => 0
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 7dd5ad269..3991cd453 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -115,6 +115,7 @@ sub update_fielddefs_definition {
 # the purpose of a column.
 #
 sub update_table_definitions {
+    my $old_params = shift;
     my $dbh = Bugzilla->dbh;
     _update_pre_checksetup_bugzillas();
 
@@ -507,7 +508,7 @@ sub update_table_definitions {
     _fix_uppercase_index_names();
 
     # 2007-05-17 LpSolit@gmail.com - Bug 344965
-    _initialize_workflow();
+    _initialize_workflow($old_params);
 
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
@@ -2779,6 +2780,7 @@ sub _fix_uppercase_index_names {
 }
 
 sub _initialize_workflow {
+    my $old_params = shift;
     my $dbh = Bugzilla->dbh;
 
     if (!$dbh->bz_column_info('bug_status', 'is_open')) {
@@ -2809,16 +2811,16 @@ sub _initialize_workflow {
     my $count = $dbh->selectrow_array('SELECT COUNT(*) FROM status_workflow');
     return if $count;
 
-    my $create = Bugzilla->params->{'commentoncreate'};
-    my $confirm = Bugzilla->params->{'commentonconfirm'};
-    my $accept = Bugzilla->params->{'commentonaccept'};
-    my $resolve = Bugzilla->params->{'commentonresolve'};
-    my $verify = Bugzilla->params->{'commentonverify'};
-    my $close = Bugzilla->params->{'commentonclose'};
-    my $reopen = Bugzilla->params->{'commentonreopen'};
+    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 = Bugzilla->params->{'commentonreassign'};
+    my $reassign = $old_params->{'commentonreassign'};
 
     # This is the default workflow.
     my @workflow = ([undef, 'UNCONFIRMED', $create],