Problem/Motivation

1. Create a page, select "Home" parent
2. Edit, select "< Main navigation>"

=> The menu link is still below home in the menu block (= doesn't show) and in the link list when editing the menu. But in the node form and the menu link edit form, the parent shows up correctly.

The reason is that while saving, the empty string in parent is considered empty and dropped, then getParentId() returns NULL. NULL for the menu tree storage however means "keep using the existing parent" (I don't even want to know why... about as strange as taxonomy term parent behavior).

Proposed resolution

Cast getParentId() to a string.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#6 menulinkcontent_menu-2781673-6.patch1.65 KBBambell
#5 interdiff-4-5.txt630 bytesBambell
#5 menulinkcontent_menu-2781673-5.patch1.03 KBBambell
#4 menulinkcontent_menu-2781673-4.patch1.03 KBBambell
#2 menu-link-no-parent-2781673-2.patch641 bytesBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir created an issue. See original summary.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Active » Needs review
Issue tags: +Needs tests
FileSize
menu-link-no-parent-2781673-2.patch641 bytes

#2769299: node:menu-link:parents:join-path doesn't work as expected has some tests, we just need to remove the token/pathauto stuff from it and just check the menu link API directly.

The test is even fully Wim Leers-compatible already.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Note: Workaround is to move the menu link to the right position in the list of links and save there. But not everyone has access to that, e.g. users might only be able to create/manage menu links for nodes they can create/update.

Bambell’s picture

Bambell CreditAttribution: Bambell at MD Systems GmbH commented
FileSize
menulinkcontent_menu-2781673-4.patch1.03 KB

Here's a test only patch against 8.2.x.

Bambell’s picture

Bambell CreditAttribution: Bambell at MD Systems GmbH commented
FileSize
menulinkcontent_menu-2781673-5.patch1.03 KB
interdiff-4-5.txt630 bytes

Or rather (small mistake)...

Bambell’s picture

Bambell CreditAttribution: Bambell at MD Systems GmbH commented
FileSize
menulinkcontent_menu-2781673-6.patch1.65 KB

And the combined patch.

The last submitted patch, 4: menulinkcontent_menu-2781673-4.patch, failed testing.

The last submitted patch, 5: menulinkcontent_menu-2781673-5.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Looks good to me

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented 
+++ b/core/modules/menu_link_content/src/Entity/MenuLinkContent.php
@@ -117,7 +117,9 @@ public function isExpanded() {
+    // Cast the parent ID to a string, only an empty string means no parent,
+    // NULL keeps the existing parent.
+    return (string) $this->get('parent')->value;

Wow, nice catch!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: menulinkcontent_menu-2781673-6.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Reviewed & tested by the community 
00:09:52.145 ERROR: Connection was broken: java.io.EOFException

No idea what happened there.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: menulinkcontent_menu-2781673-6.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs work » Needs review
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

Still green. weird, not even seeing failing results in the history?

  • catch committed b2e9479 on 8.3.x 
    Issue #2781673 by Bambell, Berdir: MenuLinkContent menu links can not be...

  • catch committed 86f64ef on 8.2.x 
    Issue #2781673 by Bambell, Berdir: MenuLinkContent menu links can not be...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.