D5->D6: outdated menu items cleanup

I have some simple steps with which I experienced the problem before:

- download a fresh Drupal 5.x tarball
- install it
- download a fresh Drupal 6 tarball
- upgrade your Drupal 5 database with the Drupal 6 code

You get a menu which looks like the one attached. (Things marked with red should not be there).
I tried to get a plan from chx on fixing this, and he asked for my D5 database to test. I told him already that a fresh D5 install will do it. (At least it did it for me while I was testing some upgrade patches).

I don't think this is acceptable user experience wise, and chx told me these core items should not appear there either. So elevating status.

Note that my screenshot is from 9 days ago, and I did not retry with later versions since then (neither I remember we did anything to fix this). Should be easy to reproduce with the above.

OK, tomorrow I will take a look at it. it is not going to be easy not sure whether I can fix it. At the end of the day we MIGHT end with a module you need to install under D5. I am sorry.

Also reproduced. I don't think it's too much to ask people to delete a few stale links in admin/build/menu - but those /admin ones are quite nasty.

Also reproduced. I unfortunately suffered the same problem. I put it down to the fact that I disabled my contributed modules, but I never un-installed them - hence I did not follow the procedure to the letter.

I agree, deleting a couple of dead links is OK, but having to delete 20+ becomes quiet a chore and should be avoided if possible.

Paddy.

There is a patch here: http://drupal.org/node/208498#comment-696412 that might be related to this.

Note that is experimental, just an idea.

I just tested http://drupal.org/node/208498#comment-696412 and it cleaned up the /admin menu items for me. Didn't do it on a clean 5.x-6.x menu table though so can't tell if there's side-effects yet.

Although the patch is over there, I'm marking this to needs review to get some more eyes on it. Here's a direct link: http://drupal.org/files/issues/menu_links_rebuild.patch

I do not particularly like this solution... if you have changed anything about a dead menu link then it won't be deleted. This also affects links saved programatically by menu_link_save which is something I even more so do not want. Let's see the outline of relevant D5 code first:

  $menu_item_list = module_invoke_all('menu', TRUE);
  foreach ($menu_item_list as $item) {
    $_menu['path index'][$item['path']] = $mid;
  }
  // Now fetch items from the DB, reassigning menu IDs as needed.
  if (module_exists('menu')) {
    $result = db_query(db_rewrite_sql('SELECT m.mid, m.* FROM {menu} m ORDER BY m.mid ASC', 'm', 'mid'));
    while ($item = db_fetch_object($result)) {
      if (isset($_menu['path index'][$item->path])) {
        // The path is already declared.
      }
      else {
        // The path was not declared, so this is a custom item or an orphaned one.
        if ($item->type & MENU_CREATED_BY_ADMIN) {
        }
      }
  }

So what we are seeing here is that a) created links need to be updated always b) modified links needs their router path set to the same path as the link path. This will automatically remove them from plain sight.

See also http://drupal.org/node/196043 and http://drupal.org/node/194585 for the reasons why things work the way they do currently.

This patch seems makes no discernible improvements to the admin menu after update for my configuration. The admin menu is identical post-update both with and without the patch.

I'll illustrate. One image is the d5 admin menu. Other image is the post-update admin menu. After update all the menu items at the top level with a circle in front of them are dead, apart from "Help". Similarly a number of the menu items at the second level, for example "Access Control", are also dead.

How does an admin distinguish between something not working properly and something left over from the update and thus can be safely deleted?

I venture this is going to be highly problematic for many users going through an upgrade cycle. I read the reasons why things work as they do. Maybe there's no alternative solution. But the current approach could put a lot of people off updating.

edit: images saved in different order so rephrased.

Far better with #17. RTBC now or after another positive test?

Well with two positive reviews, this seems ready. It won't help those who've already upgraded live sites to RC - but they may already have modified their menus and/or deleted these links anyway. So I'm marking rtbc.

I was also thinking about an upgrade path, but got to understand that those who already upgraded needed to solve their problems anyway. Committed, thanks.