diff --git a/Gemfile b/Gemfile index ee149950cf..e6fd5c3dac 100644 --- a/Gemfile +++ b/Gemfile @@ -88,7 +88,7 @@ gem 'redis', '>= 4.0', require: ['redis', 'redis/connection/hiredis'] gem 'sidekiq' gem 'sidekiq-scheduler' -gem "cable_ready", "5.0.0.pre2" +gem "cable_ready", "5.0.0.pre3" gem 'combine_pdf' gem 'wicked_pdf' diff --git a/Gemfile.lock b/Gemfile.lock index 8707a67ca4..26f619f637 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -178,7 +178,7 @@ GEM activesupport (>= 3.0.0) uniform_notifier (~> 1.11) byebug (11.1.3) - cable_ready (5.0.0.pre2) + cable_ready (5.0.0.pre3) rails (>= 5.2) thread-local (>= 1.1.0) cancancan (1.15.0) @@ -703,7 +703,7 @@ DEPENDENCIES bugsnag bullet byebug - cable_ready (= 5.0.0.pre2) + cable_ready (= 5.0.0.pre3) cancancan (~> 1.15.0) capybara catalog! diff --git a/app/controllers/concerns/order_stock_check.rb b/app/controllers/concerns/order_stock_check.rb index 235056c0bb..a3c09d1838 100644 --- a/app/controllers/concerns/order_stock_check.rb +++ b/app/controllers/concerns/order_stock_check.rb @@ -25,7 +25,7 @@ module OrderStockCheck end def reset_order_to_cart - return if Flipper.enabled? :split_checkout + return if Flipper.enabled? :split_checkout, spree_current_user OrderCheckoutRestart.new(@order).call end diff --git a/app/controllers/payment_gateways/stripe_controller.rb b/app/controllers/payment_gateways/stripe_controller.rb index cb9331f3da..fc9cb1dd93 100644 --- a/app/controllers/payment_gateways/stripe_controller.rb +++ b/app/controllers/payment_gateways/stripe_controller.rb @@ -67,12 +67,16 @@ module PaymentGateways @valid_payment_intent ||= begin return false unless params["payment_intent"]&.starts_with?("pi_") - @order.state == "payment" && - last_payment&.state == "requires_authorization" && - last_payment&.response_code == params["payment_intent"] + order_and_payment_valid? end end + def order_and_payment_valid? + @order.state.in?(["payment", "confirmation"]) && + last_payment&.state == "requires_authorization" && + last_payment&.response_code == params["payment_intent"] + end + def last_payment @last_payment ||= OrderPaymentFinder.new(@order).last_payment end diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 19eb4da3a0..63ca43a42b 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -22,6 +22,8 @@ class SplitCheckoutController < ::BaseController def update if confirm_order || update_order + return if performed? + clear_invalid_payments advance_order_state redirect_to_step @@ -45,12 +47,27 @@ class SplitCheckoutController < ::BaseController return unless validate_summary! && @order.errors.empty? @order.customer.touch :terms_and_conditions_accepted_at + + return true if redirect_to_payment_gateway + @order.confirm! order_completion_reset @order end + def redirect_to_payment_gateway + return unless selected_payment_method&.external_gateway? + return unless (redirect_url = selected_payment_method.external_payment_url(order: @order)) + + render operations: cable_car.redirect_to(url: URI(redirect_url)) + true + end + + def selected_payment_method + @selected_payment_method ||= Checkout::PaymentMethodFetcher.new(@order).call + end + def update_order - return if @order.errors.any? + return if params[:confirm_order] || @order.errors.any? @order.select_shipping_method(params[:shipping_method_id]) @order.update(order_params) @@ -95,7 +112,7 @@ class SplitCheckoutController < ::BaseController end def order_params - @order_params ||= Checkout::Params.new(@order, params).call + @order_params ||= Checkout::Params.new(@order, params, spree_current_user).call end def redirect_to_step diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index aa08cca513..c050fc93ca 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -24,8 +24,8 @@ module Spree order.update_totals order.payment_required? } - go_to_state :confirmation, if: ->(_order) { - Flipper.enabled? :split_checkout + go_to_state :confirmation, if: ->(order) { + Flipper.enabled? :split_checkout, order.created_by } go_to_state :complete end diff --git a/app/services/checkout/params.rb b/app/services/checkout/params.rb index 10715c8acc..cef90428e8 100644 --- a/app/services/checkout/params.rb +++ b/app/services/checkout/params.rb @@ -2,9 +2,10 @@ module Checkout class Params - def initialize(order, params) + def initialize(order, params, current_user) @params = params @order = order + @current_user = current_user end def call @@ -13,17 +14,18 @@ module Checkout apply_strong_parameters set_address_details set_payment_amount + set_existing_card @order_params end private - attr_reader :params, :order + attr_reader :params, :order, :current_user def apply_strong_parameters @order_params = params.require(:order).permit( - :email, :shipping_method_id, :special_instructions, :existing_card_id, + :email, :shipping_method_id, :special_instructions, :save_bill_address, :save_ship_address, bill_address_attributes: ::PermittedAttributes::Address.attributes, ship_address_attributes: ::PermittedAttributes::Address.attributes, @@ -50,6 +52,22 @@ module Checkout @order_params[:payments_attributes].first[:amount] = order.total end + def set_existing_card + return unless existing_card_selected? + + card = Spree::CreditCard.find(params[:existing_card_id]) + + if card.user_id.blank? || card.user_id != current_user&.id + raise Spree::Core::GatewayError, I18n.t(:invalid_credit_card) + end + + @order_params[:payments_attributes].first[:source] = card + end + + def existing_card_selected? + @order_params[:payments_attributes] && params[:existing_card_id].present? + end + def addresses_present? @order_params[:ship_address_attributes] && @order_params[:bill_address_attributes] end diff --git a/app/services/checkout/payment_method_fetcher.rb b/app/services/checkout/payment_method_fetcher.rb new file mode 100644 index 0000000000..15b1829325 --- /dev/null +++ b/app/services/checkout/payment_method_fetcher.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Checkout + class PaymentMethodFetcher + def initialize(order) + @order = order + end + + def call + latest_payment&.payment_method + end + + private + + attr_reader :order + + def latest_payment + order.payments.order(:created_at).last + end + end +end diff --git a/app/views/split_checkout/payment/_stripe_sca.html.haml b/app/views/split_checkout/payment/_stripe_sca.html.haml index f75a3b2aa8..36b9cb5adb 100644 --- a/app/views/split_checkout/payment/_stripe_sca.html.haml +++ b/app/views/split_checkout/payment/_stripe_sca.html.haml @@ -3,7 +3,7 @@ .checkout-input %label = t('split_checkout.step2.form.stripe.use_saved_card') - = select_tag :existing_card, + = select_tag :existing_card_id, options_for_select(stripe_card_options(@saved_credit_cards) + [[t('split_checkout.step2.form.stripe.create_new_card'), ""]], @selected_card), { "data-action": "change->stripe-cards#onSelectCard", "data-stripe-cards-target": "select" } diff --git a/package.json b/package.json index b465a2acc6..a0fcde890d 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "@hotwired/turbo": "^7.1.0", "@rails/webpacker": "5.4.3", "babel-loader": "^8.2.3", - "cable_ready": "^5.0.0-pre2", + "cable_ready": "5.0.0-pre3", "flatpickr": "^4.6.9", "foundation-sites": "^5.5.2", "jquery-ui": "1.13.0", diff --git a/spec/controllers/payment_gateways/stripe_controller_spec.rb b/spec/controllers/payment_gateways/stripe_controller_spec.rb index 596702fac4..d1819cccfb 100644 --- a/spec/controllers/payment_gateways/stripe_controller_spec.rb +++ b/spec/controllers/payment_gateways/stripe_controller_spec.rb @@ -46,17 +46,21 @@ module PaymentGateways order.payments << payment end - it "completes the order and redirects to the order confirmation page" do - expect(controller).to receive(:processing_succeeded).and_call_original - expect(controller).to receive(:order_completion_reset).and_call_original + shared_examples "successful order completion" do + it "completes the order and redirects to the order confirmation page" do + expect(controller).to receive(:processing_succeeded).and_call_original + expect(controller).to receive(:order_completion_reset).and_call_original - get :confirm, params: { payment_intent: "pi_123" } + get :confirm, params: { payment_intent: "pi_123" } - expect(order.completed?).to be true - expect(response).to redirect_to order_path(order, order_token: order.token) - expect(flash[:notice]).to eq I18n.t(:order_processed_successfully) + expect(order.completed?).to be true + expect(response).to redirect_to order_path(order, order_token: order.token) + expect(flash[:notice]).to eq I18n.t(:order_processed_successfully) + end end + include_examples "successful order completion" + it "creates a customer record" do order.update_columns(customer_id: nil) Customer.delete_all @@ -65,6 +69,17 @@ module PaymentGateways get :confirm, params: { payment_intent: "pi_123" } }.to change { Customer.count }.by(1) end + + context "using split checkout" do + before do + allow(Flipper).to receive(:enabled?).with(:split_checkout) { true } + allow(Flipper).to receive(:enabled?).with(:split_checkout, anything) { true } + + order.update_attribute :state, "confirmation" + end + + include_examples "successful order completion" + end end context "when the order is not in payment state" do diff --git a/spec/controllers/split_checkout_controller_spec.rb b/spec/controllers/split_checkout_controller_spec.rb index 9c8963a349..9d73967e63 100644 --- a/spec/controllers/split_checkout_controller_spec.rb +++ b/spec/controllers/split_checkout_controller_spec.rb @@ -141,6 +141,28 @@ describe SplitCheckoutController, type: :controller do expect(order.reload.state).to eq "confirmation" end end + + context "with a saved credit card" do + let!(:saved_card) { create(:stored_credit_card, user: user) } + let(:checkout_params) do + { + order: { + payments_attributes: [ + { payment_method_id: payment_method.id } + ] + }, + existing_card_id: saved_card.id + } + end + + it "updates and redirects to payment step" do + put :update, params: params + + expect(response).to redirect_to checkout_step_path(:summary) + expect(order.reload.state).to eq "confirmation" + expect(order.payments.first.source.id).to eq saved_card.id + end + end end context "summary step" do @@ -193,6 +215,24 @@ describe SplitCheckoutController, type: :controller do end end end + + context "when an external payment gateway is used" do + before do + expect(Checkout::PaymentMethodFetcher). + to receive_message_chain(:new, :call) { payment_method } + expect(payment_method).to receive(:external_gateway?) { true } + expect(payment_method).to receive(:external_payment_url) { "https://example.com/pay" } + end + + describe "confirming the order" do + it "redirects to the payment gateway's URL" do + put :update, params: params + + expect(response.body).to match("https://example.com/pay").and match("redirect") + expect(order.reload.state).to eq "confirmation" + end + end + end end end end diff --git a/spec/services/checkout/payment_method_fetcher_spec.rb b/spec/services/checkout/payment_method_fetcher_spec.rb new file mode 100644 index 0000000000..3607eac7fc --- /dev/null +++ b/spec/services/checkout/payment_method_fetcher_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Checkout::PaymentMethodFetcher do + let!(:order) { create(:completed_order_with_totals) } + let(:payment1) { build(:payment, order: order) } + let(:payment2) { build(:payment, order: order) } + let(:service) { Checkout::PaymentMethodFetcher.new(order) } + + describe '#call' do + context "when the order has payments" do + before do + order.payments << payment1 + order.payments << payment2 + end + + it "returns the payment_method of the most recently created payment" do + expect(service.call).to eq payment2.payment_method + end + end + + context "when the order has no payments" do + it "returns nil" do + expect(service.call).to be_nil + end + end + end +end diff --git a/yarn.lock b/yarn.lock index 9d9d1b1f78..60d19df993 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4134,10 +4134,10 @@ bytes@3.1.0: resolved "https://registry.yarnpkg.com/bytes/-/bytes-3.1.0.tgz#f6cf7933a360e0588fa9fde85651cdc7f805d1f6" integrity sha512-zauLjrfCG+xvoyaqLoV8bLVXXNGC4JqlxFCutSDWA6fJrTo2ZuvLYTqZ7aHBLZSMOopbzwv8f+wZcVzfVTI2Dg== -cable_ready@^5.0.0-pre2: - version "5.0.0-pre2" - resolved "https://registry.yarnpkg.com/cable_ready/-/cable_ready-5.0.0-pre2.tgz#c3589ebfe28a10db3998ae3711d31a072be53bfb" - integrity sha512-/Ql2e/Hz0kRRKzzz06sMXFm3+/qacYqSHfWkdt1CcsI7I6OOOhHzPAgY8VS+zqa/6+ggjWoPqHyi8Hr6j/0O6w== +cable_ready@5.0.0-pre3: + version "5.0.0-pre3" + resolved "https://registry.yarnpkg.com/cable_ready/-/cable_ready-5.0.0-pre3.tgz#d36bb9648aead748dfdf0f46dfb489799733f330" + integrity sha512-jFjTJ/K/AiVIgjKr63qXGzp9xZOt2/2UyjVpkMkSzFyDHlYILOwnK4WofGwBKS6tXwDZMfohbIzW/p+LKbrlgA== dependencies: morphdom "^2.6.1"