Problem/Motivation

(Assume trash is enabled for both media and files)

When a media entity is deleted, and its associated files are deleted with it at the same time, restoring the media entity does not currently restore the deleted files. For example, when using the media_file_delete module, the user is given the option of deleting the associated file at the same time. If the user does this, and later restores the media entity, the file remains in the trash. This is unexpected, users expect files to be treated similarly to the way path aliases are handled when nodes are deleted, in that the media entity should be fully restored and usable.

Steps to reproduce

Enable Trash for both media and files. Install the media_file_delete module and delete a media entity and its file in one action. Restore the media entity. The file is not restored with it.

Proposed resolution

Add a media trash handler that restores any files previously attached to the media entity that were deleted with it (same delete timestamp).

User interface changes

Files do not need to be restored separately when they are deleted together with a media entity.

API changes

A new media trash handler is added.

Data model changes

None

Issue fork trash-3623650

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

john.oltman created an issue. See original summary.

john.oltman’s picture

Status: Active » Needs review

amateescu made their first commit to this issue’s fork.

amateescu’s picture

Thanks for the MR! The approach is right, restoring files that share the entity's deletion timestamp follows how the path alias and menu link integrations work. I pushed a follow-up commit that reworks a few things:

  • The restore now runs from DefaultTrashHandler::postTrashRestore() for every entity type, next to the path alias and menu link integrations, instead of a media-only handler. A node with an image field trashed together with its file has the same problem, and the reference plus timestamp guard keeps it safe for any entity type.
  • Removed the unused delete side (deleteAssociatedFiles(), isFileUsedByOtherEntities() and the FileUsageInterface injection). If we want trash to delete files itself that needs its own issue, since it has to decide what happens with shared files.
  • File IDs are collected from every translation, not only the default one. Media source fields are translatable by default, so a translated document media can hold a different file per language.
  • Tests folded into TrashFileIntegrationTest with a shared fixture helper, plus a node file field case and a translated media case. createPublicFile() moved to the kernel test base so TrashFileTest can use it too.

This also needs to land on 3.1.x, which trashes and restores individual translations, so the handler API takes an array of timestamps keyed by langcode there. I'll open a separate MR for that, where files are paired with the translation that references them, so a partial restore never brings back a file that only belongs to a still-trashed translation.

  • amateescu committed ad042d50 on 3.1.x
    feat: #3623650 Media restore should restore associated files deleted at...
amateescu’s picture

Version: 3.0.33 » 3.x-dev
Status: Needs review » Fixed

Merged both MRs, thanks again!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

john.oltman’s picture

Thanks for the quick turn! Do you have an idea of when 3.0.34 will come out?

amateescu’s picture

As soon as #3623769: A path alias in the trash overrides an active alias at the same path is figured out, I like to create releases with a clean issue queue :)

amateescu’s picture

Trash 3.0.34 is out!

john.oltman’s picture

Working great, thank you!