From 06aa56164f3cf9fd34df521d7effaae9e43b64d0 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 12:06:38 +0200
Subject: [PATCH 01/43] Bring in Payment model from Spree

---
 app/models/spree/payment.rb                | 158 +++++
 app/models/spree/payment/processing.rb     | 210 +++++++
 spec/factories.rb                          |  16 +
 spec/models/spree/payment_original_spec.rb | 639 +++++++++++++++++++++
 4 files changed, 1023 insertions(+)
 create mode 100644 app/models/spree/payment.rb
 create mode 100644 app/models/spree/payment/processing.rb
 create mode 100644 spec/models/spree/payment_original_spec.rb

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
new file mode 100644
index 0000000000..ccda57105c
--- /dev/null
+++ b/app/models/spree/payment.rb
@@ -0,0 +1,158 @@
+module Spree
+  class Payment < ActiveRecord::Base
+    include Spree::Payment::Processing
+
+    IDENTIFIER_CHARS = (('A'..'Z').to_a + ('0'..'9').to_a - %w(0 1 I O)).freeze
+
+    belongs_to :order, class_name: 'Spree::Order'
+    belongs_to :source, polymorphic: true
+    belongs_to :payment_method, class_name: 'Spree::PaymentMethod'
+
+    has_many :offsets, -> { where("source_type = 'Spree::Payment' AND amount < 0 AND state = 'completed'") },
+      class_name: "Spree::Payment", foreign_key: :source_id
+    has_many :log_entries, as: :source
+
+    before_validation :validate_source
+    before_save :set_unique_identifier
+
+    after_save :create_payment_profile, if: :profiles_supported?
+
+    # update the order totals, etc.
+    after_save :update_order
+    # invalidate previously entered payments
+    after_create :invalidate_old_payments
+
+    attr_accessor :source_attributes
+    after_initialize :build_source
+
+    scope :from_credit_card, -> { where(source_type: 'Spree::CreditCard') }
+    scope :with_state, ->(s) { where(state: s.to_s) }
+    scope :completed, -> { with_state('completed') }
+    scope :pending, -> { with_state('pending') }
+    scope :failed, -> { with_state('failed') }
+    scope :valid, -> { where('state NOT IN (?)', %w(failed invalid)) }
+
+    after_rollback :persist_invalid
+
+    def persist_invalid
+      return unless ['failed', 'invalid'].include?(state)
+      state_will_change!
+      save 
+    end
+
+    # order state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
+    state_machine initial: :checkout do
+      # With card payments, happens before purchase or authorization happens
+      event :started_processing do
+        transition from: [:checkout, :pending, :completed, :processing], to: :processing
+      end
+      # When processing during checkout fails
+      event :failure do
+        transition from: [:pending, :processing], to: :failed
+      end
+      # With card payments this represents authorizing the payment
+      event :pend do
+        transition from: [:checkout, :processing], to: :pending
+      end
+      # With card payments this represents completing a purchase or capture transaction
+      event :complete do
+        transition from: [:processing, :pending, :checkout], to: :completed
+      end
+      event :void do
+        transition from: [:pending, :completed, :checkout], to: :void
+      end
+      # when the card brand isnt supported
+      event :invalidate do
+        transition from: [:checkout], to: :invalid
+      end
+    end
+
+    def currency
+      order.currency
+    end
+
+    def money
+      Spree::Money.new(amount, { currency: currency })
+    end
+    alias display_amount money
+
+    def offsets_total
+      offsets.pluck(:amount).sum
+    end
+
+    def credit_allowed
+      amount - offsets_total
+    end
+
+    def can_credit?
+      credit_allowed > 0
+    end
+
+    # see https://github.com/spree/spree/issues/981
+    def build_source
+      return if source_attributes.nil?
+      if payment_method and payment_method.payment_source_class
+        self.source = payment_method.payment_source_class.new(source_attributes)
+      end
+    end
+
+    def actions
+      return [] unless payment_source and payment_source.respond_to? :actions
+      payment_source.actions.select { |action| !payment_source.respond_to?("can_#{action}?") or payment_source.send("can_#{action}?", self) }
+    end
+
+    def payment_source
+      res = source.is_a?(Payment) ? source.source : source
+      res || payment_method
+    end
+
+    private
+
+    def validate_source
+      if source && !source.valid?
+        source.errors.each do |field, error|
+          field_name = I18n.t("activerecord.attributes.#{source.class.to_s.underscore}.#{field}")
+          self.errors.add(Spree.t(source.class.to_s.demodulize.underscore), "#{field_name} #{error}")
+        end
+      end
+      return !errors.present?
+    end
+
+    def profiles_supported?
+      payment_method.respond_to?(:payment_profiles_supported?) && payment_method.payment_profiles_supported?
+    end
+
+    def create_payment_profile
+      return unless source.is_a?(CreditCard) && source.number && !source.has_payment_profile?
+      payment_method.create_profile(self)
+    rescue ActiveMerchant::ConnectionError => e
+      gateway_error e
+    end
+
+    def invalidate_old_payments
+      order.payments.with_state('checkout').where("id != ?", self.id).each do |payment|
+        payment.invalidate!
+      end
+    end
+
+    def update_order
+      order.payments.reload
+      order.update!
+    end
+
+    # Necessary because some payment gateways will refuse payments with
+    # duplicate IDs. We *were* using the Order number, but that's set once and
+    # is unchanging. What we need is a unique identifier on a per-payment basis,
+    # and this is it. Related to #1998.
+    # See https://github.com/spree/spree/issues/1998#issuecomment-12869105
+    def set_unique_identifier
+      begin
+        self.identifier = generate_identifier
+      end while self.class.exists?(identifier: self.identifier)
+    end
+
+    def generate_identifier
+      Array.new(8){ IDENTIFIER_CHARS.sample }.join
+    end
+  end
+end
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
new file mode 100644
index 0000000000..98e2cf4c1e
--- /dev/null
+++ b/app/models/spree/payment/processing.rb
@@ -0,0 +1,210 @@
+module Spree
+  class Payment < ActiveRecord::Base
+    module Processing
+      def process!
+        if payment_method && payment_method.source_required?
+          if source
+            if !processing?
+              if payment_method.supports?(source)
+                if payment_method.auto_capture?
+                  purchase!
+                else
+                  authorize!
+                end
+              else
+                invalidate!
+                raise Core::GatewayError.new(Spree.t(:payment_method_not_supported))
+              end
+            end
+          else
+            raise Core::GatewayError.new(Spree.t(:payment_processing_failed))
+          end
+        end
+      end
+
+      def authorize!
+        started_processing!
+        gateway_action(source, :authorize, :pend)
+      end
+
+      def purchase!
+        started_processing!
+        gateway_action(source, :purchase, :complete)
+      end
+
+      def capture!
+        return true if completed?
+        started_processing!
+        protect_from_connection_error do
+          check_environment
+
+          if payment_method.payment_profiles_supported?
+            # Gateways supporting payment profiles will need access to credit card object because this stores the payment profile information
+            # so supply the authorization itself as well as the credit card, rather than just the authorization code
+            response = payment_method.capture(self, source, gateway_options)
+          else
+            # Standard ActiveMerchant capture usage
+            response = payment_method.capture(money.money.cents,
+                                              response_code,
+                                              gateway_options)
+          end
+
+          handle_response(response, :complete, :failure)
+        end
+      end
+
+      def void_transaction!
+        return true if void?
+        protect_from_connection_error do
+          check_environment
+
+          if payment_method.payment_profiles_supported?
+            # Gateways supporting payment profiles will need access to credit card object because this stores the payment profile information
+            # so supply the authorization itself as well as the credit card, rather than just the authorization code
+            response = payment_method.void(self.response_code, source, gateway_options)
+          else
+            # Standard ActiveMerchant void usage
+            response = payment_method.void(self.response_code, gateway_options)
+          end
+          record_response(response)
+
+          if response.success?
+            self.response_code = response.authorization
+            self.void
+          else
+            gateway_error(response)
+          end
+        end
+      end
+
+      def credit!(credit_amount=nil)
+        protect_from_connection_error do
+          check_environment
+
+          credit_amount ||= credit_allowed >= order.outstanding_balance.abs ? order.outstanding_balance.abs : credit_allowed.abs
+          credit_amount = credit_amount.to_f
+
+          if payment_method.payment_profiles_supported?
+            response = payment_method.credit((credit_amount * 100).round, source, response_code, gateway_options)
+          else
+            response = payment_method.credit((credit_amount * 100).round, response_code, gateway_options)
+          end
+
+          record_response(response)
+
+          if response.success?
+            self.class.create(
+              :order => order,
+              :source => self,
+              :payment_method => payment_method,
+              :amount => credit_amount.abs * -1,
+              :response_code => response.authorization,
+              :state => 'completed'
+            )
+          else
+            gateway_error(response)
+          end
+        end
+      end
+
+      def partial_credit(amount)
+        return if amount > credit_allowed
+        started_processing!
+        credit!(amount)
+      end
+
+      def gateway_options
+        options = { :email    => order.email,
+                    :customer => order.email,
+                    :ip       => order.last_ip_address,
+                    # Need to pass in a unique identifier here to make some
+                    # payment gateways happy.
+                    #
+                    # 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 => order.promo_total * 100,
+                         :currency => currency })
+
+        options.merge!({ :billing_address  => order.bill_address.try(:active_merchant_hash),
+                        :shipping_address => order.ship_address.try(:active_merchant_hash) })
+
+        options
+      end
+
+      private
+
+      def gateway_action(source, action, success_state)
+        protect_from_connection_error do
+          check_environment
+
+          response = payment_method.send(action, (amount * 100).round,
+                                         source,
+                                         gateway_options)
+          handle_response(response, success_state, :failure)
+        end
+      end
+
+      def handle_response(response, success_state, failure_state)
+        record_response(response)
+
+        if response.success?
+          unless response.authorization.nil?
+            self.response_code = response.authorization
+            self.avs_response = response.avs_result['code']
+
+            if response.cvv_result
+              self.cvv_response_code = response.cvv_result['code']
+              self.cvv_response_message = response.cvv_result['message']
+            end
+          end
+          self.send("#{success_state}!")
+        else
+          self.send(failure_state)
+          gateway_error(response)
+        end
+      end
+
+      def record_response(response)
+        log_entries.create(:details => response.to_yaml)
+      end
+
+      def protect_from_connection_error
+        begin
+          yield
+        rescue ActiveMerchant::ConnectionError => e
+          gateway_error(e)
+        end
+      end
+
+      def gateway_error(error)
+        if error.is_a? ActiveMerchant::Billing::Response
+          text = error.params['message'] || error.params['response_reason_text'] || error.message
+        elsif error.is_a? ActiveMerchant::ConnectionError
+          text = Spree.t(:unable_to_connect_to_gateway)
+        else
+          text = error.to_s
+        end
+        logger.error(Spree.t(:gateway_error))
+        logger.error("  #{error.to_yaml}")
+        raise Core::GatewayError.new(text)
+      end
+
+      # Saftey check to make sure we're not accidentally performing operations on a live gateway.
+      # Ex. When testing in staging environment with a copy of production data.
+      def check_environment
+        return if payment_method.environment == Rails.env
+        message = Spree.t(:gateway_config_unavailable) + " - #{Rails.env}"
+        raise Core::GatewayError.new(message)
+      end
+
+      # The unique identifier to be passed in to the payment gateway
+      def gateway_order_id
+        "#{order.number}-#{self.identifier}"
+      end
+    end
+  end
+end
diff --git a/spec/factories.rb b/spec/factories.rb
index ad3c804c92..327565ea1b 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -15,6 +15,22 @@ require 'spree/testing_support/factories'
 #   * order_with_inventory_unit_shipped
 #   * completed_order_with_totals
 #
+
+# allows credit card info to be saved to the database which is needed for factories to work properly
+class TestCard < Spree::CreditCard
+  def remove_readonly_attributes(attributes) attributes; end
+end
+
+FactoryBot.define do
+  factory :credit_card, class: TestCard do
+    verification_value 123
+    month 12
+    year { Time.now.year + 1 }
+    number '4111111111111111'
+    cc_type 'visa'
+  end
+end
+
 FactoryBot.define do
   factory :classification, class: Spree::Classification do
   end
diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
new file mode 100644
index 0000000000..8f28ac14dd
--- /dev/null
+++ b/spec/models/spree/payment_original_spec.rb
@@ -0,0 +1,639 @@
+require 'spec_helper'
+
+describe Spree::Payment do
+  let(:order) do
+    order = Spree::Order.new(:bill_address => Spree::Address.new,
+                             :ship_address => Spree::Address.new)
+  end
+
+  let(:gateway) do
+    gateway = Spree::Gateway::Bogus.new(:environment => 'test', :active => true)
+    gateway.stub :source_required => true
+    gateway
+  end
+
+  let(:card) do
+    mock_model(Spree::CreditCard, :number => "4111111111111111",
+                                  :has_payment_profile? => true)
+  end
+
+  let(:payment) do
+    payment = Spree::Payment.new
+    payment.source = card
+    payment.order = order
+    payment.payment_method = gateway
+    payment
+  end
+
+  let(:amount_in_cents) { payment.amount.to_f * 100 }
+
+  let!(:success_response) do
+    double('success_response', :success? => true,
+           :authorization => '123',
+           :avs_result => { 'code' => 'avs-code' },
+           :cvv_result => { 'code' => 'cvv-code', 'message' => "CVV Result"})
+  end
+
+  let(:failed_response) { double('gateway_response', :success? => false) }
+
+  before(:each) do
+    # So it doesn't create log entries every time a processing method is called
+    payment.log_entries.stub(:create)
+  end
+
+  context 'validations' do
+    it "returns useful error messages when source is invalid" do
+      payment.source = Spree::CreditCard.new
+      payment.should_not be_valid
+      cc_errors = payment.errors['Credit Card']
+      cc_errors.should include("Number can't be blank")
+      cc_errors.should include("Month is not a number")
+      cc_errors.should include("Year is not a number")
+      cc_errors.should include("Verification Value can't be blank")
+    end
+  end
+
+  # Regression test for https://github.com/spree/spree/pull/2224
+  context 'failure' do
+
+    it 'should transition to failed from pending state' do
+      payment.state = 'pending'
+      payment.failure
+      payment.state.should eql('failed')
+    end
+
+    it 'should transition to failed from processing state' do
+      payment.state = 'processing'
+      payment.failure
+      payment.state.should eql('failed')
+    end
+
+  end
+
+  context 'invalidate' do
+    it 'should transition from checkout to invalid' do
+      payment.state = 'checkout'
+      payment.invalidate
+      payment.state.should eq('invalid')
+    end
+  end
+
+  context "processing" do
+    before do
+      payment.stub(:update_order)
+      payment.stub(:create_payment_profile)
+    end
+
+    context "#process!" do
+      it "should purchase if with auto_capture" do
+        payment.payment_method.should_receive(:auto_capture?).and_return(true)
+        payment.should_receive(:purchase!)
+        payment.process!
+      end
+
+      it "should authorize without auto_capture" do
+        payment.payment_method.should_receive(:auto_capture?).and_return(false)
+        payment.should_receive(:authorize!)
+        payment.process!
+      end
+
+      it "should make the state 'processing'" do
+        payment.should_receive(:started_processing!)
+        payment.process!
+      end
+
+      it "should invalidate if payment method doesnt support source" do
+        payment.payment_method.should_receive(:supports?).with(payment.source).and_return(false)
+        expect { payment.process!}.to raise_error(Spree::Core::GatewayError)
+        payment.state.should eq('invalid')
+      end
+
+    end
+
+    context "#authorize" do
+      it "should call authorize on the gateway with the payment amount" do
+        payment.payment_method.should_receive(:authorize).with(amount_in_cents,
+                                                               card,
+                                                               anything).and_return(success_response)
+        payment.authorize!
+      end
+
+      it "should call authorize on the gateway with the currency code" do
+        payment.stub :currency => 'GBP'
+        payment.payment_method.should_receive(:authorize).with(amount_in_cents,
+                                                               card,
+                                                               hash_including({:currency => "GBP"})).and_return(success_response)
+        payment.authorize!
+      end
+
+      it "should log the response" do
+        payment.log_entries.should_receive(:create).with(:details => anything)
+        payment.authorize!
+      end
+
+      context "when gateway does not match the environment" do
+        it "should raise an exception" do
+          gateway.stub :environment => "foo"
+          expect { payment.authorize! }.to raise_error(Spree::Core::GatewayError)
+        end
+      end
+
+      context "if successful" do
+        before do
+          payment.payment_method.should_receive(:authorize).with(amount_in_cents,
+                                                                 card,
+                                                                 anything).and_return(success_response)
+        end
+
+        it "should store the response_code, avs_response and cvv_response fields" do
+          payment.authorize!
+          payment.response_code.should == '123'
+          payment.avs_response.should == 'avs-code'
+          payment.cvv_response_code.should == 'cvv-code'
+          payment.cvv_response_message.should == 'CVV Result'
+        end
+
+        it "should make payment pending" do
+          payment.should_receive(:pend!)
+          payment.authorize!
+        end
+      end
+
+      context "if unsuccessful" do
+        it "should mark payment as failed" do
+          gateway.stub(:authorize).and_return(failed_response)
+          payment.should_receive(:failure)
+          payment.should_not_receive(:pend)
+          lambda {
+            payment.authorize!
+          }.should raise_error(Spree::Core::GatewayError)
+        end
+      end
+    end
+
+    context "purchase" do
+      it "should call purchase on the gateway with the payment amount" do
+        gateway.should_receive(:purchase).with(amount_in_cents, card, anything).and_return(success_response)
+        payment.purchase!
+      end
+
+      it "should log the response" do
+        payment.log_entries.should_receive(:create).with(:details => anything)
+        payment.purchase!
+      end
+
+      context "when gateway does not match the environment" do
+        it "should raise an exception" do
+          gateway.stub :environment => "foo"
+          expect { payment.purchase!  }.to raise_error(Spree::Core::GatewayError)
+        end
+      end
+
+      context "if successful" do
+        before do
+          payment.payment_method.should_receive(:purchase).with(amount_in_cents,
+                                                                card,
+                                                                anything).and_return(success_response)
+        end
+
+        it "should store the response_code and avs_response" do
+          payment.purchase!
+          payment.response_code.should == '123'
+          payment.avs_response.should == 'avs-code'
+        end
+
+        it "should make payment complete" do
+          payment.should_receive(:complete!)
+          payment.purchase!
+        end
+      end
+
+      context "if unsuccessful" do
+        it "should make payment failed" do
+          gateway.stub(:purchase).and_return(failed_response)
+          payment.should_receive(:failure)
+          payment.should_not_receive(:pend)
+          expect { payment.purchase! }.to raise_error(Spree::Core::GatewayError)
+        end
+      end
+    end
+
+    context "#capture" do
+      before do
+        payment.stub(:complete).and_return(true)
+      end
+
+      context "when payment is pending" do
+        before do
+          payment.state = 'pending'
+        end
+
+        context "if successful" do
+          before do
+            payment.payment_method.should_receive(:capture).with(payment, card, anything).and_return(success_response)
+          end
+
+          it "should make payment complete" do
+            payment.should_receive(:complete)
+            payment.capture!
+          end
+
+          it "should store the response_code" do
+            gateway.stub :capture => success_response
+            payment.capture!
+            payment.response_code.should == '123'
+          end
+        end
+
+        context "if unsuccessful" do
+          it "should not make payment complete" do
+            gateway.stub :capture => failed_response
+            payment.should_receive(:failure)
+            payment.should_not_receive(:complete)
+            expect { payment.capture! }.to raise_error(Spree::Core::GatewayError)
+          end
+        end
+      end
+
+      # Regression test for #2119
+      context "when payment is completed" do
+        before do
+          payment.state = 'completed'
+        end
+
+        it "should do nothing" do
+          payment.should_not_receive(:complete)
+          payment.payment_method.should_not_receive(:capture)
+          payment.log_entries.should_not_receive(:create)
+          payment.capture!
+        end
+      end
+    end
+
+    context "#void" do
+      before do
+        payment.response_code = '123'
+        payment.state = 'pending'
+      end
+
+      context "when profiles are supported" do
+        it "should call payment_gateway.void with the payment's response_code" do
+          gateway.stub :payment_profiles_supported? => true
+          gateway.should_receive(:void).with('123', card, anything).and_return(success_response)
+          payment.void_transaction!
+        end
+      end
+
+      context "when profiles are not supported" do
+        it "should call payment_gateway.void with the payment's response_code" do
+          gateway.stub :payment_profiles_supported? => false
+          gateway.should_receive(:void).with('123', anything).and_return(success_response)
+          payment.void_transaction!
+        end
+      end
+
+      it "should log the response" do
+        payment.log_entries.should_receive(:create).with(:details => anything)
+        payment.void_transaction!
+      end
+
+      context "when gateway does not match the environment" do
+        it "should raise an exception" do
+          gateway.stub :environment => "foo"
+          expect { payment.void_transaction! }.to raise_error(Spree::Core::GatewayError)
+        end
+      end
+
+      context "if successful" do
+        it "should update the response_code with the authorization from the gateway" do
+          # Change it to something different
+          payment.response_code = 'abc'
+          payment.void_transaction!
+          payment.response_code.should == '12345'
+        end
+      end
+
+      context "if unsuccessful" do
+        it "should not void the payment" do
+          gateway.stub :void => failed_response
+          payment.should_not_receive(:void)
+          expect { payment.void_transaction! }.to raise_error(Spree::Core::GatewayError)
+        end
+      end
+
+      # Regression test for #2119
+      context "if payment is already voided" do
+        before do
+          payment.state = 'void'
+        end
+
+        it "should not void the payment" do
+          payment.payment_method.should_not_receive(:void)
+          payment.void_transaction!
+        end
+      end
+    end
+
+    context "#credit" do
+      before do
+        payment.state = 'complete'
+        payment.response_code = '123'
+      end
+
+      context "when outstanding_balance is less than payment amount" do
+        before do
+          payment.order.stub :outstanding_balance => 10
+          payment.stub :credit_allowed => 1000
+        end
+
+        it "should call credit on the gateway with the credit amount and response_code" do
+          gateway.should_receive(:credit).with(1000, card, '123', anything).and_return(success_response)
+          payment.credit!
+        end
+      end
+
+      context "when outstanding_balance is equal to payment amount" do
+        before do
+          payment.order.stub :outstanding_balance => payment.amount
+        end
+
+        it "should call credit on the gateway with the credit amount and response_code" do
+          gateway.should_receive(:credit).with(amount_in_cents, card, '123', anything).and_return(success_response)
+          payment.credit!
+        end
+      end
+
+      context "when outstanding_balance is greater than payment amount" do
+        before do
+          payment.order.stub :outstanding_balance => 101
+        end
+
+        it "should call credit on the gateway with the original payment amount and response_code" do
+          gateway.should_receive(:credit).with(amount_in_cents.to_f, card, '123', anything).and_return(success_response)
+          payment.credit!
+        end
+      end
+
+      it "should log the response" do
+        payment.log_entries.should_receive(:create).with(:details => anything)
+        payment.credit!
+      end
+
+      context "when gateway does not match the environment" do
+        it "should raise an exception" do
+          gateway.stub :environment => "foo"
+          lambda { payment.credit! }.should raise_error(Spree::Core::GatewayError)
+        end
+      end
+
+      context "when response is successful" do
+        it "should create an offsetting payment" do
+          Spree::Payment.should_receive(:create)
+          payment.credit!
+        end
+
+        it "resulting payment should have correct values" do
+          payment.order.stub :outstanding_balance => 100
+          payment.stub :credit_allowed => 10
+
+          offsetting_payment = payment.credit!
+          offsetting_payment.amount.to_f.should == -10
+          offsetting_payment.should be_completed
+          offsetting_payment.response_code.should == '12345'
+          offsetting_payment.source.should == payment
+        end
+      end
+    end
+  end
+
+  context "when response is unsuccessful" do
+    it "should not create a payment" do
+      gateway.stub :credit => failed_response
+      Spree::Payment.should_not_receive(:create)
+      expect { payment.credit! }.to raise_error(Spree::Core::GatewayError)
+    end
+  end
+
+  context "when already processing" do
+    it "should return nil without trying to process the source" do
+      payment.state = 'processing'
+
+      payment.should_not_receive(:authorize!)
+      payment.should_not_receive(:purchase!)
+      payment.process!.should be_nil
+    end
+  end
+
+  context "with source required" do
+    context "raises an error if no source is specified" do
+      before do
+        payment.source = nil
+      end
+
+      specify do
+        expect { payment.process! }.to raise_error(Spree::Core::GatewayError, Spree.t(:payment_processing_failed))
+      end
+    end
+  end
+
+  context "with source optional" do
+    context "raises no error if source is not specified" do
+      before do
+        payment.source = nil
+        payment.payment_method.stub(:source_required? => false)
+      end
+
+      specify do
+        expect { payment.process! }.not_to raise_error
+      end
+    end
+  end
+
+  context "#credit_allowed" do
+    it "is the difference between offsets total and payment amount" do
+      payment.amount = 100
+      payment.stub(:offsets_total).and_return(0)
+      payment.credit_allowed.should == 100
+      payment.stub(:offsets_total).and_return(80)
+      payment.credit_allowed.should == 20
+    end
+  end
+
+  context "#can_credit?" do
+    it "is true if credit_allowed > 0" do
+      payment.stub(:credit_allowed).and_return(100)
+      payment.can_credit?.should be_true
+    end
+    it "is false if credit_allowed is 0" do
+      payment.stub(:credit_allowed).and_return(0)
+      payment.can_credit?.should be_false
+    end
+  end
+
+  context "#credit" do
+    context "when amount <= credit_allowed" do
+      it "makes the state processing" do
+        payment.state = 'completed'
+        payment.stub(:credit_allowed).and_return(10)
+        payment.partial_credit(10)
+        payment.should be_processing
+      end
+      it "calls credit on the source with the payment and amount" do
+        payment.state = 'completed'
+        payment.stub(:credit_allowed).and_return(10)
+        payment.should_receive(:credit!).with(10)
+        payment.partial_credit(10)
+      end
+    end
+    context "when amount > credit_allowed" do
+      it "should not call credit on the source" do
+        payment.state = 'completed'
+        payment.stub(:credit_allowed).and_return(10)
+        payment.partial_credit(20)
+        payment.should be_completed
+      end
+    end
+  end
+
+  context "#save" do
+    it "should call order#update!" do
+      payment = Spree::Payment.create(:amount => 100, :order => order)
+      order.should_receive(:update!)
+      payment.save
+    end
+
+    context "when profiles are supported" do
+      before do
+        gateway.stub :payment_profiles_supported? => true
+        payment.source.stub :has_payment_profile? => false
+      end
+
+
+      context "when there is an error connecting to the gateway" do
+        it "should call gateway_error " do
+          pending '[Spree build] Failing spec'
+          message = double("gateway_error")
+          connection_error = ActiveMerchant::ConnectionError.new(message, nil)
+          expect(gateway).to receive(:create_profile).and_raise(connection_error)
+          lambda do
+            Spree::Payment.create(
+              :amount => 100,
+              :order => order,
+              :source => card,
+              :payment_method => gateway
+            )
+          end.should raise_error(Spree::Core::GatewayError)
+        end
+      end
+
+      context "when successfully connecting to the gateway" do
+        it "should create a payment profile" do
+          payment.payment_method.should_receive :create_profile
+          payment = Spree::Payment.create(
+            :amount => 100,
+            :order => order,
+            :source => card,
+            :payment_method => gateway
+          )
+        end
+      end
+
+
+    end
+
+    context "when profiles are not supported" do
+      before { gateway.stub :payment_profiles_supported? => false }
+
+      it "should not create a payment profile" do
+        gateway.should_not_receive :create_profile
+        payment = Spree::Payment.create(
+          :amount => 100,
+          :order => order,
+          :source => card,
+          :payment_method => gateway
+        )
+      end
+    end
+  end
+
+  context "#build_source" do
+    it "should build the payment's source" do
+      params = { :amount => 100, :payment_method => gateway,
+                 :source_attributes => {
+        :expiry =>"1 / 99",
+        :number => '1234567890123',
+        :verification_value => '123'
+      }
+      }
+
+      payment = Spree::Payment.new(params)
+      payment.should be_valid
+      payment.source.should_not be_nil
+    end
+
+    it "errors when payment source not valid" do
+      params = { :amount => 100, :payment_method => gateway,
+                 :source_attributes => {:expiry => "1 / 12" }}
+
+      payment = Spree::Payment.new(params)
+      payment.should_not be_valid
+      payment.source.should_not be_nil
+      payment.source.should have(1).error_on(:number)
+      payment.source.should have(1).error_on(:verification_value)
+    end
+  end
+
+  context "#currency" do
+    before { order.stub(:currency) { "ABC" } }
+    it "returns the order currency" do
+      payment.currency.should == "ABC"
+    end
+  end
+
+  context "#display_amount" do
+    it "returns a Spree::Money for this amount" do
+      payment.display_amount.should == Spree::Money.new(payment.amount)
+    end
+  end
+
+  # Regression test for #2216
+  context "#gateway_options" do
+    before { order.stub(:last_ip_address => "192.168.1.1") }
+
+    it "contains an IP" do
+      payment.gateway_options[:ip].should == order.last_ip_address
+    end
+  end
+
+  context "#set_unique_identifier" do
+    # Regression test for #1998
+    it "sets a unique identifier on create" do
+      payment.run_callbacks(:save)
+      payment.identifier.should_not be_blank
+      payment.identifier.size.should == 8
+      payment.identifier.should be_a(String)
+    end
+
+    context "other payment exists" do
+      let(:other_payment) {
+        payment = Spree::Payment.new
+        payment.source = card
+        payment.order = order
+        payment.payment_method = gateway
+        payment
+      }
+
+      before { other_payment.save! }
+
+      it "doesn't set duplicate identifier" do
+        payment.should_receive(:generate_identifier).and_return(other_payment.identifier)
+        payment.should_receive(:generate_identifier).and_call_original
+
+        payment.run_callbacks(:save)
+
+        payment.identifier.should_not be_blank
+        payment.identifier.should_not == other_payment.identifier
+      end
+    end
+  end
+end

From abacd06f6ba004e0dfacca994118e98cb9f492a1 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 12:10:21 +0200
Subject: [PATCH 02/43] Fix credit card instance in specs

---
 spec/factories.rb                          | 4 ----
 spec/models/spree/payment_original_spec.rb | 7 +++++--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/spec/factories.rb b/spec/factories.rb
index 327565ea1b..edfe1d257b 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -198,10 +198,6 @@ FactoryBot.modify do
     country { Spree::Country.find_by name: 'Australia' || Spree::Country.first }
   end
 
-  factory :credit_card do
-    cc_type 'visa'
-  end
-
   factory :payment do
     transient do
       distributor {
diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 8f28ac14dd..3a228c5ee9 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -13,8 +13,11 @@ describe Spree::Payment do
   end
 
   let(:card) do
-    mock_model(Spree::CreditCard, :number => "4111111111111111",
-                                  :has_payment_profile? => true)
+    create(:credit_card, :number => "4111111111111111")
+  end
+
+  before do
+    allow(card).to receive(:has_payment_profile?).and_return(true)
   end
 
   let(:payment) do

From e1ea5dbcb35c8b568a16502ddbaa8c5d74a1dfd4 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 12:32:32 +0200
Subject: [PATCH 03/43] Fix all but the 7 last payment specs

---
 spec/models/spree/payment_original_spec.rb | 102 ++++++++++-----------
 1 file changed, 49 insertions(+), 53 deletions(-)

diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 3a228c5ee9..89bd9afdb4 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -12,13 +12,9 @@ describe Spree::Payment do
     gateway
   end
 
-  let(:card) do
-    create(:credit_card, :number => "4111111111111111")
-  end
+  let(:card) { create(:credit_card) }
 
-  before do
-    allow(card).to receive(:has_payment_profile?).and_return(true)
-  end
+  before { allow(card).to receive(:has_payment_profile?).and_return(true) }
 
   let(:payment) do
     payment = Spree::Payment.new
@@ -41,18 +37,18 @@ describe Spree::Payment do
 
   before(:each) do
     # So it doesn't create log entries every time a processing method is called
-    payment.log_entries.stub(:create)
+    allow(payment.log_entries).to receive(:create)
   end
 
   context 'validations' do
     it "returns useful error messages when source is invalid" do
       payment.source = Spree::CreditCard.new
-      payment.should_not be_valid
+      expect(payment).not_to be_valid
       cc_errors = payment.errors['Credit Card']
-      cc_errors.should include("Number can't be blank")
-      cc_errors.should include("Month is not a number")
-      cc_errors.should include("Year is not a number")
-      cc_errors.should include("Verification Value can't be blank")
+      expect(cc_errors).to include("Number can't be blank")
+      expect(cc_errors).to include("Month is not a number")
+      expect(cc_errors).to include("Year is not a number")
+      expect(cc_errors).to include("Verification Value can't be blank")
     end
   end
 
@@ -62,13 +58,13 @@ describe Spree::Payment do
     it 'should transition to failed from pending state' do
       payment.state = 'pending'
       payment.failure
-      payment.state.should eql('failed')
+      expect(payment.state).to eql('failed')
     end
 
     it 'should transition to failed from processing state' do
       payment.state = 'processing'
       payment.failure
-      payment.state.should eql('failed')
+      expect(payment.state).to eql('failed')
     end
 
   end
@@ -77,7 +73,7 @@ describe Spree::Payment do
     it 'should transition from checkout to invalid' do
       payment.state = 'checkout'
       payment.invalidate
-      payment.state.should eq('invalid')
+      expect(payment.state).to eq('invalid')
     end
   end
 
@@ -108,7 +104,7 @@ describe Spree::Payment do
       it "should invalidate if payment method doesnt support source" do
         payment.payment_method.should_receive(:supports?).with(payment.source).and_return(false)
         expect { payment.process!}.to raise_error(Spree::Core::GatewayError)
-        payment.state.should eq('invalid')
+        expect(payment.state).to eq('invalid')
       end
 
     end
@@ -130,7 +126,7 @@ describe Spree::Payment do
       end
 
       it "should log the response" do
-        payment.log_entries.should_receive(:create).with(:details => anything)
+        expect(payment.log_entries).to receive(:create).with(:details => anything)
         payment.authorize!
       end
 
@@ -150,10 +146,10 @@ describe Spree::Payment do
 
         it "should store the response_code, avs_response and cvv_response fields" do
           payment.authorize!
-          payment.response_code.should == '123'
-          payment.avs_response.should == 'avs-code'
-          payment.cvv_response_code.should == 'cvv-code'
-          payment.cvv_response_message.should == 'CVV Result'
+          expect(payment.response_code).to eq('123')
+          expect(payment.avs_response).to eq('avs-code')
+          expect(payment.cvv_response_code).to eq('cvv-code')
+          expect(payment.cvv_response_message).to eq('CVV Result')
         end
 
         it "should make payment pending" do
@@ -167,9 +163,9 @@ describe Spree::Payment do
           gateway.stub(:authorize).and_return(failed_response)
           payment.should_receive(:failure)
           payment.should_not_receive(:pend)
-          lambda {
+          expect {
             payment.authorize!
-          }.should raise_error(Spree::Core::GatewayError)
+          }.to raise_error(Spree::Core::GatewayError)
         end
       end
     end
@@ -201,8 +197,8 @@ describe Spree::Payment do
 
         it "should store the response_code and avs_response" do
           payment.purchase!
-          payment.response_code.should == '123'
-          payment.avs_response.should == 'avs-code'
+          expect(payment.response_code).to eq('123')
+          expect(payment.avs_response).to eq('avs-code')
         end
 
         it "should make payment complete" do
@@ -244,7 +240,7 @@ describe Spree::Payment do
           it "should store the response_code" do
             gateway.stub :capture => success_response
             payment.capture!
-            payment.response_code.should == '123'
+            expect(payment.response_code).to eq('123')
           end
         end
 
@@ -312,7 +308,7 @@ describe Spree::Payment do
           # Change it to something different
           payment.response_code = 'abc'
           payment.void_transaction!
-          payment.response_code.should == '12345'
+          expect(payment.response_code).to eq('12345')
         end
       end
 
@@ -385,7 +381,7 @@ describe Spree::Payment do
       context "when gateway does not match the environment" do
         it "should raise an exception" do
           gateway.stub :environment => "foo"
-          lambda { payment.credit! }.should raise_error(Spree::Core::GatewayError)
+          expect { payment.credit! }.to raise_error(Spree::Core::GatewayError)
         end
       end
 
@@ -401,9 +397,9 @@ describe Spree::Payment do
 
           offsetting_payment = payment.credit!
           offsetting_payment.amount.to_f.should == -10
-          offsetting_payment.should be_completed
-          offsetting_payment.response_code.should == '12345'
-          offsetting_payment.source.should == payment
+          expect(offsetting_payment).to be_completed
+          expect(offsetting_payment.response_code).to eq('12345')
+          expect(offsetting_payment.source).to eq(payment)
         end
       end
     end
@@ -423,7 +419,7 @@ describe Spree::Payment do
 
       payment.should_not_receive(:authorize!)
       payment.should_not_receive(:purchase!)
-      payment.process!.should be_nil
+      expect(payment.process!).to be_nil
     end
   end
 
@@ -456,20 +452,20 @@ describe Spree::Payment do
     it "is the difference between offsets total and payment amount" do
       payment.amount = 100
       payment.stub(:offsets_total).and_return(0)
-      payment.credit_allowed.should == 100
+      expect(payment.credit_allowed).to eq(100)
       payment.stub(:offsets_total).and_return(80)
-      payment.credit_allowed.should == 20
+      expect(payment.credit_allowed).to eq(20)
     end
   end
 
   context "#can_credit?" do
     it "is true if credit_allowed > 0" do
       payment.stub(:credit_allowed).and_return(100)
-      payment.can_credit?.should be_true
+      expect(payment.can_credit?).to be true
     end
     it "is false if credit_allowed is 0" do
       payment.stub(:credit_allowed).and_return(0)
-      payment.can_credit?.should be_false
+      expect(payment.can_credit?).to be false
     end
   end
 
@@ -479,7 +475,7 @@ describe Spree::Payment do
         payment.state = 'completed'
         payment.stub(:credit_allowed).and_return(10)
         payment.partial_credit(10)
-        payment.should be_processing
+        expect(payment).to be_processing
       end
       it "calls credit on the source with the payment and amount" do
         payment.state = 'completed'
@@ -493,7 +489,7 @@ describe Spree::Payment do
         payment.state = 'completed'
         payment.stub(:credit_allowed).and_return(10)
         payment.partial_credit(20)
-        payment.should be_completed
+        expect(payment).to be_completed
       end
     end
   end
@@ -518,7 +514,7 @@ describe Spree::Payment do
           message = double("gateway_error")
           connection_error = ActiveMerchant::ConnectionError.new(message, nil)
           expect(gateway).to receive(:create_profile).and_raise(connection_error)
-          lambda do
+          expect do
             Spree::Payment.create(
               :amount => 100,
               :order => order,
@@ -570,8 +566,8 @@ describe Spree::Payment do
       }
 
       payment = Spree::Payment.new(params)
-      payment.should be_valid
-      payment.source.should_not be_nil
+      expect(payment).to be_valid
+      expect(payment.source).not_to be_nil
     end
 
     it "errors when payment source not valid" do
@@ -579,23 +575,23 @@ describe Spree::Payment do
                  :source_attributes => {:expiry => "1 / 12" }}
 
       payment = Spree::Payment.new(params)
-      payment.should_not be_valid
-      payment.source.should_not be_nil
-      payment.source.should have(1).error_on(:number)
-      payment.source.should have(1).error_on(:verification_value)
+      expect(payment).not_to be_valid
+      expect(payment.source).not_to be_nil
+      expect(payment.source.errors[:number]).not_to be_empty
+      expect(payment.source.errors[:verification_value]).not_to be_empty
     end
   end
 
   context "#currency" do
     before { order.stub(:currency) { "ABC" } }
     it "returns the order currency" do
-      payment.currency.should == "ABC"
+      expect(payment.currency).to eq("ABC")
     end
   end
 
   context "#display_amount" do
     it "returns a Spree::Money for this amount" do
-      payment.display_amount.should == Spree::Money.new(payment.amount)
+      expect(payment.display_amount).to eq(Spree::Money.new(payment.amount))
     end
   end
 
@@ -604,7 +600,7 @@ describe Spree::Payment do
     before { order.stub(:last_ip_address => "192.168.1.1") }
 
     it "contains an IP" do
-      payment.gateway_options[:ip].should == order.last_ip_address
+      expect(payment.gateway_options[:ip]).to eq(order.last_ip_address)
     end
   end
 
@@ -612,9 +608,9 @@ describe Spree::Payment do
     # Regression test for #1998
     it "sets a unique identifier on create" do
       payment.run_callbacks(:save)
-      payment.identifier.should_not be_blank
-      payment.identifier.size.should == 8
-      payment.identifier.should be_a(String)
+      expect(payment.identifier).not_to be_blank
+      expect(payment.identifier.size).to eq(8)
+      expect(payment.identifier).to be_a(String)
     end
 
     context "other payment exists" do
@@ -634,8 +630,8 @@ describe Spree::Payment do
 
         payment.run_callbacks(:save)
 
-        payment.identifier.should_not be_blank
-        payment.identifier.should_not == other_payment.identifier
+        expect(payment.identifier).not_to be_blank
+        expect(payment.identifier).not_to eq(other_payment.identifier)
       end
     end
   end

From 34de219233c9fc9d2dd884e2b07fa24eb0bdda91 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 12:36:51 +0200
Subject: [PATCH 04/43] Bring in missing translation

---
 config/locales/en.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 3726167337..2975120a53 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -34,6 +34,7 @@ en:
         email: Customer E-Mail
       spree/payment:
         amount: Amount
+        state: State
       spree/product:
         primary_taxon: "Product Category"
         supplier: "Supplier"

From a01f601363301137af9e348477c83ec6ceeaf8ce Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 13:06:11 +0200
Subject: [PATCH 05/43] Fix yet another spec

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

diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 89bd9afdb4..2ca0089d9d 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -396,7 +396,7 @@ describe Spree::Payment do
           payment.stub :credit_allowed => 10
 
           offsetting_payment = payment.credit!
-          offsetting_payment.amount.to_f.should == -10
+          expect(offsetting_payment.amount.to_f).to eq(-10)
           expect(offsetting_payment).to be_completed
           expect(offsetting_payment.response_code).to eq('12345')
           expect(offsetting_payment.source).to eq(payment)

From 0ad8dcc2c5d067ec5983566e40c94f71bc0f9052 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 13:06:35 +0200
Subject: [PATCH 06/43] Fix payment log entries specs

The tight coupling between doesn't give other option but to check the
private method is called. The specs successfully stub
`log_entries#create` but for some reason the model instance that gets
evaluated it's not the stubbed one.
---
 spec/models/spree/payment_original_spec.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 2ca0089d9d..7a0367c9b8 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -37,7 +37,7 @@ describe Spree::Payment do
 
   before(:each) do
     # So it doesn't create log entries every time a processing method is called
-    allow(payment.log_entries).to receive(:create)
+    allow(payment).to receive(:record_response)
   end
 
   context 'validations' do
@@ -126,8 +126,8 @@ describe Spree::Payment do
       end
 
       it "should log the response" do
-        expect(payment.log_entries).to receive(:create).with(:details => anything)
         payment.authorize!
+        expect(payment).to have_received(:record_response)
       end
 
       context "when gateway does not match the environment" do
@@ -177,8 +177,8 @@ describe Spree::Payment do
       end
 
       it "should log the response" do
-        payment.log_entries.should_receive(:create).with(:details => anything)
         payment.purchase!
+        expect(payment).to have_received(:record_response)
       end
 
       context "when gateway does not match the environment" do
@@ -292,8 +292,8 @@ describe Spree::Payment do
       end
 
       it "should log the response" do
-        payment.log_entries.should_receive(:create).with(:details => anything)
         payment.void_transaction!
+        expect(payment).to have_received(:record_response)
       end
 
       context "when gateway does not match the environment" do
@@ -374,8 +374,8 @@ describe Spree::Payment do
       end
 
       it "should log the response" do
-        payment.log_entries.should_receive(:create).with(:details => anything)
         payment.credit!
+        expect(payment).to have_received(:record_response)
       end
 
       context "when gateway does not match the environment" do

From 9935df9f2d590996210613a2061e14e332ed8f25 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 17:11:18 +0200
Subject: [PATCH 07/43] Move Pin payment method from decorator into model

