[7.x] Add hasScope function to the Base Model

[alexbowers]

This hasScope method cleans up checking if a scope exists in a dynamic way.

The use case for this is requests containing an array of filters to be applied (via scopes), the code is more expressive with:

public function index(Request $request)
    {
        $post = Post::query();

        foreach ($request->get('filters', []) as $filter) {
            if ($post->hasScope($filter)) {
                $post->{$filter}();
            }
        }

        return $post->paginate();
    }

[alexbowers]

Closes laravel/ideas#2192

[alexbowers]

The test that is failing seems to have no bearing on the code i've written. It also does not fail for me locally, any ideas?

[driesvints]

@alexbowers tests on 7.x were temporarily failing but are fixed. Can you rebase?

[alexbowers]

@driesvints Done

[koenhoeijmakers]

I don't really see the use case for a hasScope method but i'm not against it because there probably are reasons for it.

But i don't think your use case is correct and its even dangerous, letting the client decide which scopes are sent via your filters input opens up issues where somebody can register any scope.

For example: a scope scopeAdmin that allows the you to see every single Post instead of only those who are visible. Sending filters[admin] now opens up every post.

[alexbowers]

@koenhoeijmakers That can easily be controlled with a whitelist / blacklist. My use case is not an open API, but an internal one, which will make it cleaner than having a switch statement for my scopes.

Also, your access to data should already be restricted as a user as part of a Policy / default scope being applied, all that applying any additional scopes would do is further restrict the data.

For example,

If you are User 1 and you are trying to get all your posts there would by default on the controller be a restriction:

$post = Post::byAuthor(auth()->id);

or similar which would be restricting the posts to the current user, adding a filters[admin] to that would still have your original scope applied.

[n123456789k]

Create a macro.

[GrahamCampbell]

Create a macro.

I don't agree here. To me, the main benefit of this PR seems to be separation of concerns. Models now decide if they have a scope, rather than reflection by an outsider.

[GrahamCampbell]

Kinda odd that hasScope expects the scope name and not the method name, but callScope expects the actual method name. (don't change anything yet please. I am writing this comment to be discussed)

[alexbowers]

I did find that weird, but wanted to change the minimum amount possible, and since changing how callScope works would be considered a breaking change, wanted to delegate that to a future PR (if desired).

[alexbowers]

Thanks for the merge Taylor,

Any thoughts on Grahams Comment #32622 (comment) ?

[GrahamCampbell]

I think we should change it in Laravel 8. I'll send a PR.

[driesvints]

This hasn't been released yet so you can pr to 7.x

[alexbowers]

@driesvints The change Graham is on about is already released, its changing the method signature of callScope to not be callable and to to build the callable instance itself.

[GrahamCampbell]

Actually, I am thinking we can do this non-breaking on 7.x, having looked into the code. We will need another method. Not possible to change the old method because a scope could have the same name as a global function which is callable.,

[GrahamCampbell]

#32631