Timezone causes off-by-one error in reports (use Drupal time instead of server's)

Interesting. Thanks for the heads up...

What's more, the timezone offset was being subtracted from the GMT time, but here I'm -6 hours in Central time, so Drupal sends back -21600. The value should be added instead.

This patch should fix both of those problems now.

Also, I filed another issue (with a patch) here dealing with the user timezone issue.

If the code looks good, It can easily be ported back to Ubercart 1.x, but for now I will adjust the issue to 2.x-dev.

There's something not quite right, but I haven't figured out what it is yet. My timezone (both for user and site, so the other issue doesn't apply) is set to -5 GMT. When I pick a start date of Jan 1, 2008, the URL has 1199127600 as the start time. This is Dec 31, 2007 at 19:00 +0 GMT, which is equal to Dec 31, 2007 at 14:00 -5 GMT. That's not what I asked for.

I get the feeling that we're going about this the wrong way. It may only be that we need to use gmdate() instead of date(). date() relies on the server's timezone setting, which may have nothing to do with Drupal's setting.

// Timelapse while work happens.

I believe the offset was being subtracted because it's moving time back to GMT. You add the offset to GMT to get local time, so you subtract it from local time to get GMT. Using gmdate() seems to fix things, so I've rerolled the patch against HEAD. Test things out to make sure there aren't parts that need to be taken care of.

Your code still has the edge case that was originally posited... that is, try putting this code at the beginning of uc_reports_sales summary:

$time = gmmktime(4, 59, 0, 12, 1, 2008);
$timezone_offset = $time - _uc_reports_timezone_offset();

You'll see that it won't pick November, it'll pick December, even though with your actual user offset of 5 hours, that should still be November. The offset needs to be added here. I don't know if date/gmdate has any different behavior when supplying a timestamp, but even if it does, using it alone doesn't address the edge cases.

That's why I added the timestamp in places, I still have to review the patch you provided and check for places where this problem exists.

Here, try this one out.

Hmm. #3 fixes the "Sales per year" page, but #5 fixes the "Custom sales summary". The site I'm testing on doesn't really have enough orders on it for me to see if the other tab is affected very much.

New title because I had a hard time finding this issue again.

My latest patch. Don't expect it to work alone... There's another issue fixed over at http://drupal.org/node/351151 which is needed for everything to actually work correctly. Some of the errors here were red herrings... some problems were in _uc_reports_get_sales() as well.

Since #351151: _uc_reports_get_sales() is borked. is entangled with this issue, and I found some changes to make to it, I've added it in with this patch. format_date() lets you set the timezone while still translating the names of the day and month, so setting the timezone to 0 will prevent Drupal's setting from messing up the calculations we're trying to make.

The custom report form's default value was always behind a day, so this patch should fix it.

Just wanted to say, looks good to me. =)

Applied the patch and ran into the following problems:

1. PHP warning on update b/c the update function didn't return an array.
2. Was using GMT - 5 timezone. Noticed the sales summary thinks "Today" is January 1, 2009 even though it's December 31, 2008. Checked the server time to make sure it was right... it was. date('m/d/y') prints out 12/31/08. However, even though the sales summary was off, the "Sales per year" report still showed the 2008 breakdown.
3. So... on the "Sales per year" report, it says I had 1 completed order for Nov. and 2 for Dec. The default "Custom sales summary" says I had 3 for Dec. This is on the 2.x Livetest if you need to see it. When I look at the orders, there are in fact 3.

Alright, there was a few minus offsets that need to be plus. I fixed them up...

Also, I got the upgrade path fixed up, using update_sql (why didn't I know about this function before? heheh)

(Good thing we're all reviewing this..)

On the custom sales summary, the start time fields show Jan 1, 2008, like they should, but if the form is submitted, it's turned into Dec. 31, 2007 by the time the page reloads. Even though the form now shows Dec 31, submitting the form again causes the page to start from Dec 30.

Furthermore, the Sales summary doesn't show the order I just made today, even though I set it to completed. The other two pages show it correctly.

The timezones were erroneously modifying the result of the submitted form, they should be taken at face value.

Also, in the new get_sales, we had that same issue, modifying timestamps that were already modified. The solution was to pass 0 to the offset of format_date().

New patch for testing:

I'm glad this issue is over. Good job.

cYu,

Try this patch...

Dangit, UberBot, who let you out of your cage?!

Should we get this in 1.x or consider it fixed?

I'd say if this is indeed a bug on 1.x we should backport. Would this also affect the custom product report?

Hmm... it looks like the patch got messed up between 2.x and 1.x. Yeah, it should affect the custom report, I'm working on it now... needs some hand loving ...

Alrighty, got that goin on... Also, I threw in fixes to the new custom product report that got put in, plus got these #359778: Postgres 8.3 - sales reports syntax error fixes in while I was at it =).

So... dunno how to describe it, but I applied this patch and the custom reports no longer work. They return empty reports now.

I think it might be partially related to the fact that the dates were reverted to November 1999 after I submitted the forms.

I tested this one out on the livetest and everything seems to be kosher.

Rock on! Looks great. I tested the basic reports and custom reports w/ parameters, even comparing the results for detailed sales reports against the product report. All looks great. : )

Committing to the 1.x branch now.