Merge pull request #6391 from Matt-Yorkley/paypal-stock-clashes

Paypal stock clashes

app/controllers/checkout_controller.rb

@@ -5,6 +5,7 @@ require 'open_food_network/address_finder'
 class CheckoutController < Spree::StoreController
   layout 'darkswarm'
 
+  include OrderStockCheck
   include CheckoutHelper
   include OrderCyclesHelper
   include EnterprisesHelper
@@ -24,7 +25,7 @@ class CheckoutController < Spree::StoreController
 
   before_action :ensure_order_not_completed
   before_action :ensure_checkout_allowed
-  before_action :ensure_sufficient_stock_lines
+  before_action :handle_insufficient_stock
 
   before_action :associate_user
   before_action :check_authorization
@@ -77,13 +78,6 @@ class CheckoutController < Spree::StoreController
     redirect_to main_app.cart_path if @order.completed?
   end
 
-  def ensure_sufficient_stock_lines
-    if @order.insufficient_stock_lines.present?
-      flash[:error] = Spree.t(:inventory_error_flash_for_insufficient_quantity)
-      redirect_to main_app.cart_path
-    end
-  end
-
   def load_order
     @order = current_order
 

app/controllers/concerns/order_stock_check.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module OrderStockCheck
+  extend ActiveSupport::Concern
+
+  def handle_insufficient_stock
+    return if sufficient_stock?
+
+    flash[:error] = Spree.t(:inventory_error_flash_for_insufficient_quantity)
+    redirect_to main_app.cart_path
+  end
+
+  private
+
+  def sufficient_stock?
+    @sufficient_stock ||= @order.insufficient_stock_lines.blank?
+  end
+end

app/controllers/spree/paypal_controller_decorator.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 Spree::PaypalController.class_eval do
+  include OrderStockCheck
+
   before_action :enable_embedded_shopfront
   before_action :destroy_orphaned_paypal_payments, only: :confirm
   after_action :reset_order_when_complete, only: :confirm
@@ -40,6 +42,9 @@ Spree::PaypalController.class_eval do
     begin
       pp_response = provider.set_express_checkout(pp_request)
       if pp_response.success?
+        # At this point Paypal has *provisionally* accepted that the payment can now be placed,
+        # and the user will be redirected to a Paypal payment page. On completion, the user is
+        # sent back and the response is handled in the #confirm action in this controller.
         redirect_to provider.express_checkout_url(pp_response, useraction: 'commit')
       else
         flash[:error] = Spree.t('flash.generic_error', scope: 'paypal', reasons: pp_response.errors.map(&:long_message).join(" "))
@@ -51,6 +56,32 @@ Spree::PaypalController.class_eval do
     end
   end
 
+  def confirm
+    @order = current_order || raise(ActiveRecord::RecordNotFound)
+
+    # At this point the user has come back from the Paypal form, and we get one
+    # last chance to interact with the payment process before the money moves...
+    return reset_to_cart unless sufficient_stock?
+
+    @order.payments.create!({
+      source: Spree::PaypalExpressCheckout.create({
+        token: params[:token],
+        payer_id: params[:PayerID]
+      }),
+      amount: @order.total,
+      payment_method: payment_method
+    })
+    @order.next
+    if @order.complete?
+      flash.notice = Spree.t(:order_processed_successfully)
+      flash[:commerce_tracking] = "nothing special"
+      session[:order_id] = nil
+      redirect_to completion_route(@order)
+    else
+      redirect_to checkout_state_path(@order.state)
+    end
+  end
+
   def cancel
     flash[:notice] = Spree.t('flash.cancel', scope: 'paypal')
     redirect_to main_app.checkout_path
@@ -66,6 +97,10 @@ Spree::PaypalController.class_eval do
 
   private
 
+  def payment_method
+    @payment_method ||= Spree::PaymentMethod.find(params[:payment_method_id])
+  end
+
   def permit_parameters!
     params.permit(:token, :payment_method_id, :PayerID)
   end
@@ -79,6 +114,11 @@ Spree::PaypalController.class_eval do
     end
   end
 
+  def reset_to_cart
+    OrderCheckoutRestart.new(@order).call
+    handle_insufficient_stock
+  end
+
   # See #1074 and #1837 for more detail on why we need this
   # An 'orphaned' Spree::Payment is created for every call to CheckoutController#update
   # for orders that are processed using a Spree::Gateway::PayPalExpress payment method

spec/controllers/spree/paypal_controller_spec.rb

@@ -27,6 +27,23 @@ module Spree
         spree_post :confirm, payment_method_id: payment_method.id
         expect(session[:access_token]).to eq(controller.current_order.token)
       end
+
+      context "if the stock ran out whilst the payment was being placed" do
+        before do
+          allow(controller.current_order).to receive(:insufficient_stock_lines).and_return(true)
+        end
+
+        it "redirects to the cart with out of stock error" do
+          expect(spree_post(:confirm, payment_method_id: payment_method.id)).
+            to redirect_to cart_path
+
+          order = controller.current_order.reload
+
+          # Order is in "cart" state and did not complete processing of the payment
+          expect(order.state).to eq "cart"
+          expect(order.payments.count).to eq 0
+        end
+      end
     end
 
     describe '#expire_current_order' do

spec/features/consumer/shopping/checkout_paypal_spec.rb

@@ -3,6 +3,8 @@ require "spec_helper"
 feature "Check out with Paypal", js: true do
   include ShopWorkflow
   include CheckoutHelper
+  include AuthenticationHelper
+  include PaypalHelper
 
   let(:distributor) { create(:distributor_enterprise) }
   let(:supplier) { create(:supplier_enterprise) }
@@ -34,6 +36,7 @@ feature "Check out with Paypal", js: true do
       distributor_ids: [distributor.id]
     )
   end
+  let(:user) { create(:user) }
 
   before do
     distributor.shipping_methods << free_shipping
@@ -41,23 +44,43 @@ feature "Check out with Paypal", js: true do
     add_product_to_cart order, product
   end
 
-  describe "as a guest" do
+  context "as a guest" do
     it "fails with an error message" do
       visit checkout_path
       checkout_as_guest
       fill_out_form(free_shipping.name, paypal.name, save_default_addresses: false)
 
-      paypal_response = double(:response, success?: false, errors: [])
-      paypal_provider = double(
-        :provider,
-        build_set_express_checkout: nil,
-        set_express_checkout: paypal_response
-      )
-      allow_any_instance_of(Spree::PaypalController).to receive(:provider).
-        and_return(paypal_provider)
+      stub_paypal_response success: false
 
       place_order
       expect(page).to have_content "PayPal failed."
     end
   end
+
+  context "as a registered user" do
+    before { login_as user }
+
+    it "completes the checkout after successful Paypal payment" do
+      visit checkout_path
+      fill_out_details
+      fill_out_form(free_shipping.name, paypal.name, save_default_addresses: false)
+
+      # Normally the checkout would redirect to Paypal, a form would be filled out there, and the
+      # user would be redirected back to #confirm_paypal_path. Here we skip the PayPal part and
+      # jump straight to being redirected back to OFN with a "confirmed" payment.
+      stub_paypal_response(
+        success: true,
+        redirect: spree.confirm_paypal_path(
+          payment_method_id: paypal.id, token: "t123", PayerID: 'p123'
+        )
+      )
+      stub_paypal_confirm
+
+      place_order
+      expect(page).to have_content "Your order has been processed successfully"
+
+      expect(order.reload.state).to eq "complete"
+      expect(order.payments.count).to eq 1
+    end
+  end
 end

spec/requests/checkout/paypal_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe "checking out an order with a paypal express payment method", type: :request do
   include ShopWorkflow
+  include PaypalHelper
 
   let!(:address) { create(:address) }
   let!(:shop) { create(:enterprise) }
@@ -25,18 +26,6 @@ describe "checking out an order with a paypal express payment method", type: :re
     )
   end
   let(:params) { { token: 'lalalala', PayerID: 'payer1', payment_method_id: payment_method.id } }
-  let(:mocked_xml_response) {
-    "<?xml version=\"1.0\" encoding=\"UTF-8\"?>
-    <Envelope><Body>
-      <GetExpressCheckoutDetailsResponse>
-        <Ack>Success</Ack>
-        <PaymentDetails>Something</PaymentDetails>
-        <DoExpressCheckoutPaymentResponseDetails>
-          <PaymentInfo><TransactionID>s0metran$act10n</TransactionID></PaymentInfo>
-        </DoExpressCheckoutPaymentResponseDetails>
-      </GetExpressCheckoutDetailsResponse>
-    </Body></Envelope>"
-  }
 
   before do
     order.reload.update_totals
@@ -45,8 +34,7 @@ describe "checking out an order with a paypal express payment method", type: :re
     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 )
+    stub_paypal_confirm
   end
 
   context "with a flat percent calculator" do

spec/support/request/paypal_helper.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module PaypalHelper
+  # Initial request to confirm the payment can be placed (before redirecting to paypal)
+  def stub_paypal_response(options)
+    paypal_response = double(:response, success?: options[:success], errors: [])
+    paypal_provider = double(
+      :provider,
+      build_set_express_checkout: nil,
+      set_express_checkout: paypal_response,
+      express_checkout_url: options[:redirect]
+    )
+    allow_any_instance_of(Spree::PaypalController).to receive(:provider).
+      and_return(paypal_provider)
+  end
+
+  # Additional request to re-confirm the payment, when the order is finalised.
+  def stub_paypal_confirm
+    stub_request(:post, "https://api-3t.sandbox.paypal.com/2.0/")
+      .to_return(status: 200, body: mocked_xml_response )
+  end
+
+  private
+
+  def mocked_xml_response
+    "<?xml version=\"1.0\" encoding=\"UTF-8\"?>
+    <Envelope><Body>
+      <GetExpressCheckoutDetailsResponse>
+        <Ack>Success</Ack>
+        <PaymentDetails>Something</PaymentDetails>
+        <DoExpressCheckoutPaymentResponseDetails>
+          <PaymentInfo><TransactionID>s0metran$act10n</TransactionID></PaymentInfo>
+        </DoExpressCheckoutPaymentResponseDetails>
+      </GetExpressCheckoutDetailsResponse>
+    </Body></Envelope>"
+  end
+end