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

Reports\Block\Adminhtml\Sales\Grid\Column\Renderer\Date relies on global state #1468

Closed
Vinai opened this issue Jul 11, 2015 · 1 comment
Closed

Comments

@Vinai
Copy link
Contributor

Vinai commented Jul 11, 2015

The method \Magento\Reports\Block\Adminhtml\Sales\Grid\Column\Renderer\Date::render() creates it's return value by calling \IntlDateFormatter::formatObject($date, $format);.
Please note that formatObject() is called without the optional third parameter specifying the locale, which means that the default ICU locale will be used.
Here is the method signature from the PHP manual:

public static string IntlDateFormatter::formatObject ( object $object [, mixed $format = NULL [, string $locale = NULL ]] )

If the third locale is not specified as an argument, the default ICU locale is used.
If no default locale is set, this value will default to the system default.

This causes the test \Magento\Reports\Test\Unit\Block\Adminhtml\Sales\Grid\Column\Renderer\DateTest::testRender() to fail sometimes, because it depends on the system locale.
If the system locale is set to a en_US locale the test is green. If it is a non-english locale, the test fails, because the expected output does not match the expected format:

% vendor/bin/phpunit -c dev/tests/unit/phpunit.xml.dist --filter 'testRender.+#4' app/code/Magento/Reports/Test/Unit/Block/Adminhtml/Sales/Grid/Column/Renderer/DateTest.php
PHPUnit 4.1.0 by Sebastian Bergmann.

Configuration read from dev/tests/unit/phpunit.xml

F

Time: 58 ms, Memory: 6.75Mb

There was 1 failure:

1) Magento\Reports\Test\Unit\Block\Adminhtml\Sales\Grid\Column\Renderer\DateTest::testRender with data set #4 ('2014-06-25', 'en_US', 'period', 'day', 'Jun 25, 2014')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Jun 25, 2014'
+'Juni 25, 2014'

The above output is from a dev machine where the system locale is set as follows

% echo $LANG
de_DE.UTF-8

To summarize:
The date format is correctly constructed for a given locale.
However, the date constants rendered do not necessarily match that locale.

Possible fixes:

  1. The locale is set as global state using \Locale::setDefault($locale).
  2. The locale to has to be passed as to \IntlDateFormatter::formatObject() as a third argument.

Given the problems global state brings with it option number two would clearly be better.

@kokoc
Copy link
Member

kokoc commented Aug 13, 2015

Appropriate pull request is merged. Thank you again for contribution!

@kokoc kokoc closed this as completed Aug 13, 2015
magento-team pushed a commit that referenced this issue Sep 11, 2017
[TSGAB] Bugfixing (2.2-develop-pr11)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants