Skip to content

Commit

Permalink
Fixed incorrect datetime in block, ref #1525 (#4242)
Browse files Browse the repository at this point in the history 
* backport, ref #1525 #2940

* test coverage for formatTimezoneDate() 100%

* Minor change

* phpcs

* Minor fix

* rector

* Fixed tests

* phpstan l5 fix

* Fixed test .... hour w/o leading zero

---------

Co-authored-by: Ng Kiat Siong <kiatsiong.ng@gmail.com>
  • Loading branch information
@sreichel @kiatng
sreichel and kiatng authored Oct 15, 2024
1 parent b0b850e commit eda15ff
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 39 deletions.
5 changes: 0 additions & 5 deletions .phpstan.dist.baseline.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,6 @@ parameters:
count: 1
path: app/code/core/Mage/Adminhtml/Block/Customer/Edit/Tab/Tags.php

-
message: "#^Parameter \\#1 \\$date of method Mage_Core_Block_Abstract\\:\\:formatDate\\(\\) expects string\\|null, int\\<min, \\-1\\>\\|int\\<1, max\\> given\\.$#"
count: 1
path: app/code/core/Mage/Adminhtml/Block/Customer/Edit/Tab/View.php

-
message: "#^Property Mage_Adminhtml_Block_Customer_Edit_Tab_View_Sales\\:\\:\\$_collection \\(Mage_Sales_Model_Entity_Sale_Collection\\) does not accept Varien_Data_Collection_Db\\.$#"
count: 1
Expand Down
18 changes: 10 additions & 8 deletions app/code/core/Mage/Adminhtml/Block/Customer/Edit/Tab/View.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function getCustomerLog()
public function getCreateDate()
{
return ($date = $this->getCustomer()->getCreatedAt())
? $this->formatDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true, false)
? $this->formatTimezoneDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true, false)
: null;
}

Expand All @@ -82,15 +82,17 @@ public function getCreateDate()
*/
public function getStoreCreateDate()
{
if (!$this->getCustomer()->getCreatedAt()) {
$date = $this->getCustomer()->getCreatedAtTimestamp();
if (!$date) {
return null;
}
$date = Mage::app()->getLocale()->storeDate(
$this->getCustomer()->getStoreId(),
$this->getCustomer()->getCreatedAtTimestamp(),
true

return $this->formatTimezoneDate(
$date,
Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM,
true,
false
);
return $this->formatDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true);
}

public function getStoreCreateDateTimezone()
Expand All @@ -107,7 +109,7 @@ public function getStoreCreateDateTimezone()
public function getLastLoginDate()
{
return ($date = $this->getCustomerLog()->getLoginAtTimestamp())
? $this->formatDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true, false)
? $this->formatTimezoneDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true, false)
: Mage::helper('customer')->__('Never');
}

Expand Down
29 changes: 22 additions & 7 deletions app/code/core/Mage/Core/Block/Abstract.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1110,17 +1110,32 @@ public function helper($name)
/**
* Retrieve formatting date
*
* @param string $date
* @param string $format
* @param bool $showTime
* @param bool $useTimezone
* @return string
* @param string|int|Zend_Date|null $date
* @param string $format
* @param bool $showTime
* @return string
*/
public function formatDate($date = null, $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT, $showTime = false, $useTimezone = true)
public function formatDate($date = null, $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT, $showTime = false)
{
/** @var Mage_Core_Helper_Data $helper */
$helper = $this->helper('core');
return $helper->formatDate($date, $format, $showTime, $useTimezone);
return $helper->formatDate($date, $format, $showTime);
}

/**
* Retrieve formatting timezone date
*
* @param string|int|Zend_Date|null $date
*/
public function formatTimezoneDate(
$date = null,
string $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT,
bool $showTime = false,
bool $useTimezone = true
): string {
/** @var Mage_Core_Helper_Data $helper */
$helper = $this->helper('core');
return $helper->formatTimezoneDate($date, $format, $showTime, $useTimezone);
}

/**
Expand Down
64 changes: 48 additions & 16 deletions app/code/core/Mage/Core/Helper/Data.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,33 +146,48 @@ public function formatPrice($price, $includeContainer = true)
/**
* Format date using current locale options and time zone.
*
* @param string|Zend_Date|null $date If empty, return current datetime.
* @param string $format See Mage_Core_Model_Locale::FORMAT_TYPE_* constants
* @param bool $showTime Whether to include time
* @param bool $useTimezone Convert to local datetime?
* @param string|int|Zend_Date|null $date If empty, return current datetime.
* @param string $format See Mage_Core_Model_Locale::FORMAT_TYPE_* constants
* @param bool $showTime Whether to include time
* @return string
*/
public function formatDate($date = null, $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT, $showTime = false, $useTimezone = true)
public function formatDate($date = null, $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT, $showTime = false)
{
return $this->formatTimezoneDate($date, $format, $showTime);
}

