This is a great step forward, thanks!
However it isn't quite there yet, at least on my test rig.

I'm not convinced that the patch on admin is necessary, it does not tally with the examples given in
http://api.drupal.org/api/file/developer/topics/forms_api.html. The admin works wether or not I use

$item = $form_state['values']['menu'];

or

$item = &$form_state['values']['menu'];

same goes for function menu_edit_item_validate()

I have added some watchdog lines so I can see what is passing through function _menu_check_access() and therefore on to function _menu_per_role_access()

The odd thing is that links to ordinary nodes are not passing through at all even though they are in menu_links table and are visible as expected. I have an instance of a views module link which does work, even if I create a new link to it.
This behaviour occurs wether or not the menu_per_role module is enabled.

So in the case where function _menu_check_access() is being accessed menu_per_role works immediately.
Flushing the cache and rebuilding the menu (via the devel module) have so far had no effect.

Comments

hutch’s picture

On second thoughts it looks like the ampersand *is* required if the settings for menu_per_role are to be saved first time around.
Sorry for the noise ;-(
Still can't get regular nodes to be picked up though.

Eugene Dubois’s picture

I am using version 6 now, but links to normal node pages (like node/23) don't get blocked...

Eugene

Eugene Dubois’s picture

I found a workaround for this:

Create a view that only shows this node...
Then put the view in the menu, and it gets filtered :)

AlexisWilke’s picture

Eugene,

That's a good way of doing it! 8-)

Just quite a bit tedious.

Yes, the fix to the core is only for when you create a new item. Otherwise it is not necessary. A workaround (to avoid the fix) is to create the item, then edit it. But that's a really bad gotcha if you do not remember to edit your menu later.

I will do some more testing, I thought I tried with common nodes. Maybe it is because my cache is turned off...

Thank you.
Alexis Wilke

P.S. sorry for the late answer, I should get a feed of this issues thread!

AlexisWilke’s picture

Status: Active » Fixed

Okay! I just checked in my changes to the CVS. A new 6.x-1.x-dev should appear within 12h.

WARNING: I have removed the need for the menu_per_role patch. It is most certainly a good idea to remove the changes from you code library. I have done so to test my changes to the menu_per_role module. (that file was removed from the module also!)

Please, update and test. This one really works for me... 8-)

Feel free to re-open this issue if you still have problems.

Thank you.
Alexis Wilke

P.S. Just an FYI... I now use menu_per_role_menu_link_alter() and menu_per_role_translated_menu_link_alter() to make this work.

AlexisWilke’s picture

Assigned: Unassigned » AlexisWilke

Just in case, mark that I was in charge. 8-)

Status: Fixed » Closed (fixed)

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