bug #1035 Fixed external menu routes to not include additional parame…

…ters (javiereguiluz)

This PR was merged into the master branch.

Discussion
----------

Fixed external menu routes to not include additional parameters

This fixes #936.

---

As explained in #936, any route generated for the main menu includes the `menuIndex` and `submenuIndex` parameters which makes it possible to show the selected menu item.

When the route generates a URL that doesn't belong to the backend, these params are included too ... and the result is weird. Imagine a menu item with `route: 'homepage'` and the URL generated being `/?menuIndex=8&submenuIndex=2` instead of the expected `/`

This PR improves the menu to detect if the URL generated by the route belongs to the backend or not. If it doesn't belong, no menu parameters are included.

Commits
-------

7aa0420 Fixed external menu routes to not include additional parameters

Resources/views/default/menu.html.twig

@@ -5,11 +5,19 @@
         {% set menu_params = { menuIndex: item.menu_index, submenuIndex: item.submenu_index } %}
         {% set path =
             item.type == 'link' ? item.url :
-            item.type == 'route' ? path(item.route, menu_params|merge(item.params)) :
+            item.type == 'route' ? path(item.route, item.params) :
             item.type == 'entity' ? path('easyadmin', { entity: item.entity, action: 'list' }|merge(menu_params)|merge(item.params)) :
             item.type == 'empty' ? '#' : ''
         %}
 
+        {# if the URL generated for the route belongs to the backend, regenerate
+           the URL to include the menu_params to display the selected menu item
+           (this is checked comparing the beginning of the route URL with the backend homepage URL)
+        #}
+        {% if item.type == 'route' and (path starts with path('easyadmin')) %}
+            {% set path = path(item.route, menu_params|merge(item.params)) %}
+        {% endif %}
+
         <a href="{{ path }}" {% if item.target|default(false) %}target="{{ item.target }}"{% endif %}>
             {% if item.icon is not empty %}<i class="fa {{ item.icon }}"></i>{% endif %}
             {{ item.label|trans }}

Tests/Controller/CustomMenuTest.php

@@ -174,10 +174,10 @@ public function testMenuUrls()
 
     public function testMenuItemTypes()
     {
-        $expectedTypesMainMenu = array('empty', 'entity', 'entity', 'divider', 'link', 'link', 'link');
+        $expectedTypesMainMenu = array('empty', 'entity', 'entity', 'divider', 'link', 'link', 'link', 'divider', 'route', 'route');
         $expectedTypesSubMenu = array('entity', 'entity', 'divider', 'entity', 'link');
 
-        $crawler = $this->getBackendHomepage();
+        $this->getBackendHomepage();
         $backendConfig = $this->client->getContainer()->getParameter('easyadmin.config');
         $menuConfig = $backendConfig['design']['menu'];
 
@@ -189,4 +189,19 @@ public function testMenuItemTypes()
             $this->assertEquals($expectedTypesSubMenu[$i], $itemConfig['type']);
         }
     }
+
+    public function testExternalRoutesDontIncludeIndexParameters()
+    {
+        $crawler = $this->getBackendHomepage();
+
+        $this->assertEquals(
+            '/custom-route?custom_parameter=Lorem+Ipsum',
+            $crawler->filter('.sidebar-menu li:contains("Custom External Route") a')->attr('href')
+        );
+
+        $this->assertEquals(
+            '/admin/?menuIndex=9&submenuIndex=-1',
+            $crawler->filter('.sidebar-menu li:contains("Custom Internal Route") a')->attr('href')
+        );
+    }
 }

Tests/Fixtures/App/Resources/views/custom_menu/template.html.twig

@@ -0,0 +1 @@
+The template associated to the route used in the custom menu.

Tests/Fixtures/App/config/config_custom_menu.yml

@@ -1,6 +1,9 @@
 imports:
     - { resource: config_default_backend.yml }
 
+framework:
+    router: { resource: "%kernel.root_dir%/config/routing_custom_menu.yml" }
+
 easy_admin:
     design:
         menu:
@@ -18,3 +21,6 @@ easy_admin:
             - { label: 'Project Home', url: 'https://github.com/javiereguiluz/EasyAdminBundle', icon: 'home', target: '_blank' }
             - { label: 'Documentation', url: 'https://github.com/javiereguiluz/EasyAdminBundle#getting-started-guide', icon: 'book', target: '_self' }
             - { label: 'Report Issues', url: 'https://github.com/javiereguiluz/EasyAdminBundle/issues', icon: 'github', target: 'arbitrary_value' }
+            - { label: 'Misc.' }
+            - { route: 'custom_route', label: 'Custom External Route', params: { custom_parameter: 'Lorem Ipsum' } }
+            - { route: 'easyadmin', label: 'Custom Internal Route' }

Tests/Fixtures/App/config/routing_custom_menu.yml

@@ -0,0 +1,10 @@
+easy_admin_bundle:
+    resource: "@EasyAdminBundle/Controller/"
+    type:     annotation
+    prefix:   /admin/
+
+custom_route:
+    path:   /custom-route
+    defaults:
+        _controller: FrameworkBundle:Template:template
+        template:    'custom_menu/template.html.twig'