diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index 5d1fdd4a95..9fef097821 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -27,13 +27,7 @@ module Spree def show @order = Spree::Order.find_by!(number: params[:id]) - if params.key?("payment_intent") - result = ProcessPaymentIntent.new(params["payment_intent"], @order).call! - unless result.ok? - flash[:error] = "#{I18n.t("payment_could_not_process")}. #{result.error}" - end - @order.reload - end + handle_stripe_response end def empty @@ -146,6 +140,19 @@ module Spree private + # Stripe can redirect here after a payment is processed in the backoffice. + # We verify if it was successful here and persist the changes. + def handle_stripe_response + return unless params.key?("payment_intent") + + result = ProcessPaymentIntent.new(params["payment_intent"], @order).call! + + unless result.ok? + flash.now[:error] = "#{I18n.t("payment_could_not_process")}. #{result.error}" + end + @order.reload + end + def discard_empty_line_items @order.line_items = @order.line_items.select { |li| li.quantity > 0 } end diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index a2fcc94717..7ede0aa252 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/app/models/spree/gateway/stripe_sca.rb @@ -129,7 +129,19 @@ module Spree payment = fetch_payment(creditcard, gateway_options) raise Stripe::StripeError, I18n.t(:no_pending_payments) unless payment&.response_code - Stripe::PaymentIntentValidator.new.call(payment.response_code, stripe_account_id) + payment_intent_response = Stripe::PaymentIntentValidator.new. + call(payment.response_code, stripe_account_id) + + raise_if_not_in_capture_state(payment_intent_response) + + payment.response_code + end + + def raise_if_not_in_capture_state(payment_intent_response) + state = payment_intent_response.status + return if state == 'requires_capture' + + raise Stripe::StripeError, I18n.t(:invalid_payment_state, state: state) end def fetch_payment(creditcard, gateway_options) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 91fa765b1e..1151d94e92 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -50,6 +50,7 @@ module Spree scope :failed, -> { with_state('failed') } scope :valid, -> { where('state NOT IN (?)', %w(failed invalid)) } scope :authorization_action_required, -> { where.not(cvv_response_message: nil) } + scope :with_payment_intent, ->(code) { where(response_code: code) } # order state machine (see http://github.com/pluginaweek/state_machine/tree/master for details) state_machine initial: :checkout do diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb index ddf6f7496e..cd085c3391 100644 --- a/app/services/process_payment_intent.rb +++ b/app/services/process_payment_intent.rb @@ -23,21 +23,20 @@ class ProcessPaymentIntent end end - def initialize(payment_intent, order, last_payment = nil) + def initialize(payment_intent, order) @payment_intent = payment_intent @order = order - @last_payment = last_payment.presence || OrderPaymentFinder.new(order).last_payment + @payment = order.payments.pending.with_payment_intent(payment_intent).first end def call! - validate_intent! - return Result.new(ok: false) unless valid? + return Result.new(ok: false) unless payment.present? && ready_for_capture? + return Result.new(ok: true) if already_processed? - OrderWorkflow.new(order).next + process_payment - if last_payment.can_complete? - last_payment.complete! - last_payment.mark_as_processed + if payment.reload.completed? + payment.mark_as_processed Result.new(ok: true) else @@ -50,18 +49,29 @@ class ProcessPaymentIntent private - attr_reader :order, :payment_intent, :last_payment + attr_reader :order, :payment_intent, :payment - def valid? - order.present? && matches_last_payment? + def process_payment + if order.state == "payment" + # Moves the order to completed, which calls #process_payments! + OrderWorkflow.new(order).next + else + order.process_payments! + end end - def validate_intent! - Stripe::PaymentIntentValidator.new.call(payment_intent, stripe_account_id) + def ready_for_capture? + payment_intent_status == 'requires_capture' end - def matches_last_payment? - last_payment&.state == "pending" && last_payment&.response_code == payment_intent + def already_processed? + payment_intent_status == 'succeeded' + end + + def payment_intent_status + @payment_intent_status ||= Stripe::PaymentIntentValidator.new. + call(payment_intent, stripe_account_id). + status end def stripe_account_id @@ -69,6 +79,6 @@ class ProcessPaymentIntent end def preferred_enterprise_id - last_payment.payment_method.preferred_enterprise_id + payment.payment_method.preferred_enterprise_id end end diff --git a/lib/stripe/payment_intent_validator.rb b/lib/stripe/payment_intent_validator.rb index 075e064fcc..879a49eac2 100644 --- a/lib/stripe/payment_intent_validator.rb +++ b/lib/stripe/payment_intent_validator.rb @@ -8,9 +8,8 @@ module Stripe stripe_account: stripe_account_id) raise_if_last_payment_error_present(payment_intent_response) - raise_if_not_in_capture_state(payment_intent_response) - payment_intent_id + payment_intent_response end private @@ -21,12 +20,5 @@ module Stripe raise Stripe::StripeError, payment_intent_response.last_payment_error.message end - - def raise_if_not_in_capture_state(payment_intent_response) - state = payment_intent_response.status - return unless state != 'requires_capture' - - raise Stripe::StripeError, I18n.t(:invalid_payment_state, state: state) - end end end diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index 412b706cf5..ab29573117 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -82,7 +82,10 @@ describe Spree::OrdersController, type: :controller do describe "confirming a payment intent" do let(:customer) { create(:customer) } - let(:order) { create(:order, customer: customer, distributor: customer.enterprise) } + let(:order) { + create(:order_with_totals, customer: customer, distributor: customer.enterprise, + state: "payment") + } let(:payment_method) { create(:stripe_sca_payment_method) } let!(:payment) { create( :payment, @@ -102,26 +105,53 @@ describe Spree::OrdersController, type: :controller do context "with a valid payment intent" do let(:payment_intent) { "pi_123" } + let(:payment_intent_response) { double(id: "pi_123", status: "requires_capture") } before do allow_any_instance_of(Stripe::PaymentIntentValidator) .to receive(:call) .with(payment_intent, kind_of(String)) - .and_return(payment_intent) + .and_return(payment_intent_response) + + allow(Spree::Order).to receive(:find_by!) { order } end - it "completes the payment" do - get :show, params: { id: order.number, payment_intent: payment_intent } + context "when the order is in payment state" do + it "completes the payment" do + expect(order).to receive(:process_payments!) do + payment.complete! + end - expect(response.status).to eq 200 - payment.reload - expect(payment.cvv_response_message).to be nil - expect(payment.state).to eq("completed") + get :show, params: { id: order.number, payment_intent: payment_intent } + + expect(response.status).to eq 200 + payment.reload + expect(payment.state).to eq("completed") + expect(payment.cvv_response_message).to be nil + end + end + + context "when the order is already completed" do + before do + order.update_columns(state: "complete") + end + + it "should still process the payment" do + expect(order).to receive(:process_payments!) do + payment.complete! + end + + get :show, params: { id: order.number, payment_intent: payment_intent } + expect(response.status).to eq 200 + payment.reload + expect(payment.state).to eq("completed") + expect(payment.cvv_response_message).to be nil + end end end - context "with an invalid payment intent" do - let(:payment_intent) { "invalid" } + context "when the payment intent response has errors" do + let(:payment_intent) { "pi_123" } before do allow_any_instance_of(Stripe::PaymentIntentValidator) diff --git a/spec/lib/stripe/payment_intent_validator_spec.rb b/spec/lib/stripe/payment_intent_validator_spec.rb index ac4a06a83c..efebbb6577 100644 --- a/spec/lib/stripe/payment_intent_validator_spec.rb +++ b/spec/lib/stripe/payment_intent_validator_spec.rb @@ -27,23 +27,11 @@ module Stripe it "returns payment intent id and does not raise" do expect { result = validator.call(payment_intent_id, stripe_account_id) - expect(result).to eq payment_intent_id + expect(result).to eq payment_intent_response_body }.to_not raise_error Stripe::StripeError end end - context "when payment intent status is not requires status" do - let(:payment_intent_response_body) { - JSON.generate(id: payment_intent_id, status: "failed") - } - - it "raises Stripe error with an invalid_payment_state message" do - expect { - validator.call(payment_intent_id, stripe_account_id) - }.to raise_error Stripe::StripeError, "Invalid payment state: failed" - end - end - context "when payment intent contains an error" do let(:payment_intent_response_body) { JSON.generate(id: payment_intent_id, last_payment_error: { message: "No money" }) diff --git a/spec/models/spree/gateway/stripe_sca_spec.rb b/spec/models/spree/gateway/stripe_sca_spec.rb index 8ddfe99373..3eea62c02d 100644 --- a/spec/models/spree/gateway/stripe_sca_spec.rb +++ b/spec/models/spree/gateway/stripe_sca_spec.rb @@ -50,6 +50,35 @@ describe Spree::Gateway::StripeSCA, type: :model do expect(response.success?).to eq false expect(response.message).to eq "Invalid payment state: succeeded" end + + context "when payment intent state is not in 'requires_capture' state" do + before do + payment + end + + it "succeeds if payment intent state is requires_capture" do + stub_request(:post, "https://api.stripe.com/v1/payment_intents/12345/capture"). + with(body: {"amount_to_capture" => order.total}). + to_return(status: 200, body: capture_successful) + + allow(Stripe::PaymentIntentValidator).to receive_message_chain(:new, :call). + and_return(double(status: "requires_capture")) + + response = subject.purchase(order.total, credit_card, gateway_options) + + expect(response.success?).to eq true + end + + it "does not succeed if payment intent state is not requires_capture" do + allow(Stripe::PaymentIntentValidator).to receive_message_chain(:new, :call). + and_return(double(status: "not_ready_yet")) + + response = subject.purchase(order.total, credit_card, gateway_options) + + expect(response.success?).to eq false + expect(response.message).to eq "Invalid payment state: not_ready_yet" + end + end end def payment_intent(amount, status) diff --git a/spec/services/process_payment_intent_spec.rb b/spec/services/process_payment_intent_spec.rb index 5aabe3edbe..13b788395e 100644 --- a/spec/services/process_payment_intent_spec.rb +++ b/spec/services/process_payment_intent_spec.rb @@ -7,7 +7,9 @@ describe ProcessPaymentIntent do describe "processing a payment intent" do let(:customer) { create(:customer) } - let(:order) { create(:order, customer: customer, distributor: customer.enterprise, state: "payment") } + let(:order) { + create(:order_with_totals, customer: customer, distributor: customer.enterprise, state: "payment") + } let(:payment_method) { create(:stripe_sca_payment_method) } let!(:payment) { create( :payment, @@ -23,53 +25,110 @@ describe ProcessPaymentIntent do allow(Stripe::PaymentIntentValidator).to receive(:new).and_return(validator) end - context "an invalid intent" do - let(:intent) { "invalid" } + context "with an invalid intent" do + let(:intent) { "pi_123" } let(:service) { ProcessPaymentIntent.new(intent, order) } - before do - allow(validator) - .to receive(:call).with(intent, anything).and_raise(Stripe::StripeError, "error message") + context "which does not match the last payment" do + let(:intent) { "pi_456" } + + it "returns false" do + result = service.call! + + expect(result.ok?).to eq(false) + expect(result.error).to eq("") + end + + it "does not complete the payment" do + service.call! + expect(payment.reload.state).to eq("pending") + end end - it "returns the error message" do - result = service.call! + context "where the stripe payment intent validation responds with errors" do + before do + allow(validator) + .to receive(:call).with(intent, anything).and_raise(Stripe::StripeError, "error message") + end - expect(result.ok?).to eq(false) - expect(result.error).to eq("error message") - end + it "returns returns the error message" do + result = service.call! - it "does not complete the payment" do - service.call! - expect(payment.reload.state).to eq("pending") + expect(result.ok?).to eq(false) + expect(result.error).to eq("error message") + end + + it "does not complete the payment" do + service.call! + expect(payment.reload.state).to eq("pending") + end end end context "a valid intent" do let(:intent) { "pi_123" } + let(:intent_response) { double(status: "requires_capture") } let(:service) { ProcessPaymentIntent.new(intent, order) } before do allow(order).to receive(:deliver_order_confirmation_email) - allow(validator).to receive(:call).with(intent, anything).and_return(intent) + allow(validator).to receive(:call).with(intent, anything).and_return(intent_response) end it "validates the intent" do + expect(order).to receive(:process_payments!) { true } service.call! expect(validator).to have_received(:call) end - it "completes the payment" do + it "processes the order's payment" do + allow(order).to receive(:pending_payments) { [payment] } + + expect(order).to receive(:process_payments!).and_call_original + expect(payment).to receive(:purchase!) { true } service.call! - payment.reload - expect(payment.state).to eq("completed") - expect(payment.cvv_response_message).to be nil end - it "completes the order" do - service.call! - expect(order.state).to eq("complete") - expect(order).to have_received(:deliver_order_confirmation_email) + context "when payment processing succeeds" do + before do + allow(order).to receive(:process_payments!) do + payment.complete! + end + end + + it "completes the payment" do + service.call! + payment.reload + expect(payment.state).to eq("completed") + expect(payment.cvv_response_message).to be nil + end + + it "completes the order" do + service.call! + expect(order.state).to eq("complete") + expect(order).to have_received(:deliver_order_confirmation_email) + end + end + + context "when payment processing fails" do + before do + allow(order).to receive(:process_payments!) do + payment.failure! + end + end + + it "does not complete the payment" do + service.call! + payment.reload + expect(payment.state).to eq("failed") + end + + it "completes the order, but with failed payment state recorded" do + service.call! + expect(order.state).to eq("complete") + expect(order.payment_state).to eq("failed") + expect(order).to have_received(:deliver_order_confirmation_email) + end end end @@ -97,11 +156,12 @@ describe ProcessPaymentIntent do context "when the payment can't be completed" do let(:intent) { "pi_123" } - let(:service) { ProcessPaymentIntent.new(intent, order, payment) } + let(:intent_response) { double(id: "pi_123", status: "requires_capture") } + let(:service) { ProcessPaymentIntent.new(intent, order) } before do - allow(payment).to receive(:can_complete?).and_return(false) - allow(validator).to receive(:call).with(intent, anything).and_return(intent) + allow(order).to receive(:process_payments!) { nil } + allow(validator).to receive(:call).with(intent, anything).and_return(intent_response) end it "returns a failed result" do