Hide administration menu links that do not have visible child links

I was able to reproduce this on the tip of the 9.5.x branch and I updated the issue description with steps to reproduce this bug from a clean installation.

Failing test.

I spent some time trying to get this working by adding an additional access check to all of the routes which use the SystemController::systemAdminMenuBlockPage controller, but I don't think that's going to be a fruitful approach. It's difficult to check access to menu links from that controller and after digging in, it felt a little hacky.

Instead, I think it might be more useful and generally useful to add a new flag to menu items to indicate whether it should or shouldn't be shown if doesn't have children. Tentatively, I'm naming this flag transitive with a default value of FALSE. The idea is that menu links like People and Development can be marked "transitive" since their only purpose is to transit users from the root page to a child page. Thus, if they have no visible child links, they can be safely elided.

I'm trying to do that in MenuLinkTree but I'm running into a hiccup because at some point in the process of building a menu link tree, a particular item may not have any children but will have one added later. So, my current code is removing items that it shouldn't because it doesn't have a way to "know" about the child that doesn't exist yet. I think I just have to spend more time with Xdebug.

The drawback of the menu link tree based approach is that the system menu block pages would still be accessible, even without any links, if a user visited the right URL directly. That feels like a separate issue that could be handled in a follow-up though. All we'd need to do would be to throw a 403 or 404 HTTP exception in the controller instead of printing out the message.

Needs work for an update hook to add the transitive column to existing sites.

Okay, I added code to update the database schema on existing sites. I was expecting to write an update hook to do this, but it seems like the default menu tree storage service handles its own schema changes, so I made my changes in there instead. This probably needs special attention.

Needs review for tests.

Needs work to update the toolbar tests, which makes sense.

I'm trying to do that in MenuLinkTree, but I'm running into a hiccup because, at some point in the process of building a menu link tree, a particular item may not have any visible children even though one will be added later. Thus, my current code is removing items that it shouldn't because it doesn't "know" about the child that doesn't exist yet. I think I just have to spend more time with Xdebug to figure out where these missing child links are added.

To follow up on this: the issue was in the toolbar module. It does a quirky thing where it only loads the top level of the admin menu, then it loads the subtree of each of those top level items separately, rather than loading the whole tree at once. This explains why I wasn't seeing any children when I was step-debugging the first load of the admin menu (it was only loading a shallow depth).

Needs work to update the toolbar tests, which makes sense.

To clarify: it makes sense because I'm fiddling with the menu and the toolbar tests probably make some assumptions that don't hold any longer.

omkar.podey → made their first commit to this issue’s fork.

@gabesullice et al, great work on this but...
I have talked with @tim.plunkett and @lauriii and I (we?) think we should close this issue in favor of #296693-257: Restrict access to empty top level administration pages → from 2008 which has different approach to solve the same general UX problem. See my linked command but basically I think route access a better way to solve this then menu items.

+1