Add information to auto configure service

[eerison]

Add tips in the docs to use AutoconfigureTag

in the symfony 5.3 was added Autoconfigure, and it solves this Pull request: https://github.com/sonata-project/SonataAdminBundle/pull/7660#issuecomment-1127513303

I'm crating this ticket to remember to add this information/tip how we could configure Sonata's admin using this new symfony's feature

[Hanmac]

when editing the Docs, i assume the order of the Admins in the Menu is based on Group Name/Class Name order?

maybe adding info (in the docs) how to alter the menu order for the admins?

[eerison]

when editing the Docs, i assume the order of the Admins in the Menu is based on Group Name/Class Name order?

maybe adding info (in the docs) how to alter the menu order for the admins?

Hey @Hanmac

I didn't test it yet, But "maybe" it can be solve using tag's priority ?!

[Hanmac]

i need to test it a later time, currently we don't use a 8.0 in development

while talking about configuring the service, i was wondering why 'manager_type' doesn't use default value, or is that on purpose?

[VincentLanglet]

I'm crating this ticket to remember to add this information/tip how we could configure Sonata's admin using this new symfony's feature

Do you plan to open the PR ? :)

[eerison]

I'm crating this ticket to remember to add this information/tip how we could configure Sonata's admin using this new symfony's feature

Do you plan to open the PR ? :)

Hey @VincentLanglet

Honestly, not for now, because I didn't have any current project using symfony 5 version :'( and I guess it should be good to test it before add in the docs! but if I didn't create this issue, we will forgot this for sure!

[tamcy]

May I know the status of the Autoconfigure attribute? Is this supposed to be usable, or still under development?

I couldn't wait to try this out after upgrading my project to PHP 8.1 and Symfony 6.1, so that I could be saved from writing service definitions in the YAML file and can enjoy the benefits of service injection in the Admin classes.

So I started to migrate some of my admin classes from YAML service definitions to attributes. Turned out it is more complex than I had thought.

Here is the original service definition:

    admin.post:
        class: App\Admin\PostAdmin
        arguments: [ ~, App\Entity\Post, App\Controller\Backend\Post ]
        tags:
            - { name: sonata.admin, manager_type: orm, label: 'nav.posts', group: 'nav.category.posts', label_catalogue: 'admin', label_translator_strategy: 'sonata.admin.label.strategy.underscore' }
        calls:
            - [ setTranslationDomain, [ 'admin' ] ]
        public: true

To minimize the effort, I just converted the original service definition to something like this:

#[Autoconfigure(
    tags: [[
        'name' => 'sonata.admin',
        'manager_type' => 'orm',
        'code' => 'admin.posts',
        'model_class' => Post::class,
        'label' => 'nav.posts',
        'group' => 'nav.category.posts',
        'label_catalogue' => 'admin',
        'label_translator_strategy' => 'sonata.admin.label.strategy.underscore',
        'controller' => PostController::class,
    ]],
    calls: [
        ['setTranslationDomain', ['admin']],
    ],
)]
class PostAdmin {...}

I was immediately greeted with an AdminCodeNotFoundException: Admin service "admin.posts" not found in admin pool. Did you mean "(....)" or one of those: []? when I tried to visit the list page of this admin.

The reason is that Sonata tried to fetch the Admin instance via the admin's code property because by convention code is equal to the service id, which is not true when using the Autoconfigure attribute: the service id is now the FQCN. (I could be wrong here, because this makes me wonder why setting this code attribute is allowed in the first place.)

So I removed the 'code' => 'admin.posts' line. No more exception, but now the admin stopped appearing on the navigation menu. This is because I am using RoleSecurityHandler to determine the user's access right, and this handler also makes use of the admin's code to build the final role attribute to check for. And the admin's code was changed to the FQCN of the admin class.

To fix this, I visited to the profiler and discovered the new name of the role became ROLE_APP\ADMIN\POSTADMIN_ALL. I added this to the user's access list and the admin menu item reappeared. But I have to say that the role name is plain ugly.

One possible improvement is to adopt the UnderscoreLabelTranslatorStrategy for FQCN admin code, i.e. change the getBaseRole() logic to something like:

    public function getBaseRole(AdminInterface $admin): string
    {
        $code = $admin->getCode();
        if ($code === get_class($admin)) {
            return sprintf('ROLE_%s_%%s', str_replace('\\', '_',  strtoupper(preg_replace('~(?<=\\w)([A-Z])~', '_$1', $code))));
        }

        return sprintf('ROLE_%s_%%s', str_replace('.', '_', strtoupper($admin->getCode())));
    }

With the above change, the base role of my PostAdmin would become ROLE_APP_ADMIN_POST_ADMIN_%s. While extra effort is still needed for migration, this should be more acceptable, but may impact the overall performance due to the increased operations.

Anyway, that's not the end. The last problem I saw is that there is no way to control the admin's order of appearance. I tried adding the priority parameter, but it doesn't work.

#[Autoconfigure(
    tags: [[
...
        'controller' => PostController::class,
        'priority' => 10000,
    ]],
...

The cause is that AddDependencyCallsCompilerPass doesn't take the priority value into account. I need to hack into the class and add this function (modified from PriorityTaggedServiceTrait):

    private function findSortedServiceIds(ContainerBuilder $container, string $tagName): array
    {
        $indexAttribute = $defaultIndexMethod = $needsIndexes = $defaultPriorityMethod = null;

        $i = 0;
        $services = [];

        foreach ($container->findTaggedServiceIds($tagName) as $serviceId => $attributes) {
            if (isset($services[$serviceId])) {
                // only the first priority attribute is used.
                continue;
            }

            foreach ($attributes as $attribute) {
                $priority = $attribute['priority'] ?? 0;
                $services[$serviceId] = [$priority, ++$i, $serviceId, $attributes];
            }
        }

        uasort($services, static function ($a, $b) {
            return $b[0] <=> $a[0] ?: $a[1] <=> $b[1];
        });

        $refs = [];
        foreach ($services as [, , $serviceId, $attributes]) {
            $refs[$serviceId] = $attributes;
        }

        return $refs;
    }

