Trash for taxonomy?

The problem with taxonomy terms is their hierarchy, i.e. when "trashing" a term, what do you do with its children?

One option could be to execute \Drupal\taxonomy\Entity\Term::postDelete() directly on the trashed term, with the downside that restoring the deleted term from trash won't restore the hierarchy for its previous child items.

A possible fix for that could be to store additional "metadata" somewhere, which would be used in this case to store the child item IDs, and on restore you could ask the user if they want to re-parent those terms to the newly restored parent.

It's not an impossible problem, it just needs a bit of time and thought :)

Thank you, @amateescu, I can see that the hierarchy thing is probably a bit complicated :)

I think the question here also is what should happen with children when the parent is trashed. For me, the logical thing would be to also trash its child nodes (recursively, if they have their own child nodes), so they don't get orphaned, nor added to an arbitrary new parent. If they, however do have another parent, then, just as in the example here, I'd leave them be and only store metadata of the parent that's being trashed. Otherwise, we'd probably need to store metadata of all the trashed children (and possibly their children) as well.

If trashing works like this, then we wouldn't even need to ask whether to re-parent the restored children once their parent gets restored, since they'd be restored alongside of it. Of, if they had another parent, then they'd get re-attached to the restored parent as well.

I checked how Drupal's current delete on parent terms work and it seems that children terms are indeed auto-removed with the parent. So that functionality logically doesn't change from the default Drupal behavior.

What do you think?

amateescu → credited fabianx → .

we wouldn't even need to ask whether to re-parent the restored children once their parent gets restored, since they'd be restored alongside of it.

There's a tricky edge case here brought up by @Fabianx: what if a child term was trashed before its parent. In that case it shouldn't be automatically restored alongside the other children of that parent term. I've been thinking about this problem for quite a while, and I think it could be accomplished by ensuring that we only restore child terms with the deleted timestamp >= than the one of the parent term.

Joining this thread as we're also interested in trash for taxonomy terms.

I like the idea of the storing this additional metadata. Maybe part of the metadata we have a list of any additional entities trashed as a result of trashing this current entity. Let's call it `trash_chain` or something. We only add direct children in that chain. Any grandchildren would only appear in their direct parents, so handling this trash chain is a recursive bit.

This way, when we trash an entity (taxonomy term or anything else), any pre-trashed children simply wouldn't be added to the trash chain. And it would also work for any other entity type with hierarchy (e.g. menu items) or any dependency at all. At one point, this might become more flexible to allow custom dependencies.

We might be interested in contributing time and effort to get this metadata bit added.

I'm thinking an additional serialised column `deleted_metadata`. It gets added/removed together with the `deleted` column.

And in there we store this `trash_chain` list of entities (entity type + id for each).

The trash chain could be hard-coded for now just to get these few entity types sorted and to see how well the approach works. We can later transition to using plugins (ideally) or just hooks.

Maybe we can also allow enabling these entity types as "Experimental" so there's a caveat that they might not work perfectly, yet.

What do you think @amateescu?

I started adding the support for trash to taxonomy. I have created an MR that enables trashing the terms. However, there is a challenge with the term hierarchy, as mentioned in the previous comments.

Any term being trashed resets its parent to root by design. See <a href="https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/taxonomy/src/Entity/Term.php?ref_type=heads#L137">Term::preSave</a>.

I had a chat with @amateescu, and we concluded that any other temporary storage like key/value will not be compatible with the core workspaces module. This data needs to stay with the term itself. Something like @khaled.zaidan mentioned above. I like the idea of saving the immediate child terms for the current term. Maybe in an implementation of hook_pre_trash_delete for taxonomy terms?

That's possible.

My think is it would be nice if it's not specific to one entity type. I think this can apply to any entity type that there would be dependencies to an entity being trashed.

So rather than implementing hook_pre_trash_delete for a specific entity type, I was thinking that it's a separate thing (hook or plugin) that allows these dependencies to built, and then we trash those. Maybe that can be done in a hook_pre_trash_delete(), but I think it's safer to do it separately before that hook has been invoked, to ensure our trash chain is always handled first (this might be important if the a child entity being deleted needs information from its parent).

And maybe `trash_chain` isn't the best name xD I'm thinking maybe `trash_dependent_entities`?

Here's some suggested pseudo-code to the `TrashStorageTrait`:

$field_name = 'deleted';
$revisionable = $this->getEntityType()->isRevisionable();

foreach ($to_trash as $entity) {
  // Get all entity dependent entities.
  $dependent_entities = $this->invokeHook('trash_entity_dependencies', $entity);

  // First trash the dependencies. They might require an active parent to be successfully trashed.
  foreach ($dependent_entities as $dependent_entity) {
    $dependent_entity->delete();
  }

  // Allow code to run before soft-deleting.
  $this->invokeHook('pre_trash_delete', $entity);

  $entity->set($field_name, \Drupal::time()->getRequestTime());

And we would something similar for restoring (just the other opposite order: restore parent first and then the dependencies).

amateescu → credited eclipsegc → .

I liked a lot the idea of only storing the direct children of a term when it's trashed, but it still has the downside that we need an additional field to store them in, and I'd really like to avoid it if possible.

Thankfully, @eclipsegc had another idea: we could use the deleted timestamp that we already have, and restore child terms with the same timestamp as the parent. Since cascading deletes happen in the same request, all the deleted terms will have the same timestamp, so I think this approach is very feasible.

As for the trash_dependent_entities suggestion above, I'm not sure we need to generalize the idea of "dependent deletes" just yet, because it would only apply to taxonomy terms at the moment. Menu links for example, which also have a hierarchy, are not auto-deleting their children.

After looking at the MR posted by Ajit, I think there's something else we need to generalize first: we need to introduce "trash handlers", which would be responsible for all the entity type-specific customizations and hook implementations needed. Opened an issue for that: 📌 Add the concept of trash handlers Active , and postponing this one for now.