The enterprise fees are recreated and the shipping and payment fees are updated. The rest of the deleted code is not necessary (eg #with_open_adjustments). Everything else that needs to happen here is already done automatically (eg updating order totals).
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.
Closes#6727.
This avoids the authorization of all the VOs of the hub, which will go
through VOs that may have become invalid due to their underlying product
not belonging to the supplier the hub has permissions with (or any other
data integrity issue).
This is utterly confusing for the user who is only given a generic error
and doesn't understand what's wrong with the particular VO they changed,
while it may be fine after all. What's more, this often results in
a customer support request, which then may end up with a dev finding out
which VO is broken.
Also, there's no point in loading them from DB if the users didn't touch
them.
This removes an N+1 with taggings but doesn't solve the one with tags.
Using `includes(taggings: :base_tags)` based on
47da5036de/lib/acts_as_taggable_on/taggable.rb (L83-L84)
wasn't enough to solve it and I got to stop here. This is scope-creeping
too much.
We get from an initial INNER JOIN with variants and products to fetch
the variant overrides + N queries like:
```sql
SELECT "spree_variants".* FROM "spree_variants" WHERE
"spree_variants"."deleted_at" IS NULL AND "spree_variants"."id" = $1
LIMIT 1 [["id", 1545]]
SELECT "spree_products".* FROM "spree_products" WHERE
"spree_products"."id" = $1 LIMIT 1 [["id", 604]]
```
to the same initial INNER JOIN + just 2 queries like:
```sql
SELECT "spree_variants".* FROM "spree_variants" WHERE
"spree_variants"."deleted_at" IS NULL AND "spree_variants"."id" IN
(1551, 1554)
SELECT "spree_products".* FROM "spree_products" WHERE
"spree_products"."deleted_at" IS NULL AND "spree_products"."id" IN (606,
607)
```
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.
