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

Users with Past Due Subscriptions Marked as Not Active #768

Closed aaronhuisinga closed 4 years ago

aaronhuisinga commented 5 years ago

Description:

In Cashier v9, customers were returned as active when they had a subscription that was not actually canceled (determined by the ends_at field). With the addition of stripe_status in Cashier v10, the active method now takes the status into account. I believe that checking for incomplete, incomplete_expired, and unpaid is beneficial, but am unsure of why customers with a past_due status are not returned as active.

Typically this happens when a customer has a payment fail, and Stripe will then automatically retry several times over the next few days to get the payment to succeed. With this new active method logic, customers are being locked out of our application immediately rather than giving them time to update their payment source if necessary, or just allowing them to free up credit on a card.

For now we're needing to add a notCanceled() and scopeWhereNotCanceled() method to make sure customers are allowed access until their subscription is actually canceled, as it doesn't seem there is a simple way to query active customers that are past due on their subscription.

Is there:

  1. A simple way to do this that our team has missed somehow?
  2. A method that could be added to cover this usage, as I'm sure we're far from the only ones that were caught off guard by this.
  3. A modification that could be made to the active methods to remove the past_due check, as to me I'd still define them as active.
ipalaus commented 5 years ago

This is related to #766

driesvints commented 5 years ago

This is by design. When a subscription is past_due you haven't received payment and thus your client is using your app without having paid for it. We mark the user as inactive until they've paid their invoice. This is the way we handle it for Forge, Envoyer and Vapor.

I believe this is just something where opinions differ. I get your own point of view but there's two ways to look at this. Imo you should always allow your user to be able to update their billing details. If you disallow a user from accessing the app when they're inactive then you should update it so they can at least reach their billing settings.

I'm happy to discuss this further to see if we can make this more dynamic so both situations can be supported.

ipalaus commented 5 years ago

@driesvints I understand your point that is by design and everyone will implement it differently but this wasn't how Cashier v9 worked. I guess that's why Aaron is having this issue. There was no status before, so all you would get is a cancellation after X days if you configured Stripe to retry the payments Y times.

I wonder if this should be clarified somewhere?

driesvints commented 5 years ago

@ipalaus unfortunately the new status needed to be introduced to properly handle SCA and the new Payment Intents from Stripe. Like I said, I'm willing to look into to making this configurable but not sure how that can best be done. I don't have too much time atm to look into this so appreciating any help here.

ipalaus commented 5 years ago

It works for my use-case tho, so I'm happy with it. I actually think it's a better user experience as now I know when it's pending to be paid (and not canceled yet) so I show a different notification.

aaronhuisinga commented 5 years ago

@driesvints I definitely understand it's a difference in our opinions, and also understand where you're coming from with how Forge and other Laravel applications are structured. The way we run is based on how it was implemented in Cashier v9, and although it would be a simple few code changes to support the new definition, the problem for us lies in the business logic with what we've taught our existing customers to expect.

Is there a way that we could get a method added or something that would cover this use case? Right now we've switched calls from active() to notCancelled()->orWhere->onGracePeriod(), but this isn't perfect either due to it not taking other statuses into account. We just decided not to throw a bunch more time into this until we knew how this PR would be resolved.

driesvints commented 5 years ago

I'll need some more time to think this over on what the best solution is. I can't promise anything right now in terms of when that'll be but I'll try to look into this soon.

skollro commented 5 years ago

Any updates on this? Just had the same issue...

driesvints commented 4 years ago

No sorry, still haven't found the time.

driesvints commented 4 years ago

Currently looking into this and this can be done pretty easily I think. Trying to think of a good config value name though. Currently thinking about deactivate_past_due which is set to true by default for BC reasons. Thoughts?

aaronhuisinga commented 4 years ago

That makes sense to me. Great middle ground option!