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 01/16] 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
 

From e404225de0f2507abc04fb86c8e118b809636135 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 19 Nov 2020 16:35:35 +0100
Subject: [PATCH 02/16] Extract query object

---
 app/controllers/admin/customers_controller.rb |  8 +-------
 app/services/customers_with_balance.rb        | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 7 deletions(-)
 create mode 100644 app/services/customers_with_balance.rb

diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb
index 51ad5a519f..3ab665d9b5 100644
--- a/app/controllers/admin/customers_controller.rb
+++ b/app/controllers/admin/customers_controller.rb
@@ -65,13 +65,7 @@ module Admin
     def collection
       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).
-        joins(:orders).
-        merge(Spree::Order.complete.not_state(:canceled)).
-        group("customers.id").
-        select("customers.*").
-        select("SUM(total - payment_total) AS balance_value")
+      CustomersWithBalance.new(managed_enterprise_id).query
     end
 
     def managed_enterprise_id
diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
new file mode 100644
index 0000000000..0abd804ff2
--- /dev/null
+++ b/app/services/customers_with_balance.rb
@@ -0,0 +1,19 @@
+class CustomersWithBalance
+  def initialize(enterprise_id)
+    @enterprise_id = enterprise_id
+  end
+
+  def query
+    Customer.of(enterprise_id).
+      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
+
+  private
+
+  attr_reader :enterprise_id
+end

From 1e9a1f340e149cfabe0cd62d21cb73fe9b0b81cd Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 20 Nov 2020 12:38:07 +0100
Subject: [PATCH 03/16] Don't consider order total when it is canceled

---
 app/services/customers_with_balance.rb       | 16 ++++-
 spec/services/customers_with_balance_spec.rb | 65 ++++++++++++++++++++
 2 files changed, 78 insertions(+), 3 deletions(-)
 create mode 100644 spec/services/customers_with_balance_spec.rb

diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index 0abd804ff2..e34e37736f 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 class CustomersWithBalance
   def initialize(enterprise_id)
     @enterprise_id = enterprise_id
@@ -6,14 +8,22 @@ class CustomersWithBalance
   def query
     Customer.of(enterprise_id).
       includes(:bill_address, :ship_address, user: :credit_cards).
-      joins(:orders).
-      merge(Spree::Order.complete.not_state(:canceled)).
+      joins("LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id").
       group("customers.id").
       select("customers.*").
-      select("SUM(total - payment_total) AS balance_value")
+      select(outstanding_balance)
   end
 
   private
 
   attr_reader :enterprise_id
+
+  def outstanding_balance
+    <<-SQL.strip_heredoc
+       SUM(
+         CASE WHEN state = 'canceled' THEN payment_total
+         ELSE payment_total - total END
+       ) AS balance_value
+    SQL
+  end
 end
diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb
new file mode 100644
index 0000000000..687ab5900a
--- /dev/null
+++ b/spec/services/customers_with_balance_spec.rb
@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe CustomersWithBalance do
+  subject(:customers_with_balance) { described_class.new(customer.enterprise.id) }
+
+  describe '#query' do
+    let(:customer) { create(:customer) }
+
+    context 'when no orders where paid' do
+      let(:total) { 200.00 }
+      let(:order_total) { 100.00 }
+
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0)
+        create(:order, customer: customer, total: order_total, payment_total: 0)
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(-total)
+      end
+    end
+
+    context 'when an order was paid' do
+      let(:total) { 200.00 }
+      let(:order_total) { 100.00 }
+      let(:payment_total) { order_total }
+
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0)
+        create(:order, customer: customer, total: order_total, payment_total: payment_total)
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(payment_total - total)
+      end
+    end
+
+    context 'when an order is canceled' do
+      let(:total) { 200.00 }
+      let(:order_total) { 100.00 }
+      let(:payment_total) { 100.00 }
+      let(:complete_orders_total) { order_total }
+
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0)
+        create(
+          :order,
+          customer: customer,
+          total: order_total,
+          payment_total: order_total,
+          state: 'canceled'
+        )
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(payment_total - complete_orders_total)
+      end
+    end
+  end
+end

From d8872bc785c9b99bb5373b34b556cd49d7fa9436 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 24 Nov 2020 11:37:43 +0100
Subject: [PATCH 04/16] Explain why Arel can't be used in statement

---
 app/services/customers_with_balance.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index e34e37736f..13ab8d40a1 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -18,6 +18,8 @@ class CustomersWithBalance
 
   attr_reader :enterprise_id
 
