Remove a N+1 query on shipment model #3598
Conversation
|
@albanv it looks like CircleCI didn't run here for some reason. Could you push an empty commit and then remove it? |
albanv
force-pushed
the
remove-n+1-query
branch
from
97a0d65 to
f74cfa7
Compare
|
@aldesantis just did it, but the CI failed as well 😞 |
albanv
force-pushed
the
remove-n+1-query
branch
from
f74cfa7 to
0dd156d
Compare
|
There is something weird, the CI used here is the one from my org (epicery) and not solidus one |
|
@albanv I think you should opt-in to use external orbs in your |
|
@albanv looks like the CI is still failing here, any chance you can take a look? 🙏 I'm also happy to create another PR myself with your code if you want, just let me know! |
|
@albanv hey there! Just wanted to check if you had time to take a look, I'd love to merge this one, thanks! |
kennyadsl
added
Needs Work
changelog:solidus_core
In `to_package` method, I've added `product` to includes. We have discovered that `Spree::Stock::Estimator#calculate_shipping_rates` currently make a product query for each `variant`. In our test, this subtle change has drastically reduced the number of query to `product` by a factor equivalent to `variant` number.
albanv
force-pushed
the
remove-n+1-query
branch
from
351eefd to
8c6b2fb
Compare
|
Sorry for the late reply, I don't work for Epicery anymore and so goes with solidus. |
|
@kennyadsl I'm not sure about the CircleCI current failures. Is it the same than before ? Does this PR needs more work before merging ? |
|
Nope, the only failing check is with Rails master (which is unreleased so it's not an issue). We are fine now, thanks @stem ! |
Description
In
to_packagemethod, I've addedproductto includes.We have discovered that when we call
Spree::Stock::Estimator#calculate_shipping_rateswith ashipment.to_package, it currently makes aproductDB query for eachvariant.In our test, this subtle change has drastically reduced the number of queries to
productby a factor equivalent tovariantnumber.Concerning the test to add, since I'm not very sure on how to proceed, I would like some guidance. (I've looked at estimator_spec and shipment_spec but since this issue arise when both are coupled, I'm not sure where is the place to start)
Checklist: