Fivestar ratings: "Illegal choice 100 in Score element"

Thanks yaworsk for trying to recreate the issue.
I´m using the following versions:
NodeReview 6.x-1.x-dev (2011-Apr-16)
Fivestar 6.x-1.19
Voting API 6.x-2.3
Drupal core 6.22

I´ve reinstalled Voting API, Fivestar and NodeReview since I´ve been playing with different voting types ('normal' and 'label') to clean out previous votes, just in case votes of different types might be causing confusion to 'fivestar'... but the issue remains active yet.

Actually, I realize I can vote 1 or 2 stars (20 or 40 values), but cannot 3, 4, nor 5 (60, 40, 100 values).

This patch seems to solve the problem for me.
In sites/all/modules/nodereview-6.x-1.x-dev/nodereview/nodereview_node_nodereview.inc, the loop in line 100 was provinding the following values....

$options[10]=1
$options[20]=2
$options[30]=3
$options[40]=4
$options[50]=5

...and that´s why it could only be selected 1 star (value 20) and 2 stars (value 40), because the other ones had values higher than 50 (namely: 60, 80, 100).

I've tried to create a patch following this post, with the command below:

diff -up
sites/all/modules/nodereview-6.x-1.x-dev/nodereview/nodereview_node_nodereview.inc
sites/all/modules/nodereview-6.x-1.x-dev/nodereview/nodereview_node_nodereview.inc.new
> nodereview/nodereview-fivestar_type_illegal_choice-1176242-3.patch.n

Though I´m not very sure it is the right way. Please someone to provide a proper patch if mine is not ok.

The patch content is as follows:

--- sites/all/modules/nodereview-6.x-1.x-dev/nodereview/nodereview_node_nodereview.inc 2011-04-15 15:40:47.000000000 +0100
+++ sites/all/modules/nodereview-6.x-1.x-dev/nodereview/nodereview_node_nodereview.inc.new 2011-06-05 12:46:52.000000000 +0100
@@ -96,7 +96,10 @@ function _nodereview_form_review(&$form,
for ($i = 1; $i < ($maxrating + 1); $i++) {
if (NODEREVIEW_LABEL_ENABLE) {
$options[$i * 10] = variable_get('nodereview_rating_label' . $i, '');;
- }
+ }
+ elseif (NODEREVIEW_FIVESTAR_ENABLE) {
+ $options[$i * 20] = $i);
+ }
else {
$options[$i * 10] = $i;
}

The previous patch had an extra ) in line $options[$i * 20] = $i);
The corrected patch looks as follows

--- sites/all/modules/nodereview-6.x-1.x-dev/nodereview/nodereview_node_nodereview.inc 2011-04-15 15:40:47.000000000 +0100
+++ sites/all/modules/nodereview-6.x-1.x-dev/nodereview/nodereview_node_nodereview.inc.new 2011-06-05 12:46:52.000000000 +0100
@@ -96,7 +96,10 @@ function _nodereview_form_review(&$form,
for ($i = 1; $i < ($maxrating + 1); $i++) {
if (NODEREVIEW_LABEL_ENABLE) {
$options[$i * 10] = variable_get('nodereview_rating_label' . $i, '');;
- }
+ }
+ elseif (NODEREVIEW_FIVESTAR_ENABLE) {
+ $options[$i * 20] = $i;
+ }
else {
$options[$i * 10] = $i;
}

Thanks sincerely yaworsk for your comments and explanations, and sorry for the mess!! I've just got Git set up for proper patching in the future.

Following your clarification for the math, the for-loop in general may welcome the correction you suggest, which I think is the one in the code below. I've tested it for the 3 voting types (normal, label, fivestar), with different values for 'Maximum Rating' (in admin/content/nodereview), and checked that the % stored in votingapi_vote covers all the range till 100%.
Anyway after 2 wrong patches from my side, I better may just wait for yours.

if (NODEREVIEW_LABEL_ENABLE) {
$options[$i * 100/$maxrating] = variable_get('nodereview_rating_label' . $i, '');
}
else {
$options[$i * 100/$maxrating] = $i;
}

Thanks again!!

Thanks for the patch yaworsk.
I've tested the correct storage of values in the database and seems to work all right.

The only weird thing that comes up is the display of values retrieved from the DB (now that the storage bug has been sorted out, the old buggy display begs for a similar fix). For instance playing around with $maxrating and the voting type (normal, label, fivestar) I've seen things like "6,6/3" instead of "2/3".

I think the fix could be in the hook nodereview_nodeapi() at line 394 in the .module file:

    if (NODEREVIEW_FIVESTAR_ENABLE) {
          $total = $total . '%';
        }
        elseif (NODEREVIEW_LABEL_ENABLE) {
--       $total = variable_get('nodereview_rating_label' . round($total/10), '');
++       $total = variable_get('nodereview_rating_label' . round($total/$weight), '');
        }
        else {
--       $total = round($total/$weight) . '/' . $maxrating;
++       $total = round($total/$weight);

Anyway, I don´t know if this should go to a different issue.

To replicate the strange behaviour I commented in #8:
1) install nodereview
2) apply the patch in comment #7
3) provide several ratings for a given node with maxrating = 10
4) change maxrating to 6, for instance
5) provide additional ratings for the same node in (3)
6) re-visit the node in (3), and you should see strange reviews like 8,8/6

I think the bug is in the 2 lines I suggested to change in comment #8.
--bug $total = variable_get('nodereview_rating_label' . round($total/10), '');
++sugestion $total = variable_get('nodereview_rating_label' . round($total/$weight), '');
(since the value we enter in the database, after applying the patch in #7, is "$i * $weight", instead of the old "$i*10": ""$options[$i * $weight] = variable_get('nodereview_rating_label' . $i, '');"")

--bug $total = round($total/$weight) . '/' . $maxrating;
++sugestion $total = round($total/$weight);
(since "$weight = 100 / $maxrating;" we are dividing twice by $maxrating: )

Applying manually these 2 changes, the reviews seem to behave correctly in my test environment.
I hope this can be of some help.

Thanks yaworsk again for spotting a fault in my suggested solution. I missread the string concatenation as an expression.

Regarding the numerical calcultations, for some reason I wrongly thought that the stored values in the database were normalized to 100; this is: a 1/4 rating would store like 25; and if you change maxrating to 10, then the retieved 25 would be translated to 2.5/10. But I realize now from your explanation that it doesn't work like that.

Thank God there is peer-review in open-source, or for every minor fix a bunch of bugs would slip in.