From 5bca7d1f8f538f317880ada15598f11c1e0ae913 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 14 Apr 2021 11:55:46 +0200
Subject: [PATCH 1/2] Remove the old balance's code branch

This keeps the `OrderBalance` abstraction but removes the old code now
that the feature is enabled for all users in all instances and there are
no bugs reported. It's become dead code.
---
 .rubocop_manual_todo.yml                      |   1 -
 .rubocop_todo.yml                             |   1 -
 app/controllers/admin/customers_controller.rb |  20 +-
 app/controllers/spree/users_controller.rb     |   6 +-
 app/models/concerns/balance.rb                |  12 +-
 app/models/order_balance.rb                   |  10 +-
 ...omer_with_calculated_balance_serializer.rb |  28 --
 app/serializers/api/order_serializer.rb       |   8 +-
 config/initializers/feature_toggles.rb        |   5 -
 .../reports/bulk_coop/bulk_coop_report.rb     |   6 +-
 .../bulk_coop/bulk_coop_report_spec.rb        |  25 +-
 .../order_cycle_management_report.rb          |  45 +-
 .../user_balance_calculator.rb                |  18 -
 .../admin/customers_controller_spec.rb        |  12 -
 .../spree/admin/reports_controller_spec.rb    |  25 +-
 .../spree/users_controller_spec.rb            |  19 +-
 spec/features/admin/customers_spec.rb         |   3 -
 .../order_cycle_management_report_spec.rb     |  33 +-
 .../user_balance_calculator_spec.rb           | 149 -------
 spec/models/concerns/balance_spec.rb          | 106 -----
 spec/models/order_balance_spec.rb             | 416 ++++--------------
 spec/models/spree/payment_spec.rb             |   2 +-
 22 files changed, 139 insertions(+), 811 deletions(-)
 delete mode 100644 app/serializers/api/admin/customer_with_calculated_balance_serializer.rb
 delete mode 100644 config/initializers/feature_toggles.rb
 delete mode 100644 lib/open_food_network/user_balance_calculator.rb
 delete mode 100644 spec/lib/open_food_network/user_balance_calculator_spec.rb

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 740c3171aa..db05464b57 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -249,7 +249,6 @@ Layout/LineLength:
   - spec/lib/open_food_network/products_and_inventory_report_spec.rb
   - spec/lib/open_food_network/scope_variant_to_hub_spec.rb
   - spec/lib/open_food_network/tag_rule_applicator_spec.rb
-  - spec/lib/open_food_network/user_balance_calculator_spec.rb
   - spec/lib/open_food_network/users_and_enterprises_report_spec.rb
   - spec/lib/open_food_network/xero_invoices_report_spec.rb
   - spec/lib/spree/core/calculated_adjustments_spec.rb
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 75197eaba2..db04a52e17 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1213,7 +1213,6 @@ Style/FrozenStringLiteralComment:
     - 'lib/open_food_network/scope_variants_for_search.rb'
     - 'lib/open_food_network/spree_api_key_loader.rb'
     - 'lib/open_food_network/tag_rule_applicator.rb'
-    - 'lib/open_food_network/user_balance_calculator.rb'
     - 'lib/open_food_network/users_and_enterprises_report.rb'
     - 'lib/open_food_network/xero_invoices_report.rb'
     - 'lib/spree/authentication_helpers.rb'
diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb
index f20c75e853..e655b0ae8e 100644
--- a/app/controllers/admin/customers_controller.rb
+++ b/app/controllers/admin/customers_controller.rb
@@ -18,21 +18,13 @@ module Admin
         format.html
         format.json do
           render json: @collection,
-                 each_serializer: index_each_serializer,
+                 each_serializer: ::Api::Admin::CustomerWithBalanceSerializer,
                  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
@@ -70,7 +62,7 @@ module Admin
 
     def collection
       if json_request? && params[:enterprise_id].present?
-        customers_relation.
+        CustomersWithBalance.new(managed_enterprise_id).query.
           includes(
             :enterprise,
             { bill_address: [:state, :country] },
@@ -82,14 +74,6 @@ module Admin
       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/controllers/spree/users_controller.rb b/app/controllers/spree/users_controller.rb
index 93350cd187..a591e20064 100644
--- a/app/controllers/spree/users_controller.rb
+++ b/app/controllers/spree/users_controller.rb
@@ -60,11 +60,7 @@ module Spree
     private
 
     def orders_collection
-      if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, spree_current_user)
-        CompleteOrdersWithBalance.new(@user).query
-      else
-        @user.orders.where(state: 'complete').order('completed_at desc')
-      end
+      CompleteOrdersWithBalance.new(@user).query
     end
 
     def load_object
diff --git a/app/models/concerns/balance.rb b/app/models/concerns/balance.rb
index a305285622..651a439417 100644
--- a/app/models/concerns/balance.rb
+++ b/app/models/concerns/balance.rb
@@ -12,22 +12,12 @@ module 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, 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
+  # working with a collection of orders, such as an index controller action, please consider using
   # `app/queries/outstanding_balance.rb` instead so we avoid potential N+1s.
   def new_outstanding_balance
     if state.in?(FINALIZED_NON_SUCCESSFUL_STATES)
diff --git a/app/models/order_balance.rb b/app/models/order_balance.rb
index 4b7f27d2f8..1e09940808 100644
--- a/app/models/order_balance.rb
+++ b/app/models/order_balance.rb
@@ -16,11 +16,7 @@ class OrderBalance
   end
 
   def amount
-    if customer_balance_enabled?
-      order.new_outstanding_balance
-    else
-      order.old_outstanding_balance
-    end
+    order.new_outstanding_balance
   end
 
   def +(other)
@@ -30,8 +26,4 @@ class OrderBalance
   private
 
   attr_reader :order
-
-  def customer_balance_enabled?
-    OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, order.user)
-  end
 end
diff --git a/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb b/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb
deleted file mode 100644
index 6db568ed35..0000000000
--- a/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-# 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/serializers/api/order_serializer.rb b/app/serializers/api/order_serializer.rb
index dcd38aa651..f741675318 100644
--- a/app/serializers/api/order_serializer.rb
+++ b/app/serializers/api/order_serializer.rb
@@ -7,12 +7,10 @@ module Api
 
     has_many :payments, serializer: Api::PaymentSerializer
 
+    # This method relies on `balance_value` as a computed DB column. See `CompleteOrdersWithBalance`
+    # for reference.
     def outstanding_balance
-      if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, object.user)
-        -object.balance_value
-      else
-        object.old_outstanding_balance
-      end
+      -object.balance_value
     end
 
     def payments
diff --git a/config/initializers/feature_toggles.rb b/config/initializers/feature_toggles.rb
deleted file mode 100644
index 0aa6aa07d4..0000000000
--- a/config/initializers/feature_toggles.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-require 'open_food_network/feature_toggle'
-
-OpenFoodNetwork::FeatureToggle.enable(:customer_balance) do |user|
-  true
-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 5c54b492d9..9b34c4a0fc 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
@@ -168,11 +168,7 @@ module OrderManagement
         end
 
         def customer_payments_amount_owed(line_items)
-          if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, @user)
-            unique_orders(line_items).sum(&:new_outstanding_balance)
-          else
-            unique_orders(line_items).sum(&:old_outstanding_balance)
-          end
+          unique_orders(line_items).sum(&:new_outstanding_balance)
         end
 
         def customer_payments_amount_paid(line_items)
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 0fd9d5d0b5..8fdfc4e2d8 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
@@ -176,28 +176,9 @@ describe OrderManagement::Reports::BulkCoop::BulkCoopReport do
     let!(:line_item) { create(:line_item) }
     let(:order) { line_item.order }
 
-    context 'when the customer_balance feature is enabled' do
-      before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { true }
-      end
-
-      it 'calls #new_outstanding_balance' do
-        expect_any_instance_of(Spree::Order).to receive(:new_outstanding_balance)
-        subject.send(:customer_payments_amount_owed, [line_item])
-      end
-    end
-
-    context 'when the customer_balance feature is disabled' do
-      before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { false }
-      end
-
-      it 'calls #outstanding_balance' do
-        expect_any_instance_of(Spree::Order).to receive(:old_outstanding_balance)
-        subject.send(:customer_payments_amount_owed, [line_item])
-      end
+    it 'calls #new_outstanding_balance' do
+      expect_any_instance_of(Spree::Order).to receive(:new_outstanding_balance)
+      subject.send(:customer_payments_amount_owed, [line_item])
     end
   end
 end
diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index 2e562c876a..c8b71f242a 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -1,4 +1,4 @@
-require 'open_food_network/user_balance_calculator'
+# frozen_string_literal: true
 
 module OpenFoodNetwork
   class OrderCycleManagementReport
@@ -46,34 +46,21 @@ module OpenFoodNetwork
     end
 
     def search
-      if FeatureToggle.enabled?(:customer_balance, @user)
-        Spree::Order.
-          finalized.
-          not_state(:canceled).
-          distributed_by_user(@user).
-          managed_by(@user).
-          search(params[:q])
-      else
-        Spree::Order.
-          complete.
-          where("spree_orders.state != ?", :canceled).
-          distributed_by_user(@user).
-          managed_by(@user).
-          search(params[:q])
-      end
+      Spree::Order.
+        finalized.
+        not_state(:canceled).
+        distributed_by_user(@user).
+        managed_by(@user).
+        search(params[:q])
     end
 
     def orders
-      if FeatureToggle.enabled?(:customer_balance, @user)
-        search_result = search.result.order(:completed_at)
-        orders_with_balance = OutstandingBalance.new(search_result).
-          query.
-          select('spree_orders.*')
+      search_result = search.result.order(:completed_at)
+      orders_with_balance = OutstandingBalance.new(search_result).
+        query.
+        select('spree_orders.*')
 
-        filter(orders_with_balance)
-      else
-        filter search.result
-      end
+      filter(orders_with_balance)
     end
 
     def table_items
@@ -92,12 +79,10 @@ module OpenFoodNetwork
 
     private
 
+    # This method relies on `balance_value` as a computed DB column. See `CompleteOrdersWithBalance`
+    # for reference.
     def balance(order)
-      if FeatureToggle.enabled?(:customer_balance, @user)
-        order.balance_value
-      else
-        UserBalanceCalculator.new(order.email, order.distributor).balance
-      end
+      order.balance_value
     end
 
     def payment_method_row(order)
diff --git a/lib/open_food_network/user_balance_calculator.rb b/lib/open_food_network/user_balance_calculator.rb
deleted file mode 100644
index 5f0fe64f34..0000000000
--- a/lib/open_food_network/user_balance_calculator.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-module OpenFoodNetwork
-  class UserBalanceCalculator
-    def initialize(email, distributor)
-      @email = email
-      @distributor = distributor
-    end
-
-    def balance
-      -completed_orders.to_a.sum(&:old_outstanding_balance)
-    end
-
-    private
-
-    def completed_orders
-      Spree::Order.where(distributor_id: @distributor, email: @email).complete.not_state(:canceled)
-    end
-  end
-end
diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb
index b80128f47e..a63c023651 100644
--- a/spec/controllers/admin/customers_controller_spec.rb
+++ b/spec/controllers/admin/customers_controller_spec.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require 'open_food_network/user_balance_calculator'
 
 module Admin
   describe CustomersController, type: :controller do
@@ -70,11 +69,6 @@ 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
                 get :index, params: params
                 expect(json_response.first["balance"]).to eq("$-10.00")
@@ -87,9 +81,6 @@ 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!
 
@@ -118,9 +109,6 @@ 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/controllers/spree/admin/reports_controller_spec.rb b/spec/controllers/spree/admin/reports_controller_spec.rb
index 386008e0be..4ae4e0cf13 100644
--- a/spec/controllers/spree/admin/reports_controller_spec.rb
+++ b/spec/controllers/spree/admin/reports_controller_spec.rb
@@ -295,23 +295,18 @@ describe Spree::Admin::ReportsController, type: :controller do
   context 'Order Cycle Management' do
     let!(:present_objects) { [orderA1, orderA2, orderB1, orderB2] }
 
-    context 'when the customer_balance feature is enabled' do
-      before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, kind_of(Spree::User)) { true }
+    before do
+      controller_login_as_enterprise_user [coordinator1]
+    end
 
-        controller_login_as_enterprise_user [coordinator1]
-      end
+    it 'renders the delivery report' do
+      spree_post :order_cycle_management, {
+        q: { completed_at_lt: 1.day.ago },
+        shipping_method_in: [ "123" ], # We just need to search for shipping methods
+        report_type: "delivery",
+      }
 
-      it 'renders the delivery report' do
-        spree_post :order_cycle_management, {
-          q: { completed_at_lt: 1.day.ago },
-          shipping_method_in: [ "123" ], # We just need to search for shipping methods
-          report_type: "delivery",
-        }
-
-        expect(response).to have_http_status(:ok)
-      end
+      expect(response).to have_http_status(:ok)
     end
   end
 
diff --git a/spec/controllers/spree/users_controller_spec.rb b/spec/controllers/spree/users_controller_spec.rb
index fc0d7ae5be..4115860b33 100644
--- a/spec/controllers/spree/users_controller_spec.rb
+++ b/spec/controllers/spree/users_controller_spec.rb
@@ -21,6 +21,8 @@ describe Spree::UsersController, type: :controller do
     let(:orders) { assigns(:orders) }
     let(:shops) { Enterprise.where(id: orders.pluck(:distributor_id)) }
 
