One of our most FAQs is "how do I turn on unconfirmed"? The current method (set votestoconfirm to non-zero) is not very intuitive. Also, confirming by popular vote has only ever been done on a handful of bugs on b.m.o. We should remove this feature, and replace it with a simple "enable UNCONFIRMED" checkbox for each product. Gerv

The only reason it never gets used on b.m.o is because nobody knows about it. The UI needs to change so that if a bug is in the UNCONFIRMED state it states right next to the status "This bug will be confirmed if X people vote for it" in small print. Then I bet it starts working because people outside of the developers will know it's there. I'll veto removing this because I use it at InTrec. On the other hand, the edit product page for the admin needs an easier way to enable UNCONFIRMED. I bet most people could figure it out if there were just an instruction on the page stating "to enable UNCONFIRMED, set the 'votes to confirm' to a non-zero value". Or we could just add a checkbox for it, and if the box is checked, complain if votestoconfirm is zero.

> I'll veto removing this because I use it at InTrec. In what way? I didn't know your Bugzilla was public... > On the other hand, the edit > product page for the admin needs an easier way to enable UNCONFIRMED. I bet > most people could figure it out if there were just an instruction on the page > stating "to enable UNCONFIRMED, set the 'votes to confirm' to a non-zero > value". Or we could just add a checkbox for it, and if the box is checked, > complain if votestoconfirm is zero. The instruction is bad because it indicates clearly that the UI is bad. A control which needs a 13-word manual is insufficiently understandable. Complaining is bad because we are beating the user round the head for something they couldn't possibly expected to know, based on the fact that Bugzilla doesn't work the way anyone would expect. A forced link between the UNCONFIRMED feature and the voting feature is bogus, whichever way you look at it. But, if you are determined to keep it, we could decouple it by adding the "enable UNCONFIRMED" checkbox but making votestoconfirm=0 mean "infinity" - i.e. if UNCONFIRMED is on but votestoconfirm is 0 (the default), you can't confirm using votes. Or we could make "" the default, meaning infinity. Gerv

OK, I could deal with separating them. Make you turn on UNCONFIRMED explicitly, and if votestoconfirm is 0 then you can't confirm by voting. And if UNCONFIRMED is disabled and votestoconfirm is nonzero it just gets ignored. This not only sounds more intuitive, but sounds like it might be easier to program around, too (since you just check whether UNCOFIRMED is enabled in most places and the only place you'd have to deal with votestoconfirm is when you're actually voting).

[ ] Bugs start unconfirmed ... and will get confirmed if they get [ ] votes.

*** Bug 173474 has been marked as a duplicate of this bug. ***

I'd like to try my hand at decoupling unconfirmed and votestoconfirm, if no one else is working on it. Will have something to view by tomorrow.

Revise summary to reflect comments.

Accepting. Sorry for the spam.

votestoconfirm only referenced now in checksetup, sanitycheck, editproducts, and CGI.pl (where it is actually used :).

Comment on attachment 133382 [details] [diff] [review] First cut The patch is bitrotten for editproducts.cgi, so I couldn't really test it deeply, but here are some comments. >- if ($votes >= $votestoconfirm && $status eq $::unconfirmedstate) { >+ if ($votestoconfirm > 0 && $votes >= $votestoconfirm >+ && $status eq $::unconfirmedstate) { Nit: Unless there is a specific reason, move the && operator to the previous line and align $status with the first $votestoconfirm on the upper line. >+ useunconfirmed tinyint not null, Maybe default 0? In editproducts: >+ my $useunconfirmed = $::FORM{useunconfirmed} || 0; ... > SendSQL("INSERT INTO products ( " . > "name, description, milestoneurl, disallownew, votesperuser, " . >- "maxvotesperbug, votestoconfirm, defaultmilestone" . >+ "maxvotesperbug, votestoconfirm, defaultmilestone, useunconfirmed" . > " ) VALUES ( " . > SqlQuote($product) . "," . > SqlQuote($description) . "," . > SqlQuote($milestoneurl) . "," . > $disallownew . "," . > "$votesperuser, $maxvotesperbug, $votestoconfirm, " . >- SqlQuote($defaultmilestone) . ")"); >+ SqlQuote($defaultmilestone) . ", $useunconfirmed )"); Wouldn't this cause an sql injection vulnerability? >+@status = "NEW" if (UserInGroup("editbugs") || UserInGroup("canconfirm")); >+ >+SendSQL("SELECT useunconfirmed FROM products WHERE name = " . >+ SqlQuote($product)); >+push(@status, $unconfirmedstate) if (FetchOneColumn()); >+ >+push (@status, "NEW") if (!scalar(@status)); This is confusing, although it probably works. How about first checking for and possibly adding unconfirmed, then pushing new if !scalar(@status) || UIG("editbugs") || UIG("canconfirm")? This would group the NEW stuff into one place. How about getting rid of the deprecated SendSQL while you're at it?

