TrashStorageTrait delete method does not trigger pre and post delete hooks

Not triggering the delete hooks is by design, I tried this at some point and it created way more problems than it solved.

The issue with menu links should be handled by trash_menu_link_content_update(). Are you using the latest version of the module?

Oh I misread the problem you pointed out, it's about deleting nodes, not menu links.

Anyway, I still think that we shouldn't trigger the delete hooks, but handle it in a similar way with how we deal with menu_link_content deletions.

That is a good point, but there will always be a need to reacting to content being sent to trash. Maybe we can implement a new hook/event listener (if it doesn't exist already) that is triggered when an entity is sent to trash, that way developers can choose what to do.

Right, invoking a hook would be good, and we already have an issue open for it: ✨ Call hook during entity restore Needs review

✨ Call hook during entity restore Needs review now handles all the hooks needed, let's keep this issue open to fix the problem with menu_link_content entities related to deleted nodes.

While working on this thought it would be cool to also restore the menu_links if they were also in trash when the related node was restored.

But somehow

    $deletedLinks = [];
    $trashManager->executeInTrashContext('inactive', function () use ($menu_link_manager, $url,$route_parameters, &$deletedLinks) {
      $deletedLinks = $menu_link_manager->loadLinksByRoute($url->getRouteName(), $route_parameters);
    });

didn't work as I expected and $deletedLinks comes empty. I debugged this and it is because `trash_menu_link_content_update` removes the definition, so `menu_tree` doesn't contain the link anymore. We might want to review this and provide this functionality in a follow-up.

Isn't the real issue here that the menu link is still displayed even though it points to an inaccessible URL?

Isn't the real issue here that the menu link is still displayed even though it points to an inaccessible URL?

That's exactly the problem, and it's actually a core issue: 🐛 [regression] Do not bypass route access with 'link to any page' permissions for menu links Fixed

I deleted a node with a menu link, then checked the following:
- with the admin user, the menu link was still displayed
- with the anonymous user, the menu link was not displayed

Like @penyaskito mentioned in the MR, I doubt that deleting the associated menu link when a node is trashed is the right thing to do. There would be no way of restoring that node (if you're not using the `wse_menu` module).

There's also the option of unpublishing the menu link when the node is trashed, but that also has the problem that we can't know if we should publish it when the node is restored.

I'm inclined to close this as a duplicate of the core issue, any objections? :)

I wasn't aware of that regression, thanks for pointing it!

I've been using the PR patch on prod. Until the core issue is fixed, from an UX perspective I think it's better for end users to have to rethink the menus when they restore content than having to think about removing it from every menu it might be used when deleting something.

So this might serve as a temp solution and we create a follow-up blocked on the core issue?

Just implemented this module and found the above issue to be problematic. Thanks for the MR patch until it can be ultimately solved.

I would agree with @penyaskito in that asking our users to redo their menu structure is a better ask than not having the menu structures work at all.

@catapipper, since 🐛 [regression] Do not bypass route access with 'link to any page' permissions for menu links Fixed was committed to Drupal 10.3, can you please test with that patch applied instead of the one from this issue and check if it fixes your problem?

The core issue mentioned above has been fixed in Drupal 10.3, so menu links for inaccessible nodes are no longer displayed for the admin user, so I don't think this issue is a problem anymore. Please re-open if you disagree :)