From d62ab065044ea31662e5d29ae53c2a9668c5430c Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 11 Nov 2020 17:16:23 +0100
Subject: [PATCH] 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;
```
---
 app/controllers/admin/customers_controller.rb   |  7 ++++++-
 .../api/admin/customer_serializer.rb            |  7 ++-----
 spec/features/admin/customers_spec.rb           | 17 +++++++++--------
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb
index e0b44ec1d7..51ad5a519f 100644
--- a/app/controllers/admin/customers_controller.rb
+++ b/app/controllers/admin/customers_controller.rb
@@ -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
diff --git a/app/serializers/api/admin/customer_serializer.rb b/app/serializers/api/admin/customer_serializer.rb
index cafc2a6a2f..1d8b2ffc3f 100644
--- a/app/serializers/api/admin/customer_serializer.rb
+++ b/app/serializers/api/admin/customer_serializer.rb
@@ -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
diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index fe428fed00..4ff41be6c7 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -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