With the help of the bullet gem, and since we remove a couple of N+1s
already, remove them all was just a few keystrokes away. This commits
gets us from 42 SQL queries to 17, and 364.5ms to 253.9ms on my machine
where I just have the sample data's orders. As usual, this will have
a much bigger impact in scenarios with more data.
This removes the N+1 queries caused by
`Api::Admin::OrderSerialier#ready_to_capture` when used from
`Api::OrdersController#index`. While it's fine for the single-order
controller actions, it's not for this one that deals with a collection
of orders.
Fortunately, `SearchOrders` is used only in this controller action so we
can put the `includes` calls there, otherwise, we would need to refactor
it a bit to pass in a context-specific AR relation.
This makes them more changeable and robust. Ruby will raise
NoMethodError on typos while it'll silently create a new ivar without
us noticing. Also, in my experience, a reader method gives more room to
future refactorings and eases testing because methods are easier to
stub.
The controller already does so, then, we can pass it to the service and
avoid that extra round-trip to the DB and save some memory. Spree::Order
is a rather bulky object (God object code smell perhaps) and it'll
surely make a difference.
In the line below we filter them out in Ruby so it's a waste of
resources. The fundamental difference is that `#includes` and
`#references` results in LEFT JOINs, whereas `#joins` results in INNER
JOIN, and because there's a default scope on `deleted_at IS NULL`, these
are not included in the result set.
This however, requires us to move away from the current algorithm but
unfortunately we can't refactor it completely yet.
Before:
```sql
SELECT *
FROM "variant_overrides"
LEFT OUTER
JOIN "spree_variants"
ON "spree_variants"."id" = "variant_overrides"."variant_id"
AND "spree_variants"."deleted_at" IS NULL
LEFT OUTER
JOIN "spree_products"
ON "spree_products"."id" = "spree_variants"."product_id"
AND "spree_products"."deleted_at" IS NULL
WHERE "variant_overrides"."permission_revoked_at" IS NULL
AND "variant_overrides"."hub_id" IN (
SELECT "enterprises"."id"
FROM "enterprises"
INNER
JOIN "enterprise_roles"
ON "enterprise_roles"."enterprise_id" = "enterprises"."id"
WHERE (enterprise_roles.user_id = ?)
AND (sells != 'none')
ORDER BY name)
```
After:
```sql
SELECT "variant_overrides".*
FROM "variant_overrides"
INNER
JOIN "spree_variants"
ON "spree_variants"."id" = "variant_overrides"."variant_id"
AND "spree_variants"."deleted_at" IS NULL
INNER
JOIN "spree_products"
ON "spree_products"."id" = "spree_variants"."product_id"
AND "spree_products"."deleted_at" IS NULL
WHERE "variant_overrides"."permission_revoked_at" IS NULL
AND "variant_overrides"."hub_id" IN (
SELECT "enterprises"."id"
FROM "enterprises"
INNER
JOIN "enterprise_roles"
ON "enterprise_roles"."enterprise_id" = "enterprises"."id"
WHERE (enterprise_roles.user_id = ?)
AND (sells != 'none')
ORDER BY name)
```
This is covered in the test suite by
spec/controllers/admin/variant_overrides_controller_spec.rb:72. It keeps
passing so we're good to go.