PluginModule-menu items appear in two menus

Bug #1372536 reported by Tobias Zeuch on 2014-09-22
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
High
Tobias Zeuch

Bug Description

With a module-plugin, that overwrites menu_items, the returned menu items appear in the main navigation and also in the top right corner. Both are very helpful on their own, but it should be possible to add an item to either of them without the other, like, have an entry only in the upper right corner but not in the main menu or vice versa.

The calls are indistinguishable (as far as I see) and originate from web.php::main_nav, line 2683 and web.php::right_nav, line 2741

Tobias Zeuch (tobias-zeuch-8) wrote :

After investigating a bit more about the menu-structure and understanding in more detail about how the menus are built, I realized that the problem is not specific for the module plugins but is the same for interaction and artefact. The active menu-items are filtered by their path: Every entry whose path starts with "settings" or "inbox" gets displayed in the upper right corner, for the main menu, the active elements start with "content", "myportfolio" and "groups", plus admin for admin user.

To get a menu item into the first level, it would need a path without a '/', but with the two identical calls, the menu item directly appears in both menus.

Are plugins not meant to add anything to the first level of any of the menus? If not, I'm not sure what would be the easiest way to allow that. menu_items could accept a string (by default ignored) for the different menus, to adjust the output regarding on the menu it is called for. But that would compete with the logic of putting menu items into the menu using their path.
There could be another path level for the side-menu, but that would mean changes for all plugins that add to the right-nav menu and I think it would clash a little bit with the actual concept where you could add a complete menu-item subtree to the right-nav by adding the parent-entry to the right_nav()-function.
Or there could be another function right_nav_items() for plugins, also weird when there is already the possibility to add subentries to the "settings" menu for example using the menu_items() function.

Aaron Wells (u-aaronw) wrote :

Right, so although this problem has (in theory) existed for artefact and interaction plugins for a while, none of the artefact or interaction plugins in core, includes any top-level menu items, and so the problem hasn't displayed itself.

However, since modules are meant to be more generic, they should be able to add top-level menu items. So we do need to fix this for modules, and we may as well fix it for artefacts and interactions as well.

So I agree, probably the best thing is to create a second method specifically for the right nav menu items. We'll have to consider whether that will break backwards-compatibility for any existing plugins, though. (i.e. are any existing plugins passing back right-nav menu items in their menu_items() function?)

Changed in mahara:
milestone: none → 1.10.0
importance: Undecided → High
status: New → Confirmed
Tobias Zeuch (tobias-zeuch-8) wrote :

I could imagine that there are plugins who add submenu items to the user settings, which are part of the right nav menu. I'll see if I can include those submenu-entries but skip all top-level items when calling menu_items for the right-nav and then add a function to the three plugin types to add to the right nav menu. That would mean two ways to add elements to the right-nav menu for the moment, but it wouldn't break any existing plugin.
In how far do we support plugins that are not listed under https://wiki.mahara.org/index.php/Plugins ? We could skip the workaround when starting the work on 1.11 so developers will have enough time to fix their plugins. And I'll have a look at the plugin list in the wiki.

Tobias Zeuch (tobias-zeuch-8) wrote :

I checked the artefact plugins on the wiki page. The only plugin that I found adding a menu item on the top level is the calendar plugin. But I got errors when I tested it and it doesn't add any subentry to the settings menu.

And I uploaded a patch for the new function/modification of right_nav()

Son Nguyen (ngson2000) on 2014-10-06
Changed in mahara:
assignee: nobody → Son Nguyen (ngson2000)
status: Confirmed → In Progress
Son Nguyen (ngson2000) on 2014-10-06
Changed in mahara:
assignee: Son Nguyen (ngson2000) → Tobias Zeuch (tobias-zeuch-8)
Robert Lyon (robertl-9) on 2014-10-12
Changed in mahara:
milestone: 1.10.0 → 1.11.0

Reviewed: https://reviews.mahara.org/3724
Committed: http://gitorious.org/mahara/mahara/commit/2fd431ba63698e98dc40c2b650c1d21741d3d915
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 2fd431ba63698e98dc40c2b650c1d21741d3d915
Author: Tobias Zeuch <email address hidden>
Date: Thu Sep 25 11:27:43 2014 +0200

Add separate function for right nav menu

Bug 1372536: Add a separate function to PluginArtefact, PluginInteraction and
PluginModule that lets you add menu entries especially to the right nav menu,
including top level entries.

The method right_nav in web.php still includes elements from the old menu_items
function for backwards compatibility but ignores all top level entries.

Change-Id: I91ba01f060a73575c483aa81aeb9201b11f1261f
Signed-off-by: Tobias Zeuch <email address hidden>
Signed-off-by: Aaron Wells <email address hidden>

Robert Lyon (robertl-9) on 2014-10-16
Changed in mahara:
milestone: 1.11.0 → 1.10.0

Reviewed: https://reviews.mahara.org/3831
Committed: http://gitorious.org/mahara/mahara/commit/69f30fe2b525fa66cf0cfc41b4dc9cb1436917eb
Submitter: Robert Lyon (<email address hidden>)
Branch: 1.10_STABLE

commit 69f30fe2b525fa66cf0cfc41b4dc9cb1436917eb
Author: Tobias Zeuch <email address hidden>
Date: Thu Sep 25 11:27:43 2014 +0200

Add separate function for right nav menu

Bug 1372536: Add a separate function to PluginArtefact, PluginInteraction and
PluginModule that lets you add menu entries especially to the right nav menu,
including top level entries.

The method right_nav in web.php still includes elements from the old menu_items
function for backwards compatibility but ignores all top level entries.

Change-Id: I91ba01f060a73575c483aa81aeb9201b11f1261f
Signed-off-by: Tobias Zeuch <email address hidden>
Signed-off-by: Aaron Wells <email address hidden>

Robert Lyon (robertl-9) on 2014-10-16
Changed in mahara:
status: In Progress → Fix Committed
Aaron Wells (u-aaronw) on 2014-10-21
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers