From 5815d1a4a49f6bf06d170f296a74b4d610419be4 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 15 Mar 2021 17:50:44 +0100
Subject: [PATCH 1/6] Make #outstanding_balance return an OrderBalance

This will let us branch by abstraction. All existing calls to
`#outstanding_balance` will go through `OrderBalance` hence, will
check the feature toggle.

Note that by default, `OrderBalance` will end up calling
`#old_outstanding_balance`. As the name states, that's exactly what
`#outstanding_balance` was so far. This means no consumers will see any
change in behavior. We just added on item in the call stack (sort of).
---
 app/helpers/order_helper.rb                   |   2 +-
 app/models/concerns/balance.rb                |  32 +++--
 app/models/order_balance.rb                   |   6 +-
 .../subscriptions/payment_setup_spec.rb       |  10 +-
 lib/open_food_network/payments_report.rb      |   4 +-
 .../payments_controller_refunds_spec.rb       |  36 ++---
 spec/features/admin/payments_spec.rb          |  13 +-
 spec/mailers/order_mailer_spec.rb             |   8 +-
 spec/mailers/subscription_mailer_spec.rb      |   8 +-
 spec/models/concerns/balance_spec.rb          |  28 ++--
 spec/models/order_balance_spec.rb             | 124 ++++++++++++++++--
 spec/models/spree/order_spec.rb               |   2 +-
 spec/models/spree/payment_spec.rb             |   4 +-
 13 files changed, 196 insertions(+), 81 deletions(-)

diff --git a/app/helpers/order_helper.rb b/app/helpers/order_helper.rb
index c6ab5807bb..e15b8acdbb 100644
--- a/app/helpers/order_helper.rb
+++ b/app/helpers/order_helper.rb
@@ -6,6 +6,6 @@ module OrderHelper
   end
 
   def outstanding_balance_label(order)
-    order.outstanding_balance.negative? ? t(:credit_owed) : t(:balance_due)
+    order.outstanding_balance.label
   end
 end
diff --git a/app/models/concerns/balance.rb b/app/models/concerns/balance.rb
index 91019d401c..794bb40f9f 100644
--- a/app/models/concerns/balance.rb
+++ b/app/models/concerns/balance.rb
@@ -7,13 +7,28 @@ require 'active_support/concern'
 module Balance
   FINALIZED_NON_SUCCESSFUL_STATES = %w(canceled returned).freeze
 
+  # Branches by the OrderBalance abstraction
+  def outstanding_balance
+    OrderBalance.new(self)
+  end
+
+  # This method is the one we're gradually replacing with `#new_outstanding_balance`. Having them
+  # separate enables us to choose which implementation we want depending on the context using
+  # a feature toggle. This avoids inconsistent behavior during that incremental refactoring.
+  #
+  # It is meant to be removed as soon as we get product approval that the new implementation has
+  # been working correctly in production.
+  def old_outstanding_balance
+    total - payment_total
+  end
+
   # Returns the order balance by considering the total as money owed to the order distributor aka.
-  # the shop, and as a positive balance of said enterprise. If the customer pays it all, they
+  # the shop, and as a positive balance of said enterprise. If the customer pays it all, the
   # distributor and customer are even.
   #
   # Note however, this is meant to be used only in the context of a single order object. When
   # working with a collection of orders, such an index controller action, please consider using
-  # `app/queries/oustanding_balance.rb` instead so we avoid potential N+1s.
+  # `app/queries/outstanding_balance.rb` instead so we avoid potential N+1s.
   def new_outstanding_balance
     if state.in?(FINALIZED_NON_SUCCESSFUL_STATES)
       -payment_total
@@ -22,22 +37,11 @@ module Balance
     end
   end
 
-  # This method is the one we're gradually replacing with `#new_outstanding_balance`. Having them
-  # separate enables us to choose which implementation we want depending on the context using
-  # a feature toggle. This avoids incosistent behavior across the app during that incremental
-  # refactoring.
-  #
-  # It is meant to be removed as soon as we get product approval that the new implementation has
-  # been working correctly in production.
-  def outstanding_balance
-    total - payment_total
-  end
-
   def outstanding_balance?
     !outstanding_balance.zero?
   end
 
   def display_outstanding_balance
-    Spree::Money.new(outstanding_balance, currency: currency)
+    outstanding_balance.amount
   end
 end
diff --git a/app/models/order_balance.rb b/app/models/order_balance.rb
index 58fb03a353..f9200d1bc7 100644
--- a/app/models/order_balance.rb
+++ b/app/models/order_balance.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class OrderBalance
+  delegate :zero?, :abs, :to_s, to: :to_f
+
   def initialize(order)
     @order = order
   end
@@ -17,12 +19,10 @@ class OrderBalance
     if customer_balance_enabled?
       order.new_outstanding_balance
     else
-      order.outstanding_balance
+      order.old_outstanding_balance
     end
   end
 
-  delegate :zero?, to: :to_f
-
   private
 
   attr_reader :order
diff --git a/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
index 32bf3874d0..0cb4ade9e5 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
@@ -9,7 +9,7 @@ module OrderManagement
       let(:payment_setup) { OrderManagement::Subscriptions::PaymentSetup.new(order) }
 
       describe "#call!" do
-        let!(:payment){ create(:payment, amount: 10) }
+        let!(:payment) { create(:payment, amount: 10) }
 
         context "when no pending payments are present" do
           let(:payment_method) { create(:payment_method) }
@@ -17,7 +17,7 @@ module OrderManagement
 
           before do
             allow(order).to receive(:pending_payments).once { [] }
-            allow(order).to receive(:outstanding_balance) { 5 }
+            allow(order).to receive(:old_outstanding_balance) { 5 }
             allow(order).to receive(:subscription) { subscription }
           end
 
@@ -31,14 +31,14 @@ module OrderManagement
           before { allow(order).to receive(:pending_payments).once { [payment] } }
 
           context "when the payment total doesn't match the outstanding balance on the order" do
-            before { allow(order).to receive(:outstanding_balance) { 5 } }
+            before { allow(order).to receive(:old_outstanding_balance) { 5 } }
             it "updates the payment total to reflect the outstanding balance" do
               expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
             end
           end
 
           context "when the payment total matches the outstanding balance on the order" do
-            before { allow(order).to receive(:outstanding_balance) { 10 } }
+            before { allow(order).to receive(:old_outstanding_balance) { 10 } }
 
             it "does nothing" do
               expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
@@ -51,7 +51,7 @@ module OrderManagement
           let!(:payment2) { create(:payment, order: order) }
 
           before do
-            allow(order).to receive(:outstanding_balance) { 7 }
+            allow(order).to receive(:old_outstanding_balance) { 7 }
             allow(order).to receive(:pending_payments).once { [payment1, payment2] }
           end
 
diff --git a/lib/open_food_network/payments_report.rb b/lib/open_food_network/payments_report.rb
index 4b88603abf..4f8cea18ae 100644
--- a/lib/open_food_network/payments_report.rb
+++ b/lib/open_food_network/payments_report.rb
@@ -100,7 +100,7 @@ module OpenFoodNetwork
          proc { |orders| orders.first.distributor.name },
          proc { |orders| orders.to_a.sum(&:item_total) },
          proc { |orders| orders.sum(&:ship_total) },
-         proc { |orders| orders.sum(&:outstanding_balance) },
+         proc { |orders| orders.sum { |order| order.outstanding_balance.to_f } },
          proc { |orders| orders.map(&:total).sum }]
       when "payment_totals"
         [proc { |orders| orders.first.payment_state },
@@ -124,7 +124,7 @@ module OpenFoodNetwork
              }.sum(&:amount)
            }
          },
-         proc { |orders| orders.sum(&:outstanding_balance) }]
+         proc { |orders| orders.sum { |order| order.outstanding_balance.to_f } }]
       else
         [proc { |payments| payments.first.order.payment_state },
          proc { |payments| payments.first.order.distributor.name },
diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb
index d7b67a7988..5ec3543894 100644
--- a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb
+++ b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb
@@ -45,12 +45,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "voids the payment" do
             order.reload
             expect(order.payment_total).to_not eq 0
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
             spree_put :fire, params
             expect(payment.reload.state).to eq 'void'
             order.reload
             expect(order.payment_total).to eq 0
-            expect(order.outstanding_balance).to_not eq 0
+            expect(order.outstanding_balance.to_f).to_not eq 0
           end
         end
 
@@ -64,12 +64,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "does not void the payment" do
             order.reload
             expect(order.payment_total).to_not eq 0
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
             spree_put :fire, params
             expect(payment.reload.state).to eq 'completed'
             order.reload
             expect(order.payment_total).to_not eq 0
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
             expect(flash[:error]).to eq "Bup-bow!"
           end
         end
@@ -104,12 +104,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "partially refunds the payment" do
             order.reload
             expect(order.payment_total).to eq order.total + 5
-            expect(order.outstanding_balance).to eq(-5)
+            expect(order.outstanding_balance.to_f).to eq(-5)
             spree_put :fire, params
             expect(payment.reload.state).to eq 'completed'
             order.reload
             expect(order.payment_total).to eq order.total
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
           end
         end
 
@@ -123,12 +123,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "does not void the payment" do
             order.reload
             expect(order.payment_total).to eq order.total + 5
-            expect(order.outstanding_balance).to eq(-5)
+            expect(order.outstanding_balance.to_f).to eq(-5)
             spree_put :fire, params
             expect(payment.reload.state).to eq 'completed'
             order.reload
             expect(order.payment_total).to eq order.total + 5
-            expect(order.outstanding_balance).to eq(-5)
+            expect(order.outstanding_balance.to_f).to eq(-5)
             expect(flash[:error]).to eq "Bup-bow!"
           end
         end
@@ -169,12 +169,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "voids the payment" do
             order.reload
             expect(order.payment_total).to_not eq 0
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
             spree_put :fire, params
             expect(payment.reload.state).to eq 'void'
             order.reload
             expect(order.payment_total).to eq 0
-            expect(order.outstanding_balance).to_not eq 0
+            expect(order.outstanding_balance.to_f).to_not eq 0
           end
         end
 
@@ -189,12 +189,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "does not void the payment" do
             order.reload
             expect(order.payment_total).to_not eq 0
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
             spree_put :fire, params
             expect(payment.reload.state).to eq 'completed'
             order.reload
             expect(order.payment_total).to_not eq 0
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
             expect(flash[:error]).to eq "Bup-bow!"
           end
         end
@@ -211,12 +211,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "can still void the payment" do
             order.reload
             expect(order.payment_total).to_not eq 0
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
             spree_put :fire, params
             expect(payment.reload.state).to eq 'void'
             order.reload
             expect(order.payment_total).to eq 0
-            expect(order.outstanding_balance).to_not eq 0
+            expect(order.outstanding_balance.to_f).to_not eq 0
           end
         end
       end
@@ -252,12 +252,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "partially refunds the payment" do
             order.reload
             expect(order.payment_total).to eq order.total + 5
-            expect(order.outstanding_balance).to eq(-5)
+            expect(order.outstanding_balance.to_f).to eq(-5)
             spree_put :fire, params
             expect(payment.reload.state).to eq 'completed'
             order.reload
             expect(order.payment_total).to eq order.total
-            expect(order.outstanding_balance).to eq 0
+            expect(order.outstanding_balance.to_f).to eq 0
           end
         end
 
@@ -271,12 +271,12 @@ describe Spree::Admin::PaymentsController, type: :controller do
           it "does not void the payment" do
             order.reload
             expect(order.payment_total).to eq order.total + 5
-            expect(order.outstanding_balance).to eq(-5)
+            expect(order.outstanding_balance.to_f).to eq(-5)
             spree_put :fire, params
             expect(payment.reload.state).to eq 'completed'
             order.reload
             expect(order.payment_total).to eq order.total + 5
-            expect(order.outstanding_balance).to eq(-5)
+            expect(order.outstanding_balance.to_f).to eq(-5)
             expect(flash[:error]).to eq "Bup-bow!"
           end
         end
diff --git a/spec/features/admin/payments_spec.rb b/spec/features/admin/payments_spec.rb
index 2c670c44ee..d6c5f38d66 100644
--- a/spec/features/admin/payments_spec.rb
+++ b/spec/features/admin/payments_spec.rb
@@ -10,10 +10,13 @@ feature '
 
   let(:order) { create(:completed_order_with_fees) }
 
-  scenario "visiting the payment form" do
-    login_as_admin_and_visit spree.new_admin_order_payment_path order
+  describe "payments/new" do
+    it "displays the order balance as the default payment amount" do
+      login_as_admin_and_visit spree.new_admin_order_payment_path order
 
-    expect(page).to have_content "New Payment"
+      expect(page).to have_content I18n.t(:new_payment)
+      expect(page).to have_field(:payment_amount, with: order.outstanding_balance.to_f)
+    end
   end
 
   context "with sensitive payment fee" do
@@ -26,10 +29,10 @@ feature '
       payment_method.save!
     end
 
-    scenario "visiting the payment form" do
+    it "renders the new payment page" do
       login_as_admin_and_visit spree.new_admin_order_payment_path order
 
-      expect(page).to have_content "New Payment"
+      expect(page).to have_content I18n.t(:new_payment)
     end
   end
 end
diff --git a/spec/mailers/order_mailer_spec.rb b/spec/mailers/order_mailer_spec.rb
index d23b920c55..ed71ebc308 100644
--- a/spec/mailers/order_mailer_spec.rb
+++ b/spec/mailers/order_mailer_spec.rb
@@ -15,7 +15,7 @@ describe Spree::OrderMailer do
     end
 
     context 'when the order has outstanding balance' do
-      before { allow(order).to receive(:outstanding_balance) { 123 } }
+      before { allow(order).to receive(:old_outstanding_balance) { 123 } }
 
       it 'renders the amount as money' do
         expect(email.body).to include('$123')
@@ -23,7 +23,7 @@ describe Spree::OrderMailer do
     end
 
     context 'when the order has no outstanding balance' do
-      before { allow(order).to receive(:outstanding_balance) { 0 } }
+      before { allow(order).to receive(:old_outstanding_balance) { 0 } }
 
       it 'displays the payment status' do
         expect(email.body).to include(I18n.t(:email_payment_not_paid))
@@ -58,7 +58,7 @@ describe Spree::OrderMailer do
     end
 
     context 'when the order has outstanding balance' do
-      before { allow(order).to receive(:outstanding_balance) { 123 } }
+      before { allow(order).to receive(:old_outstanding_balance) { 123 } }
 
       it 'renders the amount as money' do
         expect(email.body).to include('$123')
@@ -66,7 +66,7 @@ describe Spree::OrderMailer do
     end
 
     context 'when the order has no outstanding balance' do
-      before { allow(order).to receive(:outstanding_balance) { 0 } }
+      before { allow(order).to receive(:old_outstanding_balance) { 0 } }
 
       it 'displays the payment status' do
         expect(email.body).to include(I18n.t(:email_payment_not_paid))
diff --git a/spec/mailers/subscription_mailer_spec.rb b/spec/mailers/subscription_mailer_spec.rb
index 0cd77c047a..3ea46f0d5a 100644
--- a/spec/mailers/subscription_mailer_spec.rb
+++ b/spec/mailers/subscription_mailer_spec.rb
@@ -88,7 +88,7 @@ describe SubscriptionMailer, type: :mailer do
     end
 
     context 'when the order has outstanding balance' do
-      before { allow(order).to receive(:outstanding_balance) { 123 } }
+      before { allow(order).to receive(:old_outstanding_balance) { 123 } }
 
       it 'renders the amount as money' do
         expect(email.body).to include('$123')
@@ -96,7 +96,7 @@ describe SubscriptionMailer, type: :mailer do
     end
 
     context 'when the order has no outstanding balance' do
-      before { allow(order).to receive(:outstanding_balance) { 0 } }
+      before { allow(order).to receive(:old_outstanding_balance) { 0 } }
 
       it 'displays the payment status' do
         expect(email.body).to include(I18n.t(:email_payment_not_paid))
@@ -141,7 +141,7 @@ describe SubscriptionMailer, type: :mailer do
     end
 
     context 'when the order has outstanding balance' do
-      before { allow(order).to receive(:outstanding_balance) { 123 } }
+      before { allow(order).to receive(:old_outstanding_balance) { 123 } }
 
       it 'renders the amount as money' do
         expect(email.body).to include('$123')
@@ -149,7 +149,7 @@ describe SubscriptionMailer, type: :mailer do
     end
 
     context 'when the order has no outstanding balance' do
-      before { allow(order).to receive(:outstanding_balance) { 0 } }
+      before { allow(order).to receive(:old_outstanding_balance) { 0 } }
 
       it 'displays the payment status' do
         expect(email.body).to include(I18n.t(:email_payment_not_paid))
diff --git a/spec/models/concerns/balance_spec.rb b/spec/models/concerns/balance_spec.rb
index 331e521442..a277e46245 100644
--- a/spec/models/concerns/balance_spec.rb
+++ b/spec/models/concerns/balance_spec.rb
@@ -133,12 +133,12 @@ describe Balance do
     end
   end
 
-  context "#outstanding_balance" do
+  context "#old_outstanding_balance" do
     context 'when orders are in cart state' do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'cart') }
 
       it 'returns the order balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -146,7 +146,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'address') }
 
       it 'returns the order balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -154,7 +154,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'delivery') }
 
       it 'returns the order balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -162,7 +162,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'payment') }
 
       it 'returns the order balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -170,7 +170,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') }
 
       it 'returns the customer balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -178,7 +178,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') }
 
       it 'returns the customer balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -186,7 +186,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'canceled') }
 
       it 'returns the customer balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -194,7 +194,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'resumed') }
 
       it 'returns the customer balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -202,7 +202,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'payment') }
 
       it 'returns the customer balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -210,7 +210,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'awaiting_return') }
 
       it 'returns the customer balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -218,7 +218,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'returned') }
 
       it 'returns the balance' do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -226,7 +226,7 @@ describe Balance do
       let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') }
 
       it "returns positive" do
-        expect(order.outstanding_balance).to eq(100 - 10)
+        expect(order.old_outstanding_balance).to eq(100 - 10)
       end
     end
 
@@ -234,7 +234,7 @@ describe Balance do
       let(:order) { create(:order, total: 8.20, payment_total: 10.20, state: 'complete') }
 
       it "returns negative amount" do
-        expect(order.outstanding_balance).to eq(-2.00)
+        expect(order.old_outstanding_balance).to eq(-2.00)
       end
     end
   end
diff --git a/spec/models/order_balance_spec.rb b/spec/models/order_balance_spec.rb
index 93d5ca5d2c..0fb11a202c 100644
--- a/spec/models/order_balance_spec.rb
+++ b/spec/models/order_balance_spec.rb
@@ -16,7 +16,7 @@ describe OrderBalance do
 
       context 'when the balance is postive' do
         before do
-          allow(order).to receive(:outstanding_balance) { 10 }
+          allow(order).to receive(:old_outstanding_balance) { 10 }
         end
 
         it "returns 'balance due'" do
@@ -26,7 +26,7 @@ describe OrderBalance do
 
       context 'when the balance is negative' do
         before do
-          allow(order).to receive(:outstanding_balance) { -10 }
+          allow(order).to receive(:old_outstanding_balance) { -10 }
         end
 
         it "returns 'credit owed'" do
@@ -36,7 +36,7 @@ describe OrderBalance do
 
       context 'when the balance is zero' do
         before do
-          allow(order).to receive(:outstanding_balance) { 0 }
+          allow(order).to receive(:old_outstanding_balance) { 0 }
         end
 
         it "returns 'balance due'" do
@@ -86,7 +86,7 @@ describe OrderBalance do
   describe '#amount' do
     context 'when the customer_balance feature is disabled' do
       before do
-        allow(order).to receive(:outstanding_balance) { 10 }
+        allow(order).to receive(:old_outstanding_balance) { 10 }
       end
 
       before do
@@ -124,7 +124,7 @@ describe OrderBalance do
 
       context 'when the balance is zero' do
         before do
-          allow(order).to receive(:outstanding_balance) { 0 }
+          allow(order).to receive(:old_outstanding_balance) { 0 }
         end
 
         it 'returns true' do
@@ -134,7 +134,7 @@ describe OrderBalance do
 
       context 'when the balance is positive' do
         before do
-          allow(order).to receive(:outstanding_balance) { 10 }
+          allow(order).to receive(:old_outstanding_balance) { 10 }
         end
 
         it 'returns false' do
@@ -144,7 +144,7 @@ describe OrderBalance do
 
       context 'when the balance is negative' do
         before do
-          allow(order).to receive(:outstanding_balance) { -10 }
+          allow(order).to receive(:old_outstanding_balance) { -10 }
         end
 
         it 'returns false' do
@@ -199,7 +199,7 @@ describe OrderBalance do
       end
 
       it 'calls #outstanding_balance' do
-        expect(order).to receive(:outstanding_balance)
+        expect(order).to receive(:old_outstanding_balance)
         order_balance.to_f
       end
     end
@@ -216,4 +216,112 @@ describe OrderBalance do
       end
     end
   end
+
+  describe '#abs' do
+    context 'when the customer_balance feature is disabled' do
+      before do
+        allow(OpenFoodNetwork::FeatureToggle)
+          .to receive(:enabled?).with(:customer_balance, user) { false }
+      end
+
+      context 'when the balance is zero' do
+        before do
+          allow(order).to receive(:old_outstanding_balance) { 0 }
+        end
+
+        it 'returns its absolute value' do
+          expect(order_balance.abs).to eq(0)
+        end
+      end
+
+      context 'when the balance is positive' do
+        before do
+          allow(order).to receive(:old_outstanding_balance) { 10 }
+        end
+
+        it 'returns its absolute value' do
+          expect(order_balance.abs).to eq(10)
+        end
+      end
+
+      context 'when the balance is negative' do
+        before do
+          allow(order).to receive(:old_outstanding_balance) { -10 }
+        end
+
+        it 'returns its absolute value' do
+          expect(order_balance.abs).to eq(10)
+        end
+      end
+    end
+
+    context 'when the customer_balance feature is enabled' do
+      before do
+        allow(OpenFoodNetwork::FeatureToggle)
+          .to receive(:enabled?).with(:customer_balance, user) { true }
+      end
+
+      context 'when the balance is zero' do
+        before do
+          allow(order).to receive(:new_outstanding_balance) { 0 }
+        end
+
+        it 'returns its absolute value' do
+          expect(order_balance.abs).to eq(0)
+        end
+      end
+
+      context 'when the balance is positive' do
+        before do
+          allow(order).to receive(:new_outstanding_balance) { 10 }
+        end
+
+        it 'returns its absolute value' do
+          expect(order_balance.abs).to eq(10)
+        end
+      end
+
+      context 'when the balance is negative' do
+        before do
+          allow(order).to receive(:new_outstanding_balance) { -10 }
+        end
+
+        it 'returns its absolute value' do
+          expect(order_balance.abs).to eq(10)
+        end
+      end
+    end
+  end
+
+  describe '#to_s' do
+    context 'when the customer_balance feature is disabled' do
+      before do
+        allow(OpenFoodNetwork::FeatureToggle)
+          .to receive(:enabled?).with(:customer_balance, user) { false }
+      end
+
+      before do
+        allow(order).to receive(:old_outstanding_balance) { 10 }
+      end
+
+      it 'returns the balance as a string' do
+        expect(order_balance.to_s).to eq('10')
+      end
+    end
+
+    context 'when the customer_balance feature is enabled' do
+      before do
+        allow(OpenFoodNetwork::FeatureToggle)
+          .to receive(:enabled?).with(:customer_balance, user) { true }
+      end
+
+      before do
+        allow(order).to receive(:new_outstanding_balance) { 10 }
+      end
+
+      it 'returns the balance as a string' do
+        expect(order_balance.to_s).to eq('10')
+      end
+    end
+  end
 end
diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 6dbd4764c4..4fdab0c1f2 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -310,7 +310,7 @@ describe Spree::Order do
 
   context "#display_outstanding_balance" do
     it "returns the value as a spree money" do
-      allow(order).to receive(:outstanding_balance) { 10.55 }
+      allow(order).to receive(:old_outstanding_balance) { 10.55 }
       expect(order.display_outstanding_balance).to eq Spree::Money.new(10.55)
     end
   end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 64d9c04afe..9abb417f65 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -382,7 +382,7 @@ describe Spree::Payment do
           end
 
           it "resulting payment should have correct values" do
-            allow(payment.order).to receive(:outstanding_balance) { 100 }
+            allow(payment.order).to receive(:old_outstanding_balance) { 100 }
             allow(payment).to receive(:credit_allowed) { 10 }
 
             offsetting_payment = payment.credit!
@@ -402,7 +402,7 @@ describe Spree::Payment do
             end
 
             it 'lets the new payment to be saved' do
-              allow(payment.order).to receive(:outstanding_balance) { 100 }
+              allow(payment.order).to receive(:old_outstanding_balance) { 100 }
               allow(payment).to receive(:credit_allowed) { 10 }
 
               offsetting_payment = payment.credit!

From 5082b6dc99d6b96a91d7a93fac3835917935b15f Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 15 Mar 2021 18:28:01 +0100
Subject: [PATCH 2/6] Make BalanceCalculator use old balance calculation

This class is currently used and it gets skipped when the
:customer_balance toggle is enabled, so there's no point on abstracting
the balance with `OrderBalance`. It'll never go through its
`#new_outstanding_balance` branch and it'll be removed once we active
that toggle to everyone.
---
 lib/open_food_network/user_balance_calculator.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/open_food_network/user_balance_calculator.rb b/lib/open_food_network/user_balance_calculator.rb
index 98af1600f9..5f0fe64f34 100644
--- a/lib/open_food_network/user_balance_calculator.rb
+++ b/lib/open_food_network/user_balance_calculator.rb
@@ -6,7 +6,7 @@ module OpenFoodNetwork
     end
 
     def balance
-      -completed_orders.to_a.sum(&:outstanding_balance)
+      -completed_orders.to_a.sum(&:old_outstanding_balance)
     end
 
     private

From 976227555517b3b4ccf64e4720df7f0dd293c47d Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 16 Mar 2021 13:32:41 +0100
Subject: [PATCH 3/6] Use old_outstanding_balance on disabled toggle

No need to go through the `OrderBalance` in this context since we
already check for the toggle.
---
 app/serializers/api/order_serializer.rb                       | 2 +-
 .../order_management/reports/bulk_coop/bulk_coop_report.rb    | 2 +-
 .../reports/bulk_coop/bulk_coop_report_spec.rb                | 2 +-
 spec/serializers/api/order_serializer_spec.rb                 | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/serializers/api/order_serializer.rb b/app/serializers/api/order_serializer.rb
index f5e9b001d8..dcd38aa651 100644
--- a/app/serializers/api/order_serializer.rb
+++ b/app/serializers/api/order_serializer.rb
@@ -11,7 +11,7 @@ module Api
       if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, object.user)
         -object.balance_value
       else
-        object.outstanding_balance
+        object.old_outstanding_balance
       end
     end
 
diff --git a/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb b/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb
index 1f658b189a..5c54b492d9 100644
--- a/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb
+++ b/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb
@@ -171,7 +171,7 @@ module OrderManagement
           if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, @user)
             unique_orders(line_items).sum(&:new_outstanding_balance)
           else
-            unique_orders(line_items).sum(&:outstanding_balance)
+            unique_orders(line_items).sum(&:old_outstanding_balance)
           end
         end
 
diff --git a/engines/order_management/spec/services/order_management/reports/bulk_coop/bulk_coop_report_spec.rb b/engines/order_management/spec/services/order_management/reports/bulk_coop/bulk_coop_report_spec.rb
index f667c8fdc6..0fd9d5d0b5 100644
--- a/engines/order_management/spec/services/order_management/reports/bulk_coop/bulk_coop_report_spec.rb
+++ b/engines/order_management/spec/services/order_management/reports/bulk_coop/bulk_coop_report_spec.rb
@@ -195,7 +195,7 @@ describe OrderManagement::Reports::BulkCoop::BulkCoopReport do
       end
 
       it 'calls #outstanding_balance' do
-        expect_any_instance_of(Spree::Order).to receive(:outstanding_balance)
+        expect_any_instance_of(Spree::Order).to receive(:old_outstanding_balance)
         subject.send(:customer_payments_amount_owed, [line_item])
       end
     end
diff --git a/spec/serializers/api/order_serializer_spec.rb b/spec/serializers/api/order_serializer_spec.rb
index d635cf0ada..4d515d6c7b 100644
--- a/spec/serializers/api/order_serializer_spec.rb
+++ b/spec/serializers/api/order_serializer_spec.rb
@@ -44,12 +44,12 @@ describe Api::OrderSerializer do
       end
     end
 
-    context 'when the customer_balance is not enabled' do
+    context 'when the customer_balance is disabled' do
       before do
         allow(OpenFoodNetwork::FeatureToggle)
           .to receive(:enabled?).with(:customer_balance, order.user) { false }
 
-        allow(order).to receive(:outstanding_balance).and_return(123.0)
+        allow(order).to receive(:old_outstanding_balance).and_return(123.0)
       end
 
       it 'calls #outstanding_balance on the object' do

From f494b720f8791caf28b4d7e70ec61ce69082f41b Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 16 Mar 2021 13:40:55 +0100
Subject: [PATCH 4/6] Fix spec

---
 spec/features/admin/customers_spec.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index b115d0f98d..87fa500497 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -133,6 +133,11 @@ feature 'Customers' do
         context "with an additional negative payment (or refund)" do
           let!(:payment2) { create(:payment, order: order1, state: 'completed', payment_method: payment_method, response_code: 'pi_123', amount: -25.00) }
 
+          before do
+            order1.user = user
+            order1.save!
+          end
+
           it "displays an updated customer balance" do
             visit spree.admin_order_payments_path order1
             expect(page).to have_content "$#{payment2.amount}"

From 1a1552a6eda589eab9dfaaaf63e679a424c82e90 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 17 Mar 2021 16:48:51 +0100
Subject: [PATCH 5/6] Make OrderUpdater use new balance implementation

In this class we properly calculated the balance taking into account the
orders in cancellation state so we need to use the new implementation if
we don't want to introduce a regression by using
`#old_oustanding_balance`.

I was initially a bit dubious because this method was checking
`order.payments` as well but now I see that was redundant. Is on
`payment_total` to take into account whether or not the order is paid or
any other payment related details.
---
 .../app/services/order_management/order/updater.rb  | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb
index 4aac1f7843..b391592577 100644
--- a/engines/order_management/app/services/order_management/order/updater.rb
+++ b/engines/order_management/app/services/order_management/order/updater.rb
@@ -157,8 +157,7 @@ module OrderManagement
       def infer_payment_state_from_balance
         # This part added so that we don't need to override
         # order.outstanding_balance
-        balance = order.outstanding_balance
-        balance = -1 * order.payment_total if canceled_and_paid_for?
+        balance = order.new_outstanding_balance
 
         infer_state(balance)
       end
@@ -184,20 +183,10 @@ module OrderManagement
         order.state_changed('payment')
       end
 
-      # Taken from order.outstanding_balance in Spree 2.4
-      # See: https://github.com/spree/spree/commit/7b264acff7824f5b3dc6651c106631d8f30b147a
-      def canceled_and_paid_for?
-        order.canceled? && paid?
-      end
-
       def canceled_and_not_paid_for?
         order.state == 'canceled' && order.payment_total.zero?
       end
 
-      def paid?
-        payments.present? && !payments.completed.empty?
-      end
-
       def failed_payments?
         payments.present? && payments.valid.empty?
       end

From c7b85a35911d39867dc672a0ccfbf694bae57d7c Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 22 Mar 2021 11:36:23 +0100
Subject: [PATCH 6/6] Sum balances in Payments report implementing `#+`

This avoids consumers of `OrderBalance` having to couple with the inner
details of this abstraction, which makes the code more changeable.
---
 app/models/order_balance.rb                   |   4 +
 lib/open_food_network/payments_report.rb      |   4 +-
 .../admin/reports/payments_report_spec.rb     | 100 +++++++++++++-----
 spec/models/order_balance_spec.rb             |  34 ++++++
 4 files changed, 115 insertions(+), 27 deletions(-)

diff --git a/app/models/order_balance.rb b/app/models/order_balance.rb
index f9200d1bc7..83110c23f3 100644
--- a/app/models/order_balance.rb
+++ b/app/models/order_balance.rb
@@ -23,6 +23,10 @@ class OrderBalance
     end
   end
 
+  def +(other)
+    to_f + other.to_f
+  end
+
   private
 
   attr_reader :order
diff --git a/lib/open_food_network/payments_report.rb b/lib/open_food_network/payments_report.rb
index 4f8cea18ae..4b88603abf 100644
--- a/lib/open_food_network/payments_report.rb
+++ b/lib/open_food_network/payments_report.rb
@@ -100,7 +100,7 @@ module OpenFoodNetwork
          proc { |orders| orders.first.distributor.name },
          proc { |orders| orders.to_a.sum(&:item_total) },
          proc { |orders| orders.sum(&:ship_total) },
-         proc { |orders| orders.sum { |order| order.outstanding_balance.to_f } },
+         proc { |orders| orders.sum(&:outstanding_balance) },
          proc { |orders| orders.map(&:total).sum }]
       when "payment_totals"
         [proc { |orders| orders.first.payment_state },
@@ -124,7 +124,7 @@ module OpenFoodNetwork
              }.sum(&:amount)
            }
          },
-         proc { |orders| orders.sum { |order| order.outstanding_balance.to_f } }]
+         proc { |orders| orders.sum(&:outstanding_balance) }]
       else
         [proc { |payments| payments.first.order.payment_state },
          proc { |payments| payments.first.order.distributor.name },
diff --git a/spec/features/admin/reports/payments_report_spec.rb b/spec/features/admin/reports/payments_report_spec.rb
index b618bae352..11c828593e 100644
--- a/spec/features/admin/reports/payments_report_spec.rb
+++ b/spec/features/admin/reports/payments_report_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 describe "Payments Reports" do
   include AuthenticationHelper
 
-  let!(:order) do
+  let(:order) do
     create(
       :order_with_distributor,
       state: 'complete',
@@ -13,37 +13,87 @@ describe "Payments Reports" do
       order_cycle: order_cycle
     )
   end
-  let(:order_cycle) { create(:simple_order_cycle) }
-  let!(:line_item) do
-    create(:line_item_with_shipment, order: order, product: product)
+  let(:other_order) do
+    create(
+      :order_with_distributor,
+      state: 'complete',
+      completed_at: Time.zone.now,
+      order_cycle: order_cycle,
+      distributor: order.distributor
+    )
   end
+  let(:order_cycle) { create(:simple_order_cycle) }
   let(:product) { create(:product, supplier: supplier) }
   let(:supplier) { create(:supplier_enterprise) }
 
-  before { login_as_admin }
+  before do
+    create(:line_item_with_shipment, order: order, product: product)
+    create(:line_item_with_shipment, order: other_order, product: product)
 
-  it 'shows orders with payment state, their balance and totals' do
-    visit spree.payments_admin_reports_path
+    login_as_admin
+  end
 
-    select I18n.t(:report_itemised_payment), from: "report_type"
-    find("[type='submit']").click
+  context "when choosing itemised payments report type" do
+    it "shows orders with payment state, their balance and totals" do
+      visit spree.payments_admin_reports_path
 
-    expect(page.find("#listing_orders thead tr").text).to eq([
-      I18n.t(:report_header_payment_state),
-      I18n.t(:report_header_distributor),
-      I18n.t(:report_header_product_total_price, currency: currency_symbol),
-      I18n.t(:report_header_shipping_total_price, currency: currency_symbol),
-      I18n.t(:report_header_outstanding_balance_price, currency: currency_symbol),
-      I18n.t(:report_header_total_price, currency: currency_symbol)
-    ].join(" "))
+      select I18n.t(:report_itemised_payment), from: "report_type"
+      find("[type='submit']").click
 
-    expect(page.find("#listing_orders tbody tr").text).to eq([
-      order.payment_state,
-      order.distributor.name,
-      order.item_total.to_f,
-      order.ship_total.to_f,
-      order.outstanding_balance.to_f,
-      order.total.to_f
-    ].join(" "))
+      expect(page.find("#listing_orders thead tr").text).to eq([
+        I18n.t(:report_header_payment_state),
+        I18n.t(:report_header_distributor),
+        I18n.t(:report_header_product_total_price, currency: currency_symbol),
+        I18n.t(:report_header_shipping_total_price, currency: currency_symbol),
+        I18n.t(:report_header_outstanding_balance_price, currency: currency_symbol),
+        I18n.t(:report_header_total_price, currency: currency_symbol)
+      ].join(" "))
+
+      expect(page.find("#listing_orders tbody tr").text).to eq([
+        order.payment_state,
+        order.distributor.name,
+        order.item_total.to_f + other_order.item_total.to_f,
+        order.ship_total.to_f + other_order.ship_total.to_f,
+        order.outstanding_balance.to_f + other_order.outstanding_balance.to_f,
+        order.total.to_f + other_order.total.to_f
+      ].join(" "))
+    end
+  end
+
+  context 'when choosing payment totals report type' do
+    let(:paypal) { create(:payment_method, name: "PayPal") }
+    let!(:paypal_payment) { create(:payment, order: order, payment_method: paypal, state: "completed", amount: 5) }
+
+    let(:eft) { create(:payment_method, name: "EFT") }
+    let!(:eft_payment) { create(:payment, order: other_order, payment_method: eft, state: "completed", amount: 6) }
+
+    it 'shows orders with payment state, their balance and and payment totals' do
+      visit spree.payments_admin_reports_path
+
+      select I18n.t(:report_payment_totals), from: "report_type"
+      find("[type='submit']").click
+
+      expect(page.find("#listing_orders thead tr").text).to eq([
+        I18n.t(:report_header_payment_state),
+        I18n.t(:report_header_distributor),
+        I18n.t(:report_header_product_total_price, currency: currency_symbol),
+        I18n.t(:report_header_shipping_total_price, currency: currency_symbol),
+        I18n.t(:report_header_total_price, currency: currency_symbol),
+        I18n.t(:report_header_eft_price, currency: currency_symbol),
+        I18n.t(:report_header_paypal_price, currency: currency_symbol),
+        I18n.t(:report_header_outstanding_balance_price, currency: currency_symbol),
+      ].join(" "))
+
+      expect(page.find("#listing_orders tbody tr").text).to eq([
+        order.payment_state,
+        order.distributor.name,
+        order.item_total.to_f + other_order.item_total.to_f,
+        order.ship_total.to_f + other_order.ship_total.to_f,
+        order.total.to_f + other_order.total.to_f,
+        eft_payment.amount.to_f,
+        paypal_payment.amount.to_f,
+        order.outstanding_balance.to_f + other_order.outstanding_balance.to_f,
+      ].join(" "))
+    end
   end
 end
diff --git a/spec/models/order_balance_spec.rb b/spec/models/order_balance_spec.rb
index 0fb11a202c..a4ab2ff66d 100644
--- a/spec/models/order_balance_spec.rb
+++ b/spec/models/order_balance_spec.rb
@@ -324,4 +324,38 @@ describe OrderBalance do
       end
     end
   end
+
+  describe '#+' do
+    let(:other_order_balance) { described_class.new(order) }
+
+    context 'when the customer_balance feature is disabled' do
+      before do
+        allow(OpenFoodNetwork::FeatureToggle)
+          .to receive(:enabled?).with(:customer_balance, user) { false }
+      end
+
+      before do
+        allow(order).to receive(:old_outstanding_balance) { 10 }
+      end
+
+      it 'returns the sum of balances' do
+        expect(order_balance + other_order_balance).to eq(20.0)
+      end
+    end
+
+    context 'when the customer_balance feature is enabled' do
+      before do
+        allow(OpenFoodNetwork::FeatureToggle)
+          .to receive(:enabled?).with(:customer_balance, user) { true }
+      end
+
+      before do
+        allow(order).to receive(:new_outstanding_balance) { 10 }
+      end
+
+      it 'returns the balance as a string' do
+        expect(order_balance + other_order_balance).to eq(20.0)
+      end
+    end
+  end
 end