Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
URL substitution plugins were introduced with #2786049: Make entity URL substitutions pluggable to support a wider variety of use cases..
The current substitution plugins have a plugin annotation that states which entity types a single plugin can handle.
This is too ambiguous and allows some plugins to be used with an entity type they don't support.
The file for entity example doesn't have a canonical link defined in the link template, but it is still possible to select the canonical substitution plugin.
Proposed resolution
Add a method to all substitution plugins that takes an argument of EntityTypeInterface. The plugin would then be able to use the entity type, and check if a certain link template exists, and/or check the type itself.
Remaining tasks
Find solutionAdd automated test
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | url_substitutions-2789021-8.patch | 18.08 KB | anon |
| #8 | innerdiff-2789021-6-8.txt | 9.1 KB | anon |
| #6 | url_substitutions-2789021-6.patch | 11.48 KB | anon |
| #6 | innerdiff-2789021-5-6.txt | 1.16 KB | anon |
Comments
Comment #2
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedYeah, I see what you mean. There are a few ways we could deal with this that I've thought about:
I think the method is the most flexible and will work for the most use-cases.
Comment #3
anonSo a method it is. I like your method name proposal isApplicable so lets call it that.
Whats still unclear is what type of argument it should take. I guess the easiest is to send the entity type id, like in the other method, and then load the Entity type from that in the SubstitutionManager.
For that we need to inject a EntityTypeManager I guess in the SubstitutionManager.
Any thoughts on this?
Comment #4
anonComment #5
anonComment #6
anonFixed some coding standards.
Comment #7
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedThis looks fantastic.
Comment #8
anonI found that there were some problems with the patch in #6.
When adding a file matcher to a profile, the default plugin would be set to canonical. The fix was to change the order of the arrays in the defaultConfiguration() and let the default value to be overwritten by the one defined in the file matcher plugin.
Also, the tests would fail as a part of the fix above. That is also fixed in this new patch.
I also remove the filterPlugins() method from the SubstitutionManagerInterface as that method does not need to be in the interface. Now its a protected method in the SubstitutionManager instead.
I would like this to be reviewed again before committing.
Comment #10
anonComment #11
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedBelated reply, but looks good. Thanks for getting this in.