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 671 forks source link

IncompletePayment exception doesn't return the subscription record #807

Closed tegola closed 4 years ago

tegola commented 4 years ago

Description:

When creating a subscription I want to continue to fill in custom data even if an IncompletePayment exception is thrown, but I can't because throwing the exception means the subscription record is not returned.

Steps To Reproduce:

try {
    $subscription = $user
        ->newSubscription($subscriptionName, $planId)
        ->withMetadata([
            'user_id' => $user->id,
            'venue_id' => $venue->id
        ])
        ->create($paymentMethod);
} catch (IncompletePayment $e) {
    $confirmPaymentId = $e->payment->id;
}

// The lines below won't work when IncompletePayment is thrown because
// $subscription is null
$subscription->venue_id = $venue->id;
$subscription->photo_upload_limit = 20;

This worked in previous versions, although I skipped 9.3, which is where incomplete payments started to be supported.

driesvints commented 4 years ago

Yeah, this is something I thought about as well when I was working on v10. There's a couple of ways to tackle this.

First, you could derive the subscription from the payment object. You'll have an invoice id on the payment object with which in turn will link to a subscription id which you can retrieve the subscription from.

Secondly, you could overwrite the subscription webhook of Cashier where the subscription gets updated so you can sync the meta data with your db record.

But I do believe we could maybe improve the IncompletePayment exception to also contain the subscription itself. I'll leave this open as an enhancement for now because there's ways around it (as described above).

tegola commented 4 years ago

First, you could derive the subscription from the payment object. You'll have an invoice id on the payment object with which in turn will link to a subscription id which you can retrieve the subscription from.

This means doing two more requests, one for the invoice and one for the subscription, right?

I don't really know Stripe's api to be honest, I'll have a deeper look into it.

But I do believe we could maybe improve the IncompletePayment exception to also contain the subscription itself. I'll leave this open as an enhancement for now because there's ways around it (as described above).

This is what I had hoped for. I'll keep an eye here in case you decide to improve it.

Thanks

tegola commented 4 years ago

Actually it only needs one more request to get the invoice, which holds the subscription id.

Here's my solution, if anybody is stuck on the same issue:

use App\Models\Subscription;
use Laravel\Cashier\Cashier;
use Laravel\Cashier\Exceptions\IncompletePayment;
use Stripe\Invoice as StripeInvoice;

$user = auth()->user();

try {

    // Create the subscription
    $subscription = $user
        ->newSubscription($subscriptionName, $planId)
        ->withMetadata([
            'user_id' => $user->id,
            'venue_id' => $venue->id
        ])
        ->create($paymentMethod);

} catch (IncompletePayment $e) {

    // Since the exception does not return the subscription (but it is actually
    // created), retrieve it from looking up the linked invoice
    $invoice = StripeInvoice::retrieve($e->payment->invoice, Cashier::stripeOptions());
    $subscription = Subscription::where('stripe_id', $invoice->subscription)->firstOrFail();

}

// Add custom data to the newly created subscription
$subscription->venue_id = $venue->id;
$subscription->photo_upload_limit = 20;
$subscription->save()
driesvints commented 4 years ago

I've been looking into this further but unfortunately have come to the conclusion that there's too many unknowns to properly deal with this. For example, a payment intent doesn't always have an invoice and thus it can't always be linked back to a subscription.

If anyone wants to attempt to tackle this in a clean way feel free to send in a PR.