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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quicksketch’s picture

Status: Needs review » Needs work

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:

    case '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']);
        }
      }
sin’s picture

Status: Needs work » Needs review
FileSize
966 bytes

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.

sin’s picture

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?

sin’s picture

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:

_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?

abautu’s picture

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.

TUc’s picture

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).

sin’s picture

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'].

vivianspencer’s picture

This works great for registered users but doesn't work at all for anonymous votes

vivianspencer’s picture

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

$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);
}
fossie’s picture

FileSize
1.79 KB

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

quicksketch’s picture

Status: Needs review » Fixed
FileSize
3.58 KB
4.2 KB

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.

gunzip’s picture

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)

gunzip’s picture

Status: Fixed » Needs review
crea’s picture

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.

crea’s picture

Title: CCK Fivestar NodeReference, deleting node do not remove vote » CCK Fivestar: deleting review node does not remove votes with axis other than 'vote'
Version: 6.x-1.13 » 6.x-1.x-dev
Status: Needs review » Active

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.

crea’s picture

Status: Active » Needs review

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

/**
 * 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;
  }
}
quicksketch’s picture

Status: Needs review » Fixed

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.

quicksketch’s picture

gunzip’s picture

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)

quicksketch’s picture

1. i write two comments (nodereference) to a blog post and i rate this content two times (through my two comments)

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).

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Daniel Wentsch’s picture

Issue tags: +comments, +Fivestar, +spam, +unpublished, +antispam

Posting into this issue as I think it's strongly related:
Hasn't anybody yet had the same problem with unpublished comments?
I'm using Fivestar only with comments and AntiSpam/Akismet module does a great job filtering spam out. Unfortunately spambots give votes quite often, which then are counted, weather or not the actual comment gets unpublished or even deleted. This makes the entire voting process rather useless, as there are way more spambot votes than real users' votes :(

If somebody thinks this deserves an issue of its own I'll move it to a new topic, just tell me.

bkosborne’s picture

Status: Closed (fixed) » Needs work

Okay I'm going crazy. I've been staring at VotingAPI and Fivestar code literally all day today... I'm having this problem though. Why is this set to fixed?

I was using v 1.19, and now just switched to the dev release, but whenever I delete "reviews" with CCK ratings on them for ANONYMOUS posts, the rating remains in the database. I put some callouts in the fivestar_field.inc file in the fivestar_field function where it just prints out the operation whenever the function is called.

When I create review nodes, SANITIZE and LOAD are performed - neither of which are handled in this function anyway. When I delete the node, the function isn't called at all (???). In fact, I can't find the function that IS called when the review node is deleted to remove the info from the database. Deleting review nodes for real users works fine - it's just for anonymous users.

bkosborne’s picture

Status: Needs work » Active

Okay - I found a problem. Ignore my last paragraph up there - I wasn't using DPM() properly. They were being called.

Here's what happens and why the module doesn't delete votes from anonymous users

I'm logged in as an admin, and I go to delete a review node posted by an anonymous user...

1 - fivestar_field is called with operation "delete".
2 - _fivestar_cast_vote is called, passing in the node authors user id, which in this case is 0 (for anonymous)
3 - the first three lines of _fivestar_cast_vote will determine that $uid is empty, and instead supply it the currently logged in user's id. I have no idea why this is done. This is part of the problem.
4 - the criteria array for selecting the vote from the database begins construction in _fivestar_cast_vote.
5 - the UID of the array is set to $uid, which in this case is now the currently logged in user
6 - votingapi_current_user_identifier is called, which in this case will return an array with the current logged in users id, and is combined with the existing criteria array, doing nothing (because the uid is the same in both).
7 - votingapi_select_votes is called, passing in this criteria array. this is where the votes are selected from the table.

-------- this is a BIG problem !! It's impossible to delete anonymous votes from the database. Also, if an admin who also posted a review of the node (in my case, a product) with the same value goes to delete any anonymous users vote, they will be deleting their own vote instead of the anonymous users!. This is because the current user's ID is passed in. I just tested this to be sure.

It seems to be that the module was tested/designed around forcing users to register to vote. If users are forced to register, they will all have unique user ID's, which can be used to select their vote from the votes table. My site allows anonymous votes, hence the issues.

I've been pondering a resolution to the problem. As the DB structure is written, there is no way to accurately select a specific anonymous user's vote from the voting table to delete. When an anonymous vote goes in, the IP address is recorded, which can't be used to select the vote to delete because we don't have that info recorded anywhere else.

I'll keep working on this and post back. I would appreciate any input from the commiter's. Thanks

Exploratus’s picture

This is a hiuge problem. I thought this was resolved, but it is still cropping up all over my website. Comment votes are not deleted with a comment..

bkosborne’s picture

@whatmetropolis I fixed this issue on my site and I guess forgot to post a response here. Maybe I was waiting for the maintainer to initiate contact.

I hacked the module a bit to get it working. I can only confirm that it works properly for my setup:

I have a product content type with fivestar rating enabled on it. I then use nodecomment for a "product review" content type and added a CCK fivestar rating field. The rating target is set to "nodecomment parent". If this is similar to your setup, let me know and I can help you out.

whiteph’s picture

Issue summary: View changes
Status: Active » Closed (won't fix)

We can no longer support the Drupal 6 version of Fivestar. It is in security maintenance mode only. When the Drupal 8 version of Fivestar is released, the Drupal 6 version will be officially deprecated.

whiteph’s picture