diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 299a8738ff..4cedfbbac9 100644 --- a/app/controllers/checkout_controller.rb +++ b/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 diff --git a/app/controllers/concerns/order_stock_check.rb b/app/controllers/concerns/order_stock_check.rb new file mode 100644 index 0000000000..2adfaf67be --- /dev/null +++ b/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 diff --git a/app/controllers/spree/paypal_controller_decorator.rb b/app/controllers/spree/paypal_controller_decorator.rb index ff03c823ef..71d559590a 100644 --- a/app/controllers/spree/paypal_controller_decorator.rb +++ b/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 diff --git a/spec/controllers/spree/paypal_controller_spec.rb b/spec/controllers/spree/paypal_controller_spec.rb index 9f630e892d..5e55a6d02d 100644 --- a/spec/controllers/spree/paypal_controller_spec.rb +++ b/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 diff --git a/spec/features/consumer/shopping/checkout_paypal_spec.rb b/spec/features/consumer/shopping/checkout_paypal_spec.rb index 107d425761..fc710e77f3 100644 --- a/spec/features/consumer/shopping/checkout_paypal_spec.rb +++ b/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 diff --git a/spec/requests/checkout/paypal_spec.rb b/spec/requests/checkout/paypal_spec.rb index e309d0ea5d..85ce495b9d 100644 --- a/spec/requests/checkout/paypal_spec.rb +++ b/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) { - " - - - Success - Something - - s0metran$act10n - - - " - } 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 diff --git a/spec/support/request/paypal_helper.rb b/spec/support/request/paypal_helper.rb new file mode 100644 index 0000000000..6796bb14b5 --- /dev/null +++ b/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 + " + + + Success + Something + + s0metran$act10n + + + " + end +end