laravel / cashier-stripe

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

subscribed method fails if billable has more than one subscription #1140

Closed martinbean closed 3 years ago

martinbean commented 3 years ago

Description:

If a billable entity has more than one subscription (i.e. they were subscribed, subscription expires, they subscribe at a later date creating a new subscription) then the subscribed method fails.

Digging into the source code, it’s because the subscribed method calls upon the subscription method, which just naïvely fetches the first subscription with that name. In my case, the first subscription is the expired one.

This method should be changed to instead order subscriptions by newest first, so that the subscription (and in turn, subscribed method) are acting on the latest subscription for a billable entity; not the oldest.

Steps To Reproduce:

  1. Create an expired subscription for a billable entity.
  2. Create an active subscription for a billable entity.
  3. Call subscribed on the billable entity. Method should return true but in fact returns false.
driesvints commented 3 years ago

Hey @martinbean

the subscription method, which just naïvely fetches the first subscription with that name.

This isn't exactly true. It fetches the latest created subscription of the given name which is the only considered by Cashier to be active. You should never have multiple active subscriptions under the same name. See this note in the docs:

Screenshot 2021-05-10 at 13 58 35

In my case, the first subscription is the expired one.

This shouldn't be possible. The subscriptions are ordered by their creation date here: https://github.com/laravel/cashier-stripe/blob/f1a7d5ffce278bf9c85cd3c1b86c45d688f3bf00/src/Concerns/ManagesSubscriptions.php#L106

Are you overwriting this method and changing that behavior? Can you check the created_at timestamps in the database to see which one was created first?

martinbean commented 3 years ago

@driesvints That’s not the code I’m seeing the latest version of Cashier: https://github.com/laravel/cashier-stripe/blob/12.x/src/Concerns/ManagesSubscriptions.php#L77-L97

If I call subscribed, it goes into that subscription method, which you can see you calls the first subscription matching the name with no ordering (line 96):

return $this->subscriptions->where('name', $name)->first();

This is causing an issue as the customer’s first subscription has ended, but their second one is active. However, Cashier is just checking the very first one.

driesvints commented 3 years ago

$this->subscriptions loads the eloquent models into a collection and right after the where condition is applied to that collection. Check the method right below the part you linked. That's the eloquent subscriptions relation with the created_at orderBy.

This is causing an issue as the customer’s first subscription has ended, but their second one is active. However, Cashier is just checking the very first one.

This should really not be happening based on the current state of the code. The opposite should happen. Can you please check the created_at timestamps in the database like I asked? Thanks

driesvints commented 3 years ago

Closing this issue because it's inactive, already solved, old or not relevant anymore. Feel free to reply if you're still experiencing this issue and we'll re-open this issue.