Menu bug - moving menu items around

The 'Blocks' item isn't actually deleted, it's parent is just gone/hidden (It appears to still exist in the menu_links table). It comes back as soon as you recreate the page type.

The 'has_children' column gets confused after the "Page" link is deleted and then recreated at admin/content/types.

Note: I have seen other instances where the arrow is missing when it should appear, or appears when it shouldn't, so this may be a separate, yet related issue.

During menu update we lost the cleanup functionality. Now we can put it back because of the update column.

So what happened is , http://drupal.org/node/147657 D5->D6 first attempt nuked this cleanup out of necessity but now that we have the update column, we can restore. Note that http://drupal.org/node/215361 is a sister issue, bec and me too looked around what needs to be save between the load and the save to work. Apparently, options.

Works for me. Followed dmitrig01 steps and this patch preserves blocks menu.

http://drupal.org/node/215361 is committed. Does the phpdoc needs modification now to accommodate the change in this patch?

@chx - but we also nuked the cleanup since a module that was disabled and tehn re-enable would have all its links get reset, right?

Ah yes.

Better of the same.

As I understand it - essentially this means that we flag the link as "updated" (i.e. don't deleted) when a module is disabled *only if* the link has been customized. It would be nice to have some way to handle uninstalled (rather than just disabled) modules but I don't really think that's essential.

I don't understand what the _original_options is for.

Reroll to fix a typo. Seems to be working.

The updated flag is not touched for items that do not already have their update flag set. The original_options property is ncessary because during the localization of the item, the options[attributes][title] gets set to t(description). Not something you want in the DB, I guess.

seems to be some problems still - this code may leave orphan links that can never be recovered. With the patch applied, I followed the initial procedure, except the link was to a calllback page 'admin/content/node-type/story':

mysql> select  mlid, plid, link_path, router_path, hidden, customized   from menu_links where link_path like 'admin/content/n%';
+------+------+--------------------------------------+--------------------------------------+--------+------------+
| mlid | plid | link_path                            | router_path                          | hidden | customized |
+------+------+--------------------------------------+--------------------------------------+--------+------------+
|   28 |   10 | admin/content/node                   | admin/content/node                   |      0 |          0 | 
|   42 |   10 | admin/content/node-settings          | admin/content/node-settings          |      0 |          0 | 
|   74 |   42 | admin/content/node-settings/rebuild  | admin/content/node-settings/rebuild  |     -1 |          0 | 
|  128 |   10 | admin/content/node-type/page         | admin/content/node-type/page         |     -1 |          0 | 
|  129 |    0 | admin/content/node-type/page/delete  | admin/content/node-type/page/delete  |     -1 |          0 | 
|  123 |   10 | admin/content/node-type/story        | admin/content/node-type/story        |     -1 |          0 | 
|  126 |  107 | admin/content/node-type/story        | admin/content/node-type/story        |      0 |          1 | 
|  125 |    0 | admin/content/node-type/story/delete | admin/content/node-type/story/delete |     -1 |          0 | 
+------+------+--------------------------------------+--------------------------------------+--------+------------+
8 rows in set (0.00 sec)

mysql> select  mlid, plid, link_path, router_path, hidden, customized   from menu_links where mlid = 107;                       
Empty set (0.00 sec)

What is the expected behavior for orphan links? Delete them?

I'm seeing inconsistent behavior as far as 'had_children' and re-parenting - not sure if it's a code problem or what.

Also, as far as ['_original_options'] it would be better to arrange it so that we could do sequential menu_link_load()/meu_link_save() operations wihout this intermediate step. However, the cleanest solution to this would involve renaming the DB column and then populating ['options'] only on menu_link_translate().

now I can't reproduce my earlier problems - anyone else tested this patch?

Followed dmitrig01's steps to reproduce, then same again on a clean install with the patch applied. Everything behaved as it ought to. Recreating the "page" content type put it on the same level as the blocks item in the hierarchy, which is what I'd expect to happen. Couldn't reproduce pwolanin's bug with admin/content/node-type but those links changed recently so a good chance it was a stale install? So looking pretty good. If pwolanin's issue was indeed down to a stale install it should be ready.

I'd suggest using db_placeholders() instead of $placeholders = implode(', ', array_fill(0, count($menu), "'%s'"));

I think it's critical that we rename the db column from 'options' to something like 'link_options' because there are many places where we may have the localized options getting saved back to the DB.

The part of the patch dealing with link options localization should be dealt with separately. New issue here: http://drupal.org/node/215858

This one needs updating then.

Ok I don't claim to remotely understand what's going on with this patch, but here's a re-roll with the stuff from the localization issue removed.

Not sure if

+        $child['options'] = $child['localized_options'];

is valid here - if it is obviously it'll depend on the other patch. It seems to be working though.

+        $child['options'] = $child['localized_options'];

should be removed from this patch - the other issue address the localization problem.

Cool. Figured it was probably bogus. Here it is without.

Updated:

- Let's use db_placeholders() as I said
- Let's get out the localized_options part since I was also unable to reproduce that bug
- Let's get this tested more

OK I'd never seen db_placeholders() before but Heine talked me through it.

sorry stupid typo.

Tested this with same steps as before and it seems fine.

However, if I delete both "Page" and "Story" to end up with zero content types, then blocks disappears again because "Create content" has gone. admin/build/block still works, but it's not available at admin/build/menu-customize/navigation

Then if I add a new content type with any random name. blocks reappears under Create content, and I also get the menu item for "Story" + on the node/add page, but node/add/story doesn't do anything.

Fresh install,

1. Followed dmitrig01's steps, the bug reproduces.
2. Applied the patch and the block menu is recovered after going to modules page.
3. Created new content type, repeated 1) and now block menu is there.

Catch's #25 works perfect for me.

OK I thought this might be my menu table getting messed up from the various patch iterations, but only partially. I can reproduce everything except the phantom entry for Story, which I think was just a fluke.

Steps to reproduce #28.

Put blocks underneath "story"

Delete story AND page.

blocks is no longer available in admin/build/menu-customize/navigation
admin/build/block still works

create a new content type called "test"

blocks re-appears under create content and can be moved around.

Also reproduced #30 deleting all content types.

Committed #28. The remaining issue does not seem to be critical.

Just tried with a new custom item. Moved blocks under that, deleted it. Blocks went to top of the menu tree. Tried it with two custom menu items (making blocks third level), and also fine. So I reckon this is specific to the way access works on node/add

I applied this patch: http://drupal.org/node/211979 and the blocks is back to its place.

Needs more testing.

OK since this now seems to be duplicate of that, I'll mark this back to original status.