awesomemotive / easy-digital-downloads

Sell digital downloads through WordPress
https://easydigitaldownloads.com
GNU General Public License v2.0
863 stars 474 forks source link

3.0 - Missing (?) actions and filters for new classes and tables #8461

Open daigo75 opened 3 years ago

daigo75 commented 3 years ago

Not bug report, just a question for developers

EDD 3.0 introduces new custom tables and new objects, such as the Query and Order classes, to create orders, reports and so on. I had a look at the code, but I couldn't find many actions and filters that could allow 3rd parties (me 🙋‍♂️) to perform operations, for example, when an item is added, updated or deleted. Is there a plan to introduce these actions and filters, or do the already exist, and I just couldn't find them?

Thanks in advance for the answers.

ashleyfae commented 3 years ago

We do have a few hooks in place, though they are admittedly hard to find because some are built into the base database query layer.

We have a general "transition" style hook that's similar to WordPress core's post transition hook. For example, when an order status changes, this will run:

edd_transition_order_status

With the parameters being:

A similar one runs when a customer's primary address changes. You can see how we hook into it here:

https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/customers/customer-actions.php#L14-L42

The hook actually gets triggered here, which is why it's hard to find: https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/database/engine/class-query.php#L2160-L2175

These transition hooks also run on new inserts. In that case the old value parameter would be new


The query class also has a pre_get style hook similar to WordPress core that runs before queries are executed:

https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/database/engine/class-query.php#L851


There are several filters available as well if you just CTRL+F that file for apply_filters. We use the orders query clause filter ourselves here as an example: https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/database/queries/class-order.php#L209


Ultimately we will likely add more hooks, though they might not be available right out of the gate for 3.0. I'd prefer for the code base to mature at least a little bit before we add a bunch more hooks. But if there's something very specific you're looking for (specific action + table/object/function) and you have a use case for it then us me know.

daigo75 commented 3 years ago

Thanks for the information. In my case, I would be looking for an "order updated" action of some sort. The reason is that, every time an order is saved, I need to recalculate all the totals for the order, as well as each item, in shop's base currency, and store them somewhere. I still haven't figured out where that would go, in the new database. A "catch all" low level filter would not be ideal, because it would intercept every "add" or "update" operation. That could cause significant performance issues.

After that, I would need to alter all the queries that fetch order data for reporting, so that they can take the correct amounts. For example, if orders exist for 100 EUR and 100 USD, the reports currently show "200 USD", which, of course, is wrong. Due to that, I need to be able to replace the column/field used by those reports, and make them take the correct ones. This could involve some extra JOIN, and for that I would need to be able to alter/add that clause as well.

daigo75 commented 3 years ago

By the way, I was also wondering if the role of the EDD_Payment class has changed. In EDD 2.x, payments were actually orders. Do they still exist, or have they been replaced by the Order class?

ashleyfae commented 3 years ago

The reason is that, every time an order is saved, I need to recalculate all the totals for the order, as well as each item, in shop's base currency, and store them somewhere. I still haven't figured out where that would go, in the new database.

Would you only need to do that if the amount on the order record has changed?

I would need to alter all the queries that fetch order data for reporting

I'm going to make a new issue for this and we'll see what we can do. :+1:

By the way, I was also wondering if the role of the EDD_Payment class has changed. In EDD 2.x, payments were actually orders. Do they still exist, or have they been replaced by the Order class?

Payments have been renamed to "orders". Our new EDD\Orders\Order class is meant to be an eventual replacement for EDD_Payment. However, EDD_Payment has not been formally deprecated at this time. Using EDD_Payment is the easiest way to be compatible with 2.9 and 3.0 at the same time. I expect at one point we will formally deprecate EDD_Payment, but there are no immediate plans to do so. Using the order class will be much faster though because EDD_Payment has to run a ton of backwards compatibility logic to still work.

ashleyfae commented 3 years ago

I'm writing up an issue for report filters now, but just also noting that we have a filter that modifies the entire callback function for a report: https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/reports/data/class-endpoint.php#L352

So you could hook into that, read the report ID (get_id() on the object) and conditionally return your own callback function for the data.

Rough example:

add_filter( 'edd_reports_endpoint_data_callback', function( $callback, $endpoint ) {
    /*
     * `overview_all_time_data` is the endpoint ID. See here:
     * @link https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/admin/reporting/reports.php#L233
     */
    if ( 'overview_all_time_data' === $endpoint->get_id() ) {
        // You could return an actual function name that then returns these values.
        return function() {
            $sales    = 1; // Do something to fetch your sale count.
            $earnings = 5.00; // Do something to fetch your earning count.
            return sprintf( '%d / %s', $sales, edd_currency_filter( edd_format_amount( $earnings ) ) );
        };
    }

    return $callback;
}, 10, 2 );
daigo75 commented 3 years ago

To make a comparison with WooCommerce, for which I have an equivalent solution, the idea would be the following:

when(order changes)
  if(any order, item, tax, or any other amount changes) 
    recalculate amounts in shop's base currency

Regarding EDD_Payment, the reason why I'm asking is that my existing solution already works with that class. It intercepts the add/update events and stores some additional meta against each payment. It also "swaps" the order amounts on the fly, depending on the context. EDD reports were not designed to handle amounts in different currencies, so this "trick" allows them to fetch the amounts and just sum them together and show the right totals.

Of course, this is less than ideal, therefore I would prefer a solution similar to the one used for WooCommerce: orders are created normally, they carry their "normal" totals and currency with them, and my solution adds its custom data, and totals in shop's base currency, as custom meta. Then, when the reports, stats or sales widgets are rendered, my solution can alter the queries to allow them to fetch the correct data and use it for its calculations.

daigo75 commented 3 years ago

I'm writing up an issue for report filters now, but just also noting that we have a filter that modifies the entire callback function for a report: https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/reports/data/class-endpoint.php#L352

That's what I did at the time of WooCommerce 1.6/2.0 and, frankly, it's something I'm trying to avoid. I don't want to override the whole report calculation, just replace a column here and there, or add a WHERE clause. There are only two use cases, from my side:

  1. Totals in base currency

For that, I must change some column in the SELECT. Example: Original query

SELECT total
FROM orders

New query

SELECT order_custom_data.total_base_currency
FROM 
  orders
  JOIN
  order_custom_data ON
    (order_custom_data.order_id = orders.order_id)
  1. Totals filtered by currency

For that, I must add WHERE clause . Example: Original query

SELECT total
FROM orders

New query

SELECT total
FROM 
  orders
WHERE
  orders.currency = 'SOME CURRENCY'

That's my target, it's as simple as that. The rest of the reporting logic must remain exactly the same as the core one. There's no need to copy/paste the whole reporting logic and updating it every time it changes in EDD's core.

ashleyfae commented 3 years ago

I understand that would be easier on your end, but we also have to think about what's best for our long-term maintenance of the code base. Adding filters on the query itself ties us to building them in a certain way and we lose our own flexibility in how those queries are crafted. For now at least, we'll probably end up sticking with the filter on the callback because that doesn't interfere with our own operations.

daigo75 commented 3 years ago

I understand your point of view, but I don't think that adding a filter to a query structure could be as complicated as it sounds. EDD 3.0 is still in beta, therefore there should be plenty of time to adjust the aim. As I mentioned, WooCommerce has been running its reports for 6 years using a simple query structure, and it allows to alter the SELECT, FROM, JOIN, WHERE and GROUP BY clauses with a simple filters. The way it does it is quite simple, actually, and doesn't impact on flexibility on your side. You might want to have a look at that, it's quite effective. Additionally, if your query definition structure were to change, altering the filter that modifies it would be significantly easier than cloning and modifying the whole reporting logic .

From my side, as an extension developer, I can say for certain that I won't implement a custom reporting logic, from query to data processing, in a "DIY callback". I did that in 2013 and 2014, when the only way to extend a WooCommerce report was to override entire methods (i.e. the same as the callback you mentioned) and it was an unmaintainable nightmare. Every time the reporting logic changed, I had to run after it, reverse engineer the new logic, create a new version and support multiple versions of these reports at every update. Considering how stressful it was, I'm not going to do that again. 😅

