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