/**
* Format date using current locale options and time zone.
*
* @param string|int|Zend_Date|null $date If empty, return current locale datetime.
* @param string $format See Mage_Core_Model_Locale::FORMAT_TYPE_* constants
* @param bool $showTime Whether to include time
* @param bool $useTimezone Convert to local datetime?
*/
public function formatTimezoneDate(
$date = null,
string $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT,
bool $showTime = false,
bool $useTimezone = true
): string {
if (!in_array($format, $this->_allowedFormats, true)) {
return $date;
}

$locale = Mage::app()->getLocale();
if (empty($date)) {
$date = Mage::app()->getLocale()->date(Mage::getSingleton('core/date')->gmtTimestamp(), null, null, $useTimezone);
$date = $locale->date(Mage::getSingleton('core/date')->gmtTimestamp(), null, null, $useTimezone);
} elseif (is_int($date)) {
$date = Mage::app()->getLocale()->date($date, null, null, $useTimezone);
$date = $locale->date($date, null, null, $useTimezone);
} elseif (!$date instanceof Zend_Date) {
if (($time = strtotime($date)) !== false) {
$date = Mage::app()->getLocale()->date($time, null, null, $useTimezone);
$date = $locale->date($time, null, null, $useTimezone);
} else {
return '';
}
}

$format = $showTime
? Mage::app()->getLocale()->getDateTimeFormat($format)
: Mage::app()->getLocale()->getDateFormat($format);

$format = $showTime ? $locale->getDateTimeFormat($format) : $locale->getDateFormat($format);
return $date->toString($format);
}

Expand All @@ -190,18 +205,19 @@ public function formatTime($time = null, $format = Mage_Core_Model_Locale::FORMA
return $time;
}

$locale = Mage::app()->getLocale();
if (is_null($time)) {
$date = Mage::app()->getLocale()->date(time());
$date = $locale->date(time());
} elseif ($time instanceof Zend_Date) {
$date = $time;
} else {
$date = Mage::app()->getLocale()->date(strtotime($time));
$date = $locale->date(strtotime($time));
}

if ($showDate) {
$format = Mage::app()->getLocale()->getDateTimeFormat($format);
$format = $locale->getDateTimeFormat($format);
} else {
$format = Mage::app()->getLocale()->getTimeFormat($format);
$format = $locale->getTimeFormat($format);
}

return $date->toString($format);
Expand Down Expand Up @@ -368,12 +384,14 @@ public function removeAccents($string, $german = false)

$replacements[$german] = [];
foreach ($subst as $k => $v) {
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
$replacements[$german][$k < 256 ? chr($k) : '&#' . $k . ';'] = $v;
}
}

