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

Fixed bug with backend dashboard about graph chart months calculations #3594

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

fballiano
Copy link
Contributor

Seems like Zend_Date has some bug calculating time differences when using timezones, for this reason the backend dashboard graph, when selecting 1Y or 2Y 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

@Hanmac
Copy link
Contributor

Hanmac commented Oct 12, 2023

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);
        }

@fballiano
Copy link
Contributor Author

I preferred not to change the variables names, did you have the chance to test this PR?

@elidrissidev
Copy link
Member

@fballiano Can you try setting your timezone in configuration to Etc/GMT-12, cause for me it's still skipping some months.

download

@fballiano
Copy link
Contributor Author

@kiatng mmm interesting, since your TZ is -12, the first generated day becomes 2022-12-31 12:00:00 instead of 2023-01-01 so everything gets messed up.

I think that for 1Y and 2Y we shouldn't use the timezone at all, what do you think?

@elidrissidev
Copy link
Member

@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 {

@fballiano
Copy link
Contributor Author

but I think timezone is needed for the hourly/24h reports, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard Graph is skipping Months in 1y view
6 participants