Remove source from media mappings

Updated issue summary.

This appeared today on a site using 10.0

Replicated on another install of 10.1.x

I've read this issue several times and its not clear what combination of modules (and patches) is needed to trigger it.

The changes to existing tests in the patch makes me feel there might be some behaviour changes here.

Can we get the issue summary updated here with the latest state of play - thanks!

Updated issue summary slightly. But no contrib or patches needed. This is reproducible out of the box.

Turned patch into MR. Lets see what fails.

Failure is legit. May need a new solution.

Updated steps to test after manual testing

Left a review with the path forward and root cause

On second thoughts, I think a simpler way forward would be to:

• Prevent the 'source' field from being shown in the list of fields available in the mappings
• An update path to remove such entries from any existing media type

Holy smoke, I can't believe we allow the source field -- i.e., the single most critical field on any media entity, without which everything about that entity breaks badly -- to be mapped!! That's a major oversight.

I agree with @larowlan's proposed solution in #73.

smustgrave → closed merge request !4265

Well I know where 1 test will go.

Will work on the post_update this evening/tomorrow.

This is a great start! I've left a review with some proposed changes, hopefully to make things a little simpler and more robust.

Also we'll need to re-title and re-summarize this issue.

Updated post_update for the failing test. Think maybe the oembed fixture was off too but is now passing locally.

Updated title
Updated IS in #79 to match proposed solution

It's not a big deal, but the if ($source_field) check shouldn't be needed. All media types must have a source field, or they break.

I understand it's because we have a broken fixture somewhere, but could we not adjust the fixture -- or the test which uses it?

But as I said, the extra if shouldn't matter much since all functioning media types have a source field. I think this is ready for review.

Moving this to NW for the validator. I tried getting it started but it needs work.

Kicked things along a bit

Will work on fixing test.

Try/catch fixed the issue!

Wonder if this could make 10.2.1?

Merged 11.x into this branch, resolved the remarks by @Wim Leers.

There's a failure in MediaTypeValidationTest::testRequiredPropertyValuesMissing, this was recently introduced in 📌 Configuration schema & required values: add test coverage for `nullable: true` validation support Fixed

So rebased but still can't figure out why that test is failing

Should an is_array check be added to OembedMediaMappingsConstraintValidator you think?

Added a check if fieldMap is empty, unless someone else can pin point where in the tests it's missing a field_map.

Also reverted back the name change as this isn't specific to oEmbed but all media types. The description was off (copy and paste) so I updated that.

Left some suggestions on the MR

Applied suggestions.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Bot be wrong.

Couple of minor things now - thanks!

I've been over this several times and I can't fault it now. Thanks @smustgrave for putting up with my change requests.

As this bug requires an update function only backporting to 10.3.x. I'll put it to the release managers to backport this to 10.2.x

Committed and pushed 1857ebff31 to 11.x and 7c2638e272 to 10.3.x. Thanks!

quietone → closed merge request !4355

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