Sales & Product Reports - big problem with dates causing incorrect & inaccurate data

It might be a timezone thing - can you check to see if the orders for a given "day" all occur within 24 hours of each other, even though they cross the day boundary? The time calculation might be done based on GMT, or on the timezone of your server, rather than the Drupal site timezone.

Related to #441758: Sales Reports timeline offset ?

Yeah, I'm still inclined to think it is a timezone problem - the code will only use the Drupal timezone if the Drupal API is used to get the date, but if the PHP date functions are used directly then the code might be using the server timezone, GMT, or (ugh) a combination depending on which functions were used where. To answer your one question, no I don't think it would show *all* orders from 3/13 if it were a timezone problem, it might show all orders from 3/14 GMT which is partially in 3/13 and partially in 3/14 in your timezone. 3/13@6pm + 8 hours is 3/14@2am GMT, which is close enough to when you *expect* the report to start (midnight) to strongly hint that the reports are using GMT for their calculations (with possible daylight savings time issues (+-1) and a possible fence post error (+-1) making it off by two).

I'm afraid that tracking this down is going to require a complete audit of the uc_reports code to see what date functions are being used, as well as a look at the uc_order code to see what order dates (GMT? server timezone? site timezone? customer timezone?) are being entered into the database. I'm not going to be able to do that in the next week.

Maybe someone else who's using reports can work on this? It would be awesome to get a SimpleTest written which created lots of orders then compared the report totals with what was expected. That would be an easy way to get to the bottom of this issue and also to make sure we don't have these kinds of reporting issues in the future.

I tested the reports for the Ubercart 2.2 version.

I observed that daily reports and custom reports are generated correctly and the custom reports are generated accordingly to the dates entered.

I even tried changing the reporting settings -> Reported statuses and the reports are generated accordingly.

The daylight savings time is not updated in my site. So the order placed on March 19th after 12:00 AM should fall under as oder placed on 19th but as the DST timings not updated the order is showed as placed on 18th 11:00 PM.

So the orders belonging to today and yesterday are shown when daily report is generated. So can you once look into the order timings by placing a sample order.

Thanks for that example.

My brain hurts from looking at the uc_reports code, and I have many unkind words for the person who wrote it. Thanks to your specific information in #8, I found and fixed some errors in the way the uc_reports calculates time codes. The attached patch fixes these errors for the sales summary reports at admin/store/reports/sales. The same errors are in the Yearly reports at admin/store/reports/sales/year and Custom reports at admin/store/reports/sales/custom. I am going to hold off on changing those other reports until I get confirmation from you that my patch fixes your problem.

Please do a thorough job of testing this - I don't want to commit the changes unless I'm sure the problem is fixed.

Tagging

subscribing

That patch don't work correctly with "Yesterday", please patch "function _uc_reports_get_sales($time, $period = 'day')" at uc_reports.module

function _uc_reports_get_sales($time, $period = 'day') {
  $timezone = _uc_reports_timezone_offset(); //add the timezone variable.

  // Get the current date markers.
  $date = array(
    //'day' => format_date($time, 'custom', 'j', 0),
    //'month' => format_date($time, 'custom', 'n', 0),
    //'year' => format_date($time, 'custom', 'Y', 0),
    'day' => format_date($time, 'custom', 'j', $timezone),
    'month' => format_date($time, 'custom', 'n', $timezone),
    'year' => format_date($time, 'custom', 'Y', $timezone),
  );

  // Add one to the granularity chosen, and use it to calc the new time.
  $date[$period] += 1;
  //$new_time = gmmktime(0, 0, 0, $date['month'], $date['day'], $date['year']); 
  $new_time = gmmktime(0, 0, 0, $date['month'], $date['day'], $date['year']) - $timezone;

  // Set up the default SQL for getting orders with the proper status
  // within this period.
  $order_statuses = _uc_reports_order_statuses();
  $sql_frag = " FROM {uc_orders} as o WHERE o.order_status IN $order_statuses AND created >= $time and created < $new_time";

  // Get the total value of the orders.
  $output = array('income' => 0);
  $orders = db_query("SELECT o.order_total ". $sql_frag);
  while ($order = db_fetch_object($orders)) {
    $output['income'] += $order->order_total;
  }

  // Get the total amount of orders.
  $count = db_result(db_query("SELECT COUNT(o.order_total) ". $sql_frag));
  $output['total'] = $count;

  // Average for this period.
  $output['average'] = ($count != 0) ? round($output['income'] / $count, 2) : 0;

  return $output;
}

The patch for uc_reports module with version is 2.4.

subscribe

Subscribe.

Having the same problem as #8.

@mineshaftgap and msumme: please try testing the patch in #18 and see if that fixes the problems for you.

The patch in 18 fixes the daily reports (e.g. admin/store/orders/search/results/0/0/0/0/0/0/1292302800/1292389199), but does not fix the reports overview (admin/store/reports/sales).

Have a problem with uc_reports module don't work correctly with drupal's time zone. We need patch in 2 files (with function):

1. uc_reports.admin.inc

uc_reports_sales_summary();
uc_reports_sales_year();
uc_reports_sales_custom();
uc_reports_sales_custom_form();
uc_reports_sales_custom_form_submit();

2. uc_reports.module

_uc_reports_get_sales()

The patch in 18 work correctly with my site:

1. http://www.mysite.com/admin/store/reports/sales - Sales summary
2. http://www.mysite.com/admin/store/reports/sales/year - Sales per year
2. http://www.mysite.com/admin/store/reports/sales/custom -Custom sales summary

Hi, @aaronbauman,

Please post screenshots attachment about that.

Patch #18 also solves it for me. Dumped the start/end dates from each screen and they now reflect the site timezone.

Turns out the problem wasn't completely fixed because the end dates were being messed up so that they were also off by the timezone difference. Attached patch just adds to the patch from #18 a fix for end dates.

I think it looks good, but I can't find a good site with meaningful reports to test it on.

+++ sites/all/modules/ubercart/uc_reports/uc_reports.module	2010-10-26 21:41:17.269540001 +0700
@@ -387,22 +387,5 @@ function _uc_reports_end_interval($time, $interval = 'month') {
-  $temp = strtotime('+1 '. $interval, $time) - 1;
-
-  switch ($interval) {
-    case 'day':
-      $temp = gmmktime(0, 0, -1, gmdate('n', $time), gmdate('j', $time) + 1, gmdate('Y', $time));
-      break;
-    case 'week':
-      $temp = gmmktime(0, 0, -1, gmdate('n', $time), gmdate('j', $time) + 7, gmdate('Y', $time));
-      break;
-    case 'month':
-      $temp = gmmktime(23, 59, 59, gmdate('n', $time) + 1, 0, gmdate('Y', $time));
-      break;
-    case 'year':
-      $temp = gmmktime(23, 59, 59, gmdate('n', $time), 0, gmdate('Y', $time) + 1);
-      break;
-  }
-

Why this is being removed?

Not that I fully understand this to properly review, but would like to understand the removal.

Powered by Dreditor.

#27 was not being applied properly to 2.4, and even more I had to do -p5 to patch it.

Applied is #27 re-rolled with git and with the manually application of what was failing, which basically is what I say on #30.

I tested it and at least the year summary which is what I am trying looks good.

Spoke to soon, it's not working great. Year summary is display results shifted, or at least, month column does not look right. 2010 starts with dec 2009 and end with nov 2010.

Attach is a patch with a slightly modification to #31, which was using format_date() as well on the month column of the year summary.

Also did the same approach to uc_tax_reports module.

This was kind of a blind modification, as I kind of followed what was done but not 100%, but I did test it and tax reports on my site now are coherent to custom reports I am building from the database (outside this modules) in year summary and tax reports.

Removing an empty line from the patch.

Test bot doesn't run properly on fixed point releases like 6.x-2.4. It also doesn't work properly when module dependencies are needed, as they always are for Ubercart tests. Please run the tests locally to verify that your patch doesn't break the tests.

How do we run the tests locally? Is there a way to avoid the testbot process a patch? Does this mean I need roll a patch for dev. I did try it on dev and it applies, with offsets, fine.

There's no way to avoid the testbot, nor do we want to - we want to get the testbot *fixed* so it will be more useful. It does catch some things right now, but it's of limited use until these problems get fixed. However, I think it's vital that we test all patches against the tests that we currently have in place, especially when the patch is has lots of potential consequences like this one.

To run the tests on your own site, install the SimpleTest module then go to admin/build/testing to run the Ubercart tests.

Yes, please roll the patch for -dev, as that's where it needs to be applied. This is how core Drupal works too. It would also be nice to have some *new* tests accompany any large patch like this one, to make sure this issue doesn't arise again and to verify that this patch does what it should. We're not requiring that, but it sure would be nice to have.

Well, tests for this definitely for another chapter. I have to go over some literature on how to create proper unit tests for this, but I am not really sure how to test this without doing a lot of processing (creating an order, saving it, querying the report, checking output, etc.) I guess I do need to read it through first.

Attached patch for dev. It failed 8 tests, but they are likely because i did it on a copy of my production site, which is heavily configured and modified. Mostly the fails were around text not found and stuff like that. I run the tests before applying the patch and after applying the patch. Same 8 tests failed, all other OK.

Subscribing. Our reports have been off, and I'm trying to get everything rectified. We have a TON of orders, so I'm surprised Lyle didn't ask us to test. I'd gladly have given him the 'keys to the castle' if it meant having accurate sales reports.

Committed #40. Looks alright to me, and it has to be better than what we have now. If this is still broken, please provide examples.

Sorry, this doesn't appear to be fixed on my end... I had a client on 2.7 report that there was a duplicated day in their reports. It turned out that it was Daylight Savings, and that the custom sales report was outputting two subsequent days as the same day in the custom sales report, as well as when building the intervals it's adding a full extra day to the end. So if I ask for 3/10/2012 - 3/12/2012 with a daily breakdown it's returning reports for the 10th, the 11th, the 12th, and the 13th.

One thing I noticed is that in uc_reports_sales_custom_form_submit the start and end dates are being offset based on the site time zone, but then they are getting offset again when output:

$date = format_date($subreport['start'], 'custom', $format .' - D', $timezone);

When I zeroed the $timezone value: format_date($subreport['start'], 'custom', $format .' - D', 0), the dates output as expected. However, _uc_reports_subreport_intervals is still returning one greater interval (one extra day) than it should, so even though my range ended on the 12th it still shows sales for the 13th. This seems to be because _uc_reports_subreport_intervals is generating one extra interval when the range provided straddles daylight savings.

Status change.

Let's deal with this remaining problem in the issue that's currently open: #1373910: Date ranges on custom reports is not from the beginning of the month
Please add your comments to that thread.