I'm trying to extend simplevote to allow voting for guests, but the following snippet in votingapi_set_vote() ...

    $result = db_query("SELECT * FROM {votingapi_vote} WHERE content_type='%s' AND content_id=%d AND tag='%s' AND value_type='%s' AND uid=%d", $content_type, $content_id, $vote->tag, $vote->value_type, $uid);
    while ($vobj = db_fetch_object($result)) {
      _votingapi_change_vote($vobj, $vote->value);
      $exists = TRUE;
    }
    if (!$exists) {
      _votingapi_add_vote($content_type, $content_id, $vote->value, $vote->value_type, $vote->tag);
    }

would prevent me from storing more than one vote for uid=0. ie. the second guest sending a vote for a particular subject would override any previous vote sent by another guest.

So I thought I could use a fake uid for guests, for now.

I'm wondering if you plan to add the ability to allow more than one guest to keep its own vote. :-)

Some may not wish this feature, but depending on the nature of the site (its content) it might be neccesary. Maybe something optional, anyway, it probably depends on the module using the voting api. I think.

Thanks

CommentFileSizeAuthor
#2 votingapi.module.patch3.79 KBeaton

Comments

markus_petrux’s picture

markus_petrux commented

Indeed, passing a fake UID to votingapi_set_vote() works and, surprisingly enough, the UID stored in the database is the UID of the current user, regardless of the value passed to votingapi_set_vote().

It works because the $uid is not passed to _votingapi_add_vote():

_votingapi_add_vote($content_type, $content_id, $vote->value, $vote->value_type, $vote->tag);

...which seems a bug though. It should probably look like:

_votingapi_add_vote($content_type, $content_id, $vote->value, $vote->value_type, $vote->tag, $uid);

Maybe I should have filed another issue as a bug report?

eaton’s picture

eaton commented
Status: Active » Needs review
StatusFileSize
new votingapi.module.patch3.79 KB

Most certainly. Thanks for catching this, it hasn't been exercised much.

The trick, of course, is *retrieving* the votes that are cast. Part of the difficulty right now is that the API assumes that a vote being cast by the same user ID is a *changed* vote.

I've attached a patch against the latest CVS that seems to do the trick, though I haven't explored all the options and it could easily be very inefficient. It also ignores any attempts by an anon user to vote if a registered user has already voted from the same IP address. This prevents users from 'ballot stuffing' by loggin in, voting, then logging out and voting.

Any thoughts? I'd appreciate feedback on how this solution works.

markus_petrux’s picture

markus_petrux commented

Please, don't use $_SERVER['HTTP_X_FORWARDED_FOR']. See this:
http://drupal.org/node/20471

In fact, I was about to open another issue for that (used already in _votingapi_add_vote).

Then, I would prefer to see this as an option, so the module that uses votingapi can decide what to check. For a project I'm working on, I'm particulary interested to allow users to cast votes as much as they wish. The site is ad based, so each click to cast a vote means a page refresh, with new adds, so I think that may increase the possibility to catch more ad clicks.

Thanks for quick response. :-)

markus_petrux’s picture

markus_petrux commented
Status: Needs review » Needs work

I would say...

eaton’s picture

eaton commented

Thanks for the heads up, particularly regarding the HTTP headers issue. I need to take a closer look at that one; the reason it uses that header is because of a previously filed issue; if it's not the right data to use I need to take another look at how the hostname data is stored.

I think that the piecemeal nature of this particular patch is going to be a problem. Can we step back for a moment and identify exactly how you'd like this to operate? It might make more sense for you to use the internal functions to add and update votes directly, bypassing the standard votingapi_set_vote() function entirely. That WOULD give you complete control over what gets recorded and what doesn't; it would just require rolling your own custom equivalent to the votingapi_set_vote() function. Because the calculation functions don't do any filtering based on uid or hostname, other modules that dispay votingapi based data wouldn't be affected.

Any thoughts?

markus_petrux’s picture

markus_petrux commented

Maybe you could turn _votingapi_add_vote() into a public function? I think I could simply use that (instead of votingapi_set_vote) for what I need now (ie. allow users to cast as much votes as they wish). My module could even use permissions to change behaviour if the need arises. ie. call votingapi_add_vote or votingapi_set_vote or even restrict votes to a particular role.

Note: There is a pending "bug" here. _votingapi_add_vote() needs to get the $uid.

markus_petrux’s picture

markus_petrux commented

I have changed my little module to invoke votingapi_add_vote() :-)

...but I had to call _votingapi_recalculate_results($type, $cid) :-/

Maybe you could call from _votingapi_recalculate_results() within votingapi_change_vote() and votingapi_add_vote(), so the following code could be remove from votingapi_set_vote() ?

  if ($recursion) {
    return $vobj;
  }
  else {
    return _votingapi_recalculate_results($content_type, $content_id);
  }
eaton’s picture

eaton commented

I'd prefer to make a publically exposed recalculation command -- isolating that functionality from the rest of the code is actually on my to-do list, as I want to support delayed vote processing via cron.

But in terms of automatically triggering recalculation via add and delete, I'm not sure. The goal of the API is to provide a cnetral function (set_vote) that most modules can use, and a cloud of helper functions (add_vote, delete_vote, change_vote) that can be used by modules that need much finer control. In the latter case, i'd be more comfortable with the module controlling when to handle recalculation.

markus_petrux’s picture

markus_petrux commented

recalc_results, add_vote and change_vote seem to be at the same level of the API. ie. it is used by set_vote, but it looks reasonable to be able to use them directly, so one can manage several votes and them trigger the recalc.

either way would be fine, anyway, as long as one doesn't need to invoke a "private" function.

eaton’s picture

eaton commented
Status: Needs work » Fixed

The latest CVS checkin of votingapi.module includes publically exposed add, change, and delete functions. Right now, the 'delete' function still triggers automatic recalculation, but this will be changing shortly as I touch base with the handful of module writers who've used the function and depend on its current behavior.

I've also added a votingapi_unset_vote(type, id, uid) method.

With this set of changes, there's a clear path for vote manipulation. If you want to do things 'the easy way,' use votingapi_set_vote() and votingapi_unset_vote(). The api will handle lots of stuff for you. If you want more control, votingapi_add_vote, votingapi_change_vote, votingapi_delete_vote, and votingapi_recalculate_results() will let you control the flow completely.

I haven't tagged this set of changes as 4.7 yet, as there's another batch of work -- relating to the Voting Actions system -- that is still underway. Once that's done, this CVS code will be in the 'official' 4.7 release.

markus_petrux’s picture

markus_petrux commented

Thanks for all the hard work. Looks great! :D

Cheers

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)