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

[shopsys] Eased installation of project with different domains and locales settings #1425

Merged
merged 63 commits into from Oct 18, 2019

Conversation

vitek-rostislav
Copy link
Contributor

@vitek-rostislav vitek-rostislav commented Sep 24, 2019

In this PR, all classes with multidomain data fixtures (i.e. Multidomain*DataFixture) are removed and the multi-attributes are defined in regular data fixture classes using the translator. As a part of this modification, we changed the behavior of multidomain data fixtures a little bit - now the data are different on the 2nd domain only (eg. czech locale, different currency, distinct set of customers, category tree, top products settings, ...), for the 3rd and all the other domains are used data from the 1st domain. - It makes much more sense this way - the first domain has been also used as a template for copying multidomain data in DomainDataCreator.

In order to easily implement the translator feature in data fixtures, we also removed the CSV files for products and customers data.

We have also rewritten all the tests that were tight to default domains settings so that they are now resistant against the changes in domains and locales settings (we are using domain service to access 1st domain locale instead of hardcoding "en" locale, and we are using translator where needed). We even rewritten the tests for filtering that were using specific parameter values translations IDs as these can change when a different locale is used so we are now accessing the translated values using getParameterValueByValueTextAndLocale method.

We were not able to rewrite all the tests this way (eg. functional tests for searching a particular phrase in elasticsearch depend heavily on the locale setting) - such tests are now skipped if the settings for the first domain are changed.

To make the data fixtures and tests more resistant against changes of default currencies on domains, we introduced PriceConverter that is used in tests and data fixtures everywhere where specific prices were used.

We addressed even acceptance tests - there is new LocalizationHelper that uses the translator to get the proper translations, and enables the tester to access localized routes. There is yet another new helper - NumberFormatHelper - that is able to format numbers in the proper locale and with a proper currency symbol.

In acceptance tests, there is still a need for changes when a default currency is changed - there is no way how to easily convert total prices in cart or total prices of an order (we can not just convert the total prices because of the price calculation), however, at least filtering acceptance test is resistant against the currency changes as it is possible to convert the values in price filter using PriceConverter.

Q A
Description, reason for the PR Currently, it is hard to adjust domain and locale setting as the data fixtures are often hardcoded for "en" and "cs" locale and also tests often count heavily on these settings.
New feature Yes
BC breaks No - all changes are only in project-base. Changes in the framework package are done backward-compatible
Fixes issues closes #977 closes #1351
Have you read and signed our License Agreement for contributions? Yes

@vitek-rostislav vitek-rostislav force-pushed the tl-rv-datafixtures-refactoring branch 5 times, most recently from 98c6f7b to 5bedf98 Compare September 26, 2019 12:26
@TomasLudvik TomasLudvik force-pushed the tl-rv-datafixtures-refactoring branch 2 times, most recently from f5bed87 to 46bebdf Compare October 1, 2019 16:54
@TomasLudvik TomasLudvik force-pushed the tl-rv-datafixtures-refactoring branch from 6a933e3 to 0b68383 Compare October 2, 2019 12:31
@vitek-rostislav vitek-rostislav force-pushed the tl-rv-datafixtures-refactoring branch 4 times, most recently from cfc83b9 to 6d18aec Compare October 2, 2019 14:53
@vitek-rostislav vitek-rostislav changed the title [project-base] Translator for multi-* data fixtures [project-base] Eased installtion of project with different domans and locales settings Oct 3, 2019
Copy link
Contributor

@DavidOstrozlik DavidOstrozlik left a comment

Choose a reason for hiding this comment

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

Hi, nice work. Please change the name of the PR to [shopsys].
I think that this should be targeted to the 9.0 branch as I saw some BC breaks and its a major feature.

@TomasLudvik TomasLudvik force-pushed the tl-rv-datafixtures-refactoring branch from f4e4052 to 35ac52a Compare October 3, 2019 11:48
@vitek-rostislav vitek-rostislav force-pushed the tl-rv-datafixtures-refactoring branch 2 times, most recently from 78b33c5 to adb684f Compare October 3, 2019 12:29
@grossmannmartin
Copy link
Member

Hi guys,
this PR looks like a nice candidate for checking out if SonarCloud really helps or not :)
Can you check it out during CR/fix?

vitek-rostislav and others added 24 commits October 18, 2019 09:27
…ings

- as a result, there is no need for fixing this test when a project has a different domains and locales settings
…ings

- as a result, there is no need for fixing this test when a project has a different domains and locales settings
- i.e. there is no need to fix the tests when different locales for 1st domain and/or administration are used
- proper tranlsations and formatting are used thanks to the new LocalizationHelper
- some translation messages in controllers (Paymnet, Product, Transport, Vat) rewritten to proper usage of translation parameters so it is possible to use the translation outside Twig
- creation of CurrencyFormatter extracted from PriceExtension to new CurrencyFormatterFactory
…g are run only when the first domain has English locale
…mically loaded in order to load right id for current locale
… asserting a particular product is in the pagination result anymore

- the results are ordered by product name so when using a different locale then "en", the particular product might not be on the first page
…with different locale than english

- when first domain is for example in czech language than order of products might be different
…locale than english

- when first domain is for example in czech language than results of search will be different
- testParameterFilterChoicesFromCategory does no longer require strict order of parameters and its values
- ParameterValueIds are now loaded by its translations
- other tests test localization on domains so this is redundant
…not provided

- when in admin, admin locale must be used (it can be different from the first domain locale)
- LocalizationListener:isAdminRequest() protected method exracted to AdministrationFacade:isInAdmin() public method
…rder to make this test currency independent

- if we want to assert particular prices in tests, we need to have particular prices in the order (now the prices are loaded dynamically in data fixtures based on the domain currency)
…he test is now currency independent

- PriceConverter is used for the prices in the filter
- PriceConverter is used for the prices in the filter
- PriceConverter is used for the values in price filter
…TestCase and searchPriceTestCase are now currency independent

- PriceConverter is used
- this commit addresses only the files that were modified in this PR
- for proper solution in the whole codebase, there is a separate issue - #1453
@TomasLudvik TomasLudvik merged commit a6a835b into master Oct 18, 2019
@TomasLudvik TomasLudvik changed the title [shopsys] Eased installtion of project with different domans and locales settings [shopsys] Eased installation of project with different domains and locales settings Oct 18, 2019
@TomasLudvik TomasLudvik deleted the tl-rv-datafixtures-refactoring branch October 18, 2019 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using the translator for multilocale demo data
6 participants