+  # Arel doesn't support CASE statements until v7.1.0 so we'll have to wait with SQL literals
+  # a little longer. See https://github.com/rails/arel/pull/400 for details.
   def outstanding_balance
     <<-SQL.strip_heredoc
        SUM(

From 9d0dd968b1af59261027e34920ee320f99159019 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 24 Nov 2020 17:35:22 +0100
Subject: [PATCH 05/16] Test customer balance query with guest orders

Guest orders also have an associated customer record that is created
by Spree::Order#ensure_customer at checkout. Subsequent orders will use
that one due to Spree::Order#associate_customer.
---
 spec/factories/order_factory.rb              |   2 +-
 spec/services/customers_with_balance_spec.rb | 103 +++++++++++++++----
 2 files changed, 83 insertions(+), 22 deletions(-)

diff --git a/spec/factories/order_factory.rb b/spec/factories/order_factory.rb
index 04885c0965..aec931b752 100644
--- a/spec/factories/order_factory.rb
+++ b/spec/factories/order_factory.rb
@@ -9,7 +9,7 @@ FactoryBot.define do
     user
     bill_address
     completed_at { nil }
-    email { user.email }
+    email { user&.email || customer.email }
 
     factory :order_with_totals do
       after(:create) do |order|
diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb
index 687ab5900a..c0b5a9bb00 100644
--- a/spec/services/customers_with_balance_spec.rb
+++ b/spec/services/customers_with_balance_spec.rb
@@ -12,14 +12,28 @@ describe CustomersWithBalance do
       let(:total) { 200.00 }
       let(:order_total) { 100.00 }
 
-      before do
-        create(:order, customer: customer, total: order_total, payment_total: 0)
-        create(:order, customer: customer, total: order_total, payment_total: 0)
+      context 'when non-guest order' do
+        before do
+          create(:order, customer: customer, total: order_total, payment_total: 0)
+          create(:order, customer: customer, total: order_total, payment_total: 0)
+        end
+
+        it 'returns the customer balance' do
+          customer = customers_with_balance.query.first
+          expect(customer.balance_value).to eq(-total)
+        end
       end
 
-      it 'returns the customer balance' do
-        customer = customers_with_balance.query.first
-        expect(customer.balance_value).to eq(-total)
+      context 'when guest order' do
+        before do
+          create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
+          create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
+        end
+
+        it 'returns the customer balance' do
+          customer = customers_with_balance.query.first
+          expect(customer.balance_value).to eq(-total)
+        end
       end
     end
 
@@ -28,14 +42,34 @@ describe CustomersWithBalance do
       let(:order_total) { 100.00 }
       let(:payment_total) { order_total }
 
-      before do
-        create(:order, customer: customer, total: order_total, payment_total: 0)
-        create(:order, customer: customer, total: order_total, payment_total: payment_total)
+      context 'when non-guest order' do
+        before do
+          create(:order, customer: customer, total: order_total, payment_total: 0)
+          create(:order, customer: customer, total: order_total, payment_total: payment_total)
+        end
+
+        it 'returns the customer balance' do
+          customer = customers_with_balance.query.first
+          expect(customer.balance_value).to eq(payment_total - total)
+        end
       end
 
-      it 'returns the customer balance' do
-        customer = customers_with_balance.query.first
-        expect(customer.balance_value).to eq(payment_total - total)
+      context 'when guest order' do
+        before do
+          create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
+          create(
+            :order,
+            customer: customer,
+            user: nil,
+            total: order_total,
+            payment_total: payment_total
+          )
+        end
+
+        it 'returns the customer balance' do
+          customer = customers_with_balance.query.first
+          expect(customer.balance_value).to eq(payment_total - total)
+        end
       end
     end
 
@@ -45,17 +79,44 @@ describe CustomersWithBalance do
       let(:payment_total) { 100.00 }
       let(:complete_orders_total) { order_total }
 
-      before do
-        create(:order, customer: customer, total: order_total, payment_total: 0)
-        create(
-          :order,
-          customer: customer,
-          total: order_total,
-          payment_total: order_total,
-          state: 'canceled'
-        )
+      context 'when non-guest order' do
+        before do
+          create(:order, customer: customer, total: order_total, payment_total: 0)
+          create(
+            :order,
+            customer: customer,
+            total: order_total,
+            payment_total: order_total,
+            state: 'canceled'
+          )
+        end
+
+        it 'returns the customer balance' do
+          customer = customers_with_balance.query.first
+          expect(customer.balance_value).to eq(payment_total - complete_orders_total)
+        end
       end
 
+      context 'when guest order' do
+        before do
+          create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
+          create(
+            :order,
+            customer: customer,
+            user: nil,
+            total: order_total,
+            payment_total: order_total,
+            state: 'canceled'
+          )
+        end
+
+        it 'returns the customer balance' do
+          customer = customers_with_balance.query.first
+          expect(customer.balance_value).to eq(payment_total - complete_orders_total)
+        end
+      end
+    end
+
       it 'returns the customer balance' do
         customer = customers_with_balance.query.first
         expect(customer.balance_value).to eq(payment_total - complete_orders_total)

From 96a91969c9a77d2c97bcbca151f7bfeec49a8b84 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 24 Nov 2020 17:38:21 +0100
Subject: [PATCH 06/16] Extract balance-specific serializer

So we only show the customer balance where really needed. Aggregating
the balance can be costly. Also, we avoid defensive coding.
---
 app/controllers/admin/customers_controller.rb | 15 +++---
 .../api/admin/customer_serializer.rb          | 18 +-------
 .../admin/customer_with_balance_serializer.rb | 25 ++++++++++
 .../admin/customers_controller_spec.rb        |  5 ++
 spec/features/admin/customers_spec.rb         | 38 ++++++++++-----
 .../customer_with_balance_serializer_spec.rb  | 46 +++++++++++++++++++
 6 files changed, 113 insertions(+), 34 deletions(-)
 create mode 100644 app/serializers/api/admin/customer_with_balance_serializer.rb
 create mode 100644 spec/serializers/api/admin/customer_with_balance_serializer_spec.rb

diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb
index 3ab665d9b5..630ffd361b 100644
--- a/app/controllers/admin/customers_controller.rb
+++ b/app/controllers/admin/customers_controller.rb
@@ -17,9 +17,10 @@ module Admin
       respond_to do |format|
         format.html
         format.json do
-          render_as_json @collection,
-                         tag_rule_mapping: tag_rule_mapping,
-                         customer_tags: customer_tags_by_id
+          render json: @collection,
+                 each_serializer: ::Api::Admin::CustomerWithBalanceSerializer,
+                 tag_rule_mapping: tag_rule_mapping,
+                 customer_tags: customer_tags_by_id
         end
       end
     end
@@ -63,9 +64,11 @@ module Admin
     private
 
     def collection
-      return Customer.where("1=0") unless json_request? && params[:enterprise_id].present?
-
-      CustomersWithBalance.new(managed_enterprise_id).query
+      if json_request? && params[:enterprise_id].present?
+        CustomersWithBalance.new(managed_enterprise_id).query
+      else
+        Customer.where('1=0')
+      end
     end
 
     def managed_enterprise_id
diff --git a/app/serializers/api/admin/customer_serializer.rb b/app/serializers/api/admin/customer_serializer.rb
index 1d8b2ffc3f..99d8bad28c 100644
--- a/app/serializers/api/admin/customer_serializer.rb
+++ b/app/serializers/api/admin/customer_serializer.rb
@@ -4,13 +4,11 @@ module Api
   module Admin
     class CustomerSerializer < ActiveModel::Serializer
       attributes :id, :email, :enterprise_id, :user_id, :code, :tags, :tag_list, :name,
-                 :allow_charges, :default_card_present?, :balance, :balance_status
+                 :allow_charges, :default_card_present?
 
       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
@@ -19,20 +17,6 @@ module Api
         customer_tag_list.join(",")
       end
 
-      def balance
-        Spree::Money.new(balance_value, currency: Spree::Config[:currency]).to_s
-      end
-
-      def balance_status
-        if balance_value.positive?
-          "credit_owed"
-        elsif balance_value.negative?
-          "balance_due"
-        else
-          ""
-        end
-      end
-
       def tags
         customer_tag_list.map do |tag|
           tag_rule_map = options[:tag_rule_mapping].andand[tag]
diff --git a/app/serializers/api/admin/customer_with_balance_serializer.rb b/app/serializers/api/admin/customer_with_balance_serializer.rb
new file mode 100644
index 0000000000..dfbb088991
--- /dev/null
+++ b/app/serializers/api/admin/customer_with_balance_serializer.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module Api
+  module Admin
+    class CustomerWithBalanceSerializer < CustomerSerializer
+      attributes :balance, :balance_status
+
+      delegate :balance_value, to: :object
+
+      def balance
+        Spree::Money.new(balance_value, currency: Spree::Config[:currency]).to_s
+      end
+
+      def balance_status
+        if balance_value.positive?
+          "credit_owed"
+        elsif balance_value.negative?
+          "balance_due"
+        else
+          ""
+        end
+      end
+    end
+  end
+end
diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb
index 1a4595ee19..9331da4e2c 100644
--- a/spec/controllers/admin/customers_controller_spec.rb
+++ b/spec/controllers/admin/customers_controller_spec.rb
@@ -41,6 +41,11 @@ module Admin
               expect(ActiveModel::ArraySerializer).to receive(:new)
               spree_get :index, params
             end
+
+            it 'includes the customer balance in the response' do
+              spree_get :index, params
+              expect(json_response.first["balance"]).to eq("$0.00")
+            end
           end
 
           context "and enterprise_id is not given in params" do
diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index 4ff41be6c7..37b0595a2d 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -97,9 +97,33 @@ feature 'Customers' do
 
       describe "for a shop with multiple customers" do
         before do
-          build_balance(customer1, managed_distributor1, 88)
-          build_balance(customer2, managed_distributor1, -99)
-          build_balance(customer4, managed_distributor1, 0)
+          create(
+            :order,
+            total: 0,
+            payment_total: 88,
+            distributor: managed_distributor1,
+            user: nil,
+            completed_at: Time.zone.now,
+            customer: customer1
+          )
+          create(
+            :order,
+            total: 99,
+            payment_total: 0,
+            distributor: managed_distributor1,
+            user: nil,
+            completed_at: Time.zone.now,
+            customer: customer2
+          )
+          create(
+            :order,
+            total: 0,
+            payment_total: 0,
+            distributor: managed_distributor1,
+            user: nil,
+            completed_at: Time.zone.now,
+            customer: customer4
+          )
 
           customer4.update enterprise: managed_distributor1
         end
@@ -121,14 +145,6 @@ feature 'Customers' do
             expect(page).to have_content "$0.00"
           end
         end
-
-        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
 
       it "allows updating of attributes" do
diff --git a/spec/serializers/api/admin/customer_with_balance_serializer_spec.rb b/spec/serializers/api/admin/customer_with_balance_serializer_spec.rb
new file mode 100644
index 0000000000..5c0c3f1df5
--- /dev/null
+++ b/spec/serializers/api/admin/customer_with_balance_serializer_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Api::Admin::CustomerWithBalanceSerializer do
+  let(:serialized_customer) { described_class.new(customer) }
+
+  describe '#balance' do
+    let(:customer) { double(Customer, balance_value: 1.2) }
+    let(:money) { instance_double(Spree::Money, to_s: "$1.20") }
+
+    before do
+      allow(Spree::Money).to receive(:new).with(1.2, currency: "AUD") { money }
+    end
+
+    it 'returns the balance_value as a money amount' do
+      expect(serialized_customer.balance).to eq("$1.20")
+    end
+  end
+
+  describe '#balance_status' do
+    context 'when the balance_value is positive' do
+      let(:customer) { double(Customer, balance_value: 1) }
+
+      it 'returns credit_owed' do
+        expect(serialized_customer.balance_status).to eq("credit_owed")
+      end
+    end
+
+    context 'when the balance_value is negative' do
+      let(:customer) { double(Customer, balance_value: -1) }
+
+      it 'returns credit_owed' do
+        expect(serialized_customer.balance_status).to eq("balance_due")
+      end
+    end
+
+    context 'when the balance_value is zero' do
+      let(:customer) { double(Customer, balance_value: 0) }
+
+      it 'returns credit_owed' do
+        expect(serialized_customer.balance_status).to eq("")
+      end
+    end
+  end
+end

From 93cdca85c64b119a387509094e914f88380ad632 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 24 Nov 2020 17:40:46 +0100
Subject: [PATCH 07/16] Return 0 balance when the customer has no orders

---
 app/services/customers_with_balance.rb       | 3 ++-
 spec/services/customers_with_balance_spec.rb | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index 13ab8d40a1..d64d9d5feb 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -24,7 +24,8 @@ class CustomersWithBalance
     <<-SQL.strip_heredoc
        SUM(
          CASE WHEN state = 'canceled' THEN payment_total
-         ELSE payment_total - total END
+              WHEN state IS NOT NULL THEN payment_total - total
+         ELSE 0 END
        ) AS balance_value
     SQL
   end
diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb
index c0b5a9bb00..e05f3ab8cd 100644
--- a/spec/services/customers_with_balance_spec.rb
+++ b/spec/services/customers_with_balance_spec.rb
@@ -117,9 +117,10 @@ describe CustomersWithBalance do
       end
     end
 
+    context 'when there are no orders' do
       it 'returns the customer balance' do
         customer = customers_with_balance.query.first
-        expect(customer.balance_value).to eq(payment_total - complete_orders_total)
+        expect(customer.balance_value).to eq(0)
       end
     end
   end

From 04930877ddd1a753969511f76577adce02aaa0a0 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 25 Nov 2020 16:07:48 +0100
Subject: [PATCH 08/16] Test that payment_total is stored after payment

CustomerWithBalance totally relies on `spree_orders.payment_total` so we
better cover it with tests.
---
 spec/models/spree/order_spec.rb | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 2361ff3a21..2a88499e6e 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -201,16 +201,27 @@ describe Spree::Order do
     let(:payment) { build(:payment) }
     before { allow(order).to receive_messages pending_payments: [payment], total: 10 }
 
-    it "should process the payments" do
-      expect(payment).to receive(:process!)
-      expect(order.process_payments!).to be_truthy
-    end
-
     it "should return false if no pending_payments available" do
       allow(order).to receive_messages pending_payments: []
       expect(order.process_payments!).to be_falsy
     end
 
+    context "when the processing is sucessful" do
+      it "should process the payments" do
+        expect(payment).to receive(:process!)
+        expect(order.process_payments!).to be_truthy
+      end
+
+      it "stores the payment total on the order" do
+        allow(payment).to receive(:process!)
+        allow(payment).to receive(:completed?).and_return(true)
+
+        order.process_payments!
+
+        expect(order.payment_total).to eq(payment.amount)
+      end
+    end
+
     context "when a payment raises a GatewayError" do
       before { expect(payment).to receive(:process!).and_raise(Spree::Core::GatewayError) }
 

From 539c5d067ac1becaf798144c2395b52505b7ae96 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 25 Nov 2020 17:38:51 +0100
Subject: [PATCH 09/16] Update order payment_total when voiding a payment

As is, `payment_total` is only increased after successfully processing
a payment and never updated. This inconsistency breaks
`CustomerWithBalance` which relies on it.

Needless to say that if we keep this denormalized column, we better make
it consistent. I investigated current Spree's master branch (709e686cc0)
and they also realized it was broken. Now `Payment` runs the following
from the `after_save` `update_order` callback.

```rb
order.updater.update_payment_total if completed? || void?
```

I also took the chance to rearrange tests a bit.
---
 .../admin/customers_controller_spec.rb        |  65 ++++++++++-
 spec/models/spree/payment_spec.rb             |   9 ++
 spec/services/customers_with_balance_spec.rb  | 105 +++++-------------
 3 files changed, 96 insertions(+), 83 deletions(-)

diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb
index 9331da4e2c..662f217f40 100644
--- a/spec/controllers/admin/customers_controller_spec.rb
+++ b/spec/controllers/admin/customers_controller_spec.rb
@@ -42,9 +42,68 @@ module Admin
               spree_get :index, params
             end
 
-            it 'includes the customer balance in the response' do
-              spree_get :index, params
-              expect(json_response.first["balance"]).to eq("$0.00")
+            context 'when the customer has no orders' do
+              it 'includes the customer balance in the response' do
+                spree_get :index, params
+                expect(json_response.first["balance"]).to eq("$0.00")
+              end
+            end
+
+            context 'when the customer has complete orders' do
+              let(:order) { create(:order, customer: customer, state: 'complete') }
+              let!(:line_item) { create(:line_item, order: order, price: 10.0) }
+
+              it 'includes the customer balance in the response' do
+                spree_get :index, params
+                expect(json_response.first["balance"]).to eq("$-10.00")
+              end
+            end
+
+            context 'when the customer has canceled orders' do
+              let(:order) { create(:order, customer: customer) }
+              let!(:line_item) { create(:line_item, order: order, price: 10.0) }
+              let!(:payment) { create(:payment, order: order, amount: order.total) }
+
+              before do
+                allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true)
+                order.process_payments!
+
+                order.update_attribute(:state, 'canceled')
+              end
+
+              it 'includes the customer balance in the response' do
+                spree_get :index, params
+                expect(json_response.first["balance"]).to eq("$10.00")
+              end
+            end
+
+            context 'when the customer has cart orders' do
+              let(:order) { create(:order, customer: customer, state: 'cart') }
+              let!(:line_item) { create(:line_item, order: order, price: 10.0) }
+
+              it 'includes the customer balance in the response' do
+                spree_get :index, params
+                expect(json_response.first["balance"]).to eq("$-10.00")
+              end
+            end
+
+            context 'when the customer has an order with a void payment' do
+              let(:order) { create(:order, customer: customer) }
+              let!(:line_item) { create(:line_item, order: order, price: 10.0) }
+              let!(:payment) { create(:payment, order: order, amount: order.total) }
+
+              before do
+                allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true)
+                order.process_payments!
+
+                payment.void_transaction!
+              end
+
+              it 'includes the customer balance in the response' do
+                expect(order.payment_total).to eq(0)
+                spree_get :index, params
+                expect(json_response.first["balance"]).to eq('$-10.00')
+              end
             end
           end
 
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 17c084e2a0..226855025f 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -584,6 +584,15 @@ describe Spree::Payment do
           )
         end
       end
+
+      context 'when the payment was completed but now void' do
+        let(:payment) { create(:payment, amount: 100, order: order, state: 'completed') }
+
+        it 'updates order payment total' do
+          payment.void
+          expect(order.payment_total).to eq 0
+        end
+      end
     end
 
     context "#build_source" do
diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb
index e05f3ab8cd..0edae9d1f0 100644
--- a/spec/services/customers_with_balance_spec.rb
+++ b/spec/services/customers_with_balance_spec.rb
@@ -12,28 +12,14 @@ describe CustomersWithBalance do
       let(:total) { 200.00 }
       let(:order_total) { 100.00 }
 
-      context 'when non-guest order' do
-        before do
-          create(:order, customer: customer, total: order_total, payment_total: 0)
-          create(:order, customer: customer, total: order_total, payment_total: 0)
-        end
-
-        it 'returns the customer balance' do
-          customer = customers_with_balance.query.first
-          expect(customer.balance_value).to eq(-total)
-        end
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0)
+        create(:order, customer: customer, total: order_total, payment_total: 0)
       end
 
-      context 'when guest order' do
-        before do
-          create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
-          create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
-        end
-
-        it 'returns the customer balance' do
-          customer = customers_with_balance.query.first
-          expect(customer.balance_value).to eq(-total)
-        end
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(-total)
       end
     end
 
@@ -42,34 +28,14 @@ describe CustomersWithBalance do
       let(:order_total) { 100.00 }
       let(:payment_total) { order_total }
 
-      context 'when non-guest order' do
-        before do
-          create(:order, customer: customer, total: order_total, payment_total: 0)
-          create(:order, customer: customer, total: order_total, payment_total: payment_total)
-        end
-
-        it 'returns the customer balance' do
-          customer = customers_with_balance.query.first
-          expect(customer.balance_value).to eq(payment_total - total)
-        end
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0)
+        create(:order, customer: customer, total: order_total, payment_total: payment_total)
       end
 
-      context 'when guest order' do
-        before do
-          create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
-          create(
-            :order,
-            customer: customer,
-            user: nil,
-            total: order_total,
-            payment_total: payment_total
-          )
-        end
-
-        it 'returns the customer balance' do
-          customer = customers_with_balance.query.first
-          expect(customer.balance_value).to eq(payment_total - total)
-        end
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(payment_total - total)
       end
     end
 
@@ -77,43 +43,22 @@ describe CustomersWithBalance do
       let(:total) { 200.00 }
       let(:order_total) { 100.00 }
       let(:payment_total) { 100.00 }
-      let(:complete_orders_total) { order_total }
+      let(:non_canceled_orders_total) { order_total }
 
-      context 'when non-guest order' do
-        before do
-          create(:order, customer: customer, total: order_total, payment_total: 0)
-          create(
-            :order,
-            customer: customer,
-            total: order_total,
-            payment_total: order_total,
-            state: 'canceled'
-          )
-        end
-
-        it 'returns the customer balance' do
-          customer = customers_with_balance.query.first
-          expect(customer.balance_value).to eq(payment_total - complete_orders_total)
-        end
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0)
+        create(
+          :order,
+          customer: customer,
+          total: order_total,
+          payment_total: order_total,
+          state: 'canceled'
+        )
       end
 
-      context 'when guest order' do
-        before do
-          create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
-          create(
-            :order,
-            customer: customer,
-            user: nil,
-            total: order_total,
-            payment_total: order_total,
-            state: 'canceled'
-          )
-        end
-
-        it 'returns the customer balance' do
-          customer = customers_with_balance.query.first
-          expect(customer.balance_value).to eq(payment_total - complete_orders_total)
-        end
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(payment_total - non_canceled_orders_total)
       end
     end
 

From 46d997406a9bf07e626986d422024b7852b41b60 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 26 Nov 2020 09:59:27 +0100
Subject: [PATCH 10/16] Filter out cart orders when fetching customers

We only care about non-cart orders and skipping carts, saves PostgreSQL
query planner to go through thousands of records in production use cases
(my food hub).

We go from

```sql
 ->  Index Scan using index_spree_orders_on_customer_id on spree_orders  (cost=0.42..12049.45 rows=152002 width=15) (actual time=0.015..11.703 rows=13867 loops=1)
```

to

```sql
->  Index Scan using index_spree_orders_on_customer_id on spree_orders  (cost=0.42..12429.46 rows=10802 width=15) (actual time=0.025..17.705 rows=9954 loops=1)
```
---
 app/services/customers_with_balance.rb        |  9 +++++-
 .../admin/customers_controller_spec.rb        |  4 +--
 spec/features/admin/customers_spec.rb         |  6 ++--
 spec/services/customers_with_balance_spec.rb  | 30 +++++++++++++++----
 4 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index d64d9d5feb..3cf667882a 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -8,7 +8,7 @@ class CustomersWithBalance
   def query
     Customer.of(enterprise_id).
       includes(:bill_address, :ship_address, user: :credit_cards).
-      joins("LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id").
+      joins(left_join_non_cart_orders).
       group("customers.id").
       select("customers.*").
       select(outstanding_balance)
@@ -29,4 +29,11 @@ class CustomersWithBalance
        ) AS balance_value
     SQL
   end
+
+  def left_join_non_cart_orders
+    <<-SQL.strip_heredoc
+      LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id
+        AND spree_orders.state != 'cart'
+    SQL
+  end
 end
diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb
index 662f217f40..e33dd651ee 100644
--- a/spec/controllers/admin/customers_controller_spec.rb
+++ b/spec/controllers/admin/customers_controller_spec.rb
@@ -83,12 +83,12 @@ module Admin
 
               it 'includes the customer balance in the response' do
                 spree_get :index, params
-                expect(json_response.first["balance"]).to eq("$-10.00")
+                expect(json_response.first["balance"]).to eq("$0.00")
               end
             end
 
             context 'when the customer has an order with a void payment' do
-              let(:order) { create(:order, customer: customer) }
+              let(:order) { create(:order, customer: customer, state: 'complete') }
               let!(:line_item) { create(:line_item, order: order, price: 10.0) }
               let!(:payment) { create(:payment, order: order, amount: order.total) }
 
diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index 37b0595a2d..f0c1d2c6fa 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -103,7 +103,7 @@ feature 'Customers' do
             payment_total: 88,
             distributor: managed_distributor1,
             user: nil,
-            completed_at: Time.zone.now,
+            state: 'complete',
             customer: customer1
           )
           create(
@@ -112,7 +112,7 @@ feature 'Customers' do
             payment_total: 0,
             distributor: managed_distributor1,
             user: nil,
-            completed_at: Time.zone.now,
+            state: 'complete',
             customer: customer2
           )
           create(
@@ -121,7 +121,7 @@ feature 'Customers' do
             payment_total: 0,
             distributor: managed_distributor1,
             user: nil,
-            completed_at: Time.zone.now,
+            state: 'complete',
             customer: customer4
           )
 
diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb
index 0edae9d1f0..0ad61fb8de 100644
--- a/spec/services/customers_with_balance_spec.rb
+++ b/spec/services/customers_with_balance_spec.rb
@@ -8,13 +8,30 @@ describe CustomersWithBalance do
   describe '#query' do
     let(:customer) { create(:customer) }
 
+    context 'when orders are in cart state' do
+      let(:total) { 200.00 }
+      let(:order_total) { 100.00 }
+
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0, state: 'cart')
+        create(:order, customer: customer, total: order_total, payment_total: 0, state: 'cart')
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(0)
+      end
+    end
+
     context 'when no orders where paid' do
       let(:total) { 200.00 }
       let(:order_total) { 100.00 }
 
       before do
-        create(:order, customer: customer, total: order_total, payment_total: 0)
-        create(:order, customer: customer, total: order_total, payment_total: 0)
+        order = create(:order, customer: customer, total: order_total, payment_total: 0)
+        order.update_attribute(:state, 'checkout')
+        order = create(:order, customer: customer, total: order_total, payment_total: 0)
+        order.update_attribute(:state, 'checkout')
       end
 
       it 'returns the customer balance' do
@@ -29,8 +46,10 @@ describe CustomersWithBalance do
       let(:payment_total) { order_total }
 
       before do
-        create(:order, customer: customer, total: order_total, payment_total: 0)
-        create(:order, customer: customer, total: order_total, payment_total: payment_total)
+        order = create(:order, customer: customer, total: order_total, payment_total: 0)
+        order.update_attribute(:state, 'checkout')
+        order = create(:order, customer: customer, total: order_total, payment_total: payment_total)
+        order.update_attribute(:state, 'checkout')
       end
 
       it 'returns the customer balance' do
@@ -46,7 +65,8 @@ describe CustomersWithBalance do
       let(:non_canceled_orders_total) { order_total }
 
       before do
-        create(:order, customer: customer, total: order_total, payment_total: 0)
+        order = create(:order, customer: customer, total: order_total, payment_total: 0)
+        order.update_attribute(:state, 'checkout')
         create(
           :order,
           customer: customer,

From 2e9bae0ea58410082dc9de2bb6abb00d34d089b3 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 26 Nov 2020 10:16:19 +0100
Subject: [PATCH 11/16] Move relation includes out of query object

This query object is meant to be reusable but those includes are
context-specific and will likely not be needed when reusing the query
elsewhere. If we keep them there, chances are next dev might not notice
it and will introduce a performance regression.
---
 app/controllers/admin/customers_controller.rb | 3 ++-
 app/services/customers_with_balance.rb        | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb
index 630ffd361b..7a8598e268 100644
--- a/app/controllers/admin/customers_controller.rb
+++ b/app/controllers/admin/customers_controller.rb
@@ -65,7 +65,8 @@ module Admin
 
     def collection
       if json_request? && params[:enterprise_id].present?
-        CustomersWithBalance.new(managed_enterprise_id).query
+        CustomersWithBalance.new(managed_enterprise_id).query.
+          includes(:bill_address, :ship_address, user: :credit_cards)
       else
         Customer.where('1=0')
       end
diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index 3cf667882a..12651ec14a 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -7,7 +7,6 @@ class CustomersWithBalance
 
   def query
     Customer.of(enterprise_id).
-      includes(:bill_address, :ship_address, user: :credit_cards).
       joins(left_join_non_cart_orders).
       group("customers.id").
       select("customers.*").

From 398467e7ed257cee3d462b1db9a51858180cb97f Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 10 Dec 2020 15:56:20 +0100
Subject: [PATCH 12/16] Hide new balance impl. under feature toggle

This makes it possible to deploy it without releasing it to users since
the toggle is not enabled for anyone.

It aims to make the balance calculation consistent across pages.
---
 app/controllers/admin/customers_controller.rb | 20 ++++++-
 ...omer_with_calculated_balance_serializer.rb | 28 ++++++++++
 app/services/customers_with_balance.rb        |  8 +--
 lib/open_food_network/feature_toggle.rb       | 10 +++-
 .../admin/customers_controller_spec.rb        | 55 +++++++++++++++++++
 spec/features/admin/customers_spec.rb         |  3 +
 6 files changed, 116 insertions(+), 8 deletions(-)
 create mode 100644 app/serializers/api/admin/customer_with_calculated_balance_serializer.rb

diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb
index 7a8598e268..4a2f4242e9 100644
--- a/app/controllers/admin/customers_controller.rb
+++ b/app/controllers/admin/customers_controller.rb
@@ -18,13 +18,21 @@ module Admin
         format.html
         format.json do
           render json: @collection,
-                 each_serializer: ::Api::Admin::CustomerWithBalanceSerializer,
+                 each_serializer: index_each_serializer,
                  tag_rule_mapping: tag_rule_mapping,
                  customer_tags: customer_tags_by_id
         end
       end
     end
 
+    def index_each_serializer
+      if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, spree_current_user)
+        ::Api::Admin::CustomerWithBalanceSerializer
+      else
+        ::Api::Admin::CustomerWithCalculatedBalanceSerializer
+      end
+    end
+
     def show
       render_as_json @customer, ams_prefix: params[:ams_prefix]
     end
@@ -65,13 +73,21 @@ module Admin
 
     def collection
       if json_request? && params[:enterprise_id].present?
-        CustomersWithBalance.new(managed_enterprise_id).query.
+        customers_relation.
           includes(:bill_address, :ship_address, user: :credit_cards)
       else
         Customer.where('1=0')
       end
     end
 
+    def customers_relation
+      if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, spree_current_user)
+        CustomersWithBalance.new(managed_enterprise_id).query
+      else
+        Customer.of(managed_enterprise_id)
+      end
+    end
+
     def managed_enterprise_id
       @managed_enterprise_id ||= Enterprise.managed_by(spree_current_user).
         select('enterprises.id').find_by(id: params[:enterprise_id])
diff --git a/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb b/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb
new file mode 100644
index 0000000000..6db568ed35
--- /dev/null
+++ b/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Api
+  module Admin
+    class CustomerWithCalculatedBalanceSerializer < CustomerSerializer
+      attributes :balance, :balance_status
+
+      def balance
+        Spree::Money.new(balance_value, currency: Spree::Config[:currency]).to_s
+      end
+
+      def balance_status
+        if balance_value.positive?
+          "credit_owed"
+        elsif balance_value.negative?
+          "balance_due"
+        else
+          ""
+        end
+      end
+
+      def balance_value
+        @balance_value ||=
+          OpenFoodNetwork::UserBalanceCalculator.new(object.email, object.enterprise).balance
+      end
+    end
+  end
+end
diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index 12651ec14a..3bf9c633ff 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -1,12 +1,12 @@
 # frozen_string_literal: true
 
 class CustomersWithBalance
-  def initialize(enterprise_id)
-    @enterprise_id = enterprise_id
+  def initialize(enterprise)
+    @enterprise = enterprise
   end
 
   def query
-    Customer.of(enterprise_id).
+    Customer.of(enterprise).
       joins(left_join_non_cart_orders).
       group("customers.id").
       select("customers.*").
@@ -15,7 +15,7 @@ class CustomersWithBalance
 
   private
 
-  attr_reader :enterprise_id
+  attr_reader :enterprise
 
   # Arel doesn't support CASE statements until v7.1.0 so we'll have to wait with SQL literals
   # a little longer. See https://github.com/rails/arel/pull/400 for details.
diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb
index d40a89b0db..12aaa90a03 100644
--- a/lib/open_food_network/feature_toggle.rb
+++ b/lib/open_food_network/feature_toggle.rb
@@ -36,12 +36,12 @@ module OpenFoodNetwork
     end
 
     def initialize
-      @features = Thread.current[:features]
+      @features = Thread.current[:features] || {}
     end
 
     def enabled?(feature_name, user)
       if user.present?
-        feature = features[feature_name]
+        feature = features.fetch(feature_name, NullFeature.new)
         feature.enabled?(user)
       else
         true?(env_variable_value(feature_name))
@@ -78,4 +78,10 @@ module OpenFoodNetwork
 
     attr_reader :users
   end
+
+  class NullFeature
+    def enabled?(_user)
+      false
+    end
+  end
 end
diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb
index e33dd651ee..b15b5efdd8 100644
--- a/spec/controllers/admin/customers_controller_spec.rb
+++ b/spec/controllers/admin/customers_controller_spec.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
+require 'open_food_network/user_balance_calculator'
 
 module Admin
   describe CustomersController, type: :controller do
@@ -42,6 +43,49 @@ module Admin
               spree_get :index, params
             end
 
+            context 'when the customer_balance feature is enabled' do
+              let(:customers_with_balance) { instance_double(CustomersWithBalance) }
+
+              before do
+                allow(OpenFoodNetwork::FeatureToggle)
+                  .to receive(:enabled?).with(:customer_balance, enterprise.owner) { true }
+              end
+
+              it 'calls CustomersWithBalance' do
+                allow(CustomersWithBalance)
+                  .to receive(:new).with(enterprise) { customers_with_balance }
+
+                expect(customers_with_balance).to receive(:query) { Customer.none }
+
+                spree_get :index, params
+              end
+
+              it 'serializes using CustomerWithBalanceSerializer' do
+                expect(Api::Admin::CustomerWithBalanceSerializer).to receive(:new)
+
+                spree_get :index, params
+              end
+            end
+
+            context 'when the customer_balance feature is not enabled' do
+              let(:calculator) do
+                instance_double(OpenFoodNetwork::UserBalanceCalculator, balance: 0)
+              end
+
+              it 'calls Customer.of' do
+                expect(Customer).to receive(:of).twice.with(enterprise) { Customer.none }
+
+                spree_get :index, params
+              end
+
+              it 'serializes calling the UserBalanceCalculator' do
+                expect(OpenFoodNetwork::UserBalanceCalculator)
+                  .to receive(:new).with(customer.email, customer.enterprise) { calculator }
+
+                spree_get :index, params
+              end
+            end
+
             context 'when the customer has no orders' do
               it 'includes the customer balance in the response' do
                 spree_get :index, params
@@ -53,6 +97,11 @@ module Admin
               let(:order) { create(:order, customer: customer, state: 'complete') }
               let!(:line_item) { create(:line_item, order: order, price: 10.0) }
 
+              before do
+                allow(OpenFoodNetwork::FeatureToggle)
+                  .to receive(:enabled?).with(:customer_balance, enterprise.owner) { true }
+              end
+
               it 'includes the customer balance in the response' do
                 spree_get :index, params
                 expect(json_response.first["balance"]).to eq("$-10.00")
@@ -65,6 +114,9 @@ module Admin
               let!(:payment) { create(:payment, order: order, amount: order.total) }
 
               before do
+                allow(OpenFoodNetwork::FeatureToggle)
+                  .to receive(:enabled?).with(:customer_balance, enterprise.owner) { true }
+
                 allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true)
                 order.process_payments!
 
@@ -93,6 +145,9 @@ module Admin
               let!(:payment) { create(:payment, order: order, amount: order.total) }
 
               before do
+                allow(OpenFoodNetwork::FeatureToggle)
+                  .to receive(:enabled?).with(:customer_balance, enterprise.owner) { true }
+
                 allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true)
                 order.process_payments!
 
diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index f0c1d2c6fa..47b2b11df2 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -97,6 +97,9 @@ feature 'Customers' do
 
       describe "for a shop with multiple customers" do
         before do
+          allow(OpenFoodNetwork::FeatureToggle)
+            .to receive(:enabled?).with(:customer_balance, user) { true }
+
           create(
             :order,
             total: 0,

From 5018e88a346db9fca2dd52fd678f6831ff20639b Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 16 Dec 2020 12:13:08 +0100
Subject: [PATCH 13/16] Consider all states prior to checkout in balance

---
 app/services/customers_with_balance.rb       | 16 +++++--
 spec/services/customers_with_balance_spec.rb | 48 ++++++++++++++++----
 2 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index 3bf9c633ff..d17f1997cb 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -7,7 +7,7 @@ class CustomersWithBalance
 
   def query
     Customer.of(enterprise).
-      joins(left_join_non_cart_orders).
+      joins(left_join_complete_orders).
       group("customers.id").
       select("customers.*").
       select(outstanding_balance)
@@ -29,10 +29,20 @@ class CustomersWithBalance
     SQL
   end
 
-  def left_join_non_cart_orders
+  def left_join_complete_orders
     <<-SQL.strip_heredoc
       LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id
-        AND spree_orders.state != 'cart'
+        AND #{complete_orders.to_sql}
     SQL
   end
+
+  def complete_orders
+    states_group = prior_to_completion_states.map { |state| Arel::Nodes.build_quoted(state) }
+    Arel::Nodes::NotIn.new(Spree::Order.arel_table[:state], states_group)
+  end
+
+  # All the states an order can be in before completing the checkout
+  def prior_to_completion_states
+    %w(cart address delivery payment)
+  end
 end
diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb
index 0ad61fb8de..5ff95a0057 100644
--- a/spec/services/customers_with_balance_spec.rb
+++ b/spec/services/customers_with_balance_spec.rb
@@ -7,11 +7,10 @@ describe CustomersWithBalance do
 
   describe '#query' do
     let(:customer) { create(:customer) }
+    let(:total) { 200.00 }
+    let(:order_total) { 100.00 }
 
     context 'when orders are in cart state' do
-      let(:total) { 200.00 }
-      let(:order_total) { 100.00 }
-
       before do
         create(:order, customer: customer, total: order_total, payment_total: 0, state: 'cart')
         create(:order, customer: customer, total: order_total, payment_total: 0, state: 'cart')
@@ -23,10 +22,43 @@ describe CustomersWithBalance do
       end
     end
 
-    context 'when no orders where paid' do
-      let(:total) { 200.00 }
-      let(:order_total) { 100.00 }
+    context 'when orders are in address state' do
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0, state: 'address')
+        create(:order, customer: customer, total: order_total, payment_total: 50, state: 'address')
+      end
 
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(0)
+      end
+    end
+
+    context 'when orders are in delivery state' do
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0, state: 'delivery')
+        create(:order, customer: customer, total: order_total, payment_total: 50, state: 'delivery')
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(0)
+      end
+    end
+
+    context 'when orders are in payment state' do
+      before do
+        create(:order, customer: customer, total: order_total, payment_total: 0, state: 'payment')
+        create(:order, customer: customer, total: order_total, payment_total: 50, state: 'payment')
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(0)
+      end
+    end
+
+    context 'when no orders where paid' do
       before do
         order = create(:order, customer: customer, total: order_total, payment_total: 0)
         order.update_attribute(:state, 'checkout')
@@ -41,8 +73,6 @@ describe CustomersWithBalance do
     end
 
     context 'when an order was paid' do
