mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-03-01 02:03:22 +00:00
Refactor DB query to aggregate customer balance
It's simpler and many orders of magnitude more efficient to ask the DB
to aggregate the customer balance based on their orders. It removes
a nasty N+1.
The resulting SQL query is:
```sql
SELECT customers.*, SUM(spree_orders.total - spree_orders.payment_total) AS balance
FROM "customers"
INNER JOIN "spree_orders"
ON "spree_orders"."customer_id" = "customers"."id"
WHERE "customers"."enterprise_id" = 1
AND (completed_at IS NOT NULL)
AND (state != 'canceled')
GROUP BY customers.id
ORDER BY email;
```
This commit is contained in:
@@ -66,7 +66,12 @@ module Admin
|
||||
return Customer.where("1=0") unless json_request? && params[:enterprise_id].present?
|
||||
|
||||
Customer.of(managed_enterprise_id).
|
||||
includes(:bill_address, :ship_address, user: :credit_cards)
|
||||
includes(:bill_address, :ship_address, user: :credit_cards).
|
||||
joins(:orders).
|
||||
merge(Spree::Order.complete.not_state(:canceled)).
|
||||
group("customers.id").
|
||||
select("customers.*").
|
||||
select("SUM(total - payment_total) AS balance_value")
|
||||
end
|
||||
|
||||
def managed_enterprise_id
|
||||
|
||||
@@ -9,6 +9,8 @@ module Api
|
||||
has_one :ship_address, serializer: Api::AddressSerializer
|
||||
has_one :bill_address, serializer: Api::AddressSerializer
|
||||
|
||||
delegate :balance_value, to: :object
|
||||
|
||||
def name
|
||||
object.name.presence || object.bill_address.andand.full_name
|
||||
end
|
||||
@@ -51,11 +53,6 @@ module Api
|
||||
|
||||
options[:customer_tags].andand[object.id] || []
|
||||
end
|
||||
|
||||
def balance_value
|
||||
@balance_value ||=
|
||||
OpenFoodNetwork::UserBalanceCalculator.new(object.email, object.enterprise).balance
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -97,9 +97,9 @@ feature 'Customers' do
|
||||
|
||||
describe "for a shop with multiple customers" do
|
||||
before do
|
||||
mock_balance(customer1.email, managed_distributor1, 88)
|
||||
mock_balance(customer2.email, managed_distributor1, -99)
|
||||
mock_balance(customer4.email, managed_distributor1, 0)
|
||||
build_balance(customer1, managed_distributor1, 88)
|
||||
build_balance(customer2, managed_distributor1, -99)
|
||||
build_balance(customer4, managed_distributor1, 0)
|
||||
|
||||
customer4.update enterprise: managed_distributor1
|
||||
end
|
||||
@@ -122,11 +122,12 @@ feature 'Customers' do
|
||||
end
|
||||
end
|
||||
|
||||
def mock_balance(email, enterprise, balance)
|
||||
user_balance_calculator = instance_double(OpenFoodNetwork::UserBalanceCalculator)
|
||||
expect(OpenFoodNetwork::UserBalanceCalculator).to receive(:new).
|
||||
with(email, enterprise).and_return(user_balance_calculator)
|
||||
expect(user_balance_calculator).to receive(:balance).and_return(balance)
|
||||
def build_balance(customer, enterprise, balance)
|
||||
order = build(:order, total: balance, payment_total: 0, distributor: enterprise)
|
||||
order.email = customer.email
|
||||
order.customer_id = customer.id
|
||||
order.completed_at = Time.zone.now
|
||||
order.save!
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user