It would be great if Menu Link Weight could be used with Menu Admin Per Menu.

From the project page for Menu Admin Per Menu:

By default, Drupal allows only users with the Administer menus and menu items permission to add, modify or delete menu items.

Menu Admin per Menu allows to give roles per menu admin permissions without giving them full admin permission.

For instance, you may let certain users manage the items of the Main or Navigation menus but not those of the Management menu.

When using Menu Admin Per Menu, roles that may add/edit/delete menu items in particular menus will have a permission per menu instead of the standard "administer menu" permission.

This means that most of the users with permissions from Menu Admin Per Menu cannot use the nice draggable interface provided by Menu Link Weight since Menu Link Weight is checking for users to have the permission "administer menu".

From menu_link_weight.module:

function menu_link_weight_form_node_form_alter(&$form, &$form_state) {
  if (!user_access('administer menu') || !isset($form['menu']['link'])) {
    // Only allow users with "administer menu" permissions.
    return;
  }
  ...
}
CommentFileSizeAuthor
#5 2617614-5.patch959 bytesstefan.r
#4 interdiff-2617614-2-4.txt1.04 KBgenjohnson
#4 2617614-4.patch884 bytesgenjohnson
#2 2617614-2.patch843 bytesstefan.r

Comments

genjohnson created an issue. See original summary.

stefan.r’s picture

stefan.r commented
Status: Active » Needs review
StatusFileSize
new 2617614-2.patch843 bytes
stefan.r’s picture

stefan.r commented

@genjohnson can you please test and review this patch - if all looks well I will commit it.

genjohnson’s picture

genjohnson commented
StatusFileSize
new 2617614-4.patch884 bytes
new interdiff-2617614-2-4.txt1.04 KB

@stefan.r wow, thank you for such a quick response!

The patch in #2 applied cleanly. I tested with a user who had the 'administer menus' permission and a user who only had permission to administer the Main Menu (through Menu Admin Per Menu) and everything worked just fine.

A couple of suggestions...

  1. Only $form['menu']['link'] is passed to _menu_filter_parent_options(), so I'm not sure that checking for isset($form['menu']['link']['parent']) is necessary. After testing with your patch I tried using isset($form['menu']['link']) instead and it did not seem to change anything.
  2. The function _menu_admin_per_menu_filter_parent_options() has been in Menu Admin Per Menu since the first 7.x release, so it doesn't seem to me that checking for both the existence of the module and the existence of the function is necessary. I would just check for the existence of the module, which would also make the if statement a little easier to read (because it's pretty long).
  3. Just reading through the if statement, I do not find it very clear what we're trying to accomplish. I would recommend expanding the comment inside to explain what's going on.

Attached is a patch and interdiff with these suggestions.

stefan.r’s picture

stefan.r commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 2617614-5.patch959 bytes

Thanks!

1 is the same check as in menu_admin_per_menu_node_form_alter so I believe we should keep this stricter check, just in case.

For 2 the worry would be for that helper function to disappear in a future version which could give a fatal error and the function_exists is not expensive so we can keep that as well. I'll commit this with the updated comment.

  • stefan.r committed 9f5f0d0 on 7.x-1.x 
    Issue #2617614 by stefan.r, genjohnson: Add support for Menu Admin Per...
stefan.r’s picture

stefan.r commented
Status: Reviewed & tested by the community » Fixed
stefan.r’s picture

stefan.r commented
Status: Fixed » Closed (fixed)