laravel / cashier-stripe

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

Subscription scopes doesn't work #940

Closed joke2k closed 4 years ago

joke2k commented 4 years ago

Description:

Hi, I found a bug using Subscription's scopes. All scopes are overwritten by scope-named method (active overwrite scopeActive.. etc.) Other users experienced similar issues: https://laracasts.com/discuss/channels/general-discussion/error-when-calling-scope-deprecated-non-static-method?page=0

Steps To Reproduce:

\Laravel\Cashier\Subscription::active()->count()

results:

ErrorException
Non-static method Laravel\Cashier\Subscription::active() should not be called statically

I tried also to call \Laravel\Cashier\Subscription::scopeActive()->count() but the result is simliar. It is the same for ALL scope methods.

One suggestion (that brokes back-compatibility, but these methods never worked) is to rename all scope-named method with a is prefix. (eg. isActive isIncomplete).

I found a tricky fix here https://laravel.io/forum/non-static-method-should-not-be-called-statically-for-scopes

driesvints commented 4 years ago
$users = User::whereHas('subscriptions', function ($query) {
    $query->whereName('gold')->active();
})->get();

https://github.com/laravel/cashier/pull/609

I'll try to document these when I get back from vacation.

joke2k commented 4 years ago

The bug remains. These scopes are not usable directly with the Subscription model.

driesvints commented 4 years ago

@joke2k did you even read the example I posted above? There's no bug here.

joke2k commented 4 years ago

@driesvints yes, I read it and thank you. But in my opinion, the bug exists and it is with wrong naming of scope and the scope-named methods.

https://laravel.com/docs/7.x/eloquent#local-scopes

If I read that Subscription has a scopeActive I'm expecting to use it as Subscription::active() receiving an error (like I did), and in Laravel this is a bug. You propose to use whereHas on User model and could work but that is not the same (returning User instead Subscription, not necessary join, need to collect all subscriptions and flatten in order to use them... ).

btw, put a lebel "wontfix" to this issue