From 3c73282d575ff55199e6977036a2bbeb0cac01cd Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 12 Jul 2017 16:20:34 +0200 Subject: [PATCH] Manually reset the order once completed Spree does not call after_ methods any more as of https://github.com/spree/spree/pull/2557, so our #after_complete method is never triggered and thus the order never reset. This makes the condition: ```ruby if current_order.andand.distributor == @order.distributor ``` in app/views/spree/orders/form/_update_buttons.html.haml return false and as a result the "Back To Cart" button is not shown. This commit resets the order (emptying the session[:order_id] and creating a new order, aka. cart) right from the CheckoutController#update rather than relying on infernal callbacks (of what the Spree core team itself was unhappy about since long ago https://github.com/spree/spree/issues/2488). There is the first place where we know the order has been successfully completed. --- app/controllers/checkout_controller.rb | 2 + .../spree/checkout_controller_decorator.rb | 4 -- app/helpers/checkout_helper.rb | 12 +----- app/models/reset_order_service.rb | 15 ++++++++ spec/controllers/checkout_controller_spec.rb | 4 ++ .../spree/checkout_controller_spec.rb | 37 ++++++++----------- .../consumer/shopping/checkout_spec.rb | 20 +++++++--- 7 files changed, 52 insertions(+), 42 deletions(-) create mode 100644 app/models/reset_order_service.rb diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 5432ef8b2a..15d4058909 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -40,6 +40,8 @@ class CheckoutController < Spree::CheckoutController set_default_bill_address set_default_ship_address + reset_order + flash[:success] = t(:order_processed_successfully) respond_to do |format| format.html do diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index 7841c86c36..e5146394d5 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -32,8 +32,4 @@ Spree::CheckoutController.class_eval do @order.bill_address ||= preferred_bill_address || last_used_bill_address || Spree::Address.default @order.ship_address ||= preferred_ship_address || last_used_ship_address || nil end - - def after_complete - reset_order - end end diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index 278422edbb..37b84040b6 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -105,17 +105,7 @@ module CheckoutHelper end def reset_order - distributor = current_order.distributor - token = current_order.token - - session[:order_id] = nil - @current_order = nil - current_order(true) - - current_order.set_distributor!(distributor) - current_order.tokenized_permission.token = token - current_order.tokenized_permission.save! - session[:access_token] = token + ResetOrderService.new(self).call end def payment_method_price(method, order) diff --git a/app/models/reset_order_service.rb b/app/models/reset_order_service.rb new file mode 100644 index 0000000000..faef5bff63 --- /dev/null +++ b/app/models/reset_order_service.rb @@ -0,0 +1,15 @@ +class ResetOrderService < SimpleDelegator + def call + distributor = current_order.distributor + token = current_order.token + + session[:order_id] = nil + __getobj__.instance_variable_set(:@current_order, nil) + current_order(true) + + current_order.set_distributor!(distributor) + current_order.tokenized_permission.token = token + current_order.tokenized_permission.save! + session[:access_token] = token + end +end diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 1f5e40a841..c4f770cb6c 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -106,6 +106,8 @@ describe CheckoutController do end it "returns order confirmation url on success" do + expect(controller).to receive(:reset_order) + order.stub(:update_attributes).and_return true order.stub(:state).and_return "complete" @@ -116,6 +118,8 @@ describe CheckoutController do describe "stale object handling" do it "retries when a stale object error is encountered" do + expect(controller).to receive(:reset_order) + order.stub(:update_attributes).and_return true controller.stub(:state_callback) diff --git a/spec/controllers/spree/checkout_controller_spec.rb b/spec/controllers/spree/checkout_controller_spec.rb index b9b85c92fa..e8840c8bad 100644 --- a/spec/controllers/spree/checkout_controller_spec.rb +++ b/spec/controllers/spree/checkout_controller_spec.rb @@ -4,45 +4,40 @@ require 'support/request/authentication_workflow' describe Spree::CheckoutController do - include AuthenticationWorkflow - context "After completing an order" do - it "should create a new empty order" do - controller.current_order(true) - controller.send(:after_complete) - session[:order_id].should_not be_nil + let!(:order) { controller.current_order(true) } + + it "creates a new empty order" do + controller.reset_order + expect(controller.session[:order_id]).not_to be_nil end - it "should clear the current order cache" do - order = controller.current_order(true) - controller.send(:after_complete) - controller.current_order.should_not == order + it "clears the current order cache" do + controller.reset_order + expect(controller.current_order).not_to eq order end - it "should set the new order's distributor to the same as the old order" do - order = controller.current_order(true) + it "sets the new order's distributor to the same as the old order" do distributor = create(:distributor_enterprise) order.set_distributor!(distributor) - controller.send(:after_complete) + controller.reset_order - controller.current_order.distributor.should == distributor + expect(controller.current_order.distributor).to eq distributor end - it "should set the new order's token to the same as the old order, and preserve the access token in the session" do - order = controller.current_order(true) + it "sets the new order's token to the same as the old order, and preserve the access token in the session" do + controller.reset_order - controller.send(:after_complete) - - controller.current_order.token.should == order.token - session[:access_token].should == order.token + expect(controller.current_order.token).to eq order.token + expect(session[:access_token]).to eq order.token end end context "rendering edit from within spree for the current checkout state" do let!(:order) { controller.current_order(true) } let!(:line_item) { create(:line_item, order: order) } - let!(:user) { create_enterprise_user } + let!(:user) { create(:user) } it "redirects to the OFN checkout page" do controller.stub(:skip_state_validation?) { true } diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 220fdcb89b..6f8cd59de1 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -245,13 +245,16 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do describe "purchasing" do it "takes us to the order confirmation page when we submit a complete form" do toggle_details + within "#details" do fill_in "First Name", with: "Will" fill_in "Last Name", with: "Marshall" fill_in "Email", with: "test@test.com" fill_in "Phone", with: "0468363090" end + toggle_billing + within "#billing" do fill_in "Address", with: "123 Your Face" select "Australia", from: "Country" @@ -259,35 +262,40 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do fill_in "City", with: "Melbourne" fill_in "Postcode", with: "3066" end + toggle_shipping + within "#shipping" do choose sm2.name fill_in 'Any comments or special instructions?', with: "SpEcIaL NoTeS" end + toggle_payment + within "#payment" do choose pm1.name end expect do place_order - page.should have_content "Your order has been processed successfully" + expect(page).to have_content "Your order has been processed successfully" end.to enqueue_job ConfirmOrderJob # And the order's special instructions should be set - o = Spree::Order.complete.first - expect(o.special_instructions).to eq "SpEcIaL NoTeS" + order = Spree::Order.complete.first + expect(order.special_instructions).to eq "SpEcIaL NoTeS" # And the Spree tax summary should not be displayed - page.should_not have_content product.tax_category.name + expect(page).not_to have_content product.tax_category.name # And the total tax for the order, including shipping and fee tax, should be displayed # product tax ($10.00 @ 10% = $0.91) # + fee tax ($ 1.23 @ 10% = $0.11) # + shipping tax ($ 4.56 @ 25% = $0.91) # = $1.93 - page.should have_content "(includes tax)" - page.should have_content with_currency(1.93) + expect(page).to have_content '(includes tax)' + expect(page).to have_content with_currency(1.93) + expect(page).to have_content 'Back To Store' end context "with basic details filled" do