Bug 162060: Remove the relationship between "votestoconfirm" and whether or not the UNCONFIRMED status is available, by adding a checkbox to enable the UNCONFIRMED status in editproducts.cgi.

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

14 files changed, 130 insertions, 59 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index ab035fcba..1eaafb698 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1122,9 +1122,7 @@ 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.
+        if (!$product->allows_unconfirmed) {
             @valid_statuses = grep {$_->name ne 'UNCONFIRMED'} @valid_statuses;
         }
     }
@@ -1157,9 +1155,13 @@ sub _check_bug_status {
             }
         }
     }
+
     # Time to validate the bug status.
     $new_status = Bugzilla::Status->check($new_status) unless ref($new_status);
-    if (!grep {$_->name eq $new_status->name} @valid_statuses) {
+    # We skip this check if we are changing from a status to itself.
+    if ( (!$old_status || $old_status->id != $new_status->id)
+          && !grep {$_->name eq $new_status->name} @valid_statuses) 
+    {
         ThrowUserError('illegal_bug_status_transition',
                        { old => $old_status, new => $new_status });
     }
@@ -2804,7 +2806,7 @@ sub 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) {
+    if (!$self->product_obj->allows_unconfirmed) {
         @statuses = grep { $_->name ne 'UNCONFIRMED' } @statuses;
     }
 
@@ -2816,6 +2818,11 @@ sub statuses_available {
         push(@available, $status);
     }
 
+    # If this bug has an inactive status set, it should still be in the list.
+    if (!grep($_->name eq $self->status->name, @available)) {
+        unshift(@available, $self->status);
+    }
+
     $self->{'statuses_available'} = \@available;
     return $self->{'statuses_available'};
 }
@@ -3367,7 +3374,10 @@ sub CheckIfVotedConfirmed {
     my $bug = new Bugzilla::Bug($id);
 
     my $ret = 0;
-    if (!$bug->everconfirmed && $bug->votes >= $bug->product_obj->votes_to_confirm) {
+    if (!$bug->everconfirmed
+        and $bug->product_obj->votes_to_confirm
+        and $bug->votes >= $bug->product_obj->votes_to_confirm) 
+    {
         $bug->add_comment('', { type => CMT_POPULAR_VOTES });
 
         if ($bug->bug_status eq 'UNCONFIRMED') {
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index e4dcfd966..a1102dd64 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -1224,6 +1224,8 @@ use constant ABSTRACT_SCHEMA => {
                                   DEFAULT => 0},
             defaultmilestone  => {TYPE => 'varchar(20)',
                                   NOTNULL => 1, DEFAULT => "'---'"},
+            allows_unconfirmed => {TYPE => 'BOOLEAN', NOTNULL => 1,
+                                   DEFAULT => 'FALSE'},
         ],
         INDEXES => [
             products_name_idx   => {FIELDS => ['name'],
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 414731fbe..adbcb285f 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -592,6 +592,8 @@ sub update_table_definitions {
 
     $dbh->bz_drop_column('products', 'milestoneurl');
 
+    _add_allows_unconfirmed_to_product_table();
+
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
     ################################################################
@@ -3328,6 +3330,16 @@ sub _set_attachment_comment_types {
     _populate_bugs_fulltext($bug_ids);
 }
 
+sub _add_allows_unconfirmed_to_product_table {
+    my $dbh = Bugzilla->dbh;
+    if (!$dbh->bz_column_info('products', 'allows_unconfirmed')) {
+        $dbh->bz_add_column('products', 'allows_unconfirmed',
+            { TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE' });
+        $dbh->do('UPDATE products SET allows_unconfirmed = 1 
+                   WHERE votestoconfirm > 0');
+    }
+}
+
 1;
 
 __END__
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index 410f1bd20..0228aca02 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -58,6 +58,7 @@ use constant DB_COLUMNS => qw(
    maxvotesperbug
    votestoconfirm
    defaultmilestone
+   allows_unconfirmed
 );
 
 use constant REQUIRED_CREATE_FIELDS => qw(
@@ -74,9 +75,11 @@ use constant UPDATE_COLUMNS => qw(
     votesperuser
     maxvotesperbug
     votestoconfirm
+    allows_unconfirmed
 );
 
 use constant VALIDATORS => {
+    allows_unconfirmed => \&Bugzilla::Object::check_boolean,
     classification   => \&_check_classification,
     name             => \&_check_name,
     description      => \&_check_description,
@@ -631,6 +634,7 @@ sub set_is_active { $_[0]->set('isactive', $_[1]); }
 sub set_votes_per_user { $_[0]->set('votesperuser', $_[1]); }
 sub set_votes_per_bug { $_[0]->set('maxvotesperbug', $_[1]); }
 sub set_votes_to_confirm { $_[0]->set('votestoconfirm', $_[1]); }
+sub set_allows_unconfirmed { $_[0]->set('allows_unconfirmed', $_[1]); }
 
 sub set_group_controls {
     my ($self, $group, $settings) = @_;
@@ -882,6 +886,7 @@ sub flag_types {
 ####      Accessors      ######
 ###############################
 
+sub allows_unconfirmed { return $_[0]->{'allows_unconfirmed'}; }
 sub description       { return $_[0]->{'description'};       }
 sub is_active         { return $_[0]->{'isactive'};       }
 sub votes_per_user    { return $_[0]->{'votesperuser'};      }
@@ -941,6 +946,7 @@ Bugzilla::Product - Bugzilla product class.
     my votestoconfirm    = $product->votes_to_confirm;
     my $defaultmilestone = $product->default_milestone;
     my $classificationid = $product->classification_id;
+    my $allows_unconfirmed = $product->allows_unconfirmed;
 
 =head1 DESCRIPTION
 
diff --git a/attachment.cgi b/attachment.cgi
index bc1cb90f7..20a96d09d 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -496,7 +496,8 @@ sub insert {
       ($bug_status) = grep {$_->name eq $bug_status} @{$bug->status->can_change_to};
 
       if ($bug_status && $bug_status->is_open
-          && ($bug_status->name ne 'UNCONFIRMED' || $bug->product_obj->votes_to_confirm))
+          && ($bug_status->name ne 'UNCONFIRMED' 
+              || $bug->product_obj->allows_unconfirmed))
       {
           $bug->set_status($bug_status->name);
           $bug->clear_resolution();
diff --git a/editproducts.cgi b/editproducts.cgi
index a328ca678..8433ed16b 100755
--- a/editproducts.cgi
+++ b/editproducts.cgi
@@ -176,17 +176,22 @@ if ($action eq 'new') {
 
     check_token_data($token, 'add_product');
 
-    my $product =
-      Bugzilla::Product->create({classification   => $classification_name,
-                                 name             => $product_name,
-                                 description      => scalar $cgi->param('description'),
-                                 version          => scalar $cgi->param('version'),
-                                 defaultmilestone => scalar $cgi->param('defaultmilestone'),
-                                 isactive         => scalar $cgi->param('is_active'),
-                                 votesperuser     => scalar $cgi->param('votesperuser'),
-                                 maxvotesperbug   => scalar $cgi->param('maxvotesperbug'),
-                                 votestoconfirm   => scalar $cgi->param('votestoconfirm'),
-                                 create_series    => scalar $cgi->param('createseries')});
+    my %create_params = (
+        classification   => $classification_name,
+        name             => $product_name,
+        description      => scalar $cgi->param('description'),
+        version          => scalar $cgi->param('version'),
+        defaultmilestone => scalar $cgi->param('defaultmilestone'),
+        isactive         => scalar $cgi->param('is_active'),
+        create_series    => scalar $cgi->param('createseries'),
+        allows_unconfirmed => scalar $cgi->param('allows_unconfirmed'),
+    );
+    if (Bugzilla->params->{'usevotes'}) {
+        $create_params{votesperuser}   = $cgi->param('votesperuser');
+        $create_params{maxvotesperbug} = $cgi->param('maxvotesperbug');
+        $create_params{votestoconfirm} = $cgi->param('votestoconfirm');
+    }
+    my $product = Bugzilla::Product->create(\%create_params);
 
     delete_token($token);
 
@@ -294,9 +299,12 @@ if ($action eq 'update') {
     $product->set_description(scalar $cgi->param('description'));
     $product->set_default_milestone(scalar $cgi->param('defaultmilestone'));
     $product->set_is_active(scalar $cgi->param('is_active'));
-    $product->set_votes_per_user(scalar $cgi->param('votesperuser'));
-    $product->set_votes_per_bug(scalar $cgi->param('maxvotesperbug'));
-    $product->set_votes_to_confirm(scalar $cgi->param('votestoconfirm'));
+    if (Bugzilla->params->{'usevotes'}) {
+        $product->set_votes_per_user(scalar $cgi->param('votesperuser'));
+        $product->set_votes_per_bug(scalar $cgi->param('maxvotesperbug'));
+        $product->set_votes_to_confirm(scalar $cgi->param('votestoconfirm'));
+    }
+    $product->set_allows_unconfirmed(scalar $cgi->param('allows_unconfirmed'));
 
     my $changes = $product->update();
 
diff --git a/enter_bug.cgi b/enter_bug.cgi
index 7c88f8d47..31e106959 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -520,8 +520,10 @@ my $initial_statuses = Bugzilla::Status->can_change_to();
 @$initial_statuses = grep { $_->is_open } @$initial_statuses;
 
 my @status = map { $_->name } @$initial_statuses;
-# UNCONFIRMED is illegal if votes_to_confirm = 0.
-@status = grep {$_ ne 'UNCONFIRMED'} @status unless $product->votes_to_confirm;
+# UNCONFIRMED is illegal if allows_unconfirmed is false.
+if (!$product->allows_unconfirmed) {
+    @status = grep {$_ ne 'UNCONFIRMED'} @status;
+}
 scalar(@status) || ThrowUserError('no_initial_bug_status');
 
 # If the user has no privs...
diff --git a/importxml.pl b/importxml.pl
index 05f95d646..1a61c5ead 100755
--- a/importxml.pl
+++ b/importxml.pl
@@ -913,7 +913,7 @@ sub process_bug {
     
     # Check everconfirmed 
     my $everconfirmed;
-    if ($product->votes_to_confirm) {
+    if ($product->allows_unconfirmed) {
         $everconfirmed = $bug_fields{'everconfirmed'} || 0;
     }
     else {
diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index 614273769..036286454 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -976,7 +976,8 @@ BugCheck("bugs WHERE bug_status IN ($confirmed_open_states) AND everconfirmed =
 Status('bug_check_votes_everconfirmed');
 
 BugCheck("bugs INNER JOIN products ON bugs.product_id = products.id " .
-         "WHERE everconfirmed = 0 AND votestoconfirm <= votes",
+         "WHERE everconfirmed = 0 AND votestoconfirm > 0
+                AND votestoconfirm <= votes",
          'bug_check_votes_everconfirmed_error_text');
 
 ###########################################################################
diff --git a/template/en/default/admin/products/create.html.tmpl b/template/en/default/admin/products/create.html.tmpl
index 664564040..f4a2161aa 100644
--- a/template/en/default/admin/products/create.html.tmpl
+++ b/template/en/default/admin/products/create.html.tmpl
@@ -25,15 +25,17 @@
 [% PROCESS global/header.html.tmpl
   title = title
   style_urls = ['skins/standard/admin.css']
+  javascript_urls = ['js/util.js']
 %]
 
 [% DEFAULT
   product.votesperuser = "0",
   product.maxvotesperbug  = "10000",
-  product.votestoconfirm = "0",
+  product.votes_to_confirm = "0",
   product.is_active = 1,
   version = "unspecified",
   product.defaultmilestone = constants.DEFAULT_MILESTONE
+  product.allows_unconfirmed = 0
 %]
 
 <form method="post" action="editproducts.cgi">
diff --git a/template/en/default/admin/products/edit-common.html.tmpl b/template/en/default/admin/products/edit-common.html.tmpl
index 67dd5ae64..2c94402d6 100644
--- a/template/en/default/admin/products/edit-common.html.tmpl
+++ b/template/en/default/admin/products/edit-common.html.tmpl
@@ -69,39 +69,44 @@
        [% ' checked="checked"' IF product.is_active %]>
   </td>
 </tr>
-
-[% IF !Param('usevotes') %]
-<tr class="param_disabled">
-  <td colspan="2"
-      style="font-family: arial; font-style: italic; font-size: 0.7em; text-align: center;">
-    The 'usevotes' parameter is currently 'off'. These voting
-    settings will take effect when the parameter is next enabled.</td>
-</tr>
-[% END %]
-<tr [% IF !Param('usevotes') %]class="param_disabled" [% END %]>
-  <th align="right">Maximum votes per person:</th>
-  <td><input size="5" maxlength="5" name="votesperuser" 
-             value="[% product.votesperuser FILTER html %]">
-  </td>
-</tr>
-<tr [% IF !Param('usevotes') %]class="param_disabled" [% END %]>
-  <th align="right">
-    Maximum votes a person can put on a single [% terms.bug %]:
-  </th>
-  <td><input size="5" maxlength="5" name="maxvotesperbug" 
-             value="[% product.maxvotesperbug FILTER html %]">
-  </td>
-</tr>
-<tr [% IF !Param('usevotes') %]class="param_disabled" [% END %]>
+<tr>
   <th align="right">
-    Confirmation threshold:
+    <label for="allows_unconfirmed">Enable the
+      [%+ display_value('bug_status', 'UNCONFIRMED') FILTER html %] status
+      in this product:</label>
   </th>
-  <td>
-    Enter the number of votes [% terms.abug %] in this product needs to
-    automatically get out of the
-    <a href="page.cgi?id=fields.html#status">[% display_value("bug_status", "UNCONFIRMED") FILTER html %]</a>
-    state.<br>
-    <input size="5" maxlength="5" name="votestoconfirm" 
-           value="[% product.votestoconfirm FILTER html %]">
+  <td><input type="checkbox" id="allows_unconfirmed" name="allows_unconfirmed"
+             [% ' checked="checked"' IF product.allows_unconfirmed %]
+             [% IF Param('usevotes') %]
+               onchange="bz_toggleClass('votes_to_confirm_container', 
+                                        'bz_default_hidden')"
+             [% END %]>
+    [% IF Param('usevotes') %]
+      <span id="votes_to_confirm_container"
+            [% ' class="bz_default_hidden"' IF !product.allows_unconfirmed %]>
+        ...and automatically confirm [% terms.bugs %] if they get
+        <input size="3" maxlength="5" name="votestoconfirm" id="votestoconfirm"
+               value="[% product.votes_to_confirm FILTER html %]">
+         votes. (Setting this to 0 disables auto-confirming [% terms.bugs %]
+         by vote.)
+      </span>
+    [% END %]
   </td>
 </tr>
+
+[% IF Param('usevotes') %]
+  <tr>
+    <th align="right">Maximum votes per person:</th>
+    <td><input size="5" maxlength="5" name="votesperuser" id="votesperuser"
+               value="[% product.votesperuser FILTER html %]">
+    </td>
+  </tr>
+  <tr>
+    <th align="right">
+      Maximum votes a person can put on a single [% terms.bug %]:
+    </th>
+    <td><input size="5" maxlength="5" name="maxvotesperbug" id="maxvotesperbug"
+               value="[% product.maxvotesperbug FILTER html %]">
+    </td>
+  </tr>
+[% END %]
diff --git a/template/en/default/admin/products/edit.html.tmpl b/template/en/default/admin/products/edit.html.tmpl
index e6480c453..976739f78 100644
--- a/template/en/default/admin/products/edit.html.tmpl
+++ b/template/en/default/admin/products/edit.html.tmpl
@@ -29,6 +29,7 @@
 [% PROCESS global/header.html.tmpl
   title = title
   style_urls = ['skins/standard/admin.css']
+  javascript_urls = ['js/util.js']
 %]
 
 [% group_control = {${constants.CONTROLMAPNA}        => 'NA',
diff --git a/template/en/default/admin/products/updated.html.tmpl b/template/en/default/admin/products/updated.html.tmpl
index 594f84327..6e484ff34 100644
--- a/template/en/default/admin/products/updated.html.tmpl
+++ b/template/en/default/admin/products/updated.html.tmpl
@@ -39,6 +39,8 @@
   style_urls = ['skins/standard/admin.css']
 %]
 
+[% PROCESS "global/field-descs.none.tmpl" %]
+
 [% IF changes.name.defined %]
   <p>
   Updated product name from '[% changes.name.0 FILTER html %]' to
@@ -101,6 +103,24 @@
   [% checkvotes = 1 %]
 [% END %]
 
+[% IF changes.allows_unconfirmed.defined %]
+  <p>
+  [% IF product.allows_unconfirmed %]
+    The product now allows the 
+    [%+ display_value('bug_status', 'UNCONFIRMED') FILTER html %] status.
+  [% ELSE %]
+    The product no longer allows the 
+    [%+ display_value('bug_status', 'UNCONFIRMED') FILTER html %] status.
+    Note that any 
+    <a href="buglist.cgi?product=
+            [%- product.name FILTER url_quote %]&amp;bug_status=UNCONFIRMED"> 
+    [%- terms.bugs %] that currently have the 
+    [%+ display_value('bug_status', 'UNCONFIRMED') FILTER html %] status</a>
+    will remain in that status until they are edited.
+  [% END %]
+  </p>
+[% END %]
+
 [% IF !changes.keys.size %]
   <p>Nothing changed for product '[% product.name FILTER html %]'.</p>
 [% END %]
diff --git a/template/en/default/attachment/create.html.tmpl b/template/en/default/attachment/create.html.tmpl
index 9150b2b32..f00a0ade4 100644
--- a/template/en/default/attachment/create.html.tmpl
+++ b/template/en/default/attachment/create.html.tmpl
@@ -77,7 +77,8 @@
           <label for="takebug">take [% terms.bug %]</label>
           [% bug_statuses = [] %]
           [% FOREACH bug_status = bug.status.can_change_to %]
-            [% NEXT IF bug_status.name == "UNCONFIRMED" && !bug.product_obj.votes_to_confirm %]
+            [% NEXT IF bug_status.name == "UNCONFIRMED" 
+                       && !bug.product_obj.allows_unconfirmed %]
             [% bug_statuses.push(bug_status) IF bug_status.is_open %]
           [% END %]
           [% IF bug_statuses.size %]