I also got puzzled by the mismatch between documentation and program in 2.16.4: It's non-obvious that using "0 votes" to move a bug out from UNCONFIRMED state will disable the UNCONFIRMED state. I thought someone will have to change state manually from UNCONFIRMED to NEW after confirming the bug. But UNCONFIRMED is not available at all. This does not match the help texts, nor does it match the documentation.

All 2.18 bugs that haven't been touched in over 60 days and aren't flagged as blockers are getting pushed out to 2.20

This bug has not been touched by its owner in over six months, even though it is targeted to 2.20, for which the freeze is 10 days away. Unsetting the target milestone, on the assumption that nobody is actually working on it or has any plans to soon. If you are the owner, and you plan to work on the bug, please give it a real target milestone. If you are the owner, and you do *not* plan to work on it, please reassign it to nobody@bugzilla.org or a .bugs component owner. If you are *anybody*, and you get this comment, and *you* plan to work on the bug, please reassign it to yourself if you have the ability.

*** Bug 312280 has been marked as a duplicate of this bug. ***

*** Bug 325179 has been marked as a duplicate of this bug. ***

Another thing I'm missing is to be able to create an UNCONFIRMED bug while having canconfirm permission. A parameter for that option would be great.

Antonio: as far as I know, you can: just visit enter_bug.cgi and change the dropdown labelled "Initial State". Gerv

(In reply to comment #17) Gerv, the "Initial state" field is not a dropdown when you have "canconfirm" permission It's set fix to NEW.

(In reply to comment #18) > Gerv, the "Initial state" field is not a dropdown when you have "canconfirm" > permission It's set fix to NEW. No, you can still select other options from that drop down. Available options are: NEW, UNCONFIRMED, ASSIGNED.

I certainly get the three options Reed talks about. Antonio: are you seeing this problem on bugzilla.mozilla.org? If so, which account are you using? (I can "impersonate" you and go and look for myself.) Here's the code that populates the list: http://lxr.mozilla.org/mozilla/source/webtools/bugzilla/enter_bug.cgi#488 If you are only getting NEW, then a) the user concerned must not have either canconfirm or editbugs, and b) the number of votestoconfirm for the product must be 0 (i.e. you aren't using UNCONFIRMED in that product). Gerv

Thank you both for your answers. In my enter_bug.cgi (release 2.22 - revision 1.126 tagged Bugzilla_Stable) it looks like this: if ($votestoconfirm) { if (UserInGroup("editbugs") || UserInGroup("canconfirm")) { push(@status, "NEW"); } push(@status, 'UNCONFIRMED'); } else { push(@status, "NEW"); } Since $votestoconfirm is 0, there's no dropdown. I'm glad to see the change in HEAD, looking forward to the next stable release :) Toni

This adds a boolean option to products that specifies whether or not the UNCONFIRMED status is allowed in this product. Ideally we'd have a more generic system than this for fields to control other fields (and we do have such a system, but it's just not ready for this), but we need this as a stopgap so that I can work on bug 372979. I had to make it so that Bugzilla always allows transitions from a status to itself, because when you disable UNCO in a product now, bugs will remain UNCO even though it's not a valid status.

Comment on attachment 356365 [details] [diff] [review] v1 I also want to look at it.