// convert string from default database format (UTF-8)
// to encoding which replacement arrays made with (ISO-8859-1)
// phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged
if ($s = @iconv('UTF-8', 'ISO-8859-1', $string)) {
$string = $s;
}
Expand Down Expand Up @@ -571,6 +589,7 @@ public function decorateArray($array, $prefix = 'decorated_', $forceSetAll = fal
* @param mixed $value
* @param bool $dontSkip
*/
// phpcs:ignore Ecg.PHP.PrivateClassMember.PrivateClassMemberError
private function _decorateArrayObject($element, $key, $value, $dontSkip)
{
if ($dontSkip) {
Expand Down Expand Up @@ -616,6 +635,7 @@ public function assocToXml(array $array, $rootName = '_')
* @return SimpleXMLElement
* @throws Exception
*/
// phpcs:ignore Ecg.PHP.PrivateClassMember.PrivateClassMemberError
private function _assocToXml(array $array, $rootName, SimpleXMLElement &$xml)
{
$hasNumericKey = false;
Expand Down Expand Up @@ -765,14 +785,18 @@ public function mergeFiles(
// check whether merger is required
$shouldMerge = $mustMerge || !$targetFile;
if (!$shouldMerge) {
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
if (!file_exists($targetFile)) {
$shouldMerge = true;
} else {
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
$targetMtime = filemtime($targetFile);
foreach ($srcFiles as $file) {
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
if (!file_exists($file)) {
// no translation intentionally
Mage::logException(new Exception(sprintf('File %s not found.', $file)));
// phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged,Ecg.Security.ForbiddenFunction.Found
} elseif (@filemtime($file) > $targetMtime) {
$shouldMerge = true;
break;
Expand All @@ -783,8 +807,10 @@ public function mergeFiles(

// merge contents into the file
if ($shouldMerge) {
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
if ($targetFile && !is_writable(dirname($targetFile))) {
// no translation intentionally
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
throw new Exception(sprintf('Path %s is not writeable.', dirname($targetFile)));
}

Expand All @@ -795,6 +821,7 @@ public function mergeFiles(
}
if (!empty($srcFiles)) {
foreach ($srcFiles as $key => $file) {
// phpcs:ignore Ecg.Security.DiscouragedFunction.Discouraged
$fileExt = strtolower(pathinfo($file, PATHINFO_EXTENSION));
if (!in_array($fileExt, $extensionsFilter)) {
unset($srcFiles[$key]);
Expand All @@ -809,11 +836,14 @@ public function mergeFiles(

$data = '';
foreach ($srcFiles as $file) {
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
if (!file_exists($file)) {
continue;
}
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
$contents = file_get_contents($file) . "\n";
if ($beforeMergeCallback && is_callable($beforeMergeCallback)) {
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
$contents = call_user_func($beforeMergeCallback, $file, $contents);
}
$data .= $contents;
Expand All @@ -823,6 +853,7 @@ public function mergeFiles(
throw new Exception(sprintf("No content found in files:\n%s", implode("\n", $srcFiles)));
}
if ($targetFile) {
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
file_put_contents($targetFile, $data, LOCK_EX);
} else {
return $data; // no need to write to file, just return data
Expand Down Expand Up @@ -878,6 +909,7 @@ public function getPublicFilesValidPath()
public function checkLfiProtection($name)
{
if (preg_match('#\.\.[\\\/]#', $name)) {
// phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
throw new Mage_Core_Exception($this->__('Requested file may not include parent directory traversal ("../", "..\\" notation)'));
}
return true;
Expand Down
8 changes: 5 additions & 3 deletions app/design/adminhtml/default/default/template/customer/tab/view.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@
* @category design
* @package default_default
* @copyright Copyright (c) 2006-2020 Magento, Inc. (https://www.magento.com)
* @copyright Copyright (c) 2021-2022 The OpenMage Contributors (https://www.openmage.org)
* @copyright Copyright (c) 2021-2024 The OpenMage Contributors (https://www.openmage.org)
* @license https://opensource.org/licenses/afl-3.0.php Academic Free License (AFL 3.0)
*/
?>
<?php

/**
* Template for block Mage_Adminhtml_Block_Customer_Edit_Tab_View
*
* @see Mage_Adminhtml_Block_Customer_Edit_Tab_View
* @var Mage_Adminhtml_Block_Customer_Edit_Tab_View $this
*/
?>
<?php
Expand Down
75 changes: 75 additions & 0 deletions tests/unit/Mage/Core/Helper/DataTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@

namespace OpenMage\Tests\Unit\Mage\Core\Helper;

use Generator;
use Mage;
use Mage_Core_Helper_Data;
use Mage_Core_Model_Encryption;
use Mage_Core_Model_Locale;
use PHPUnit\Framework\TestCase;
use Varien_Crypt_Mcrypt;
use Varien_Date;

class DataTest extends TestCase
{
Expand Down Expand Up @@ -71,6 +74,78 @@ public function testValidateKey(): void
$this->assertInstanceOf(Varien_Crypt_Mcrypt::class, $this->subject->validateKey('test'));
}

/**
* @dataProvider provideFormatTimezoneDate
* @group Mage_Core
* @group Mage_Core_Helper
* @group Dates
*/
public function testFormatTimezoneDate(
string $expectedResult,
$data,
string $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT,
bool $showTime = false,
bool $useTimezone = true
): void {
$this->assertSame($expectedResult, $this->subject->formatTimezoneDate($data, $format, $showTime, $useTimezone));
}

public function provideFormatTimezoneDate(): Generator
{
$date = date_create()->getTimestamp();
$dateShort = date('m/j/Y', $date);
$dateLong = date('F j, Y', $date);
$dateShortTime = date('m/j/Y g:i A', $date);

yield 'null' => [
$dateShort,
null
];
yield 'empty date' => [
$dateShort,
''
];
yield 'string date' => [
$dateShort,
'now'
];
yield 'numeric date' => [
$dateShort,
'0'
];
yield 'invalid date' => [
'',
'invalid'
];
yield 'invalid format' => [
(string)$date,
$date,
'invalid',
];
yield 'date short' => [
$dateShort,
$date
];
yield 'date long' => [
$dateLong,
$date,
'long'
];
// yield 'date short w/ time and w/ timezone' => [
// $dateShortTime,
// $date,
// 'short',
// true
// ];
yield 'date short w/ time' => [
$dateShortTime,
$date,
'short',
true,
false,
];
}

/**
* @group Mage_Core
* @group Mage_Core_Helper
Expand Down

0 comments on commit eda15ff

Please sign in to comment.