+    let(:outstanding_balance) { instance_double(OutstandingBalance) }
+
     before do
       allow(controller).to receive(:spree_current_user) { u1 }
     end
@@ -43,20 +45,11 @@ describe Spree::UsersController, type: :controller do
       expect(orders).not_to include d1o3
     end
 
-    context 'when the customer_balance feature is enabled' do
-      let(:outstanding_balance) { instance_double(OutstandingBalance) }
+    it 'calls OutstandingBalance' do
+      allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance)
+      expect(outstanding_balance).to receive(:query) { Spree::Order.none }
 
-      before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, controller.spree_current_user) { true }
-      end
-
-      it 'calls OutstandingBalance' do
-        allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance)
-        expect(outstanding_balance).to receive(:query) { Spree::Order.none }
-
-        spree_get :show
-      end
+      spree_get :show
     end
   end
 
diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index 87fa500497..f6ff3689f0 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -104,9 +104,6 @@ feature 'Customers' do
         let!(:payment1) { create(:payment, order: order1, state: 'completed', payment_method: payment_method, response_code: 'pi_123', amount: 88.00) }
 
         before do
-          allow(OpenFoodNetwork::FeatureToggle)
-            .to receive(:enabled?).with(:customer_balance, user) { true }
-
           customer4.update enterprise: managed_distributor1
         end
 
diff --git a/spec/lib/open_food_network/order_cycle_management_report_spec.rb b/spec/lib/open_food_network/order_cycle_management_report_spec.rb
index 7e6afa240b..3beb2bd985 100644
--- a/spec/lib/open_food_network/order_cycle_management_report_spec.rb
+++ b/spec/lib/open_food_network/order_cycle_management_report_spec.rb
@@ -16,6 +16,15 @@ module OpenFoodNetwork
       end
 
       describe "fetching orders" do
+        let(:customers_with_balance) { instance_double(CustomersWithBalance) }
+
+        it 'calls the OutstandingBalance query object' do
+          outstanding_balance = instance_double(OutstandingBalance, query: Spree::Order.none)
+          expect(OutstandingBalance).to receive(:new).and_return(outstanding_balance)
+
+          subject.orders
+        end
+
         it "fetches completed orders" do
           o1 = create(:order)
           o2 = create(:order, completed_at: 1.day.ago, state: 'complete')
@@ -27,27 +36,11 @@ module OpenFoodNetwork
           expect(subject.orders).to eq([order])
         end
 
-        context 'when the customer_balance feature is enabled' do
-          let(:customers_with_balance) { instance_double(CustomersWithBalance) }
+        it 'orders them by id' do
+          order1 = create(:order, completed_at: 1.day.ago, state: 'complete')
+          order2 = create(:order, completed_at: 2.days.ago, state: 'complete')
 
-          before do
-            allow(OpenFoodNetwork::FeatureToggle)
-              .to receive(:enabled?).with(:customer_balance, anything) { true }
-          end
-
-          it 'calls OutstandingBalance query object' do
-            outstanding_balance = instance_double(OutstandingBalance, query: Spree::Order.none)
-            expect(OutstandingBalance).to receive(:new).and_return(outstanding_balance)
-
-            subject.orders
-          end
-
-          it 'orders them by id' do
-            order1 = create(:order, completed_at: 1.day.ago, state: 'complete')
-            order2 = create(:order, completed_at: 2.days.ago, state: 'complete')
-
-            expect(subject.orders.pluck(:id)).to eq([order2.id, order1.id])
-          end
+          expect(subject.orders.pluck(:id)).to eq([order2.id, order1.id])
         end
 
         it "does not show cancelled orders" do
