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

[16.x] Store subscription renewal date #1675

Closed
wants to merge 2 commits into from

Conversation

clementmas
Copy link
Contributor

@clementmas clementmas commented May 3, 2024

I believe there's a real need from the community to store the subscription renewal date to display it to users or use for cron jobs. See: #361, #515, #874, #1054, #1102, Stack Overflow, Laracasts, gist, etc.

The current workaround requires making an extra API request $user->subscription()->asStripeSubscription()->current_period_end that can be avoided if we store the renewal date $subscription->renews_at when creating, updating and swapping subscriptions.

Note: I considered using the ends_at column instead of creating a new renews_at column but they'll have different values when using $subscription->cancelAt().

I understand the need to keep Cashier simple and functional but I think this is a relatively simple change that adds a lot of value.

This PR does not break any existing feature but it does require running this database migration to upgrade:

// php artisan make:migration add_cashier_subscriptions_renews_at_column --table=subscriptions
Schema::table('subscriptions', function (Blueprint $table) {
    $table->timestamp('renews_at')->nullable()->after('quantity');
});

src/Subscription.php Outdated Show resolved Hide resolved
@driesvints
Copy link
Member

Tbh, I'd also like to see this in Cashier now.. thanks for the PR! The only downside here is that this is gonna need a new major release because current apps won't have the renews_at column.

@driesvints driesvints changed the title Store subscription renewal date [16.x] Store subscription renewal date May 3, 2024
@taylorotwell taylorotwell marked this pull request as draft May 3, 2024 17:23
@sts-ryan-holton
Copy link

Is it better to start using dateTime instead of timestamp nowadays? Especially with the future timestamp issue approaching

@clementmas clementmas marked this pull request as ready for review May 6, 2024 04:38
@@ -163,11 +164,14 @@ protected function handleCustomerSubscriptionUpdated(array $payload)
}
}

// Renewal date...
$subscription->renews_at = Carbon::createFromTimestamp($data['current_period_end']);
Copy link
Member

Choose a reason for hiding this comment

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

Is this webhook endpoint invoked on every billing cycle's successful payment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The customer.subscription.updated webhook is triggered when the billing period ends, usually an hour before the renewal payment is attempted.

It is triggered again if the payment fails to set the subscription status to "past_due".

So it'll keep Cashier's renews_at column up to date with Stripe subscriptions' billing periods.

However, the one hour delay that might cause confusion. A user could see a future renewal date thinking the payment went through when it could actually fail a few minutes later.
A workaround would be to update renews_at inside another webhook instead (probably invoice.payment_succeeded).

These are the webhook events I recorded

Successful subscription renewal:

  • invoice.created
  • customer.subscription.updated (new current_period_start|end are set)
  • [~ 1 hour delay]
  • payment_intent.created
  • payment_intent.succeeded
  • customer.updated
  • invoice.updated (status is "paid")
  • invoice.paid
  • invoice.payment_succeeded
  • invoice.finalized

Failed subscription renewal:

  • invoice.created
  • customer.subscription.updated (new current_period_start|end are set)
  • [~ 1 hour delay]
  • payment_intent.created
  • payment_intent.payment_failed
  • customer.updated
  • invoice.updated (status is "open")
  • customer.subscription.updated (set subscription status to "past_due")
  • invoice.payment_failed
  • invoice.updated
  • invoice.finalized

Successful subscription swap:

  • payment_intent.created
  • payment_intent.succeeded
  • invoiceitem.created (proration adjustment)
  • invoice.created
  • invoice.paid
  • invoice.payment_succeeded
  • invoice.finalized
  • customer.subscription.updated

Successful subscription creation:

  • payment_intent.created
  • invoice.created
  • invoice.finalized (status is "open")
  • customer.subscription.created
  • payment_method.attached
  • payment_intent.succeeded
  • invoice.updated (status is "paid")
  • invoice.paid
  • invoice.payment_succeeded

