Taxonomy singular slug fix

[andysh-uk]

Fixes #2036.

Taxonomies now use the singular_slug when configured to do so, otherwise they fall back to the slug.

I've replicated the relationship between Content -> ContentFillListener -> content type config -> ContentType with taxonomies, so we have:

Taxonomy -> TaxonomyFillListener -> taxonomies config -> TaxonomyType.

The Twig getLink filter now uses the TaxonomyType singular_slug/slug value (Bolt seems to automatically substitute slug if singular_slug is not configured, so the method call looks like it only uses singular_slug - but this is the same as Content.)

There are a couple of issues I have:

• I feel this should have a test, as a behaviour change in future could break links. I'm happy to code such a test but I have no idea where to start, so any guidance would be appreciated 😄
• This is a potentially high-impact change. All sites created using Bolt will have singular_slug configured on the default categories and tags taxonomies, but this isn't being used currently. I feel the behaviour is now correct based on what is configured, but it is going to change the URLs generated with |link on a taxonomy. This should perhaps be either strongly called out in the release notes, or go into a bigger update - e.g. 4.2.

[I-Valchev]

Hi @andysh-uk , good catch, I know this has been bugging a colleague of mine, @nestordedios , for some time now 😅

For tests
I agree, it'd be great to have more tests! It is best to look at tests/e2e/display_record.feature to add some more tests there. There is a bit of documentation here, albeit not too much. You can take a look at the existing steps. For me, the IDE also auto-completes what is available?

If you bump into issues with it, reach me or @bobdenotter on Slack.

[I-Valchev]

About the big change, the other issue is that right now the addition to services.yaml won't be automatically included in people's existing projects. Prior to 4.0.0, we sort of sneaked those changes in because it was not a stable release. Now, we should not be introducing breaking changes in .x and .x.x releases. Let's see how best to approach this as well, as I'm sure it'll pop up in future too.

[andysh-uk]

Thanks @I-Valchev, I did wonder about the services.yaml change following the previous issue with the Doctrine change in 4.1 😄

Let me see how this works when that listener is not configured - I feel it would be acceptable to go into a smaller release if the code works the way it does prior to this change if services.yaml is not updated.

That way, you could highlight it in the release notes that to opt-in to this new behaviour for existing projects, you have to make the change manually.

I'm assuming newly-created projects would get the updated services.yaml, and so would start with the desired behaviour from the get-go?

[andysh-uk]

@bobdenotter @I-Valchev I'm struggling to understand the PHPstan failure on this PR. I can't see that it's anything to do with what I've changed but of course I could be wrong.

Are you able to offer any pointers please?

On my local branch on my Mac, phpstan runs fine without errors:

aheathershaw@Andys-MBP bolt-core % git branch
  master
* taxonomy-singular-slug-fix

aheathershaw@Andys-MBP bolt-core % php vendor/bin/phpstan analyse                
Note: Using configuration file /Users/aheathershaw/Git/bolt-core/phpstan.neon.
 261/261 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


                                                                                                                        
 [OK] No errors                                                                                                         
                                                                                                                        

aheathershaw@Andys-MBP bolt-core %

[bobdenotter]

Hi @andysh-uk ,

The failure is probably unrelated to your changes, see #2043

Sometimes it happens that a minor update in ECS or PHPStan suddenly detects a (valid) issue that it didn't spot previously. :-)

I'm going to rebase this PR, and then it'll probably pass.

[bobdenotter]

Merging in to 4.1 as well as master! Thanks, @andysh-uk

[bobdenotter]

(Oh, about that breakage, there's also this one: #2048)

[andysh-uk]

Thanks for sorting Phpstan @bobdenotter. I've just created another small PR #2051 , which prevents an exception being thrown if services.yml does not have the new listener configure - it falls back to existing behaviour.