allow to change 'label_translator_strategy' for all admins at global config of 'sonata_admin.admin_services' key

[nnmer]

Closes #3190
Follows PR #4577

Changelog

### Fixed
- patch to proper overwrite 'label_translator_strategy' from yml config with **sonata_admin.admin_services.label_translator_strategy** key

Subject

when we want to setup label_translator_strategy globally for all admins, we should use option sonata.admin_services.sonata_admin.admin_services which should be set in global configuration for sonata_admin.
But it doesn't work, first it rise same errors as described in issue #3190, but then it was not really applying the new strategy and still was using 'native' strategy.

This PR suppose to fix that and make the option available. Example:

sonata_admin:
  admin_services:
    label_translator_strategy: sonata.admin.label.strategy.noop

[greg0ire]

Please format your commit message according to our rules :

• The commit message subject must be less than 50 characters and tell us what you did.
• The commit message body should tell us why you did it. It is optional but highly recommended.

Bad example :

Fixed bug #989 by removing call to defraculate()

Good example:

Remove call to defraculate()

Calling this function caused a bug because it interferes 
with calls to getSchmeckles().
Fixes #989

More details in CONTRIBUTING.md

[nnmer]

@greg0ire updated

[nnmer]

hm...still fail at:

There was 1 failure:
1) Sonata\AdminBundle\Tests\DependencyInjection\AddDependencyCallsCompilerPassTest::testTranslatorDisabled
Failed asserting that exception message 'Unrecognized options "sonata_post_admin, sonata_news_admin" under "sonata_admin.admin_services"' contains 'The "translator" service is not yet enabled.

[greg0ire]

Uuuuuh why did you remove this?

[nnmer]

$rootNode = $treeBuilder->root('sonata_admin', 'array');

this already defining the global node type, and mentioned ->prototype('array') breaks underlying nodes, configuration in yml doesn't work and throws that error as in #3190 .

[greg0ire]

I think you are removing the ability to set admin services on an admin per admin basis when doing that, aren't you? Why not create a default_admin_service node instead, and not touch that? This looks like a BC-break.

[jlamur]

Please add a test to avoid breaking this feature again in the future.

[nnmer]

@OskarStark @jlamur test added. But not sure on the underlying core admin bundle logic, where is a label_translation_strategy getting over the particular admin service settings. From what i found it is the global service definition, so my test watching only that.

PS. not sure why StyleCI still failing

[jlamur]

not sure why StyleCI still failing

Public methods must be above protected ones.

[jlamur]

Isn't this code supposed to achieve the exact same goal?

[nnmer]

you mean $definition->addMethodCall($method, $args); ?
currently 'label_translator_strategy' => 'sonata.admin.label.strategy.native' is hardcoded, and there is no way to globaly change the stratagy, only one by one for each admin service

[jlamur]

My bad, I missed the fact that $settings is not $overwriteAdminConfiguration.

Anyway, if other properties sonata_admin.admin_services.* work out of the box (I mean without such ternary condition here), I don't get why you have to override this one here.

[nnmer]

not sure about other properties yet. I was needed to change the label translation strategy for all my admin services and faced this issue

but i guess this settings will have similar issue if will try to set them in a global configuration area

[jlamur]

That's why I asked. Could you test if they work? If not we will need a fix for all of them with a cleaner solution.

[greg0ire]

Related #3319 , which I closed because it did not seem very useful to others. But since you are proposing it too, it might mean that it would in fact be useful.

[jlamur]

This documentation saying that its possible to set the sonata_admin.admin_services.label_translator_strategy globally is wrong.

In fact this is a dump-reference and its misleading because it doesn't show that there should be a key like this: sonata_admin.admin_services.[admin_id].label_translator_strategy to define it per admin only.

Moreover, the issue is not only limited to label_translator_strategy but all other configurations:

• model_manager,
• form_contractor,
• show_builder,
• list_builder,
• datagrid_builder,
• translator,
• configuration_pool,
• route_generator,
• validator,
• security_handler,
• label,
• menu_factory,
• route_builder,
• label_translator_strategy,
• pager_type,
• templates.

IMO this PR should focus on implementing all these feature globally and not only the label_translator_strategy.
As suggested by @greg0ire here, it should be achieved by adding a default_admin_service node.

[SonataCI]

Could you please rebase your PR and fix merge conflicts?

[core23]

This issue should be fixed in the latest stable version.

If this is still present, feel free to reopen this issue

[VincentLanglet]

@core23 This issue is still present. I saw no way to set global default admin services.

[jorrit]

I think @jlamur is right. Perhaps a _defaults node is appropriate? This would require a simple change to AddDependencyCallsCompilerPass::applyDefaults() and of course some documentation changes.