---
 app/models/spree/payment.rb                | 12 ++++++
 app/models/spree/payment_decorator.rb      | 12 ------
 spec/models/spree/payment_original_spec.rb | 43 ++++++++++++++++++++++
 spec/models/spree/payment_spec.rb          | 42 ---------------------
 4 files changed, 55 insertions(+), 54 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index ccda57105c..404a6759ee 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -101,6 +101,18 @@ module Spree
       payment_source.actions.select { |action| !payment_source.respond_to?("can_#{action}?") or payment_source.send("can_#{action}?", self) }
     end
 
+    # Pin payments lacks void and credit methods, but it does have refund
+    # Here we swap credit out for refund and remove void as a possible action
+    def actions_with_pin_payment_adaptations
+      actions = actions_without_pin_payment_adaptations
+      if payment_method.is_a? Gateway::Pin
+        actions << 'refund' if actions.include? 'credit'
+        actions.reject! { |a| ['credit', 'void'].include? a }
+      end
+      actions
+    end
+    alias_method_chain :actions, :pin_payment_adaptations
+
     def payment_source
       res = source.is_a?(Payment) ? source.source : source
       res || payment_method
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
index ec84a83a83..f9e1d244a0 100644
--- a/app/models/spree/payment_decorator.rb
+++ b/app/models/spree/payment_decorator.rb
@@ -35,18 +35,6 @@ module Spree
       I18n.t('payment_method_fee')
     end
 
-    # Pin payments lacks void and credit methods, but it does have refund
-    # Here we swap credit out for refund and remove void as a possible action
-    def actions_with_pin_payment_adaptations
-      actions = actions_without_pin_payment_adaptations
-      if payment_method.is_a? Gateway::Pin
-        actions << 'refund' if actions.include? 'credit'
-        actions.reject! { |a| ['credit', 'void'].include? a }
-      end
-      actions
-    end
-    alias_method_chain :actions, :pin_payment_adaptations
-
     def refund!(refund_amount = nil)
       protect_from_connection_error do
         check_environment
diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 7a0367c9b8..df33d4780f 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -635,4 +635,47 @@ describe Spree::Payment do
       end
     end
   end
+
+  describe "available actions" do
+    context "for most gateways" do
+      let(:payment) { create(:payment, source: create(:credit_card)) }
+
+      it "can capture and void" do
+        expect(payment.actions).to match_array %w(capture void)
+      end
+
+      describe "when a payment has been taken" do
+        before do
+          allow(payment).to receive(:state) { 'completed' }
+          allow(payment).to receive(:order) { double(:order, payment_state: 'credit_owed') }
+        end
+
+        it "can void and credit" do
+          expect(payment.actions).to match_array %w(void credit)
+        end
+      end
+    end
+
+    context "for Pin Payments" do
+      let(:d) { create(:distributor_enterprise) }
+      let(:pin) { Spree::Gateway::Pin.create! name: 'pin', distributor_ids: [d.id] }
+      let(:payment) { create(:payment, source: create(:credit_card), payment_method: pin) }
+
+      it "does not void" do
+        expect(payment.actions).not_to include 'void'
+      end
+
+      describe "when a payment has been taken" do
+        before do
+          allow(payment).to receive(:state) { 'completed' }
+          allow(payment).to receive(:order) { double(:order, payment_state: 'credit_owed') }
+        end
+
+        it "can refund instead of crediting" do
+          expect(payment.actions).not_to include 'credit'
+          expect(payment.actions).to     include 'refund'
+        end
+      end
+    end
+  end
 end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 46a493fa96..ecd5d7b3b6 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -2,48 +2,6 @@ require 'spec_helper'
 
 module Spree
   describe Payment do
-    describe "available actions" do
-      context "for most gateways" do
-        let(:payment) { create(:payment, source: create(:credit_card)) }
-
-        it "can capture and void" do
-          expect(payment.actions).to match_array %w(capture void)
-        end
-
-        describe "when a payment has been taken" do
-          before do
-            allow(payment).to receive(:state) { 'completed' }
-            allow(payment).to receive(:order) { double(:order, payment_state: 'credit_owed') }
-          end
-
-          it "can void and credit" do
-            expect(payment.actions).to match_array %w(void credit)
-          end
-        end
-      end
-
-      context "for Pin Payments" do
-        let(:d) { create(:distributor_enterprise) }
-        let(:pin) { Gateway::Pin.create! name: 'pin', distributor_ids: [d.id] }
-        let(:payment) { create(:payment, source: create(:credit_card), payment_method: pin) }
-
-        it "does not void" do
-          expect(payment.actions).not_to include 'void'
-        end
-
-        describe "when a payment has been taken" do
-          before do
-            allow(payment).to receive(:state) { 'completed' }
-            allow(payment).to receive(:order) { double(:order, payment_state: 'credit_owed') }
-          end
-
-          it "can refund instead of crediting" do
-            expect(payment.actions).not_to include 'credit'
-            expect(payment.actions).to     include 'refund'
-          end
-        end
-      end
-    end
 
     describe "refunding" do
       let(:payment) { create(:payment) }

From 31d0d4bcae96010fb5ff32b56201422b37e5aaf1 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 17:54:12 +0200
Subject: [PATCH 08/43] Fix error "no parent is saved"

The exact error is

```
ActiveRecord::RecordNotSaved:
       You cannot call create unless the parent is saved
```

raised from app/models/spree/payment_decorator.rb:29:in `ensure_correct_adjustment'
---
 spec/models/spree/payment_original_spec.rb | 39 ++++++++++++++++++----
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index df33d4780f..009f55d9d7 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -17,7 +17,7 @@ describe Spree::Payment do
   before { allow(card).to receive(:has_payment_profile?).and_return(true) }
 
   let(:payment) do
-    payment = Spree::Payment.new
+    payment = create(:payment)
     payment.source = card
     payment.order = order
     payment.payment_method = gateway
@@ -54,7 +54,6 @@ describe Spree::Payment do
 
   # Regression test for https://github.com/spree/spree/pull/2224
   context 'failure' do
-
     it 'should transition to failed from pending state' do
       payment.state = 'pending'
       payment.failure
@@ -472,6 +471,12 @@ describe Spree::Payment do
   context "#credit" do
     context "when amount <= credit_allowed" do
       it "makes the state processing" do
+        payment.payment_method.name = 'Gateway'
+        payment.payment_method.distributors << create(:distributor_enterprise)
+        payment.payment_method.save!
+
+        payment.order = create(:order)
+
         payment.state = 'completed'
         payment.stub(:credit_allowed).and_return(10)
         payment.partial_credit(10)
@@ -496,7 +501,12 @@ describe Spree::Payment do
 
   context "#save" do
     it "should call order#update!" do
-      payment = Spree::Payment.create(:amount => 100, :order => order)
+      gateway.name = 'Gateway'
+      gateway.distributors << create(:distributor_enterprise)
+      gateway.save!
+
+      order = create(:order)
+      payment = Spree::Payment.create(:amount => 100, :order => order, :payment_method => gateway)
       order.should_receive(:update!)
       payment.save
     end
@@ -527,10 +537,17 @@ describe Spree::Payment do
 
       context "when successfully connecting to the gateway" do
         it "should create a payment profile" do
-          payment.payment_method.should_receive :create_profile
+          gateway.name = 'Gateway'
+          gateway.distributors << create(:distributor_enterprise)
+          gateway.save!
+
+          payment.payment_method = gateway
+
+          expect(gateway).to receive(:create_profile)
+
           payment = Spree::Payment.create(
             :amount => 100,
-            :order => order,
+            :order => create(:order),
             :source => card,
             :payment_method => gateway
           )
@@ -544,10 +561,14 @@ describe Spree::Payment do
       before { gateway.stub :payment_profiles_supported? => false }
 
       it "should not create a payment profile" do
+        gateway.name = 'Gateway'
+        gateway.distributors << create(:distributor_enterprise)
+        gateway.save!
+
         gateway.should_not_receive :create_profile
         payment = Spree::Payment.create(
           :amount => 100,
-          :order => order,
+          :order => create(:order),
           :source => card,
           :payment_method => gateway
         )
@@ -615,9 +636,13 @@ describe Spree::Payment do
 
     context "other payment exists" do
       let(:other_payment) {
+        gateway.name = 'Gateway'
+        gateway.distributors << create(:distributor_enterprise)
+        gateway.save!
+
         payment = Spree::Payment.new
         payment.source = card
-        payment.order = order
+        payment.order = create(:order)
         payment.payment_method = gateway
         payment
       }

From eafaa97b0ea28184ad256f543e1b2621b6a9ec8a Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:01:16 +0200
Subject: [PATCH 09/43] Temporarily skip spec

I'll move on to other easier issues and get back to it when we're in
a better position.
---
 spec/models/spree/payment_original_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 009f55d9d7..80b9379eab 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -536,7 +536,7 @@ describe Spree::Payment do
       end
 
       context "when successfully connecting to the gateway" do
-        it "should create a payment profile" do
+        xit "should create a payment profile" do
           gateway.name = 'Gateway'
           gateway.distributors << create(:distributor_enterprise)
           gateway.save!

From 322c4d0f3ff190d4c17b5619db01db4fa0e799e2 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:03:53 +0200
Subject: [PATCH 10/43] Move decorator's callbacks to model

---
 app/models/spree/payment.rb           | 2 +-
 app/models/spree/payment_decorator.rb | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 404a6759ee..b3976fcb8d 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -18,7 +18,7 @@ module Spree
     after_save :create_payment_profile, if: :profiles_supported?
 
     # update the order totals, etc.
-    after_save :update_order
+    after_save :ensure_correct_adjustment, :update_order
     # invalidate previously entered payments
     after_create :invalidate_old_payments
 
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
index f9e1d244a0..91b958c159 100644
--- a/app/models/spree/payment_decorator.rb
+++ b/app/models/spree/payment_decorator.rb
@@ -8,8 +8,6 @@ module Spree
 
     has_one :adjustment, as: :source, dependent: :destroy
 
-    after_save :ensure_correct_adjustment, :update_order
-
     localize_number :amount
 
     # We bypass this after_rollback callback that is setup in Spree::Payment

From 6d9a518616e258613a5d0f5103d3afb45fe3f722 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:06:01 +0200
Subject: [PATCH 11/43] Move method from decorator to model

---
 app/models/spree/payment.rb           | 10 +++++++---
 app/models/spree/payment_decorator.rb | 10 ----------
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index b3976fcb8d..82276d427b 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -89,11 +89,15 @@ module Spree
     end
 
     # see https://github.com/spree/spree/issues/981
+    #
+    # Import from future Spree v.2.3.0 d470b31798f37
     def build_source
       return if source_attributes.nil?
-      if payment_method and payment_method.payment_source_class
-        self.source = payment_method.payment_source_class.new(source_attributes)
-      end
+      return unless payment_method.andand.payment_source_class
+
+      self.source = payment_method.payment_source_class.new(source_attributes)
+      source.payment_method_id = payment_method.id
+      source.user_id = order.user_id if order
     end
 
     def actions
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
index 91b958c159..c333fc531d 100644
--- a/app/models/spree/payment_decorator.rb
+++ b/app/models/spree/payment_decorator.rb
@@ -60,16 +60,6 @@ module Spree
       end
     end
 
-    # Import from future Spree v.2.3.0 d470b31798f37
-    def build_source
-      return if source_attributes.nil?
-      return unless payment_method.andand.payment_source_class
-
-      self.source = payment_method.payment_source_class.new(source_attributes)
-      source.payment_method_id = payment_method.id
-      source.user_id = order.user_id if order
-    end
-
     private
 
     def calculate_refund_amount(refund_amount = nil)

From 48910aeb7710270fd0a625d2e55984e69759fa58 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:09:09 +0200
Subject: [PATCH 12/43] Move #refund! to the processing.rb

---
 app/models/spree/payment/processing.rb     | 32 +++++++++
 app/models/spree/payment_decorator.rb      | 32 ---------
 spec/models/spree/payment_original_spec.rb | 80 ++++++++++++++++++++++
 spec/models/spree/payment_spec.rb          | 80 ----------------------
 4 files changed, 112 insertions(+), 112 deletions(-)

diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 98e2cf4c1e..ad7eb87aca 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -107,6 +107,33 @@ module Spree
         end
       end
 
+      def refund!(refund_amount = nil)
+        protect_from_connection_error do
+          check_environment
+
+          refund_amount = calculate_refund_amount(refund_amount)
+
+          if payment_method.payment_profiles_supported?
+            response = payment_method.refund((refund_amount * 100).round, source, response_code, gateway_options)
+          else
+            response = payment_method.refund((refund_amount * 100).round, response_code, gateway_options)
+          end
+
+          record_response(response)
+
+          if response.success?
+            self.class.create(order: order,
+                              source: self,
+                              payment_method: payment_method,
+                              amount: refund_amount.abs * -1,
+                              response_code: response.authorization,
+                              state: 'completed')
+          else
+            gateway_error(response)
+          end
+        end
+      end
+
       def partial_credit(amount)
         return if amount > credit_allowed
         started_processing!
@@ -137,6 +164,11 @@ module Spree
 
       private
 
+      def calculate_refund_amount(refund_amount = nil)
+        refund_amount ||= credit_allowed >= order.outstanding_balance.abs ? order.outstanding_balance.abs : credit_allowed.abs
+        refund_amount.to_f
+      end
+
       def gateway_action(source, action, success_state)
         protect_from_connection_error do
           check_environment
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
index c333fc531d..065b5ca99d 100644
--- a/app/models/spree/payment_decorator.rb
+++ b/app/models/spree/payment_decorator.rb
@@ -33,40 +33,8 @@ module Spree
       I18n.t('payment_method_fee')
     end
 
-    def refund!(refund_amount = nil)
-      protect_from_connection_error do
-        check_environment
-
-        refund_amount = calculate_refund_amount(refund_amount)
-
-        if payment_method.payment_profiles_supported?
-          response = payment_method.refund((refund_amount * 100).round, source, response_code, gateway_options)
-        else
-          response = payment_method.refund((refund_amount * 100).round, response_code, gateway_options)
-        end
-
-        record_response(response)
-
-        if response.success?
-          self.class.create(order: order,
-                            source: self,
-                            payment_method: payment_method,
-                            amount: refund_amount.abs * -1,
-                            response_code: response.authorization,
-                            state: 'completed')
-        else
-          gateway_error(response)
-        end
-      end
-    end
-
     private
 
-    def calculate_refund_amount(refund_amount = nil)
-      refund_amount ||= credit_allowed >= order.outstanding_balance.abs ? order.outstanding_balance.abs : credit_allowed.abs
-      refund_amount.to_f
-    end
-
     def create_payment_profile
       return unless source.is_a?(CreditCard)
       return unless source.try(:save_requested_by_customer?)
diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 80b9379eab..a29d8ccee6 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -703,4 +703,84 @@ describe Spree::Payment do
       end
     end
   end
+
+  describe "refunding" do
+    let(:payment) { create(:payment) }
+    let(:success) { double(success?: true, authorization: 'abc123') }
+    let(:failure) { double(success?: false) }
+
+    it "always checks the environment" do
+      allow(payment.payment_method).to receive(:refund) { success }
+      expect(payment).to receive(:check_environment)
+      payment.refund!
+    end
+
+    describe "calculating refund amount" do
+      it "returns the parameter amount when given" do
+        expect(payment.send(:calculate_refund_amount, 123)).to be === 123.0
+      end
+
+      it "refunds up to the value of the payment when the outstanding balance is larger" do
+        allow(payment).to receive(:credit_allowed) { 123 }
+        allow(payment).to receive(:order) { double(:order, outstanding_balance: 1000) }
+        expect(payment.send(:calculate_refund_amount)).to eq(123)
+      end
+
+      it "refunds up to the outstanding balance of the order when the payment is larger" do
+        allow(payment).to receive(:credit_allowed) { 1000 }
+        allow(payment).to receive(:order) { double(:order, outstanding_balance: 123) }
+        expect(payment.send(:calculate_refund_amount)).to eq(123)
+      end
+    end
+
+    describe "performing refunds" do
+      before do
+        allow(payment).to receive(:calculate_refund_amount) { 123 }
+        expect(payment.payment_method).to receive(:refund).and_return(success)
+      end
+
+      it "performs the refund without payment profiles" do
+        allow(payment.payment_method).to receive(:payment_profiles_supported?) { false }
+        payment.refund!
+      end
+
+      it "performs the refund with payment profiles" do
+        allow(payment.payment_method).to receive(:payment_profiles_supported?) { true }
+        payment.refund!
+      end
+    end
+
+    it "records the response" do
+      allow(payment).to receive(:calculate_refund_amount) { 123 }
+      allow(payment.payment_method).to receive(:refund).and_return(success)
+      expect(payment).to receive(:record_response).with(success)
+      payment.refund!
+    end
+
+    it "records a payment on success" do
+      allow(payment).to receive(:calculate_refund_amount) { 123 }
+      allow(payment.payment_method).to receive(:refund).and_return(success)
+      allow(payment).to receive(:record_response)
+
+      expect do
+        payment.refund!
+      end.to change(Spree::Payment, :count).by(1)
+
+      p = Spree::Payment.last
+      expect(p.order).to eq(payment.order)
+      expect(p.source).to eq(payment)
+      expect(p.payment_method).to eq(payment.payment_method)
+      expect(p.amount).to eq(-123)
+      expect(p.response_code).to eq(success.authorization)
+      expect(p.state).to eq('completed')
+    end
+
+    it "logs the error on failure" do
+      allow(payment).to receive(:calculate_refund_amount) { 123 }
+      allow(payment.payment_method).to receive(:refund).and_return(failure)
+      allow(payment).to receive(:record_response)
+      expect(payment).to receive(:gateway_error).with(failure)
+      payment.refund!
+    end
+  end
 end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index ecd5d7b3b6..55586d368b 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -3,86 +3,6 @@ require 'spec_helper'
 module Spree
   describe Payment do
 
-    describe "refunding" do
-      let(:payment) { create(:payment) }
-      let(:success) { double(success?: true, authorization: 'abc123') }
-      let(:failure) { double(success?: false) }
-
-      it "always checks the environment" do
-        allow(payment.payment_method).to receive(:refund) { success }
-        expect(payment).to receive(:check_environment)
-        payment.refund!
-      end
-
-      describe "calculating refund amount" do
-        it "returns the parameter amount when given" do
-          expect(payment.send(:calculate_refund_amount, 123)).to be === 123.0
-        end
-
-        it "refunds up to the value of the payment when the outstanding balance is larger" do
-          allow(payment).to receive(:credit_allowed) { 123 }
-          allow(payment).to receive(:order) { double(:order, outstanding_balance: 1000) }
-          expect(payment.send(:calculate_refund_amount)).to eq(123)
-        end
-
-        it "refunds up to the outstanding balance of the order when the payment is larger" do
-          allow(payment).to receive(:credit_allowed) { 1000 }
-          allow(payment).to receive(:order) { double(:order, outstanding_balance: 123) }
-          expect(payment.send(:calculate_refund_amount)).to eq(123)
-        end
-      end
-
-      describe "performing refunds" do
-        before do
-          allow(payment).to receive(:calculate_refund_amount) { 123 }
-          expect(payment.payment_method).to receive(:refund).and_return(success)
-        end
-
-        it "performs the refund without payment profiles" do
-          allow(payment.payment_method).to receive(:payment_profiles_supported?) { false }
-          payment.refund!
-        end
-
-        it "performs the refund with payment profiles" do
-          allow(payment.payment_method).to receive(:payment_profiles_supported?) { true }
-          payment.refund!
-        end
-      end
-
-      it "records the response" do
-        allow(payment).to receive(:calculate_refund_amount) { 123 }
-        allow(payment.payment_method).to receive(:refund).and_return(success)
-        expect(payment).to receive(:record_response).with(success)
-        payment.refund!
-      end
-
-      it "records a payment on success" do
-        allow(payment).to receive(:calculate_refund_amount) { 123 }
-        allow(payment.payment_method).to receive(:refund).and_return(success)
-        allow(payment).to receive(:record_response)
-
-        expect do
-          payment.refund!
-        end.to change(Payment, :count).by(1)
-
-        p = Payment.last
-        expect(p.order).to eq(payment.order)
-        expect(p.source).to eq(payment)
-        expect(p.payment_method).to eq(payment.payment_method)
-        expect(p.amount).to eq(-123)
-        expect(p.response_code).to eq(success.authorization)
-        expect(p.state).to eq('completed')
-      end
-
-      it "logs the error on failure" do
-        allow(payment).to receive(:calculate_refund_amount) { 123 }
-        allow(payment.payment_method).to receive(:refund).and_return(failure)
-        allow(payment).to receive(:record_response)
-        expect(payment).to receive(:gateway_error).with(failure)
-        payment.refund!
-      end
-    end
-
     describe "applying transaction fees" do
       let!(:order) { create(:order) }
       let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) }

From 861726200c74e6d68741ebe88bcfcd8b1f89aac0 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:17:07 +0200
Subject: [PATCH 13/43] Move localize_number from decorator to model

---
 app/models/spree/payment.rb                | 3 +++
 app/models/spree/payment_decorator.rb      | 4 ----
 spec/models/spree/payment_original_spec.rb | 4 ++++
 spec/models/spree/payment_spec.rb          | 4 ----
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 82276d427b..6804c9fcd4 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -1,6 +1,9 @@
 module Spree
   class Payment < ActiveRecord::Base
     include Spree::Payment::Processing
+    extend Spree::LocalizedNumber
+
+    localize_number :amount
 
     IDENTIFIER_CHARS = (('A'..'Z').to_a + ('0'..'9').to_a - %w(0 1 I O)).freeze
 
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
index 065b5ca99d..0ccca1c4d4 100644
--- a/app/models/spree/payment_decorator.rb
+++ b/app/models/spree/payment_decorator.rb
@@ -2,14 +2,10 @@ require 'spree/localized_number'
 
 module Spree
   Payment.class_eval do
-    extend Spree::LocalizedNumber
-
     delegate :line_items, to: :order
 
     has_one :adjustment, as: :source, dependent: :destroy
 
-    localize_number :amount
-
     # We bypass this after_rollback callback that is setup in Spree::Payment
     # The issues the callback fixes are not experienced in OFN:
     #   if a payment fails on checkout the state "failed" is persisted correctly
diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index a29d8ccee6..7d3385b3da 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -40,6 +40,10 @@ describe Spree::Payment do
     allow(payment).to receive(:record_response)
   end
 
+  context "extends LocalizedNumber" do
+    it_behaves_like "a model using the LocalizedNumber module", [:amount]
+  end
+
   context 'validations' do
     it "returns useful error messages when source is invalid" do
       payment.source = Spree::CreditCard.new
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 55586d368b..a0e0fd4ff3 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -108,9 +108,5 @@ module Spree
         end
       end
     end
-
-    context "extends LocalizedNumber" do
-      it_behaves_like "a model using the LocalizedNumber module", [:amount]
-    end
   end
 end

From 3fb6193098d1b897bfc01b4906955cc8dbf4a987 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:27:02 +0200
Subject: [PATCH 14/43] Move adjustments logic from decorator into model

---
 app/models/spree/payment.rb                | 32 ++++++++++++++++++++++
 app/models/spree/payment_decorator.rb      | 32 ----------------------
 spec/models/spree/payment_original_spec.rb | 31 +++++++++++++++++++++
 spec/models/spree/payment_spec.rb          | 22 ---------------
 4 files changed, 63 insertions(+), 54 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 6804c9fcd4..ef386f2218 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -15,6 +15,8 @@ module Spree
       class_name: "Spree::Payment", foreign_key: :source_id
     has_many :log_entries, as: :source
 
+    has_one :adjustment, as: :source, dependent: :destroy
+
     before_validation :validate_source
     before_save :set_unique_identifier
 
@@ -125,8 +127,38 @@ module Spree
       res || payment_method
     end
 
+    def ensure_correct_adjustment
+      revoke_adjustment_eligibility if ['failed', 'invalid'].include?(state)
+      return if adjustment.try(:finalized?)
+
+      if adjustment
+        adjustment.originator = payment_method
+        adjustment.label = adjustment_label
+        adjustment.save
+      else
+        payment_method.create_adjustment(adjustment_label, order, self, true)
+        association(:adjustment).reload
+      end
+    end
+
+    def adjustment_label
+      I18n.t('payment_method_fee')
+    end
+
     private
 
+    # Don't charge fees for invalid or failed payments.
+    # This is called twice for failed payments, because the persistence of the 'failed'
+    # state is acheived through some trickery using an after_rollback callback on the
+    # payment model. See Spree::Payment#persist_invalid
+    def revoke_adjustment_eligibility
+      return unless adjustment.try(:reload)
+      return if adjustment.finalized?
+
+      adjustment.update_attribute(:eligible, false)
+      adjustment.finalize!
+    end
+
     def validate_source
       if source && !source.valid?
         source.errors.each do |field, error|
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
index 0ccca1c4d4..3c88894bc2 100644
--- a/app/models/spree/payment_decorator.rb
+++ b/app/models/spree/payment_decorator.rb
@@ -4,31 +4,11 @@ module Spree
   Payment.class_eval do
     delegate :line_items, to: :order
 
-    has_one :adjustment, as: :source, dependent: :destroy
-
     # We bypass this after_rollback callback that is setup in Spree::Payment
     # The issues the callback fixes are not experienced in OFN:
     #   if a payment fails on checkout the state "failed" is persisted correctly
     def persist_invalid; end
 
-    def ensure_correct_adjustment
-      revoke_adjustment_eligibility if ['failed', 'invalid'].include?(state)
-      return if adjustment.try(:finalized?)
-
-      if adjustment
-        adjustment.originator = payment_method
-        adjustment.label = adjustment_label
-        adjustment.save
-      else
-        payment_method.create_adjustment(adjustment_label, order, self, true)
-        association(:adjustment).reload
-      end
-    end
-
-    def adjustment_label
-      I18n.t('payment_method_fee')
-    end
-
     private
 
     def create_payment_profile
@@ -41,17 +21,5 @@ module Spree
     rescue ActiveMerchant::ConnectionError => e
       gateway_error e
     end
-
-    # Don't charge fees for invalid or failed payments.
-    # This is called twice for failed payments, because the persistence of the 'failed'
-    # state is acheived through some trickery using an after_rollback callback on the
-    # payment model. See Spree::Payment#persist_invalid
-    def revoke_adjustment_eligibility
-      return unless adjustment.try(:reload)
-      return if adjustment.finalized?
-
-      adjustment.update_attribute(:eligible, false)
-      adjustment.finalize!
-    end
   end
 end
diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 7d3385b3da..70bce9a2b5 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -787,4 +787,35 @@ describe Spree::Payment do
       payment.refund!
     end
   end
+
+  describe "applying transaction fees" do
+    let!(:order) { create(:order) }
+    let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) }
+
+    before do
+      order.reload.update!
+    end
+
+    context "when order-based calculator" do
+      let!(:shop) { create(:enterprise) }
+      let!(:payment_method) { create(:payment_method, calculator: calculator) }
+
+      let!(:calculator) do
+        Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10)
+      end
+
+      context "when order complete and inventory tracking enabled" do
+        let!(:order) { create(:completed_order_with_totals, distributor: shop) }
+        let!(:variant) { order.line_items.first.variant }
+        let!(:inventory_item) { create(:inventory_item, enterprise: shop, variant: variant) }
+
+        it "creates adjustment" do
+          payment = create(:payment, order: order, payment_method: payment_method,
+                           amount: order.total)
+          expect(payment.adjustment).to be_present
+          expect(payment.adjustment.amount).not_to eq(0)
+        end
+      end
+    end
+  end
 end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index a0e0fd4ff3..434ce3a2a3 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -11,28 +11,6 @@ module Spree
         order.reload.update!
       end
 
-      context "when order-based calculator" do
-        let!(:shop) { create(:enterprise) }
-        let!(:payment_method) { create(:payment_method, calculator: calculator) }
-
-        let!(:calculator) do
-          Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10)
-        end
-
-        context "when order complete and inventory tracking enabled" do
-          let!(:order) { create(:completed_order_with_totals, distributor: shop) }
-          let!(:variant) { order.line_items.first.variant }
-          let!(:inventory_item) { create(:inventory_item, enterprise: shop, variant: variant) }
-
-          it "creates adjustment" do
-            payment = create(:payment, order: order, payment_method: payment_method,
-                                       amount: order.total)
-            expect(payment.adjustment).to be_present
-            expect(payment.adjustment.amount).not_to eq(0)
-          end
-        end
-      end
-
       context "to Stripe payments" do
         let(:shop) { create(:enterprise) }
         let(:payment_method) { create(:stripe_payment_method, distributor_ids: [create(:distributor_enterprise).id], preferred_enterprise_id: shop.id) }

From cf6138da6611c94b7da2356f6ca652a543094c09 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:29:26 +0200
Subject: [PATCH 15/43] Replace model method with its decorated version

---
 app/models/spree/payment.rb           |  6 +++++-
 app/models/spree/payment_decorator.rb | 13 -------------
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index ef386f2218..de13489922 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -174,7 +174,11 @@ module Spree
     end
 
     def create_payment_profile
-      return unless source.is_a?(CreditCard) && source.number && !source.has_payment_profile?
+      return unless source.is_a?(CreditCard)
+      return unless source.try(:save_requested_by_customer?)
+      return unless source.number || source.gateway_payment_profile_id
+      return unless source.gateway_customer_profile_id.nil?
+
       payment_method.create_profile(self)
     rescue ActiveMerchant::ConnectionError => e
       gateway_error e
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
index 3c88894bc2..a66346dd7e 100644
--- a/app/models/spree/payment_decorator.rb
+++ b/app/models/spree/payment_decorator.rb
@@ -8,18 +8,5 @@ module Spree
     # The issues the callback fixes are not experienced in OFN:
     #   if a payment fails on checkout the state "failed" is persisted correctly
     def persist_invalid; end
-
-    private
-
-    def create_payment_profile
-      return unless source.is_a?(CreditCard)
-      return unless source.try(:save_requested_by_customer?)
-      return unless source.number || source.gateway_payment_profile_id
-      return unless source.gateway_customer_profile_id.nil?
-
-      payment_method.create_profile(self)
-    rescue ActiveMerchant::ConnectionError => e
-      gateway_error e
-    end
   end
 end

From d49068ce66bbe578d6d36d69eefbe5995c24d916 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:31:19 +0200
Subject: [PATCH 16/43] Move method delegation from decorator to model

---
 app/models/spree/payment.rb           | 2 ++
 app/models/spree/payment_decorator.rb | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index de13489922..a434400442 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -7,6 +7,8 @@ module Spree
 
     IDENTIFIER_CHARS = (('A'..'Z').to_a + ('0'..'9').to_a - %w(0 1 I O)).freeze
 
+    delegate :line_items, to: :order
+
     belongs_to :order, class_name: 'Spree::Order'
     belongs_to :source, polymorphic: true
     belongs_to :payment_method, class_name: 'Spree::PaymentMethod'
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
index a66346dd7e..f79529e306 100644
--- a/app/models/spree/payment_decorator.rb
+++ b/app/models/spree/payment_decorator.rb
@@ -2,8 +2,6 @@ require 'spree/localized_number'
 
 module Spree
   Payment.class_eval do
-    delegate :line_items, to: :order
-
     # We bypass this after_rollback callback that is setup in Spree::Payment
     # The issues the callback fixes are not experienced in OFN:
     #   if a payment fails on checkout the state "failed" is persisted correctly

From d8b748a851d8d964b821b80c451bad80ebb7bfa1 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 26 Jun 2020 18:37:53 +0200
Subject: [PATCH 17/43] Merge alias_method method and its original version

---
 app/models/spree/payment.rb | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index a434400442..8ae3c91c87 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -107,22 +107,19 @@ module Spree
       source.user_id = order.user_id if order
     end
 
-    def actions
-      return [] unless payment_source and payment_source.respond_to? :actions
-      payment_source.actions.select { |action| !payment_source.respond_to?("can_#{action}?") or payment_source.send("can_#{action}?", self) }
-    end
-
     # Pin payments lacks void and credit methods, but it does have refund
     # Here we swap credit out for refund and remove void as a possible action
-    def actions_with_pin_payment_adaptations
-      actions = actions_without_pin_payment_adaptations
+    def actions
+      return [] unless payment_source and payment_source.respond_to? :actions
+      actions = payment_source.actions.select { |action| !payment_source.respond_to?("can_#{action}?") or payment_source.send("can_#{action}?", self) }
+
       if payment_method.is_a? Gateway::Pin
         actions << 'refund' if actions.include? 'credit'
         actions.reject! { |a| ['credit', 'void'].include? a }
       end
+
       actions
     end
-    alias_method_chain :actions, :pin_payment_adaptations
 
     def payment_source
       res = source.is_a?(Payment) ? source.source : source

From 8fbbb0bb6496c04aafb0268725775714730ed5bc Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 10 Jul 2020 11:40:04 +0200
Subject: [PATCH 18/43] Bring back our card factory modification

Merging Spree's an our factory didn't really work.
---
 spec/factories.rb | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/spec/factories.rb b/spec/factories.rb
index edfe1d257b..455fc9b9a4 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -16,21 +16,6 @@ require 'spree/testing_support/factories'
 #   * completed_order_with_totals
 #
 
-# allows credit card info to be saved to the database which is needed for factories to work properly
-class TestCard < Spree::CreditCard
-  def remove_readonly_attributes(attributes) attributes; end
-end
-
-FactoryBot.define do
-  factory :credit_card, class: TestCard do
-    verification_value 123
-    month 12
-    year { Time.now.year + 1 }
-    number '4111111111111111'
-    cc_type 'visa'
-  end
-end
-
 FactoryBot.define do
   factory :classification, class: Spree::Classification do
   end
@@ -198,6 +183,10 @@ FactoryBot.modify do
     country { Spree::Country.find_by name: 'Australia' || Spree::Country.first }
   end
 
+  factory :credit_card do
+    cc_type 'visa'
+  end
+
   factory :payment do
     transient do
       distributor {

From 562f397b22e81d922f79e91af29b621ca12394ad Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 10 Jul 2020 11:46:59 +0200
Subject: [PATCH 19/43] Isolate Spree's specs into their own context

This way we don't mix contexts while merging in our own decorator tests.
---
 spec/models/spree/payment_original_spec.rb | 1354 ++++++++++----------
 1 file changed, 678 insertions(+), 676 deletions(-)

diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 70bce9a2b5..418c09882c 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -1,554 +1,576 @@
 require 'spec_helper'
 
 describe Spree::Payment do
-  let(:order) do
-    order = Spree::Order.new(:bill_address => Spree::Address.new,
-                             :ship_address => Spree::Address.new)
-  end
-
-  let(:gateway) do
-    gateway = Spree::Gateway::Bogus.new(:environment => 'test', :active => true)
-    gateway.stub :source_required => true
-    gateway
-  end
-
-  let(:card) { create(:credit_card) }
-
-  before { allow(card).to receive(:has_payment_profile?).and_return(true) }
-
-  let(:payment) do
-    payment = create(:payment)
-    payment.source = card
-    payment.order = order
-    payment.payment_method = gateway
-    payment
-  end
-
-  let(:amount_in_cents) { payment.amount.to_f * 100 }
-
-  let!(:success_response) do
-    double('success_response', :success? => true,
-           :authorization => '123',
-           :avs_result => { 'code' => 'avs-code' },
-           :cvv_result => { 'code' => 'cvv-code', 'message' => "CVV Result"})
-  end
-
-  let(:failed_response) { double('gateway_response', :success? => false) }
-
-  before(:each) do
-    # So it doesn't create log entries every time a processing method is called
-    allow(payment).to receive(:record_response)
-  end
-
-  context "extends LocalizedNumber" do
-    it_behaves_like "a model using the LocalizedNumber module", [:amount]
-  end
-
-  context 'validations' do
-    it "returns useful error messages when source is invalid" do
-      payment.source = Spree::CreditCard.new
-      expect(payment).not_to be_valid
-      cc_errors = payment.errors['Credit Card']
-      expect(cc_errors).to include("Number can't be blank")
-      expect(cc_errors).to include("Month is not a number")
-      expect(cc_errors).to include("Year is not a number")
-      expect(cc_errors).to include("Verification Value can't be blank")
-    end
-  end
-
-  # Regression test for https://github.com/spree/spree/pull/2224
-  context 'failure' do
-    it 'should transition to failed from pending state' do
-      payment.state = 'pending'
-      payment.failure
-      expect(payment.state).to eql('failed')
+  context 'original specs from Spree' do
+    let(:order) do
+      order = Spree::Order.new(:bill_address => Spree::Address.new,
+                               :ship_address => Spree::Address.new)
     end
 
-    it 'should transition to failed from processing state' do
-      payment.state = 'processing'
-      payment.failure
-      expect(payment.state).to eql('failed')
+    let(:gateway) do
+      gateway = Spree::Gateway::Bogus.new(:environment => 'test', :active => true)
+      gateway.stub :source_required => true
+      gateway
     end
 
-  end
+    let(:card) { create(:credit_card) }
 
-  context 'invalidate' do
-    it 'should transition from checkout to invalid' do
-      payment.state = 'checkout'
-      payment.invalidate
-      expect(payment.state).to eq('invalid')
-    end
-  end
+    before { allow(card).to receive(:has_payment_profile?).and_return(true) }
 
-  context "processing" do
-    before do
-      payment.stub(:update_order)
-      payment.stub(:create_payment_profile)
+    let(:payment) do
+      payment = create(:payment)
+      payment.source = card
+      payment.order = order
+      payment.payment_method = gateway
+      payment
     end
 
-    context "#process!" do
-      it "should purchase if with auto_capture" do
-        payment.payment_method.should_receive(:auto_capture?).and_return(true)
-        payment.should_receive(:purchase!)
-        payment.process!
+    let(:amount_in_cents) { payment.amount.to_f * 100 }
+
+    let!(:success_response) do
+      double('success_response', :success? => true,
+             :authorization => '123',
+             :avs_result => { 'code' => 'avs-code' },
+             :cvv_result => { 'code' => 'cvv-code', 'message' => "CVV Result"})
+    end
+
+    let(:failed_response) { double('gateway_response', :success? => false) }
+
+    before(:each) do
+      # So it doesn't create log entries every time a processing method is called
+      allow(payment).to receive(:record_response)
+    end
+
+    context "extends LocalizedNumber" do
+      it_behaves_like "a model using the LocalizedNumber module", [:amount]
+    end
+
+    context 'validations' do
+      it "returns useful error messages when source is invalid" do
+        payment.source = Spree::CreditCard.new
+        expect(payment).not_to be_valid
+        cc_errors = payment.errors['Credit Card']
+        expect(cc_errors).to include("Number can't be blank")
+        expect(cc_errors).to include("Month is not a number")
+        expect(cc_errors).to include("Year is not a number")
+        expect(cc_errors).to include("Verification Value can't be blank")
+      end
+    end
+
+    # Regression test for https://github.com/spree/spree/pull/2224
+    context 'failure' do
+      it 'should transition to failed from pending state' do
+        payment.state = 'pending'
+        payment.failure
+        expect(payment.state).to eql('failed')
       end
 
-      it "should authorize without auto_capture" do
-        payment.payment_method.should_receive(:auto_capture?).and_return(false)
-        payment.should_receive(:authorize!)
-        payment.process!
+      it 'should transition to failed from processing state' do
+        payment.state = 'processing'
+        payment.failure
+        expect(payment.state).to eql('failed')
       end
 
-      it "should make the state 'processing'" do
-        payment.should_receive(:started_processing!)
-        payment.process!
-      end
+    end
 
-      it "should invalidate if payment method doesnt support source" do
-        payment.payment_method.should_receive(:supports?).with(payment.source).and_return(false)
-        expect { payment.process!}.to raise_error(Spree::Core::GatewayError)
+    context 'invalidate' do
+      it 'should transition from checkout to invalid' do
+        payment.state = 'checkout'
+        payment.invalidate
         expect(payment.state).to eq('invalid')
       end
-
     end
 
-    context "#authorize" do
-      it "should call authorize on the gateway with the payment amount" do
-        payment.payment_method.should_receive(:authorize).with(amount_in_cents,
-                                                               card,
-                                                               anything).and_return(success_response)
-        payment.authorize!
+    context "processing" do
+      before do
+        payment.stub(:update_order)
+        payment.stub(:create_payment_profile)
       end
 
-      it "should call authorize on the gateway with the currency code" do
-        payment.stub :currency => 'GBP'
-        payment.payment_method.should_receive(:authorize).with(amount_in_cents,
-                                                               card,
-                                                               hash_including({:currency => "GBP"})).and_return(success_response)
-        payment.authorize!
-      end
-
-      it "should log the response" do
-        payment.authorize!
-        expect(payment).to have_received(:record_response)
-      end
-
-      context "when gateway does not match the environment" do
-        it "should raise an exception" do
-          gateway.stub :environment => "foo"
-          expect { payment.authorize! }.to raise_error(Spree::Core::GatewayError)
+      context "#process!" do
+        it "should purchase if with auto_capture" do
+          payment.payment_method.should_receive(:auto_capture?).and_return(true)
+          payment.should_receive(:purchase!)
+          payment.process!
         end
+
+        it "should authorize without auto_capture" do
+          payment.payment_method.should_receive(:auto_capture?).and_return(false)
+          payment.should_receive(:authorize!)
+          payment.process!
+        end
+
+        it "should make the state 'processing'" do
+          payment.should_receive(:started_processing!)
+          payment.process!
+        end
+
+        it "should invalidate if payment method doesnt support source" do
+          payment.payment_method.should_receive(:supports?).with(payment.source).and_return(false)
+          expect { payment.process!}.to raise_error(Spree::Core::GatewayError)
+          expect(payment.state).to eq('invalid')
+        end
+
       end
 
-      context "if successful" do
-        before do
+      context "#authorize" do
+        it "should call authorize on the gateway with the payment amount" do
           payment.payment_method.should_receive(:authorize).with(amount_in_cents,
                                                                  card,
                                                                  anything).and_return(success_response)
-        end
-
-        it "should store the response_code, avs_response and cvv_response fields" do
-          payment.authorize!
-          expect(payment.response_code).to eq('123')
-          expect(payment.avs_response).to eq('avs-code')
-          expect(payment.cvv_response_code).to eq('cvv-code')
-          expect(payment.cvv_response_message).to eq('CVV Result')
-        end
-
-        it "should make payment pending" do
-          payment.should_receive(:pend!)
           payment.authorize!
         end
-      end
 
-      context "if unsuccessful" do
-        it "should mark payment as failed" do
-          gateway.stub(:authorize).and_return(failed_response)
-          payment.should_receive(:failure)
-          payment.should_not_receive(:pend)
-          expect {
-            payment.authorize!
-          }.to raise_error(Spree::Core::GatewayError)
-        end
-      end
-    end
-
-    context "purchase" do
-      it "should call purchase on the gateway with the payment amount" do
-        gateway.should_receive(:purchase).with(amount_in_cents, card, anything).and_return(success_response)
-        payment.purchase!
-      end
-
-      it "should log the response" do
-        payment.purchase!
-        expect(payment).to have_received(:record_response)
-      end
-
-      context "when gateway does not match the environment" do
-        it "should raise an exception" do
-          gateway.stub :environment => "foo"
-          expect { payment.purchase!  }.to raise_error(Spree::Core::GatewayError)
-        end
-      end
-
-      context "if successful" do
-        before do
-          payment.payment_method.should_receive(:purchase).with(amount_in_cents,
-                                                                card,
-                                                                anything).and_return(success_response)
+        it "should call authorize on the gateway with the currency code" do
+          payment.stub :currency => 'GBP'
+          payment.payment_method.should_receive(:authorize).with(amount_in_cents,
+                                                                 card,
+                                                                 hash_including({:currency => "GBP"})).and_return(success_response)
+          payment.authorize!
         end
 
-        it "should store the response_code and avs_response" do
-          payment.purchase!
-          expect(payment.response_code).to eq('123')
-          expect(payment.avs_response).to eq('avs-code')
+        it "should log the response" do
+          payment.authorize!
+          expect(payment).to have_received(:record_response)
         end
 
-        it "should make payment complete" do
-          payment.should_receive(:complete!)
-          payment.purchase!
-        end
-      end
-
-      context "if unsuccessful" do
-        it "should make payment failed" do
-          gateway.stub(:purchase).and_return(failed_response)
-          payment.should_receive(:failure)
-          payment.should_not_receive(:pend)
-          expect { payment.purchase! }.to raise_error(Spree::Core::GatewayError)
-        end
-      end
-    end
-
-    context "#capture" do
-      before do
-        payment.stub(:complete).and_return(true)
-      end
-
-      context "when payment is pending" do
-        before do
-          payment.state = 'pending'
+        context "when gateway does not match the environment" do
+          it "should raise an exception" do
+            gateway.stub :environment => "foo"
+            expect { payment.authorize! }.to raise_error(Spree::Core::GatewayError)
+          end
         end
 
         context "if successful" do
           before do
-            payment.payment_method.should_receive(:capture).with(payment, card, anything).and_return(success_response)
+            payment.payment_method.should_receive(:authorize).with(amount_in_cents,
+                                                                   card,
+                                                                   anything).and_return(success_response)
           end
 
-          it "should make payment complete" do
-            payment.should_receive(:complete)
-            payment.capture!
-          end
-
-          it "should store the response_code" do
-            gateway.stub :capture => success_response
-            payment.capture!
+          it "should store the response_code, avs_response and cvv_response fields" do
+            payment.authorize!
             expect(payment.response_code).to eq('123')
+            expect(payment.avs_response).to eq('avs-code')
+            expect(payment.cvv_response_code).to eq('cvv-code')
+            expect(payment.cvv_response_message).to eq('CVV Result')
+          end
+
+          it "should make payment pending" do
+            payment.should_receive(:pend!)
+            payment.authorize!
           end
         end
 
         context "if unsuccessful" do
-          it "should not make payment complete" do
-            gateway.stub :capture => failed_response
+          it "should mark payment as failed" do
+            gateway.stub(:authorize).and_return(failed_response)
             payment.should_receive(:failure)
-            payment.should_not_receive(:complete)
-            expect { payment.capture! }.to raise_error(Spree::Core::GatewayError)
+            payment.should_not_receive(:pend)
+            expect {
+              payment.authorize!
+            }.to raise_error(Spree::Core::GatewayError)
           end
         end
       end
 
-      # Regression test for #2119
-      context "when payment is completed" do
-        before do
-          payment.state = 'completed'
+      context "purchase" do
+        it "should call purchase on the gateway with the payment amount" do
+          gateway.should_receive(:purchase).with(amount_in_cents, card, anything).and_return(success_response)
+          payment.purchase!
         end
 
-        it "should do nothing" do
-          payment.should_not_receive(:complete)
-          payment.payment_method.should_not_receive(:capture)
-          payment.log_entries.should_not_receive(:create)
-          payment.capture!
+        it "should log the response" do
+          payment.purchase!
+          expect(payment).to have_received(:record_response)
+        end
+
+        context "when gateway does not match the environment" do
+          it "should raise an exception" do
+            gateway.stub :environment => "foo"
+            expect { payment.purchase!  }.to raise_error(Spree::Core::GatewayError)
+          end
+        end
+
+        context "if successful" do
+          before do
+            payment.payment_method.should_receive(:purchase).with(amount_in_cents,
+                                                                  card,
+                                                                  anything).and_return(success_response)
+          end
+
+          it "should store the response_code and avs_response" do
+            payment.purchase!
+            expect(payment.response_code).to eq('123')
+            expect(payment.avs_response).to eq('avs-code')
+          end
+
+          it "should make payment complete" do
+            payment.should_receive(:complete!)
+            payment.purchase!
+          end
+        end
+
+        context "if unsuccessful" do
+          it "should make payment failed" do
+            gateway.stub(:purchase).and_return(failed_response)
+            payment.should_receive(:failure)
+            payment.should_not_receive(:pend)
+            expect { payment.purchase! }.to raise_error(Spree::Core::GatewayError)
+          end
+        end
+      end
+
+      context "#capture" do
+        before do
+          payment.stub(:complete).and_return(true)
+        end
+
+        context "when payment is pending" do
+          before do
+            payment.state = 'pending'
+          end
+
+          context "if successful" do
+            before do
+              payment.payment_method.should_receive(:capture).with(payment, card, anything).and_return(success_response)
+            end
+
+            it "should make payment complete" do
+              payment.should_receive(:complete)
+              payment.capture!
+            end
+
+            it "should store the response_code" do
+              gateway.stub :capture => success_response
+              payment.capture!
+              expect(payment.response_code).to eq('123')
+            end
+          end
+
+          context "if unsuccessful" do
+            it "should not make payment complete" do
+              gateway.stub :capture => failed_response
+              payment.should_receive(:failure)
+              payment.should_not_receive(:complete)
+              expect { payment.capture! }.to raise_error(Spree::Core::GatewayError)
+            end
+          end
+        end
+
+        # Regression test for #2119
+        context "when payment is completed" do
+          before do
+            payment.state = 'completed'
+          end
+
+          it "should do nothing" do
+            payment.should_not_receive(:complete)
+            payment.payment_method.should_not_receive(:capture)
+            payment.log_entries.should_not_receive(:create)
+            payment.capture!
+          end
+        end
+      end
+
+      context "#void" do
+        before do
+          payment.response_code = '123'
+          payment.state = 'pending'
+        end
+
+        context "when profiles are supported" do
+          it "should call payment_gateway.void with the payment's response_code" do
+            gateway.stub :payment_profiles_supported? => true
+            gateway.should_receive(:void).with('123', card, anything).and_return(success_response)
+            payment.void_transaction!
+          end
+        end
+
+        context "when profiles are not supported" do
+          it "should call payment_gateway.void with the payment's response_code" do
+            gateway.stub :payment_profiles_supported? => false
+            gateway.should_receive(:void).with('123', anything).and_return(success_response)
+            payment.void_transaction!
+          end
+        end
+
+        it "should log the response" do
+          payment.void_transaction!
+          expect(payment).to have_received(:record_response)
+        end
+
+        context "when gateway does not match the environment" do
+          it "should raise an exception" do
+            gateway.stub :environment => "foo"
+            expect { payment.void_transaction! }.to raise_error(Spree::Core::GatewayError)
+          end
+        end
+
+        context "if successful" do
+          it "should update the response_code with the authorization from the gateway" do
+            # Change it to something different
+            payment.response_code = 'abc'
+            payment.void_transaction!
+            expect(payment.response_code).to eq('12345')
+          end
+        end
+
+        context "if unsuccessful" do
+          it "should not void the payment" do
+            gateway.stub :void => failed_response
+            payment.should_not_receive(:void)
+            expect { payment.void_transaction! }.to raise_error(Spree::Core::GatewayError)
+          end
+        end
+
+        # Regression test for #2119
+        context "if payment is already voided" do
+          before do
+            payment.state = 'void'
+          end
+
+          it "should not void the payment" do
+            payment.payment_method.should_not_receive(:void)
+            payment.void_transaction!
+          end
+        end
+      end
+
+      context "#credit" do
+        before do
+          payment.state = 'complete'
+          payment.response_code = '123'
+        end
+
+        context "when outstanding_balance is less than payment amount" do
+          before do
+            payment.order.stub :outstanding_balance => 10
+            payment.stub :credit_allowed => 1000
+          end
+
+          it "should call credit on the gateway with the credit amount and response_code" do
+            gateway.should_receive(:credit).with(1000, card, '123', anything).and_return(success_response)
+            payment.credit!
+          end
+        end
+
+        context "when outstanding_balance is equal to payment amount" do
+          before do
+            payment.order.stub :outstanding_balance => payment.amount
+          end
+
+          it "should call credit on the gateway with the credit amount and response_code" do
+            gateway.should_receive(:credit).with(amount_in_cents, card, '123', anything).and_return(success_response)
+            payment.credit!
+          end
+        end
+
+        context "when outstanding_balance is greater than payment amount" do
+          before do
+            payment.order.stub :outstanding_balance => 101
+          end
+
+          it "should call credit on the gateway with the original payment amount and response_code" do
+            gateway.should_receive(:credit).with(amount_in_cents.to_f, card, '123', anything).and_return(success_response)
+            payment.credit!
+          end
+        end
+
+        it "should log the response" do
+          payment.credit!
+          expect(payment).to have_received(:record_response)
+        end
+
+        context "when gateway does not match the environment" do
+          it "should raise an exception" do
+            gateway.stub :environment => "foo"
+            expect { payment.credit! }.to raise_error(Spree::Core::GatewayError)
+          end
+        end
+
+        context "when response is successful" do
+          it "should create an offsetting payment" do
+            Spree::Payment.should_receive(:create)
+            payment.credit!
+          end
+
+          it "resulting payment should have correct values" do
+            payment.order.stub :outstanding_balance => 100
+            payment.stub :credit_allowed => 10
+
+            offsetting_payment = payment.credit!
+            expect(offsetting_payment.amount.to_f).to eq(-10)
+            expect(offsetting_payment).to be_completed
+            expect(offsetting_payment.response_code).to eq('12345')
+            expect(offsetting_payment.source).to eq(payment)
+          end
         end
       end
     end
 
-    context "#void" do
-      before do
-        payment.response_code = '123'
-        payment.state = 'pending'
+    context "when response is unsuccessful" do
+      it "should not create a payment" do
+        gateway.stub :credit => failed_response
+        Spree::Payment.should_not_receive(:create)
+        expect { payment.credit! }.to raise_error(Spree::Core::GatewayError)
       end
+    end
 
-      context "when profiles are supported" do
-        it "should call payment_gateway.void with the payment's response_code" do
-          gateway.stub :payment_profiles_supported? => true
-          gateway.should_receive(:void).with('123', card, anything).and_return(success_response)
-          payment.void_transaction!
-        end
+    context "when already processing" do
+      it "should return nil without trying to process the source" do
+        payment.state = 'processing'
+
+        payment.should_not_receive(:authorize!)
+        payment.should_not_receive(:purchase!)
+        expect(payment.process!).to be_nil
       end
+    end
 
-      context "when profiles are not supported" do
-        it "should call payment_gateway.void with the payment's response_code" do
-          gateway.stub :payment_profiles_supported? => false
-          gateway.should_receive(:void).with('123', anything).and_return(success_response)
-          payment.void_transaction!
-        end
-      end
-
-      it "should log the response" do
-        payment.void_transaction!
-        expect(payment).to have_received(:record_response)
-      end
-
-      context "when gateway does not match the environment" do
-        it "should raise an exception" do
-          gateway.stub :environment => "foo"
-          expect { payment.void_transaction! }.to raise_error(Spree::Core::GatewayError)
-        end
-      end
-
-      context "if successful" do
-        it "should update the response_code with the authorization from the gateway" do
-          # Change it to something different
-          payment.response_code = 'abc'
-          payment.void_transaction!
-          expect(payment.response_code).to eq('12345')
-        end
-      end
-
-      context "if unsuccessful" do
-        it "should not void the payment" do
-          gateway.stub :void => failed_response
-          payment.should_not_receive(:void)
-          expect { payment.void_transaction! }.to raise_error(Spree::Core::GatewayError)
-        end
-      end
-
-      # Regression test for #2119
-      context "if payment is already voided" do
+    context "with source required" do
+      context "raises an error if no source is specified" do
         before do
-          payment.state = 'void'
+          payment.source = nil
         end
 
-        it "should not void the payment" do
-          payment.payment_method.should_not_receive(:void)
-          payment.void_transaction!
+        specify do
+          expect { payment.process! }.to raise_error(Spree::Core::GatewayError, Spree.t(:payment_processing_failed))
         end
       end
     end
 
+    context "with source optional" do
+      context "raises no error if source is not specified" do
+        before do
+          payment.source = nil
+          payment.payment_method.stub(:source_required? => false)
+        end
+
+        specify do
+          expect { payment.process! }.not_to raise_error
+        end
+      end
+    end
+
+    context "#credit_allowed" do
+      it "is the difference between offsets total and payment amount" do
+        payment.amount = 100
+        payment.stub(:offsets_total).and_return(0)
+        expect(payment.credit_allowed).to eq(100)
+        payment.stub(:offsets_total).and_return(80)
+        expect(payment.credit_allowed).to eq(20)
+      end
+    end
+
+    context "#can_credit?" do
+      it "is true if credit_allowed > 0" do
+        payment.stub(:credit_allowed).and_return(100)
+        expect(payment.can_credit?).to be true
+      end
+      it "is false if credit_allowed is 0" do
+        payment.stub(:credit_allowed).and_return(0)
+        expect(payment.can_credit?).to be false
+      end
+    end
+
     context "#credit" do
-      before do
-        payment.state = 'complete'
-        payment.response_code = '123'
+      context "when amount <= credit_allowed" do
+        it "makes the state processing" do
+          payment.payment_method.name = 'Gateway'
+          payment.payment_method.distributors << create(:distributor_enterprise)
+          payment.payment_method.save!
+
+          payment.order = create(:order)
+
+          payment.state = 'completed'
+          payment.stub(:credit_allowed).and_return(10)
+          payment.partial_credit(10)
+          expect(payment).to be_processing
+        end
+        it "calls credit on the source with the payment and amount" do
+          payment.state = 'completed'
+          payment.stub(:credit_allowed).and_return(10)
+          payment.should_receive(:credit!).with(10)
+          payment.partial_credit(10)
+        end
+      end
+      context "when amount > credit_allowed" do
+        it "should not call credit on the source" do
+          payment.state = 'completed'
+          payment.stub(:credit_allowed).and_return(10)
+          payment.partial_credit(20)
+          expect(payment).to be_completed
+        end
+      end
+    end
+
+    context "#save" do
+      it "should call order#update!" do
+        gateway.name = 'Gateway'
+        gateway.distributors << create(:distributor_enterprise)
+        gateway.save!
+
+        order = create(:order)
+        payment = Spree::Payment.create(:amount => 100, :order => order, :payment_method => gateway)
+        order.should_receive(:update!)
+        payment.save
       end
 
-      context "when outstanding_balance is less than payment amount" do
+      context "when profiles are supported" do
         before do
-          payment.order.stub :outstanding_balance => 10
-          payment.stub :credit_allowed => 1000
+          gateway.stub :payment_profiles_supported? => true
+          payment.source.stub :has_payment_profile? => false
         end
 
-        it "should call credit on the gateway with the credit amount and response_code" do
-          gateway.should_receive(:credit).with(1000, card, '123', anything).and_return(success_response)
-          payment.credit!
-        end
-      end
 
-      context "when outstanding_balance is equal to payment amount" do
-        before do
-          payment.order.stub :outstanding_balance => payment.amount
+        context "when there is an error connecting to the gateway" do
+          it "should call gateway_error " do
+            pending '[Spree build] Failing spec'
+            message = double("gateway_error")
+            connection_error = ActiveMerchant::ConnectionError.new(message, nil)
+            expect(gateway).to receive(:create_profile).and_raise(connection_error)
+            expect do
+              Spree::Payment.create(
+                :amount => 100,
+                :order => order,
+                :source => card,
+                :payment_method => gateway
+              )
+            end.should raise_error(Spree::Core::GatewayError)
+          end
         end
 
-        it "should call credit on the gateway with the credit amount and response_code" do
-          gateway.should_receive(:credit).with(amount_in_cents, card, '123', anything).and_return(success_response)
-          payment.credit!
-        end
-      end
+        context "when successfully connecting to the gateway" do
+          xit "should create a payment profile" do
+            gateway.name = 'Gateway'
+            gateway.distributors << create(:distributor_enterprise)
+            gateway.save!
 
-      context "when outstanding_balance is greater than payment amount" do
-        before do
-          payment.order.stub :outstanding_balance => 101
-        end
+            payment.payment_method = gateway
 
-        it "should call credit on the gateway with the original payment amount and response_code" do
-          gateway.should_receive(:credit).with(amount_in_cents.to_f, card, '123', anything).and_return(success_response)
-          payment.credit!
-        end
-      end
+            expect(gateway).to receive(:create_profile)
 
-      it "should log the response" do
-        payment.credit!
-        expect(payment).to have_received(:record_response)
-      end
-
-      context "when gateway does not match the environment" do
-        it "should raise an exception" do
-          gateway.stub :environment => "foo"
-          expect { payment.credit! }.to raise_error(Spree::Core::GatewayError)
-        end
-      end
-
-      context "when response is successful" do
-        it "should create an offsetting payment" do
-          Spree::Payment.should_receive(:create)
-          payment.credit!
-        end
-
-        it "resulting payment should have correct values" do
-          payment.order.stub :outstanding_balance => 100
-          payment.stub :credit_allowed => 10
-
-          offsetting_payment = payment.credit!
-          expect(offsetting_payment.amount.to_f).to eq(-10)
-          expect(offsetting_payment).to be_completed
-          expect(offsetting_payment.response_code).to eq('12345')
-          expect(offsetting_payment.source).to eq(payment)
-        end
-      end
-    end
-  end
-
-  context "when response is unsuccessful" do
-    it "should not create a payment" do
-      gateway.stub :credit => failed_response
-      Spree::Payment.should_not_receive(:create)
-      expect { payment.credit! }.to raise_error(Spree::Core::GatewayError)
-    end
-  end
-
-  context "when already processing" do
-    it "should return nil without trying to process the source" do
-      payment.state = 'processing'
-
-      payment.should_not_receive(:authorize!)
-      payment.should_not_receive(:purchase!)
-      expect(payment.process!).to be_nil
-    end
-  end
-
-  context "with source required" do
-    context "raises an error if no source is specified" do
-      before do
-        payment.source = nil
-      end
-
-      specify do
-        expect { payment.process! }.to raise_error(Spree::Core::GatewayError, Spree.t(:payment_processing_failed))
-      end
-    end
-  end
-
-  context "with source optional" do
-    context "raises no error if source is not specified" do
-      before do
-        payment.source = nil
-        payment.payment_method.stub(:source_required? => false)
-      end
-
-      specify do
-        expect { payment.process! }.not_to raise_error
-      end
-    end
-  end
-
-  context "#credit_allowed" do
-    it "is the difference between offsets total and payment amount" do
-      payment.amount = 100
-      payment.stub(:offsets_total).and_return(0)
-      expect(payment.credit_allowed).to eq(100)
-      payment.stub(:offsets_total).and_return(80)
-      expect(payment.credit_allowed).to eq(20)
-    end
-  end
-
-  context "#can_credit?" do
-    it "is true if credit_allowed > 0" do
-      payment.stub(:credit_allowed).and_return(100)
-      expect(payment.can_credit?).to be true
-    end
-    it "is false if credit_allowed is 0" do
-      payment.stub(:credit_allowed).and_return(0)
-      expect(payment.can_credit?).to be false
-    end
-  end
-
-  context "#credit" do
-    context "when amount <= credit_allowed" do
-      it "makes the state processing" do
-        payment.payment_method.name = 'Gateway'
-        payment.payment_method.distributors << create(:distributor_enterprise)
-        payment.payment_method.save!
-
-        payment.order = create(:order)
-
-        payment.state = 'completed'
-        payment.stub(:credit_allowed).and_return(10)
-        payment.partial_credit(10)
-        expect(payment).to be_processing
-      end
-      it "calls credit on the source with the payment and amount" do
-        payment.state = 'completed'
-        payment.stub(:credit_allowed).and_return(10)
-        payment.should_receive(:credit!).with(10)
-        payment.partial_credit(10)
-      end
-    end
-    context "when amount > credit_allowed" do
-      it "should not call credit on the source" do
-        payment.state = 'completed'
-        payment.stub(:credit_allowed).and_return(10)
-        payment.partial_credit(20)
-        expect(payment).to be_completed
-      end
-    end
-  end
-
-  context "#save" do
-    it "should call order#update!" do
-      gateway.name = 'Gateway'
-      gateway.distributors << create(:distributor_enterprise)
-      gateway.save!
-
-      order = create(:order)
-      payment = Spree::Payment.create(:amount => 100, :order => order, :payment_method => gateway)
-      order.should_receive(:update!)
-      payment.save
-    end
-
-    context "when profiles are supported" do
-      before do
-        gateway.stub :payment_profiles_supported? => true
-        payment.source.stub :has_payment_profile? => false
-      end
-
-
-      context "when there is an error connecting to the gateway" do
-        it "should call gateway_error " do
-          pending '[Spree build] Failing spec'
-          message = double("gateway_error")
-          connection_error = ActiveMerchant::ConnectionError.new(message, nil)
-          expect(gateway).to receive(:create_profile).and_raise(connection_error)
-          expect do
-            Spree::Payment.create(
+            payment = Spree::Payment.create(
               :amount => 100,
-              :order => order,
+              :order => create(:order),
               :source => card,
               :payment_method => gateway
             )
-          end.should raise_error(Spree::Core::GatewayError)
+          end
         end
+
+
       end
 
-      context "when successfully connecting to the gateway" do
-        xit "should create a payment profile" do
+      context "when profiles are not supported" do
+        before { gateway.stub :payment_profiles_supported? => false }
+
+        it "should not create a payment profile" do
           gateway.name = 'Gateway'
           gateway.distributors << create(:distributor_enterprise)
           gateway.save!
 
-          payment.payment_method = gateway
-
-          expect(gateway).to receive(:create_profile)
-
+          gateway.should_not_receive :create_profile
           payment = Spree::Payment.create(
             :amount => 100,
             :order => create(:order),
@@ -557,263 +579,243 @@ describe Spree::Payment do
           )
         end
       end
-
-
     end
 
-    context "when profiles are not supported" do
-      before { gateway.stub :payment_profiles_supported? => false }
+    context "#build_source" do
+      it "should build the payment's source" do
+        params = { :amount => 100, :payment_method => gateway,
+                   :source_attributes => {
+          :expiry =>"1 / 99",
+          :number => '1234567890123',
+          :verification_value => '123'
+        }
+        }
 
-      it "should not create a payment profile" do
-        gateway.name = 'Gateway'
-        gateway.distributors << create(:distributor_enterprise)
-        gateway.save!
+        payment = Spree::Payment.new(params)
+        expect(payment).to be_valid
+        expect(payment.source).not_to be_nil
+      end
 
-        gateway.should_not_receive :create_profile
-        payment = Spree::Payment.create(
-          :amount => 100,
-          :order => create(:order),
-          :source => card,
-          :payment_method => gateway
-        )
+      it "errors when payment source not valid" do
+        params = { :amount => 100, :payment_method => gateway,
+                   :source_attributes => {:expiry => "1 / 12" }}
+
+        payment = Spree::Payment.new(params)
+        expect(payment).not_to be_valid
+        expect(payment.source).not_to be_nil
+        expect(payment.source.errors[:number]).not_to be_empty
+        expect(payment.source.errors[:verification_value]).not_to be_empty
       end
     end
-  end
 
-  context "#build_source" do
-    it "should build the payment's source" do
-      params = { :amount => 100, :payment_method => gateway,
-                 :source_attributes => {
-        :expiry =>"1 / 99",
-        :number => '1234567890123',
-        :verification_value => '123'
-      }
-      }
-
-      payment = Spree::Payment.new(params)
-      expect(payment).to be_valid
-      expect(payment.source).not_to be_nil
+    context "#currency" do
+      before { order.stub(:currency) { "ABC" } }
+      it "returns the order currency" do
+        expect(payment.currency).to eq("ABC")
+      end
     end
 
-    it "errors when payment source not valid" do
-      params = { :amount => 100, :payment_method => gateway,
-                 :source_attributes => {:expiry => "1 / 12" }}
-
-      payment = Spree::Payment.new(params)
-      expect(payment).not_to be_valid
-      expect(payment.source).not_to be_nil
-      expect(payment.source.errors[:number]).not_to be_empty
-      expect(payment.source.errors[:verification_value]).not_to be_empty
-    end
-  end
-
-  context "#currency" do
-    before { order.stub(:currency) { "ABC" } }
-    it "returns the order currency" do
-      expect(payment.currency).to eq("ABC")
-    end
-  end
-
-  context "#display_amount" do
-    it "returns a Spree::Money for this amount" do
-      expect(payment.display_amount).to eq(Spree::Money.new(payment.amount))
-    end
-  end
-
-  # Regression test for #2216
-  context "#gateway_options" do
-    before { order.stub(:last_ip_address => "192.168.1.1") }
-
-    it "contains an IP" do
-      expect(payment.gateway_options[:ip]).to eq(order.last_ip_address)
-    end
-  end
-
-  context "#set_unique_identifier" do
-    # Regression test for #1998
-    it "sets a unique identifier on create" do
-      payment.run_callbacks(:save)
-      expect(payment.identifier).not_to be_blank
-      expect(payment.identifier.size).to eq(8)
-      expect(payment.identifier).to be_a(String)
+    context "#display_amount" do
+      it "returns a Spree::Money for this amount" do
+        expect(payment.display_amount).to eq(Spree::Money.new(payment.amount))
+      end
     end
 
-    context "other payment exists" do
-      let(:other_payment) {
-        gateway.name = 'Gateway'
-        gateway.distributors << create(:distributor_enterprise)
-        gateway.save!
+    # Regression test for #2216
+    context "#gateway_options" do
+      before { order.stub(:last_ip_address => "192.168.1.1") }
 
-        payment = Spree::Payment.new
-        payment.source = card
-        payment.order = create(:order)
-        payment.payment_method = gateway
-        payment
-      }
-
-      before { other_payment.save! }
-
-      it "doesn't set duplicate identifier" do
-        payment.should_receive(:generate_identifier).and_return(other_payment.identifier)
-        payment.should_receive(:generate_identifier).and_call_original
+      it "contains an IP" do
+        expect(payment.gateway_options[:ip]).to eq(order.last_ip_address)
+      end
+    end
 
+    context "#set_unique_identifier" do
+      # Regression test for #1998
+      it "sets a unique identifier on create" do
         payment.run_callbacks(:save)
-
         expect(payment.identifier).not_to be_blank
-        expect(payment.identifier).not_to eq(other_payment.identifier)
+        expect(payment.identifier.size).to eq(8)
+        expect(payment.identifier).to be_a(String)
+      end
+
+      context "other payment exists" do
+        let(:other_payment) {
+          gateway.name = 'Gateway'
+          gateway.distributors << create(:distributor_enterprise)
+          gateway.save!
+
+          payment = Spree::Payment.new
+          payment.source = card
+          payment.order = create(:order)
+          payment.payment_method = gateway
+          payment
+        }
+
+        before { other_payment.save! }
+
+        it "doesn't set duplicate identifier" do
+          payment.should_receive(:generate_identifier).and_return(other_payment.identifier)
+          payment.should_receive(:generate_identifier).and_call_original
+
+          payment.run_callbacks(:save)
+
+          expect(payment.identifier).not_to be_blank
+          expect(payment.identifier).not_to eq(other_payment.identifier)
+        end
       end
     end
-  end
 
-  describe "available actions" do
-    context "for most gateways" do
-      let(:payment) { create(:payment, source: create(:credit_card)) }
+    describe "available actions" do
+      context "for most gateways" do
+        let(:payment) { create(:payment, source: create(:credit_card)) }
 
-      it "can capture and void" do
-        expect(payment.actions).to match_array %w(capture void)
+        it "can capture and void" do
+          expect(payment.actions).to match_array %w(capture void)
+        end
+
+        describe "when a payment has been taken" do
+          before do
+            allow(payment).to receive(:state) { 'completed' }
+            allow(payment).to receive(:order) { double(:order, payment_state: 'credit_owed') }
+          end
+
+          it "can void and credit" do
+            expect(payment.actions).to match_array %w(void credit)
+          end
+        end
       end
 
-      describe "when a payment has been taken" do
+      context "for Pin Payments" do
+        let(:d) { create(:distributor_enterprise) }
+        let(:pin) { Spree::Gateway::Pin.create! name: 'pin', distributor_ids: [d.id] }
+        let(:payment) { create(:payment, source: create(:credit_card), payment_method: pin) }
+
+        it "does not void" do
+          expect(payment.actions).not_to include 'void'
+        end
+
+        describe "when a payment has been taken" do
+          before do
+            allow(payment).to receive(:state) { 'completed' }
+            allow(payment).to receive(:order) { double(:order, payment_state: 'credit_owed') }
+          end
+
+          it "can refund instead of crediting" do
+            expect(payment.actions).not_to include 'credit'
+            expect(payment.actions).to     include 'refund'
+          end
+        end
+      end
+    end
+
+    describe "refunding" do
+      let(:payment) { create(:payment) }
+      let(:success) { double(success?: true, authorization: 'abc123') }
+      let(:failure) { double(success?: false) }
+
+      it "always checks the environment" do
+        allow(payment.payment_method).to receive(:refund) { success }
+        expect(payment).to receive(:check_environment)
+        payment.refund!
+      end
+
+      describe "calculating refund amount" do
+        it "returns the parameter amount when given" do
+          expect(payment.send(:calculate_refund_amount, 123)).to be === 123.0
+        end
+
+        it "refunds up to the value of the payment when the outstanding balance is larger" do
+          allow(payment).to receive(:credit_allowed) { 123 }
+          allow(payment).to receive(:order) { double(:order, outstanding_balance: 1000) }
+          expect(payment.send(:calculate_refund_amount)).to eq(123)
+        end
+
+        it "refunds up to the outstanding balance of the order when the payment is larger" do
+          allow(payment).to receive(:credit_allowed) { 1000 }
+          allow(payment).to receive(:order) { double(:order, outstanding_balance: 123) }
+          expect(payment.send(:calculate_refund_amount)).to eq(123)
+        end
+      end
+
+      describe "performing refunds" do
         before do
-          allow(payment).to receive(:state) { 'completed' }
-          allow(payment).to receive(:order) { double(:order, payment_state: 'credit_owed') }
+          allow(payment).to receive(:calculate_refund_amount) { 123 }
+          expect(payment.payment_method).to receive(:refund).and_return(success)
         end
 
-        it "can void and credit" do
-          expect(payment.actions).to match_array %w(void credit)
-        end
-      end
-    end
-
-    context "for Pin Payments" do
-      let(:d) { create(:distributor_enterprise) }
-      let(:pin) { Spree::Gateway::Pin.create! name: 'pin', distributor_ids: [d.id] }
-      let(:payment) { create(:payment, source: create(:credit_card), payment_method: pin) }
-
-      it "does not void" do
-        expect(payment.actions).not_to include 'void'
-      end
-
-      describe "when a payment has been taken" do
-        before do
-          allow(payment).to receive(:state) { 'completed' }
-          allow(payment).to receive(:order) { double(:order, payment_state: 'credit_owed') }
+        it "performs the refund without payment profiles" do
+          allow(payment.payment_method).to receive(:payment_profiles_supported?) { false }
+          payment.refund!
         end
 
-        it "can refund instead of crediting" do
-          expect(payment.actions).not_to include 'credit'
-          expect(payment.actions).to     include 'refund'
+        it "performs the refund with payment profiles" do
+          allow(payment.payment_method).to receive(:payment_profiles_supported?) { true }
+          payment.refund!
         end
       end
-    end
-  end
 
-  describe "refunding" do
-    let(:payment) { create(:payment) }
-    let(:success) { double(success?: true, authorization: 'abc123') }
-    let(:failure) { double(success?: false) }
-
-    it "always checks the environment" do
-      allow(payment.payment_method).to receive(:refund) { success }
-      expect(payment).to receive(:check_environment)
-      payment.refund!
-    end
-
-    describe "calculating refund amount" do
-      it "returns the parameter amount when given" do
-        expect(payment.send(:calculate_refund_amount, 123)).to be === 123.0
-      end
-
-      it "refunds up to the value of the payment when the outstanding balance is larger" do
-        allow(payment).to receive(:credit_allowed) { 123 }
-        allow(payment).to receive(:order) { double(:order, outstanding_balance: 1000) }
-        expect(payment.send(:calculate_refund_amount)).to eq(123)
-      end
-
-      it "refunds up to the outstanding balance of the order when the payment is larger" do
-        allow(payment).to receive(:credit_allowed) { 1000 }
-        allow(payment).to receive(:order) { double(:order, outstanding_balance: 123) }
-        expect(payment.send(:calculate_refund_amount)).to eq(123)
-      end
-    end
-
-    describe "performing refunds" do
-      before do
+      it "records the response" do
         allow(payment).to receive(:calculate_refund_amount) { 123 }
-        expect(payment.payment_method).to receive(:refund).and_return(success)
-      end
-
-      it "performs the refund without payment profiles" do
-        allow(payment.payment_method).to receive(:payment_profiles_supported?) { false }
+        allow(payment.payment_method).to receive(:refund).and_return(success)
+        expect(payment).to receive(:record_response).with(success)
         payment.refund!
       end
 
-      it "performs the refund with payment profiles" do
-        allow(payment.payment_method).to receive(:payment_profiles_supported?) { true }
+      it "records a payment on success" do
+        allow(payment).to receive(:calculate_refund_amount) { 123 }
+        allow(payment.payment_method).to receive(:refund).and_return(success)
+        allow(payment).to receive(:record_response)
+
+        expect do
+          payment.refund!
+        end.to change(Spree::Payment, :count).by(1)
+
+        p = Spree::Payment.last
+        expect(p.order).to eq(payment.order)
+        expect(p.source).to eq(payment)
+        expect(p.payment_method).to eq(payment.payment_method)
+        expect(p.amount).to eq(-123)
+        expect(p.response_code).to eq(success.authorization)
+        expect(p.state).to eq('completed')
+      end
+
+      it "logs the error on failure" do
+        allow(payment).to receive(:calculate_refund_amount) { 123 }
+        allow(payment.payment_method).to receive(:refund).and_return(failure)
+        allow(payment).to receive(:record_response)
+        expect(payment).to receive(:gateway_error).with(failure)
         payment.refund!
       end
     end
 
-    it "records the response" do
-      allow(payment).to receive(:calculate_refund_amount) { 123 }
-      allow(payment.payment_method).to receive(:refund).and_return(success)
-      expect(payment).to receive(:record_response).with(success)
-      payment.refund!
-    end
+    describe "applying transaction fees" do
+      let!(:order) { create(:order) }
+      let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) }
 
-    it "records a payment on success" do
-      allow(payment).to receive(:calculate_refund_amount) { 123 }
-      allow(payment.payment_method).to receive(:refund).and_return(success)
-      allow(payment).to receive(:record_response)
-
-      expect do
-        payment.refund!
-      end.to change(Spree::Payment, :count).by(1)
-
-      p = Spree::Payment.last
-      expect(p.order).to eq(payment.order)
-      expect(p.source).to eq(payment)
-      expect(p.payment_method).to eq(payment.payment_method)
-      expect(p.amount).to eq(-123)
-      expect(p.response_code).to eq(success.authorization)
-      expect(p.state).to eq('completed')
-    end
-
-    it "logs the error on failure" do
-      allow(payment).to receive(:calculate_refund_amount) { 123 }
-      allow(payment.payment_method).to receive(:refund).and_return(failure)
-      allow(payment).to receive(:record_response)
-      expect(payment).to receive(:gateway_error).with(failure)
-      payment.refund!
-    end
-  end
-
-  describe "applying transaction fees" do
-    let!(:order) { create(:order) }
-    let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) }
-
-    before do
-      order.reload.update!
-    end
-
-    context "when order-based calculator" do
-      let!(:shop) { create(:enterprise) }
-      let!(:payment_method) { create(:payment_method, calculator: calculator) }
-
-      let!(:calculator) do
-        Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10)
+      before do
+        order.reload.update!
       end
 
-      context "when order complete and inventory tracking enabled" do
-        let!(:order) { create(:completed_order_with_totals, distributor: shop) }
-        let!(:variant) { order.line_items.first.variant }
-        let!(:inventory_item) { create(:inventory_item, enterprise: shop, variant: variant) }
+      context "when order-based calculator" do
+        let!(:shop) { create(:enterprise) }
+        let!(:payment_method) { create(:payment_method, calculator: calculator) }
 
-        it "creates adjustment" do
-          payment = create(:payment, order: order, payment_method: payment_method,
-                           amount: order.total)
-          expect(payment.adjustment).to be_present
-          expect(payment.adjustment.amount).not_to eq(0)
+        let!(:calculator) do
+          Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10)
+        end
+
+        context "when order complete and inventory tracking enabled" do
+          let!(:order) { create(:completed_order_with_totals, distributor: shop) }
+          let!(:variant) { order.line_items.first.variant }
+          let!(:inventory_item) { create(:inventory_item, enterprise: shop, variant: variant) }
+
+          it "creates adjustment" do
+            payment = create(:payment, order: order, payment_method: payment_method,
+                             amount: order.total)
+            expect(payment.adjustment).to be_present
+            expect(payment.adjustment.amount).not_to eq(0)
+          end
         end
       end
     end

From 2f4648342fdf291ba5c32cd30e692f2c95a1f444 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 10 Jul 2020 11:50:20 +0200
Subject: [PATCH 20/43] Merge decorator specs with Spree's ones

They are now isolated from each other.
---
 spec/models/spree/payment_original_spec.rb | 86 +++++++++++++++++++++
 spec/models/spree/payment_spec.rb          | 90 ----------------------
 2 files changed, 86 insertions(+), 90 deletions(-)
 delete mode 100644 spec/models/spree/payment_spec.rb

diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_original_spec.rb
index 418c09882c..07559f5314 100644
--- a/spec/models/spree/payment_original_spec.rb
+++ b/spec/models/spree/payment_original_spec.rb
@@ -820,4 +820,90 @@ describe Spree::Payment do
       end
     end
   end
+
+  context 'OFN specs from previously decorated model' do
+    describe "applying transaction fees" do
+      let!(:order) { create(:order) }
+      let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) }
+
+      before do
+        order.reload.update!
+      end
+
+      context "to Stripe payments" do
+        let(:shop) { create(:enterprise) }
+        let(:payment_method) { create(:stripe_payment_method, distributor_ids: [create(:distributor_enterprise).id], preferred_enterprise_id: shop.id) }
+        let(:payment) { create(:payment, order: order, payment_method: payment_method, amount: order.total) }
+        let(:calculator) { Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10) }
+
+        before do
+          payment_method.calculator = calculator
+          payment_method.save!
+
+          allow(order).to receive(:pending_payments) { [payment] }
+        end
+
+        context "when the payment fails" do
+          let(:failed_response) { ActiveMerchant::Billing::Response.new(false, "This is an error message") }
+
+          before do
+            allow(payment_method).to receive(:purchase) { failed_response }
+          end
+
+          it "makes the transaction fee ineligible and finalizes it" do
+            # Decided to wrap the save process in order.process_payments!
+            # since that is the context it is usually performed in
+            order.process_payments!
+            expect(order.payments.count).to eq 1
+            expect(order.payments).to include payment
+            expect(payment.state).to eq "failed"
+            expect(payment.adjustment.eligible?).to be false
+            expect(payment.adjustment.finalized?).to be true
+            expect(order.adjustments.payment_fee.count).to eq 1
+            expect(order.adjustments.payment_fee.eligible).to_not include payment.adjustment
+          end
+        end
+
+        context "when the payment information is invalid" do
+          before do
+            allow(payment_method).to receive(:supports?) { false }
+          end
+
+          it "makes the transaction fee ineligible and finalizes it" do
+            # Decided to wrap the save process in order.process_payments!
+            # since that is the context it is usually performed in
+            order.process_payments!
+            expect(order.payments.count).to eq 1
+            expect(order.payments).to include payment
+            expect(payment.state).to eq "invalid"
+            expect(payment.adjustment.eligible?).to be false
+            expect(payment.adjustment.finalized?).to be true
+            expect(order.adjustments.payment_fee.count).to eq 1
+            expect(order.adjustments.payment_fee.eligible).to_not include payment.adjustment
+          end
+        end
+
+        context "when the payment is processed successfully" do
+          let(:successful_response) { ActiveMerchant::Billing::Response.new(true, "Yay!") }
+
+          before do
+            allow(payment_method).to receive(:purchase) { successful_response }
+          end
+
+          it "creates an appropriate adjustment" do
+            # Decided to wrap the save process in order.process_payments!
+            # since that is the context it is usually performed in
+            order.process_payments!
+            expect(order.payments.count).to eq 1
+            expect(order.payments).to include payment
+            expect(payment.state).to eq "completed"
+            expect(payment.adjustment.eligible?).to be true
+            expect(order.adjustments.payment_fee.count).to eq 1
+            expect(order.adjustments.payment_fee.eligible).to include payment.adjustment
+            expect(payment.adjustment.amount).to eq 1.5
+          end
+        end
+      end
+    end
+  end
 end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
deleted file mode 100644
index 434ce3a2a3..0000000000
--- a/spec/models/spree/payment_spec.rb
+++ /dev/null
@@ -1,90 +0,0 @@
-require 'spec_helper'
-
-module Spree
-  describe Payment do
-
-    describe "applying transaction fees" do
-      let!(:order) { create(:order) }
-      let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) }
-
-      before do
-        order.reload.update!
-      end
-
-      context "to Stripe payments" do
-        let(:shop) { create(:enterprise) }
-        let(:payment_method) { create(:stripe_payment_method, distributor_ids: [create(:distributor_enterprise).id], preferred_enterprise_id: shop.id) }
-        let(:payment) { create(:payment, order: order, payment_method: payment_method, amount: order.total) }
-        let(:calculator) { Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10) }
-
-        before do
-          payment_method.calculator = calculator
-          payment_method.save!
-
-          allow(order).to receive(:pending_payments) { [payment] }
-        end
-
-        context "when the payment fails" do
-          let(:failed_response) { ActiveMerchant::Billing::Response.new(false, "This is an error message") }
-
-          before do
-            allow(payment_method).to receive(:purchase) { failed_response }
-          end
-
-          it "makes the transaction fee ineligible and finalizes it" do
-            # Decided to wrap the save process in order.process_payments!
-            # since that is the context it is usually performed in
-            order.process_payments!
-            expect(order.payments.count).to eq 1
-            expect(order.payments).to include payment
-            expect(payment.state).to eq "failed"
-            expect(payment.adjustment.eligible?).to be false
-            expect(payment.adjustment.finalized?).to be true
-            expect(order.adjustments.payment_fee.count).to eq 1
-            expect(order.adjustments.payment_fee.eligible).to_not include payment.adjustment
-          end
-        end
-
-        context "when the payment information is invalid" do
-          before do
-            allow(payment_method).to receive(:supports?) { false }
-          end
-
-          it "makes the transaction fee ineligible and finalizes it" do
-            # Decided to wrap the save process in order.process_payments!
-            # since that is the context it is usually performed in
-            order.process_payments!
-            expect(order.payments.count).to eq 1
-            expect(order.payments).to include payment
-            expect(payment.state).to eq "invalid"
-            expect(payment.adjustment.eligible?).to be false
-            expect(payment.adjustment.finalized?).to be true
-            expect(order.adjustments.payment_fee.count).to eq 1
-            expect(order.adjustments.payment_fee.eligible).to_not include payment.adjustment
-          end
-        end
-
-        context "when the payment is processed successfully" do
-          let(:successful_response) { ActiveMerchant::Billing::Response.new(true, "Yay!") }
-
-          before do
-            allow(payment_method).to receive(:purchase) { successful_response }
-          end
-
-          it "creates an appropriate adjustment" do
-            # Decided to wrap the save process in order.process_payments!
-            # since that is the context it is usually performed in
-            order.process_payments!
-            expect(order.payments.count).to eq 1
-            expect(order.payments).to include payment
-            expect(payment.state).to eq "completed"
-            expect(payment.adjustment.eligible?).to be true
-            expect(order.adjustments.payment_fee.count).to eq 1
-            expect(order.adjustments.payment_fee.eligible).to include payment.adjustment
-            expect(payment.adjustment.amount).to eq 1.5
-          end
-        end
-      end
-    end
-  end
-end

From 683794636bdbbd7709162b04f3548cfbd1406c88 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 10 Jul 2020 11:51:46 +0200
Subject: [PATCH 21/43] Rename spec file

---
 spec/models/spree/{payment_original_spec.rb => payment_spec.rb} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename spec/models/spree/{payment_original_spec.rb => payment_spec.rb} (100%)

diff --git a/spec/models/spree/payment_original_spec.rb b/spec/models/spree/payment_spec.rb
similarity index 100%
rename from spec/models/spree/payment_original_spec.rb
rename to spec/models/spree/payment_spec.rb

From 55d52b875f51603df574e9a9a49274459a97b3d3 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 10 Jul 2020 15:04:29 +0200
Subject: [PATCH 22/43] Run rubocop autocorrect on payment model

---
 app/models/spree/payment.rb | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 8ae3c91c87..9b3c961e78 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   class Payment < ActiveRecord::Base
     include Spree::Payment::Processing
@@ -14,7 +16,7 @@ module Spree
     belongs_to :payment_method, class_name: 'Spree::PaymentMethod'
 
     has_many :offsets, -> { where("source_type = 'Spree::Payment' AND amount < 0 AND state = 'completed'") },
-      class_name: "Spree::Payment", foreign_key: :source_id
+             class_name: "Spree::Payment", foreign_key: :source_id
     has_many :log_entries, as: :source
 
     has_one :adjustment, as: :source, dependent: :destroy
@@ -43,8 +45,9 @@ module Spree
 
     def persist_invalid
       return unless ['failed', 'invalid'].include?(state)
+
       state_will_change!
-      save 
+      save
     end
 
     # order state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
@@ -74,9 +77,7 @@ module Spree
       end
     end
 
-    def currency
-      order.currency
-    end
+    delegate :currency, to: :order
 
     def money
       Spree::Money.new(amount, { currency: currency })
@@ -110,8 +111,9 @@ module Spree
     # Pin payments lacks void and credit methods, but it does have refund
     # Here we swap credit out for refund and remove void as a possible action
     def actions
-      return [] unless payment_source and payment_source.respond_to? :actions
-      actions = payment_source.actions.select { |action| !payment_source.respond_to?("can_#{action}?") or payment_source.send("can_#{action}?", self) }
+      return [] unless payment_source&.respond_to?(:actions)
+
+      actions = payment_source.actions.select { |action| !payment_source.respond_to?("can_#{action}?") || payment_source.send("can_#{action}?", self) }
 
       if payment_method.is_a? Gateway::Pin
         actions << 'refund' if actions.include? 'credit'
@@ -162,10 +164,10 @@ module Spree
       if source && !source.valid?
         source.errors.each do |field, error|
           field_name = I18n.t("activerecord.attributes.#{source.class.to_s.underscore}.#{field}")
-          self.errors.add(Spree.t(source.class.to_s.demodulize.underscore), "#{field_name} #{error}")
+          errors.add(Spree.t(source.class.to_s.demodulize.underscore), "#{field_name} #{error}")
         end
       end
-      return !errors.present?
+      errors.blank?
     end
 
     def profiles_supported?
@@ -184,9 +186,7 @@ module Spree
     end
 
     def invalidate_old_payments
-      order.payments.with_state('checkout').where("id != ?", self.id).each do |payment|
-        payment.invalidate!
-      end
+      order.payments.with_state('checkout').where("id != ?", id).each(&:invalidate!)
     end
 
     def update_order
@@ -202,7 +202,7 @@ module Spree
     def set_unique_identifier
       begin
         self.identifier = generate_identifier
-      end while self.class.exists?(identifier: self.identifier)
+      end while self.class.exists?(identifier: identifier)
     end
 
     def generate_identifier

From cf64d3a290ce9f43c21e2846155742ad08dac9eb Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 10 Jul 2020 15:07:12 +0200
Subject: [PATCH 23/43] Merge skipped callback from decorator into model

If we don't want that callback we can just as well remove it now that we
own that code.
---
 app/models/spree/payment.rb           |  9 ---------
 app/models/spree/payment_decorator.rb | 10 ----------
 2 files changed, 19 deletions(-)
 delete mode 100644 app/models/spree/payment_decorator.rb

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 9b3c961e78..46a9d924b3 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -41,15 +41,6 @@ module Spree
     scope :failed, -> { with_state('failed') }
     scope :valid, -> { where('state NOT IN (?)', %w(failed invalid)) }
 
-    after_rollback :persist_invalid
-
-    def persist_invalid
-      return unless ['failed', 'invalid'].include?(state)
-
-      state_will_change!
-      save
-    end
-
     # order state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
     state_machine initial: :checkout do
       # With card payments, happens before purchase or authorization happens
diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb
deleted file mode 100644
index f79529e306..0000000000
--- a/app/models/spree/payment_decorator.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-require 'spree/localized_number'
-
-module Spree
-  Payment.class_eval do
-    # We bypass this after_rollback callback that is setup in Spree::Payment
-    # The issues the callback fixes are not experienced in OFN:
-    #   if a payment fails on checkout the state "failed" is persisted correctly
-    def persist_invalid; end
-  end
-end

From 3435d5ac979487064095ad995b60288032d83a16 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 10 Jul 2020 15:18:10 +0200
Subject: [PATCH 24/43] Fix Rubocop non-metrics issues in payment model

---
 app/models/spree/payment.rb | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 46a9d924b3..a9e0b6b460 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -15,9 +15,9 @@ module Spree
     belongs_to :source, polymorphic: true
     belongs_to :payment_method, class_name: 'Spree::PaymentMethod'
 
-    has_many :offsets, -> { where("source_type = 'Spree::Payment' AND amount < 0 AND state = 'completed'") },
+    has_many :offsets, -> { where("source_type = 'Spree::Payment' AND amount < 0").completed },
              class_name: "Spree::Payment", foreign_key: :source_id
-    has_many :log_entries, as: :source
+    has_many :log_entries, as: :source, dependent: :destroy
 
     has_one :adjustment, as: :source, dependent: :destroy
 
@@ -71,7 +71,7 @@ module Spree
     delegate :currency, to: :order
 
     def money
-      Spree::Money.new(amount, { currency: currency })
+      Spree::Money.new(amount, currency: currency)
     end
     alias display_amount money
 
@@ -84,7 +84,7 @@ module Spree
     end
 
     def can_credit?
-      credit_allowed > 0
+      credit_allowed.positive?
     end
 
     # see https://github.com/spree/spree/issues/981
@@ -104,7 +104,10 @@ module Spree
     def actions
       return [] unless payment_source&.respond_to?(:actions)
 
-      actions = payment_source.actions.select { |action| !payment_source.respond_to?("can_#{action}?") || payment_source.send("can_#{action}?", self) }
+      actions = payment_source.actions.select do |action|
+        !payment_source.respond_to?("can_#{action}?") ||
+          payment_source.__send__("can_#{action}?", self)
+      end
 
       if payment_method.is_a? Gateway::Pin
         actions << 'refund' if actions.include? 'credit'
@@ -162,7 +165,8 @@ module Spree
     end
 
     def profiles_supported?
-      payment_method.respond_to?(:payment_profiles_supported?) && payment_method.payment_profiles_supported?
+      payment_method.respond_to?(:payment_profiles_supported?) &&
+        payment_method.payment_profiles_supported?
     end
 
     def create_payment_profile
@@ -191,9 +195,7 @@ module Spree
     # and this is it. Related to #1998.
     # See https://github.com/spree/spree/issues/1998#issuecomment-12869105
     def set_unique_identifier
-      begin
-        self.identifier = generate_identifier
-      end while self.class.exists?(identifier: identifier)
+      self.identifier = generate_identifier while self.class.exists?(identifier: identifier)
     end
 
     def generate_identifier

From 66dbd85eb401b5b47cfc0475219ee026339c5b01 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 10 Jul 2020 15:43:41 +0200
Subject: [PATCH 25/43] Run rubocop autocorrect on payment/processing.rb

---
 app/models/spree/payment/processing.rb | 88 ++++++++++++++------------
 1 file changed, 46 insertions(+), 42 deletions(-)

diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index ad7eb87aca..22ff948283 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -1,8 +1,10 @@
+# frozen_string_literal: true
+
 module Spree
   class Payment < ActiveRecord::Base
     module Processing
       def process!
-        if payment_method && payment_method.source_required?
+        if payment_method&.source_required?
           if source
             if !processing?
               if payment_method.supports?(source)
@@ -13,11 +15,11 @@ module Spree
                 end
               else
                 invalidate!
-                raise Core::GatewayError.new(Spree.t(:payment_method_not_supported))
+                raise Core::GatewayError, Spree.t(:payment_method_not_supported)
               end
             end
           else
-            raise Core::GatewayError.new(Spree.t(:payment_processing_failed))
+            raise Core::GatewayError, Spree.t(:payment_processing_failed)
           end
         end
       end
@@ -34,6 +36,7 @@ module Spree
 
       def capture!
         return true if completed?
+
         started_processing!
         protect_from_connection_error do
           check_environment
@@ -55,29 +58,30 @@ module Spree
 
       def void_transaction!
         return true if void?
+
         protect_from_connection_error do
           check_environment
 
           if payment_method.payment_profiles_supported?
             # Gateways supporting payment profiles will need access to credit card object because this stores the payment profile information
             # so supply the authorization itself as well as the credit card, rather than just the authorization code
-            response = payment_method.void(self.response_code, source, gateway_options)
+            response = payment_method.void(response_code, source, gateway_options)
           else
             # Standard ActiveMerchant void usage
-            response = payment_method.void(self.response_code, gateway_options)
+            response = payment_method.void(response_code, gateway_options)
           end
           record_response(response)
 
           if response.success?
             self.response_code = response.authorization
-            self.void
+            void
           else
             gateway_error(response)
           end
         end
       end
 
-      def credit!(credit_amount=nil)
+      def credit!(credit_amount = nil)
         protect_from_connection_error do
           check_environment
 
@@ -94,12 +98,12 @@ module Spree
 
           if response.success?
             self.class.create(
-              :order => order,
-              :source => self,
-              :payment_method => payment_method,
-              :amount => credit_amount.abs * -1,
-              :response_code => response.authorization,
-              :state => 'completed'
+              order: order,
+              source: self,
+              payment_method: payment_method,
+              amount: credit_amount.abs * -1,
+              response_code: response.authorization,
+              state: 'completed'
             )
           else
             gateway_error(response)
@@ -136,28 +140,29 @@ module Spree
 
       def partial_credit(amount)
         return if amount > credit_allowed
+
         started_processing!
         credit!(amount)
       end
 
       def gateway_options
-        options = { :email    => order.email,
-                    :customer => order.email,
-                    :ip       => order.last_ip_address,
+        options = { email: order.email,
+                    customer: order.email,
+                    ip: order.last_ip_address,
                     # Need to pass in a unique identifier here to make some
                     # payment gateways happy.
                     #
                     # For more information, please see Spree::Payment#set_unique_identifier
-                    :order_id => gateway_order_id }
+                    order_id: gateway_order_id }
 
-        options.merge!({ :shipping => order.ship_total * 100,
-                         :tax      => order.tax_total * 100,
-                         :subtotal => order.item_total * 100,
-                         :discount => order.promo_total * 100,
-                         :currency => currency })
+        options.merge!({ shipping: order.ship_total * 100,
+                         tax: order.tax_total * 100,
+                         subtotal: order.item_total * 100,
+                         discount: order.promo_total * 100,
+                         currency: currency })
 
-        options.merge!({ :billing_address  => order.bill_address.try(:active_merchant_hash),
-                        :shipping_address => order.ship_address.try(:active_merchant_hash) })
+        options.merge!({ billing_address: order.bill_address.try(:active_merchant_hash),
+                         shipping_address: order.ship_address.try(:active_merchant_hash) })
 
         options
       end
@@ -193,49 +198,48 @@ module Spree
               self.cvv_response_message = response.cvv_result['message']
             end
           end
-          self.send("#{success_state}!")
+          send("#{success_state}!")
         else
-          self.send(failure_state)
+          send(failure_state)
           gateway_error(response)
         end
       end
 
       def record_response(response)
-        log_entries.create(:details => response.to_yaml)
+        log_entries.create(details: response.to_yaml)
       end
 
       def protect_from_connection_error
-        begin
-          yield
-        rescue ActiveMerchant::ConnectionError => e
-          gateway_error(e)
-        end
+        yield
+      rescue ActiveMerchant::ConnectionError => e
+        gateway_error(e)
       end
 
       def gateway_error(error)
-        if error.is_a? ActiveMerchant::Billing::Response
-          text = error.params['message'] || error.params['response_reason_text'] || error.message
-        elsif error.is_a? ActiveMerchant::ConnectionError
-          text = Spree.t(:unable_to_connect_to_gateway)
-        else
-          text = error.to_s
-        end
+        text = if error.is_a? ActiveMerchant::Billing::Response
+                 error.params['message'] || error.params['response_reason_text'] || error.message
+               elsif error.is_a? ActiveMerchant::ConnectionError
+                 Spree.t(:unable_to_connect_to_gateway)
+               else
+                 error.to_s
+               end
         logger.error(Spree.t(:gateway_error))
         logger.error("  #{error.to_yaml}")
-        raise Core::GatewayError.new(text)
+        raise Core::GatewayError, text
       end
 
       # Saftey check to make sure we're not accidentally performing operations on a live gateway.
       # Ex. When testing in staging environment with a copy of production data.
       def check_environment
         return if payment_method.environment == Rails.env
+
         message = Spree.t(:gateway_config_unavailable) + " - #{Rails.env}"
-        raise Core::GatewayError.new(message)
+        raise Core::GatewayError, message
       end
 
       # The unique identifier to be passed in to the payment gateway
       def gateway_order_id
-        "#{order.number}-#{self.identifier}"
+        "#{order.number}-#{identifier}"
       end
     end
   end

From 42658b5255ee8200b94a671bd6c7d44cfb0ca863 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 13 Jul 2020 09:43:18 +0200
Subject: [PATCH 26/43] Refactor `#process!` nested ifs to guard clauses

