Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix explicitly set use_intl_templates #5979

Closed

Conversation

franmomu
Copy link
Member

Subject

Ref: #5977

Basically, when use_intl_templates was explicitly set to false but with SonataIntlBundle enabled, it kept loading the intl templates.

I am targeting this branch, because this is BC.

Closes #5977

Changelog

### Fixed
- Setting explicitly `use_intl_templates` to `false` does not load the intl templates

To test:

composer config repositories.franmomu vcs https://github.com/franmomu/SonataAdminBundle
composer require sonata-project/admin-bundle "dev-fix_override_use_intl_templates as 3.62.1"

PS: I'll try later to add the missing services to #5975

@franmomu franmomu added the patch label Mar 21, 2020
@core23
Copy link
Member

core23 commented Mar 21, 2020

Can you add a function test to cover this bug?

@franmomu franmomu closed this Mar 21, 2020
@franmomu franmomu reopened this Mar 21, 2020
@franmomu
Copy link
Member Author

Can you add a function test to cover this bug?

This one covers that case.

@greg0ire greg0ire requested a review from core23 March 21, 2020 14:11
@franmomu franmomu closed this Mar 21, 2020
@franmomu franmomu reopened this Mar 21, 2020
@franmomu
Copy link
Member Author

Restarting Travis to see if it works...

@greg0ire
Copy link
Contributor

I think there is an ongoing scheduled maintenance: https://twitter.com/traviscistatus

@franmomu franmomu closed this Mar 21, 2020
@franmomu franmomu reopened this Mar 21, 2020
@@ -212,7 +212,9 @@ public function prepend(ContainerBuilder $container)
$configs = $container->getExtensionConfig($this->getAlias());
$config = $this->processConfiguration(new Configuration(), $configs);

$useIntlTemplates = $config['use_intl_templates'] || isset($bundles['SonataIntlBundle']);
$defaultUseIntlTemplates = $config['use_intl_templates'] || isset($bundles['SonataIntlBundle']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct? I must use the intl templates if i installed the SonataIntlBundle?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before #5835, when you had installed SonataIntlBundle, the intl templates were automatically loaded from SonataIntlBundle. This simulated that behaviour.

The problem was that if you set use_intl_templates to false (explicitly) with the SonataIntlBundle installed, it did't matter, it was like it was set to true. Now this is fixed by the next line. So to sum up:

  • SonataIntlBundle installed and nothing configured: it uses intl templates
  • SonataIntlBundle not installed but use_intl_templates set to true: it loades intl templates
  • No matter if SonataIntlBundle is installed if you set use_intl_templates to false: it won't load intl templates

@phansys
Copy link
Member

phansys commented Mar 21, 2020

You should update the docs too.

@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@franmomu franmomu closed this Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option use_intl_templates is broken
5 participants