Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

[Proposal] Local query scopes as classes #636

Closed
jaripekkala opened this issue Jun 13, 2017 · 11 comments
Closed

[Proposal] Local query scopes as classes #636

jaripekkala opened this issue Jun 13, 2017 · 11 comments

Comments

@jaripekkala
Copy link

https://laravel.com/docs/5.4/eloquent#query-scopes

Currently

class User extends Model
{
    public function scopeActive(Builder $query)
    {
        return $query->where('active', 1);
    }
}
$users = User::active()->get();

Proposed

class ActiveScope implements Scope
{
    public function apply(Builder $builder, Model $model)
    {
        $builder->where('active', 1);
    }
}
$users = User::scoped(ActiveScope::class)->get();
@jaripekkala jaripekkala changed the title [Proposal] Local scopes as classes [Proposal] Local query scopes as classes Jun 13, 2017
@Dylan-DPC-zz
Copy link

What's the gain?

@jaripekkala
Copy link
Author

Just an alternative way to write query scopes.

Keeps the models clean.

Also nice when sharing the same scope between models. So far I have just written a scope trait or just duplicated the code.

Maybe easier to break the scope logic into smaller pieces.

class ActiveScope implements Scope
{
    public function apply(Builder $builder, Model $model)
    {
        $this->verified($builder);
        $this->emailed($builder);
        $this->notOld($builder);

        // ...
    }
}

@Dylan-DPC-zz
Copy link

ya but other than apply I don't see any other method you will have in the class. Also if you want common functionality across models you could use a trait

@ajcastro
Copy link

ajcastro commented Mar 15, 2019

The current disadvantage of the current local scope is that in IDE like in sublime, when you f12 or find symbol for the method, it could not really find it because its method is actually scopeActive and not active.
This adds additional overhead when debugging or tracing code.

@mfn
Copy link

mfn commented Mar 15, 2019

This issue is years old and the problem you described isn't specific to Sublime, any IDE has this issue.

Laravel is simple full of wonderful magic and some if it you can't escape if you want to use certain patterns.

I'd also say any project seriously using laravel should use https://github.com/barryvdh/laravel-ide-helper which, AFAIK, support scopes by creating phpdoc on the classes to allow at least autocomplete in modern IDEs.

@jaripekkala
Copy link
Author

I still think this would be a cool feature.

However I do see the point of not adding every single cool feature to the core.

@alies-dev
Copy link

alies-dev commented Mar 15, 2019

A semi hacky way to use Scope classes for local queries:

public static function getNumberOfAdults(): int
{
    return self::query()
        ->tap(function (Builder $query) {
            (new AgeScope(18))->apply($query, $query->getModel());
        })
        ->count();
}

As you can see there are to ugly parts:

  1. Useless $query->getModel()
  2. Extra Lambda/Closure.

ideally it should looks like:

public static function getNumberOfAdults(): int
{
    return self::query()
        ->scoped(new AgeScope(18))
        ->count();
}

or for scopes without arguments:

public static function getNumberOfAdults(): int
{
    return self::query()
        ->scoped(AgeScope::class)
        ->count();
}

@ajcastro
Copy link

ajcastro commented Mar 16, 2019

@lptn I think that will not work if your queries have other chained where conditions because afaik the static::query() instantiates a new query builder instance.. that will only work if that is the first method that you will call in the method chain..

public static function adults() 
{
  return self::query()
        ->scope(new AgeScope(18));
}

User::adults()->where('is_active', 1)->get(); // this will work 
User::where('is_active', 1)->adults()->get(); // will not work

Remember that query scopes design should be chainable anywhere in the method chain.

My approach is something like this:

class User
{
    public function newEloquentBuilder()
    {
        return new UserQueryBuilder;
    }    

}

class UserQueryBuilder extends \Illuminate\Database\Eloquent\Builder
{
    public function adults()
    {
        $this->query->where('age', '>=', 18); 

        return $this;
    }

    public function active()
    {
        $this->scope(new ActiveScope); // Reusable object query scopes if you want reusable queries

        return $this;
    }
}

class ActiveScope 
{
    public function apply($query) // maybe we can also use __invoke()
    {
        $query->where('is_active', 1);
    }
}

// All query scopes will work the usual way, anywhere in the method chain
User::active()->adults()->where(...)->orderBy(...)->get(); 
User::where(...)->where(...)->active()->adults()->get(); 

The find symbol will work now because we have exact methods names, for easier code tracing.

@mpyw
Copy link

mpyw commented May 3, 2019

The only disadvantage of this feature is that users must know what scopes are applicable to the model. It's very unclear unlike to the trait approach; in the trait case users don't have to know but implementers have to.

@mpyw
Copy link

mpyw commented May 3, 2019

// The macro should be called only in model definitions

Builder::macro('scoped', function ($scope, ...$parameters): Builder {
    /** @var Builder $query */
    $query = $this;

    if (is_string($scope)) {
        $scope = new $scope(...$parameters);
    }
    if (!$scope instanceof Scope) {
        throw new LogicException('$scope must be an instance of Scope');
    }
    $scope->apply($query, $query->getModel());
    
    return $query;
});
// Class scope should be re-defined as a method scope

class User extends Model
{
    public function scopeActive(Builder $query): Builder
    {
        return $query->scoped(ActiveScope::class);
    }
}

@mpyw
Copy link

mpyw commented May 4, 2019

I've published as a package:
mpyw/laravel-local-class-scope: A tiny macro that reuse a global scope class as a local scope

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants