From aa46a4b5da84ed1487467456c11aba1c7f531eee Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 7 Aug 2020 18:44:04 +0100
Subject: [PATCH 01/10] Bring models related to taxes and adjustments from
 spree_core

---
 app/models/spree/adjustment.rb         | 121 ++++++++++++++
 app/models/spree/calculator.rb         |  39 +++++
 app/models/spree/tax_category.rb       |  18 +++
 app/models/spree/tax_rate.rb           |  86 ++++++++++
 spec/models/spree/adjustment_spec.rb   | 210 +++++++++++++++++++++++++
 spec/models/spree/tax_category_spec.rb |  27 ++++
 6 files changed, 501 insertions(+)
 create mode 100644 app/models/spree/adjustment.rb
 create mode 100644 app/models/spree/calculator.rb
 create mode 100644 app/models/spree/tax_category.rb
 create mode 100644 app/models/spree/tax_rate.rb
 create mode 100644 spec/models/spree/tax_category_spec.rb

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
new file mode 100644
index 0000000000..9168d55429
--- /dev/null
+++ b/app/models/spree/adjustment.rb
@@ -0,0 +1,121 @@
+# Adjustments represent a change to the +item_total+ of an Order. Each adjustment
+# has an +amount+ that can be either positive or negative.
+#
+# Adjustments can be open/closed/finalized
+#
+# Once an adjustment is finalized, it cannot be changed, but an adjustment can
+# toggle between open/closed as needed
+#
+# Boolean attributes:
+#
+# +mandatory+
+#
+# If this flag is set to true then it means the the charge is required and will not
+# be removed from the order, even if the amount is zero. In other words a record
+# will be created even if the amount is zero. This is useful for representing things
+# such as shipping and tax charges where you may want to make it explicitly clear
+# that no charge was made for such things.
+#
+# +eligible?+
+#
+# This boolean attributes stores whether this adjustment is currently eligible
+# for its order. Only eligible adjustments count towards the order's adjustment
+# total. This allows an adjustment to be preserved if it becomes ineligible so
+# it might be reinstated.
+module Spree
+  class Adjustment < ActiveRecord::Base
+    belongs_to :adjustable, polymorphic: true
+    belongs_to :source, polymorphic: true
+    belongs_to :originator, polymorphic: true
+
+    validates :label, presence: true
+    validates :amount, numericality: true
+
+    after_save :update_adjustable
+    after_destroy :update_adjustable
+
+    state_machine :state, initial: :open do
+      event :close do
+        transition from: :open, to: :closed
+      end
+
+      event :open do
+        transition from: :closed, to: :open
+      end
+
+      event :finalize do
+        transition from: [:open, :closed], to: :finalized
+      end
+    end
+
+    scope :tax, -> { where(originator_type: 'Spree::TaxRate', adjustable_type: 'Spree::Order') }
+    scope :price, -> { where(adjustable_type: 'Spree::LineItem') }
+    scope :shipping, -> { where(originator_type: 'Spree::ShippingMethod') }
+    scope :optional, -> { where(mandatory: false) }
+    scope :eligible, -> { where(eligible: true) }
+    scope :charge, -> { where('amount >= 0') }
+    scope :credit, -> { where('amount < 0') }
+    scope :promotion, -> { where(originator_type: 'Spree::PromotionAction') }
+    scope :return_authorization, -> { where(source_type: "Spree::ReturnAuthorization") }
+
+    def promotion?
+      originator_type == 'Spree::PromotionAction'
+    end
+
+    # Update the boolean _eligible_ attribute which determines which adjustments
+    # count towards the order's adjustment_total.
+    def set_eligibility
+      result = mandatory || ((amount != 0 || promotion?) && eligible_for_originator?)
+      update_column(:eligible, result)
+    end
+
+    # Allow originator of the adjustment to perform an additional eligibility of the adjustment
+    # Should return _true_ if originator is absent or doesn't implement _eligible?_
+    def eligible_for_originator?
+      return true if originator.nil?
+      !originator.respond_to?(:eligible?) || originator.eligible?(source)
+    end
+
+    # Update both the eligibility and amount of the adjustment. Adjustments
+    # delegate updating of amount to their Originator when present, but only if
+    # +locked+ is false. Adjustments that are +locked+ will never change their amount.
+    #
+    # Adjustments delegate updating of amount to their Originator when present,
+    # but only if when they're in "open" state, closed or finalized adjustments
+    # are not recalculated.
+    #
+    # It receives +calculable+ as the updated source here so calculations can be
+    # performed on the current values of that source. If we used +source+ it 
+    # could load the old record from db for the association. e.g. when updating
+    # more than on line items at once via accepted_nested_attributes the order
+    # object on the association would be in a old state and therefore the
+    # adjustment calculations would not performed on proper values
+    def update!(calculable = nil)
+      return if immutable?
+      # Fix for #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?
+      set_eligibility
+    end
+
+    def currency
+      adjustable ? adjustable.currency : Spree::Config[:currency]
+    end
+
+    def display_amount
+      Spree::Money.new(amount, { currency: currency })
+    end
+
+    def immutable?
+      state != "open"
+    end
+
+    private
+
+      def update_adjustable
+        adjustable.update! if adjustable.is_a? Order
+      end
+  end
+end
diff --git a/app/models/spree/calculator.rb b/app/models/spree/calculator.rb
new file mode 100644
index 0000000000..b9aeed3333
--- /dev/null
+++ b/app/models/spree/calculator.rb
@@ -0,0 +1,39 @@
+module Spree
+  class Calculator < ActiveRecord::Base
+    belongs_to :calculable, polymorphic: true
+
+    # This method must be overriden in concrete calculator.
+    #
+    # It should return amount computed based on #calculable and/or optional parameter
+    def compute(something = nil)
+      raise NotImplementedError, 'please use concrete calculator'
+    end
+
+    # overwrite to provide description for your calculators
+    def self.description
+      'Base Calculator'
+    end
+
+    ###################################################################
+
+    def self.register(*klasses)
+    end
+
+    # Returns all calculators applicable for kind of work
+    def self.calculators
+      Rails.application.config.spree.calculators
+    end
+
+    def to_s
+      self.class.name.titleize.gsub("Calculator\/", "")
+    end
+
+    def description
+      self.class.description
+    end
+
+    def available?(object)
+      true
+    end
+  end
+end
diff --git a/app/models/spree/tax_category.rb b/app/models/spree/tax_category.rb
new file mode 100644
index 0000000000..6e0dd27da1
--- /dev/null
+++ b/app/models/spree/tax_category.rb
@@ -0,0 +1,18 @@
+module Spree
+  class TaxCategory < ActiveRecord::Base
+    acts_as_paranoid
+    validates :name, presence: true, uniqueness: { scope: :deleted_at }
+
+    has_many :tax_rates, dependent: :destroy
+
+    before_save :set_default_category
+
+    def set_default_category
+      #set existing default tax category to false if this one has been marked as default
+
+      if is_default && tax_category = self.class.where(is_default: true).first
+        tax_category.update_column(:is_default, false) unless tax_category == self
+      end
+    end
+  end
+end
diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb
new file mode 100644
index 0000000000..fde950b3cd
--- /dev/null
+++ b/app/models/spree/tax_rate.rb
@@ -0,0 +1,86 @@
+module Spree
+  class DefaultTaxZoneValidator < ActiveModel::Validator
+    def validate(record)
+      if record.included_in_price
+        record.errors.add(:included_in_price, Spree.t(:included_price_validation)) unless Zone.default_tax
+      end
+    end
+  end
+end
+
+module Spree
+  class TaxRate < ActiveRecord::Base
+    acts_as_paranoid
+    include Spree::Core::CalculatedAdjustments
+    belongs_to :zone, class_name: "Spree::Zone"
+    belongs_to :tax_category, class_name: "Spree::TaxCategory"
+
+    validates :amount, presence: true, numericality: true
+    validates :tax_category_id, presence: true
+    validates_with DefaultTaxZoneValidator
+
+    scope :by_zone, ->(zone) { where(zone_id: zone) }
+
+    # Gets the array of TaxRates appropriate for the specified order
+    def self.match(order)
+      return [] unless order.tax_zone
+      all.select do |rate|
+        (!rate.included_in_price && (rate.zone == order.tax_zone || rate.zone.contains?(order.tax_zone) || (order.tax_address.nil? && rate.zone.default_tax))) ||
+        (rate.included_in_price && !order.tax_address.nil? && !rate.zone.contains?(order.tax_zone) && rate.zone.default_tax)
+      end
+    end
+
+    def self.adjust(order)
+      order.adjustments.tax.destroy_all
+      order.line_item_adjustments.where(originator_type: 'Spree::TaxRate').destroy_all
+
+      self.match(order).each do |rate|
+        rate.adjust(order)
+      end
+    end
+
+    # For Vat the default rate is the rate that is configured for the default category
+    # It is needed for every price calculation (as all customer facing prices include vat )
+    # The function returns the actual amount, which may be 0 in case of wrong setup, but is never nil
+    def self.default
+      category = TaxCategory.includes(:tax_rates).where(is_default: true).first
+      return 0 unless category
+
+      address ||= Address.new(country_id: Spree::Config[:default_country_id])
+      rate = category.tax_rates.detect { |rate| rate.zone.include? address }.try(:amount)
+
+      rate || 0
+    end
+
+    # Creates necessary tax adjustments for the order.
+    def adjust(order)
+      label = create_label
+      if included_in_price
+        if Zone.default_tax.contains? order.tax_zone
+          order.line_items.each { |line_item| create_adjustment(label, line_item, line_item) }
+        else
+          amount = -1 * calculator.compute(order)
+          label = Spree.t(:refund) + label
+
+          order.adjustments.create(
+            amount: amount,
+            source: order,
+            originator: self,
+            state: "closed",
+            label: label
+          )
+        end
+      else
+        create_adjustment(label, order, order)
+      end
+    end
+
+    private
+
+      def create_label
+        label = ""
+        label << (name.present? ? name : tax_category.name) + " "
+        label << (show_rate_in_label? ? "#{amount * 100}%" : "")
+      end
+  end
+end
diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 0ea3eea273..123ece3534 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -2,6 +2,216 @@ require 'spec_helper'
 
 module Spree
   describe Adjustment do
+    let(:order) { mock_model(Spree::Order, update!: nil) }
+    let(:adjustment) { Spree::Adjustment.create(:label => "Adjustment", :amount => 5) }
+
+    describe "scopes" do
+      let!(:arbitrary_adjustment) { create(:adjustment, source: nil, label: "Arbitrary") }
+      let!(:return_authorization_adjustment) { create(:adjustment, source: create(:return_authorization)) }
+
+      it "returns return_authorization adjustments" do
+        expect(Spree::Adjustment.return_authorization.to_a).to eq [return_authorization_adjustment]
+      end
+    end
+
+    context "#update!" do
+      context "when originator present" do
+        let(:originator) { double("originator", update_adjustment: nil) }
+        before do
+          originator.stub update_amount: true
+          adjustment.stub originator: originator, label: 'adjustment', amount: 0
+        end
+        it "should do nothing when closed" do
+          adjustment.close
+          originator.should_not_receive(:update_adjustment)
+          adjustment.update!
+        end
+        it "should do nothing when finalized" do
+          adjustment.finalize
+          originator.should_not_receive(:update_adjustment)
+          adjustment.update!
+        end
+        it "should set the eligibility" do
+          adjustment.should_receive(:set_eligibility)
+          adjustment.update!
+        end
+        it "should ask the originator to update_adjustment" do
+          originator.should_receive(:update_adjustment)
+          adjustment.update!
+        end
+      end
+      it "should do nothing when originator is nil" do
+        adjustment.stub originator: nil
+        adjustment.should_not_receive(:amount=)
+        adjustment.update!
+      end
+    end
+
+    context "#promotion?" do
+      it "returns false if not promotion adjustment" do
+        expect(adjustment.promotion?).to eq false
+      end
+
+      it "returns true if promotion adjustment" do
+        adjustment.originator_type = "Spree::PromotionAction"
+        expect(adjustment.promotion?).to eq true
+      end
+    end
+
+    context "#eligible? after #set_eligibility" do
+      context "when amount is 0" do
+        before { adjustment.amount = 0 }
+        it "should be eligible if mandatory?" do
+          adjustment.mandatory = true
+          adjustment.set_eligibility
+          adjustment.should be_eligible
+        end
+        it "should be eligible if `promotion?` even if not `mandatory?`" do
+          adjustment.should_receive(:promotion?).and_return(true)
+          adjustment.mandatory = false
+          adjustment.set_eligibility
+          adjustment.should be_eligible
+        end
+        it "should not be eligible unless mandatory?" do
+          adjustment.mandatory = false
+          adjustment.set_eligibility
+          adjustment.should_not be_eligible
+        end
+      end
+
+      context "when amount is greater than 0" do
+        before { adjustment.amount = 25.00 }
+        it "should be eligible if mandatory?" do
+          adjustment.mandatory = true
+          adjustment.set_eligibility
+          adjustment.should be_eligible
+        end
+        it "should be eligible if not mandatory and eligible for the originator" do
+          adjustment.mandatory = false
+          adjustment.stub(eligible_for_originator?: true)
+          adjustment.set_eligibility
+          adjustment.should be_eligible
+        end
+        it "should not be eligible if not mandatory not eligible for the originator" do
+          adjustment.mandatory = false
+          adjustment.stub(eligible_for_originator?: false)
+          adjustment.set_eligibility
+          adjustment.should_not be_eligible
+        end
+      end
+    end
+
+    context "#save" do
+      it "should call order#update!" do
+        adjustment = Spree::Adjustment.new(
+          :adjustable => order,
+          :amount => 10,
+          :label => "Foo"
+        )
+        order.should_receive(:update!)
+        adjustment.save
+      end
+    end
+
+    context "adjustment state" do
+      let(:adjustment) { create(:adjustment, state: 'open') }
+
+      context "#immutable?" do
+        it "is true when adjustment state isn't open" do
+          adjustment.state = "closed"
+          adjustment.should be_immutable
+          adjustment.state = "finalized"
+          adjustment.should be_immutable
+        end
+
+        it "is false when adjustment state is open" do
+          adjustment.state = "open"
+          adjustment.should_not be_immutable
+        end
+      end
+
+      context "#finalized?" do
+        it "is true when adjustment state is finalized" do
+          adjustment.state = "finalized"
+          adjustment.should be_finalized
+        end
+
+        it "is false when adjustment state isn't finalized" do
+          adjustment.state = "closed"
+          adjustment.should_not be_finalized
+          adjustment.state = "open"
+          adjustment.should_not be_finalized
+        end
+      end
+    end
+
+    context "#eligible_for_originator?" do
+      context "with no originator" do
+        specify { adjustment.should be_eligible_for_originator }
+      end
+      context "with originator that doesn't have 'eligible?'" do
+        before { adjustment.originator = mock_model(Spree::TaxRate) }
+        specify { adjustment.should be_eligible_for_originator }
+      end
+      context "with originator that has 'eligible?'" do
+        let(:originator) { Spree::TaxRate.new }
+        before { adjustment.originator = originator }
+        context "and originator is eligible for order" do
+          before { originator.stub(eligible?: true) }
+          specify { adjustment.should be_eligible_for_originator }
+        end
+        context "and originator is not eligible for order" do
+          before { originator.stub(eligible?: false) }
+          specify { adjustment.should_not be_eligible_for_originator }
+        end
+      end
+    end
+
+    context "#display_amount" do
+      before { adjustment.amount = 10.55 }
+
+      context "with display_currency set to true" do
+        before { Spree::Config[:display_currency] = true }
+
+        it "shows the currency" do
+          adjustment.display_amount.to_s.should == "$10.55 USD"
+        end
+      end
+
+      context "with display_currency set to false" do
+        before { Spree::Config[:display_currency] = false }
+
+        it "does not include the currency" do
+          adjustment.display_amount.to_s.should == "$10.55"
+        end
+      end
+
+      context "with currency set to JPY" do
+        context "when adjustable is set to an order" do
+          before do
+            order.stub(:currency) { 'JPY' }
+            adjustment.adjustable = order
+          end
+
+          it "displays in JPY" do
+            adjustment.display_amount.to_s.should == "¥11"
+          end
+        end
+
+        context "when adjustable is nil" do
+          it "displays in the default currency" do
+            adjustment.display_amount.to_s.should == "$10.55"
+          end
+        end
+      end
+    end
+
+    context '#currency' do
+      it 'returns the globally configured currency' do
+        adjustment.currency.should == 'USD'
+      end
+    end
+
     it "has metadata" do
       adjustment = create(:adjustment, metadata: create(:adjustment_metadata))
       expect(adjustment.metadata).to be
diff --git a/spec/models/spree/tax_category_spec.rb b/spec/models/spree/tax_category_spec.rb
new file mode 100644
index 0000000000..ea2c329aea
--- /dev/null
+++ b/spec/models/spree/tax_category_spec.rb
@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+describe Spree::TaxCategory do
+  context 'default tax category' do
+    let(:tax_category) { create(:tax_category) }
+    let(:new_tax_category) { create(:tax_category) }
+
+    before do
+      tax_category.update_column(:is_default, true)
+    end
+
+    it "should undefault the previous default tax category" do
+      new_tax_category.update_attributes({:is_default => true})
+      new_tax_category.is_default.should be_true
+
+      tax_category.reload
+      tax_category.is_default.should be_false
+    end
+
+    it "should undefault the previous default tax category except when updating the existing default tax category" do
+      tax_category.update_column(:description, "Updated description")
+
+      tax_category.reload
+      tax_category.is_default.should be_true
+    end
+  end
+end

From da683e3ecf56a317d67e6fa985e79906d2245481 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 7 Aug 2020 18:54:23 +0100
Subject: [PATCH 02/10] Merge decorators with original code from spree_core

---
 app/models/spree/adjustment.rb           | 61 ++++++++++++++++++++++-
 app/models/spree/adjustment_decorator.rb | 62 ------------------------
 app/models/spree/calculator.rb           | 16 ++++++
 app/models/spree/calculator_decorator.rb | 19 --------
 app/models/spree/tax_rate.rb             | 47 ++++++++++++++++--
 app/models/spree/tax_rate_decorator.rb   | 61 -----------------------
 6 files changed, 119 insertions(+), 147 deletions(-)
 delete mode 100644 app/models/spree/adjustment_decorator.rb
 delete mode 100644 app/models/spree/calculator_decorator.rb
 delete mode 100644 app/models/spree/tax_rate_decorator.rb

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index 9168d55429..37de2b20df 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -1,3 +1,6 @@
+require 'spree/localized_number'
+require 'concerns/adjustment_scopes'
+
 # Adjustments represent a change to the +item_total+ of an Order. Each adjustment
 # has an +amount+ that can be either positive or negative.
 #
@@ -24,9 +27,18 @@
 # it might be reinstated.
 module Spree
   class Adjustment < ActiveRecord::Base
+    extend Spree::LocalizedNumber
+
+    # Deletion of metadata is handled in the database.
+    # So we don't need the option `dependent: :destroy` as long as
+    # AdjustmentMetadata has no destroy logic itself.
+    has_one :metadata, class_name: 'AdjustmentMetadata'
+
     belongs_to :adjustable, polymorphic: true
     belongs_to :source, polymorphic: true
     belongs_to :originator, polymorphic: true
+    belongs_to :tax_rate, -> { where spree_adjustments: { originator_type: 'Spree::TaxRate' } },
+               foreign_key: 'originator_id'
 
     validates :label, presence: true
     validates :amount, numericality: true
@@ -50,14 +62,26 @@ module Spree
 
     scope :tax, -> { where(originator_type: 'Spree::TaxRate', adjustable_type: 'Spree::Order') }
     scope :price, -> { where(adjustable_type: 'Spree::LineItem') }
-    scope :shipping, -> { where(originator_type: 'Spree::ShippingMethod') }
     scope :optional, -> { where(mandatory: false) }
-    scope :eligible, -> { where(eligible: true) }
     scope :charge, -> { where('amount >= 0') }
     scope :credit, -> { where('amount < 0') }
     scope :promotion, -> { where(originator_type: 'Spree::PromotionAction') }
     scope :return_authorization, -> { where(source_type: "Spree::ReturnAuthorization") }
 
+    scope :enterprise_fee, -> { where(originator_type: 'EnterpriseFee') }
+    scope :admin,          -> { where(source_type: nil, originator_type: nil) }
+    scope :included_tax,   -> {
+      where(originator_type: 'Spree::TaxRate', adjustable_type: 'Spree::LineItem')
+    }
+
+    scope :with_tax,       -> { where('spree_adjustments.included_tax <> 0') }
+    scope :without_tax,    -> { where('spree_adjustments.included_tax = 0') }
+    scope :payment_fee,    -> { where(AdjustmentScopes::PAYMENT_FEE_SCOPE) }
+    scope :shipping,       -> { where(AdjustmentScopes::SHIPPING_SCOPE) }
+    scope :eligible,       -> { where(AdjustmentScopes::ELIGIBLE_SCOPE) }
+
+    localize_number :amount
+
     def promotion?
       originator_type == 'Spree::PromotionAction'
     end
@@ -112,6 +136,39 @@ module Spree
       state != "open"
     end
 
+    def set_included_tax!(rate)
+      tax = amount - (amount / (1 + rate))
+      set_absolute_included_tax! tax
+    end
+
+    def set_absolute_included_tax!(tax)
+      # This rubocop issue can now fixed by renaming Adjustment#update! to something else,
+      #   then AR's update! can be used instead of update_attributes!
+      # rubocop:disable Rails/ActiveRecordAliases
+      update_attributes! included_tax: tax.round(2)
+      # rubocop:enable Rails/ActiveRecordAliases
+    end
+
+    def display_included_tax
+      Spree::Money.new(included_tax, currency: currency)
+    end
+
+    def has_tax?
+      included_tax > 0
+    end
+
+    def self.without_callbacks
+      skip_callback :save, :after, :update_adjustable
+      skip_callback :destroy, :after, :update_adjustable
+
+      result = yield
+    ensure
+      set_callback :save, :after, :update_adjustable
+      set_callback :destroy, :after, :update_adjustable
+
+      result
+    end
+
     private
 
       def update_adjustable
diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb
deleted file mode 100644
index cba3b3e879..0000000000
--- a/app/models/spree/adjustment_decorator.rb
+++ /dev/null
@@ -1,62 +0,0 @@
-require 'spree/localized_number'
-require 'concerns/adjustment_scopes'
-
-module Spree
-  Adjustment.class_eval do
-    extend Spree::LocalizedNumber
-
-    # Deletion of metadata is handled in the database.
-    # So we don't need the option `dependent: :destroy` as long as
-    # AdjustmentMetadata has no destroy logic itself.
-    has_one :metadata, class_name: 'AdjustmentMetadata'
-    belongs_to :tax_rate, -> { where spree_adjustments: { originator_type: 'Spree::TaxRate' } },
-               foreign_key: 'originator_id'
-
-    scope :enterprise_fee, -> { where(originator_type: 'EnterpriseFee') }
-    scope :admin,          -> { where(source_type: nil, originator_type: nil) }
-    scope :included_tax,   -> {
-      where(originator_type: 'Spree::TaxRate', adjustable_type: 'Spree::LineItem')
-    }
-
-    scope :with_tax,       -> { where('spree_adjustments.included_tax <> 0') }
-    scope :without_tax,    -> { where('spree_adjustments.included_tax = 0') }
-    scope :payment_fee,    -> { where(AdjustmentScopes::PAYMENT_FEE_SCOPE) }
-    scope :shipping,       -> { where(AdjustmentScopes::SHIPPING_SCOPE) }
-    scope :eligible,       -> { where(AdjustmentScopes::ELIGIBLE_SCOPE) }
-
-    localize_number :amount
-
-    def set_included_tax!(rate)
-      tax = amount - (amount / (1 + rate))
-      set_absolute_included_tax! tax
-    end
-
-    def set_absolute_included_tax!(tax)
-      # This rubocop issue can only be fixed when Adjustment#update! is brought from Spree to OFN
-      #   and renamed to something else, then AR's update! can be used instead of update_attributes!
-      # rubocop:disable Rails/ActiveRecordAliases
-      update_attributes! included_tax: tax.round(2)
-      # rubocop:enable Rails/ActiveRecordAliases
-    end
-
-    def display_included_tax
-      Spree::Money.new(included_tax, currency: currency)
-    end
-
-    def has_tax?
-      included_tax > 0
-    end
-
-    def self.without_callbacks
-      skip_callback :save, :after, :update_adjustable
-      skip_callback :destroy, :after, :update_adjustable
-
-      result = yield
-    ensure
-      set_callback :save, :after, :update_adjustable
-      set_callback :destroy, :after, :update_adjustable
-
-      result
-    end
-  end
-end
diff --git a/app/models/spree/calculator.rb b/app/models/spree/calculator.rb
index b9aeed3333..4ce5b53c13 100644
--- a/app/models/spree/calculator.rb
+++ b/app/models/spree/calculator.rb
@@ -35,5 +35,21 @@ module Spree
     def available?(object)
       true
     end
+
+    private
+
+    # Given an object which might be an Order or a LineItem (amongst
+    # others), return a collection of line items.
+    def line_items_for(object)
+      if object.is_a?(Spree::LineItem)
+        [object]
+      elsif object.respond_to? :line_items
+        object.line_items
+      elsif object.respond_to?(:order) && object.order.present?
+        object.order.line_items
+      else
+        [object]
+      end
+    end
   end
 end
diff --git a/app/models/spree/calculator_decorator.rb b/app/models/spree/calculator_decorator.rb
deleted file mode 100644
index 5d130d86aa..0000000000
--- a/app/models/spree/calculator_decorator.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-module Spree
-  Calculator.class_eval do
-    private
-
-    # Given an object which might be an Order or a LineItem (amongst
-    # others), return a collection of line items.
-    def line_items_for(object)
-      if object.is_a?(Spree::LineItem)
-        [object]
-      elsif object.respond_to? :line_items
-        object.line_items
-      elsif object.respond_to?(:order) && object.order.present?
-        object.order.line_items
-      else
-        [object]
-      end
-    end
-  end
-end
diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb
index fde950b3cd..d31345e0da 100644
--- a/app/models/spree/tax_rate.rb
+++ b/app/models/spree/tax_rate.rb
@@ -22,11 +22,12 @@ module Spree
     scope :by_zone, ->(zone) { where(zone_id: zone) }
 
     # Gets the array of TaxRates appropriate for the specified order
-    def self.match(order)
+    def match(order)
+      return [] if order.distributor && !order.distributor.charges_sales_tax
       return [] unless order.tax_zone
+
       all.select do |rate|
-        (!rate.included_in_price && (rate.zone == order.tax_zone || rate.zone.contains?(order.tax_zone) || (order.tax_address.nil? && rate.zone.default_tax))) ||
-        (rate.included_in_price && !order.tax_address.nil? && !rate.zone.contains?(order.tax_zone) && rate.zone.default_tax)
+        rate.zone == order.tax_zone || rate.zone.contains?(order.tax_zone) || rate.zone.default_tax
       end
     end
 
@@ -73,6 +74,32 @@ module Spree
       else
         create_adjustment(label, order, order)
       end
+
+      order.adjustments(:reload)
+      order.line_items(:reload)
+      # TaxRate adjustments (order.adjustments.tax) and price adjustments (tax included on line items) consist of 100% tax
+      (order.adjustments.tax + order.price_adjustments).each do |adjustment|
+        adjustment.set_absolute_included_tax! adjustment.amount
+      end
+    end
+
+    # Manually apply a TaxRate to a particular amount. TaxRates normally compute against
+    # LineItems or Orders, so we mock out a line item here to fit the interface
+    # that our calculator (usually DefaultTax) expects.
+    def compute_tax(amount)
+      line_item = LineItem.new quantity: 1
+      line_item.tax_category = tax_category
+      line_item.define_singleton_method(:price) { amount }
+
+      # Tax on adjustments (represented by the included_tax field) is always inclusive of
+      # tax. However, there's nothing to stop an admin from setting one up with a tax rate
+      # that's marked as not inclusive of tax, and that would result in the DefaultTax
+      # calculator generating a slightly incorrect value. Therefore, we treat the tax
+      # rate as inclusive of tax for the calculations below, regardless of its original
+      # setting.
+      with_tax_included_in_price do
+        calculator.compute line_item
+      end
     end
 
     private
@@ -82,5 +109,19 @@ module Spree
         label << (name.present? ? name : tax_category.name) + " "
         label << (show_rate_in_label? ? "#{amount * 100}%" : "")
       end
+
+    def with_tax_included_in_price
+      old_included_in_price = included_in_price
+
+      self.included_in_price = true
+      calculator.calculable.included_in_price = true
+
+      result = yield
+    ensure
+      self.included_in_price = old_included_in_price
+      calculator.calculable.included_in_price = old_included_in_price
+
+      result
+    end
   end
 end
diff --git a/app/models/spree/tax_rate_decorator.rb b/app/models/spree/tax_rate_decorator.rb
deleted file mode 100644
index f15ded6de8..0000000000
--- a/app/models/spree/tax_rate_decorator.rb
+++ /dev/null
@@ -1,61 +0,0 @@
-module Spree
-  TaxRate.class_eval do
-    class << self
-      def match(order)
-        return [] if order.distributor && !order.distributor.charges_sales_tax
-        return [] unless order.tax_zone
-
-        all.select do |rate|
-          rate.zone == order.tax_zone || rate.zone.contains?(order.tax_zone) || rate.zone.default_tax
-        end
-      end
-    end
-
-    def adjust_with_included_tax(order)
-      adjust_without_included_tax(order)
-
-      order.adjustments(:reload)
-      order.line_items(:reload)
-      # TaxRate adjustments (order.adjustments.tax) and price adjustments (tax included on line items) consist of 100% tax
-      (order.adjustments.tax + order.price_adjustments).each do |adjustment|
-        adjustment.set_absolute_included_tax! adjustment.amount
-      end
-    end
-    alias_method_chain :adjust, :included_tax
-
-    # Manually apply a TaxRate to a particular amount. TaxRates normally compute against
-    # LineItems or Orders, so we mock out a line item here to fit the interface
-    # that our calculator (usually DefaultTax) expects.
-    def compute_tax(amount)
-      line_item = LineItem.new quantity: 1
-      line_item.tax_category = tax_category
-      line_item.define_singleton_method(:price) { amount }
-
-      # Tax on adjustments (represented by the included_tax field) is always inclusive of
-      # tax. However, there's nothing to stop an admin from setting one up with a tax rate
-      # that's marked as not inclusive of tax, and that would result in the DefaultTax
-      # calculator generating a slightly incorrect value. Therefore, we treat the tax
-      # rate as inclusive of tax for the calculations below, regardless of its original
-      # setting.
-      with_tax_included_in_price do
-        calculator.compute line_item
-      end
-    end
-
-    private
-
-    def with_tax_included_in_price
-      old_included_in_price = included_in_price
-
-      self.included_in_price = true
-      calculator.calculable.included_in_price = true
-
-      result = yield
-    ensure
-      self.included_in_price = old_included_in_price
-      calculator.calculable.included_in_price = old_included_in_price
-
-      result
-    end
-  end
-end

From ff0aa377a17a5c0c26fd76c7d5aac8021c6841df Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 7 Aug 2020 18:56:07 +0100
Subject: [PATCH 03/10] Run rubocop autocorrect

---
 app/models/spree/adjustment.rb         | 12 ++++++++----
 app/models/spree/calculator.rb         |  9 +++++----
 app/models/spree/tax_category.rb       |  4 +++-
 app/models/spree/tax_rate.rb           | 14 ++++++++------
 spec/models/spree/adjustment_spec.rb   |  8 ++++----
 spec/models/spree/tax_category_spec.rb |  4 +++-
 6 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index 37de2b20df..975fc82201 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spree/localized_number'
 require 'concerns/adjustment_scopes'
 
@@ -97,6 +99,7 @@ module Spree
     # Should return _true_ if originator is absent or doesn't implement _eligible?_
     def eligible_for_originator?
       return true if originator.nil?
+
       !originator.respond_to?(:eligible?) || originator.eligible?(source)
     end
 
@@ -109,13 +112,14 @@ module Spree
     # are not recalculated.
     #
     # It receives +calculable+ as the updated source here so calculations can be
-    # performed on the current values of that source. If we used +source+ it 
+    # performed on the current values of that source. If we used +source+ it
     # could load the old record from db for the association. e.g. when updating
     # more than on line items at once via accepted_nested_attributes the order
     # object on the association would be in a old state and therefore the
     # adjustment calculations would not performed on proper values
     def update!(calculable = nil)
       return if immutable?
+
       # Fix for #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.
@@ -171,8 +175,8 @@ module Spree
 
     private
 
-      def update_adjustable
-        adjustable.update! if adjustable.is_a? Order
-      end
+    def update_adjustable
+      adjustable.update! if adjustable.is_a? Order
+    end
   end
 end
diff --git a/app/models/spree/calculator.rb b/app/models/spree/calculator.rb
index 4ce5b53c13..40bea838f5 100644
--- a/app/models/spree/calculator.rb
+++ b/app/models/spree/calculator.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   class Calculator < ActiveRecord::Base
     belongs_to :calculable, polymorphic: true
@@ -5,7 +7,7 @@ module Spree
     # This method must be overriden in concrete calculator.
     #
     # It should return amount computed based on #calculable and/or optional parameter
-    def compute(something = nil)
+    def compute(_something = nil)
       raise NotImplementedError, 'please use concrete calculator'
     end
 
@@ -16,8 +18,7 @@ module Spree
 
     ###################################################################
 
-    def self.register(*klasses)
-    end
+    def self.register(*klasses); end
 
     # Returns all calculators applicable for kind of work
     def self.calculators
@@ -32,7 +33,7 @@ module Spree
       self.class.description
     end
 
-    def available?(object)
+    def available?(_object)
       true
     end
 
diff --git a/app/models/spree/tax_category.rb b/app/models/spree/tax_category.rb
index 6e0dd27da1..7bcad54fe7 100644
--- a/app/models/spree/tax_category.rb
+++ b/app/models/spree/tax_category.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   class TaxCategory < ActiveRecord::Base
     acts_as_paranoid
@@ -8,7 +10,7 @@ module Spree
     before_save :set_default_category
 
     def set_default_category
-      #set existing default tax category to false if this one has been marked as default
+      # set existing default tax category to false if this one has been marked as default
 
       if is_default && tax_category = self.class.where(is_default: true).first
         tax_category.update_column(:is_default, false) unless tax_category == self
diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb
index d31345e0da..d7dd93db8f 100644
--- a/app/models/spree/tax_rate.rb
+++ b/app/models/spree/tax_rate.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   class DefaultTaxZoneValidator < ActiveModel::Validator
     def validate(record)
@@ -35,7 +37,7 @@ module Spree
       order.adjustments.tax.destroy_all
       order.line_item_adjustments.where(originator_type: 'Spree::TaxRate').destroy_all
 
-      self.match(order).each do |rate|
+      match(order).each do |rate|
         rate.adjust(order)
       end
     end
@@ -104,11 +106,11 @@ module Spree
 
     private
 
-      def create_label
-        label = ""
-        label << (name.present? ? name : tax_category.name) + " "
-        label << (show_rate_in_label? ? "#{amount * 100}%" : "")
-      end
+    def create_label
+      label = ""
+      label << (name.presence || tax_category.name) + " "
+      label << (show_rate_in_label? ? "#{amount * 100}%" : "")
+    end
 
     def with_tax_included_in_price
       old_included_in_price = included_in_price
diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 123ece3534..2eec4943c7 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 module Spree
   describe Adjustment do
     let(:order) { mock_model(Spree::Order, update!: nil) }
-    let(:adjustment) { Spree::Adjustment.create(:label => "Adjustment", :amount => 5) }
+    let(:adjustment) { Spree::Adjustment.create(label: "Adjustment", amount: 5) }
 
     describe "scopes" do
       let!(:arbitrary_adjustment) { create(:adjustment, source: nil, label: "Arbitrary") }
@@ -104,9 +104,9 @@ module Spree
     context "#save" do
       it "should call order#update!" do
         adjustment = Spree::Adjustment.new(
-          :adjustable => order,
-          :amount => 10,
-          :label => "Foo"
+          adjustable: order,
+          amount: 10,
+          label: "Foo"
         )
         order.should_receive(:update!)
         adjustment.save
diff --git a/spec/models/spree/tax_category_spec.rb b/spec/models/spree/tax_category_spec.rb
index ea2c329aea..4ae82eb02d 100644
--- a/spec/models/spree/tax_category_spec.rb
+++ b/spec/models/spree/tax_category_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe Spree::TaxCategory do
@@ -10,7 +12,7 @@ describe Spree::TaxCategory do
     end
 
     it "should undefault the previous default tax category" do
-      new_tax_category.update_attributes({:is_default => true})
+      new_tax_category.update({ is_default: true })
       new_tax_category.is_default.should be_true
 
       tax_category.reload

From 967380c54290aaf0b4a7ca5554f600e328c54e49 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 7 Aug 2020 19:02:17 +0100
Subject: [PATCH 04/10] Fix easy rubocop issues

---
 app/models/spree/adjustment.rb         |  2 +-
 app/models/spree/tax_category.rb       |  6 +++---
 app/models/spree/tax_rate.rb           | 21 ++++++++++++---------
 spec/models/spree/tax_category_spec.rb |  3 ++-
 4 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index 975fc82201..add4e33391 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -158,7 +158,7 @@ module Spree
     end
 
     def has_tax?
-      included_tax > 0
+      included_tax.positive?
     end
 
     def self.without_callbacks
diff --git a/app/models/spree/tax_category.rb b/app/models/spree/tax_category.rb
index 7bcad54fe7..a856e06e10 100644
--- a/app/models/spree/tax_category.rb
+++ b/app/models/spree/tax_category.rb
@@ -12,9 +12,9 @@ module Spree
     def set_default_category
       # set existing default tax category to false if this one has been marked as default
 
-      if is_default && tax_category = self.class.where(is_default: true).first
-        tax_category.update_column(:is_default, false) unless tax_category == self
-      end
+      return unless is_default && tax_category = self.class.find_by(is_default: true)
+
+      tax_category.update_column(:is_default, false) unless tax_category == self
     end
   end
 end
diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb
index d7dd93db8f..33878099b2 100644
--- a/app/models/spree/tax_rate.rb
+++ b/app/models/spree/tax_rate.rb
@@ -3,9 +3,11 @@
 module Spree
   class DefaultTaxZoneValidator < ActiveModel::Validator
     def validate(record)
-      if record.included_in_price
-        record.errors.add(:included_in_price, Spree.t(:included_price_validation)) unless Zone.default_tax
-      end
+      return unless record.included_in_price
+
+      return if Zone.default_tax
+
+      record.errors.add(:included_in_price, Spree.t(:included_price_validation))
     end
   end
 end
@@ -42,15 +44,15 @@ module Spree
       end
     end
 
-    # For Vat the default rate is the rate that is configured for the default category
-    # It is needed for every price calculation (as all customer facing prices include vat )
-    # The function returns the actual amount, which may be 0 in case of wrong setup, but is never nil
+    # For VAT, the default rate is the rate that is configured for the default category
+    # It is needed for every price calculation (as all customer facing prices include VAT)
+    # Here we return the actual amount, which may be 0 in case of wrong setup, but is never nil
     def self.default
-      category = TaxCategory.includes(:tax_rates).where(is_default: true).first
+      category = TaxCategory.includes(:tax_rates).find_by(is_default: true)
       return 0 unless category
 
       address ||= Address.new(country_id: Spree::Config[:default_country_id])
-      rate = category.tax_rates.detect { |rate| rate.zone.include? address }.try(:amount)
+      rate = category.tax_rates.detect { |tax_rate| tax_rate.zone.include? address }.try(:amount)
 
       rate || 0
     end
@@ -79,7 +81,8 @@ module Spree
 
       order.adjustments(:reload)
       order.line_items(:reload)
-      # TaxRate adjustments (order.adjustments.tax) and price adjustments (tax included on line items) consist of 100% tax
+      # TaxRate adjustments (order.adjustments.tax)
+      #   and price adjustments (tax included on line items) consist of 100% tax
       (order.adjustments.tax + order.price_adjustments).each do |adjustment|
         adjustment.set_absolute_included_tax! adjustment.amount
       end
diff --git a/spec/models/spree/tax_category_spec.rb b/spec/models/spree/tax_category_spec.rb
index 4ae82eb02d..fb4bd99bc0 100644
--- a/spec/models/spree/tax_category_spec.rb
+++ b/spec/models/spree/tax_category_spec.rb
@@ -19,7 +19,8 @@ describe Spree::TaxCategory do
       tax_category.is_default.should be_false
     end
 
-    it "should undefault the previous default tax category except when updating the existing default tax category" do
+    it "undefaults the previous default tax category
+      except when updating the existing default tax category" do
       tax_category.update_column(:description, "Updated description")
 
       tax_category.reload

From b629a4f91258d7ef31028984c21e013ed35b6122 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 7 Aug 2020 19:30:55 +0100
Subject: [PATCH 05/10] Make new specs pass

---
 app/models/spree/tax_rate.rb           |  6 +--
 spec/models/spree/adjustment_spec.rb   | 52 +++++++++++++-------------
 spec/models/spree/tax_category_spec.rb |  6 +--
 3 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb
index 33878099b2..dcdd078fd7 100644
--- a/app/models/spree/tax_rate.rb
+++ b/app/models/spree/tax_rate.rb
@@ -1,4 +1,4 @@
-# frozen_string_literal: true
+# frozen_string_literal: false
 
 module Spree
   class DefaultTaxZoneValidator < ActiveModel::Validator
@@ -26,7 +26,7 @@ module Spree
     scope :by_zone, ->(zone) { where(zone_id: zone) }
 
     # Gets the array of TaxRates appropriate for the specified order
-    def match(order)
+    def self.match(order)
       return [] if order.distributor && !order.distributor.charges_sales_tax
       return [] unless order.tax_zone
 
@@ -39,7 +39,7 @@ module Spree
       order.adjustments.tax.destroy_all
       order.line_item_adjustments.where(originator_type: 'Spree::TaxRate').destroy_all
 
-      match(order).each do |rate|
+      self.match(order).each do |rate|
         rate.adjust(order)
       end
     end
diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 2eec4943c7..cdd01f3e00 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 module Spree
   describe Adjustment do
-    let(:order) { mock_model(Spree::Order, update!: nil) }
+    let(:order) { build(:order) }
     let(:adjustment) { Spree::Adjustment.create(label: "Adjustment", amount: 5) }
 
     describe "scopes" do
@@ -64,18 +64,18 @@ module Spree
         it "should be eligible if mandatory?" do
           adjustment.mandatory = true
           adjustment.set_eligibility
-          adjustment.should be_eligible
+          expect(adjustment).to be_eligible
         end
         it "should be eligible if `promotion?` even if not `mandatory?`" do
           adjustment.should_receive(:promotion?).and_return(true)
           adjustment.mandatory = false
           adjustment.set_eligibility
-          adjustment.should be_eligible
+          expect(adjustment).to be_eligible
         end
         it "should not be eligible unless mandatory?" do
           adjustment.mandatory = false
           adjustment.set_eligibility
-          adjustment.should_not be_eligible
+          expect(adjustment).to_not be_eligible
         end
       end
 
@@ -84,19 +84,19 @@ module Spree
         it "should be eligible if mandatory?" do
           adjustment.mandatory = true
           adjustment.set_eligibility
-          adjustment.should be_eligible
+          expect(adjustment).to be_eligible
         end
         it "should be eligible if not mandatory and eligible for the originator" do
           adjustment.mandatory = false
           adjustment.stub(eligible_for_originator?: true)
           adjustment.set_eligibility
-          adjustment.should be_eligible
+          expect(adjustment).to be_eligible
         end
         it "should not be eligible if not mandatory not eligible for the originator" do
           adjustment.mandatory = false
           adjustment.stub(eligible_for_originator?: false)
           adjustment.set_eligibility
-          adjustment.should_not be_eligible
+          expect(adjustment).to_not be_eligible
         end
       end
     end
@@ -119,50 +119,50 @@ module Spree
       context "#immutable?" do
         it "is true when adjustment state isn't open" do
           adjustment.state = "closed"
-          adjustment.should be_immutable
+          expect(adjustment).to be_immutable
           adjustment.state = "finalized"
-          adjustment.should be_immutable
+          expect(adjustment).to be_immutable
         end
 
         it "is false when adjustment state is open" do
           adjustment.state = "open"
-          adjustment.should_not be_immutable
+          expect(adjustment).to_not be_immutable
         end
       end
 
       context "#finalized?" do
         it "is true when adjustment state is finalized" do
           adjustment.state = "finalized"
-          adjustment.should be_finalized
+          expect(adjustment).to be_finalized
         end
 
         it "is false when adjustment state isn't finalized" do
           adjustment.state = "closed"
-          adjustment.should_not be_finalized
+          expect(adjustment).to_not be_finalized
           adjustment.state = "open"
-          adjustment.should_not be_finalized
+          expect(adjustment).to_not be_finalized
         end
       end
     end
 
     context "#eligible_for_originator?" do
       context "with no originator" do
-        specify { adjustment.should be_eligible_for_originator }
+        specify { expect(adjustment).to be_eligible_for_originator }
       end
       context "with originator that doesn't have 'eligible?'" do
-        before { adjustment.originator = mock_model(Spree::TaxRate) }
-        specify { adjustment.should be_eligible_for_originator }
+        before { adjustment.originator = build(:tax_rate) }
+        specify { expect(adjustment).to be_eligible_for_originator }
       end
       context "with originator that has 'eligible?'" do
         let(:originator) { Spree::TaxRate.new }
         before { adjustment.originator = originator }
         context "and originator is eligible for order" do
           before { originator.stub(eligible?: true) }
-          specify { adjustment.should be_eligible_for_originator }
+          specify { expect(adjustment).to be_eligible_for_originator }
         end
         context "and originator is not eligible for order" do
           before { originator.stub(eligible?: false) }
-          specify { adjustment.should_not be_eligible_for_originator }
+          specify { expect(adjustment).to_not be_eligible_for_originator }
         end
       end
     end
@@ -174,7 +174,7 @@ module Spree
         before { Spree::Config[:display_currency] = true }
 
         it "shows the currency" do
-          adjustment.display_amount.to_s.should == "$10.55 USD"
+          expect(adjustment.display_amount.to_s).to eq "$10.55 #{Spree::Config[:currency]}"
         end
       end
 
@@ -182,7 +182,7 @@ module Spree
         before { Spree::Config[:display_currency] = false }
 
         it "does not include the currency" do
-          adjustment.display_amount.to_s.should == "$10.55"
+          expect(adjustment.display_amount.to_s).to eq "$10.55"
         end
       end
 
@@ -194,13 +194,13 @@ module Spree
           end
 
           it "displays in JPY" do
-            adjustment.display_amount.to_s.should == "¥11"
+            expect(adjustment.display_amount.to_s).to eq "¥11"
           end
         end
 
         context "when adjustable is nil" do
           it "displays in the default currency" do
-            adjustment.display_amount.to_s.should == "$10.55"
+            expect(adjustment.display_amount.to_s).to eq "$10.55"
           end
         end
       end
@@ -208,7 +208,7 @@ module Spree
 
     context '#currency' do
       it 'returns the globally configured currency' do
-        adjustment.currency.should == 'USD'
+        expect(adjustment.currency).to eq Spree::Config[:currency]
       end
     end
 
@@ -415,8 +415,10 @@ module Spree
         end
 
         context "when enterprise fees inherit their tax_category from the product they are applied to" do
-          let(:product_tax_rate)             { create(:tax_rate, included_in_price: true, calculator: ::Calculator::DefaultTax.new, zone: zone, amount: 0.2) }
-          let(:product_tax_category)         { create(:tax_category, tax_rates: [product_tax_rate]) }
+          let(:product_tax_rate) {
+            create(:tax_rate, included_in_price: true, calculator: ::Calculator::DefaultTax.new, zone: zone, amount: 0.2)
+          }
+          let(:product_tax_category) { create(:tax_category, tax_rates: [product_tax_rate]) }
 
           before do
             variant.product.update_attribute(:tax_category_id, product_tax_category.id)
diff --git a/spec/models/spree/tax_category_spec.rb b/spec/models/spree/tax_category_spec.rb
index fb4bd99bc0..77e9a02501 100644
--- a/spec/models/spree/tax_category_spec.rb
+++ b/spec/models/spree/tax_category_spec.rb
@@ -13,10 +13,10 @@ describe Spree::TaxCategory do
 
     it "should undefault the previous default tax category" do
       new_tax_category.update({ is_default: true })
-      new_tax_category.is_default.should be_true
+      expect(new_tax_category.is_default).to be_truthy
 
       tax_category.reload
-      tax_category.is_default.should be_false
+      expect(tax_category.is_default).to be_falsy
     end
 
     it "undefaults the previous default tax category
@@ -24,7 +24,7 @@ describe Spree::TaxCategory do
       tax_category.update_column(:description, "Updated description")
 
       tax_category.reload
-      tax_category.is_default.should be_true
+      expect(tax_category.is_default).to be_truthy
     end
   end
 end

From e96428e7e2e01954d0c3572b791f375b90d9644e Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 7 Aug 2020 19:32:07 +0100
Subject: [PATCH 06/10] Transpec adjustment_spec

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

diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index cdd01f3e00..97fedd8645 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -18,31 +18,31 @@ module Spree
       context "when originator present" do
         let(:originator) { double("originator", update_adjustment: nil) }
         before do
-          originator.stub update_amount: true
-          adjustment.stub originator: originator, label: 'adjustment', amount: 0
+          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
-          originator.should_not_receive(:update_adjustment)
+          expect(originator).not_to receive(:update_adjustment)
           adjustment.update!
         end
         it "should do nothing when finalized" do
           adjustment.finalize
-          originator.should_not_receive(:update_adjustment)
+          expect(originator).not_to receive(:update_adjustment)
           adjustment.update!
         end
         it "should set the eligibility" do
-          adjustment.should_receive(:set_eligibility)
+          expect(adjustment).to receive(:set_eligibility)
           adjustment.update!
         end
         it "should ask the originator to update_adjustment" do
-          originator.should_receive(:update_adjustment)
+          expect(originator).to receive(:update_adjustment)
           adjustment.update!
         end
       end
       it "should do nothing when originator is nil" do
-        adjustment.stub originator: nil
-        adjustment.should_not_receive(:amount=)
+        allow(adjustment).to receive_messages originator: nil
+        expect(adjustment).not_to receive(:amount=)
         adjustment.update!
       end
     end
@@ -67,7 +67,7 @@ module Spree
           expect(adjustment).to be_eligible
         end
         it "should be eligible if `promotion?` even if not `mandatory?`" do
-          adjustment.should_receive(:promotion?).and_return(true)
+          expect(adjustment).to receive(:promotion?).and_return(true)
           adjustment.mandatory = false
           adjustment.set_eligibility
           expect(adjustment).to be_eligible
@@ -88,13 +88,13 @@ module Spree
         end
         it "should be eligible if not mandatory and eligible for the originator" do
           adjustment.mandatory = false
-          adjustment.stub(eligible_for_originator?: true)
+          allow(adjustment).to receive_messages(eligible_for_originator?: true)
           adjustment.set_eligibility
           expect(adjustment).to be_eligible
         end
         it "should not be eligible if not mandatory not eligible for the originator" do
           adjustment.mandatory = false
-          adjustment.stub(eligible_for_originator?: false)
+          allow(adjustment).to receive_messages(eligible_for_originator?: false)
           adjustment.set_eligibility
           expect(adjustment).to_not be_eligible
         end
@@ -108,7 +108,7 @@ module Spree
           amount: 10,
           label: "Foo"
         )
-        order.should_receive(:update!)
+        expect(order).to receive(:update!)
         adjustment.save
       end
     end
@@ -157,11 +157,11 @@ module Spree
         let(:originator) { Spree::TaxRate.new }
         before { adjustment.originator = originator }
         context "and originator is eligible for order" do
-          before { originator.stub(eligible?: true) }
+          before { allow(originator).to receive_messages(eligible?: true) }
           specify { expect(adjustment).to be_eligible_for_originator }
         end
         context "and originator is not eligible for order" do
-          before { originator.stub(eligible?: false) }
+          before { allow(originator).to receive_messages(eligible?: false) }
           specify { expect(adjustment).to_not be_eligible_for_originator }
         end
       end
@@ -189,7 +189,7 @@ module Spree
       context "with currency set to JPY" do
         context "when adjustable is set to an order" do
           before do
-            order.stub(:currency) { 'JPY' }
+            allow(order).to receive(:currency) { 'JPY' }
             adjustment.adjustable = order
           end
 

From 51ed9a6b786eaabf1287a75cf8c6c6d5cb7d8b76 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 7 Aug 2020 19:38:56 +0100
Subject: [PATCH 07/10] Fix comment and point out that it's a fix to a spree
 issue

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

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index add4e33391..d87a0e7314 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -120,7 +120,7 @@ module Spree
     def update!(calculable = nil)
       return if immutable?
 
-      # Fix for #3381
+      # 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

From 4931edc67cb4693e7345ad22b2faa73a1f7784dc Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 7 Aug 2020 19:44:12 +0100
Subject: [PATCH 08/10] Remove code related to promotions, we dont have
 promotions in OFN

---
 app/models/spree/adjustment.rb         |  7 +-----
 app/models/spree/payment/processing.rb |  2 +-
 spec/models/spree/adjustment_spec.rb   | 30 +++++++++++---------------
 3 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index d87a0e7314..ec86edcad4 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -67,7 +67,6 @@ module Spree
     scope :optional, -> { where(mandatory: false) }
     scope :charge, -> { where('amount >= 0') }
     scope :credit, -> { where('amount < 0') }
