From 8db598bff7db4f0f2183f83e93136351db557f64 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 19 Mar 2021 15:53:28 +0000
Subject: [PATCH 01/12] Simplify return adjustment creation

---
 app/models/spree/return_authorization.rb | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb
index fc5acb3f02..dd9b0b76a3 100644
--- a/app/models/spree/return_authorization.rb
+++ b/app/models/spree/return_authorization.rb
@@ -89,10 +89,13 @@ module Spree
         Spree::StockMovement.create!(stock_item_id: iu.find_stock_item.id, quantity: 1)
       end
 
-      credit = Adjustment.new(amount: amount.abs * -1, label: Spree.t(:rma_credit))
-      credit.source = self
-      credit.adjustable = order
-      credit.save
+      Adjustment.create(
+        amount: amount.abs * -1,
+        label: I18n.t('spree.rma_credit'),
+        order: order,
+        adjustable: order,
+        originator: self
+      )
 
       order.return if inventory_units.all?(&:returned?)
       order.update!

From 3c1883dac20a76172718740061b4cecd2a9854bb Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 19 Mar 2021 15:55:14 +0000
Subject: [PATCH 02/12] Remove use of :source polymorphic association for
 return adjustments

---
 app/models/spree/adjustment.rb                 |  2 +-
 spec/models/spree/adjustment_spec.rb           |  2 +-
 spec/models/spree/return_authorization_spec.rb | 14 +++++++++-----
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index ef34d0eb9f..7e6e314b4c 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -66,7 +66,7 @@ module Spree
     scope :optional, -> { where(mandatory: false) }
     scope :charge, -> { where('amount >= 0') }
     scope :credit, -> { where('amount < 0') }
-    scope :return_authorization, -> { where(source_type: "Spree::ReturnAuthorization") }
+    scope :return_authorization, -> { where(originator_type: "Spree::ReturnAuthorization") }
     scope :inclusive, -> { where(included: true) }
     scope :additional, -> { where(included: false) }
 
diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 39e48cafc6..c1af27f7fe 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -9,7 +9,7 @@ module Spree
 
     describe "scopes" do
       let!(:arbitrary_adjustment) { create(:adjustment, source: nil, label: "Arbitrary") }
-      let!(:return_authorization_adjustment) { create(:adjustment, source: create(:return_authorization)) }
+      let!(:return_authorization_adjustment) { create(:adjustment, originator: create(:return_authorization)) }
 
       it "returns return_authorization adjustments" do
         expect(Spree::Adjustment.return_authorization.to_a).to eq [return_authorization_adjustment]
diff --git a/spec/models/spree/return_authorization_spec.rb b/spec/models/spree/return_authorization_spec.rb
index d37bac00ed..5fa696f522 100644
--- a/spec/models/spree/return_authorization_spec.rb
+++ b/spec/models/spree/return_authorization_spec.rb
@@ -82,11 +82,15 @@ describe Spree::ReturnAuthorization do
 
     it "should add credit for specified amount" do
       return_authorization.amount = 20
-      mock_adjustment = double
-      expect(mock_adjustment).to receive(:source=).with(return_authorization)
-      expect(mock_adjustment).to receive(:adjustable=).with(order)
-      expect(mock_adjustment).to receive(:save)
-      expect(Spree::Adjustment).to receive(:new).with(amount: -20, label: Spree.t(:rma_credit)).and_return(mock_adjustment)
+
+      expect(Spree::Adjustment).to receive(:create).with(
+        amount: -20,
+        label: I18n.t('spree.rma_credit'),
+        order: order,
+        adjustable: order,
+        originator: return_authorization
+      )
+
       return_authorization.receive!
     end
 

From 4c5ecbc2d4d4eae46ca4c9365d948ef8e6998263 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 19 Mar 2021 16:02:57 +0000
Subject: [PATCH 03/12] Migrate return adjustments source to originator

---
 .../20210319155627_update_return_adjustments.rb | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 db/migrate/20210319155627_update_return_adjustments.rb

diff --git a/db/migrate/20210319155627_update_return_adjustments.rb b/db/migrate/20210319155627_update_return_adjustments.rb
new file mode 100644
index 0000000000..cd6a9788c6
--- /dev/null
+++ b/db/migrate/20210319155627_update_return_adjustments.rb
@@ -0,0 +1,17 @@
+class UpdateReturnAdjustments < ActiveRecord::Migration[5.0]
+  class Spree::Adjustment < ActiveRecord::Base
+    belongs_to :source, polymorphic: true
+  end
+
+  def up
+    Spree::Adjustment.where(source_type: 'Spree::ReturnAuthorization').update_all(
+      "originator_id = source_id, originator_type = 'Spree::ReturnAuthorization', source_id = NULL, source_type = NULL"
+    )
+  end
+
+  def down
+    Spree::Adjustment.where(originator_type: 'Spree::ReturnAuthorization').update_all(
+      "source_id = originator_id, source_type = 'Spree::ReturnAuthorization', originator_id = NULL, originator_type = NULL"
+    )
+  end
+end

From c480d43bda1e178d515a32325644aa05c9240e03 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 19 Mar 2021 16:05:43 +0000
Subject: [PATCH 04/12] Override #compute_amount in ReturnAuthorization

In some cases adjustments are updated (recalculated) via their originator. In the case of return adjustments, there's no calculation (and no calculator).
---
 app/models/spree/return_authorization.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb
index dd9b0b76a3..32a6a0a0d9 100644
--- a/app/models/spree/return_authorization.rb
+++ b/app/models/spree/return_authorization.rb
@@ -64,6 +64,11 @@ module Spree
       order.shipped_shipments.collect{ |s| s.inventory_units.to_a }.flatten
     end
 
+    # Used when Adjustment#update! wants to update the related adjustment
+    def compute_amount(*args)
+      amount.abs * -1
+    end
+
     private
 
     def must_have_shipped_units

From fcb8145a6c1aac4002e311f956f83c985a13cb9c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 23 Mar 2021 11:18:10 +0000
Subject: [PATCH 05/12] Bring in changes to Adjustment#update! and
 CalculatedAdjustments#update_adjustment

---
 app/models/spree/adjustment.rb                  | 12 +++++++-----
 lib/spree/core/calculated_adjustments.rb        | 13 -------------
 .../spree/core/calculated_adjustments_spec.rb   |  9 ---------
 spec/models/spree/adjustment_spec.rb            | 17 ++++++++---------
 4 files changed, 15 insertions(+), 36 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index 7e6e314b4c..a0bd3e1541 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -100,12 +100,14 @@ module Spree
     # adjustment calculations would not performed on proper values
     def update!(calculable = nil, force: false)
       return if immutable? && !force
+      return if originator.blank?
 
-      # Fix for Spree issue #3381
-      # If we attempt to call 'source' before the reload, then source is currently
-      # the order object. After calling a reload, the source is the Shipment.
-      reload
-      originator.update_adjustment(self, calculable || source) if originator.present?
+      amount = originator.compute_amount(calculable || adjustable)
+
+      update_columns(
+        amount: amount,
+        updated_at: Time.zone.now,
+      )
     end
 
     def currency
diff --git a/lib/spree/core/calculated_adjustments.rb b/lib/spree/core/calculated_adjustments.rb
index 944eff67b6..f773567647 100644
--- a/lib/spree/core/calculated_adjustments.rb
+++ b/lib/spree/core/calculated_adjustments.rb
@@ -54,21 +54,8 @@ module Spree
             end
           end
 
-          # Updates the amount of the adjustment using our Calculator and
-          #   calling the +compute+ method with the +calculable+
-          #   referenced passed to the method.
-          def update_adjustment(adjustment, calculable)
-            # Adjustment calculations done on Spree::Shipment objects MUST
-            # be done on their to_package'd variants instead
-            # It's only the package that contains the correct information.
-            # See https://github.com/spree/spree_active_shipping/pull/96 et. al
-            calculable = calculable.to_package if calculable.is_a?(Spree::Shipment)
-            adjustment.update_column(:amount, compute_amount(calculable))
-          end
-
           # Calculate the amount to be used when creating an adjustment
           # NOTE: May be overriden by classes where this module is included into.
-          # Such as Spree::Promotion::Action::CreateAdjustment.
           def compute_amount(calculable)
             calculator.compute(calculable)
           end
diff --git a/spec/lib/spree/core/calculated_adjustments_spec.rb b/spec/lib/spree/core/calculated_adjustments_spec.rb
index 484654e2eb..f6870807e1 100644
--- a/spec/lib/spree/core/calculated_adjustments_spec.rb
+++ b/spec/lib/spree/core/calculated_adjustments_spec.rb
@@ -66,13 +66,4 @@ describe Spree::Core::CalculatedAdjustments do
       end
     end
   end
-
-  context "#update_adjustment" do
-    it "should update the adjustment using its calculator (and the specified source)" do
-      adjustment = double(:adjustment).as_null_object
-      calculable = double :calculable
-      expect(adjustment).to receive(:update_column).with(:amount, 10)
-      tax_rate.update_adjustment(adjustment, calculable)
-    end
-  end
 end
diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index c1af27f7fe..377162d135 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -18,39 +18,38 @@ module Spree
 
     context "#update!" do
       context "when originator present" do
-        let(:originator) { double("originator", update_adjustment: nil) }
+        let(:originator) { double("originator", compute_amount: 10.0) }
         before do
-          allow(originator).to receive_messages update_amount: true
           allow(adjustment).to receive_messages originator: originator, label: 'adjustment', amount: 0
         end
 
         it "should do nothing when closed" do
           adjustment.close
-          expect(originator).not_to receive(:update_adjustment)
+          expect(originator).not_to receive(:compute_amount)
           adjustment.update!
         end
 
         it "should do nothing when finalized" do
           adjustment.finalize
-          expect(originator).not_to receive(:update_adjustment)
+          expect(originator).not_to receive(:compute_amount)
           adjustment.update!
         end
 
-        it "should ask the originator to update_adjustment" do
-          expect(originator).to receive(:update_adjustment)
+        it "should ask the originator to recalculate the amount" do
+          expect(originator).to receive(:compute_amount)
           adjustment.update!
         end
 
         context "using the :force argument" do
           it "should update adjustments without changing their state" do
-            expect(originator).to receive(:update_adjustment)
+            expect(originator).to receive(:compute_amount)
             adjustment.update!(force: true)
             expect(adjustment.state).to eq "open"
           end
 
           it "should update closed adjustments" do
             adjustment.close
-            expect(originator).to receive(:update_adjustment)
+            expect(originator).to receive(:compute_amount)
             adjustment.update!(force: true)
           end
         end
@@ -58,7 +57,7 @@ module Spree
 
       it "should do nothing when originator is nil" do
         allow(adjustment).to receive_messages originator: nil
-        expect(adjustment).not_to receive(:amount=)
+        expect(adjustment).not_to receive(:update_columns)
         adjustment.update!
       end
     end

From 4b4f29641ec148e7ec9505ffe8f6ceec5c2af991 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 23 Mar 2021 13:46:08 +0000
Subject: [PATCH 06/12] Add test for ReturnAuthorization#compute_amount
 override

---
 spec/models/spree/adjustment_spec.rb | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 377162d135..4bc4e39f32 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -490,5 +490,21 @@ module Spree
         end
       end
     end
+
+    context "return authorization adjustments" do
+      let!(:return_authorization) { create(:return_authorization, amount: 123) }
+      let(:order) { return_authorization.order }
+      let!(:return_adjustment) {
+        create(:adjustment, originator: return_authorization, order: order,
+                            adjustable: order, amount: 456)
+      }
+
+      describe "updating the adjustment" do
+        it "sets a negative value equal to the return authorization amount" do
+          return_adjustment.update!
+          expect(return_adjustment.reload.amount).to eq(-1 * return_authorization.amount)
+        end
+      end
+    end
   end
 end

From e71f47a2e3660472af9687472c3c39db0c4a5984 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 24 Mar 2021 10:26:57 +0000
Subject: [PATCH 07/12] Use underscore in unused arguments

---
 app/models/spree/return_authorization.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb
index 32a6a0a0d9..20dab21122 100644
--- a/app/models/spree/return_authorization.rb
+++ b/app/models/spree/return_authorization.rb
@@ -65,7 +65,7 @@ module Spree
     end
 
     # Used when Adjustment#update! wants to update the related adjustment
-    def compute_amount(*args)
+    def compute_amount(*_args)
       amount.abs * -1
     end
 

From dbe227bd410754c77a3b61fbf104c39faf9eb4bf Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 24 Mar 2021 10:28:45 +0000
Subject: [PATCH 08/12] Use #change in adjustment spec with changing value

---
 spec/models/spree/adjustment_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 4bc4e39f32..2b0ba378ac 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -501,8 +501,8 @@ module Spree
 
       describe "updating the adjustment" do
         it "sets a negative value equal to the return authorization amount" do
-          return_adjustment.update!
-          expect(return_adjustment.reload.amount).to eq(-1 * return_authorization.amount)
+          expect { return_adjustment.update! }.
+            to change { return_adjustment.reload.amount }.to(-123)
         end
       end
     end

From 7882c427f7e9e43ea564a40447a0ed4675f10d82 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 24 Mar 2021 15:20:57 +0000
Subject: [PATCH 09/12] Simplify creation of negative amounts

---
 app/models/spree/return_authorization.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb
index 20dab21122..f05afee8c4 100644
--- a/app/models/spree/return_authorization.rb
+++ b/app/models/spree/return_authorization.rb
@@ -66,7 +66,7 @@ module Spree
 
     # Used when Adjustment#update! wants to update the related adjustment
     def compute_amount(*_args)
-      amount.abs * -1
+      -amount.abs
     end
 
     private
@@ -95,7 +95,7 @@ module Spree
       end
 
       Adjustment.create(
-        amount: amount.abs * -1,
+        amount: -amount.abs,
         label: I18n.t('spree.rma_credit'),
         order: order,
         adjustable: order,

From 08491d9ad479e663140941f62d6bcf198e33ef90 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 24 Mar 2021 15:30:47 +0000
Subject: [PATCH 10/12] Use an instance double in #compute_amount test

---
 spec/models/spree/adjustment_spec.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 2b0ba378ac..886d1786d6 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -18,7 +18,8 @@ module Spree
 
     context "#update!" do
       context "when originator present" do
-        let(:originator) { double("originator", compute_amount: 10.0) }
+        let(:originator) { instance_double(EnterpriseFee, compute_amount: 10.0) }
+
         before do
           allow(adjustment).to receive_messages originator: originator, label: 'adjustment', amount: 0
         end

From d447864b7eb5553b326c97156e09cc7669399c0b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 24 Mar 2021 15:32:53 +0000
Subject: [PATCH 11/12] Update test description

---
 spec/models/spree/adjustment_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 886d1786d6..f1a580c234 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -500,7 +500,7 @@ module Spree
                             adjustable: order, amount: 456)
       }
 
-      describe "updating the adjustment" do
+      describe "#update!" do
         it "sets a negative value equal to the return authorization amount" do
           expect { return_adjustment.update! }.
             to change { return_adjustment.reload.amount }.to(-123)

From b6b367c378307d979533f10fac70f78f33a5f7b6 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sat, 27 Mar 2021 12:51:22 +0000
Subject: [PATCH 12/12] Add return adjustments test in checkout helper

---
 spec/helpers/checkout_helper_spec.rb | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb
index 0283fabc23..4bfdb1b43b 100644
--- a/spec/helpers/checkout_helper_spec.rb
+++ b/spec/helpers/checkout_helper_spec.rb
@@ -57,5 +57,18 @@ describe CheckoutHelper, type: :helper do
       expect(admin_fee_summary.label).to eq I18n.t(:orders_form_admin)
       expect(admin_fee_summary.amount).to eq 123
     end
+
+    context "with return authorization adjustments" do
+      let!(:return_adjustment) {
+        create(:adjustment, originator_type: 'Spree::ReturnAuthorization', adjustable: order,
+                            source: nil, order: order)
+      }
+
+      it "includes return adjustments" do
+        adjustments = helper.checkout_adjustments_for(order)
+
+        expect(adjustments).to include return_adjustment
+      end
+    end
   end
 end