diff --git a/spec/lib/open_food_network/user_balance_calculator_spec.rb b/spec/lib/open_food_network/user_balance_calculator_spec.rb
deleted file mode 100644
index 36ba738e55..0000000000
--- a/spec/lib/open_food_network/user_balance_calculator_spec.rb
+++ /dev/null
@@ -1,149 +0,0 @@
-# frozen_string_literal: true
-
-require 'open_food_network/user_balance_calculator'
-require 'spec_helper'
-
-module OpenFoodNetwork
-  describe UserBalanceCalculator do
-    describe "finding the account balance of a user with a hub" do
-      let!(:user1) { create(:user) }
-      let!(:hub1) { create(:distributor_enterprise) }
-
-      let!(:o1) {
-        create(:order_with_totals_and_distribution, :completed,
-               user: user1, distributor: hub1,
-               completed_at: 1.day.ago)
-      } # total=13 (10 + 3 shipping fee)
-      let!(:o2) {
-        create(:order_with_totals_and_distribution, :completed,
-               user: user1, distributor: hub1,
-               completed_at: 1.day.ago)
-      } # total=13 (10 + 3 shipping fee)
-      let!(:p1) {
-        create(:payment, order: o1, amount: 15.00,
-                         state: "completed")
-      }
-      let!(:p2) {
-        create(:payment, order: o2, amount: 2.00,
-                         state: "completed")
-      }
-
-      before do
-        # Sanity check the order
-        expect(o1.total).to eq 13
-        expect(o1.ready_to_ship?).to eq true
-        expect(o1.paid?).to eq true
-      end
-
-      it "finds the correct balance for this email and enterprise" do
-        expect(UserBalanceCalculator.new(o1.email, hub1).balance).to eq(-9) # = 15 + 2 - 13 - 13
-      end
-
-      context "with another hub" do
-        let!(:hub2) { create(:distributor_enterprise) }
-        let!(:o3) {
-          create(:order_with_totals_and_distribution, :completed,
-                 user: user1, distributor: hub2,
-                 completed_at: 1.day.ago)
-        } # total=13 (10 + 3 shipping fee)
-        let!(:p3) {
-          create(:payment, order: o3, amount: 15.00,
-                           state: "completed")
-        }
-
-        it "does not find the balance for other enterprises" do
-          expect(UserBalanceCalculator.new(o3.email, hub2).balance).to eq(2) # = 15 - 13
-        end
-      end
-
-      context "with another user" do
-        let!(:user2) { create(:user) }
-        let!(:o4) {
-          create(:order_with_totals_and_distribution, :completed,
-                 user: user2, distributor: hub1,
-                 completed_at: 1.day.ago)
-        } # total=13 (10 + 3 shipping fee)
-        let!(:p3) {
-          create(:payment, order: o4, amount: 20.00,
-                           state: "completed")
-        }
-
-        it "does not find the balance for other users" do
-          expect(UserBalanceCalculator.new(o4.email, hub1).balance).to eq(7) # = 20 - 13
-        end
-      end
-
-      context "with canceled orders" do
-        let!(:o4) {
-          create(:order_with_totals_and_distribution, :completed,
-                 user: user1, distributor: hub1,
-                 completed_at: 1.day.ago, state: "canceled")
-        } # total=13 (10 + 3 shipping fee)
-        let!(:p4) {
-          create(:payment, order: o4, amount: 20.00,
-                           state: "completed")
-        }
-
-        it "does not include canceled orders in the balance" do
-          expect(UserBalanceCalculator.new(o4.email, hub1).balance).to eq(-9) # = 15 + 2 - 13 - 13
-        end
-      end
-
-      context "with void payments" do
-        let!(:o4) {
-          create(:order_with_totals_and_distribution, :completed,
-                 user: user1, distributor: hub1,
-                 completed_at: 1.day.ago)
-        } # total=13 (10 + 3 shipping fee)
-        let!(:p4) {
-          create(:payment, order: o4, amount: 20.00,
-                           state: "void")
-        }
-
-        it "does not include void in the balance" do
-          expect(UserBalanceCalculator.new(o4.email, hub1).balance).to eq(-22) # = 15 + 2 - 13 - 13 - 10
-        end
-      end
-
-      context "with invalid payments" do
-        let!(:o4) {
-          create(:order_with_totals_and_distribution, :completed,
-                 user: user1, distributor: hub1,
-                 completed_at: 1.day.ago)
-        } # total=13 (10 + 3 shipping fee)
-        let!(:p4) {
-          create(:payment, order: o4, amount: 20.00,
-                           state: "invalid")
-        }
-
-        it "does not include invalid payments in the balance" do
-          expect(UserBalanceCalculator.new(o4.email, hub1).balance).to eq(-22) # = 15 + 2 - 13 - 13 - 10
-        end
-      end
-
-      context "with multiple payments on single order" do
-        let!(:o4) {
-          create(:order_with_totals_and_distribution, :completed,
-                 user: user1, distributor: hub1,
-                 completed_at: 1.day.ago)
-        } # total=13 (10 + 3 shipping fee)
-        let!(:p4) {
-          create(:payment, order: o4, amount: 4.00,
-                           state: "completed")
-        }
-        let!(:p5) {
-          create(:payment, order: o4, amount: 5.00,
-                           state: "completed")
-        }
-        let!(:p6) {
-          create(:payment, order: o4, amount: 6.00,
-                           state: "completed")
-        }
-
-        it "includes orders with multiple payments in the balance" do
-          expect(UserBalanceCalculator.new(o4.email, hub1).balance).to eq(-7) # = 15 + 2 + 4 + 5 + 6 - 13 - 13 - 10
-        end
-      end
-    end
-  end
-end
diff --git a/spec/models/concerns/balance_spec.rb b/spec/models/concerns/balance_spec.rb
index a277e46245..b723284456 100644
--- a/spec/models/concerns/balance_spec.rb
+++ b/spec/models/concerns/balance_spec.rb
@@ -132,110 +132,4 @@ describe Balance do
       end
     end
   end
-
-  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.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when orders are in address state' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'address') }
-
-      it 'returns the order balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when orders are in delivery state' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'delivery') }
-
-      it 'returns the order balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when orders are in payment state' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'payment') }
-
-      it 'returns the order balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when no orders where paid' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') }
-
-      it 'returns the customer balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when an order was paid' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') }
-
-      it 'returns the customer balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when an order is canceled' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'canceled') }
-
-      it 'returns the customer balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when an order is resumed' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'resumed') }
-
-      it 'returns the customer balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when an order is in payment' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'payment') }
-
-      it 'returns the customer balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when an order is awaiting_return' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'awaiting_return') }
-
-      it 'returns the customer balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when an order is returned' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'returned') }
-
-      it 'returns the balance' do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when payment_total is less than total' do
-      let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') }
-
-      it "returns positive" do
-        expect(order.old_outstanding_balance).to eq(100 - 10)
-      end
-    end
-
-    context 'when payment_total is greater than total' do
-      let(:order) { create(:order, total: 8.20, payment_total: 10.20, state: 'complete') }
-
-      it "returns negative amount" do
-        expect(order.old_outstanding_balance).to eq(-2.00)
-      end
-    end
-  end
 end
diff --git a/spec/models/order_balance_spec.rb b/spec/models/order_balance_spec.rb
index f024969074..bad8998733 100644
--- a/spec/models/order_balance_spec.rb
+++ b/spec/models/order_balance_spec.rb
@@ -8,426 +8,174 @@ describe OrderBalance do
   let(:user) { order.user }
 
   describe '#label' do
-    context 'when the customer_balance feature is disabled' do
+    context 'when the balance is positive' do
       before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { false }
+        allow(order).to receive(:new_outstanding_balance) { 10 }
       end
 
-      context 'when the balance is positive' do
-        before do
-          allow(order).to receive(:old_outstanding_balance) { 10 }
-        end
-
-        it "returns 'balance due'" do
-          expect(order_balance.label).to eq(I18n.t(:balance_due))
-        end
-      end
-
-      context 'when the balance is negative' do
-        before do
-          allow(order).to receive(:old_outstanding_balance) { -10 }
-        end
-
-        it "returns 'credit owed'" do
-          expect(order_balance.label).to eq(I18n.t(:credit_owed))
-        end
-      end
-
-      context 'when the balance is zero' do
-        before do
-          allow(order).to receive(:old_outstanding_balance) { 0 }
-        end
-
-        it "returns 'balance due'" do
-          expect(order_balance.label).to eq(I18n.t(:balance_due))
-        end
+      it "returns 'balance due'" do
+        expect(order_balance.label).to eq(I18n.t(:balance_due))
       end
     end
 
-    context 'when the customer_balance feature is enabled' do
+    context 'when the balance is negative' do
       before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { true }
+        allow(order).to receive(:new_outstanding_balance) { -10 }
       end
 
-      context 'when the balance is positive' do
-        before do
-          allow(order).to receive(:new_outstanding_balance) { 10 }
-        end
+      it "returns 'credit owed'" do
+        expect(order_balance.label).to eq(I18n.t(:credit_owed))
+      end
+    end
 
-        it "returns 'balance due'" do
-          expect(order_balance.label).to eq(I18n.t(:balance_due))
-        end
+    context 'when the balance is zero' do
+      before do
+        allow(order).to receive(:new_outstanding_balance) { 0 }
       end
 