Following Rubocop's indications.
---
 app/models/spree/payment/processing.rb | 32 ++++++++++++--------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 22ff948283..b2421d5c8a 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -4,23 +4,21 @@ module Spree
   class Payment < ActiveRecord::Base
     module Processing
       def process!
-        if payment_method&.source_required?
-          if source
-            if !processing?
-              if payment_method.supports?(source)
-                if payment_method.auto_capture?
-                  purchase!
-                else
-                  authorize!
-                end
-              else
-                invalidate!
-                raise Core::GatewayError, Spree.t(:payment_method_not_supported)
-              end
-            end
-          else
-            raise Core::GatewayError, Spree.t(:payment_processing_failed)
-          end
+        return unless payment_method&.source_required?
+
+        raise Core::GatewayError, Spree.t(:payment_processing_failed) unless source
+
+        return if processing?
+
+        unless payment_method.supports?(source)
+          invalidate!
+          raise Core::GatewayError.new(Spree.t(:payment_method_not_supported))
+        end
+
+        if payment_method.auto_capture?
+          purchase!
+        else
+          authorize!
         end
       end
 

From a8af3a27b18ce26d1ce274903cb7ee7f9e25f5ee Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 13 Jul 2020 09:56:05 +0200
Subject: [PATCH 27/43] Fix all but Metrics Rubocop cops in processing.rb

