| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | poll.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | danielhonrade |
| Status: | closed (fixed) |
| Issue tags: | poll patch |
Issue Summary
Editing an existing poll clears all existing votes, but for some reason the "Cancel your vote" button is still available. And if "Cancel your vote" button is selected, the vote count will go to -1!
Steps to reproduce:
1. Create a poll with two choices
2. Vote for the first choice -> now it has 1 vote
3. Click "Edit" tab, add a third choice and click "Save" button
4. You'll return to the poll results -> now all choices have 0 votes
5. Click "Cancel your vote" button
6. Click "Results" tab -> now the first choice has -100% (-1 votes) and Total votes is -1!
This seems to be somehow related to permissions, because I could not reproduce this with the admin (user 1) account. With admin user the editing does not automatically clear existing votes in step 3.
Anyway this is reproducible with an authenticated user that has permissions to "delete own poll content", "edit own poll content" and "vote on polls", but no other permissions related to the Poll module.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| votes_cleared_but_cancel_option_not.png | 8.76 KB | Ignored: Check issue status. | None | None |
| negative_vote_count.png | 7.74 KB | Ignored: Check issue status. | None | None |
Comments
#1
First of all, editing a poll is not intended to clear all votes. This is revealed in the following comment inside the poll_update() function:
Then the reason why votes disappear (when editing a poll):
1. Form data contains votes only if the user has permission to 'administer nodes'
<?phpfunction poll_form(&$node, $form_state) {
...
// Add the current choices to the form.
for ($delta = 0; $delta < $choice_count; $delta++) {
...
$form['choice_wrapper']['choice'][$delta] = _poll_choice_form($delta, $text, $votes);
}
...
}
?>
<?php
function _poll_choice_form($delta, $value = '', $votes = 0) {
$admin = user_access('administer nodes');
...
if ($admin) {
$form['chvotes'] = array(
'#type' => 'textfield',
'#title' => t('Votes for choice @n', array('@n' => ($delta + 1))),
'#default_value' => $votes,
'#size' => 5,
'#maxlength' => 7,
'#parents' => array('choice', $delta, 'chvotes'),
);
}
return $form;
}
?>
2. poll_update() function assumes that the $node contains all data (not just updated data)
<?php
function poll_update($node) {
...
// Clean poll choices.
db_query('DELETE FROM {poll_choices} WHERE nid = %d', $node->nid);
...
foreach ($node->choice as $old_chorder => $choice) {
$chvotes = isset($choice['chvotes']) ? (int)$choice['chvotes'] : 0;
$chtext = $choice['chtext'];
if (!empty($chtext)) {
db_query("INSERT INTO {poll_choices} (nid, chtext, chvotes, chorder) VALUES (%d, '%s', %d, %d)", $node->nid, $chtext, $chvotes, $new_chorder);
...
}
...
}
}
?>
#2
#3
/* 6/8/09 danielhonrade bug report
*
* Hi, I was hacking the poll core module (which is really bad, I know but
* I only have few days to finish the project (http://pinometro.com) , this is temporary anyway, I am
* planning to create a module that would integrate to poll module with the following
* features:
* - use of google chart
* - add default values for choices
* - allow/disallow users to add choices and change other settings
* - require users to vote before commenting and not otherwise
*
* then I encountered this SAME PROBLEM, which I thought at first was the result of
* my hacking but when I installed a fresh Drupal6 and tested the poll again,
* it's still there, you can check the bug by:
* 1) Install fresh Drupal 6
* 2) Allow authenticated users to vote, create/edit/delete own poll
* 3) Login as a new authenticated user and not as admin, so you don't have node administer perm
* 4) Create a poll, then vote and edit then save
* 5) You'll notice that the vote values become 0
* 5) Cancel your vote, the result becomes -1
*
* PROBLEM: if an authenticated user without node administer permission but
* has a permission to edit his/any poll, everytime he edits his poll or
* any poll the vote values become 0 and when he goes to view tab,
* if has voted, the cancel button still exists even though vote values are 0
* so if he decided to cancel, the result is negative value which
* the validator function doesn't even notice
*
* I created a patch, which I am sure, it's not the best, but it works
*
* SOLUTION 1: I added an else statement where I copied the the content in the
* if($admin) {...} and added '#disabled' => 'disabled', and changed
* '#default_value' => $votes, to '#value' => $votes, because if the
* attribute is disabled #default_value doesn't pass its value
*
* SOLUTION 2: I added an else statement where I copied the the content in the
* if($admin) {...} and just change the '#type' => 'textfield', to '#type' => 'hidden',
* but you'll have nothing under the heading VOTE COUNT which is the original
* state when you create a poll in the original poll module
*/
<?php
// PATCH, just replace this function with this one
function _poll_choice_form($delta, $value = '', $votes = 0) {
$admin = user_access('administer nodes');
$form = array(
'#tree' => TRUE,
);
// We'll manually set the #parents property of these fields so that
// their values appear in the $form_state['values']['choice'] array.
$form['chtext'] = array(
'#type' => 'textfield',
'#title' => t('Choice @n', array('@n' => ($delta + 1))),
'#default_value' => $value,
'#parents' => array('choice', $delta, 'chtext'),
);
if ($admin) {
$form['chvotes'] = array(
'#type' => 'textfield',
'#title' => t('Votes for choice @n', array('@n' => ($delta + 1))),
'#default_value' => $votes,
'#size' => 5,
'#maxlength' => 7,
'#parents' => array('choice', $delta, 'chvotes'),
);
} else { // <-- add this (daniel's SOLUTION 1 module patch )
$form['chvotes'] = array(
'#type' => 'textfield',
'#title' => t('Votes for choice @n', array('@n' => ($delta + 1))),
'#value' => $votes,
'#size' => 5,
'#maxlength' => 7,
'#disabled' => 'disabled',
'#parents' => array('choice', $delta, 'chvotes'),
);
} // <--patch upto here
return $form;
}
?>
#4
I am having problems with getting the right diff program. But here is the patch file, anyway
/* Based on the latest Drupal 6.12-dev
* PROBLEM: if an authenticated user without node administer permission but
* has a permission to edit his/any poll, everytime he edits his poll or
* any poll the vote values become 0 and when he goes to view tab,
* if has voted, the cancel button still exists even though vote values are 0
* so if he decided to cancel, the result is negative value which
* the validator function doesn't even notice
*
* I created a patch, which I am sure, it's not the best, but it works
*
* SOLUTION 1: I added an else statement where I copied the the content in the
* if($admin) {...} and added '#disabled' => 'disabled', and changed
* '#default_value' => $votes, to '#value' => $votes, because if the
* attribute is disabled #default_value doesn't pass its value
*
* SOLUTION 2: I added an else statement where I copied the the content in the
* if($admin) {...} and just change the '#type' => 'textfield', to '#type' => 'hidden',
* but you'll have nothing under the heading VOTE COUNT which is the original
* state when you create a poll in the original poll module
*/
313: if ($admin) {314: $form['chvotes'] = array(
315: '#type' => 'textfield',
316: '#title' => t('Votes for choice @n', array('@n' => ($delta + 1))),
317: '#default_value' => $votes,
318: '#size' => 5,
319: '#maxlength' => 7,
320: '#parents' => array('choice', $delta, 'chvotes'),
321: );
322
323: } else { // <-- add this (daniel's solution 1 )
324: '#type' => 'textfield',
325: '#title' => t('Votes for choice @n', array('@n' => ($delta + 1))),
326: '#value' => $votes,
327: '#size' => 5,
328: '#maxlength' => 7,
329: '#disabled' => 'disabled',
330: '#parents' => array('choice', $delta, 'chvotes'),
331: );
332: } // <--patch upto here
#5
Solution at #3 works for me.
#6
This should also work for 6.12 and 6.12-dev, I just tested it, now when a user edits his poll, it gives him a disabled number of votes which makes sense since he has no administrative privileges to change the number of votes, if he wants to delete his poll, he can also, thanks for testing. Unlike before, when the user edits his poll, they all become zeros and the vote cancel still appears and when he cancels the vote becomes negative 1.
#7
This is still an issue in 6.13. First off, the votes are set back to zero even if you don't change or add any of the vote options. Also, when viewing the poll, if you click the 'Votes' tab, you still get a list of who voted, despite the vote count being zero. Doesn't make any sense to me..
The patch in #3 works ... i think it should be committed.
#8
I have marked #484878: Poll module bug patch as duplicate of this report.
#9
And this is still an issue in 6.14. Solution at #3 works also for me.
#10
#11
Still exists in 6.16.
#12
When you submit a patch, you need to change the status to "needs review" so the test bot will see it.
You also need to see if this happens in D7 as it is unlikely to get fixed otherwise.
#13
The last submitted patch, poll-issue-pollmodulepatch-comment1.patch, failed testing.
#14
I got around this problem with a submit handler:
function fb_form_alter(&$form, $form_state, $form_id) {
switch ($form_id) {
case 'poll_node_form':
$form['buttons']['submit']['#submit'][] = 'fb_poll_vote_refresh';
return;
}
}
/**
* There is a core bug (http://drupal.org/node/362256) when editing a poll.
* We will recalculate the votes values on submit.
* One could also reinstate the vote values to the form (see the patch).
*/
function fb_poll_vote_refresh($form, &$form_state) {
$node = $form['#node'];
$update = db_query("UPDATE {poll_choices} c, {poll_votes} v SET c.chvotes = (SELECT COUNT(chorder) FROM {poll_votes} WHERE chorder=c.chorder AND nid=%d GROUP BY chorder) WHERE c.chorder=v.chorder AND c.nid=v.nid", $node->nid);
}
#15
Let's try this one.
#16
The last submitted patch, poll_362256.patch, failed testing.
#17
Arrrggghhh, how do you fix the line endings?
#18
@NancyDru #14
Be carefull, the code of #14 updates all polls with the vote-counts of the current poll!!! We need to only update the current node. Change the WHERE statement to
c.nid=%dlike this:<?php
function fb_form_alter(&$form, $form_state, $form_id) {
switch ($form_id) {
case 'poll_node_form':
$form['buttons']['submit']['#submit'][] = 'fb_poll_vote_refresh';
return;
}
}
/**
* There is a core bug (<a href="http://drupal.org/node/362256" title="http://drupal.org/node/362256" rel="nofollow">http://drupal.org/node/362256</a>) when editing a poll.
* We will recalculate the votes values on submit.
* One could also reinstate the vote values to the form (see the patch).
*/
function fb_poll_vote_refresh($form, &$form_state) {
$node = $form['#node'];
$update = db_query("UPDATE {poll_choices} c, {poll_votes} v SET c.chvotes = (SELECT COUNT(chorder) FROM {poll_votes} WHERE chorder=c.chorder AND nid=%d GROUP BY chorder) WHERE c.chorder=v.chorder AND c.nid=%d", $node->nid, $node->nid);
}
?>
This code needs to be placed in a custom module in order to work.
I dont feel good in recalculating the number of votes from the chorder. The chid would be much better, but i guess we dont have another choice here, since the chid is not stored in poll_votes.
#19
reroll of poll_362256.patch
#20
The last submitted patch, poll_362256-2.patch, failed testing.
#21
what about this?
#22
The last submitted patch, poll_362256-3.patch, failed testing.
#23
Nice try on the patches.
I had caught that code error and fixed it, I just didn't come back here and update. But thanks for finding it and telling me any way. Yes, it is in a custom module. No, I don't like the recalculation either; a fix for this problem would be much nicer.
#24
last try...
#25
Thanks for sharing the code, it saved me a couple of hours. I just wanted to help anyone who just copies and pastes the code. I did that and it worked (seemed to) until i realized that all polls had the same number of votes :)
I think its bad design, that the chid gets not saved in the poll_votes table.
#26
#27
The last submitted patch, poll_362256-4.patch, failed testing.
#28
#29
...try this
#30
#31
It looks like this is fixed in Drupal 7 - how about just backporting that fix?
http://api.drupal.org/api/function/_poll_choice_form/7
#32
Here's a backport of the D7 approach.
#33
The patch applies fine and fixes the issue - sort of...
Adding another choice after voting works fine - as it doesn't reset all votes to value 0 while giving you the option to cancel. There still is the possibility of a foolish admin messing things up. If you manually change all of the vote values to 0 after editing the poll, you still have the cancel your vote button.
Is that worth fixing, or simply the admin's fault?
#34
@mikestefff - sounds like that situation is a bug not yet addressed in Drupal 7, and I'm not sure what the right behavior is. If the admin changes any vote values (up or down), what does it mean for a user to "cancel" their vote?
#35
@pwolanin - I don't think it's even an issue worth addressing, I just thought I should point it out so everyone is at least aware of the conditions. Otherwise, the patch works and I suggest it's committed. I'm very surprised this has been lingering for the several releases now. I'm not going to change the status of the issue - I'll leave that to you.
Thanks
#36
given this is a straight backport from 7, I hope we are in good shape.
#37
I can't comment on the code itself, as I didn't analyze it. Functionality wise, it works fine.
#38
Backport patch #32 works well. Same problem of clearing votes from non-admin editors.
#39
Committed @pwolanin's backport to Drupal 6. Thanks for all the testing feedback!
#40
Nice. Thanks for clearing this up.
#41
Fabulous, Gábor and Peter. Any ideas when the next 6.x release will arrive?
#42
Automatically closed -- issue fixed for 2 weeks with no activity.