I can also add that the WC team asked me to do the same with the latest analytics reports, which used hard-coded query logic. I had to change a handful of columns (same like just described), and they told me to override the callback instead and return ready made data. I gave a hard "no" to that as well, and engaged with the core developers to explain the suggested approach in detail. About 8 months later, the analytics had the filters I suggested, with some minor changes, and could be overridden with a handful of filters, are going to remain unaltered in future updates.

If you would like to discuss this in more detail, I'm available for a chat as well. In this era of "zoom everything", it's easy enough to have a quick conversation. I'm pretty sure that the changes I suggested are much easier to implement than it may seem. I've been dealing with these kind of issues for over 7 years, I'm happy to share my experience.

ashleyfae commented 3 years ago

Would you be willing to email a copy of your plugin to me at ashley@sandhillsdev.com ? I will take a look at what you're doing now and how we can help you make that happen in 3.0.

We will use this new issue to address filtering in reports: https://github.com/easydigitaldownloads/easy-digital-downloads/issues/8463 I will definitely bear what you said in mind and see if we can make something work for you.

With regards to updating orders, I do want to point out that in 3.0 orders are no longer editable after creation. As you know, in 2.x you have free control to edit order amounts/totals/etc. after an order is completed. But in 3.0 you can no longer do that. So really, once an order is fully built, it shouldn't be getting updates. If you have a use case that perhaps I'm not thinking of then can you start a new discussion here: https://github.com/easydigitaldownloads/easy-digital-downloads/discussions ?

I'm not trying to stop this discussion, I just want to see about moving it over to the new discussions feature that we recently enabled. :smile:

daigo75 commented 3 years ago

I could send you a copy of the plugin I developed, but that won't help much, because it's not compatible with EDD 3.0, and it doesn't include the solution I described. The reports in previous versions of EDD were almost completely hard-coded, therefore I had to adopt an ugly hack, where I swapped order data back and forth, from order currency to base currency, depending on the context, every time the EDD_Payment class is loaded.

This is clunky at best, and I'm glad to see that EDD 3.0 will be more flexible. Still, the existing plugin will not work with it, and it will require the implementation of new logic that is not yet there.

If you would like to have a chat about that, even in person (virtually), I can explain this in more detail. Otherwise, we could risk spending a very long time, writing back and forth, trying to understand what we're trying to do, and why.

As for the orders, the fact that they can't be changed is not relevant to my case. As long as there is an "order created", or "order updated" event, which I can intercept, I will be able to add the custom meta to it. If the order never changes after the creation, that's fine with me.

ashleyfae commented 3 years ago

The reason I asked for a copy for the plugin was so I could see what you're doing now in EDD 2.x as a good reference. For 3.0, our goal right now is to fix bugs and ensure as much as possible what you're doing in 2.x is still possible in 3.0. If it's a new request for a concept that's not in 2.x but will make things simpler for you moving forward then we'll have to revisit it after 3.0, as we're in a feature freeze for the actual 3.0 milestone.

Seeing the code will help me understand this better than a call.


I recommend using our status transition hook for working with created orders. It fires when an order is first added too. I'd recommend running your logic when the payment is first added and when it's changed to complete.

daigo75 commented 3 years ago

I can surely send you a copy of the EDD Currency Switcher as it is now, but it won't be a good reference. The current version will only help you to understand how the plugin is working at the moment, which is not how it's going to work in the future.

As you will see, due to the architecture of EDD 2.0, I had to adopt several hacks to make the solution work, especially in relation to the sales reports. Most of these hacks won't work with EDD 3.0, because they are tightly coupled with the "older" implementation, which was based on global functions, rather than the more modern OOP architecture. In any case, I plan to drop these hacks altogether, as I never really liked the "jumps through hoops" I had to do to make the features work.

I envision a version of the plugin compatible with EDD 3.0 to be significantly different from the existing one, almost if it were a completely new product. Whether it will be possible to implement such compatibility or not will depend on the time and effort required by it.