Issue #3271688: Media Mappings breaking revisions
Merge request reports
Activity
419 419 // set. However, that is only set once parent::save() is called, so work 420 420 // around that by setting it here. 421 if (!isset($this->original) && $id = $this->id()) { 422 $this->original = $this->entityTypeManager() 423 ->getStorage('media') 424 ->loadUnchanged($id); 421 if (!isset($this->original)) { 422 if ($this->isNewRevision() && $id = $this->id()) { 423 $this->original = $this->entityTypeManager() 424 ->getStorage('media') 425 ->loadUnchanged($id); 426 } 427 elseif ($id = $this->getRevisionId()) { 428 $this->original = $this->entityTypeManager() 429 ->getStorage('media') 430 ->loadUnchanged($id); 444 451 } 445 452 446 453 // Set thumbnail. 447 if ($translation->shouldUpdateThumbnail($this->isNew())) { 454 if ($translation->shouldUpdateThumbnail($this->isDefaultRevision())) { 364 364 */ 365 365 public function postSave(EntityStorageInterface $storage, $update = TRUE) { 366 366 parent::postSave($storage, $update); 367 $is_new = !$update; 367 $is_latest_revision = !$update; 331 331 /** 332 332 * Determines if the thumbnail should be updated for a media item. 333 333 * 334 * @param bool $is_new 334 * @param bool $is_latest_revision 335 335 * Specifies whether the media item is new. 336 336 * 337 337 * @return bool 338 338 * TRUE if the thumbnail should be updated, FALSE otherwise. 339 339 */ 340 protected function shouldUpdateThumbnail($is_new = FALSE) { 340 protected function shouldUpdateThumbnail(bool $is_latest_revision = FALSE) { 341 341 // Update thumbnail if we don't have a thumbnail yet or when the source 342 342 // field value changes. 343 return !$this->get('thumbnail')->entity || $is_new || $this->hasSourceFieldChanged(); 343 return !$this->get('thumbnail')->entity || $is_latest_revision || $this->hasSourceFieldChanged(); -
I stepped through this in the debugger and I don't think the changes here are correct.
The source of the issue is this
Which ends up at line 437 of Media::prepareSave with this call
$translation->set($entity_field_name, $media_source->getMetadata($translation, $metadata_attribute_name));Which for the 'mimetype' ends up calling
$translation->set('field_media_image', 'image/jpeg');The issue is field_media_image is the ImageItem and this should only be setting the mimetype, not overwriting the whole field.
So I think we probably want to do something like
$value = $translation->get($entity_field_name); $value[$media_attribute_name] = $media_source->getMetadata($translation, $metadata_attribute_name); $translation->set($entity_field_name, $value);But that doesn't work for the 'name' attribute, because in that case we only want to be setting 'value' and the metadata_attribute_name in that case is 'name'
So we might need to change \Drupal\media\Entity\MediaType::getFieldMap to return some additional data, but that'd be an API change.
So we most likely need to introduce a method on MediaType to massage the metadata value into the correct structure.
e.g. MediaType::massageMetadata($metadata_name, $value, $existing_values)
Which would need to bubble down to the source plugin.
In this instance the image source plugin would know that the stuff relating to images needs to be set in keys inside $existing_values and not wipe out the target_id
