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

@@ -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

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)

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

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