Then change the line

        foreach ($container->findTaggedServiceIds(TaggedAdminInterface::ADMIN_TAG) as $id => $tags) {

to:

        foreach ($this->findSortedServiceIds($container, TaggedAdminInterface::ADMIN_TAG) as $id => $tags) {

Now the priority attribute is correctly acknowledged.

So it seems to me that there's still something that needs to be tackled on the Sonata side. I didn't went any further because I started to wonder if I did anything wrong or had misundertood anything, hence my question. Thanks in advance.

[VincentLanglet]

The reason is that Sonata tried to fetch the Admin instance via the admin's code property because by convention code is equal to the service id, which is not true when using the Autoconfigure attribute: the service id is now the FQCN. (I could be wrong here, because this makes me wonder why setting this code attribute is allowed in the first place.)

This not fully true.

In the following code,

admin.post:
        class: App\Admin\PostAdmin
        arguments: [ ~, App\Entity\Post, App\Controller\Backend\Post ]
        tags:
            - { name: sonata.admin, manager_type: orm, label: 'nav.posts', group: 'nav.category.posts', label_catalogue: 'admin', label_translator_strategy: 'sonata.admin.label.strategy.underscore' }
        calls:
            - [ setTranslationDomain, [ 'admin' ] ]
        public: true

You're are not passing any code ; which is the first of arguments ~ , therefore this is using the service id. And the service id is admin.post.

But when migrating your code, I would expect 'code' => 'admin.post', (withouts`) to work. IMHO we should focus on this rather than adding a security prefix.

Anyway, that's not the end. The last problem I saw is that there is no way to control the admin's order of appearance. I tried adding the priority parameter, but it doesn't work.

Does it work when you're using it in the yaml definition ?

[Hanmac]

Anyway, that's not the end. The last problem I saw is that there is no way to control the admin's order of appearance. I tried adding the priority parameter, but it doesn't work.

Does it work when you're using it in the yaml definition ?

i can confirm that the menu entries are added in the order in the yaml file

[tamcy]

In the following code,

admin.post:
        class: App\Admin\PostAdmin
        arguments: [ ~, App\Entity\Post, App\Controller\Backend\Post ]
        tags:
            - { name: sonata.admin, manager_type: orm, label: 'nav.posts', group: 'nav.category.posts', label_catalogue: 'admin', label_translator_strategy: 'sonata.admin.label.strategy.underscore' }
        calls:
            - [ setTranslationDomain, [ 'admin' ] ]
        public: true

You're are not passing any code ; which is the first of arguments ~ , therefore this is using the service id. And the service id is admin.post.

But when migrating your code, I would expect 'code' => 'admin.post', (withouts`) to work. IMHO we should focus on this rather than adding a security prefix.

From my testing, if an admin's service id isn't the same as its code, the admin item will still appears on the navigation menu, but the following exception will be thrown when user tries to access the admin (this also happens to YAML defined admin services):

Sonata\AdminBundle\Exception\AdminCodeNotFoundException:
Admin service "admin.backend_user" not found in admin pool. Did you mean "admin.my_account" or one of those: []?

  at ...\vendor\sonata-project\admin-bundle\src\Admin\Pool.php:307
  at Sonata\AdminBundle\Admin\Pool->getInstance('admin.backend_user')
     (...\vendor\sonata-project\admin-bundle\src\Admin\Pool.php:199)
  at Sonata\AdminBundle\Admin\Pool->getAdminByAdminCode('admin.backend_user')
     (...\vendor\sonata-project\admin-bundle\src\Request\AdminFetcher.php:43)
  at Sonata\AdminBundle\Request\AdminFetcher->get(object(Request))
     (...\vendor\sonata-project\admin-bundle\src\Controller\CRUDController.php:955)
  at Sonata\AdminBundle\Controller\CRUDController->configureAdmin(object(Request))
     (...\vendor\sonata-project\admin-bundle\src\EventListener\ConfigureCRUDControllerListener.php:40)
  at Sonata\AdminBundle\EventListener\ConfigureCRUDControllerListener->onKernelController(object(ControllerEvent), 'kernel.controller', object(TraceableEventDispatcher))
     (...\vendor\symfony\event-dispatcher\Debug\WrappedListener.php:115)
  at Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke(object(ControllerEvent), 'kernel.controller', object(TraceableEventDispatcher))
     (...\vendor\symfony\event-dispatcher\EventDispatcher.php:230)
  at Symfony\Component\EventDispatcher\EventDispatcher->callListeners(array(object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener), object(WrappedListener)), 'kernel.controller', object(ControllerEvent))
     (...\vendor\symfony\event-dispatcher\EventDispatcher.php:59)
  at Symfony\Component\EventDispatcher\EventDispatcher->dispatch(object(ControllerEvent), 'kernel.controller')
     (...\vendor\symfony\event-dispatcher\Debug\TraceableEventDispatcher.php:153)
  at Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher->dispatch(object(ControllerEvent), 'kernel.controller')
     (...\vendor\symfony\http-kernel\HttpKernel.php:141)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (...\vendor\symfony\http-kernel\HttpKernel.php:75)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (...\vendor\symfony\http-kernel\Kernel.php:202)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (...\vendor\symfony\runtime\Runner\Symfony\HttpKernelRunner.php:35)
  at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
     (...\vendor\autoload_runtime.php:29)
  at require_once('...\vendor\autoload_runtime.php')
     (...\public\index.php:5)                

This is thrown by the getInstance() method of Sonata\AdminBundle\Admin\Pool: while the method is named getAdminByAdminCode(), it actually tries to fetch the admin instance by calling getInstance(), which tries to fetch the admin from the service locator through its service id. And it looks like "service id" and "code" are interchangeable in the admin bundle: the method signature is "getInstance(string $id)", while its docblock says "Returns a new admin instance depends on the given code.".

I can make the attribute work again by hacking into AddDependencyCallsCompilerPass and add the following code after this line:

                if (isset($attributes['code']) && $attributes['code'] !== $id) {
                    $adminServices[$attributes['code']] = new Reference($id);
                    $admins[] = $attributes['code'];
                }

The above code registers the admin with its code when it is different from the service id, only in the admin pool's service locator. It doesn't affect the global container. This also removes the need for a separate role prefix configuration, if the code property isn't going to be removed (I remember I've read about the proposal of the deprecation of code somewhere, but I can't find it now). Also, as I don't know this library well enough, I am not sure if this will cause any side effects. So please help to see if this is a viable fix.

[VincentLanglet]

From my testing, if an admin's service id isn't the same as its code, the admin item will still appears on the navigation menu, but the following exception will be thrown when user tries to access the admin (this also happens to YAML defined admin services):

IMHO this is a bug we should fix.

I can make the attribute work again by hacking into AddDependencyCallsCompilerPass and add the following code after this line:

                if (isset($attributes['code']) && $attributes['code'] !== $id) {
                    $adminServices[$attributes['code']] = new Reference($id);
                    $admins[] = $attributes['code'];
                }

The above code registers the admin with its code when it is different from the service id, only in the admin pool's service locator. It doesn't affect the global container. This also removes the need for a separate role prefix configuration, if the code property isn't going to be removed (I remember I've read about the proposal of the deprecation of code somewhere, but I can't find it now). Also, as I don't know this library well enough, I am not sure if this will cause any side effects. So please help to see if this is a viable fix.

I think indeed using

$code = $attributes['code'] ?? $id;

and then passing them in the pool (and renaming the property from adminServiceIds to adminServiceCodes), would be the best way to follow the way code was meant. WDYT @jordisala1991

Can you try a PR @tamcy ? It will be easier to see the impact on it, especially thanks to our tests.

[tamcy]

@VincentLanglet I have submitted PR #7940, please see if this is what you meant, thanks!

While not exactly related, I would like to know the official stance about tagging the same service with sonata.admin more than once, especially after the changes in #7684. It looks like the framework should now prohibit developers from doing so, i.e. throw an RuntimeException if the following definition is encountered:

services:
    App\Admin\FooAdmin:
        tags:
            - { name: sonata.admin, code: app.admin.foo, model_class: App\Entity\Foo, manager_type: orm, label_translator_strategy: sonata.admin.label.strategy.underscore, group: 'group_one' }
            - { name: sonata.admin, code: app.admin.foo, model_class: App\Entity\Foo, manager_type: orm, label_translator_strategy: sonata.admin.label.strategy.underscore, group: 'group_two' }

The reason is that specifying different model_class/code/controller in different tags will not achieve what we expect. In reality only one of them works. However, the above definition is useful when we want to show an admin item in more than one group. Before that, the model_class/code/controller parameters were specified in the constructor, so the sonata.admin tag could somewhat be seen as navigation item registration, but now it's no longer the case.

[VincentLanglet]

While not exactly related, I would like to know the official stance about tagging the same service with sonata.admin more than once, especially after the changes in https://github.com/sonata-project/SonataAdminBundle/pull/7684. It looks like the framework should now prohibit developers from doing so

I never did this, and never thought this was possible/supported.

The reason is that specifying different model_class/code/controller in different tags will not achieve what we expect. In reality only one of them works.

Why ?

services:
    App\Admin\FooOrBarAdmin:
        tags:
            - { name: sonata.admin, code: app.admin.foo, model_class: App\Entity\Foo, manager_type: orm, label_translator_strategy: sonata.admin.label.strategy.underscore, group: 'group_one' }
            - { name: sonata.admin, code: app.admin.bar, model_class: App\Entity\Bar, manager_type: orm, label_translator_strategy: sonata.admin.label.strategy.underscore, group: 'group_two' }

doesn't work ?

[tamcy]

services:
    App\Admin\FooOrBarAdmin:
        tags:
            - { name: sonata.admin, code: app.admin.foo, model_class: App\Entity\Foo, manager_type: orm, label_translator_strategy: sonata.admin.label.strategy.underscore, group: 'group_one' }
            - { name: sonata.admin, code: app.admin.bar, model_class: App\Entity\Bar, manager_type: orm, label_translator_strategy: sonata.admin.label.strategy.underscore, group: 'group_two' }

doesn't work ?

It doesn't work because ultimately there is only one single service, all method calls will be done against the service, not the admin code. Tagging a service with sonata.admin twice doesn't change this fact.

[VincentLanglet]

It doesn't work because ultimately there is only one single service, and all method calls will be done against the service id, not the admin code. Having two distinct admin code for a single service doesn't change this fact.

Ok. It's not meant to be used this way anyway, so I'm not against an exception.

[VincentLanglet]

@tamcy With all the recent PR, does the auto-configure fully working ?

Could be a great time to add documentation then :)

