shouldUpdateThumbnail does not update thumbnail is source field changed

Created on 28 June 2022, over 2 years ago
Updated 22 April 2024, 8 months ago

Problem/Motivation

The logic in \Drupal\media\Entity\Media::shouldUpdateThumbnail is broken, where the check for hasSourceFieldChanged is never ran.

Steps to reproduce

Change a media entity's source field values and check for updated media thumbnail

Proposed resolution

Change

return !$this->get('thumbnail')->entity || $is_new || $this->hasSourceFieldChanged();

To

return $is_new || $this->hasSourceFieldChanged() || !$this->get('thumbnail')->entity;
  • if new, always download
  • if source changed, always download
  • download if thumbnail missing

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
MediaΒ  β†’

Last updated about 20 hours ago

Created by

πŸ‡ΊπŸ‡ΈUnited States mglaman WI, USA

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡©πŸ‡ͺGermany IT-Cru Munich

    Existing MR needs rebase.

  • First commit to issue fork.
  • πŸ‡ͺπŸ‡ΈSpain vidorado Pamplona (Navarra)

    @mglaman, could you confirm if this issue is still relevant? I’m unable to reproduce it.

    1. I created a media item of type Image and the thumbnail displays correctly on /admin/content/media, which is the expected behavior.
    2. If I update the field_media_image field (the source field), the thumbnail updates as expected on /admin/content/media. This aligns with the behavior I’ve always observed.
    3. Thinking you might be referring to changing the media source field on the media type configuration, I attempted that, but it’s not allowed for an existing media type.

    It looks like this restriction was introduced at least as early as 2017/12/12, so I’m guessing you’re not referring to that scenario.

    I might be missing something here. Could you clarify? :D

  • πŸ‡ͺπŸ‡ΈSpain vidorado Pamplona (Navarra)

    vidorado β†’ changed the visibility of the branch 11.x to hidden.

  • πŸ‡ͺπŸ‡ΈSpain vidorado Pamplona (Navarra)

    vidorado β†’ changed the visibility of the branch 3293135-shouldupdatethumbnail-does-not to hidden.

  • Pipeline finished with Success
    7 days ago
    Total: 4055s
    #368952
Production build 0.71.5 2024