Closed (fixed)
Project:
Commerce Reporting
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
11 Sep 2012 at 19:56 UTC
Updated:
8 Jun 2014 at 19:58 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
googletorp commentedThe sales overview showed what could be the root cause, 2 x november month, when doing a report with a monthly granularity.
Comment #2
cvangysel commentedRight, that's weird behaviour and once it's fixed in the table view the chart should automatically follow.
Could you maybe find out how to reproduce the error? December seems to be completely skipped, so for some reason it thinks the data retrieved from the database in December is actually November.
Which time zone is configured for the Drupal installation?
Comment #3
torgospizzaWe're getting this same issue, on a new install of Drupal Commerce and Commerce Reports. In our case there are two Novembers and nothing showing up for March, except for the first / earliest instance in which February is nullified. We're set to the timezone Los Angeles (-0700). I'm attempting to dig into this now.
Comment #4
pjcdawkins commentedWe also have freaky months.
Comment #5
pjcdawkins commentedComment #6
pjcdawkins commentedSorry for multiple comments. It looks like the Views query does this:
This is in BST timezone (which I assume is the reason for the 3600 seconds). So that looks right.
I expect the problem lies somewhere in
commerce_reports_handler_field_date::post_execute(), which I don't understand at all yet.Comment #7
cvangysel commentedI promise I'll fix this if someone can give me steps (or settings) to reproduce the problem.
Comment #8
torgospizzaFor me, there were no steps, except that I've been migrating old orders, products, and order line items (products) from Ubercart to Commerce. All of our data is the same, and as you can see in our earlier screenshot, all of our data is intact.
When I visit the Sales Reports page, if I'm in a "several years, broken down monthly" report, that is where the duplicate and empty months are shown. So in the screenshot I posted in #3, there is an empty March, an empty February. But if I were to run a report for that month, broken down by days or weeks, all of the product reports are there, and I'm given (presumably) correct totals for each day of those "empty" months. I haven't quite figured out where the duplicates come from, though.
Aside from the migrated data, this was a new Commerce install that I created a couple months back, and have kept up with updates as they are made available. I could theoretically wipe the database and start with the latest code, but I'm not convinced that will make a difference.
So with regard to "steps to reproduce" for us, there are none: simply run reports for a year more, broken down by month, and the errors are created. In fact even in your earlier screenshots for the module, you can see that your month of March had no sales! (http://drupal.org/files/project-images/dashboard_highcharts_0.png and http://drupal.org/files/project-images/dashboard_0.png)
Perhaps the issue is more closely related to Views and how it aggregates some of this data? I haven't had a lot of time to dig back into it myself (I'm now dealing with issues involving Commerce File) but am happy to try things if it means getting this very important functionality fixed :)
Comment #9
cvangysel commentedI'm actually guessing there's something going wrong with
DATE_FORMAT((DATE_ADD('19700101', INTERVAL commerce_order.created SECOND) + INTERVAL 3600 SECOND), '%Y-%m')and it probably is related to timezone issues.
Could someone that's having the problem run the following query on their MySQL installation:
SELECT @@global.time_zone, @@session.time_zone;and post the results back? Please also include the time zone configured in your PHP installation.
What also might come in handy is the raw results of the query used by Views, combined with the content generated by Views.
Comment #10
torgospizzaI ran the query you asked for and received "SYSTEM SYSTEM".
In our server's phpinfo() "Default Timezone" is set to America/Los_Angeles, however under "directives" there is no value set for date.timezone.
Also in the Date settings for Drupal. America/Los_Angeles is configured as the default timezone as well.
Comment #11
torgospizzaAnd here is, according to Views, the query that is run for the Yearly reports. In the Preview it results in a chart, but I can see that the chart also has the same month issues - duplicate Novembers, and nothing for March.
See screengrab of the resulting chart, attached.
Comment #12
torgospizzaRunning the above query through the mysql command line appears to return correct results. I've pasted it here as a text file since the columns are extremely wide.
Comment #13
cvangysel commentedThanks! I identified the problem and I'm thinking about a fix.
I'll write it somewhere this week. I need to think about it a bit because I realise there are also implications on correctness of the solution.
Comment #14
torgospizzaExcellent! Let me know if there's anything else I can provide, or if you'd like to discuss your proposed solution. Happy to help.
Comment #15
pjcdawkins commentedSeconding everything torgosPizza has said; I seem to have very similar set-up, just a different timezone and different source data.
Comment #16
cvangysel commentedI think I fixed this in the HEAD of 7.x-3.x, can someone verify this?
Comment #17
pjcdawkins commentedJust tested commit f94522b - it doesn't appear to have made a difference. Will try HEAD later.
Comment #18
pjcdawkins commentedJust to update: no it doesn't seem to make any difference at all.
By the way, for the overview graphs, would it be possible to utilise something like the Annotated Timeline (http://code.google.com/apis/ajax/playground/?type=visualization#annotate...)? Then the graph would take care of the date calculations. But I realise the SQL queries themselves need to be fixed to show the tabular reports.
Comment #19
torgospizzaIf it's still broken, I'll set this to needs work. I can't test yet but will try to soon.
Comment #20
pjcdawkins commentedMarked #1849208: Zeros appear in the weekly sales report as a duplicate of this issue.
Potentially relevant comment in that thread, by aidanlis:
http://drupal.org/node/1849208#comment-6772600
Comment #21
cvangysel commentedThe reason for commerce_reports_handler_field_date:post_execute is that when you query the database you only get results for the months data is available for. commerce_reports_handler_field_date:post_execute tries to fill the data that isn't available.
Comment #22
sher1 commentedWe having this same no sales for August 2012 but we also have 2 rows for July with different sales amounts. I have upgraded to beta2 and the speed increase is very noticeable and very appreciated but the problem is still there. When I run the views query against mysql directly (removing curly braces of course) it seems to work the way it would be expected to.
Comment #23
sher1 commentedCheck that. The report shows
December 2012 15 $101.00 $6.73
November 2012 281 $2,227.75 $7.93
October 2012 100 $555.75 $5.56
September 2012 127 $616.50 $4.85
August 2012 0
July 2012 200 $1,265.50 $6.33
July 2012 571 $1,015.00 $1.78
June 2012 96 $595.50 $6.20
While this query, taken from views
SELECT date_format(from_unixtime(MIN(commerce_order.created)),'%M %Y') AS order_date,
MIN(commerce_order.created) AS commerce_order_created_granularity,
COUNT(commerce_order.order_id) AS order_id,
MIN(commerce_order.order_id) AS order_id_1,
'commerce_order' AS field_data_commerce_order_total_commerce_order_entity_type,
SUM(field_data_commerce_order_total.commerce_order_total_amount) AS field_data_commerce_order_total_commerce_order_total_amount,
SUM(field_data_commerce_order_total.commerce_order_total_currency_code) AS field_data_commerce_order_total_commerce_order_total_currenc,
AVG(field_data_commerce_order_total.commerce_order_total_amount) AS field_data_commerce_order_total_commerce_order_total_amount_1,
AVG(field_data_commerce_order_total.commerce_order_total_currency_code) AS field_data_commerce_order_total_commerce_order_total_currenc_1
FROM
commerce_order
LEFT JOIN field_data_commerce_order_total ON commerce_order.order_id = field_data_commerce_order_total.entity_id AND (field_data_commerce_order_total.entity_type = 'commerce_order' AND field_data_commerce_order_total.deleted = '0')
WHERE (( (commerce_order.created BETWEEN 1322697600 AND 1357084800) )AND(( (commerce_order.status IN ('pending', 'processing', 'completed')) )))
GROUP BY DATE_FORMAT((DATE_ADD('19700101', INTERVAL commerce_order.created SECOND) + INTERVAL -25200 SECOND), '%Y-%m'), field_data_commerce_order_total_commerce_order_entity_type
ORDER BY commerce_order_created_granularity DESC
shows
December 2012 1354401743 15 2684 commerce_order 10100 0 673.3333 0
November 2012 1351796754 281 2169 commerce_order 222775 0 792.7936 0
October 2012 1349106501 100 1971 commerce_order 55575 0 555.7500 0
September 2012 1346517426 127 1714 commerce_order 61650 0 485.4331 0
August 2012 1343806642 200 1292 commerce_order 126550 0 632.7500 0
July 2012 1341154149 571 239 commerce_order 101500 0 177.7583 0
June 2012 1338579980 96 21 commerce_order 59550 0 620.3125 0
The sales totals don't match at all. Is this just some views hocus pocus? ;)
Comment #24
torgospizza@sher1: See the comment above, in #20,
To test this theory out, you'd have to comment out or remove that method from the module file. At the moment, this seems to indicate that the Views "post_execute" method in the date handler, which sounds like it is triggered after the Views query is executed, appears to be affecting the reports in this way.
Comment #25
sher1 commented@torgosPizza, that did it. Problem went away completely. I even read comment #20 above and it just didn't click. Thanks for pointing it out.
Comment #26
pjcdawkins commentedPlease note comment #21: to display reasonable graphs, dates do have to be interpolated. The code in the
commerce_reports_handler_field_date::post_execute()method is necessary. Bypassing the method is only a weak workaround. We've identified that there's probably a bug in that code. Further comments here should be about fixing that bug.Comment #27
pjcdawkins commented[edit: sorry, double post]
Comment #28
cvangysel commentedI rewrote the post_execute interpolation bit using an easier to understand (but less performant) version.
The decrease in performance will not be noticeable in most cases, so I hope this fixes this problem. If you want to test the updated code, checkout the Git repository or install the development release of the module.
Comment #29
aidanlis commentedNot working here ... on both the stable and the dev I'm seeing weirdness. Whole months without sales and "today/yesterday" which don't match any of the days.
The SQL query outputs the correct data, so it's definitely that post processing in the views handler which is making weirdness.
Comment #30
aidanlis commentedIf you have a high volume store and don't need interpolation (because you make a sale at least every day), this fixes everything:
Comment #31
torgospizzaWhich is pretty much the same thing as commenting the entire function out :)
A hack like this is probably not an ideal fix for the module at large.
Comment #32
aidanlis commentedRight, I didn't have time to look at the interpolation logic ... but given most eCommerce sites sell at least one item per day, simply disabling the function will solve most peoples problems.
Comment #33
torgospizzaYou may have missed the comment in #26 about focusing on fixing the actual bugs contained in the post_execute() method. Let's try to take care of the root cause rather than finding new ways to bypass it! (I'll take a look at it myself next chance I get..)
Comment #34
aidanlis commentedAh, I did indeed miss that comment. What about letting the SQL do the work and doing a LEFT JOIN on a generated date table? That way we bypass the timezone problems which seem to be popping up too.
This would let us mark up holidays, weekends, sales, etc too.
Comment #35
pjcdawkins commentedComment #36
strings6 commentedFirst, a note of thank you to the author for this module. Much appreciated!
I'm having the same issue with 7.x-3.0-beta2. I'm getting 0's for March and January. Curious-- what's the status on fixing this?
Comment #37
strings6 commentedHello,
Can anyone tell me if the 7.x-3.x-dev version fixes this issue, and if so, when it is slotted to be pushed to the recommended release version?
Thanks.
Comment #38
torgospizzaThis actually appears to be fixed in the latest dev. I won't mark as fixed yet unless someone can confirm (and I can defer to the maintainers). But as of now it looks great - no repeated months and nothing with an erroneous 0! Great work all.
Comment #39
torgospizzaOn second look it's not quite correct. If I select, say, April 2010 for my Start Date, the report itself actually starts at February 2010. So setting this back to active.
Comment #40
strings6 commentedcvangysel - any near-future plans to work on this? A lot of fiscal years end on June 30, so clients will likely be wanting to use Commerce Reports to look at yearly numbers. No pressure :-)
Comment #41
aidanlis commentedTimezones are a pain in the arse ... if someone wants to rewrite it in the database layer using a GROUP BY %Y-%m we can remove all of the interpolation code.
Comment #42
torgospizzaI just wanted to confirm that it's definitely a timezone issue. It seems that the issue is just because of, with a difference of a couple hours, a report for June 2012 through June 2013 will take, for example, "June 1 at 7am" as all time intervals (using the MIN interpolation code) but instead of May at 7am adds and converts the first month's day as June 1st again but at midnight. This means the report goes ends up having a duplicate June 2012.
Oddly enough, setting my site's default time zone to UTC seems to be a good workaround for now.
Comment #43
marcus178 commentedI just tried the dev version and it's not working right on that either, for me March is always empty. I can also confirm changing to the UTC timezone clears the problem but this is not an ideal resolution for myself
Comment #44
michfuer commentedI was running into the same types of problems when setting 'Monthly' granularity on the filter. It appears the _normalizeMonthly() function doesn't always properly return the first day of the month for a given timestamp, so in post_execute() when the $values are first normalized you can end up getting duplicates. Applying the attached patch to the 7.x-3.0-beta2 fixes the problem for me.
Comment #45
michfuer commentedI ended up running into a couple of more issues. The first was that _normalizeWeekly() was causing duplicates as well when running the reports against that granularity. I updated it to return 12am on Monday of the week corresponding to the input date. The other problem is that the $end_date in query() is calculated with hardcoded intervals (the seconds in a month, a week, etc.). This causes inconsistencies in the results because each month isn't the same interval. I added a conversion function that takes the normalized end date, and returns the appropriate query end date for a given granularity input. See the attached patch.
Comment #46
michfuer commentedThis patch is the same as #45 but adds the year to the weekly and daily end dates in the conversion function. This should add a bit more robustness to the calculations.
Comment #47
michfuer commentedThis adds a Daily normalization function. This is important for eliminating duplicates in post_execute() when granularity is Daily.
Comment #48
michfuer commentedSo I was trying to keep all the patches independent of each other by diff'ing them against the 7.x-3.0-beta2 version. The problem is that there are enough changes in the field date handler file, that trying to apply each patch, one after the other, can cause some of the subsequent patches to fail. So this latest patch is basically a bundle of the previous ones along with the removal of pretty much all uses of the hardcoded 'interval' in the date handler.
Comment #49
risse commentedChecked it in version 7.x-3.0-beta2, seems to work just fine. Big thanks for this patch, I have waited for it for a while!
Comment #50
torgospizzaAwesome work, @michfuer! Thanks for taking the initiative.
Will the patch apply to the latest dev? I know there were some changes in 3.x-dev in an attempt to fix the sales reporting issue; would it be possible to get a reroll against the current dev version? (It appears that this is general procedure when writing patches for Drupal code, as per https://drupal.org/node/707484.)
Comment #51
marcus178 commentedI've applied the last patch to beta2 but not seeing any improvement, it's still reporting zero sales for March.
Comment #52
pjcdawkins commentedUnfortunately I had the same experience as @marcus178. Many thanks for the work though. I'm sure it fixes other bugs and may help to fix this one. But the bug isn't fixed.
Can future patches be made against -dev? I think that's important: there may be new work in -dev that needs to be rethought anyway.
Comment #53
michfuer commented@torgosPizza, @pjcdawkins: Yeah, I should've patched against dev, but when I first started working on it I noticed that dev had different issues of it's own (namely the Date field won't sort in the Sales report), so I took the path of least resistance since beta2 seemed to be working better.
Here's the patch against dev. The biggest difference in the dev version is that post_execute() was simplified quite a bit. The Date sort still doesn't work, but that's a problem for another issue (and day!) :)
Comment #54
strings6 commented@michfuer and @everyone else who worked on this:
You guys rock! The new dev version is working for me, looking great.
Does anyone find any reason not to push the dev version to a stable release? It is 10x's better than the beta-2 version in my opinion because the sales reports works correctly, but I'm not aware of what other issues could be wrong with the dev version. Anyone have any objections to that motion?
Comment #55
dpearcefl commentedAny other comments on the patch in #53?
Comment #56
strings6 commentedHi all,
Now I'm a bit curious, because I made the assumption that the Sept 30 update to the dev branch included this new patch from michfuer, but when we checked, it had not been included.
I reported previously that the problem appeared to be fixed in dev, so now I'm not sure why it appears to be working just fine if the patch wasn't really added. Did something else in dev fix (or perhaps mostly fix) the problem?
Our ultimate goal here is to get all the patches applied to dev that are needed, and then once everything seems to be gelling, finally update the stable release that hasn't been updated for quite some time.
Any input on that?
Comment #57
aidanlis commented@strings6 I think the bug is mostly timezone issues so I wouldn't be surprised if it looked fixed but wasn't.
Comment #58
mglamanI have had the same issue and frankly it seems like more work to fix this than to migrate Commerce Reports to utilize Views Date Format SQL module.
It took 20 minutes to switch over the Sales report and provides better date grouping in my opinion and leaves less arbitrary code to be maintained within the module.
I plan on submitting the patch in next few days, was wondering if others had feedback.
Comment #59
dpearcefl commentedAre we waiting on a patch for this issue? Just trying to understand why it's marked "Needs review".
Comment #60
mglamanPlease review the 7.x-4.x release which should resolve date ranges, and note module changes as stated in #2129735: Commerce Reports 4.x Review
Comment #61
mglamanClosing out, since granularity filter is not relevant to 4.x