diff options
| -rw-r--r-- | Bugzilla/CGI.pm | 8 | ||||
| -rw-r--r-- | extensions/Voting/Extension.pm | 163 | ||||
| -rw-r--r-- | extensions/Voting/template/en/default/pages/voting/user.html.tmpl | 19 |
3 files changed, 94 insertions, 96 deletions
diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index 58a681491..c003e9295 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -505,9 +505,9 @@ sub redirect_search_url { # GET requests that lacked a list_id are always redirected. POST requests # are only redirected if they're under the CGI_URI_LIMIT though. - my $uri_length = length($self->self_url()); - if ($self->request_method() ne 'POST' or $uri_length < CGI_URI_LIMIT) { - print $self->redirect(-url => $self->self_url()); + my $self_url = $self->self_url(); + if ($self->request_method() ne 'POST' or length($self_url) < CGI_URI_LIMIT) { + print $self->redirect(-url => $self_url); exit; } } @@ -561,7 +561,7 @@ sub url_is_attachment_base { $regex =~ s/\\\%bugid\\\%/\\d+/; } $regex = "^$regex"; - return ($self->self_url =~ $regex) ? 1 : 0; + return ($self->url =~ $regex) ? 1 : 0; } sub set_dated_content_disp { diff --git a/extensions/Voting/Extension.pm b/extensions/Voting/Extension.pm index aecd2382c..0b79d3d21 100644 --- a/extensions/Voting/Extension.pm +++ b/extensions/Voting/Extension.pm @@ -38,7 +38,7 @@ use Bugzilla::User; use Bugzilla::Util qw(detaint_natural); use Bugzilla::Token; -use List::Util qw(min); +use List::Util qw(min sum); use constant VERSION => BUGZILLA_VERSION; use constant DEFAULT_VOTES_PER_BUG => 1; @@ -217,7 +217,7 @@ sub bug_end_of_update { # If some votes have been removed, RemoveVotes() returns # a list of messages to send to voters. @msgs = _remove_votes($bug->id, 0, 'votes_bug_moved'); - _confirm_if_vote_confirmed($bug->id); + _confirm_if_vote_confirmed($bug); foreach my $msg (@msgs) { MessageToMTA($msg); @@ -419,10 +419,10 @@ sub _page_user { } # If a bug_id is given, and we're editing, we'll add it to the votes list. - + my $bug_id = $input->{bug_id}; $bug_id = $bug_id->[0] if ref($bug_id) eq 'ARRAY'; - my $bug = Bugzilla::Bug->check($bug_id) if $bug_id; + my $bug = Bugzilla::Bug->check({ id => $bug_id, cache => 1 }) if $bug_id; my $who_id = $input->{user_id} || $user->id; # Logged-out users must specify a user_id. @@ -451,52 +451,38 @@ sub _page_user { foreach my $product (@{ $user->get_selectable_products }) { next unless ($product->{votesperuser} > 0); - my @bugs; - my @bug_ids; - my $total = 0; - my $onevoteonly = 0; - my $vote_list = - $dbh->selectall_arrayref('SELECT votes.bug_id, votes.vote_count, - bugs.short_desc - FROM votes - INNER JOIN bugs - ON votes.bug_id = bugs.bug_id - WHERE votes.who = ? - AND bugs.product_id = ? - ORDER BY votes.bug_id', - undef, ($who->id, $product->id)); - - foreach (@$vote_list) { - my ($id, $count, $summary) = @$_; - $total += $count; - - # Next if user can't see this bug. So, the totals will be correct - # and they can see there are votes 'missing', but not on what bug - # they are. This seems a reasonable compromise; the alternative is - # to lie in the totals. - next if !$user->can_see_bug($id); - - push (@bugs, { id => $id, - summary => $summary, - count => $count }); - push (@bug_ids, $id); - push (@all_bug_ids, $id); - } + $dbh->selectall_arrayref('SELECT votes.bug_id, votes.vote_count + FROM votes + INNER JOIN bugs + ON votes.bug_id = bugs.bug_id + WHERE votes.who = ? + AND bugs.product_id = ?', + undef, ($who->id, $product->id)); + + my %votes = map { $_->[0] => $_->[1] } @$vote_list; + my @bug_ids = sort keys %votes; + # Exclude bugs that the user can no longer see. + @bug_ids = @{ $user->visible_bugs(\@bug_ids) }; + next unless scalar @bug_ids; + + push(@all_bug_ids, @bug_ids); + my @bugs = @{ Bugzilla::Bug->new_from_list(\@bug_ids) }; + $_->{count} = $votes{$_->id} foreach @bugs; + # We include votes from bugs that the user can no longer see. + my $total = sum(values %votes) || 0; + my $onevoteonly = 0; $onevoteonly = 1 if (min($product->{votesperuser}, $product->{maxvotesperbug}) == 1); - # Only add the product for display if there are any bugs in it. - if ($#bugs > -1) { - push (@products, { name => $product->name, - bugs => \@bugs, - bug_ids => \@bug_ids, - onevoteonly => $onevoteonly, - total => $total, - maxvotes => $product->{votesperuser}, - maxperbug => $product->{maxvotesperbug} }); - } + push(@products, { name => $product->name, + bugs => \@bugs, + bug_ids => \@bug_ids, + onevoteonly => $onevoteonly, + total => $total, + maxvotes => $product->{votesperuser}, + maxperbug => $product->{maxvotesperbug} }); } if ($canedit && $bug) { @@ -530,6 +516,7 @@ sub _update_votes { # IDs and the field values are the number of votes. my @buglist = grep {/^\d+$/} keys %$input; + my (%bugs, %votes); # If no bugs are in the buglist, let's make sure the user gets notified # that their votes will get nuked if they continue. @@ -545,20 +532,23 @@ sub _update_votes { exit; } } + else { + $user->visible_bugs(\@buglist); + my $bugs_obj = Bugzilla::Bug->new_from_list(\@buglist); + $bugs{$_->id} = $_ foreach @$bugs_obj; + } - # Call check() on each bug ID to make sure it is a positive - # integer representing an existing bug that the user is authorized - # to access, and make sure the number of votes submitted is also - # a non-negative integer (a series of digits not preceded by a - # minus sign). - my (%votes, @bugs); + # Call check_is_visible() on each bug to make sure it is an existing bug + # that the user is authorized to access, and make sure the number of votes + # submitted is also an integer. foreach my $id (@buglist) { - my $bug = Bugzilla::Bug->check($id); - push(@bugs, $bug); - $id = $bug->id; - $votes{$id} = $input->{$id}; - detaint_natural($votes{$id}) - || ThrowUserError("voting_must_be_nonnegative"); + my $bug = $bugs{$id} + or ThrowUserError('bug_id_does_not_exist', { bug_id => $id }); + $bug->check_is_visible; + $id = $bug->id; + $votes{$id} = $input->{$id}; + detaint_natural($votes{$id}) + || ThrowUserError("voting_must_be_nonnegative"); } my $token = $cgi->param('token'); @@ -571,10 +561,10 @@ sub _update_votes { # If the user is voting for bugs, make sure they aren't overstuffing # the ballot box. - if (scalar @bugs) { + if (scalar @buglist) { my (%prodcount, %products); - foreach my $bug (@bugs) { - my $bug_id = $bug->id; + foreach my $bug_id (keys %bugs) { + my $bug = $bugs{$bug_id}; my $prod = $bug->product; $products{$prod} ||= $bug->product_obj; $prodcount{$prod} ||= 0; @@ -598,56 +588,65 @@ sub _update_votes { } } - # Update the user's votes in the database. If the user did not submit - # any votes, they may be using a form with checkboxes to remove all their - # votes (checkboxes are not submitted along with other form data when - # they are not checked, and Bugzilla uses them to represent single votes - # for products that only allow one vote per bug). In that case, we still - # need to clear the user's votes from the database. - my %affected; + # Update the user's votes in the database. $dbh->bz_start_transaction(); - # Take note of, and delete the user's old votes from the database. - my $bug_list = $dbh->selectcol_arrayref('SELECT bug_id FROM votes + my $old_list = $dbh->selectall_arrayref('SELECT bug_id, vote_count FROM votes WHERE who = ?', undef, $who); - foreach my $id (@$bug_list) { - $affected{$id} = 1; - } - $dbh->do('DELETE FROM votes WHERE who = ?', undef, $who); + my %old_votes = map { $_->[0] => $_->[1] } @$old_list; my $sth_insertVotes = $dbh->prepare('INSERT INTO votes (who, bug_id, vote_count) VALUES (?, ?, ?)'); + my $sth_updateVotes = $dbh->prepare('UPDATE votes SET vote_count = ? + WHERE bug_id = ? AND who = ?'); - # Insert the new values in their place - foreach my $id (@buglist) { - if ($votes{$id} > 0) { + my %affected = map { $_ => 1 } (@buglist, keys %old_votes); + my @deleted_votes; + + foreach my $id (keys %affected) { + if (!$votes{$id}) { + push(@deleted_votes, $id); + next; + } + if ($votes{$id} == ($old_votes{$id} || 0)) { + delete $affected{$id}; + next; + } + # We use 'defined' in case 0 was accidentally stored in the DB. + if (defined $old_votes{$id}) { + $sth_updateVotes->execute($votes{$id}, $id, $who); + } + else { $sth_insertVotes->execute($who, $id, $votes{$id}); } - $affected{$id} = 1; + } + + if (@deleted_votes) { + $dbh->do('DELETE FROM votes WHERE who = ? AND ' . + $dbh->sql_in('bug_id', \@deleted_votes), undef, $who); } # Update the cached values in the bugs table - print $cgi->header(); my @updated_bugs = (); my $sth_getVotes = $dbh->prepare("SELECT SUM(vote_count) FROM votes WHERE bug_id = ?"); - my $sth_updateVotes = $dbh->prepare("UPDATE bugs SET votes = ? - WHERE bug_id = ?"); + $sth_updateVotes = $dbh->prepare('UPDATE bugs SET votes = ? WHERE bug_id = ?'); foreach my $id (keys %affected) { $sth_getVotes->execute($id); my $v = $sth_getVotes->fetchrow_array || 0; $sth_updateVotes->execute($v, $id); - my $confirmed = _confirm_if_vote_confirmed($id); + my $confirmed = _confirm_if_vote_confirmed($bugs{$id} || $id); push (@updated_bugs, $id) if $confirmed; } $dbh->bz_commit_transaction(); + print $cgi->header() if scalar @updated_bugs; $vars->{'type'} = "votes"; $vars->{'title_tag'} = 'change_votes'; foreach my $bug_id (@updated_bugs) { @@ -858,7 +857,7 @@ sub _remove_votes { # confirm a bug has been reduced, check if the bug is now confirmed. sub _confirm_if_vote_confirmed { my $id = shift; - my $bug = new Bugzilla::Bug($id); + my $bug = ref $id ? $id : new Bugzilla::Bug({ id => $id, cache => 1 }); my $ret = 0; if (!$bug->everconfirmed diff --git a/extensions/Voting/template/en/default/pages/voting/user.html.tmpl b/extensions/Voting/template/en/default/pages/voting/user.html.tmpl index 18221bda1..627011fd4 100644 --- a/extensions/Voting/template/en/default/pages/voting/user.html.tmpl +++ b/extensions/Voting/template/en/default/pages/voting/user.html.tmpl @@ -109,8 +109,7 @@ </tr> [% FOREACH bug = product.bugs %] - <tr [% IF bug.id == this_bug.id && canedit %] - class="bz_bug_being_voted_on" [% END %]> + <tr [% IF bug.id == this_bug.id && canedit %] class="bz_bug_being_voted_on"[% END %]> <td> [% IF bug.id == this_bug.id && canedit %] [% IF product.onevoteonly %] @@ -120,25 +119,25 @@ [% END %] [%- END %] </td> - <td align="right"><a name="vote_[% bug.id FILTER html %]"> + <td align="right"><a name="vote_[% bug.id FILTER none %]"> [% IF canedit %] [% IF product.onevoteonly %] - <input type="checkbox" name="[% bug.id FILTER html %]" value="1" - [% " checked" IF bug.count %] id="bug_[% bug.id FILTER html %]"> + <input type="checkbox" name="[% bug.id FILTER none %]" value="1" + [% " checked" IF bug.count %] id="bug_[% bug.id FILTER none %]"> [% ELSE %] - <input name="[% bug.id FILTER html %]" value="[% bug.count FILTER html %]" - size="2" id="bug_[% bug.id FILTER html %]"> + <input name="[% bug.id FILTER none %]" value="[% bug.count FILTER html %]" + size="2" id="bug_[% bug.id FILTER none %]"> [% END %] [% ELSE %] [% bug.count FILTER html %] [% END %] </a></td> <td align="center"> - [% bug.id FILTER bug_link(bug.id) FILTER none %] + [% PROCESS bug/link.html.tmpl bug = bug, link_text = bug.id %] </td> <td> - [% bug.summary FILTER html %] - (<a href="page.cgi?id=voting/bug.html&bug_id=[% bug.id FILTER uri %]">Show Votes</a>) + [% bug.short_desc FILTER html %] + (<a href="page.cgi?id=voting/bug.html&bug_id=[% bug.id FILTER none %]">Show Votes</a>) </td> </tr> [% END %] |