-    scope :promotion, -> { where(originator_type: 'Spree::PromotionAction') }
     scope :return_authorization, -> { where(source_type: "Spree::ReturnAuthorization") }
 
     scope :enterprise_fee, -> { where(originator_type: 'EnterpriseFee') }
@@ -84,14 +83,10 @@ module Spree
 
     localize_number :amount
 
-    def promotion?
-      originator_type == 'Spree::PromotionAction'
-    end
-
     # Update the boolean _eligible_ attribute which determines which adjustments
     # count towards the order's adjustment_total.
     def set_eligibility
-      result = mandatory || ((amount != 0 || promotion?) && eligible_for_originator?)
+      result = mandatory || (amount != 0 && eligible_for_originator?)
       update_column(:eligible, result)
     end
 
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 2579f76de7..c3316a898d 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -182,7 +182,7 @@ module Spree
         options.merge!({ shipping: order.ship_total * 100,
                          tax: order.tax_total * 100,
                          subtotal: order.item_total * 100,
-                         discount: order.promo_total * 100,
+                         discount: 0,
                          currency: currency })
 
         options.merge!({ billing_address: order.bill_address.try(:active_merchant_hash),
diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 97fedd8645..4c1252900b 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -21,25 +21,30 @@ module Spree
           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)
           adjustment.update!
         end
+
         it "should do nothing when finalized" do
           adjustment.finalize
           expect(originator).not_to receive(:update_adjustment)
           adjustment.update!
         end
+
         it "should set the eligibility" do
           expect(adjustment).to receive(:set_eligibility)
           adjustment.update!
         end
+
         it "should ask the originator to update_adjustment" do
           expect(originator).to receive(:update_adjustment)
           adjustment.update!
         end
       end
+
       it "should do nothing when originator is nil" do
         allow(adjustment).to receive_messages originator: nil
         expect(adjustment).not_to receive(:amount=)
@@ -47,17 +52,6 @@ module Spree
       end
     end
 
-    context "#promotion?" do
-      it "returns false if not promotion adjustment" do
-        expect(adjustment.promotion?).to eq false
-      end
-
-      it "returns true if promotion adjustment" do
-        adjustment.originator_type = "Spree::PromotionAction"
-        expect(adjustment.promotion?).to eq true
-      end
-    end
-
     context "#eligible? after #set_eligibility" do
       context "when amount is 0" do
         before { adjustment.amount = 0 }
@@ -66,12 +60,7 @@ module Spree
           adjustment.set_eligibility
           expect(adjustment).to be_eligible
         end
-        it "should be eligible if `promotion?` even if not `mandatory?`" do
-          expect(adjustment).to receive(:promotion?).and_return(true)
-          adjustment.mandatory = false
-          adjustment.set_eligibility
-          expect(adjustment).to be_eligible
-        end
+
         it "should not be eligible unless mandatory?" do
           adjustment.mandatory = false
           adjustment.set_eligibility
@@ -81,17 +70,20 @@ module Spree
 
       context "when amount is greater than 0" do
         before { adjustment.amount = 25.00 }
+
         it "should be eligible if mandatory?" do
           adjustment.mandatory = true
           adjustment.set_eligibility
           expect(adjustment).to be_eligible
         end
+
         it "should be eligible if not mandatory and eligible for the originator" do
           adjustment.mandatory = false
           allow(adjustment).to receive_messages(eligible_for_originator?: true)
           adjustment.set_eligibility
           expect(adjustment).to be_eligible
         end
+
         it "should not be eligible if not mandatory not eligible for the originator" do
           adjustment.mandatory = false
           allow(adjustment).to receive_messages(eligible_for_originator?: false)
@@ -149,17 +141,21 @@ module Spree
       context "with no originator" do
         specify { expect(adjustment).to be_eligible_for_originator }
       end
+
       context "with originator that doesn't have 'eligible?'" do
         before { adjustment.originator = build(:tax_rate) }
         specify { expect(adjustment).to be_eligible_for_originator }
       end
+
       context "with originator that has 'eligible?'" do
         let(:originator) { Spree::TaxRate.new }
         before { adjustment.originator = originator }
+
         context "and originator is eligible for order" do
           before { allow(originator).to receive_messages(eligible?: true) }
           specify { expect(adjustment).to be_eligible_for_originator }
         end
+
         context "and originator is not eligible for order" do
           before { allow(originator).to receive_messages(eligible?: false) }
           specify { expect(adjustment).to_not be_eligible_for_originator }

From 8867ec977cd1d965206fb1076eca708584dce32b Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 3 Sep 2020 22:52:17 +0100
Subject: [PATCH 09/10] Bring missing factory from spree_core and use ofn's
 calculator

---
 spec/factories/return_authorization_factory.rb | 9 +++++++++
 spec/models/spree/adjustment_spec.rb           | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)
 create mode 100644 spec/factories/return_authorization_factory.rb

diff --git a/spec/factories/return_authorization_factory.rb b/spec/factories/return_authorization_factory.rb
new file mode 100644
index 0000000000..ad77351a40
--- /dev/null
+++ b/spec/factories/return_authorization_factory.rb
@@ -0,0 +1,9 @@
+FactoryBot.define do
+  factory :return_authorization, class: Spree::ReturnAuthorization do
+    number '100'
+    amount 100.00
+    association(:order, factory: :shipped_order)
+    reason 'no particular reason'
+    state 'received'
+  end
+end
diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb
index 4c1252900b..ef66733ec1 100644
--- a/spec/models/spree/adjustment_spec.rb
+++ b/spec/models/spree/adjustment_spec.rb
@@ -245,7 +245,7 @@ module Spree
         let!(:zone)        { create(:zone_with_member) }
         let!(:order)       { create(:order, bill_address: create(:address)) }
         let!(:line_item)   { create(:line_item, order: order) }
-        let(:tax_rate)     { create(:tax_rate, included_in_price: true, calculator: Calculator::FlatRate.new(preferred_amount: 0.1)) }
+        let(:tax_rate)     { create(:tax_rate, included_in_price: true, calculator: ::Calculator::FlatRate.new(preferred_amount: 0.1)) }
         let(:adjustment)   { line_item.adjustments(:reload).first }
 
         before do

From 6e8fe080cb632471693c066e1cd74412bc4d8d70 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Sat, 5 Sep 2020 18:39:56 +0100
Subject: [PATCH 10/10] Fix easy rubocop issues

---
 app/models/spree/adjustment.rb         |  2 +-
 app/models/spree/payment/processing.rb | 10 +++++-----
 app/models/spree/tax_rate.rb           |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index ec86edcad4..8d398860fc 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -128,7 +128,7 @@ module Spree
     end
 
     def display_amount
-      Spree::Money.new(amount, { currency: currency })
+      Spree::Money.new(amount, currency: currency)
     end
 
     def immutable?
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index c3316a898d..c5a9979ee3 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -179,11 +179,11 @@ module Spree
                     # For more information, please see Spree::Payment#set_unique_identifier
                     order_id: gateway_order_id }
 
-        options.merge!({ shipping: order.ship_total * 100,
-                         tax: order.tax_total * 100,
-                         subtotal: order.item_total * 100,
-                         discount: 0,
-                         currency: currency })
+        options.merge!(shipping: order.ship_total * 100,
+                       tax: order.tax_total * 100,
+                       subtotal: order.item_total * 100,
+                       discount: 0,
+                       currency: currency)
 
         options.merge!({ billing_address: order.bill_address.try(:active_merchant_hash),
                          shipping_address: order.ship_address.try(:active_merchant_hash) })
diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb
index dcdd078fd7..0e25c59b63 100644
--- a/app/models/spree/tax_rate.rb
+++ b/app/models/spree/tax_rate.rb
@@ -39,7 +39,7 @@ module Spree
       order.adjustments.tax.destroy_all
       order.line_item_adjustments.where(originator_type: 'Spree::TaxRate').destroy_all
 
-      self.match(order).each do |rate|
+      match(order).each do |rate|
         rate.adjust(order)
       end
     end