add "setFormTheme" and "setFilterTheme" calls + change "initialize" priority

[dmaicher]

Subject

I am targeting this branch, because its BC and part of fixing #7104

Changelog

### Fixed
- `AdminInterface::initialize()` is the last method called
- `admin_services.templates.form` is correctly set
- `admin_services.templates.filter` is correctly set
- default service setter calls on `AdminInterface` are only added if there are no calls defined yet

[franmomu]

should we check if the user has already call setFormTheme in the service definition and merge the arguments?

[VincentLanglet]

I would say that if you want to set extra form theme, you should do it with a lower priority instead.

For example sonata-project/SonataDoctrineORMAdminBundle#1427

[franmomu]

I mean, if the user has a definition like:

Sonata\AdminBundle\Tests\App\Admin\FooAdmin:
        arguments: [~, Sonata\AdminBundle\Tests\App\Model\Foo, Sonata\AdminBundle\Controller\CRUDController]
        tags:
            - {name: sonata.admin, manager_type: test, label: Foo}
        calls:
            - [ setFormTheme, [['foo.html.twig' ]]]

I guess this setFormTheme call from the compiler pass will override the parameters previously set in this definition.

[VincentLanglet]

With this config, what is called first ?
setFormTheme, [['foo.html.twig' ]] or $definition->addMethodCall('setFormTheme', [$overwriteAdminConfiguration['templates']['form'] ?? []]); ?

I would expect the setFormTheme, [['foo.html.twig' ]] to be called last and override the global configuration. This would allow to remove a form theme and use another one instead for example. Isn't the case ?

But maybe the best would be to expose an addFormTheme method instead.

[franmomu]

I would say setFormTheme, [['foo.html.twig' ]] is called first as it is in the definition of the service, and the compiler pass retrieves this definition, but I don't know.

We merge the arguments in the ORM:

https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/7493cfffcbe9996316e2dd0cdd9ac5092cf62a18/src/DependencyInjection/Compiler/AddTemplatesCompilerPass.php#L44

Maybe we can do that.

[VincentLanglet]

Which one ?

[dmaicher]

There was 1 failure:

1) Sonata\AdminBundle\Tests\DependencyInjection\Compiler\AddDependencyCallsCompilerPassTest::testProcessResultingConfig
None of the method calls matched the expected method "setRouteBuilder" with arguments Array &0 (
    0 => 'sonata.admin.route.path_info'
) with any invocation order index
Failed asserting that Symfony\Component\DependencyInjection\Definition Object &000000004bf94b930000000046f6a723 (
    'class' => 'Sonata\AdminBundle\Tests\DependencyInjection\Compiler\MockAdmin'
    'file' => null
    'factory' => null
    'shared' => false
    'deprecation' => Array &0 ()
    'properties' => Array &1 ()
    'calls' => Array &2 (
        0 => Array &3 (
            0 => 'setManagerType'
            1 => Array &4 (
                0 => 'orm'
            )
        )
        1 => Array &5 (
            0 => 'setPagerType'
            1 => Array &6 (
                0 => 'simple'
            )
        )
        2 => Array &7 (
            0 => 'setLabel'
            1 => Array &8 (
                0 => 'Foo'
            )
        )
        3 => Array &9 (
            0 => 'setListModes'
            1 => Array &10 (
                0 => Array &11 (
                    'list' => Array &12 (
                        'class' => 'fa fa-list fa-fw'
                    )
                    'mosaic' => Array &13 (
                        'class' => 'fa fa-th-large fa-fw'
                    )
                )
            )
        )
        4 => Array &14 (
            0 => 'setTemplateRegistry'
            1 => Array &15 (
                0 => Symfony\Component\DependencyInjection\Reference Object &000000004bf94ffb0000000046f6a723 (
                    'id' => 'sonata_news_admin.template_registry'
                    'invalidBehavior' => 1
                )
            )
        )
        5 => Array &16 (
            0 => 'setSecurityInformation'
            1 => Array &17 (
                0 => '%sonata.admin.configuration.security.information%'
            )
        )
        6 => Array &18 (
            0 => 'setFormTheme'
            1 => Array &19 (
                0 => Array &20 ()
            )
        )
        7 => Array &21 (
            0 => 'setFilterTheme'
            1 => Array &22 (
                0 => Array &23 ()
            )
        )
    )
    'instanceof' => Array &24 ()
    'autoconfigured' => false
    'configurator' => null
    'tags' => Array &25 (
        'sonata.admin' => Array &26 (
            0 => Array &27 (
                'group' => 'sonata_group_two'
                'label' => '5 Entry'
                'manager_type' => 'orm'
            )
        )
    )
    'public' => true
    'synthetic' => false
    'abstract' => false
    'lazy' => false
    'decoratedService' => null
    'autowired' => false
    'changes' => Array &28 (
        'public' => true
        'class' => true
        'shared' => true
    )
    'bindings' => Array &29 ()
    'errors' => Array &30 ()
    'arguments' => Array &31 (
        0 => 'sonata_news_admin'
        1 => 'Sonata\AdminBundle\Tests\DependencyInjection\Compiler\News'
        2 => 'Sonata\AdminBundle\Controller\CRUDController'
    )
    'innerServiceId' => null
    'decorationOnInvalid' => null
) has a method call to "setRouteBuilder" with the given arguments..

/var/www/SonataAdminBundle/vendor/matthiasnoback/symfony-dependency-injection-test/PhpUnit/DefinitionHasMethodCallConstraint.php:59
/var/www/SonataAdminBundle/vendor/matthiasnoback/symfony-dependency-injection-test/PhpUnit/AbstractContainerBuilderTestCase.php:188
/var/www/SonataAdminBundle/tests/DependencyInjection/Compiler/AddDependencyCallsCompilerPassTest.php:195

[dmaicher]

So we are not setting default service calls then anymore, right? If there is no overwrite and no call yet.

[dmaicher]

maybe like this then? tests pass

diff --git a/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php b/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php
index b7af06402..c4b4508e9 100644
--- a/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php
+++ b/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php
@@ -338,7 +338,7 @@ class AddDependencyCallsCompilerPass implements CompilerPassInterface
 
             $method = $this->generateSetterMethodName($attr);
 
-            if (isset($overwriteAdminConfiguration[$attr]) || !$definition->hasMethodCall($method)) {
+            if (!$definition->hasMethodCall($method)) {
                 $args = [new Reference($overwriteAdminConfiguration[$attr] ?? $addServiceId)];
                 if ('translator' === $attr) {
                     $args[] = false;

so this would never overwrite any existing call

[VincentLanglet]

Indeed, since we're doing

$overwriteAdminConfiguration[$attr] ?? $addServiceId

As soon as there is no call, we have something to set.

[VincentLanglet]

Thanks !