The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.When I upgraded Video Embed Field to 8.x0-2.0-alpha1 and ran the database update, I found that the upgrade had added a new field ("field_media_video_embed_field _1") to the default one that had previously been created when video_embed_media was installed ("field_media_video_embed_field"). When I investigated, I found that the following section of media_entity_update_8201() was failing to find my video type's source field, and thus creating a new one:
/** @var \Drupal\media\MediaTypeInterface $media_type */
$media_type = \Drupal::entityTypeManager()->getStorage('media_type')
->load($config->get('id'));
$media_source = $media_type->getSource();
$source_field = $media_source->getSourceFieldDefinition($media_type); // Failing here! --BD.
if (!$source_field) {
$source_field = $media_source->createSourceField($media_type);
$source_field->getFieldStorageDefinition()->save();
$source_field->save();
$media_type
->set('source_configuration', [
'source_field' => $source_field->getName(),
]);
}
Changing the behavior of VideoEmbedField::getSourceFieldDefinition() to account for the fact that the old version of Video Embed Media wasn't setting a source_field configuration value addresses this. I recognize that I'm hard-coding a field name in here, but I thought this line in the patched code made that a reasonably safe choice:
$field = !empty($this->configuration['source_field']) ? $this->configuration['source_field'] : 'field_media_video_embed_field';
Hope this helps!
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2927149-updating-from-media-entity-to-core-media.patch | 1.67 KB | bdimaggio |
Comments
Comment #2
bdimaggioComment #3
phenaproximaComment #4
marcoscanoManually tested this, and I can confirm that:
- The problem can be reproduced (i.e. an unexpected field "_1" is created on VEF media types, though not enabled in form/view displays)
- The patch in #2 solves the issue, and the upgrade works as expected with the patch applied.
I personally don't see an issue with defining the field machine name in code, it was already there in the 1.x branch, so we are not inventing anything new :)
For me this is OK to go.
Comment #5
sam152 commentedComment #7
sam152 commentedSome testing would have been nice here, but don't want to hold up the fix. If anyone feels like following it up with a test, that would be cool.
Comment #8
sam152 commentedGiven this is a pretty nasty bugfix in the upgrade, tagging alpha2.
Comment #9
m4oliveiJust something to be aware of here, if you are extending the VideoEmbedField class in your own module to create a new MediaSource plugin, you need to be careful to also declare a
source_fieldmachine name. Otherwise what happens is when the media type is saved, the source field storage is created here:\Drupal\media\MediaSourceBase::createSourceFieldStorage
Which calls \Drupal\media\MediaSourceBase::getSourceFieldName, which names the source field by the plugin id, so you get a field created named
field_media_' . $this->getPluginId(), in our examplefield_media_yt_push. However, then, the plugin refers to$this->configuration['source_field']in a bunch of places which will resolve tofield_media_video_embed_field, which blows up, b/c thats not the field that was auto-created.Setting the default configuration to either empty, or the machine name according to your plugin id, in our case
field_media_yt_push, fixes the issue.