Add default_admin_services node

[VincentLanglet]

Subject

BC

Changelog

### Added
- Added `default_admin_services` config key to overwrite the default services provided to all the admin.

### Deprecated
- Deprecated `admin_services` config key.

[jordisala1991]

One question: Why dont you decorate the service you want to modify? It is a feature available at the symfony dependency injection component and it allows to not write more code to do the same

[VincentLanglet]

One question: Why dont you decorate the service you want to modify? It is a feature available at the symfony dependency injection component and it allows to not write more code to do the same

I don't always want to modify a service.
For instance, #4637 ; you don't want to decorate the default label translator strategy ; just using another one.

[VincentLanglet]

Please review @sonata-project/contributors

[VincentLanglet]

I will update the tests after #7127

[VincentLanglet]

Please review @sonata-project/contributors

[franmomu]

If the use case was to select one of the available label translation strategies, what about to just add an option to set the default one?

[VincentLanglet]

If the use case was to select one of the available label translation strategies, what about to just add an option to set the default one?

What's the issue with the option default_admin_services.label_translator_strategy I added ?

Other use case can be:

• Setting the default form theme to use default_admin_services.templates.form
• Setting the default pager type to use default_admin_services.pager_type
• Setting the default data source to use default_admin_services.data_source
  And so on.

[franmomu]

IMO for modifying existing services you can do it with a service decorator as @jordisala1991 said, I think we don't need to provide a way to override them (it is the way it's documented in Symfony for overriding services from a bundle).

For templates we already have a sonata_admin.templates configuration that I think we can use instead of creating another global one.

[VincentLanglet]

IMO for modifying existing services you can do it with a service decorator as @jordisala1991 said, I think we don't need to provide a way to override them (it is the way it's documented in Symfony for overriding services from a bundle).

I may not understanding correctly decoration, but I don't think it solves the issue.
It's not about modifying a service, it's about choosing another one.

In #4637, the goal is to use a different label_translator_strategy.
It can be either one provided by sonata, either one custom.
It would avoid configuring label_translator_strategy to every admin.

For pager_type, you can use default or simple.
If you want to use the simple for every admin, this is the only way to do it.

Another example: I wrote a whole custom datasource service and I'd like to use it for almost all my admin.
I don't want to decorate the existing datasource service, my datasource has nothing in config with the existing one.

For templates we already have a sonata_admin.templates configuration that I think we can use instead of creating another global one.

We can add form, filter, view keys in sonata_admin.templates if you prefer.
I just followed the previous was it was done.

[franmomu]

What I meant is:

• If we want to override a service, it can be done with service decoration.
• If we have several choices, we can define the default one and provide an option for that.

I may not understanding correctly decoration, but I don't think it solves the issue.
It's not about modifying a service, it's about choosing another one.

In #4637, the goal is to use a different label_translator_strategy.
It can be either one provided by sonata, either one custom.
It would avoid configuring label_translator_strategy to every admin.

For example in DoctrineBundle:

doctrine:
    dbal:
        default_connection: default
    orm:
        default_entity_manager: ~

We could add:

sonata_admin:
    default_label_strategy: 'sonata.admin.label.strategy.native' # by default

This way, instead of:

SonataAdminBundle/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php

Line 323 in a8f16a1

'label_translator_strategy' => 'sonata.admin.label.strategy.native',

could be

'label_translator_strategy' => 'sonata.admin.label.strategy.default',

and sonata.admin.label.strategy.default would be an alias to the service specified in sonata_admin.default_label_stategy.

If for example we create a service that receives the label translation strategy, this way we could inject 'sonata.admin.label.strategy.default'.

For pager_type, you can use default or simple.
If you want to use the simple for every admin, this is the only way to do it.

In this case (I know it was that way, but this is a new feature) sonata_admin.default_admin_services.pager_type I would expect a service and in this case it's a choice (default, simple or other custom one), so not sure if it's in the right place or naming.

Another example: I wrote a whole custom datasource service and I'd like to use it for almost all my admin.
I don't want to decorate the existing datasource service, my datasource has nothing in config with the existing one.

When decorating the service there is no need to configure it with the existing one, you can use it or not.

For templates we already have a sonata_admin.templates configuration that I think we can use instead of creating another global one.

We can add form, filter, view keys in sonata_admin.templates if you prefer.
I just followed the previous was it was done.

IMHO if we already have a configuration for templates (and if it's possible), we should use it, AFAIK the view key it is not needed because those templates are the ones in sonata_admin.templates.

[VincentLanglet]

What I meant is:

• If we want to override a service, it can be done with service decoration.

When I read the documentation

# config/services.yaml
services:
    App\Mailer: ~

    # this replaces the old App\Mailer definition with the new one, the
    # old definition is lost
    App\Mailer:
        class: App\NewMailer

The old definition is lost.

When decorating the service there is no need to configure it with the existing one, you can use it or not.

You might want to use your custom service 90% of the time and still keep the old definition sometimes.
So decoration doesn't solve the issue here, isn't it ?

In this case (I know it was that way, but this is a new feature) sonata_admin.default_admin_services.pager_type I would expect a service and in this case it's a choice (default, simple or other custom one), so not sure if it's in the right place or naming.

I just followed the existing feature, and the way it's done for attributes of the tag sonata.admin.

        tags:
            - name: sonata.admin
            - pager_type: default
            - label_translator_strategy: sonata.admin.label.strategy.native

SonataAdminBundle/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php

Line 256 in a8f16a1

public function applyConfigurationFromAttribute(Definition $definition, array $attributes)

SonataAdminBundle/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php

Line 351 in a8f16a1

$pagerType = $overwriteAdminConfiguration['pager_type'] ?? $attributes['pager_type'] ?? Pager::TYPE_DEFAULT;

IMHO we should keep consistency between the sonata_admin config and the sonata.admin tag attributes.

What I have in mind is to allow the user to change the default values of the sonata.admin tag attributes.
Maybe I can change the name of default_admin_services ? @franmomu

[VincentLanglet]

@franmomu @jordisala1991 Can you take a new look ?

• I moved the template part to the already existing template key.
• Does another name than default_admin_services could make a consensus ?

[VincentLanglet]

Can I get some review @sonata-project/contributors

[franmomu]

Should these new values be documented somewhere?

[VincentLanglet]

I updated the doc :)

[peter-gribanov]

Perhaps it would be worthwhile to somehow more explicitly state that this change breaks BC, and give an upgrade example.

-sonata_doctrine_orm_admin:
-    templates:
-        form:
-            - '@Frontend/Admin/form/doctrine_orm_admin_fields.html.twig'
-            - '@Frontend/Admin/form/admin_fields.html.twig'
-        filter:
-            - '@Frontend/Admin/form/filter_fields.html.twig'
+sonata_admin:
+    templates:
+        form_theme:
+            - '@Frontend/Admin/form/doctrine_orm_admin_fields.html.twig'
+            - '@Frontend/Admin/form/admin_fields.html.twig'
+        filter_theme:
+            - '@Frontend/Admin/form/filter_fields.html.twig'

[VincentLanglet]

Perhaps it would be worthwhile to somehow more explicitly state that this change breaks BC, and give an upgrade example.

-sonata_doctrine_orm_admin:
-    templates:
-        form:
-            - '@Frontend/Admin/form/doctrine_orm_admin_fields.html.twig'
-            - '@Frontend/Admin/form/admin_fields.html.twig'
-        filter:
-            - '@Frontend/Admin/form/filter_fields.html.twig'
+sonata_admin:
+    templates:
+        form_theme:
+            - '@Frontend/Admin/form/doctrine_orm_admin_fields.html.twig'
+            - '@Frontend/Admin/form/admin_fields.html.twig'
+        filter_theme:
+            - '@Frontend/Admin/form/filter_fields.html.twig'

This PR is not supposed to break BC.
The old way to set templates is still available in 3.x.
The release 3.98.2 introduced an issue, fixed in SonataDoctrineORMAdminBundle/releases@3.34.0.