Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class PaymentIntentSubscriber
include Omnes::Subscriber

handle :"stripe.payment_intent.succeeded", with: :complete_payment
handle :"stripe.payment_intent.payment_failed", with: :fail_payment

# Captures a payment.
#
Expand All @@ -15,8 +16,7 @@ class PaymentIntentSubscriber
#
# @param event [SolidusStripe::Webhook::Event]
def complete_payment(event)
payment_intent_id = event.data.object.id
payment = Spree::Payment.find_by!(response_code: payment_intent_id)
payment = extract_payment_from_event(event)
return if payment.completed?

payment.complete!.tap do
Expand All @@ -27,6 +27,32 @@ def complete_payment(event)
)
end
end

# Fails a payment.
#
# Marks a Solidus payment associated to a Stripe payment intent as
# failed, adding a log entry about the event.
#
# @param event [SolidusStripe::Webhook::Event]
def fail_payment(event)
payment = extract_payment_from_event(event)
return if payment.failed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return if payment.failed?
return if !payment.can_failure?

I'd say that checking more generally if we can transition to failed is more generic and safe. You could be on a state (like checkout) where payment is not failed but doing the transition will raise an error. Same goes for the complete! event of the same file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion, @loicginoux. If Solidus payment is in the :checkout state, but we're already getting a payment_intent.payment_failed webhook, that's probably an abnormal situation. I think it's better to be stricter so we can't detect any failure in our integration.


payment.failure!.tap do
SolidusStripe::LogEntries.payment_log(
payment,
success: false,
message: "Payment was marked as failed after payment_intent.failed webhook"
)
end
end

private

def extract_payment_from_event(event)
payment_intent_id = event.data.object.id
Spree::Payment.find_by!(response_code: payment_intent_id)
end
end
end
end
1 change: 1 addition & 0 deletions lib/solidus_stripe/webhook/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class Event

CORE_EVENTS = Set[*%i[
payment_intent.succeeded
payment_intent.payment_failed
]].freeze
private_constant :CORE_EVENTS

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
require "solidus_stripe_spec_helper"

RSpec.describe SolidusStripe::WebhooksController, type: %i[request webhook_request] do
describe "POST /create payment_intent.payment_failed" do
it "transitions the associated payment to failed" do
payment_method = create(:stripe_payment_method)
stripe_payment_intent = Stripe::PaymentIntent.construct_from(id: "pi_123")
payment = create(:payment,
payment_method: payment_method,
response_code: stripe_payment_intent.id,
state: "pending")
context = SolidusStripe::Webhook::EventWithContextFactory.from_object(
payment_method: payment_method,
object: stripe_payment_intent,
type: "payment_intent.payment_failed"
)

expect do
webhook_request(context)
end.to change { payment.reload.state }.from("pending").to("failed")
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@

described_class.new.complete_payment(event)

details = payment.log_entries.last.parsed_details
expect(details.success?).to be(true)
expect(
payment.log_entries.last.parsed_details.message
details.message
).to eq "Capture was successful after payment_intent.succeeded webhook"
end

Expand All @@ -61,4 +63,65 @@
expect(payment.log_entries.count).to be(0)
end
end

describe "#fail_payment" do
it "fails a pending payment" do
payment_method = create(:stripe_payment_method)
stripe_payment_intent = Stripe::PaymentIntent.construct_from(id: "pi_123")
payment = create(:payment,
payment_method: payment_method,
response_code: stripe_payment_intent.id,
state: "pending")
event = SolidusStripe::Webhook::EventWithContextFactory.from_object(
payment_method: payment_method,
object: stripe_payment_intent,
type: "payment_intent.payment_failed"
).solidus_stripe_object

described_class.new.fail_payment(event)

expect(payment.reload.state).to eq "failed"
end

it "adds a log entry to the payment" do
payment_method = create(:stripe_payment_method)
stripe_payment_intent = Stripe::PaymentIntent.construct_from(id: "pi_123")
payment = create(:payment,
payment_method: payment_method,
response_code: stripe_payment_intent.id,
state: "pending")
event = SolidusStripe::Webhook::EventWithContextFactory.from_object(
payment_method: payment_method,
object: stripe_payment_intent,
type: "payment_intent.payment_failed"
).solidus_stripe_object

described_class.new.fail_payment(event)

details = payment.log_entries.last.parsed_details
expect(details.success?).to be(false)
expect(
details.message
).to eq "Payment was marked as failed after payment_intent.failed webhook"
end

it "does nothing if the payment is already failed" do
payment_method = create(:stripe_payment_method)
stripe_payment_intent = Stripe::PaymentIntent.construct_from(id: "pi_123")
payment = create(:payment,
payment_method: payment_method,
response_code: stripe_payment_intent.id,
state: "failed")
event = SolidusStripe::Webhook::EventWithContextFactory.from_object(
payment_method: payment_method,
object: stripe_payment_intent,
type: "payment_intent.payment_failed",
).solidus_stripe_object

described_class.new.fail_payment(event)

expect(payment.reload.state).to eq "failed"
expect(payment.log_entries.count).to be(0)
end
end
end