Fix "Give @title 5/5" rating link

MR 33 should be the correct fix, i.e. just a missed update from entity_type to entity_type_id.

I'll circle back and update all the tests where this is affected after 📌 Improve test coverage and add back "Allow voting" display option Needs review lands.

This bug was introduced by the changes made in 🐛 Deprecated function addcslashes() warning caused by empty value Fixed , specifically the IMO gratuitous and out-of-scope and un-discussed changing of the $entity_type local variable and the 'entity_type' array key to $entity_type_id and 'entity_type_id' in a lot (but NOT ALL) of the Fivestar code. One of the places that did not get changed was the Fivestar element, which why MR !33 above had to make that change to restore the correct behavior.

This error was compounded by the fact that the above issue also introduced a test case for the "Give it" output that was written using the WRONG behavior caused by that issue. So now the fix has to correct the original mistake plus fix the test to look for the correct expected output.

This is one of the reasons why I push back on throwing all sorts of unrelated changes into one big patch. Rather than simply fix the issue, a whole lot of other things were thrown into the pot and in combination they broke something else and locked in that broken behavior by providing a broken test.

This is also why I insist on adding test cases for bug fixes and for new features. The purpose of a test is to first of all establish the correct behavior, then demonstrate (for a bug) that the code behaves *incorrectly*, causing the test to fail, and that the bug fix causes the test to work. Likewise for a new feature - the test should demonstrate how the feature is supposed to work and serves to prove the patch implements the feature properly and that future changes to this module don't break the functionality of the new feature.

So while it's easy and tempting to just change a few variables here and make the errors go away, that easy way out is the path that led us to the current situation, where Fivestar is a mess of spaghetti code with almost no testing in place - not even basic testing of the field type/widget/formatters, let alone more complex tests.

I'm going to fix this by first changing the test to test for "Give @title X/Y", which is the way it always used to work going back to at least Drupal 7. The test will then fail, because the code is currently doing the wrong thing. Then I will be reverting the name change from entity_type to entity_type_id everywhere, which should make the test run green again.

The changes that MR !33 made to src/Element/Fivestar.php will not be needed, because those lines of code have been working for at least 7 years that I have been able to trace back, so they're not the cause of the problem. The cause it the renaming of the variables elsewhere and the test case that is testing for the wrong behavior.

I will be making these changes in the new 3.0.x branch, and opening a new MR because that's easier (for me) than trying to fix up what has been done up until now.

TR → changed the visibility of the branch 3447761-fix-give-title to hidden.

TR → changed the visibility of the branch 3.0.x to hidden.

TR → changed the visibility of the branch 8.x-1.x to hidden.

Test fails as expected - it is looking for "Give Test Node 5/5" but is finding the (wrong) value "Give Test Node 5/5" instead.

Now lets change those variable names so that the correct value appears.

And there you have it. Test now passes, and wrong output that was noted in the original post is now fixed and tested. If patches in future issues breaks this functionality, the test will tell us so we don't introduce this bug again.

@TR nicely done, LGTM.

After this lands I look forward to progressing 📌 Improve test coverage and add back "Allow voting" display option Needs review where I've tried to improve the tests in general.

Automatically closed - issue fixed for 2 weeks with no activity.