---
 app/models/spree/payment/processing.rb | 101 +++++++++++++++++--------
 1 file changed, 68 insertions(+), 33 deletions(-)

diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index b2421d5c8a..08a6ebd069 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -39,16 +39,18 @@ module Spree
         protect_from_connection_error do
           check_environment
 
-          if payment_method.payment_profiles_supported?
-            # Gateways supporting payment profiles will need access to credit card object because this stores the payment profile information
-            # so supply the authorization itself as well as the credit card, rather than just the authorization code
-            response = payment_method.capture(self, source, gateway_options)
-          else
-            # Standard ActiveMerchant capture usage
-            response = payment_method.capture(money.money.cents,
+          response = if payment_method.payment_profiles_supported?
+                       # Gateways supporting payment profiles will need access to credit
+                       # card object because this stores the payment profile information
+                       # so supply the authorization itself as well as the credit card,
+                       # rather than just the authorization code
+                       payment_method.capture(self, source, gateway_options)
+                     else
+                       # Standard ActiveMerchant capture usage
+                       payment_method.capture(money.money.cents,
                                               response_code,
                                               gateway_options)
-          end
+                     end
 
           handle_response(response, :complete, :failure)
         end
@@ -60,14 +62,17 @@ module Spree
         protect_from_connection_error do
           check_environment
 
-          if payment_method.payment_profiles_supported?
-            # Gateways supporting payment profiles will need access to credit card object because this stores the payment profile information
-            # so supply the authorization itself as well as the credit card, rather than just the authorization code
-            response = payment_method.void(response_code, source, gateway_options)
-          else
-            # Standard ActiveMerchant void usage
-            response = payment_method.void(response_code, gateway_options)
-          end
+          response = if payment_method.payment_profiles_supported?
+                       # Gateways supporting payment profiles will need access to credit
+                       # card object because this stores the payment profile information
+                       # so supply the authorization itself as well as the credit card,
+                       # rather than just the authorization code
+                       payment_method.void(response_code, source, gateway_options)
+                     else
+                       # Standard ActiveMerchant void usage
+                       payment_method.void(response_code, gateway_options)
+                     end
+
           record_response(response)
 
           if response.success?
@@ -83,14 +88,28 @@ module Spree
         protect_from_connection_error do
           check_environment
 
-          credit_amount ||= credit_allowed >= order.outstanding_balance.abs ? order.outstanding_balance.abs : credit_allowed.abs
+          credit_amount ||= if credit_allowed >= order.outstanding_balance.abs
+                              order.outstanding_balance.abs
+                            else
+                              credit_allowed.abs
+                            end
+
           credit_amount = credit_amount.to_f
 
-          if payment_method.payment_profiles_supported?
-            response = payment_method.credit((credit_amount * 100).round, source, response_code, gateway_options)
-          else
-            response = payment_method.credit((credit_amount * 100).round, response_code, gateway_options)
-          end
+          response = if payment_method.payment_profiles_supported?
+                       payment_method.credit(
+                         (credit_amount * 100).round,
+                         source,
+                         response_code,
+                         gateway_options
+                       )
+                     else
+                       payment_method.credit(
+                         (credit_amount * 100).round,
+                         response_code,
+                         gateway_options
+                       )
+                     end
 
           record_response(response)
 
@@ -115,11 +134,20 @@ module Spree
 
           refund_amount = calculate_refund_amount(refund_amount)
 
-          if payment_method.payment_profiles_supported?
-            response = payment_method.refund((refund_amount * 100).round, source, response_code, gateway_options)
-          else
-            response = payment_method.refund((refund_amount * 100).round, response_code, gateway_options)
-          end
+          response = if payment_method.payment_profiles_supported?
+                       payment_method.refund(
+                         (refund_amount * 100).round,
+                         source,
+                         response_code,
+                         gateway_options
+                       )
+                     else
+                       payment_method.refund(
+                         (refund_amount * 100).round,
+                         response_code,
+                         gateway_options
+                       )
+                     end
 
           record_response(response)
 
@@ -168,7 +196,11 @@ module Spree
       private
 
       def calculate_refund_amount(refund_amount = nil)
-        refund_amount ||= credit_allowed >= order.outstanding_balance.abs ? order.outstanding_balance.abs : credit_allowed.abs
+        refund_amount ||= if credit_allowed >= order.outstanding_balance.abs
+                            order.outstanding_balance.abs
+                          else
+                            credit_allowed.abs
+                          end
         refund_amount.to_f
       end
 
@@ -176,9 +208,12 @@ module Spree
         protect_from_connection_error do
           check_environment
 
-          response = payment_method.send(action, (amount * 100).round,
-                                         source,
-                                         gateway_options)
+          response = payment_method.public_send(
+            action,
+            (amount * 100).round,
+            source,
+            gateway_options
+          )
           handle_response(response, success_state, :failure)
         end
       end
@@ -196,9 +231,9 @@ module Spree
               self.cvv_response_message = response.cvv_result['message']
             end
           end
-          send("#{success_state}!")
+          __send__("#{success_state}!")
         else
-          send(failure_state)
+          __send__(failure_state)
           gateway_error(response)
         end
       end

From 3a64cc426adce209cfcf064fe7d9dd34e9df6532 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 13 Jul 2020 09:58:10 +0200
Subject: [PATCH 28/43] Reuse #calculate_refund_amount method

---
 app/models/spree/payment/processing.rb | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 08a6ebd069..3d77de36fe 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -88,13 +88,7 @@ module Spree
         protect_from_connection_error do
           check_environment
 
-          credit_amount ||= if credit_allowed >= order.outstanding_balance.abs
-                              order.outstanding_balance.abs
-                            else
-                              credit_allowed.abs
-                            end
-
-          credit_amount = credit_amount.to_f
+          credit_amount = calculate_refund_amount(credit_amount)
 
           response = if payment_method.payment_profiles_supported?
                        payment_method.credit(

From 70afcee3fc29ae81f5712ec45c5acda013ac50ec Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 15 Jul 2020 14:18:36 +0200
Subject: [PATCH 29/43] Fix Spree's spec clashing with a customization

`#save_requested_by_customer` is an accessor we added and thus, the
Spree's spec didn't consider.
---
 spec/models/spree/payment_spec.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 07559f5314..ee4c0bc93d 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -541,16 +541,17 @@ describe Spree::Payment do
         end
 
         context "when successfully connecting to the gateway" do
-          xit "should create a payment profile" do
+          it "should create a payment profile" do
             gateway.name = 'Gateway'
             gateway.distributors << create(:distributor_enterprise)
             gateway.save!
 
             payment.payment_method = gateway
+            payment.source.save_requested_by_customer = true
 
             expect(gateway).to receive(:create_profile)
 
-            payment = Spree::Payment.create(
+            Spree::Payment.create(
               :amount => 100,
               :order => create(:order),
               :source => card,

From dd5e679f6961e3543d5ed6aef3c557c3f86b6bda Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 16 Jul 2020 15:30:28 +0200
Subject: [PATCH 30/43] Address code review comments

Mostly styling issues.
---
 app/models/spree/payment.rb            | 6 +-----
 app/models/spree/payment/processing.rb | 2 +-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index a9e0b6b460..66c401cd8a 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -10,6 +10,7 @@ module Spree
     IDENTIFIER_CHARS = (('A'..'Z').to_a + ('0'..'9').to_a - %w(0 1 I O)).freeze
 
     delegate :line_items, to: :order
+    delegate :currency, to: :order
 
     belongs_to :order, class_name: 'Spree::Order'
     belongs_to :source, polymorphic: true
@@ -68,8 +69,6 @@ module Spree
       end
     end
 
-    delegate :currency, to: :order
-
     def money
       Spree::Money.new(amount, currency: currency)
     end
@@ -87,9 +86,6 @@ module Spree
       credit_allowed.positive?
     end
 
-    # see https://github.com/spree/spree/issues/981
-    #
-    # Import from future Spree v.2.3.0 d470b31798f37
     def build_source
       return if source_attributes.nil?
       return unless payment_method.andand.payment_source_class
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 3d77de36fe..14caece4af 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -12,7 +12,7 @@ module Spree
 
         unless payment_method.supports?(source)
           invalidate!
-          raise Core::GatewayError.new(Spree.t(:payment_method_not_supported))
+          raise Core::GatewayError, Spree.t(:payment_method_not_supported)
         end
 
         if payment_method.auto_capture?

From 26ed601996dd360903ec5acf33b20c41d472e1e0 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 15 Jul 2020 19:14:46 +0200
Subject: [PATCH 31/43] Test the payment controller handles GatewayError

After that, we can TDD a second one that also handles validation errors.
---
 config/locales/en.yml                         |  1 +
 .../payments/payments_controller_spec.rb      | 24 ++++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 2975120a53..b9612340a1 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -35,6 +35,7 @@ en:
       spree/payment:
         amount: Amount
         state: State
+        source: Source
       spree/product:
         primary_taxon: "Product Category"
         supplier: "Supplier"
diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
index 6932efa236..2acc3da19c 100644
--- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
+++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
@@ -10,7 +10,7 @@ describe Spree::Admin::PaymentsController, type: :controller do
     allow(controller).to receive(:spree_current_user) { user }
   end
 
-  context "#create" do
+  describe "#create" do
     let!(:payment_method) { create(:payment_method, distributors: [shop]) }
     let(:params) { { amount: order.total, payment_method_id: payment_method.id } }
 
@@ -138,4 +138,26 @@ describe Spree::Admin::PaymentsController, type: :controller do
       end
     end
   end
+
+  describe '#fire' do
+    context 'on credit event' do
+      let(:shop) { create(:enterprise) }
+      let(:payment_method) { create(:stripe_sca_payment_method, distributor_ids: [create(:distributor_enterprise).id], preferred_enterprise_id: shop.id) }
+      let(:order) { create(:order, state: 'complete') }
+      let(:payment) { create(:payment, order: order, payment_method: payment_method, amount: order.total) }
+
+      let(:params) { { e: 'credit', order_id: order.number, id: payment.id } }
+
+      it 'handles gateway errors' do
+        allow(request).to receive(:referer) { 'http://foo.com' }
+        allow_any_instance_of(payment_method.class)
+          .to receive(:credit).and_raise(Spree::Core::GatewayError, 'error message')
+
+        spree_put :fire, params
+
+        expect(flash[:error]).to eq('error message')
+        expect(response).to redirect_to('http://foo.com')
+      end
+    end
+  end
 end

From 59da07de66953c04c65bbb57b62a1741daaaa450 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 17 Jul 2020 14:06:55 +0200
Subject: [PATCH 32/43] Handle all errors when dealing with payment event

This basically catches ActiveRecord::RecordInvalid caused by an invalid
credit record, for instance, but also other situations we haven't
forseen.
---
 app/controllers/spree/admin/payments_controller.rb  |  2 +-
 app/models/spree/payment/processing.rb              |  2 +-
 .../orders/payments/payments_controller_spec.rb     | 13 ++++++++++++-
 spec/models/spree/payment_spec.rb                   |  2 +-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb
index 215f69972a..408e6e5c81 100644
--- a/app/controllers/spree/admin/payments_controller.rb
+++ b/app/controllers/spree/admin/payments_controller.rb
@@ -61,7 +61,7 @@ module Spree
         else
           flash[:error] = t(:cannot_perform_operation)
         end
-      rescue Spree::Core::GatewayError => e
+      rescue StandardError => e
         flash[:error] = e.message
       ensure
         redirect_to request.referer
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 14caece4af..2d01a91aea 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -108,7 +108,7 @@ module Spree
           record_response(response)
 
           if response.success?
-            self.class.create(
+            self.class.create!(
               order: order,
               source: self,
               payment_method: payment_method,
diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
index 2acc3da19c..3a04e932d6 100644
--- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
+++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
@@ -148,8 +148,9 @@ describe Spree::Admin::PaymentsController, type: :controller do
 
       let(:params) { { e: 'credit', order_id: order.number, id: payment.id } }
 
+      before { allow(request).to receive(:referer) { 'http://foo.com' } }
+
       it 'handles gateway errors' do
-        allow(request).to receive(:referer) { 'http://foo.com' }
         allow_any_instance_of(payment_method.class)
           .to receive(:credit).and_raise(Spree::Core::GatewayError, 'error message')
 
@@ -158,6 +159,16 @@ describe Spree::Admin::PaymentsController, type: :controller do
         expect(flash[:error]).to eq('error message')
         expect(response).to redirect_to('http://foo.com')
       end
+
+      it 'handles validation errors' do
+        allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
+        allow(payment).to receive(:credit!).and_raise(StandardError, 'validation error')
+
+        spree_put :fire, params
+
+        expect(flash[:error]).to eq('validation error')
+        expect(response).to redirect_to('http://foo.com')
+      end
     end
   end
 end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 77f09e2ef3..a9027d8d34 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -391,7 +391,7 @@ describe Spree::Payment do
 
         context "when response is successful" do
           it "should create an offsetting payment" do
-            Spree::Payment.should_receive(:create)
+            Spree::Payment.should_receive(:create!)
             payment.credit!
           end
 

From 73b1b1f172881351104025c80d5fe67550f30041 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 17 Jul 2020 13:32:31 +0200
Subject: [PATCH 33/43] DRY specs and fix rubocop failures

---
 .../payments/payments_controller_spec.rb      | 23 ++++++++++++++-----
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
index 3a04e932d6..4f6a36f592 100644
--- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
+++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe Spree::Admin::PaymentsController, type: :controller do
@@ -141,17 +143,27 @@ describe Spree::Admin::PaymentsController, type: :controller do
 
   describe '#fire' do
     context 'on credit event' do
-      let(:shop) { create(:enterprise) }
-      let(:payment_method) { create(:stripe_sca_payment_method, distributor_ids: [create(:distributor_enterprise).id], preferred_enterprise_id: shop.id) }
+      let(:payment_method) do
+        create(
+          :stripe_sca_payment_method,
+          distributor_ids: [create(:distributor_enterprise).id],
+          preferred_enterprise_id: create(:enterprise).id
+        )
+      end
       let(:order) { create(:order, state: 'complete') }
-      let(:payment) { create(:payment, order: order, payment_method: payment_method, amount: order.total) }
+      let(:payment) do
+        create(:payment, order: order, payment_method: payment_method, amount: order.total)
+      end
 
       let(:params) { { e: 'credit', order_id: order.number, id: payment.id } }
 
-      before { allow(request).to receive(:referer) { 'http://foo.com' } }
+      before do
+        allow(request).to receive(:referer) { 'http://foo.com' }
+        allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
+      end
 
       it 'handles gateway errors' do
-        allow_any_instance_of(payment_method.class)
+        allow(payment.payment_method)
           .to receive(:credit).and_raise(Spree::Core::GatewayError, 'error message')
 
         spree_put :fire, params
@@ -161,7 +173,6 @@ describe Spree::Admin::PaymentsController, type: :controller do
       end
 
       it 'handles validation errors' do
-        allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
         allow(payment).to receive(:credit!).and_raise(StandardError, 'validation error')
 
         spree_put :fire, params

From 1c026479f55e780aefb295234a02c6aa01e735c8 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 22 Jul 2020 11:08:34 +0200
Subject: [PATCH 34/43] Replace spec's syntax to RSpec 3

---
 spec/models/spree/payment_spec.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index a9027d8d34..9ee488a4c7 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -391,13 +391,13 @@ describe Spree::Payment do
 
         context "when response is successful" do
           it "should create an offsetting payment" do
-            Spree::Payment.should_receive(:create!)
+            expect(Spree::Payment).to receive(:create!)
             payment.credit!
           end
 
           it "resulting payment should have correct values" do
-            payment.order.stub :outstanding_balance => 100
-            payment.stub :credit_allowed => 10
+            allow(payment.order).to receive(:outstanding_balance) { 100 }
+            allow(payment).to receive(:credit_allowed) { 10 }
 
             offsetting_payment = payment.credit!
             expect(offsetting_payment.amount.to_f).to eq(-10)

From f2fd426c4ad73f49df3c6493a28545c553ce12ac Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 17 Jul 2020 16:23:23 +0200
Subject: [PATCH 35/43] Fix old Spree specs

Given the importance of this code, it doesn't bring me much confidence.
Apparently, this specs where using a non-existent state by mistake and
this went unnoticed because the payment creation was failing silently in
payment/processing.rb.

This unearthed the fact that our `#ensure_correct_adjustment` needs the
order to be persisted to succeed.
---
 spec/models/spree/payment_spec.rb | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 9ee488a4c7..43a0a05127 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -2,11 +2,7 @@ require 'spec_helper'
 
 describe Spree::Payment do
   context 'original specs from Spree' do
-    let(:order) do
-      order = Spree::Order.new(:bill_address => Spree::Address.new,
-                               :ship_address => Spree::Address.new)
-    end
-
+    let(:order) { create(:order) }
     let(:gateway) do
       gateway = Spree::Gateway::Bogus.new(:environment => 'test', :active => true)
       gateway.stub :source_required => true
@@ -339,7 +335,7 @@ describe Spree::Payment do
 
       context "#credit" do
         before do
-          payment.state = 'complete'
+          payment.state = 'completed'
           payment.response_code = '123'
         end
 

From f2b28a198d3c386d93337f49fa61c7a6a6fedb8a Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 22 Jul 2020 14:44:58 +0200
Subject: [PATCH 36/43] Replace before_validation with custom validation

No reason to use a callback when custom validation methods can be
defined.
---
 app/models/spree/payment.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 66c401cd8a..8126ad851c 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -22,7 +22,7 @@ module Spree
 
     has_one :adjustment, as: :source, dependent: :destroy
 
-    before_validation :validate_source
+    validate :validate_source
     before_save :set_unique_identifier
 
     after_save :create_payment_profile, if: :profiles_supported?

From 0f0a7041471450379f0fa72c5d64148b5683d77e Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 17 Jul 2020 14:07:05 +0200
Subject: [PATCH 37/43] Skip source validation when applying credit

The original payment may not be valid because its credit card may be
expired. Stripe gives this as a valid scenario returning a success and
we should do too.

When creating the credit payment we end up validating all sources in
a chain as follows.

```
Payment being persisted -> source payment -> original credit card.
```

The source payment was valid when created (It would not be persisted
otherwise) but its source card may now be expired, and that's legit.

There was also an issue with the `#invalidate_old_payments` callback. It
was causing the original payment to be validated again and thus the
credit payment failed to be persisted due to the original credit card
being expired. Switching this callback to use `#update_column` skips
validations and so we don't validate the source payment. We only care
about the state there, so it should be fine.
---
 app/models/spree/payment.rb                   | 20 +++++++++++++++++--
 app/models/spree/payment/processing.rb        |  3 ++-
 .../payments/payments_controller_spec.rb      | 10 ++++++++++
 spec/models/spree/payment_spec.rb             | 19 ++++++++++++++++++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 8126ad851c..f940db551f 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -32,7 +32,19 @@ module Spree
     # invalidate previously entered payments
     after_create :invalidate_old_payments
 
+    # Prevents #validate_source to end up in a chain of validations, from
+    # payment to payment up until the original credit card record.
+    #
+    # All payments are validated before saving thus, we get nothing by checking
+    # they are valid in the future. Quite the opposite. Stripe is accepting
+    # refunds whose source are cards that were valid when the payment was
+    # placed but are now expired, and we consider them invalid.
+    #
+    # Skipping the source validation when applying a refund avoids this
+    # situation. We trust the payment gateway.
+    attr_accessor :skip_source_validation
     attr_accessor :source_attributes
+
     after_initialize :build_source
 
     scope :from_credit_card, -> { where(source_type: 'Spree::CreditCard') }
@@ -151,7 +163,7 @@ module Spree
     end
 
     def validate_source
-      if source && !source.valid?
+      if source && !skip_source_validation && !source.valid?
         source.errors.each do |field, error|
           field_name = I18n.t("activerecord.attributes.#{source.class.to_s.underscore}.#{field}")
           errors.add(Spree.t(source.class.to_s.demodulize.underscore), "#{field_name} #{error}")
@@ -176,8 +188,12 @@ module Spree
       gateway_error e
     end
 
+    # Inherited from Spree, makes newly entered payments invalidate previously
+    # entered payments so the most recent payment is used by the gateway.
     def invalidate_old_payments
-      order.payments.with_state('checkout').where("id != ?", id).each(&:invalidate!)
+      order.payments.with_state('checkout').where("id != ?", id).each do |payment|
+        payment.update_column(:state, 'invalid')
+      end
     end
 
     def update_order
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 2d01a91aea..ec7d154f66 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -114,7 +114,8 @@ module Spree
               payment_method: payment_method,
               amount: credit_amount.abs * -1,
               response_code: response.authorization,
-              state: 'completed'
+              state: 'completed',
+              skip_source_validation: true
             )
           else
             gateway_error(response)
diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
index 4f6a36f592..27ea7dac50 100644
--- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
+++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
@@ -157,6 +157,8 @@ describe Spree::Admin::PaymentsController, type: :controller do
 
       let(:params) { { e: 'credit', order_id: order.number, id: payment.id } }
 
+      let(:successful_response) { ActiveMerchant::Billing::Response.new(true, "Yay!") }
+
       before do
         allow(request).to receive(:referer) { 'http://foo.com' }
         allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
@@ -180,6 +182,14 @@ describe Spree::Admin::PaymentsController, type: :controller do
         expect(flash[:error]).to eq('validation error')
         expect(response).to redirect_to('http://foo.com')
       end
+
+      it 'displays a success message and redirects to the referer' do
+        allow(payment_method).to receive(:credit) { successful_response }
+
+        spree_put :fire, params
+
+        expect(flash[:success]).to eq(I18n.t(:payment_updated))
+      end
     end
   end
 end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 43a0a05127..144122631b 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -401,6 +401,25 @@ describe Spree::Payment do
             expect(offsetting_payment.response_code).to eq('12345')
             expect(offsetting_payment.source).to eq(payment)
           end
+
+          context 'and the source payment card is expired' do
+            let(:card) do
+              Spree::CreditCard.new(month: 12, year: 1995, number: '4111111111111111')
+            end
+
+            let(:successful_response) do
+              ActiveMerchant::Billing::Response.new(true, "Yay!")
+            end
+
+            it 'lets the new payment to be saved' do
+              allow(payment.order).to receive(:outstanding_balance) { 100 }
+              allow(payment).to receive(:credit_allowed) { 10 }
+
+              offsetting_payment = payment.credit!
+
+              expect(offsetting_payment).to be_valid
+            end
+          end
         end
       end
     end

From c0f72f89f2daa7f97609682f0a26c4133e5da1b9 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 22 Jul 2020 16:52:31 +0200
Subject: [PATCH 38/43] Handle #refund! as we do with #credit!

---
 app/models/spree/payment/processing.rb        |  2 +-
 .../payments/payments_controller_spec.rb      | 64 +++++++++++++++----
 spec/models/spree/payment_spec.rb             |  2 +-
 3 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index ec7d154f66..f50a23ccb0 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -147,7 +147,7 @@ module Spree
           record_response(response)
 
           if response.success?
-            self.class.create(order: order,
+            self.class.create!(order: order,
                               source: self,
                               payment_method: payment_method,
                               amount: refund_amount.abs * -1,
diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
index 27ea7dac50..8e78be69d8 100644
--- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
+++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
@@ -142,23 +142,23 @@ describe Spree::Admin::PaymentsController, type: :controller do
   end
 
   describe '#fire' do
+    let(:payment_method) do
+      create(
+        :stripe_sca_payment_method,
+        distributor_ids: [create(:distributor_enterprise).id],
+        preferred_enterprise_id: create(:enterprise).id
+      )
+    end
+    let(:order) { create(:order, state: 'complete') }
+    let(:payment) do
+      create(:payment, order: order, payment_method: payment_method, amount: order.total)
+    end
+
+    let(:successful_response) { ActiveMerchant::Billing::Response.new(true, "Yay!") }
+
     context 'on credit event' do
-      let(:payment_method) do
-        create(
-          :stripe_sca_payment_method,
-          distributor_ids: [create(:distributor_enterprise).id],
-          preferred_enterprise_id: create(:enterprise).id
-        )
-      end
-      let(:order) { create(:order, state: 'complete') }
-      let(:payment) do
-        create(:payment, order: order, payment_method: payment_method, amount: order.total)
-      end
-
       let(:params) { { e: 'credit', order_id: order.number, id: payment.id } }
 
-      let(:successful_response) { ActiveMerchant::Billing::Response.new(true, "Yay!") }
-
       before do
         allow(request).to receive(:referer) { 'http://foo.com' }
         allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
@@ -191,5 +191,41 @@ describe Spree::Admin::PaymentsController, type: :controller do
         expect(flash[:success]).to eq(I18n.t(:payment_updated))
       end
     end
+
+    context 'on refund event' do
+      let(:params) { { e: 'refund', order_id: order.number, id: payment.id } }
+
+      before do
+        allow(request).to receive(:referer) { 'http://foo.com' }
+        allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
+      end
+
+      it 'handles gateway errors' do
+        allow(payment.payment_method)
+          .to receive(:refund).and_raise(Spree::Core::GatewayError, 'error message')
+
+        spree_put :fire, params
+
+        expect(flash[:error]).to eq('error message')
+        expect(response).to redirect_to('http://foo.com')
+      end
+
+      it 'handles validation errors' do
+        allow(payment).to receive(:refund!).and_raise(StandardError, 'validation error')
+
+        spree_put :fire, params
+
+        expect(flash[:error]).to eq('validation error')
+        expect(response).to redirect_to('http://foo.com')
+      end
+
+      it 'displays a success message and redirects to the referer' do
+        allow(payment_method).to receive(:refund) { successful_response }
+
+        spree_put :fire, params
+
+        expect(flash[:success]).to eq(I18n.t(:payment_updated))
+      end
+    end
   end
 end
diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb
index 144122631b..200efab868 100644
--- a/spec/models/spree/payment_spec.rb
+++ b/spec/models/spree/payment_spec.rb
@@ -725,7 +725,7 @@ describe Spree::Payment do
       end
     end
 
-    describe "refunding" do
+    describe "refund!" do
       let(:payment) { create(:payment) }
       let(:success) { double(success?: true, authorization: 'abc123') }
       let(:failure) { double(success?: false) }

From 813459ee384b711d1141c0790f32caa956cb0a07 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 22 Jul 2020 17:05:45 +0200
Subject: [PATCH 39/43] Clarify method documentation

---
 app/models/spree/payment.rb | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index f940db551f..13fc27c6aa 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -32,16 +32,12 @@ module Spree
     # invalidate previously entered payments
     after_create :invalidate_old_payments
 
-    # Prevents #validate_source to end up in a chain of validations, from
-    # payment to payment up until the original credit card record.
+    # Skips the validation of the source (for example, credit card) of the payment.
     #
-    # All payments are validated before saving thus, we get nothing by checking
-    # they are valid in the future. Quite the opposite. Stripe is accepting
-    # refunds whose source are cards that were valid when the payment was
-    # placed but are now expired, and we consider them invalid.
-    #
-    # Skipping the source validation when applying a refund avoids this
-    # situation. We trust the payment gateway.
+    # This is used in refunds as the validation of the card can fail but the refund can go through,
+    #    we trust the payment gateway in these cases. For example, Stripe is accepting refunds with
+    #    source cards that were valid when the payment was placed but are now expired, and we
+    #    consider them invalid.
     attr_accessor :skip_source_validation
     attr_accessor :source_attributes
 
@@ -188,10 +184,13 @@ module Spree
       gateway_error e
     end
 
-    # Inherited from Spree, makes newly entered payments invalidate previously
-    # entered payments so the most recent payment is used by the gateway.
+    # Makes newly entered payments invalidate previously entered payments so the most recent payment
+    # is used by the gateway.
     def invalidate_old_payments
       order.payments.with_state('checkout').where("id != ?", id).each do |payment|
+
+        # Using update_column skips validations and so it skips validate_source. As we are just
+        # invalidating past payments here, we don't want to validate all of them at this stage.
         payment.update_column(:state, 'invalid')
       end
     end

From 4d9fbb68d6cc5f538c067546f780d8ad7194096a Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 22 Jul 2020 17:10:14 +0200
Subject: [PATCH 40/43] Add missing attribute to skip source validation

---
 app/models/spree/payment/processing.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index f50a23ccb0..724df22d87 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -152,7 +152,8 @@ module Spree
                               payment_method: payment_method,
                               amount: refund_amount.abs * -1,
                               response_code: response.authorization,
-                              state: 'completed')
+                              state: 'completed',
+                              skip_source_validation: true)
           else
             gateway_error(response)
           end

From e6943ce554e38cb4593ad081a1f67b9638fee914 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 22 Jul 2020 17:11:32 +0200
Subject: [PATCH 41/43] Fix simple Rubocop issues

---
 app/models/spree/payment.rb            |  1 -
 app/models/spree/payment/processing.rb | 16 +++++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 13fc27c6aa..9ca91809dc 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -188,7 +188,6 @@ module Spree
     # is used by the gateway.
     def invalidate_old_payments
       order.payments.with_state('checkout').where("id != ?", id).each do |payment|
-
         # Using update_column skips validations and so it skips validate_source. As we are just
         # invalidating past payments here, we don't want to validate all of them at this stage.
         payment.update_column(:state, 'invalid')
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 724df22d87..2579f76de7 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -147,13 +147,15 @@ module Spree
           record_response(response)
 
           if response.success?
-            self.class.create!(order: order,
-                              source: self,
-                              payment_method: payment_method,
-                              amount: refund_amount.abs * -1,
-                              response_code: response.authorization,
-                              state: 'completed',
-                              skip_source_validation: true)
+            self.class.create!(
+              order: order,
+              source: self,
+              payment_method: payment_method,
+              amount: refund_amount.abs * -1,
+              response_code: response.authorization,
+              state: 'completed',
+              skip_source_validation: true
+            )
           else
             gateway_error(response)
           end

From 357037e429d983d34dc5e87b1b8ab3818615c198 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 23 Jul 2020 17:31:34 +0200
Subject: [PATCH 42/43] Recalculate adjustments when invalidating payments

Switching from `#invalidate` to `#update_column` skipped both
validations and callbacks and thus, `#ensure_correct_adjustments` was no
longer called for older payments.
---
 app/models/spree/payment.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 9ca91809dc..8b54e5c760 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -191,6 +191,7 @@ module Spree
         # Using update_column skips validations and so it skips validate_source. As we are just
         # invalidating past payments here, we don't want to validate all of them at this stage.
         payment.update_column(:state, 'invalid')
+        payment.ensure_correct_adjustment
       end
     end
 

From 97f551a2dd21e5fcd64f0fb74004525c4625d93d Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 23 Jul 2020 17:37:50 +0200
Subject: [PATCH 43/43] Replace literal with AR's 4 `#not`

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

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 8b54e5c760..754119034a 100644
--- a/app/models/spree/payment.rb
+++ b/app/models/spree/payment.rb
@@ -187,7 +187,7 @@ module Spree
     # Makes newly entered payments invalidate previously entered payments so the most recent payment
     # is used by the gateway.
     def invalidate_old_payments
-      order.payments.with_state('checkout').where("id != ?", id).each do |payment|
+      order.payments.with_state('checkout').where.not(id: id).each do |payment|
         # Using update_column skips validations and so it skips validate_source. As we are just
         # invalidating past payments here, we don't want to validate all of them at this stage.
         payment.update_column(:state, 'invalid')