Problem/Motivation

  1. Add a view, expose a menu item and place it in the Main navigation.
  2. Go edit the Main navigation menu, save the page.
  3. The views menu item is now gone.
  4. Go back to edit the view, it will say the view is invalid, and the menu item is half-broken.

Found at Drupalaton 2014 training.

Proposed resolution

Do not remove the views menu item in this case.

Remaining tasks

Do it. Tests.

User interface changes

None.

API changes

Likely none.

CommentFileSizeAuthor
#9 interdiff.txt3.61 KBdawehner
#9 menu-2318435-9.patch11.98 KBdawehner
#7 interdiff.txt973 bytesGábor Hojtsy
#7 menu-2318435-7.patch8.81 KBGábor Hojtsy
#3 menu-2318435-3.patch8.14 KBdawehner
#3 menu-2318435-3.patch8.14 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue summary: View changes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Related issues: +#2301317: MenuLinkNG part4: Conversion

The solution for this is probably identical to that in the interdiff at #2301317-50: MenuLinkNG part4: Conversion.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review
FileSize
menu-2318435-3.patch8.14 KB
menu-2318435-3.patch8.14 KB

This will interfere with #2202493: views_menu_link_defaults() does not set a parent for links but just partly

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Looks good. On a quick look this may need config schema changes as well :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/modules/user/config/install/views.view.user_admin_people.yml
index 5bd1ab3..d446faf 100644
--- a/core/modules/views/config/schema/views.display.schema.yml

--- a/core/modules/views/config/schema/views.display.schema.yml
+++ b/core/modules/views/config/schema/views.display.schema.yml

+++ b/core/modules/views/config/schema/views.display.schema.yml
+++ b/core/modules/views/config/schema/views.display.schema.yml
@@ -34,7 +34,7 @@ views.display.page:

@@ -34,7 +34,7 @@ views.display.page:
         weight:
           type: integer
           label: 'Weight'
-        name:
+        menu_name:
           type: string
           label: 'Menu name'
         context:

You are absolute right!

Status: Needs review » Needs work

The last submitted patch, 3: menu-2318435-3.patch, failed testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Needs review
FileSize
menu-2318435-7.patch8.81 KB
interdiff.txt973 bytes

Yeah there was still one schema change missing and the file view was not changed yet.

Status: Needs review » Needs work

The last submitted patch, 7: menu-2318435-7.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
menu-2318435-9.patch11.98 KB
interdiff.txt3.61 KB

Fixed the remaining failures, stupid me.

On top of that there is now a test which ensures that the menu link still appears in the block.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Yay! The new tests look good.

+++ b/core/modules/views/src/Tests/Plugin/DisplayPageWebTest.php
@@ -21,6 +21,13 @@ class DisplayPageWebTest extends PluginTestBase {
+  public static $modules = ['menu_ui', 'block'];

@@ -93,6 +101,24 @@ public function testPageDisplayMenu() {
+    $admin_user = $this->drupalCreateUser(['administer menu']);

Are we using this array syntax now?!

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

The fix looks good. @dawehner said we use such square bracket syntax elsewhere as well. https://www.drupal.org/coding-standards#array says nothing about this array syntax not allowed, or the keyword version required.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 455428c and pushed to 8.0.x. Thanks!

  • alexpott committed 455428c on 8.0.x 
    Issue #2318435 by dawehner, Gábor Hojtsy: Fixed Views menu items...

Status: Fixed » Closed (fixed)

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