Fix \Knp\Menu\MenuItem @Label issue

By using menu.getLabel() instead of menu.label, we're avoiding a conflict between the getLabel method and a possible child called with the key 'label'
sonata-project · Oct 18, 2020 · 8978dfa · 8978dfa
1 parent cb2739b
commit 8978dfa
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 5 deletions.
diff --git a/src/Resources/views/Core/tab_menu_template.html.twig b/src/Resources/views/Core/tab_menu_template.html.twig
@@ -119,8 +119,9 @@
 {% endblock %}
 
 {% block label %}
+{# We use method accessor instead of ".label" since `item` implements `ArrayAccess` and could have a property called "label". #}
 {{-
-    item.label|trans(
+    item.getLabel()|trans(
         item.getExtra('translation_params', {}),
         item.getExtra(
             'translation_domain',

diff --git a/src/Resources/views/Menu/sonata_menu.html.twig b/src/Resources/views/Menu/sonata_menu.html.twig
@@ -52,11 +52,13 @@
         {%- if is_link|default(false) -%}
             {{ icon|default|raw }}
         {%- endif -%}
+        {# We use method accessor instead of ".label" since `item` implements `ArrayAccess` and could have a property called "label". #}
+        {%- set item_label = item.getLabel() -%}
         {%- if options.allow_safe_labels and item.extra('safe_label', false) -%}
-            {{ item.label|raw }}
+            {{ item_label|raw }}
         {%- else -%}
             {%- set translation_domain = item.extra('label_catalogue', 'messages') -%}
-            {{ item.label|trans(item.extra('label_translation_parameters', {}), translation_domain) }}
+            {{ item_label|trans(item.extra('label_translation_parameters', {}), translation_domain) }}
         {%- endif -%}
     {% endapply %}
 {% endblock %}
diff --git a/src/Resources/views/standard_layout.html.twig b/src/Resources/views/standard_layout.html.twig
@@ -100,7 +100,8 @@ file that was distributed with this source code.
                             {% endif %}
 
                             {%- set translation_domain = menu.extra('translation_domain', 'messages') -%}
-                            {%- set label = menu.label -%}
+                            {# We use method accessor instead of ".label" since `menu` implements `ArrayAccess` and could have a property called "label". #}
+                            {%- set label = menu.getLabel() -%}
                             {%- if translation_domain is not same as(false) -%}
                                 {%- set label = label|trans(menu.extra('translation_params', {}), translation_domain) -%}
                             {%- endif -%}
@@ -163,7 +164,8 @@ file that was distributed with this source code.
                                                 {% if action is defined %}
                                                     {% for menu in breadcrumbs_builder.breadcrumbs(admin, action) %}
                                                         {%- set translation_domain = menu.extra('translation_domain', 'messages') -%}
-                                                        {%- set label = menu.label -%}
+                                                        {# We use method accessor instead of ".label" since `menu` implements `ArrayAccess` and could have a property called "label". #}
+                                                        {%- set label = menu.getLabel() -%}
                                                         {%- if translation_domain is not same as(false) -%}
                                                             {%- set label = label|trans(menu.extra('translation_params', {}), translation_domain) -%}
                                                         {%- endif -%}

diff --git a/tests/App/Admin/FooAdmin.php b/tests/App/Admin/FooAdmin.php
@@ -13,15 +13,23 @@
 
 namespace Sonata\AdminBundle\Tests\App\Admin;
 
+use Knp\Menu\ItemInterface as MenuItemInterface;
 use Sonata\AdminBundle\Admin\AbstractAdmin;
+use Sonata\AdminBundle\Admin\AdminInterface;
 use Sonata\AdminBundle\Datagrid\ListMapper;
 use Sonata\AdminBundle\Form\FormMapper;
 use Sonata\AdminBundle\Show\ShowMapper;
+use Sonata\AdminBundle\Tests\App\Model\Foo;
 use Sonata\AdminBundle\Templating\TemplateRegistry;
 use Symfony\Component\Form\Extension\Core\Type\TextType;
 
 final class FooAdmin extends AbstractAdmin
 {
+    public function getNewInstance()
+    {
+        return new Foo('test_id', 'foo_name');
+    }
+
     protected function configureListFields(ListMapper $list): void
     {
         $list->add('name', TemplateRegistry::TYPE_STRING, [
@@ -38,4 +46,10 @@ protected function configureShowFields(ShowMapper $show): void
     {
         $show->add('name', TemplateRegistry::TYPE_STRING);
     }
+
+    protected function configureTabMenu(MenuItemInterface $menu, $action, ?AdminInterface $childAdmin = null)
+    {
+        // Check conflict between `MenuItemInterface::getLabel()` method and menu item with a child with the key `label`
+        $menu->addChild('label')->addChild('label');
+    }
 }