252214 - Allow max of zero votes per bug

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[timeless]

Update product  
    
Sorry, the max votes per bug must be a positive integer.
Back to the query page or edit more products. 

--
That's not nice. Zero is a nice number. Please be nice to zero.

Comment 1

[Dave Miller [:justdave]]

Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.

Comment 2

[A. Karl Kornel]

I'm also seeing this on my installation (2.18.1).  The weird thing is I can
create a product that sets a maximum of zero votes per bug, and Bugzilla will
create the product without complaining one bit (see bug 296452).

Also, the OS and Hardware fields can probably be changed to "All", and it might
be good to change "Version" to 2.18.1, which is the latest stable release
displaying this bug.

Comment 3

[Frédéric Buclin]

Valid. This should allow to disable votes on a per product basis.

Comment 4

[A. Karl Kornel]

Attachment: Update editproducts.cgi to allow $maxvotesperbug=0

A quick check of editproducts.cgi found the 2 lines that would need to be
changed.  I'm checking out the rest of Bugzilla to see if there's anything else
that would need to be changed; I'll post additional patches if needed.

Comment 5

[A. Karl Kornel]

I was able to apply the patch to my copy of 2.18.1.  Assuming this patch actually works, would it be 
possible to apply it to 2.18?

Comment 6

[Frédéric Buclin]

Alfred, just for your information, an official reviewer has to grant review to
your patch before you can request approval for the bug. But Vladd has now
granted review, so it's ok. :)

Comment 7

[Dave Miller [:justdave]]

does this patch apply to both branches cleanly?

Comment 8

[Dave Miller [:justdave]]

Sorry, it says so in comment 5.

Comment 9

[Frédéric Buclin]

Tip:

Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.84; previous revision: 1.83
done


2.18.1:

Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.53.2.6; previous revision: 1.53.2.5
done

Comment 10

[Frédéric Buclin]

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

Comment 11

[Frédéric Buclin]

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

Comment 12

[Stephen Lee]

As far as I can tell, it is the votesperuser parameter which usually disables
voting for a product, and (from a brief look at the code) I think that if you
set maxvotesperbug to 0 (but with votesperuser > 0), you would retain the voting
controls on the bug but not actually be able to vote.

Is anyone able to confirm this?

Also:
- What is this intending to acheive that can't already be acheived by setting
votesperuser to zero for the product? I would think it's simple enough to set
votesperuser rather than maxvotesper bug, and saves having to complicate the
backend code.
- The 'duplicate' bug 296452 mentioned that a negative number had been allowed
when products were created. Was that aspect already fixed elsewhere, or has it
ended up dropped?

Comment 13

[A. Karl Kornel]

(In reply to comment #12)
> As far as I can tell, it is the votesperuser parameter which usually disables
> voting for a product, and (from a brief look at the code) I think that if you
> set maxvotesperbug to 0 (but with votesperuser > 0), you would retain the voting
> controls on the bug but not actually be able to vote.
> 
> Is anyone able to confirm this?

Could you show where in the code you think this is being allowed?  Also, what version of Bugzilla are 
you using?  If you're talking about lines 506-512 in Bugzilla/Bug.pm, then it may be a bug.

> Also:
> - What is this intending to acheive that can't already be acheived by setting
> votesperuser to zero for the product? I would think it's simple enough to set
> votesperuser rather than maxvotesper bug, and saves having to complicate the
> backend code.

Possibly nothing.  As I said in bug 296542, "Either it means that [this bug] is actually a bug (that zero 
should be a valid value when editing a product), or it means that the zero value is actually invalid, and 
should have been caught at the time the product was first added."  I guess it turned out that the first of 
the two possibilities was true.

(Just as a side note, the solution to this bug actually removed one test, simplifying the code instead of 
complicating it!)

> - The 'duplicate' bug 296452 mentioned that a negative number had been allowed
> when products were created. Was that aspect already fixed elsewhere, or has it
> ended up dropped?

The problem seems to still exist.  However, the negative number problem was never really part of the 
original problem as described (in this bug and in 296452).  If a bug doesn't already exist to track this 
problem, you're welcome to report it yourself!

Comment 14

[Stephen Lee]

(In reply to comment #13)
> Could you show where in the code you think this is being allowed?

http://lxr.mozilla.org/bugzilla/source/template/en/default/bug/edit.html.tmpl#467
[% IF bug.use_votes %]
... which I believe gets its value from:

http://lxr.mozilla.org/bugzilla/source/Bugzilla/Bug.pm#511
      && $::prodmaxvotes{$self->{product}} > 0;
... which I believe gets its value from:

http://lxr.mozilla.org/bugzilla/source/globals.pl#191
        $::prodmaxvotes{$p} = $votesperuser;
... which was read from the DB a few lines earlier.


Thus (assuming my analysis is correct), one way to keep this change and fix the
specific problem I described could be to have globals.pl also read
maxvotesperbug, forcing $::prodmaxvotes{$p} to zero if that is zero.

This however, is just the first thing I thought of to which this change could
cause a problem, and my 'fix' may have its own knock-on effects, so an audit
would be needed of other uses of votesperuser, maxvotesperbug, and values
derived from these.


Again, I don't really understand why we needed this fix in the first place.

The only reasons given here are "Zero is a nice number. Please be nice to zero",
and comment 3: "This should allow to disable votes on a per product basis". The
former seems a spurious reason (I think lots of non-natural numbers are "nice",
but this doesn't make them valid inputs), and the latter is functionality that
is already available from the immediately adjacent votesperuser control on the
editproducts page.

However, unless there are plans to use the votes for something other than
putting on bugs, it seems meaningless to me to say, in effect,
"You have 10 votes, and you can put no more than 0 of these on any bug".


Since the data validation is still outstanding (which I had, apparently
incorrectly, assumed to be the main thrust of bug 296452), my vote would be for
"Don't allow max of zero votes per bug", which could also include the data
validation for createproduct.

My first question with this is whether there would be strong disagreement (e.g.
was this fix actually required, or just "I don't see why not"-ed).
Second question if we go down that route is whether to implement that by filing
a new bug, or by proposing this bug be re-opened, backed out and WONTFIX-ed.


> If a bug doesn't already exist to track [the negative number] problem,
> you're welcome to report it yourself!

I'll consider this later when I've verified for myself whether or not that
problem still exists. If we keep this fix, I guess a suitable summary would be
"Can create product with max votes per bug at -1, but then can't modify product"
Even if I do that I think I'll let someone else do the bugs for the case of -2,
-3, etc. ;-)