From 544a8470cf98ead09bf8a063b5309af635dc9454 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Sat, 23 Sep 2017 10:33:58 +1000 Subject: [PATCH] Set transaction fee adjustments to ineligible if payment is invalid or failed --- app/models/spree/payment_decorator.rb | 23 ++++-- spec/models/spree/payment_spec.rb | 102 ++++++++++++++++++++++---- 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb index 31ed7f26395f..842cf0de4134 100644 --- a/app/models/spree/payment_decorator.rb +++ b/app/models/spree/payment_decorator.rb @@ -8,12 +8,12 @@ module Spree def ensure_correct_adjustment destroy_orphaned_paypal_payments if payment_method.is_a?(Spree::Gateway::PayPalExpress) - # Don't charge for invalid payments. - # PayPalExpress always creates a payment that is invalidated later. - # Unknown: What about failed payments? - if state == "invalid" - adjustment.andand.destroy - elsif 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 @@ -117,5 +117,16 @@ def destroy_orphaned_paypal_payments orphaned_payments = order.payments.where(payment_method_id: payment_method_id, source_id: nil) orphaned_payments.each(&:destroy) 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.reload + return if adjustment.finalized? + adjustment.update_attribute(:eligible, false) + adjustment.finalize! + end end end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 07f6bb94be29..61a8555dd3e3 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -129,23 +129,23 @@ module Spree let!(:order) { create(:order) } let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) } - # Mimicing call from PayPalController#confirm in spree_paypal_express - def create_new_paypal_express_payment(order, payment_method) - order.payments.create!({ - :source => Spree::PaypalExpressCheckout.create({ - :token => "123", - :payer_id => "456" - }, :without_protection => true), - :amount => order.total, - :payment_method => payment_method - }, :without_protection => true) - end - before do - order.update_totals + order.reload.update! end - context "for paypal express payments with transaction fees" do + context "to paypal express payments" do + # Mimicing call from PayPalController#confirm in spree_paypal_express + def create_new_paypal_express_payment(order, payment_method) + order.payments.create!({ + :source => Spree::PaypalExpressCheckout.create({ + :token => "123", + :payer_id => "456" + }, :without_protection => true), + :amount => order.total, + :payment_method => payment_method + }, :without_protection => true) + end + let!(:payment_method) { Spree::Gateway::PayPalExpress.create!(name: "PayPalExpress", distributor_ids: [create(:distributor_enterprise).id], environment: Rails.env) } let(:calculator) { Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10) } @@ -211,6 +211,80 @@ def create_new_paypal_express_payment(order, payment_method) 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) } + 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 " 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 + end + end + end end end end