Skip to content

Commit

Permalink
Remove dependency of date renderer on global state locale
Browse files Browse the repository at this point in the history
Summary
==
This patch introduces a test to reproduce the bug described in issue #1468.
`\Magento\Reports\Test\Unit\Block\Adminhtml\Sales\Grid\Column\Renderer\DateTest::testDateIsRenderedIndependentOfSystemDefaultLocale`

The existing test `testRender` was refactored a little to avoid code duplication with the new test.

The patch also includes a fix for the issue, which simply is to pass the locale that was used to create the date format as a third argument to `\IntlDateFormatter::formatObject()`.

The following contents of the commit message are a slightly abbreviated copy of the issue #1468.

-----

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](http://php.net/manual/en/intldateformatter.formatobject.php):

```
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.

``
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 to **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.
  • Loading branch information
Vinai committed Jul 11, 2015
1 parent c028812 commit a408079
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public function render(\Magento\Framework\Object $row)
} else {
$date = $this->_localeDate->date(new \DateTime($data), null, false);
}
return \IntlDateFormatter::formatObject($date, $format);
return \IntlDateFormatter::formatObject($date, $format, $this->_localeResolver->getLocale());
}
return $this->getColumn()->getDefault();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,44 @@ class DateTest extends \PHPUnit_Framework_TestCase
*/
protected $localeDate;

/**
* @var string
*/
private $globalStateLocaleBackup;

/**
* @param string $locale
*/
private function mockGridDateRendererBehaviorWithLocale($locale)
{
$this->resolverMock->expects($this->any())->method('getLocale')->willReturn($locale);
$this->localeDate->expects($this->any())->method('getDateFormat')->willReturnCallback(
function ($value) use ($locale) {
return (new \IntlDateFormatter(
$locale,
$value,
\IntlDateFormatter::NONE
))->getPattern();
}
);
}

/**
* @param string $objectDataIndex
* @param string $periodType
*/
private function mockGridDateColumnConfig($objectDataIndex, $periodType)
{
$columnMock = $this->getMockBuilder('Magento\Backend\Block\Widget\Grid\Column')
->disableOriginalConstructor()
->setMethods(['getIndex', 'getPeriodType'])
->getMock();
$columnMock->expects($this->once())->method('getIndex')->willReturn($objectDataIndex);
$columnMock->expects($this->atLeastOnce())->method('getPeriodType')->willReturn($periodType);

$this->date->setColumn($columnMock);
}

/**
* {@inheritDoc}
*/
Expand All @@ -41,15 +79,15 @@ protected function setUp()
$this->localeDate
->expects($this->once())
->method('date')
->will($this->returnArgument(0));
->willReturnArgument(0);

$this->contextMock = $this->getMockBuilder('Magento\Backend\Block\Context')
->disableOriginalConstructor()
->getMock();
$this->contextMock
->expects($this->once())
->method('getLocaleDate')
->will($this->returnValue($this->localeDate));
->willReturn($this->localeDate);

$this->resolverMock = $this->getMockBuilder('Magento\Framework\Locale\ResolverInterface')
->getMock();
Expand All @@ -58,43 +96,41 @@ protected function setUp()
$this->contextMock,
$this->resolverMock
);

$this->globalStateLocaleBackup = \Locale::getDefault();
}

protected function tearDown()
{
$this->restoreTheDefaultLocaleGlobalState();
}

private function restoreTheDefaultLocaleGlobalState()
{
if (\Locale::getDefault() !== $this->globalStateLocaleBackup) {
\Locale::setDefault($this->globalStateLocaleBackup);
}
}

/**
* @param string $data
* @param string $index
* @param string $locale
* @param string $index
* @param string $period
* @param string $result
* @dataProvider datesDataProvider
* @return void
*/
public function testRender($data, $index, $locale, $period, $result)
public function testRender($data, $locale, $index, $period, $result)
{
$this->resolverMock->expects($this->any())->method('getLocale')->will($this->returnValue($locale));
$this->localeDate->expects($this->any())->method('getDateFormat')->willReturnCallback(
function ($value) use ($locale) {
return (new \IntlDateFormatter(
$locale,
$value,
\IntlDateFormatter::NONE
))->getPattern();
}
);
$this->mockGridDateRendererBehaviorWithLocale($locale);
$this->mockGridDateColumnConfig($index, $period);

$objectMock = $this->getMockBuilder('Magento\Framework\Object')
->setMethods(['getData'])
->getMock();
$objectMock->expects($this->once())->method('getData')->will($this->returnValue($data));

$columnMock = $this->getMockBuilder('Magento\Backend\Block\Widget\Grid\Column')
->disableOriginalConstructor()
->setMethods(['getIndex', 'getPeriodType'])
->getMock();
$columnMock->expects($this->once())->method('getIndex')->will($this->returnValue($index));
$columnMock->expects($this->atLeastOnce())->method('getPeriodType')->will($this->returnValue($period));
$objectMock->expects($this->once())->method('getData')->willReturn($data);

$this->date->setColumn($columnMock);

$this->assertEquals($result, $this->date->render($objectMock));
}
Expand All @@ -107,39 +143,53 @@ public function datesDataProvider()
return [
[
'data' => '2000',
'index' => 'period',
'locale' => 'en_US',
'index' => 'period',
'period' => 'year',
'result' => '2000'
],
[
'data' => '2030',
'index' => 'period',
'locale' => 'en_US',
'index' => 'period',
'period' => 'year',
'result' => '2030'
],
[
'data' => '2000-01',
'index' => 'period',
'locale' => 'en_US',
'index' => 'period',
'period' => 'month',
'result' => '1/2000'
],
[
'data' => '2030-12',
'index' => 'period',
'locale' => 'en_US',
'index' => 'period',
'period' => 'month',
'result' => '12/2030'
],
[
'data' => '2014-06-25',
'index' => 'period',
'locale' => 'en_US',
'index' => 'period',
'period' => 'day',
'result' => 'Jun 25, 2014'
]
];
}

public function testDateIsRenderedIndependentOfSystemDefaultLocale()
{
\Locale::setDefault('de_DE');
$this->mockGridDateRendererBehaviorWithLocale('en_US');
$this->mockGridDateColumnConfig('period', 'day');

$objectMock = $this->getMockBuilder('Magento\Framework\Object')
->setMethods(['getData'])
->getMock();
$objectMock->expects($this->any())->method('getData')->willReturn('2014-06-25');

$this->assertEquals('Jun 25, 2014', $this->date->render($objectMock));
}
}

0 comments on commit a408079

Please sign in to comment.