Comment on attachment 356365 [details] [diff] [review] v1 >Index: Bugzilla/Product.pm >+sub allows_unconfirmed { return $_[0]->{'allows_unconfirmed'}; } Nit: it would be great if you could mention it in the POD. Also, why allows_ instead of allow_? This looks strange to me. >Index: template/en/default/admin/products/updated.html.tmpl >+ status. Note that any <a href="...">bugs "bugs" must be [% terms.bugs %] (t/009bugwords........NOK 187/263). The reasons I deny review (besides the runtest error above) are: 1) If allows_unconfirmed = 1 and votes_to_confirm = 0, you can file bugs as UNCONFIRMED, and sanitycheck.cgi then complains that you have bugs with enough votes to be confirmed, but which aren't. That's problematic. 2) If you have bugs with the UNCONFIRMED status, and you then set allows_unconfirmed to 0, you can resolve bugs as RESOLVED (as expected); but when you want to reopen the bug, UNCONFIRMED is listed despite it's no longer a valid bug status, and trying to select it (e.g. reporters with no privs) will throw an error: "You are not allowed to change the bug status from RESOLVED to UNCONFIRMED." Everything else looks good to me.

I suspect the last problem you mentioned will be handled bug bug 512606.

Okay, here's a patch with everything else fixed other than the RESOLVED -> UNCONFIRMED transition (because I think that other patch will just fix that problem outright).

Comment on attachment 398464 [details] [diff] [review] v2 >Index: Bugzilla/Bug.pm >@@ -1101,8 +1101,8 @@ > @valid_statuses = @{Bugzilla::Status->can_change_to()}; > } > >- if (!$product->votes_to_confirm) { >- # UNCONFIRMED becomes an invalid status if votes_to_confirm is 0, >+ if (!$product->allows_unconfirmed) { >+ # UNCONFIRMED becomes an invalid status if allows_unconfirmed is false, > # even if you are in editbugs. > @valid_statuses = grep {$_->name ne 'UNCONFIRMED'} @valid_statuses; > } This part is bitrotten as I asked you to move this block in the previous else block. >+ [%+ get_status('UNCONFIRMED') FILTER html %] status in this get_status() no longer exists. Several places are affected.

Oh, also, the UI when editing a product is confusing IMO. You first ask the number of votes to go out of UNCONFIRMED, and then you ask if UNCONFIRMED should be used or not. IMO, both should be merged in a more logical way, e.g. by following the idea from comment 4.

... and the UI should also mention that 0 votes to confirm means that you can never confirm bugs this way.

I fixed the bitrot, and the get_status stuff, and the new UI was a good idea, so I did that too. And in fact, there's no reason to show the voting parameters now when usevotes is off (since they do absolutely nothing now when voting is off) so they're now hidden totally in that case.

Comment on attachment 417385 [details] [diff] [review] v3 >Index: Bugzilla/Bug.pm >+ # We skip this check if we are changing from a status to itself. But show_bug.cgi doesn't display UNCONFIRMED in the bug status select box when the bug was originally marked as UNCONFIRMED and "use UNCONFIRMED" has been turned off meanwhile. This is annoying, because editing a bug force you to change its bug status, and 1) maybe you want to keep its status alone, and 2) powerless users will get an error that they are not allowed to confirm bugs. >Index: template/en/default/admin/products/edit-common.html.tmpl >+[% IF Param('usevotes') %] >+ <td><input size="5" maxlength="5" name="votesperuser" >+ value="[% product.votesperuser FILTER html %]"> While you are on it, please add an ID to the text field. This makes the QA team's life much easier to write Selenium scripts. >+ <td><input size="5" maxlength="5" name="maxvotesperbug" >+ value="[% product.maxvotesperbug FILTER html %]"> Same here. Also, with usevotes = off, you can no longer edit products. You get the error: '' is an invalid value for the Votes Per User field, which should contain a non-negative number. editproducts.cgi shouldn't call $product->set_votes_*() when usevotes = off. I didn't check if this is also a problem on product creation.

2 days before the hard freeze...

Okay, this fixes all the issues you mentioned! :-)

Comment on attachment 418122 [details] [diff] [review] v4 >Index: Bugzilla/Bug.pm >+ # If this bug has an inactive status set, it should still be in the list. >+ if (!grep($_ eq $self->status->name, @available)) { @available contains objects, so you want $_->name instead of $_ alone. >Index: template/en/default/admin/products/edit-common.html.tmpl >+ <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.) Setting votestoconfirm to 0 confirms all bugs despite what the comment says. Bugzilla::Bug::CheckIfVotedConfirmed() must be fixed: if (!$bug->everconfirmed && $bug->votes >= $bug->product_obj->votes_to_confirm) It must first check that votes_to_confirm is not zero. Not exactly related to this bug, but sanitycheck.cgi complains when a bug which has enough votes is set back to UNCONFIRMED, when the status workflow allows this transition. And every time a new user votes for the bug, the bug comes back to NEW, even if a power user wants to keep it UNCONFIRMED. That's pretty confusing to the average (and even advanced) user, IMO. Should we forbird to set bugs back to UNCONFIRMED when there are enough votes on them?

(In reply to comment #34) Thanks for catching all that stuff there. :-) I'll attach a new patch soon. > Not exactly related to this bug, but sanitycheck.cgi complains when a bug which > has enough votes is set back to UNCONFIRMED, when the status workflow allows > this transition. And every time a new user votes for the bug, the bug comes > back to NEW, even if a power user wants to keep it UNCONFIRMED. That's pretty > confusing to the average (and even advanced) user, IMO. Should we forbird to > set bugs back to UNCONFIRMED when there are enough votes on them? Oh, auugh. That will confuse the heck out of users who are trying to re-unconfirm bugs and can't do it for only *some* of them. No, I think we should let people set them back to UNCONFIRMED if they really want to. Voting just auto-confirms them and they're *most likely* to stay that way, I suppose. Probably the solution is to make it so that bugs are only confirmed if they go from below the number of votes required to equal or greater than the number of votes required, so that the confirmation only happens once. Also, we should remove the voting system from core and make it into an extension. :-)

Okay, this fixes the errors found.

Comment on attachment 418265 [details] [diff] [review] v5 I have no more ideas about what else to test. :) r=LpSolit

Great, thank you! :-) Checking in attachment.cgi; /cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v <-- attachment.cgi new revision: 1.166; previous revision: 1.165 done Checking in editproducts.cgi; /cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi new revision: 1.154; previous revision: 1.153 done Checking in enter_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v <-- enter_bug.cgi new revision: 1.176; previous revision: 1.175 done Checking in importxml.pl; /cvsroot/mozilla/webtools/bugzilla/importxml.pl,v <-- importxml.pl new revision: 1.95; previous revision: 1.94 done Checking in sanitycheck.cgi; /cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v <-- sanitycheck.cgi new revision: 1.148; previous revision: 1.147 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.307; previous revision: 1.306 done Checking in Bugzilla/Product.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Product.pm,v <-- Product.pm new revision: 1.44; previous revision: 1.43 done Checking in Bugzilla/DB/Schema.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v <-- Schema.pm new revision: 1.128; previous revision: 1.127 done Checking in Bugzilla/Install/DB.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v <-- DB.pm new revision: 1.81; previous revision: 1.80 done Checking in template/en/default/admin/products/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/create.html.tmpl,v <-- create.html.tmpl new revision: 1.9; previous revision: 1.8 done Checking in template/en/default/admin/products/edit-common.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/edit-common.html.tmpl,v <-- edit-common.html.tmpl new revision: 1.13; previous revision: 1.12 done Checking in template/en/default/admin/products/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.15; previous revision: 1.14 done Checking in template/en/default/admin/products/updated.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/updated.html.tmpl,v <-- updated.html.tmpl new revision: 1.11; previous revision: 1.10 done Checking in template/en/default/attachment/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/create.html.tmpl,v <-- create.html.tmpl new revision: 1.46; previous revision: 1.45 done

Added to the release notes in bug 547466.