From f4b1c5de9cb966d62d37c6f142e07fcc5bee11fa Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 Sep 2017 13:29:36 +1000 Subject: [PATCH] Remove orphaned PayPalExpress payments when processing actual PayPalExpress payment Fixes both #1074 and #1837 --- app/models/spree/payment_decorator.rb | 6 ++ spec/models/spree/payment_spec.rb | 88 +++++++++++++++++++++++++++ spec/requests/paypal_confirm_spec.rb | 68 +++++++++++++++++++++ 3 files changed, 162 insertions(+) create mode 100644 spec/requests/paypal_confirm_spec.rb diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb index 98b996cce7..576f2f764e 100644 --- a/app/models/spree/payment_decorator.rb +++ b/app/models/spree/payment_decorator.rb @@ -5,6 +5,7 @@ module Spree after_save :ensure_correct_adjustment, :update_order 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? @@ -81,5 +82,10 @@ module Spree refund_amount.to_f end + def destroy_orphaned_paypal_payments + return unless source_type == "Spree::PaypalExpressCheckout" + orphaned_payments = order.payments.where(payment_method_id: payment_method_id, source_id: nil) + orphaned_payments.each(&:destroy) + end end end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 0877959cbb..07f6bb94be 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -124,5 +124,93 @@ module Spree 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) } + + # 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 + end + + context "for paypal express payments with transaction fees" do + 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) } + + before do + payment_method.calculator = calculator + payment_method.save! + end + + context "when a payment already exists on the order" do + let!(:existing_payment) do + order.payments.create!(payment_method_id: payment_method.id, amount: order.total) + end + + context "and the existing payment uses the same payment method" do + context "and the existing payment does not have a source" do + it "removes the existing payment (and associated fees)" do + new_payment = create_new_paypal_express_payment(order, payment_method) + + order.reload + expect(order.payments.count).to eq 1 + expect(order.payments).to include new_payment + expect(order.payments).to_not include existing_payment + expect(order.adjustments.payment_fee.count).to eq 1 + expect(order.adjustments.payment_fee.first.amount).to eq 1.5 + end + end + + context "and the existing payment has a source" do + let!(:source) { Spree::PaypalExpressCheckout.create(token: "123") } + + before do + existing_payment.source = source + existing_payment.save! + end + + it "does not remove or invalidate the existing payment" do + new_payment = create_new_paypal_express_payment(order, payment_method) + + order.reload + expect(order.payments.count).to eq 2 + expect(order.payments).to include new_payment, existing_payment + expect(new_payment.state).to eq "checkout" + expect(existing_payment.state).to eq "checkout" + end + end + end + + context "and the existing payment uses a different method" do + let!(:another_payment_method) { Spree::Gateway::PayPalExpress.create!(name: "PayPalExpress", distributor_ids: [create(:distributor_enterprise).id], environment: Rails.env) } + + before do + existing_payment.update_attributes(payment_method_id: another_payment_method.id) + end + + it "does not remove or invalidate the existing payment" do + new_payment = create_new_paypal_express_payment(order, payment_method) + + order.reload + expect(order.payments).to include new_payment, existing_payment + expect(new_payment.state).to eq "checkout" + expect(existing_payment.state).to eq "checkout" + end + end + end + end + end end end diff --git a/spec/requests/paypal_confirm_spec.rb b/spec/requests/paypal_confirm_spec.rb new file mode 100644 index 0000000000..2f36d3a612 --- /dev/null +++ b/spec/requests/paypal_confirm_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +describe "confirming an order with paypal express payment method", type: :request do + include ShopWorkflow + + let!(:address) { create(:address) } + let!(:shop) { create(:enterprise) } + let!(:shipping_method) { create(:shipping_method, distributor_ids: [shop.id]) } + let!(:order) { create(:order, distributor: shop, ship_address: address.dup, bill_address: address.dup) } + let!(:shipment) { create(:shipment, order: order, shipping_method: shipping_method) } + let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) } + let!(:payment_method) { Spree::Gateway::PayPalExpress.create!(name: "PayPalExpress", distributor_ids: [create(:distributor_enterprise).id], environment: Rails.env) } + let(:params) { { token: 'lalalala', PayerID: 'payer1', payment_method_id: payment_method.id } } + let(:mocked_xml_response) { + " + + + Success + Something + + s0metran$act10n + + + " + } + + before do + order.reload.update_totals + order.shipping_method = shipping_method + expect(order.next).to be true # => address + expect(order.next).to be true # => delivery + expect(order.next).to be true # => payment + set_order order + + stub_request(:post, "https://api-3t.sandbox.paypal.com/2.0/") + .to_return(:status => 200, :body => mocked_xml_response ) + end + + context "with a flat percent calculator" do + let(:calculator) { Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10) } + + before do + payment_method.calculator = calculator + payment_method.save! + order.payments.create!(payment_method_id: payment_method.id, amount: order.total) + end + + it "destroys the old payment and processes the order" do + # Sanity check to condition of the order before we confirm the payment + expect(order.payments.count).to eq 1 + expect(order.payments.first.state).to eq "checkout" + expect(order.adjustments.payment_fee.count).to eq 1 + expect(order.adjustments.payment_fee.first.amount).to eq 1.5 + + get spree.confirm_paypal_path, params + + # Processing was successful, order is complete + expect(response).to redirect_to spree.order_path(order, :token => order.token) + expect(order.reload.complete?).to be true + + # We have only one payment, and one transaction fee + expect(order.payments.count).to eq 1 + expect(order.payments.first.state).to eq "completed" + expect(order.adjustments.payment_fee.count).to eq 1 + expect(order.adjustments.payment_fee.first.amount).to eq 1.5 + end + end +end