craftcms / commerce

Fully integrated ecommerce for Craft CMS.
https://craftcms.com/commerce
Other
226 stars 170 forks source link

Ability to Eagerload Purchasables and Products to Orders #1603

Closed hahnzilla closed 4 years ago

hahnzilla commented 4 years ago

What are you trying to do? I'd love to be able to eagerload lineItems, purchasables/variants, and products onto an order. In this case, when I run something like:

        foreach ($order->getLineItems() as $item) {
            $item->purchasable->product->hsCode;
        }

it produces a few n+1s without a way to get around it.

What's your proposed solution?

Would love to just be able to do something like this:

        $order = Order::find()
            ->with([
                'lineItems',
                'lineItems.purchasable',
                'lineItems.purchasable.product'
            ])
            ->id(2138648)
            ->one();
sjelfull commented 4 years ago

+1. Recently built a custom reporting page, and easily reached 10000+ queries.

This would have saved me writing custom queries for this.

Anubarak commented 4 years ago

I would totally love this. I had to write a 500+ lines script to eager load everything and went down from 20.000 Queries to 150 Queries. Craft Commerce in it's current state is way too query heavy sometimes. I really hope they are going to improve the performance for reports/order lists in the future.

There are many functions that query elements again and again with no caching and there is nearly no way around it. I started to build caching wrappers for so many Commerce functions. You can't even call $order->getEmail() without performance issues

Even "simple" things like checking the lineItems total will result in double queries because it fetches all adjustments of the order (that's okay, it's a must) and loads the adjusters LineItem from the Database to check if their Ids are equal. Each LineItem will thus refetch itself from the DB.

        foreach ($adjustments as $adjustment) {
            // Since the line item may not yet be saved and won't have an ID, we need to check the adjuster references this as it's line item.
            $hasLineItemId = (bool)$adjustment->lineItemId;
            $hasLineItem = (bool)$adjustment->getLineItem();

            if (($hasLineItemId && $adjustment->lineItemId == $this->id) || ($hasLineItem && $adjustment->getLineItem() === $this)) {
                $lineItemAdjustments[] = $adjustment;
            }
        }

Please, do something about this. It would be so awesome to be able to use functions like $order->getCustomer and $order->getUser() in the future again without a massive amount of duplicates.

lukeholder commented 4 years ago

OK added this for the next release:

{% for order in craft.orders.limit(100).all() %}
<strong>{{ order.id }}</strong>
<ul>
<li>Line Items: {{ order.lineItems|length }}</li>
<li>Transactions: {{ order.transactions|length }}</li>
<li>Adjustments: {{ order.adjustments|length }}</li>
</ul>
{% endfor %}

= 300+ queries

{% for order in craft.orders.withAdjustments().withTransactions().withLineItems().limit(100).all() %}
<strong>{{ order.id }}</strong>
<ul>
<li>Line Items: {{ order.lineItems|length }}</li>
<li>Transactions: {{ order.transactions|length }}</li>
<li>Adjustments: {{ order.adjustments|length }}</li>
</ul>
{% endfor %}

= 4 queries

We can't use the normal with eager loading as that is reserved for elements only in core. This is the best we can do for now. When GraphQL support is added to orders, the eager loading will happen by default for these child records of an order 👍

hahnzilla commented 4 years ago

This is amazing! And so quick!

What do you think about extending this to variants and products from the line item?

Perhaps withLineItems() can take an arg of what it can eager load? Something like withLineItems(['purchasable', 'purchasable.product', 'purchasable.product.featuredImage'])

lukeholder commented 4 years ago

I am currently adding support for more eager-loaded things on the order, like addresses and customers. Those will also be in the next release.

The issue with eager loading the products/purchasables is that it is it isn't just commerce variants that could be eager loaded - some installs have custom purchasables like digital products or tickets etc, and since we only store the purchasableId on the line item, I can't do a multi-element query to get them all. If we stored the purchasable type on the line item we could do it a query per element type. Adding the type name to the line item would need a migration, and not sure we will get this into the next release at this point.

Can you submit a new ticket/issue/FR for eager-loading purchasables on order line items? Will get it after the release it out. Thanks.

lukeholder commented 4 years ago

@dhahn ^

hahnzilla commented 4 years ago

yeah absolutely! Thanks for the detailed response totally makes sense

lukeholder commented 4 years ago

Added support in 5209a083cd2da8beec25c8af8402c234b6397536 for eager loading order related customers, users, and addresses.

withAddresses withCustomer (loads the user too)

lukeholder commented 4 years ago

Gotten a large 1000+ order index queries down to ~60 db queries. Super fast.

Anubarak commented 4 years ago

@lukeholder Thank you so much. This is really awesome and makes my life so much easier.