menu.inc must not unset MenuLink::menu_name

While I agree with the fix, I've usually seen this when a parent menu link is referenced that doesn't exist.

unset() is definitely wrong, but I would argue that it is a mandatory information now (as menu links need to live in a menu?), and when it is missing it should be handled *somehow* and not silently ignored. I'm not sure how... exception? watchdog?

Especially the typical reason for this (missing parent definition) should probably be handled earlier, again, *somehow*.

Thanks for the follow-up issue to address @Berdir's concern. This looks good to me.

+1

This causes failures in \Drupal\menu_link\MenuLinkStorageController, which expects the property to exist.

This made me expect test coverage... What's up with that?

The only way that I'm aware of is an invalid default menu link, and the issue linked above is supposed to change that.

This is only a follow-up of that, and fixing that should prevent that this will happen, so this is really just a small cleanup because using unset() on class properties is just wrong.

+++ b/core/includes/menu.inc
@@ -2625,7 +2625,7 @@ function menu_link_rebuild_defaults() {
           // Unset the default menu name so it's populated from the parent.
-          unset($menu_link->menu_name);
+          $menu_link->menu_name = NULL;

No longer unsetting :) Since http://uk1.php.net/property_exists will return something different after this change I suggest changing the comment to something like "Set the default menu name to NULL so it's populated from the parent."

Updated comment looks fine, forgot about this issue.

10: drupal_2198571_10.patch queued for re-testing.

It is still green.

Committed de35da9 and pushed to 8.x. Thanks!

I get this error now when I enable a contrib module:

WD menu: Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column    [error]
'menu_name' cannot be null: INSERT INTO {menu_links} (menu_name) VALUES (:db_insert_placeholder_0); Array
(
    [:db_insert_placeholder_0] => 
)
 in Drupal\menu_link\MenuLinkStorageController->save() (line 107 of
/home/jbrown/sites/drupal8/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php).

Adding 'menu_name' => 'admin', to composer_manager_menu_link_defaults() fixes the problem:

http://drupalcode.org/project/composer_manager.git/blob/refs/heads/8.x-1...

I'm not sure if that module is doing something wrong or not.

Problem solved. The parent routes had to be changed:

system.admin.config.system => system.admin_config_system