-      let(:total) { 200.00 }
-      let(:order_total) { 100.00 }
       let(:payment_total) { order_total }
 
       before do
@@ -59,8 +89,6 @@ describe CustomersWithBalance do
     end
 
     context 'when an order is canceled' do
-      let(:total) { 200.00 }
-      let(:order_total) { 100.00 }
       let(:payment_total) { 100.00 }
       let(:non_canceled_orders_total) { order_total }
 

From aa3fc71626cafd6152d916369850051f18f2a208 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 16 Dec 2020 12:46:37 +0100
Subject: [PATCH 14/16] Consider resumed & payment orders in balance

These order states are already taken care of because they fall in the
`WHEN state IS NOT NULL` case.
---
 app/services/customers_with_balance.rb       |  2 ++
 spec/services/customers_with_balance_spec.rb | 32 ++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index d17f1997cb..f24c06d408 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -29,6 +29,8 @@ class CustomersWithBalance
     SQL
   end
 
+  # The resulting orders are in states that belong after the checkout. Only these can be considered
+  # for a customer's balance.
   def left_join_complete_orders
     <<-SQL.strip_heredoc
       LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id
diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb
index 5ff95a0057..5bac659d23 100644
--- a/spec/services/customers_with_balance_spec.rb
+++ b/spec/services/customers_with_balance_spec.rb
@@ -110,6 +110,38 @@ describe CustomersWithBalance do
       end
     end
 
+    context 'when an order is resumed' do
+      let(:payment_total) { order_total }
+
+      before do
+        order = create(:order, customer: customer, total: order_total, payment_total: 0)
+        order.update_attribute(:state, 'checkout')
+        order = create(:order, customer: customer, total: order_total, payment_total: payment_total)
+        order.update_attribute(:state, 'resumed')
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(payment_total - total)
+      end
+    end
+
+    context 'when an order is in payment' do
+      let(:payment_total) { order_total }
+
+      before do
+        order = create(:order, customer: customer, total: order_total, payment_total: 0)
+        order.update_attribute(:state, 'checkout')
+        order = create(:order, customer: customer, total: order_total, payment_total: payment_total)
+        order.update_attribute(:state, 'payment')
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(payment_total - total)
+      end
+    end
+
     context 'when there are no orders' do
       it 'returns the customer balance' do
         customer = customers_with_balance.query.first

From dfed20d6342a342a58c4d212584e5096a20d6eaa Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 16 Dec 2020 19:02:07 +0100
Subject: [PATCH 15/16] Consider awaiting_return, returned in balance

These states are reached when the order is complete and shipped. An
admin can create a new return authorization, which will set the order in
`awaiting_return` state. It's only after, when we call
`return_authorization#receive` that the return authorization moves to
`received` state and the order to `returned`.

You can do so from the UI by editing the return authorization and
clicking on Receive. However, we didn't find any order in such state in
UK, FR and AU. The UX is quite obfuscated. That must be why no users
clicked on it.

The `returned` state cannot count for the balance as is, since some of
the products are returned to the shop. That's enough for now.
---
 app/services/customers_with_balance.rb       | 13 +++++++-
 spec/services/customers_with_balance_spec.rb | 33 ++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb
index f24c06d408..56a3c2d171 100644
--- a/app/services/customers_with_balance.rb
+++ b/app/services/customers_with_balance.rb
@@ -22,7 +22,7 @@ class CustomersWithBalance
   def outstanding_balance
     <<-SQL.strip_heredoc
        SUM(
-         CASE WHEN state = 'canceled' THEN payment_total
+         CASE WHEN state IN #{non_fulfilled_states_group.to_sql} THEN payment_total
               WHEN state IS NOT NULL THEN payment_total - total
          ELSE 0 END
        ) AS balance_value
@@ -43,8 +43,19 @@ class CustomersWithBalance
     Arel::Nodes::NotIn.new(Spree::Order.arel_table[:state], states_group)
   end
 
+  def non_fulfilled_states_group
+    states_group = non_fulfilled_states.map { |state| Arel::Nodes.build_quoted(state) }
+    Arel::Nodes::Grouping.new(states_group)
+  end
+
   # All the states an order can be in before completing the checkout
   def prior_to_completion_states
     %w(cart address delivery payment)
   end
+
+  # All the states of a complete order but that shouldn't count towards the balance. Those that the
+  # customer won't enjoy.
+  def non_fulfilled_states
+    %w(canceled returned)
+  end
 end
diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb
index 5bac659d23..fefd1d9090 100644
--- a/spec/services/customers_with_balance_spec.rb
+++ b/spec/services/customers_with_balance_spec.rb
@@ -142,6 +142,39 @@ describe CustomersWithBalance do
       end
     end
 
+    context 'when an order is awaiting_return' do
+      let(:payment_total) { order_total }
+
+      before do
+        order = create(:order, customer: customer, total: order_total, payment_total: 0)
+        order.update_attribute(:state, 'checkout')
+        order = create(:order, customer: customer, total: order_total, payment_total: payment_total)
+        order.update_attribute(:state, 'awaiting_return')
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(payment_total - total)
+      end
+    end
+
+    context 'when an order is returned' do
+      let(:payment_total) { order_total }
+      let(:non_returned_orders_total) { order_total }
+
+      before do
+        order = create(:order, customer: customer, total: order_total, payment_total: 0)
+        order.update_attribute(:state, 'checkout')
+        order = create(:order, customer: customer, total: order_total, payment_total: payment_total)
+        order.update_attribute(:state, 'returned')
+      end
+
+      it 'returns the customer balance' do
+        customer = customers_with_balance.query.first
+        expect(customer.balance_value).to eq(payment_total - non_returned_orders_total)
+      end
+    end
+
     context 'when there are no orders' do
       it 'returns the customer balance' do
         customer = customers_with_balance.query.first

From 33c72ecab021ddd74cf1896b80d2cf75ba7b627b Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 11 Jan 2021 15:55:34 +0100
Subject: [PATCH 16/16] Defend from nil BETA_TESTERS var

---
 lib/open_food_network/feature_toggle.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb
index 12aaa90a03..5ee51a82f9 100644
--- a/lib/open_food_network/feature_toggle.rb
+++ b/lib/open_food_network/feature_toggle.rb
@@ -31,6 +31,8 @@ module OpenFoodNetwork
     end
 
     def self.enable(feature_name, user_emails)
+      return unless user_emails.present?
+
       Thread.current[:features] ||= {}
       Thread.current[:features][feature_name] = Feature.new(user_emails)
     end