[tamcy]

Well, those PRs are more about fixing an edge-case problem that cause errors when using annotations to define admin services, which is usually not used when services are defined with YAML, i.e. defining a service with its id difference from its code. The Autoconfigure tag already works in my case if no code is defined.

I am able the verify the following code/annotation works fine after the PRs are merged:

#[Autoconfigure(
    tags: [
        [
            'name' => 'sonata.admin',
            'code' => 'admin.document',
            'manager_type' => 'orm',
            'model_class' => Document::class,
            'controller' => DocumentController::class,
            'label' => 'nav.document',
            'group' => 'nav.category.document',
            'label_catalogue' => 'admin',
            'label_translator_strategy' => 'sonata.admin.label.strategy.underscore',
            'priority' => 7010,
        ]
    ],
    calls: [
        ['setTranslationDomain', ['admin']],
    ],
)]
class DocumentAdmin extends AbstractAdmin
{
   // ...
}

What I am not sure is the more advanced configurations like defining child admins, so I cannot guarantee if it is fully working. But I think it is not a bad idea to encourage devs to try this feature out, as the above example should serve most of the cases. Even when there's a problem they can still fall back to the old way to define admin services until the issue is resolved.

[haivala]

ChildAdmin definition is just another calls method addChild like in your example the setTranslationDomain so I don't see why it would not work.