Skip to content

Fixes translations by re-running the loadData for its correct locale #10913

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

Merged
merged 2 commits into from
Oct 30, 2017

Conversation

wiardvanrij
Copy link
Contributor

@wiardvanrij wiardvanrij commented Sep 17, 2017

Description

When running setup:static-content:deploy with multiple languages specified, it will generate a js-translation.json. Even if the locale is set properly for each language, it will translate just based on
$this->translator->getData(); in Magento\Framework\Phrase\Renderer

In other words it will use the data retrieved from the loadData function in the Framework/Translation.php
This uses a constructor which sets the locale; \Magento\Framework\Locale\ResolverInterface $locale and obviously followed by $this->_locale = $locale;

The problem here is that the loadData only gets ran once on init. It will keep using this for every other locale as 'base'. Therefore a potential locale change should also re-run the loadData.

By forcing a "rebuild" of the loadData with the correct locale, we will parse the data with the correct translations.

As extra because we are forcing the locale & rebuild we can remove the FallbackContext filler for the locale as we are setting it anyways.

Fixed Issues

  1. [2.2.0-RC2.2] Static content deploy with multiple locales: js-translation.json files are the same #10673: [2.2.0-RC2.2] Static content deploy with multiple locales: js-translation.json files are the same

Manual testing scenarios

1 Run php bin/magento setup:static-content:deploy -f -t Magento/luma nl_NL fr_FR
2 Run diff -rwq pub/static/frontend/Magento/luma/nl_NL/ pub/static/frontend/Magento/luma/fr_FR/
3 js-translation.json are now different from each other with the correct language translations

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Sep 17, 2017

CLA assistant check
All committers have signed the CLA.

@wiardvanrij
Copy link
Contributor Author

I do not know why the unit test gives me Call to a member function loadData() on null. It makes no sense.. Anyone got an idea?

@vrann
Copy link
Contributor

vrann commented Sep 18, 2017

@douweegbertje: This class \Magento\Translation\Test\Unit\Model\Json\PreProcessorTest creates a mock of TranslateInterface so it can not possibly have the right logic for setLocale which should return proper object:

 $this->translateMock = $this->getMockForAbstractClass(\Magento\Framework\TranslateInterface::class);

Please create the mock for setLocale method.

@magento-engcom-team magento-engcom-team changed the base branch from develop to 2.3-develop October 20, 2017 15:33
@okorshenko okorshenko self-assigned this Oct 25, 2017
@okorshenko okorshenko added this to the October 2017 milestone Oct 25, 2017
@magento-team magento-team merged commit b16e763 into magento:2.3-develop Oct 30, 2017
magento-team pushed a commit that referenced this pull request Oct 30, 2017
magento-team pushed a commit that referenced this pull request Oct 30, 2017
[EngCom] Public Pull Requests - develop
 - MAGETWO-82942: Send email to subscribers only when are new #11604
 - MAGETWO-82752: Fixes translations by re-running the loadData for its correct locale #10913
 - MAGETWO-82460: Fix AcountManagementTest unit test fail randomly #11605
 - MAGETWO-82070: Update knockout #11269
davidalger added a commit to davidalger/capistrano-magento2 that referenced this pull request May 29, 2020
…3 and through 2.2.2 or possible up to but not including 2.3.0

Indicator of being fixed in 2.2.2 and later: magento/magento2#10673 (comment)

Indicator of it being fixed in 2.3.0 and later: magento/magento2#10913

Related to magento/magento2#7862 which is now closed as resolved in 2.2.x somewhere
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants