Add support for taxonomy terms

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 :)

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 Term::preSave.

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).

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: #3506131: Add the concept of trash handlers, and postponing this one for now.

The trash handlers issue is now fixed. I plan to give this another shot.

I picked this issue up as part of Tag1's sponsored work for open source development. I used Claude code with Cline in VS Code to work through this.

We cannot rely on the deleted value for the term in restore as mentioned in #13 above. This is because of the way core handles term deletions.
Say there is a hierarchy of terms A -> B and the term A is deleted:

• The Drupal\taxonomy\Entity\Term::preSave is called, and then the term is deleted. And Drupal\taxonomy\Entity\Term::postDelete is called to delete the orphaned terms.
• From the term A's postDelete, the term B->delete() is called.
• This calls the Drupal\taxonomy\Entity\Term::preSave before deleting. And the function resets the parent to root. The term gets deleted after that. This removes any trace of the previous hierarchy.

We might have to rethink a way to store the hierarchy while trashing a term. Maybe we go with storing it with the term itself, as mentioned in previous comments.

I have added test for possible scenarios. Most of them should pass, except restoring multiple terms in the hierarchy.

Setting this to "Need review" for now to get quick feedback.

Thank you, Very much needed feature.

Tested, Thank you :)
The diff from the MR is working well

The MR is still a work in progress, see #18.

Noted; you are right Andrei

Having more testing rounds, with number of scenarios

I would love to follow with Drupal Core Behavior when deleting terms

Tech root level
├── Tech 1 level 1
│   ├── Tech 1 level 2
│   │   ├── Tech 1 level 3
│   │   ├── Tech 1 level 3
│   │   └── Tech 1 level 3
│   ├── Tech 1 level 2
│   │   ├── Tech 1 level 3
│   │   ├── Tech 1 level 3
│   │   └── Tech 1 level 3
│   └── Tech 1 level 2
│       ├── Tech 1 level 3
│       ├── Tech 1 level 3
│       └── Tech 1 level 3
├── Tech 2 level 1
│   ├── Tech 2 level 2
│   │   ├── Tech 2 level 3
│   │   ├── Tech 2 level 3
│   │   └── Tech 2 level 3
│   ├── Tech 2 level 2
│   │   ├── Tech 2 level 3
│   │   ├── Tech 2 level 3
│   │   └── Tech 2 level 3
│   └── Tech 2 level 2
│       ├── Tech 2 level 3
│       ├── Tech 2 level 3
│       └── Tech 2 level 3
└── Tech 3 level 1
    ├── Tech 3 level 2
    │   ├── Tech 3 level 3
    │   ├── Tech 3 level 3
    │   └── Tech 3 level 3
    ├── Tech 3 level 2
    │   ├── Tech 3 level 3
    │   ├── Tech 3 level 3
    │   └── Tech 3 level 3
    └── Tech 3 level 2
        ├── Tech 3 level 3
        ├── Tech 3 level 3
        └── Tech 3 level 3


Sport root level
├── Sport 1 level 1
│   ├── Sport 1 level 2
│   │   ├── Sport 1 level 3
│   │   ├── Sport 1 level 3
│   │   └── Sport 1 level 3
│   ├── Sport 1 level 2
│   │   ├── Sport 1 level 3
│   │   ├── Sport 1 level 3
│   │   └── Sport 1 level 3
│   └── Sport 1 level 2
│       ├── Sport 1 level 3
│       ├── Sport 1 level 3
│       └── Sport 1 level 3
├── Sport 2 level 1
│   ├── Sport 2 level 2
│   │   ├── Sport 2 level 3
│   │   ├── Sport 2 level 3
│   │   └── Sport 2 level 3
│   ├── Sport 2 level 2
│   │   ├── Sport 2 level 3
│   │   ├── Sport 2 level 3
│   │   └── Sport 2 level 3
│   └── Sport 2 level 2
│       ├── Sport 2 level 3
│       ├── Sport 2 level 3
│       └── Sport 2 level 3
└── Sport 3 level 1
    ├── Sport 3 level 2
    │   ├── Sport 3 level 3
    │   ├── Sport 3 level 3
    │   └── Sport 3 level 3
    ├── Sport 3 level 2
    │   ├── Sport 3 level 3
    │   ├── Sport 3 level 3
    │   └── Sport 3 level 3
    └── Sport 3 level 2
        ├── Sport 3 level 3
        ├── Sport 3 level 3
        └── Sport 3 level 3

What are the accountable scenarios?

1- Deletion / Restore scenarios for root terms

When I delete "Tech root level"
Then all its child terms will be deleted.

When I restore "Tech root level"
Then all its child terms will be restored.

2- Deletion / Restore scenarios for 1st-level child terms

When I delete "Tech 1 level 1"
Then all its children ("Tech 1 level 2" and their descendants) will be deleted.

When I restore "Tech 1 level 1"
Then all its children ("Tech 1 level 2" and their descendants) will be restored.

3- Deletion / Restore scenarios for 2nd-level child terms

When I delete "Tech 1 level 2"
Then all its children ("Tech 1 level 3") will be deleted.

When I restore "Tech 1 level 2"
Then all its children ("Tech 1 level 3") will be restored.

4- Deletion / Restore scenarios for leaf terms

When I delete "Tech 1 level 3"
Then only this term will be deleted (no children under it).

When I restore "Tech 1 level 3"
Then only this term will be restored.

5- Cross-vocabulary isolation

When I delete "Tech root level"
Then no terms under "Sport root level" are affected.

When I delete "Sport root level"
Then no terms under "Tech root level" are affected.

6- Partial deletion within multiple branches

When I delete "Tech 2 level 1"
Then only its subtree ("Tech 2 level 2" and "Tech 2 level 3") will be deleted.

When I restore "Tech 2 level 1"
Then its subtree ("Tech 2 level 2" and "Tech 2 level 3") will be restored.

7- Mixed operations

When I delete "Tech 1 level 2" and "Tech 3 level 1"
Then only those branches and their children are deleted, while other Tech terms remain.

When I restore "Tech 1 level 2" and "Tech 3 level 1"
Then only those branches and their children are restored.

The tree could be serialized and stored as an extra nested data relation, which restore could read it and apply a restore for it after the root restore

Patch based on latest MR version

Re rolled https://www.drupal.org/project/trash/issues/3491947#comment-16258520 for 3.0.23

Made a mistake in #25.

Looked into this today and figured out the answer to #18, instead of calling Term::postDelete() we loop through the hierarchy and delete the orphans ourselves.

Found and fixed some bugs, I think the test coverage is sufficient at this point. So.. this is finally ready!