Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve one-time payment webhook logic #375

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

vincanger
Copy link
Collaborator

@vincanger vincanger commented Feb 19, 2025

Description

Fixes #371

Adds a new webhook event handler for payment_intent.succeeded. One-time payment products (i.e. "credits"), characterized by Stripe as having the payment mode (not the subscription mode), do not send an invoice.paid event (although subscriptions do), so we have to confirm payment and handle it better than just using the checkout.session.completed event, which can be sent even in the case that a payment fails.

Contributor Checklist

Make sure to do the following steps if they are applicable to your PR:

Copy link
Collaborator

@infomiho infomiho left a comment

Choose a reason for hiding this comment

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

If I understood correctly, we want to give users access when we are sure that payment succeeded. That's why we introduced a new hook payment_intent.succeeded.

I've left some comments.

try {
const metadata = returnMetadataByMode({ mode, priceId });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const metadata = returnMetadataByMode({ mode, priceId });
const paymentIntentData = getPaymentIntentData({ mode, priceId });

Related to the suggested fn rename below.

default:
assertUnreachable(plan.effect);
}
const { subscriptionPlan } = getPlanEffectDetailsById(planId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here it feels like we are implicitly saying: we only want to update the subscriptionPlan value for subscription plans by not using the numOfCreditsPurchased value here. Can we be explicit about it? Something like:

if (plan.effect.kind === 'credits') {
  return;
}

Copy link
Collaborator

@infomiho infomiho left a comment

Choose a reason for hiding this comment

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

Good job 👍

@vincanger vincanger merged commit e1a9436 into main Feb 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add payment_intent.succeeded webhook handler to stripe webhook
2 participants