-
-
Notifications
You must be signed in to change notification settings - Fork 439
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
Fixed bug with backend dashboard about graph chart months calculations #3594
Conversation
i did something similar like, because i was using the $jsFormat for graph js switch ($helper->getParam('period')) {
case '24h':
$dateTimeFormat = 'Y-m-d H:00';
$interval = DateInterval::createFromDateString('1 hour');
$jsFormat = "YYYY-MM-DD HH:00";
$jsPeriod = "hour";
break;
case '7d':
case '1m':
$dateTimeFormat = 'Y-m-d';
$interval = DateInterval::createFromDateString('1 day');
$jsFormat = "YYYY-MM-DD";
$jsPeriod = 'day';
break;
case '1y':
case '2y':
$dateTimeFormat = 'Y-m';
$interval = DateInterval::createFromDateString('1 month');
$jsFormat = "YYYY-MM";
$jsPeriod = 'month';
break;
}
while($dateTimeStart < $dateTimeEnd){
$d = $dateTimeStart->format($dateTimeFormat);
if (isset($values[$d])) {
$result[] = $values[$d];
} else {
$result[] = [
'revenue' => '0.00000000',
'quantity' => '0',
'range' => $d
];
}
$dateTimeStart = $dateTimeStart->add($interval);
} |
I preferred not to change the variables names, did you have the chance to test this PR? |
@fballiano Can you try setting your timezone in configuration to Etc/GMT-12, cause for me it's still skipping some months. |
@kiatng mmm interesting, since your TZ is I think that for |
@fballiano This patch works fine for me diff --git a/app/code/core/Mage/Adminhtml/Block/Dashboard/Graph.php b/app/code/core/Mage/Adminhtml/Block/Dashboard/Graph.php
index 700d3b3b17..155ce525cf 100644
--- a/app/code/core/Mage/Adminhtml/Block/Dashboard/Graph.php
+++ b/app/code/core/Mage/Adminhtml/Block/Dashboard/Graph.php
@@ -187,14 +187,9 @@ class Mage_Adminhtml_Block_Dashboard_Graph extends Mage_Adminhtml_Block_Dashboar
$this->setAxisLabels($axis, $this->getRowsData($attr, true));
}
- $timezoneLocal = Mage::app()->getStore()->getConfig(Mage_Core_Model_Locale::XML_PATH_DEFAULT_TIMEZONE);
-
list($dateStart, $dateEnd) = Mage::getResourceModel('reports/order_collection')
->getDateRange($this->getDataHelper()->getParam('period'), '', '', true);
- $dateStart->setTimezone($timezoneLocal);
- $dateEnd->setTimezone($timezoneLocal);
-
$dates = [];
$datas = [];
diff --git a/app/code/core/Mage/Reports/Model/Resource/Order/Collection.php b/app/code/core/Mage/Reports/Model/Resource/Order/Collection.php
index 19a497d3e0..c0f41d28c3 100644
--- a/app/code/core/Mage/Reports/Model/Resource/Order/Collection.php
+++ b/app/code/core/Mage/Reports/Model/Resource/Order/Collection.php
@@ -316,6 +316,9 @@ class Mage_Reports_Model_Resource_Order_Collection extends Mage_Sales_Model_Reso
$dateEnd = Mage::app()->getLocale()->date();
$dateStart = clone $dateEnd;
+ $dateStart->setTimezone('Etc/UTC');
+ $dateEnd->setTimezone('Etc/UTC');
+
// go to the end of a day
$dateEnd->setHour(23);
$dateEnd->setMinute(59);
@@ -361,9 +364,6 @@ class Mage_Reports_Model_Resource_Order_Collection extends Mage_Sales_Model_Reso
break;
}
- $dateStart->setTimezone('Etc/UTC');
- $dateEnd->setTimezone('Etc/UTC');
-
if ($returnObjects) {
return [$dateStart, $dateEnd];
} else {
|
but I think timezone is needed for the hourly/24h reports, no? |
@fballiano are you going to revive this issue? |
@Hanmac I left openmage as of today, I will maybe write a statement in the next days. |
@sreichel should we try to revive this MR? |
@Hanmac yes, we should. What about #4398 (comment)? It would help to write 1-2 tests first to reproduce the problem. Think i have already started it here https://github.com/OpenMage/magento-lts/pull/4454/files#diff-eb2f96cabcfb9a6ec74208eea3cbdc399c44d07f3c39d6257e97a68b092a6c2b |
i don't trust the idea of adding YetAnotherDateTime library PHP Datetime + Intl has gotten good enough So i don't see a good reason why we need another lib? |
Seems like Zend_Date has some bug calculating time differences when using timezones, for this reason the backend dashboard graph, when selecting
1Y
or2Y
miscalculates the months to show and for this reason the graph is missing data.More detailed explanation in bug report #3593
This PR rewrites the whole thing using
DateTime
, which seems more correct to me.Fixes #3593