Skip to content

Commit eda15ff

Browse files
sreichelkiatng
sreichel
and
kiatng
authored
Fixed incorrect datetime in block, ref #1525 (#4242)
* 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>
1 parent b0b850e commit eda15ff

File tree

6 files changed

+160
-39
lines changed

6 files changed

+160
-39
lines changed

‎.phpstan.dist.baseline.neon

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,11 +205,6 @@ parameters:
205205
count: 1
206206
path: app/code/core/Mage/Adminhtml/Block/Customer/Edit/Tab/Tags.php
207207

208-
-
209-
message: "#^Parameter \\#1 \\$date of method Mage_Core_Block_Abstract\\:\\:formatDate\\(\\) expects string\\|null, int\\<min, \\-1\\>\\|int\\<1, max\\> given\\.$#"
210-
count: 1
211-
path: app/code/core/Mage/Adminhtml/Block/Customer/Edit/Tab/View.php
212-
213208
-
214209
message: "#^Property Mage_Adminhtml_Block_Customer_Edit_Tab_View_Sales\\:\\:\\$_collection \\(Mage_Sales_Model_Entity_Sale_Collection\\) does not accept Varien_Data_Collection_Db\\.$#"
215210
count: 1

‎app/code/core/Mage/Adminhtml/Block/Customer/Edit/Tab/View.php

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function getCustomerLog()
7373
public function getCreateDate()
7474
{
7575
return ($date = $this->getCustomer()->getCreatedAt())
76-
? $this->formatDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true, false)
76+
? $this->formatTimezoneDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true, false)
7777
: null;
7878
}
7979

@@ -82,15 +82,17 @@ public function getCreateDate()
8282
*/
8383
public function getStoreCreateDate()
8484
{
85-
if (!$this->getCustomer()->getCreatedAt()) {
85+
$date = $this->getCustomer()->getCreatedAtTimestamp();
86+
if (!$date) {
8687
return null;
8788
}
88-
$date = Mage::app()->getLocale()->storeDate(
89-
$this->getCustomer()->getStoreId(),
90-
$this->getCustomer()->getCreatedAtTimestamp(),
91-
true
89+
90+
return $this->formatTimezoneDate(
91+
$date,
92+
Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM,
93+
true,
94+
false
9295
);
93-
return $this->formatDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true);
9496
}
9597

9698
public function getStoreCreateDateTimezone()
@@ -107,7 +109,7 @@ public function getStoreCreateDateTimezone()
107109
public function getLastLoginDate()
108110
{
109111
return ($date = $this->getCustomerLog()->getLoginAtTimestamp())
110-
? $this->formatDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true, false)
112+
? $this->formatTimezoneDate($date, Mage_Core_Model_Locale::FORMAT_TYPE_MEDIUM, true, false)
111113
: Mage::helper('customer')->__('Never');
112114
}
113115

‎app/code/core/Mage/Core/Block/Abstract.php

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,17 +1110,32 @@ public function helper($name)
11101110
/**
11111111
* Retrieve formatting date
11121112
*
1113-
* @param string $date
1114-
* @param string $format
1115-
* @param bool $showTime
1116-
* @param bool $useTimezone
1117-
* @return string
1113+
* @param string|int|Zend_Date|null $date
1114+
* @param string $format
1115+
* @param bool $showTime
1116+
* @return string
11181117
*/
1119-
public function formatDate($date = null, $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT, $showTime = false, $useTimezone = true)
1118+
public function formatDate($date = null, $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT, $showTime = false)
11201119
{
11211120
/** @var Mage_Core_Helper_Data $helper */
11221121
$helper = $this->helper('core');
1123-
return $helper->formatDate($date, $format, $showTime, $useTimezone);
1122+
return $helper->formatDate($date, $format, $showTime);
1123+
}
1124+
1125+
/**
1126+
* Retrieve formatting timezone date
1127+
*
1128+
* @param string|int|Zend_Date|null $date
1129+
*/
1130+
public function formatTimezoneDate(
1131+
$date = null,
1132+
string $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT,
1133+
bool $showTime = false,
1134+
bool $useTimezone = true
1135+
): string {
1136+
/** @var Mage_Core_Helper_Data $helper */
1137+
$helper = $this->helper('core');
1138+
return $helper->formatTimezoneDate($date, $format, $showTime, $useTimezone);
11241139
}
11251140

11261141
/**

‎app/code/core/Mage/Core/Helper/Data.php

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -146,33 +146,48 @@ public function formatPrice($price, $includeContainer = true)
146146
/**
147147
* Format date using current locale options and time zone.
148148
*
149-
* @param string|Zend_Date|null $date If empty, return current datetime.
150-
* @param string $format See Mage_Core_Model_Locale::FORMAT_TYPE_* constants
151-
* @param bool $showTime Whether to include time
152-
* @param bool $useTimezone Convert to local datetime?
149+
* @param string|int|Zend_Date|null $date If empty, return current datetime.
150+
* @param string $format See Mage_Core_Model_Locale::FORMAT_TYPE_* constants
151+
* @param bool $showTime Whether to include time
153152
* @return string
154153
*/
155-
public function formatDate($date = null, $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT, $showTime = false, $useTimezone = true)
154+
public function formatDate($date = null, $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT, $showTime = false)
156155
{
156+
return $this->formatTimezoneDate($date, $format, $showTime);
157+
}
158+
159+
/**
160+
* Format date using current locale options and time zone.
161+
*
162+
* @param string|int|Zend_Date|null $date If empty, return current locale datetime.
163+
* @param string $format See Mage_Core_Model_Locale::FORMAT_TYPE_* constants
164+
* @param bool $showTime Whether to include time
165+
* @param bool $useTimezone Convert to local datetime?
166+
*/
167+
public function formatTimezoneDate(
168+
$date = null,
169+
string $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT,
170+
bool $showTime = false,
171+
bool $useTimezone = true
172+
): string {
157173
if (!in_array($format, $this->_allowedFormats, true)) {
158174
return $date;
159175
}
176+
177+
$locale = Mage::app()->getLocale();
160178
if (empty($date)) {
161-
$date = Mage::app()->getLocale()->date(Mage::getSingleton('core/date')->gmtTimestamp(), null, null, $useTimezone);
179+
$date = $locale->date(Mage::getSingleton('core/date')->gmtTimestamp(), null, null, $useTimezone);
162180
} elseif (is_int($date)) {
163-
$date = Mage::app()->getLocale()->date($date, null, null, $useTimezone);
181+
$date = $locale->date($date, null, null, $useTimezone);
164182
} elseif (!$date instanceof Zend_Date) {
165183
if (($time = strtotime($date)) !== false) {
166-
$date = Mage::app()->getLocale()->date($time, null, null, $useTimezone);
184+
$date = $locale->date($time, null, null, $useTimezone);
167185
} else {
168186
return '';
169187
}
170188
}
171189

172-
$format = $showTime
173-
? Mage::app()->getLocale()->getDateTimeFormat($format)
174-
: Mage::app()->getLocale()->getDateFormat($format);
175-
190+
$format = $showTime ? $locale->getDateTimeFormat($format) : $locale->getDateFormat($format);
176191
return $date->toString($format);
177192
}
178193

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

208+
$locale = Mage::app()->getLocale();
193209
if (is_null($time)) {
194-
$date = Mage::app()->getLocale()->date(time());
210+
$date = $locale->date(time());
195211
} elseif ($time instanceof Zend_Date) {
196212
$date = $time;
197213
} else {
198-
$date = Mage::app()->getLocale()->date(strtotime($time));
214+
$date = $locale->date(strtotime($time));
199215
}
200216

201217
if ($showDate) {
202-
$format = Mage::app()->getLocale()->getDateTimeFormat($format);
218+
$format = $locale->getDateTimeFormat($format);
203219
} else {
204-
$format = Mage::app()->getLocale()->getTimeFormat($format);
220+
$format = $locale->getTimeFormat($format);
205221
}
206222

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

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

375392
// convert string from default database format (UTF-8)
376393
// to encoding which replacement arrays made with (ISO-8859-1)
394+
// phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged
377395
if ($s = @iconv('UTF-8', 'ISO-8859-1', $string)) {
378396
$string = $s;
379397
}
@@ -571,6 +589,7 @@ public function decorateArray($array, $prefix = 'decorated_', $forceSetAll = fal
571589
* @param mixed $value
572590
* @param bool $dontSkip
573591
*/
592+
// phpcs:ignore Ecg.PHP.PrivateClassMember.PrivateClassMemberError
574593
private function _decorateArrayObject($element, $key, $value, $dontSkip)
575594
{
576595
if ($dontSkip) {
@@ -616,6 +635,7 @@ public function assocToXml(array $array, $rootName = '_')
616635
* @return SimpleXMLElement
617636
* @throws Exception
618637
*/
638+
// phpcs:ignore Ecg.PHP.PrivateClassMember.PrivateClassMemberError
619639
private function _assocToXml(array $array, $rootName, SimpleXMLElement &$xml)
620640
{
621641
$hasNumericKey = false;
@@ -765,14 +785,18 @@ public function mergeFiles(
765785
// check whether merger is required
766786
$shouldMerge = $mustMerge || !$targetFile;
767787
if (!$shouldMerge) {
788+
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
768789
if (!file_exists($targetFile)) {
769790
$shouldMerge = true;
770791
} else {
792+
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
771793
$targetMtime = filemtime($targetFile);
772794
foreach ($srcFiles as $file) {
795+
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
773796
if (!file_exists($file)) {
774797
// no translation intentionally
775798
Mage::logException(new Exception(sprintf('File %s not found.', $file)));
799+
// phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged,Ecg.Security.ForbiddenFunction.Found
776800
} elseif (@filemtime($file) > $targetMtime) {
777801
$shouldMerge = true;
778802
break;
@@ -783,8 +807,10 @@ public function mergeFiles(
783807

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

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

810837
$data = '';
811838
foreach ($srcFiles as $file) {
839+
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
812840
if (!file_exists($file)) {
813841
continue;
814842
}
843+
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
815844
$contents = file_get_contents($file) . "\n";
816845
if ($beforeMergeCallback && is_callable($beforeMergeCallback)) {
846+
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
817847
$contents = call_user_func($beforeMergeCallback, $file, $contents);
818848
}
819849
$data .= $contents;
@@ -823,6 +853,7 @@ public function mergeFiles(
823853
throw new Exception(sprintf("No content found in files:\n%s", implode("\n", $srcFiles)));
824854
}
825855
if ($targetFile) {
856+
// phpcs:ignore Ecg.Security.ForbiddenFunction.Found
826857
file_put_contents($targetFile, $data, LOCK_EX);
827858
} else {
828859
return $data; // no need to write to file, just return data
@@ -878,6 +909,7 @@ public function getPublicFilesValidPath()
878909
public function checkLfiProtection($name)
879910
{
880911
if (preg_match('#\.\.[\\\/]#', $name)) {
912+
// phpcs:ignore Ecg.Classes.ObjectInstantiation.DirectInstantiation
881913
throw new Mage_Core_Exception($this->__('Requested file may not include parent directory traversal ("../", "..\\" notation)'));
882914
}
883915
return true;

‎app/design/adminhtml/default/default/template/customer/tab/view.phtml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@
99
* @category design
1010
* @package default_default
1111
* @copyright Copyright (c) 2006-2020 Magento, Inc. (https://www.magento.com)
12-
* @copyright Copyright (c) 2021-2022 The OpenMage Contributors (https://www.openmage.org)
12+
* @copyright Copyright (c) 2021-2024 The OpenMage Contributors (https://www.openmage.org)
1313
* @license https://opensource.org/licenses/afl-3.0.php Academic Free License (AFL 3.0)
1414
*/
15-
?>
16-
<?php
15+
1716
/**
1817
* Template for block Mage_Adminhtml_Block_Customer_Edit_Tab_View
18+
*
19+
* @see Mage_Adminhtml_Block_Customer_Edit_Tab_View
20+
* @var Mage_Adminhtml_Block_Customer_Edit_Tab_View $this
1921
*/
2022
?>
2123
<?php

‎tests/unit/Mage/Core/Helper/DataTest.php

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,14 @@
1717

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

20+
use Generator;
2021
use Mage;
2122
use Mage_Core_Helper_Data;
2223
use Mage_Core_Model_Encryption;
24+
use Mage_Core_Model_Locale;
2325
use PHPUnit\Framework\TestCase;
2426
use Varien_Crypt_Mcrypt;
27+
use Varien_Date;
2528

2629
class DataTest extends TestCase
2730
{
@@ -71,6 +74,78 @@ public function testValidateKey(): void
7174
$this->assertInstanceOf(Varien_Crypt_Mcrypt::class, $this->subject->validateKey('test'));
7275
}
7376

77+
/**
78+
* @dataProvider provideFormatTimezoneDate
79+
* @group Mage_Core
80+
* @group Mage_Core_Helper
81+
* @group Dates
82+
*/
83+
public function testFormatTimezoneDate(
84+
string $expectedResult,
85+
$data,
86+
string $format = Mage_Core_Model_Locale::FORMAT_TYPE_SHORT,
87+
bool $showTime = false,
88+
bool $useTimezone = true
89+
): void {
90+
$this->assertSame($expectedResult, $this->subject->formatTimezoneDate($data, $format, $showTime, $useTimezone));
91+
}
92+
93+
public function provideFormatTimezoneDate(): Generator
94+
{
95+
$date = date_create()->getTimestamp();
96+
$dateShort = date('m/j/Y', $date);
97+
$dateLong = date('F j, Y', $date);
98+
$dateShortTime = date('m/j/Y g:i A', $date);
99+
100+
yield 'null' => [
101+
$dateShort,
102+
null
103+
];
104+
yield 'empty date' => [
105+
$dateShort,
106+
''
107+
];
108+
yield 'string date' => [
109+
$dateShort,
110+
'now'
111+
];
112+
yield 'numeric date' => [
113+
$dateShort,
114+
'0'
115+
];
116+
yield 'invalid date' => [
117+
'',
118+
'invalid'
119+
];
120+
yield 'invalid format' => [
121+
(string)$date,
122+
$date,
123+
'invalid',
124+
];
125+
yield 'date short' => [
126+
$dateShort,
127+
$date
128+
];
129+
yield 'date long' => [
130+
$dateLong,
131+
$date,
132+
'long'
133+
];
134+
// yield 'date short w/ time and w/ timezone' => [
135+
// $dateShortTime,
136+
// $date,
137+
// 'short',
138+
// true
139+
// ];
140+
yield 'date short w/ time' => [
141+
$dateShortTime,
142+
$date,
143+
'short',
144+
true,
145+
false,
146+
];
147+
}
148+
74149
/**
75150
* @group Mage_Core
76151
* @group Mage_Core_Helper

0 commit comments

Comments
 (0)