I had this code in an update hook, since some versions of this database are a little out of sync with others:
$menus = menu_get_names();
if (!in_array('secondary-links', $menus)) {
// There was an update that deleted the secondary-links menu. Put it back.
$ret[] = update_sql("INSERT INTO {menu_custom} (menu_name, title, description) VALUES ('secondary-links', 'Secondary links', 'Secondary links are often used for pages like legal notices, contact details, and other secondary navigation items that play a lesser role than primary links')");
}
However, because the menu_get_names() function checks against the menu_links table instead of the menu_custom table, it will only bring up the name of menus that have links assigned. Secondary links did not, so people were getting errors during the upgrade path since it executed the insert statement regardless.
I'm not sure if this is by design or not, but it's definitely not what I would've expected from a function called menu_get_names(). :)
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 784856.patch | 2.28 KB | dmitrig01 |
| #2 | 784856.patch | 965 bytes | dmitrig01 |
Comments
Comment #1
dmitrig01 commentedThis does seem somewhat illogical.
Comment #2
dmitrig01 commentedSo this is an interesting problem.
Basically, there isn't any way to tell what menus there are except by the menu_name property in the menu_links table. System module defines menu blocks for the default menus, but after that different modules can define their own blocks and menu_names if they want.
However, menu.module does have a menu_custom table. It also has a
menu_get_menusfunction, thus rendingmenu_get_namesuseless. It's not used anywhere in core anyway. Should you decide to go this route, the attached patch removes the function altogether.That being said, I don't know what the right thing is to do. However I think we can remove
menu_get_namesbecause it's useless.Comment #4
dmitrig01 commentedComment #5
cwgordon7 commentedNice catch, good removal of dead code, and passes tests too, so this is good to go.
Comment #6
sun#4: 784856.patch queued for re-testing.
Comment #7
sunI agree it doesn't make sense. However, that's an API change, so it's webchick's call.
Comment #8
sunAlthough badly needed, this is D8 material according to the rules (I had to learn today). It may be backported at a later point in time (though that's unlikely).
Comment #9
catchComment #10
dries commentedCommitted to 8.x. Thanks!
I'd be comfortable backporting this to D7, but I'll leave it up to @webchick to consider. Updating the version field.
Comment #11
webchickSorry, I don't think so. You never know what contributed/custom code is using those "dead" functions.
Comment #13
jhodgdonPresumably this is an 8.x API change that needs update doc (but please don't document it now, as we're figuring out the system for API change docs in 8.x now)?
Issue summary:
The menu_get_names() function has been removed.
So... for API change doc, what do we suggest any users of this function should do instead?
Comment #14
catchThey can use menu_get_menus() I think.
Comment #15
jhodgdonThat looks like a reasonable suggestion.
Proposed API change notification (again, please don't put this anywhere until we figure out what we're doing in d8; changed status to needs work until it's recorded though):
title: menu_get_names() function has been removed
removed in: 8.x-UNSTABLE1
body:
The menu_get_names() function from Drupal 7.x has been removed in Drupal 8.x, because it probably didn't work as you'd expect anyway. If you were using it for something, you can probably use menu_get_menus() instead to accomplish the same task.
Thoughts on this proposed doc?
Comment #16
pillarsdotnet commentedProbably ought to update the 7.x docs as we3ll.
Comment #17
jhodgdonIn what way are you suggesting updating the 7.x docs? I don't believe there was a change committed to 7.x.
Comment #18
pillarsdotnet commentedThere should be a note in the document header of the
menu_get_names()function stating that its use is deprecated in favor of themenu_get_menus()function.EDIT: For now, I posted a note to the api page.
Comment #19
jhodgdonLet's leave the 7.x documentation at that.
Comment #20
jhodgdonI think this needs a change notification node made?
Comment #21
pillarsdotnet commentedTagging as per the agreed-upon workflow.
Comment #22
aspilicious commentedhttp://drupal.org/node/1357900
Comment #23
jhodgdonWhat happened to the block that shows the change notices on issues in the right sidebar????? I was just about to mark this "needs work" again and say the change notice doesn't exist...
Filed:
#1358328: Change notice block not showing on issue pages