Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check we're not trying to delete default media thumbnails #854

Closed

Conversation

antbrown
Copy link

When using the "Delete media and files" views bulk operation on a FITS media type, for example, Drupal comes back saying we don't have permission to delete "x" number of files. This is because one of the files it is trying to delete is the default thumbnail for the media type. This is confusing to clients and so I suggest we remove default thumbnails from the list of inaccessible entities.

GitHub Issue: (link)

What does this Pull Request do?

Removes default thumbnails from the list of inaccessible entities when deleting multiple media and files.

What's new?

A in-depth description of the changes made by this PR. Technical details and
possible side effects.

  • Checks whether one of the files being deleted is a default thumbnail for the type of media being deleted.

How should this be tested?

Create a media item on a repository item which uses a default thumbnail (eg: FITs), then use the "Delete media and files" action on it and you should see a warning: "X items have not been deleted because you do not have the necessary permissions".

Checkout this pull request branch, repeat steps above and you should not see the warning.

Documentation Status

This is under-the-hood, shouldn't need any docs updated.

Additional Notes:

Let me know if I'm using the system incorrectly, and/or whether there is a better solution to this.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/8-x-committers

// May not be allowed access because it is a default thumbnail for media type.
if ($field->getName() == 'thumbnail') {
$default_thumbnail_filename = $entity->getSource()->getPluginDefinition()['default_thumbnail_filename'];
$default_thumbnail_uri = \Drupal::config('media.settings')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be using dependency injection instead of the \Drupal thing... up in the ::create() method, to load up the config factory, and either pass it to the constructor to get the particular config object and then to do so inside the constructor, or to grab and pass the config object in the ::create() method to the constructor.

@rosiel
Copy link
Member

rosiel commented Jul 10, 2023

I encountered this in #960. I found that in Drupal 10 (at least), the user admin was easily able to delete the default thumbnails, causing general havoc in the site.

My approach was to not consider files in the thumbnail field eligible for deletion, knowing that where (namely: Image media) there is a custom thumbnail, the same file is also in the source field (field_media_image) so we would delete it as we should.

Is that an ok approach?

@rosiel
Copy link
Member

rosiel commented Jul 12, 2023

closing as completed with #960

@rosiel rosiel closed this Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants