The Admin Toolbar module does not display the "Updates report" menu item.

Comments

Pasqualle created an issue. See original summary.

pasqualle’s picture

Status: Active » Needs review
StatusFileSize
new1.68 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2961757-2-admin_toolbar.patch, failed testing. View results

pasqualle’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Hm. I'm not sure about that test failure, looks like it's unrelated to this patch.

But anyway... Why should we need a hook_menu_links_discovered_alter() here? Maybe we should address the root of the problem -- why is it not showing up in the menu links that *are* discovered automatically by the menu system?

jhodgdon’s picture

Status: Needs review » Postponed (maintainer needs more info)

In other words, we have a config_update_ui.links.task.yml that adds this to the Configuration Manager's admin page. This is the same way that the links in core/modules/config/config.links.task.yml are added to that page. Are they showing up in Admin Toolbar? Because there is no hook_menu_links_discovered_alter() in config.module in Core.

jhodgdon’s picture

The failing test problem is from this related issue.

pasqualle’s picture

Status: Postponed (maintainer needs more info) » Needs review

I do not know why it works like this, but this is how it works. See:
https://cgit.drupalcode.org/admin_toolbar/tree/admin_toolbar_tools/admin...

jhodgdon’s picture

Oh, I see, they've special-cased certain Core modules. It seems to me that that philosophy is the problem -- Admin Toolbar should be looking at the local task tree so that contrib modules that add to these local tasks are automatically detected, rather than special casing some modules to get their menu tasks into the toolbar.

jhodgdon’s picture

What I'm saying is that they've basically made it so that every contrib module that adds to Core's local tasks has to implement this hook if they want to be listed in Admin Toolbar. They've also implemented the hook on behalf of a bunch of core modules. They should have instead solved the problem in a more systematic way, by looking at the local tasks. This would have (a) greatly simplified their code and (b) made it so contrib modules didn't have to implement this hook in order to get listed.

pasqualle’s picture

I guess the admin toolbar does not want to display all the menu items and local tasks as it could make the menu too large and can slow down the admin interface. Killing the whole purpose of the module: "providing a fast access to all administration pages".
Even these special cased core local tasks are moved into separate module admin_toolbar_tools.

I guess this is the reason why it choose to use a hook, but I might be wrong.

jhodgdon’s picture

Status: Needs review » Needs work

So... I'm seeing now that this big hook implementation is part of the Admin Toolbar Extra Tools module, and its effect is that it adds a bunch of the Core local tasks (some? all? I don't know) as links to the core Tools menu (which is the stated purpose of this add-on module). These links are present no matter what someone is doing with the Tools menu -- they've used a Core hook, not a hook that would be specific to Admin Toolbar. Which means that if we implement this core hook in our module, and someone is *not* using the Admin Toolbar Extra Tools menu, we've added our local tasks to the Tools menu, and they shouldn't be there (the rest of the Core links that are added by Admin Toolbar Extra Tools won't be there either).

So I think what we could do in this module is go ahead and implement the hook, but only add the links if the Admin Toolbar Extra Tools module is installed (otherwise, just return and do nothing). Presumably if someone installs that module, they would also want our module's pages added to the "extra tools" (and hence to the Admin Toolbar). So if you put that module exists check into the patch, with a comment as to why we're doing this, I'll be willing to commit it.

Thanks!

pasqualle’s picture

Status: Needs work » Needs review
StatusFileSize
new1.77 KB
jhodgdon’s picture

Status: Needs review » Needs work

Looks OK, but can you add a comment to explain why we are doing this? And I assume you've tested and this patch works to add links to your site? Thanks!

  • jhodgdon committed 7d485bd on 8.x-1.x authored by Pasqualle
    Issue #2961757 by Pasqualle: Display the Updates report in admin toolbar
    
jhodgdon’s picture

Status: Needs work » Fixed

This is low-risk, since it only happens if the admin_toolbar_tools module is present... so I went ahead and added a comment and committed this. Sorry for the delay -- I have been out on vacation for a few weeks. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.