Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Should menu_node_node_update look like this :
function menu_node_node_update($node) {
if (!empty($node->menu_node_links)) {
foreach ($node->menu_node_links as $link) {
menu_node_record_save($node, $link, 'update');
}
}
}I changed it to call menu_node_record_save instead of _menu_node_invoke. What is happening in my case is that we have been editing existing menu items that used to point to taxonomy terms to point to nodes instead. With the code change above, the appropriate entry gets added to the menu_node table the next time the node is updated.
I also updated menu_node_menu_link_update to look like this:
function menu_node_menu_link_update($link) {
$node = NULL;
if(strstr($link['link_path'],'node/')){
$nid = str_replace("node/","",$link['link_path']);
$node = node_load($nid);
}
if ($node) {
menu_node_record_save($node, $link);
}
else{
//make sure record with mlid is removed as it no longer points to a node
db_delete('menu_node')
->condition('mlid', $link['mlid'])
->execute();
}
}That way it is sure to remove links from the table if they no longer point to a node.
Comments
Comment #1
agentrickardThis issue needs:
* Steps to replicate the error.
* A proper patch. (http://drupal.org/patch/create)
Comment #2
agentrickardComment #3
ryan.wyse CreditAttribution: ryan.wyse commentedI ran into a similar issue here. I had created dummy menu items that pointed to . Since these weren't nodes the module didn't create an entry in the menu_node table. Then when I went to update the menu items to point to their correct node locations this module didn't have any records to update, so it didn't do anything. To fix this I just added an else, so if there isn't a record to update the module attempts to create a new record.
Comment #5
gabriel.achille CreditAttribution: gabriel.achille commentedpatch re-packaged
I added a hook_update to synchronize database.
Comment #6
gabriel.achille CreditAttribution: gabriel.achille commentedActually: previous patch didn't address the problem raised by ablondeau. ablondeau approch seems more completed. So this patch is based on his suggestion instead. However I did not include the first part (change in
menu_node_node_updatefunction) because it seems unnecessary.For those who are already running dev version + patch #5 of the module: disable/enable the module to synchronize the menu_node table.
Comment #7
gabriel.achille CreditAttribution: gabriel.achille commentedfix: previous patch didn't filter properly all possible internal menu paths (did not make distinction between "node/123" and "node/add/article"). Causing some sql exception (...)
Comment #8
bdimaggio#7 took care of the problem for me. Thanks gabriel.achille!
Comment #9
agentrickardBetter to fix menu_node_get_node() than add the ugly IF statement here. There is a reason to abstract that lookup.
Comment #10
gabriel.achille CreditAttribution: gabriel.achille commentedIndeed my If statement was ugly.
I changed that.
However because one case in the "If" involved calling
menu_node_record_deleteI'm reluctant to include thatmenu_node_get_node... Nowmenu_node_menu_link_updateis a kind of combination of code inmenu_node_menu_link_insertandmenu_node_menu_link_deletewhich make sense.What do you think ?
Comment #11
agentrickardLooks pretty good. Can we be sure that $link['link_path'] is canonical (e.g. node/6) and not an alias?
Comment #12
gabriel.achille CreditAttribution: gabriel.achille commentedI'd say yes we can, we've already made this assumption in
menu_node_menu_link_insert()which is very similar. And the api.drupal documentation for hook_menu_link_update says that the associative array $link is defining a menu link as passed intomenu_link_save()ie with $link['link_path'] normalized (e.g. node/6).Comment #13
agentrickardLooks good. Thanks.
This module really needs a new maintainer, though, as I never use it.
Comment #14
mexicoder CreditAttribution: mexicoder commentedI realise the status of this module is seeking a new maintainer but wondered about possibility of a release incorporating the patch from #10?
Comment #15
gabriel.achille CreditAttribution: gabriel.achille commentedI'm quiet confident with my last patch: i'm using it in 2 relatively big sites but agentrickard has been quiet busy recently.
@agentrickard: as we discuss earlier i'm ready to help! :)
Comment #16
mexicoder CreditAttribution: mexicoder commentedHi gabriel.achille yes I am using the patch and it all seems to be working fine and is stable. That's why I posted -would be great to have it moved into a full release when time allows.
Comment #17
agentrickard@gabriel.achille
You have commit access and can roll releases. https://drupal.org/node/457420/maintainers
Comment #18
gabriel.achille CreditAttribution: gabriel.achille commentedThanks agentrickard!
Here is the patch i'm actually planing to commit: i added back the hook_update_N from earlier patches to synchronize the menu_node table. It's necessary to fix properly the possibly existing incorrect records...
Comment #19
gabriel.achille CreditAttribution: gabriel.achille commentedcommited !
Comment #20
mexicoder CreditAttribution: mexicoder commentedGreat thanks guys
Comment #21
agentrickardI think the menu_node_disable() call in the update hook is not necessary, actually.
Comment #22
gabriel.achille CreditAttribution: gabriel.achille commentedIn my view, i would keep the call to menu_node_disable(): it is necessary to make sure we delete wrong records in menu_node table. Previous version of the module didn't delete records for example when editor was changing a menu link (admin/structure/menu/item/[mlid]/edit) from the path node/[nid] to a NON node path (like <front>) the record in menu_node table wasn't delete.
Comment #23
agentrickardCool.