-      context 'when the balance is negative' do
-        before do
-          allow(order).to receive(:new_outstanding_balance) { -10 }
-        end
-
-        it "returns 'credit owed'" do
-          expect(order_balance.label).to eq(I18n.t(:credit_owed))
-        end
-      end
-
-      context 'when the balance is zero' do
-        before do
-          allow(order).to receive(:new_outstanding_balance) { 0 }
-        end
-
-        it "returns 'balance due'" do
-          expect(order_balance.label).to eq(I18n.t(:balance_due))
-        end
+      it "returns 'balance due'" do
+        expect(order_balance.label).to eq(I18n.t(:balance_due))
       end
     end
   end
 
   describe '#display_amount' do
-    context 'when the customer_balance feature is disabled' do
-      before do
-        allow(order).to receive(:old_outstanding_balance) { 10 }
-      end
-
-      before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { false }
-      end
-
-      it 'returns the balance wraped in a Money object' do
-        expect(order_balance.display_amount).to eq(Spree::Money.new(10, currency: ENV['currency']))
-      end
+    before do
+      allow(order).to receive(:new_outstanding_balance) { 20 }
     end
 
-    context 'when the customer_balance feature is enabled' do
-      before do
-        allow(order).to receive(:new_outstanding_balance) { 20 }
-      end
-
-      before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { true }
-      end
-
-      it 'returns the balance wraped in a Money object' do
-        expect(order_balance.display_amount).to eq(Spree::Money.new(20, currency: ENV['currency']))
-      end
+    it 'returns the balance wraped in a Money object' do
+      expect(order_balance.display_amount).to eq(Spree::Money.new(20, currency: ENV['currency']))
     end
   end
 
   describe '#zero?' do
-    context 'when the customer_balance feature is disabled' do
+    context 'when the balance is zero' do
       before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { false }
+        allow(order).to receive(:new_outstanding_balance) { 0 }
       end
 
-      context 'when the balance is zero' do
-        before do
-          allow(order).to receive(:old_outstanding_balance) { 0 }
-        end
-
-        it 'returns true' do
-          expect(order_balance.zero?).to eq(true)
-        end
-      end
-
-      context 'when the balance is positive' do
-        before do
-          allow(order).to receive(:old_outstanding_balance) { 10 }
-        end
-
-        it 'returns false' do
-          expect(order_balance.zero?).to eq(false)
-        end
-      end
-
-      context 'when the balance is negative' do
-        before do
-          allow(order).to receive(:old_outstanding_balance) { -10 }
-        end
-
-        it 'returns false' do
-          expect(order_balance.zero?).to eq(false)
-        end
+      it 'returns true' do
+        expect(order_balance.zero?).to eq(true)
       end
     end
 
-    context 'when the customer_balance feature is enabled' do
+    context 'when the balance is positive' do
       before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { true }
+        allow(order).to receive(:new_outstanding_balance) { 10 }
       end
 
-      context 'when the balance is zero' do
-        before do
-          allow(order).to receive(:new_outstanding_balance) { 0 }
-        end
+      it 'returns false' do
+        expect(order_balance.zero?).to eq(false)
+      end
+    end
 
-        it 'returns true' do
-          expect(order_balance.zero?).to eq(true)
-        end
+    context 'when the balance is negative' do
+      before do
+        allow(order).to receive(:new_outstanding_balance) { -10 }
       end
 
-      context 'when the balance is positive' do
-        before do
-          allow(order).to receive(:new_outstanding_balance) { 10 }
-        end
-
-        it 'returns false' do
-          expect(order_balance.zero?).to eq(false)
-        end
-      end
-
-      context 'when the balance is negative' do
-        before do
-          allow(order).to receive(:new_outstanding_balance) { -10 }
-        end
-
-        it 'returns false' do
-          expect(order_balance.zero?).to eq(false)
-        end
+      it 'returns false' do
+        expect(order_balance.zero?).to eq(false)
       end
     end
   end
 
   describe '#amount' do
-    context 'when the customer_balance feature is disabled' do
       before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { false }
+        allow(order).to receive(:new_outstanding_balance) { 123 }
       end
 
-      it 'calls #outstanding_balance' do
-        expect(order).to receive(:old_outstanding_balance)
-        order_balance.amount
-      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
-
-      it 'calls #new_outstanding_balance' do
-        expect(order).to receive(:new_outstanding_balance)
-        order_balance.amount
-      end
+    it 'calls #new_outstanding_balance' do
+      expect(order).to receive(:new_outstanding_balance)
+      expect(order_balance.amount).to eq(123)
     end
   end
 
   describe '#abs' do
-    context 'when the customer_balance feature is disabled' do
+    context 'when the balance is zero' do
       before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { false }
+        allow(order).to receive(:new_outstanding_balance) { 0 }
       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
+      it 'returns its absolute value' do
+        expect(order_balance.abs).to eq(0)
       end
     end
 
-    context 'when the customer_balance feature is enabled' do
+    context 'when the balance is positive' do
       before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { true }
+        allow(order).to receive(:new_outstanding_balance) { 10 }
       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(10)
+      end
+    end
 
-        it 'returns its absolute value' do
-          expect(order_balance.abs).to eq(0)
-        end
+    context 'when the balance is negative' do
+      before do
+        allow(order).to receive(:new_outstanding_balance) { -10 }
       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
+      it 'returns its absolute value' do
+        expect(order_balance.abs).to eq(10)
       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
+    before do
+      allow(order).to receive(:new_outstanding_balance) { 10 }
     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
+    it 'returns the balance as a string' do
+      expect(order_balance.to_s).to eq('10')
     end
   end
 
-  describe '#to_f and #to_d' 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 float or decimal' do
-        expect(order_balance.to_f).to eq(10.0)
-        expect(order_balance.to_d).to eq(10.0)
-      end
+  describe '#to_f' do
+    before do
+      allow(order).to receive(:new_outstanding_balance) { 10 }
     end
 
-    context 'when the customer_balance feature is enabled' do
-      before do
-        allow(OpenFoodNetwork::FeatureToggle)
-          .to receive(:enabled?).with(:customer_balance, user) { true }
-      end
+    it 'returns the balance as a float' do
+      expect(order_balance.to_f).to eq(10.0)
+    end
+  end
 
-      before do
-        allow(order).to receive(:new_outstanding_balance) { 10 }
-      end
+  describe '#to_d' do
+    before do
+      allow(order).to receive(:new_outstanding_balance) { 10 }
+    end
 
-      it 'returns the balance as a float or decimal' do
-        expect(order_balance.to_f).to eq(10.0)
-        expect(order_balance.to_d).to eq(10.0)
-      end
+    it 'returns the balance as a decimal' do
+      expect(order_balance.to_d).to eq(10.0)
     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
+    before do
+      allow(order).to receive(:new_outstanding_balance) { 10 }
     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
+    it 'returns the balance as a string' do
+      expect(order_balance + other_order_balance).to eq(20.0)
     end
   end
 
-  context "with comparison operators" 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 'correctly returns true or false' do
-        expect(order_balance > 5).to eq true
-        expect(order_balance > 20).to eq false
-        expect(order_balance < 15).to eq true
-        expect(order_balance < 5).to eq false
-      end
+  describe '#< and #>' do
+    before do
+      allow(order).to receive(:new_outstanding_balance) { 10 }
     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 'correctly returns true or false' do
-        expect(order_balance > 5).to eq true
-        expect(order_balance > 20).to eq false
-        expect(order_balance < 15).to eq true
-        expect(order_balance < 5).to eq false
-      end
+    it 'correctly returns true or false' do
+      expect(order_balance > 5).to eq true
+      expect(order_balance > 20).to eq false
+      expect(order_balance < 15).to eq true
+      expect(order_balance < 5).to eq false
     end
   end
 end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 2f6554a607..2820316bd0 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -402,7 +402,7 @@ describe Spree::Payment do
             end
 
             it 'lets the new payment to be saved' do
-              allow(payment.order).to receive(:old_outstanding_balance) { 100 }
+              allow(payment.order).to receive(:new_outstanding_balance) { 100 }
               allow(payment).to receive(:credit_allowed) { 10 }
 
               offsetting_payment = payment.credit!

From d26c52c2fa65170a52dcc524ab5d27b3a53999dc Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 14 Apr 2021 12:00:59 +0200
Subject: [PATCH 2/2] Memoize OrderBalance instance

There's no need to create an instance of such class for every call to
one of its methods. The balance is computed each time anyway, so it'll
always be up-to-date.
---
 app/models/concerns/balance.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/concerns/balance.rb b/app/models/concerns/balance.rb
index 651a439417..af70eacce5 100644
--- a/app/models/concerns/balance.rb
+++ b/app/models/concerns/balance.rb
@@ -9,7 +9,7 @@ module Balance
 
   # Branches by the OrderBalance abstraction
   def outstanding_balance
-    OrderBalance.new(self)
+    @order_balance ||= OrderBalance.new(self)
   end
 
   # Returns the order balance by considering the total as money owed to the order distributor aka.