Copy link
Member

Choose a reason for hiding this comment

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

@clementmas I don't understand sorry. From your examples above, customer.subscription.updated is always triggered on time with the new current_period_start|end set? I don't see why we should listen to a different webhook? Subscription created is also covered with its own event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The webhooks for new and swapped subscriptions are triggered on time but there's a delay for renewed subscriptions.

The customer.subscription.updated webhook is triggered an hour before the payment to renew the subscription is attempted.

See webhook events with timestamps

Apr 1, 2024, 1:15:06 PM: invoice.upcoming
Apr 7, 2024, 1:14:27 PM: invoice.created
Apr 7, 2024, 1:14:27 PM: customer.subscription.updated
Apr 7, 2024, 2:15:18 PM: payment_intent.created
Apr 7, 2024, 1:15:19 PM: payment_intent.succeeded
...

So during that hour, the renewal date is technically wrong. It should display "today"'s date instead of next month/year.

But if that's not a concern, then yes, it's much simpler to keep relying on the customer.subscription.created and customer.subscription.updated webhooks.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't worry about that. I'd always rely on the customer.subscription.updated event.

Copy link
Member

Choose a reason for hiding this comment

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

@ybert I now wonder if billing_cycle_anchor is more what we need? https://docs.stripe.com/billing/subscriptions/billing-cycle

Although I don't know what happens for metered billing.

Copy link
Contributor

@ybert ybert May 23, 2024

Choose a reason for hiding this comment

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

@driesvints I just tested it, and in my use cases, current_period_end and billing_cycle_anchor are the same after cancellation.
I use the code pasted above in production for a while and it seems to work.

Copy link
Member

Choose a reason for hiding this comment

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

@taylorotwell but what Stripe shows IS the accurate date at the time the event is shown. We have no way of knowing when to change the value if we delay it until the payment has been done as that's a complex process with lots of unknown and side effects that could happen. The fact that the renewal date changes when a payment fails is such a side effect. We should simple focus on reflecting what Stripe tells us. current_period_end will always be the correct indication when a subscription renews.

@ybert yeah I agree with the above code, feel free to adopt that in your PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@driesvints we can ignore the billing period inside the customer.subscription.updated webhook and solely update the renewal date when receiving the invoice.payment_succeeded webhook.

That's the only way to really know when a subscription has been renewed.

The invoice.payment_succeeded webhook is triggered when swapping subscriptions, including when downgrading and the amount due is 0 (I just checked).
It's also triggered for metered billing renewals and when switching from a trial to an active subscription.

It would indeed be easier to just rely on Stripe's billing cycle but delaying the update to another webhook is not very complex. We do need to check for side effects though so I'll update the code I already have running in production to test this scenario.

Thanks @ybert for your catching the cancellation issue.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @clementmas. I just feel there's gonna be quite a bit of side effects from relying on the payment succeeded webhook. When you're ready feel free to try this but I'd suggest to create a new draft PR so we can inspect the differences.

@taylorotwell taylorotwell marked this pull request as draft May 10, 2024 20:08
@driesvints driesvints marked this pull request as ready for review May 17, 2024 13:22
@driesvints driesvints marked this pull request as draft May 21, 2024 12:53
@driesvints driesvints changed the base branch from 15.x to 16.x June 27, 2024 08:24
@driesvints
Copy link
Member

@clementmas I'm sorry but given there's no consensus on how to approach this I believe it's best to let this one go for now. Thank you for your time and work.

@driesvints driesvints closed this Jul 2, 2024
@clementmas
Copy link
Contributor Author

I haven't found any side effects in production from relying on the invoice.payment_succeeded webhook instead of customer.subscription.updated. The renewal date is updated properly and on time.

I updated the webhook code on my fork: clementmas@c412462

I invite other developers to try it, improve it if necessary, and hopefully we can PR it again.

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.

5 participants