ChildrenVoter affects non Sonata Menus

[benrcole]

Environment

Sonata packages

$ composer show sonata-project/*

sonata-project/admin-bundle              3.10.3 The missing Symfony Admin G...
sonata-project/block-bundle              3.2.0  Symfony SonataBlockBundle
sonata-project/cache                     1.0.7  Cache library
sonata-project/core-bundle               3.1.2  Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.2    Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2  Doctrine2 behavioral extens...
sonata-project/doctrine-orm-admin-bundle 3.1.2  Symfony Sonata / Integrate ...
sonata-project/easy-extends-bundle       2.1.10 Symfony SonataEasyExtendsBu...
sonata-project/exporter                  1.7.0  Lightweight Exporter library
sonata-project/google-authenticator      1.0.2  Library to integrate Google...
sonata-project/user-bundle               3.2.0  Symfony SonataUserBundle

Symfony packages

$ composer show symfony/*
symfony/assetic-bundle     v2.8.1  Integrates Assetic into Symfony2
symfony/monolog-bundle     2.12.0  Symfony MonologBundle
symfony/phpunit-bridge     v3.2.1  Symfony PHPUnit Bridge
symfony/polyfill-apcu      v1.3.0  Symfony polyfill backporting apcu_* func...
symfony/polyfill-intl-icu  v1.3.0  Symfony polyfill for intl's ICU-related ...
symfony/polyfill-mbstring  v1.3.0  Symfony polyfill for the Mbstring extension
symfony/polyfill-php54     v1.3.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php55     v1.3.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php56     v1.3.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php70     v1.3.0  Symfony polyfill backporting some PHP 7....
symfony/polyfill-util      v1.3.0  Symfony utilities for portability of PHP...
symfony/security-acl       v3.0.0  Symfony Security Component - ACL (Access...
symfony/swiftmailer-bundle v2.4.2  Symfony SwiftmailerBundle
symfony/symfony            v2.8.15 The Symfony PHP framework

Other

$ composer show knplabs/*
knplabs/knp-menu        2.2.0 An object oriented menu library
knplabs/knp-menu-bundle 2.1.3 This bundle provides an integration of the Kn...

Subject

The ChildrenVoter included as part of SonataAdminBundle is affecting other KnpMenus in different parts of applications.

Building a breadcrumb as per the KnpMenuBundle documentation relies on which returns true for the root node based on the ChildrenVoter result.

As per the response from https://github.com/KnpLabs/KnpMenuBundle/issues/331 there doesnt seem to be a way to remove a Voter from a specifc Menu so is it possible to restrict the ChildrenVoter to only SonataAdmin Menus?

More info http://stackoverflow.com/questions/41304270/can-you-make-knpmenu-voters-private

Steps to reproduce

Call isCurrent on a KnpMenu root node where the current page is a child node of the root.

Expected results

Any parent returns true for isCurrent if the ChildrenVoter is loaded.

This is correct behavior for SonataAdmin menu links but shouldnt be applied to non SonataAdmin menus.

Actual results

Only the current item returns true for non SonataAdmin menus.

[greg0ire]

Link to ChildrenVoter for the lazy

[greg0ire]

Can you var_dump $item in a sonata menu, to see if there is anything specific to sonata we could use to do the restriction suggested by stof?

[benrcole]

object(Knp\Menu\MenuItem)[2325]
  protected 'name' => string 'TournamentParticipant' (length=21)
  protected 'label' => null
  protected 'linkAttributes' => 
    array (size=0)
      empty
  protected 'childrenAttributes' => 
    array (size=0)
      empty
  protected 'labelAttributes' => 
    array (size=0)
      empty
  protected 'uri' => string '{}' (length=62)
  protected 'attributes' => 
    array (size=0)
      empty
  protected 'extras' => 
    array (size=3)
      'translation_domain' => string 'messages' (length=8)
      'admin' => 
        object(EventBundle\Admin\TournamentParticipantAdmin)[2329]
          protected 'listFieldDescriptions' => 
            array (size=0)
              ...
          protected 'showFieldDescriptions' => 
            array (size=0)
              ...
          protected 'formFieldDescriptions' => 
            array (size=0)
              ...
          protected 'filterFieldDescriptions' => 
            array (size=0)
              ...
          protected 'maxPerPage' => int 32
          protected 'maxPageLinks' => int 25
          protected 'baseRouteName' => null
          protected 'baseRoutePattern' => null
          protected 'baseControllerName' => string 'SonataAdminBundle:CRUD' (length=22)
          protected 'classnameLabel' => string 'TournamentParticipant' (length=21)
          protected 'translationDomain' => string 'messages' (length=8)
          protected 'formOptions' => 
            array (size=0)
              ...
          protected 'datagridValues' => 
            array (size=2)
              ...
          protected 'perPageOptions' => 
            array (size=5)
              ...
          protected 'pagerType' => string 'default' (length=7)
          protected 'code' => string 'event.admin.tournament_participant' (length=34)
          protected 'label' => string 'TournamentParticipant' (length=21)
          protected 'persistFilters' => boolean false
          protected 'routes' => null
          protected 'subject' => null
          protected 'children' => 
            array (size=0)
              ...
          protected 'parent' => null
          protected 'baseCodeRoute' => string 'event.admin.tournament_participant' (length=34)
          protected 'parentAssociationMapping' => null
          protected 'parentFieldDescription' => null
          protected 'currentChild' => boolean false
          protected 'uniqid' => null
          protected 'modelManager' => 
            object(Sonata\DoctrineORMAdminBundle\Model\ModelManager)[2614]
              ...
          protected 'request' => null
          protected 'translator' => 
            object(Symfony\Bundle\FrameworkBundle\Translation\Translator)[63]
              ...
          protected 'formContractor' => 
            object(Sonata\DoctrineORMAdminBundle\Builder\FormContractor)[2613]
              ...
          protected 'listBuilder' => 
            object(Sonata\DoctrineORMAdminBundle\Builder\ListBuilder)[2599]
              ...
          protected 'showBuilder' => 
            object(Sonata\DoctrineORMAdminBundle\Builder\ShowBuilder)[2547]
              ...
          protected 'datagridBuilder' => 
            object(Sonata\DoctrineORMAdminBundle\Builder\DatagridBuilder)[2592]
              ...
          protected 'routeBuilder' => 
            object(Sonata\AdminBundle\Route\PathInfoBuilder)[2562]
              ...
          protected 'datagrid' => null
          protected 'routeGenerator' => 
            object(Sonata\AdminBundle\Route\DefaultRouteGenerator)[2589]
              ...
          protected 'breadcrumbs' => 
            array (size=0)
              ...
          protected 'securityHandler' => 
            object(Sonata\AdminBundle\Security\Handler\NoopSecurityHandler)[2564]
              ...
          protected 'validator' => 
            object(Symfony\Component\Validator\Validator\RecursiveValidator)[2563]
              ...
          protected 'configurationPool' => 
            object(Sonata\AdminBundle\Admin\Pool)[1017]
              ...
          protected 'menu' => null
          protected 'menuFactory' => 
            object(Knp\Menu\MenuFactory)[1009]
              ...
          protected 'loaded' => 
            array (size=4)
              ...
          protected 'formTheme' => 
            array (size=1)
              ...
          protected 'filterTheme' => 
            array (size=1)
              ...
          protected 'templates' => 
            array (size=39)
              ...
          protected 'extensions' => 
            array (size=1)
              ...
          protected 'labelTranslatorStrategy' => 
            object(Sonata\AdminBundle\Translator\NativeLabelTranslatorStrategy)[2560]
              ...
          protected 'supportsPreviewMode' => boolean false
          protected 'securityInformation' => 
            array (size=0)
              ...
          protected 'cacheIsGranted' => 
            array (size=1)
              ...
          protected 'searchResultActions' => 
            array (size=2)
              ...
          protected 'listModes' => 
            array (size=2)
              ...
          protected 'accessMapping' => 
            array (size=0)
              ...
          private 'class' (Sonata\AdminBundle\Admin\AbstractAdmin) => string 'EventBundle\Entity\TournamentParticipant' (length=40)
          private 'subClasses' (Sonata\AdminBundle\Admin\AbstractAdmin) => 
            array (size=0)
              ...
          private 'list' (Sonata\AdminBundle\Admin\AbstractAdmin) => null
          private 'show' (Sonata\AdminBundle\Admin\AbstractAdmin) => null
          private 'form' (Sonata\AdminBundle\Admin\AbstractAdmin) => null
          private 'filter' (Sonata\AdminBundle\Admin\AbstractAdmin) => null
          private 'cachedBaseRouteName' (Sonata\AdminBundle\Admin\AbstractAdmin) => null
          private 'cachedBaseRoutePattern' (Sonata\AdminBundle\Admin\AbstractAdmin) => null
          private 'formGroups' (Sonata\AdminBundle\Admin\AbstractAdmin) => boolean false
          private 'formTabs' (Sonata\AdminBundle\Admin\AbstractAdmin) => boolean false
          private 'showGroups' (Sonata\AdminBundle\Admin\AbstractAdmin) => boolean false
          private 'showTabs' (Sonata\AdminBundle\Admin\AbstractAdmin) => boolean false
          private 'managerType' (Sonata\AdminBundle\Admin\AbstractAdmin) => string 'orm' (length=3)
          private 'breadcrumbsBuilder' (Sonata\AdminBundle\Admin\AbstractAdmin) => null
      'routes' => 
        array (size=1)
          0 => 
            array (size=2)
              ...
  protected 'display' => boolean true
  protected 'displayChildren' => boolean true
  protected 'children' => 
    array (size=0)
      empty
  protected 'parent' => 
    object(Knp\Menu\MenuItem)[2364]
      protected 'name' => string 'Events' (length=6)
      protected 'label' => string 'Events' (length=6)
      protected 'linkAttributes' => 
        array (size=0)
          empty
      protected 'childrenAttributes' => 
        array (size=1)
          'class' => string 'active treeview-menu' (length=20)
      protected 'labelAttributes' => 
        array (size=0)
          empty
      protected 'uri' => null
      protected 'attributes' => 
        array (size=0)
          empty
      protected 'extras' => 
        array (size=3)
          'icon' => string '<i class='fa fa-plus'></i>' (length=26)
          'label_catalogue' => string 'SonataAdminBundle' (length=17)
          'roles' => 
            array (size=0)
              ...
      protected 'display' => boolean true
      protected 'displayChildren' => boolean true
      protected 'children' => 
        array (size=2)
          'Tournament' => 
            object(Knp\Menu\MenuItem)[2328]
              ...
          'TournamentParticipant' => 
            &object(Knp\Menu\MenuItem)[2325]
      protected 'parent' => 
        object(Knp\Menu\MenuItem)[2366]
          protected 'name' => string 'root' (length=4)
          protected 'label' => null
          protected 'linkAttributes' => 
            array (size=0)
              ...
          protected 'childrenAttributes' => 
            array (size=0)
              ...
          protected 'labelAttributes' => 
            array (size=0)
              ...
          protected 'uri' => null
          protected 'attributes' => 
            array (size=0)
              ...
          protected 'extras' => 
            array (size=0)
              ...
          protected 'display' => boolean true
          protected 'displayChildren' => boolean true
          protected 'children' => 
            array (size=4)
              ...
          protected 'parent' => null
          protected 'isCurrent' => null
          protected 'factory' => 
            object(Knp\Menu\MenuFactory)[1009]
              ...
      protected 'isCurrent' => null
      protected 'factory' => 
        object(Knp\Menu\MenuFactory)[1009]
          private 'extensions' => 
            array (size=2)
              ...
          private 'sorted' => 
            array (size=2)
              ...
  protected 'isCurrent' => null
  protected 'factory' => 
    object(Knp\Menu\MenuFactory)[1009]
      private 'extensions' => 
        array (size=2)
          0 => 
            array (size=1)
              ...
          -10 => 
            array (size=1)
              ...
      private 'sorted' => 
        array (size=2)
          0 => 
            object(Knp\Menu\Integration\Symfony\RoutingExtension)[1011]
              ...
          1 => 
            object(Knp\Menu\Factory\CoreExtension)[1010]
              ...

[greg0ire]

Well, for one, the extras array is full of Sonata-specific things… could you make a PR doing the restriction based on it?

[benrcole]

...
use Sonata\AdminBundle\Admin\AbstractAdmin;
...
    public function matchItem(ItemInterface $item)
    {
        $children = $item->getChildren();
        $match = null;
        foreach ($children as $child) {
            if ($this->matcher->isCurrent($child) && $child->getExtra('admin') instanceof AbstractAdmin) {
                $match = true;
                break;
            }
        }
        return $match;
    }

The above seems to work although it's premised on every relevant current node having an attached Admin class.

Looking at the MenuBuilder it looks like some Items may not. In that case we only currently have translation_domain that could be matched and that can be overridden by users.

Alternatively we could add something else to each MenuItem that's created by the AdminBundle but that feels a little messy.

To be honest, I think this is a flaw in the design of the global nature of the Matcher setup and I'm reluctant to hack anything else into the otherwise clean nature of the AdminBundle.

Thoughts?

[greg0ire]

I think adding something else, like a marker, just for that would be the way to go. But you're right, it would be great to have a dedicated thing for that.

[greg0ire]

The best course of action IMO would be to hack something, raise an issue in knp/menu, and mark the hack for "removal when the issue is solved".

[benrcole]

OK - happy to do this. Will submit a PR later - am hacking on a VPS at the moment

[gido]

I dealed with this exact same issue last week. I found the same hack as commented by @benrcole but as said before for now we don't a clear way to identify each MenuItem for sure.

I was on my way to create a PR (#4272) to simplify to overriding of Menu Voter shipped with Sonata by exposing classnames as parameters in the DIC config.

I will try to find the time to create a PR to add a marker on each MenuItem of Sonata Menu to be sure those voters have no side effect.

For the history, these voters were added to simplify twig logic in PR https://github.com/sonata-project/SonataAdminBundle/pull/3577/files#diff-7aad62c164c47d66f90ce813f6f91c49