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

8 files changed, 63 insertions, 96 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],
diff --git a/checksetup.pl b/checksetup.pl
index 81eab9b87..6d2037b9e 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -155,7 +155,7 @@ create_htaccess() if $lc_hash->{'create_htaccess'};
 
 # Remove parameters from the params file that no longer exist in Bugzilla,
 # and set the defaults for new ones
-update_params();
+my %old_params = update_params();
 
 ###########################################################################
 # Pre-compile --TEMPLATE-- code
@@ -193,7 +193,7 @@ Bugzilla::Field::populate_field_definitions();
 # Update the tables to the current definition --TABLE--
 ###########################################################################
 
-Bugzilla::Install::DB::update_table_definitions();        
+Bugzilla::Install::DB::update_table_definitions(\%old_params);
 
 ###########################################################################
 # Bugzilla uses --GROUPS-- to assign various rights to its users.
diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index c1444ac82..ab44a3a79 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -201,7 +201,7 @@ if ($cgi->param('repair_creation_date')) {
                                         WHERE bug_id = ?');
 
     # All bugs have an entry in the 'longdescs' table when they are created,
-    # even if 'commentoncreate' is turned off.
+    # even if no comment is required.
     my $sth_getDate = $dbh->prepare('SELECT MIN(bug_when) FROM longdescs
                                      WHERE bug_id = ?');
 
diff --git a/template/en/default/admin/admin.html.tmpl b/template/en/default/admin/admin.html.tmpl
index 15316ed64..0555cf0f5 100644
--- a/template/en/default/admin/admin.html.tmpl
+++ b/template/en/default/admin/admin.html.tmpl
@@ -75,18 +75,18 @@
         can also edit some specific attributes of products such as
         <a href="editcomponents.cgi">components</a>, <a href="editversions.cgi">versions</a>
         and <a href="editmilestones.cgi">milestones</a> directly.</dd>
-      </dl>
-    </td>
 
-    <td class="admin_links">
-      <dl>
         [% class = user.groups.editcomponents ? "" : "forbidden" %]
         <dt class="[% class %]"><a href="editflagtypes.cgi">Flags</a></dt>
         <dd class="[% class %]">A flag is a custom 4-states attribute of [% terms.bugs %]
         and/or attachments. These states are: granted, denied, requested and undefined.
         You can set as many flags as desired per [% terms.bug %], and define which users
         are allowed to edit them.</dd>
+      </dl>
+    </td>
 
+    <td class="admin_links">
+      <dl>
         [% class = user.groups.admin ? "" : "forbidden" %]
         <dt class="[% class %]"><a href="editfields.cgi">Custom Fields</a></dt>
         <dd class="[% class %]">[% terms.Bugzilla %] lets you define fields which are
@@ -102,6 +102,11 @@
         to some given list. This is also the place where you define legal values for some
         types of custom fields.</dd>
 
+        <dt class="[% class %]"><a href="editworkflow.cgi">[%terms.Bug %] Status Workflow</a></dt>
+        <dd class="[% class %]">Customize your workflow and choose initial [% terms.bug %]
+        statuses available on [% terms.bug %] creation and allowed [% terms.bug %] status
+        transitions when editing existing [% terms.bugs %].</dd>
+
         [% class = user.groups.creategroups ? "" : "forbidden" %]
         <dt class="[% class %]"><a href="editgroups.cgi">Groups</a></dt>
         <dd class="[% class %]">Define groups which will be used in the installation.
diff --git a/template/en/default/admin/params/bugchange.html.tmpl b/template/en/default/admin/params/bugchange.html.tmpl
index b9475d41c..022e0cac1 100644
--- a/template/en/default/admin/params/bugchange.html.tmpl
+++ b/template/en/default/admin/params/bugchange.html.tmpl
@@ -38,36 +38,15 @@
   musthavemilestoneonaccept => "If you are using Target Milestone, do you want to require that " _
                                "the milestone be set in order for a user to ACCEPT a ${terms.bug}?",
 
-  commentoncreate => "If this option is on, the user needs to enter a description " _
-                     "when entering a new ${terms.bug}.",
-
-  commentonaccept => "If this option is on, the user needs to enter a short comment if " _
-                     "he accepts the ${terms.bug}.",
-
   commentonclearresolution => "If this option is on, the user needs to enter a short comment if " _
                               "the ${terms.bug}'s resolution is cleared.",
 
-  commentonconfirm => "If this option is on, the user needs to enter a short comment " _
-                      "when confirming a ${terms.bug}.",
-
-  commentonresolve => "If this option is on, the user needs to enter a short comment if " _
-                      "the $terms.bug is resolved.",
-
-  commentonreassign => "If this option is on, the user needs to enter a short comment if " _
-                       "the $terms.bug is reassigned.",
+  commentonchange_resolution => "If this option is on, the user needs to enter a short " _
+                                "comment if the resolution of the $terms.bug changes.",
 
   commentonreassignbycomponent => "If this option is on, the user needs to enter a short comment if " _
                                   "the $terms.bug is reassigned by component.",
 
-  commentonreopen => "If this option is on, the user needs to enter a short comment if " _
-                     "the $terms.bug is reopened.",
-
-  commentonverify => "If this option is on, the user needs to enter a short comment if " _
-                     "the $terms.bug is verified.",
-
-  commentonclose => "If this option is on, the user needs to enter a short comment if " _
-                    "the $terms.bug is closed.",
-
   commentonduplicate => "If this option is on, the user needs to enter a short comment " _
                         "if the $terms.bug is marked as duplicate.",