Problem/Motivation
When installing on 8.7 with default configuration, the entity:media matcher is no longer defined, because the media entity no longer has a canonical route.
Proposed resolution
Add a special case for media entities.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | linkit-media-87-3040749-8.patch | 8.15 KB | berdir |
| #8 | linkit-media-87-3040749-8-test-only.patch | 5.01 KB | berdir |
| #4 | linkit-media-87-3040749-4.patch | 2.9 KB | berdir |
| #2 | linkit-media-87-3040749-2.patch | 2.31 KB | berdir |
Comments
Comment #2
berdirUpdating the patcher and also the media substitution plugin to check if there is a canonical link template and that the source field actually is a file entity reference, because the current code also breaks on things like remote media that isn't a file entity.
Comment #4
berdirAlso fixing that test fail.
Comment #5
pcate commentedJust updated to 8.7 and ran into this issue. Patch #4 fixed it.
Comment #6
anonNot sure I link the
return NULLpart here. I think that will break the filter as it uses$url->getGeneratedUrl(),Can't we use the empty GeneratedUrl instead of NULL?
Comment #7
anonSome kind of test for this would be good too.
Comment #8
berdirAdded a media matcher test with some basic test coverage, that covers this regression, more tests could be added in a separate issue IMHO.
I also extended SubstitutionPluginTest to cover a media entity without a compatible source field, and rewrote the whole implementation to use available API methods and check everything explicitly: having a field and that field being a reference to a file entity, otherwise falling back to the canonical if available. And yes, makes sense to return an empty Url object. Added test coverage with and without standalone canonical URLs.
Removed the change in EntityMatcherDeriverTest, that isn't strictly required anymore with 8.7.0.
no interdiff, almost nothing the same as the previous patch.
Comment #9
berdirFixed that HEAD test fail in #3052546: Convert LinkitUpdateTest to phpunit
Comment #10
chris burge commented@Berdir - Thanks for tackling this issue! Patch #8 tests successfully for me.
Comment #14
anonThanks for the patch, and the tests. This is commited.