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

Improvement - Extending reports #3842

Closed
daigo75 opened this issue Sep 28, 2015 · 4 comments
Closed

Improvement - Extending reports #3842

daigo75 opened this issue Sep 28, 2015 · 4 comments

Comments

@daigo75
Copy link
Contributor

daigo75 commented Sep 28, 2015

Several customers asked to have the possibility of running reports filtered by currency (see example http://prntscr.com/8liaae). From a UI perspective, this is relatively easy, but the architecture of EDD reports doesn't seem to allow altering the data retrieved for the report. For example, method EDD_Payment_Stats::get_earnings() uses the following logic:

  1. Retrieve all the payments in a date range.
  2. Fetch the total for each payment.

Neither operation can be altered, due to the lack of filters. There is a filter for edd_stats_earnings_args, but that won't suffice, because order currency is bundled with the _edd_payment_meta and cannot be used retrieved a simple meta query.

Proposed solution

  1. Add a filter to allow manipulating the list of posts retrieved by get_posts(). This would mean retrieving all posts, like it's already done, then discarding the ones that don't match the selected currency, and it would not be super-efficient, but it would do the job.
  2. Add a filter for the report's transient key, so that the currency can be taken into account. This is necessary to store a cached copy of each report, for each currency (and other data, if needed).
  3. Add a filter to override the earnings retrieval, in order to return the data for the appropriate currency.

I prepared an example of how I would extend the method in this gist: https://gist.github.com/daigo75/25a00fdaa9507910cb28. I only modified EDD_Payment_Stats::get_earnings(), other methods would have to be reviewed and extended as well.

Note

It seems that global function edd_get_earnings_by_date() is still used, despite its code documentation saying "This is getting deprecated soon. Use EDD_Payment_Stats with the get_earnings() method instead". If EDD reports are still going to use that function, then it would become necessary to modify it too, so that the same filters can be applied to the data it retrieves.

@pippinsplugins
Copy link
Contributor

Thanks @daigo75! I'm sure we can get something in to make it more flexible.

@pippinsplugins pippinsplugins added this to the 2.6 milestone Sep 28, 2015
@daigo75
Copy link
Contributor Author

daigo75 commented Sep 28, 2015

You're welcome. Please leave the UI as a lower priority, we can always render extra filters somewhere and "inject" them via JavaScript. The key would be to alter the data passed to the report renderer with the minimum amount of intervention.

In short, I would rather not copy/paste entire classes to rewrite bits and pieces of them, as that could cause countless headaches. Been there, done that (WooCommerce, as you might have guessed by now). 😄

@pippinsplugins
Copy link
Contributor

👍

@pippinsplugins pippinsplugins modified the milestones: 2.7, 2.6 May 4, 2016
@pippinsplugins pippinsplugins modified the milestones: 2.8, 2.7 Dec 7, 2016
@pippinsplugins pippinsplugins modified the milestone: 2.8 Feb 14, 2017
@cklosowski
Copy link
Contributor

EDD 3.0's reports and stats APIs fully support querying by currency.

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

No branches or pull requests

3 participants