CCK Fivestar: deleting review node does not remove votes with axis other than 'vote'
sin - October 8, 2008 - 16:38
| Project: | Fivestar |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | sin |
| Status: | closed |
Description
Hi!
I have a review node type with Node Reference field and Fivestar field set up to cast vote on referenced node. When I delete review node the vote do not get deleted from target node.
Please review a patch which corrects this problem for me.
CCK 6.x-2.0-rc8
Voting API 6.x-2.0-rc1
| Attachment | Size |
|---|---|
| CCK_fivestar_nodefield_delete.patch.txt | 1.11 KB |
#1
I think this would be a great addition... but I think this might have already been fixed. The patch you've provided seems to be against the 1.12 version, not 1.13 as it doesn't apply. In 1.13, we're already using logic that deletes votes when the node is removed:
<?phpcase 'delete':
foreach ($items as $delta => $item) {
if ($field['dynamic_target'] && !empty($node->$field['dynamic_target'])) {
$items[$delta]['target'] = $node->$field['dynamic_target'];
}
elseif (!empty($item['php_target'])) {
// Use eval rather than drupal_eval to allow access to local variables.
$items[$delta]['target'] = eval($item['target']);
}
if (is_numeric($items[$delta]['target'])) {
_fivestar_cast_vote('node', $items[$delta]['target'], 0, $items[$delta]['axis'], $node->uid);
votingapi_recalculate_results('node', $items[$delta]['target']);
}
}
?>
#2
Hi, quicksketch, the issue is in 6.x-1.13, I checked again, and exactly with the code you posted above...
is_numeric($items[$delta]['target']) is FALSE for me, because $items[$delta]['target'] contains an array(0 => array('nid' => ...)), so I used workaround.
I've cleaned up the patch.
#3
There is another issue, related to this.
When review node is unpublished the vote is still cast by its NodeReference field. I was required to change this behaviour. Is it a feature or a bug?
#4
The patch above solves vote deletion problem only for registered users. It do not do anything with anonymous votes and they do not revoked from object node (target of NodeReference field) when I delete anonymous review with fivestar_field.
I do not found a place where fivestar_field relation with votingapi_vote table is stored. I need vote_id from this relation to delete vote on review node with fivestar_field deletion. Can anybody give a clue here? Isn't it worth to create vote_id field in content_type_review CCK table?
In the code above on fivestar_field deletion we call:
<?php_fivestar_cast_vote('node', $items[$delta]['target'], 0, $items[$delta]['axis'], $node->uid);
?>
where $node->uid == 0 in described case and do not represent unique vote, imo vite_id is required here to delete relevant vote.
Any ideas how to implement anonymous vote deletion when deleting node with fivestar_field?
#5
I don't know if this helps, but this is something I've notived in Fivestar 1.2.2 (for Drupal 5): the code for deleting the vote is there, but it doesn't work when the field has an axis other than "vote". I tracked down the problem to the following issue: fivestars "declares" database columns: rating and target, and the "field settings": 'stars', 'dynamic_target', 'php_target', 'axis'. When CCK calls fivestar_field for insert and update events, then the $items[$delta] arrays also contain the axis information (besides rating and target). Yet, when it is called for delete events, the $items[$delta] arrays only contain the rating and target. No axis. So Fivestar tries to cancel a vote on the "vote" axis.
In order to solve this problem, I added the line
$axis = $field['axis'];at the begining of fivestar_field and replaced $items[$delta]['axis'] with $axis in the update and delete sections of the main switch.
#6
I have exactly the same use case as sin. A review node that reviews a referenced node by using fivestar.
I wrote a patch that both solves the is_numeric() call from #2 and the axis-issue ($items[$delta]['axis'] is not set on the 'delete' operation).
#7
Isn't using $field['axis'] instead of $items[$delta]['axis'] can break multivalue fivestar fields? I actually never use them and don't know if they are supported at all. I think we should track why delete and update do not set correct $items[$delta]['axis'].
#8
This works great for registered users but doesn't work at all for anonymous votes
#9
Ok I've found a solution that'll remove anonymous votes also, it's not pretty but works for me
using the Rules module I created a rule which executes the following PHP code once a review has been deleted, just replace 'field_service_provider' with your node reference field
<?php$result = db_query('DELETE FROM votingapi_vote WHERE content_type = "node" AND content_id = %d AND timestamp = %d', $node->field_service_provider[0]['nid'], $node->changed);
votingapi_recalculate_results('node', $node->field_service_provider[0]['nid']);
if ($result) {
drupal_set_message('Successfully removed vote for '. $node->title);
} else {
drupal_set_message('Failed to remove vote for '. $node->title);
}
?>
#10
I don't know what was wrong with the patches above, but I couldn't patch my clean fivestar module.
Hereby another patch with additional check to see if the referenced node is published. If it becomes unpublished, the vote is also removed from the table.
Using the $field['asis'] seems to work within the delete case of the field $op.
If someone can check this patch it will be great.
HTH,
Fossie
#11
Based on sin's original patch I've updated the code to abstract it slightly for consistency between update/insert/delete. This should correct the problems described here. Please reopen if this problem exists in the latest CVS version.
#12
sorry if i reopen this, but i supspect that the empty axis problem persists (as stated above by other users)
i had to put a line like this in the deleted branch of the switch to get it working:
if (empty($items[$delta]['axis'])) $items[$delta]['axis'] = $field['axis'];(i also think that the 2 branches of the switch can be unified as the code now looks very similar)
#13
#14
I can confirm what gunzip said.
I have multi-axis CCK voting system setup.
I just installed latest CVS 5.x version and deleting review node won't remove it's vote. However unpublishing and publishing review node works as expected both ways.
#15
Meanwhile I switched my dev site to Drupal 6.x
Tried latest dev http://ftp.drupal.org/files/projects/fivestar-6.x-1.x-dev.tar.gz
Problem still exists, only 'vote' axis vote is deleted from the votingapi_vote table when review node is deleted.
#16
Sorry no time for proper patch, I will just provide final code that works for me.
As it was noted in #5 by abautu, there was no reason to use $items[$delta]['axis'] because 'axis' is field setting, not field value.
Also I united branches as gunzip asked
<?php
/**
* Implementation of hook_field().
*/
function fivestar_field($op, &$node, $field, &$items, $teaser, $page) {
$fieldname = $field['field_name'];
switch ($op) {
case 'insert':
case 'update':
case 'delete':
foreach ($items as $delta => $item) {
if ($node->status == 0 || $op == 'delete') {
$rating = 0;
} else {
$rating = $items[$delta]['rating'];
}
$target = fivestar_field_target($node, $field, $item);
if (is_numeric($target)) {
_fivestar_cast_vote('node', $target, $rating, $field['axis'], $node->uid, FALSE, TRUE);
votingapi_recalculate_results('node', $target);
}
}
break;
case 'sanitize':
$items[0]['stars'] = $field['stars'];
break;
}
}
?>
#17
Thanks crea, I really, really would've preferred a patch for this but given that I'm releasing a new version today I made sure to include your changes. It looks like it works alright, it should be included in the 1.14 release.
#18
Patch of changes: http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/fivestar/fi...
#19
i leave this as fixed but i wonder: is it really right to erase (set to zero) __all__ votes from the user when a node is deleted ?
probably it's better to subtract the field value/rating from the current vote (i mean something like
_fivestar_cast_vote('node', $target, -$rating, $field['axis'], $node->uid, FALSE, TRUE... notice the minus sign, i just don't know if votingapi support this)
assume this:
1. i write two comments (nodereference) to a blog post and i rate this content two times (through my two comments)
2. i delete one of this two comments => both of my votes are deleted for the blog post
(this is strictly related to what sin said in #4: vote_id is crucial)
#20
Fivestar only allows one vote per user no matter what, so the assumption is that this review contains the only rating that user has made. If a user posts multiple reviews, only the last review actually has an effect on tallied average for that piece of content.
That said, a negative vote (such as -100) would have a very odd effect, it would actually cause the user's vote to become the opposite of what they had voted when the node was deleted. That is, if they gave an item 5 stars, it would effectively mark their vote as negative 5 stars. However... this isn't really supported by Fivestar (or VotingAPI either, afaik) since Fivestar has no way of showing a negative percentage (nor would it really make any sense).
#21
Automatically closed -- issue fixed for 2 weeks with no activity.