Cancel rating icon is not working

Hi @ShekharPaatni,

I have tested this functionality and this works as expected.

Testing steps:-

1. Added a field of type fivestar rating in a content type.
2. Selected a default value of stars in field settings.
3. Checked Allow users to cancel their ratings. checkbox and saved the field.
4. Edited the same field and clicked on cross button.
5. It clears the stars selected.

But it is not working when I try to create a node and rate that node. Now if I try clicking on cross button either using node form or node view than it does not work.

Thanks!

Hi @ShekharPaatni and @gg24,

I have tested the functionality, it clears the stars on clicking cross button. But however on node view for page refresh, it does not reflect it.

Testing steps:
1. Added a field of type fivestar rating in a content type.
2. Selected a default value of stars in field settings.
3. Checked Allow users to cancel their ratings. checkbox and saved the field.
4. Edited the same field and clicked on cross button.
5. It clears the stars selected.
6. Added content with fivestar field. On node view after clicking the cross button it clears the stars but on refresh it is not getting reflected

Hi @pratik_kamble and @gg24 ,

The issue of the cancel rating is done, now i am working with the functionality of the node rating.

Regards,
@ShekharPaatni

Thanks for you work on this @ShekharPaatni, @gg24 & @pratik_kamble ... looking forward to getting it committed

@ShekharPaatni Are you planning to finish the work on this?

This is certainly not an elegant solution but for me this works.

latest patch doesn't apply.

rerolled patch.

Re-rolled

Test added

Here's a patch with just the test from #17. This should fail, to confirm the problem and to confirm that the test is actually triggering the problem and verifying the fix.

Good. That was as expected.

In this hunk:

     }

     // Add new vote.
-    $vote_manager->addVote($entity, $vote_rating, $field_settings['vote_type'], $owner->id());
+    if ($vote_rating !== 'cancel') {
+      $vote_manager->addVote($entity, $vote_rating, $field_settings['vote_type'], $owner->id());
+    }

     // Check to see if there is a target entity. If so, add the vote to that
     // entity as well.

Shouldn't the if ($vote_rating !== 'cancel') conditional also encompass the next code block starting with "Check to see if there is a target entity"? If we're canceling, we can skip that block as well. That was done in the original patch in #11.

Also, shouldn't this be using $form_state->hasValue('vote'), which is a boolean instead of $form_state->getValue('vote'), which could have an actual value of 0 and therefore evaluate to FALSE?

re #20:
Fixed first point.
Second point can't be changed because cancel actually passes along an empty value back and hasValue considers it to exist. getValue is actually the right thing. I tried playing around with it and tests failed when I changed to hasValue. Yeah tests.

#21 looks good. However for the test, rather than just tack the asserts onto an existing test, I think it'd make more sense to split it off into it's own test method? Also, the cancel button should work without JS right, so does this need to be an Ajax/WebDriverTestBase test (i.e. could it be in Functional\FivestarTest instead?

Would also be nice to see this go green once the expanded test coverage over in #3447756: Improve test coverage and add back "Allow voting" display option lands.