Corresponding menu item not deleted when deleting node [#328564]

Works as admin, but not as normal user, even with 'Allow non-admins to create menus items.' enabled.

I think the 'Allow non-admins to create menus items.' setting is supposed to mean deleting as well? Just not implemented yet?

On node update or insert, the function _nodehierarchy_create_menu is called (in addition to other nodehierarchy functions). But on node delete only nodehierarchy_delete_node is called. No extra attempt is made to delete corresponding menu items. So the menu item gets deleted by the menu module, but that only works if the user has administer menus rights.

I think this can be fixed by explicitly calling

menu_node_form_delete($node);
menu_rebuild();

on node deletion (if 'Allow non-admins to create menus items.' is enabled).

Comment	File	Size	Author
#1	nodehierarchy-328564-1.patch	1.42 KB	hopla

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

hopla CreditAttribution: hopla commented 31 October 2008 at 15:43

Status:

Active

» Needs review

File	Size
nodehierarchy-328564-1.patch	1.42 KB

Well, I just did it! Please verify it and be nice, this is my first patch submit to Drupal :)

Comment #2

ronan CreditAttribution: ronan commented 5 November 2008 at 23:20

The patch looks good, but I'm not sure about adding it since Drupal core does not delete menu items when a user who does not have 'administer menu' deletes a node. This seems like an oversight (what use is a menu item pointing to a deleted node). But I'm not going to override any core security checks until I have a chance to research the matter a little more.

Comment #3

ronan CreditAttribution: ronan commented 6 November 2008 at 15:54

Status:

Needs review

» Fixed

I went ahead and committed your first version of the fix. I removed the part of the patch which deletes the child menu items, as I don't want to change the current behavior too much in this branch.

I can see how the current behavior (orphaned menu items messing up your navigation menu) is undesirable, but it is at least expected. I think the proper solution here would be to ask the user when they're deleting what they would like to do with the menu items. That should be easy enough to code it's just a matter of making sure the choice is simple and not confusing for the user. I'll work on that as soon as I have a chance.

Thanks again for your patch.
R

Comment #4

ronan CreditAttribution: ronan commented 12 November 2008 at 03:33

This is now in 6.x-1.x-dev also

Comment #5

hopla CreditAttribution: hopla commented 24 November 2008 at 14:36

Hi Ronan,

Nice to see that you like my patch :) I agree with you on making the deletion of child menu items a choice. This way all possible use cases can be served. I only implemented it this way because it's better in my use case to have them all deleted :)

Maybe it's best to make this a setting in the admin menu and not a 'user choice at deletion time'? Because I think the admin would know best what to do and he would also like it to be done consistently? (I can't think of a use case where one time you would like to have them all deleted and some other time you wouldn't.)

Just a thought.

Hopla