laravel / cashier-stripe

Laravel Cashier provides an expressive, fluent interface to Stripe's subscription billing services.
https://laravel.com/docs/billing
MIT License
2.37k stars 667 forks source link

Confirm Payment notification never sent #1676

Closed ragulka closed 3 weeks ago

ragulka commented 3 months ago

Cashier Stripe Version

15.3.2

Laravel Version

10.48.9

PHP Version

8.2

Database Driver & Version

No response

Description

It looks like when a subscription is created via Stripe Checkout, and a future payment (on subscription renewal) requires confirmation, the notification is never triggered, because of the is_on_session_checkout check here.

This check was added in this PR to prevent confirmation notifications being sent while the customer is in the checkout session, which makes sense, because the initial payment is not off-session: the customer can already see that they need to confirm the payment when checking out.

However, it seems to me that this flag should be cleared from subscription metadata once the initial payment is successful, so that future renewals can still trigger this notification.

Steps To Reproduce

  1. Create a subscription via checkout, ie
$user->newSubscription('default', 'price_1H9g3eLzK4Z2r3a4')
    ->checkout([
        'success_url' => route('billing'),
        'cancel_url' => route('billing'),
    ]);
  1. Complete the checkout with a valid test card that allows a successful payment
  2. In Stripe Dashboard, update the subscription to use the "Always authenticate" 3DS card (4000002760003184)
  3. Use a test clock on the subscription, and advance the clock to a date after the next renewal
  4. Stripe sends the webhooks as expected
  5. Cashier receives the webhook, but skips processing it, because of this check.
driesvints commented 3 months ago

Hi @ragulka. Thank you for flagging this. If that's the case then yes we should fix this. I guess it's the invoice.payment_succeeded we need to check for this? We should be able to derive the subscribe for it and clear this metadata key on the subscription.

github-actions[bot] commented 3 months ago

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

ragulka commented 3 months ago

I guess it's the invoice.payment_succeeded we need to check for this? We should be able to derive the subscribe for it and clear this metadata key on the subscription.

Correct - invoice.payment_succeeded seems to be the webhook to handle. I can try and put together a PR for this?

driesvints commented 3 months ago

Yes please! ❤️

driesvints commented 1 month ago

@ragulka are you still up for sending in that PR?

driesvints commented 3 weeks ago

Since we only had one ticket about this and there's no activity here I'm gonna close this one.

ragulka commented 3 weeks ago

@driesvints sorry - somehow I did not see the notifications from this thread. I'd be up to submit a PR, I've just been rather tight with time.

driesvints commented 3 weeks ago

@ragulka no worries. Would still very much appreciate a PR when you can spare the time.