From dd73af8e7a78c963c93bf706e7c8c31785a6dd0c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 3 Apr 2021 18:52:05 +0100 Subject: [PATCH 1/6] Remove duplicate checkout controller --- app/controllers/spree/checkout_controller.rb | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 app/controllers/spree/checkout_controller.rb diff --git a/app/controllers/spree/checkout_controller.rb b/app/controllers/spree/checkout_controller.rb deleted file mode 100644 index e639505e29..0000000000 --- a/app/controllers/spree/checkout_controller.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -# This controller (and respective route in the Spree engine) -# is only needed for the spree_paypal_express gem that redirects here explicitly. -# -# According to the rails docs it would be possible to redirect -# to CheckoutController directly in the routes -# with a slash like "to: '/checkout#edit'", but it does not work in this case. -module Spree - class CheckoutController < ::BaseController - def edit - flash.keep - redirect_to main_app.checkout_path - end - end -end From fe507f63fc95d1fb13a529265b44132a2adc3558 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 3 Apr 2021 18:52:39 +0100 Subject: [PATCH 2/6] Move checkout_state route from spree to main_app --- config/routes.rb | 5 +++-- config/routes/spree.rb | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 5236a42c62..22e0db432a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -68,8 +68,9 @@ Openfoodnetwork::Application.routes.draw do resources :webhooks, only: [:create] end - get '/checkout', :to => 'checkout#edit' , :as => :checkout - put '/checkout', :to => 'checkout#update' , :as => :update_checkout + get '/checkout', to: 'checkout#edit' , as: :checkout + put '/checkout', to: 'checkout#update' , as: :update_checkout + get '/checkout/:state', to: 'checkout#edit', as: :checkout_state get '/checkout/paypal_payment/:order_id', to: 'checkout#paypal_payment', as: :paypal_payment get 'embedded_shopfront/shopfront_session', to: 'application#shopfront_session' diff --git a/config/routes/spree.rb b/config/routes/spree.rb index a16ccaf107..f93a869170 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -170,7 +170,6 @@ Spree::Core::Engine.routes.draw do resources :products # Used by spree_paypal_express - get '/checkout/:state', :to => 'checkout#edit', :as => :checkout_state get '/content/cvv', :to => 'content#cvv', :as => :cvv get '/content/*path', :to => 'content#show', :as => :content get '/paypal', :to => "paypal#express", :as => :paypal_express From e2e943e39435a1e389a47f747882dba3901bdd15 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 3 Apr 2021 18:55:18 +0100 Subject: [PATCH 3/6] Update uses of checkout_state route --- app/controllers/spree/orders_controller.rb | 2 +- app/controllers/spree/paypal_controller.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index 7eb12a39c5..71d4aa713d 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -92,7 +92,7 @@ module Spree format.html do if params.key?(:checkout) @order.next_transition.run_callbacks if @order.cart? - redirect_to checkout_state_path(@order.checkout_steps.first) + redirect_to main_app.checkout_state_path(@order.checkout_steps.first) elsif @order.complete? redirect_to order_path(@order) else diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index e139d53908..cae37e716b 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -27,11 +27,11 @@ module Spree 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(" ")) - redirect_to spree.checkout_state_path(:payment) + redirect_to main_app.checkout_state_path(:payment) end rescue SocketError flash[:error] = Spree.t('flash.connection_failed', scope: 'paypal') - redirect_to spree.checkout_state_path(:payment) + redirect_to main_app.checkout_state_path(:payment) end end @@ -57,7 +57,7 @@ module Spree session[:order_id] = nil redirect_to completion_route(@order) else - redirect_to checkout_state_path(@order.state) + redirect_to main_app.checkout_state_path(@order.state) end end From 4a18ba256dda579d40dab30731881b73eb70f26b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 5 Apr 2021 14:00:07 +0100 Subject: [PATCH 4/6] Add test for paypal controller redirect --- spec/controllers/spree/paypal_controller_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/controllers/spree/paypal_controller_spec.rb b/spec/controllers/spree/paypal_controller_spec.rb index 5e55a6d02d..0764a79489 100644 --- a/spec/controllers/spree/paypal_controller_spec.rb +++ b/spec/controllers/spree/paypal_controller_spec.rb @@ -44,6 +44,17 @@ module Spree expect(order.payments.count).to eq 0 end end + + context "when order completion fails" do + before do + allow(previous_order).to receive(:complete?).and_return(false) + end + + it "redirects to checkout state path" do + expect(spree_post(:confirm, payment_method_id: payment_method.id)). + to redirect_to checkout_state_path(:cart) + end + end end describe '#expire_current_order' do From 06c01955f577947f69ef53d8e160603324cbccc8 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 12 Apr 2021 16:44:45 +0100 Subject: [PATCH 5/6] Rename describe block names to controller actions --- spec/controllers/spree/paypal_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/spree/paypal_controller_spec.rb b/spec/controllers/spree/paypal_controller_spec.rb index 0764a79489..761ff189ab 100644 --- a/spec/controllers/spree/paypal_controller_spec.rb +++ b/spec/controllers/spree/paypal_controller_spec.rb @@ -4,13 +4,13 @@ require 'spec_helper' module Spree describe PaypalController, type: :controller do - context 'when cancelling' do + context '#cancel' do it 'redirects back to checkout' do expect(spree_get(:cancel)).to redirect_to checkout_path end end - context 'when confirming' do + context '#confirm' do let(:previous_order) { controller.current_order(true) } let(:payment_method) { create(:payment_method) } From 0dde8112d24132c6073b185af2c8fbfdb60a9fb8 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 12 Apr 2021 17:23:49 +0100 Subject: [PATCH 6/6] Add redirect specs for PaypalController#express --- .../spree/paypal_controller_spec.rb | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/controllers/spree/paypal_controller_spec.rb b/spec/controllers/spree/paypal_controller_spec.rb index 761ff189ab..b239667772 100644 --- a/spec/controllers/spree/paypal_controller_spec.rb +++ b/spec/controllers/spree/paypal_controller_spec.rb @@ -57,6 +57,48 @@ module Spree end end + describe "#express" do + let(:order) { create(:order_with_distributor) } + let(:response) { true } + let(:provider_success_url) { "https://test.com/success" } + let(:response_mock) { double(:response, success?: response, errors: [] ) } + let(:provider_mock) { double(:provider, build_set_express_checkout: true, + set_express_checkout: response_mock, + express_checkout_url: provider_success_url) } + + before do + allow(controller).to receive(:current_order) { order } + allow(controller).to receive(:provider) { provider_mock } + allow(controller).to receive(:express_checkout_request_details) { {} } + end + + context "when processing is successful" do + it "redirects to a success URL generated by the payment provider" do + expect(spree_post :express).to redirect_to provider_success_url + end + end + + context "when processing fails" do + let(:response) { false } + + it "redirects to checkout_state_path with a flash error" do + expect(spree_post :express).to redirect_to checkout_state_path(:payment) + expect(flash[:error]).to eq "PayPal failed. " + end + end + + context "when a SocketError is encountered during processing" do + before do + allow(response_mock).to receive(:success?).and_raise(SocketError) + end + + it "redirects to checkout_state_path with a flash error" do + expect(spree_post :express).to redirect_to checkout_state_path(:payment) + expect(flash[:error]).to eq "Could not connect to PayPal." + end + end + end + describe '#expire_current_order' do it 'empties the order_id of the session' do expect(session).to receive(:[]=).with(:order_id, nil)