StarsFormatter - Add back user, smart and dual option for star display and text

@fenstrat Applied the last patch. it couldn’t apply successfully . getting some error. can you please check on-this .

@Rinku Jacob 13 which patch did you try to apply?

The first one on #6 won't apply until 📌 Port tests to 8.x-1.x Fixed is first applied/committed. That's why I posted the 2nd combined patch which should apply to 8.x-1.x-dev.

Patch in #6 doesn't apply. Some of that is because of recent changes to the Fivestar code base, but there are quite a few hunks that fail outright because they contain context that was never in Fivestar (but may be added by other patches in other issues).
For example:

diff --git a/src/Element/Fivestar.php b/src/Element/Fivestar.php
index eb26de2..20eb2c1 100644
--- a/src/Element/Fivestar.php
+++ b/src/Element/Fivestar.php
@@ -53,6 +53,9 @@ class Fivestar extends FormElement {
    * Process callback: process fivestar element.
    */
   public static function process(array &$element, FormStateInterface $form_state, &$complete_form) {
+    /** @var \Drupal\Core\Render\RendererInterface $renderer */
+    $renderer = \Drupal::service('renderer');
+
     /** @var \Drupal\Core\Entity\EntityFormInterface $form_object */
     $form_object = $form_state->getFormObject();
     /** @var \Drupal\Core\Entity\EntityInterface|null $parent_entity */

This hunk fails because the $form_object lines were never part of Fivestar.

I have re-rolled #6 to account for the recent changes to Fivestar, but I don't know what the intention was here so I can't correct the stuff like $form_object which may or may not be relevant to this patch.

Likewise, the changes to FivestarTest and FivestarTestTrait all fail because it seems they are a diff from a version of FivestarTestTrait that has never been committed. I did not attempt to re-roll these hunks.

I removed the deletion of FivestarAjaxTest and the addition of FivestarJsTest because they are not relevant to this issue. I also removed the addition of assertFivestarStarOutput() because that method is not used at all in this patch.

I am not going to mess with the combined patch because there's far too many out-of-scope changes in there. This current issue is "StarsFormatter - Add back user, smart and dual option for star display and text" and those are the only things that should be done here.

So here's the re-rolled patch for this feature. It will not apply, for the reasons I mention above.

Thanks for working on this guys!

I'm adding a clean version of #9 that does apply and works.

Some notes.

Single Vote behavior issue:
I'm noticing that stars won't display after reloading the page. This occurs on content that only have one vote, e.g first vote. I'll share a fix that works for me next.

Attaching a fix for the single vote issue, now vote is kept after reloading the page.

The issue occurs when the Cron option is used when tallying results. There is a period of time where `$results` is empty, so the new code added in `VoteResultManager::getResults` for adding the current user's vote will not get executed. Running Cron fixes the issue too, but the same will happens if the user updates their vote.

The latest patch doesn't have allow_vote widget option. Is that by design?

Re #12: Yes, leaving out allow_vote is by design. It is a feature part of [##3131954].

This doesn't break any tests, but it also doesn't have any either. Probably should add them. But first, reviews.

#11 works here. I tested the "dual" option and its ok.
Works together with last "Rate content permission is not ported to D8" patch 🐛 Rate content permission is not ported to D8 Needs work .

What happened to the changes to the test cases that was part of #9?

This issue was started three years ago with a patch that just added "dual", without "smart" or "user" and with no test cases.

It feels like we're getting further from the goal of porting these three missing options from the Drupal 7 version of this module, with test cases to both demonstrate these new options work and to ensure they continue to work when other changes are made to the module.

@DrupalBubb: You only tested "dual" - that was part of the original patch. Can you take the time to test the other options?

@edysmp: You removed the changes to the test cases that were part of #9. Can you add them back in and make sure all the new options have proper tests?

I gave the options for text output "smart" and "user" a try. Works on nodes.
Did not test the other situations like comments, views, percentage vote-type, and stars configurations.
Formatter configuration of the voting field does not open at the first try, second try works always. Can't check on alpha5.

Today i gave the star configurations a chance. Smart and dual seem broken while dual configured as text output.

Dual text output only works correct if there is a user vote recorded. If there is no own vote (but from others), it shows no information about that situation, instead of someting like "No own vote."
But this should not show up if there are no votes at all, no double output needed.

In MR30 I've re-rolled the approach from #6 and also updated the tests.

1. Changes to constructor property promotion in VoteResultManager
2. Removes FivestarAjaxTest::createFivestarFieldTwo() as it was unneeded duplication, instead FivestarTestTrait::createFivestarField() has been updated to support the required options when creating fields for testing.
3. Fixes an issue in `Drupal\fivestar\Element\Fivestar` that was introduced in 📌 Fivestar Element drupal-check issues. Fixed where the 'smart' formatter wasn't working as expected. Yay for tests catching this.

I gave MR30 a test on a new drupal, again using 'dual' for text output. No real new informations, but maybe this was intended. Don't know. But here is my review:

The javascript fails opening the formatter configuration while running first time, like patch #11
The 'dual' text output still lacks the information "No own rating" if there are votes from others. See #20
The stars show the wrong colors when using average star display. should be (if logged in) average: orange, user: yellow

After this I tried the other star display options, still using 'dual' for text output:
Average, user and smart seem to work, but dual is broken. Smart and dual should be removed. Smart is mixing display information, dual showing two fivestar widgets? both useless.