Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sumit kumar created an issue. See original summary.

Sumit kumar’s picture

Title: Cancel rating on when you add five star field into you form or contenttype not working » Cancel rating icon is not wroking. when you add five star field into you form or contenttype not working
Sumit kumar’s picture

ShekharPaatni’s picture

Status: Active » Needs review
FileSize
882 bytes
gg24’s picture

Status: Needs review » Reviewed & tested by the community

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!

gg24’s picture

Status: Reviewed & tested by the community » Needs work
pratik_kamble’s picture

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

ShekharPaatni’s picture

Assigned: Unassigned » ShekharPaatni

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

dbt102’s picture

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

TR’s picture

Title: Cancel rating icon is not wroking. when you add five star field into you form or contenttype not working » Cancel rating icon is not working. when you add five star field into you form or contenttype not working
Issue tags: -ui, -code

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

Harlor’s picture

Status: Needs work » Needs review
FileSize
1.78 KB

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

edysmp’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

latest patch doesn't apply.

edysmp’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.79 KB

rerolled patch.

TR’s picture

Assigned: ShekharPaatni » Unassigned
heddn’s picture

TR’s picture

Issue tags: +Needs tests
heddn’s picture

Issue tags: -Needs tests
FileSize
2.86 KB
1.32 KB

Test added

TR’s picture

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.

Status: Needs review » Needs work

The last submitted patch, 18: 2899838-17-test-only.patch, failed testing. View results

TR’s picture

Status: Needs work » Needs review

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?

heddn’s picture

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.