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

Comments

Berdir created an issue. See original summary.

berdir’s picture

Status: Active » Needs review
StatusFileSize
new2.31 KB

Updating 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.

Status: Needs review » Needs work

The last submitted patch, 2: linkit-media-87-3040749-2.patch, failed testing. View results

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new2.9 KB

Also fixing that test fail.

pcate’s picture

Just updated to 8.7 and ran into this issue. Patch #4 fixed it.

anon’s picture

Status: Needs review » Needs work

Not sure I link the return NULL part here. I think that will break the filter as it uses $url->getGeneratedUrl(),

Can't we use the empty GeneratedUrl instead of NULL?

anon’s picture

Some kind of test for this would be good too.

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new5.01 KB
new8.15 KB

Added 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.

berdir’s picture

chris burge’s picture

@Berdir - Thanks for tackling this issue! Patch #8 tests successfully for me.

The last submitted patch, 8: linkit-media-87-3040749-8-test-only.patch, failed testing. View results

The last submitted patch, 8: linkit-media-87-3040749-8-test-only.patch, failed testing. View results

  • anon committed fd85351 on 8.x-5.x authored by Berdir
    Issue #3040749 by Berdir, anon: Drupal 8.7 no longer has a dedicated...
anon’s picture

Status: Needs review » Fixed

Thanks for the patch, and the tests. This is commited.

Status: Fixed » Closed (fixed)

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