Is this usage of query scopes correct?

I have a model with three visibilities;

public (for everyone, regardless of authentication status)
private (only for the creator)
auth (for every authenticated user)

To simplify this logic, I figured I'd use query scopes, however I've never used them before so I'm wondering if I'm using them correctly.

My model's code (irrelevant bits omitted)

class MyModel extends Model
{
    public const VISIBILITY_PUBLIC = 1;
    public const VISIBILITY_PRIVATE = 2;
    public const VISIBILITY_AUTH = 3;

    public static function visible(): Collection
    {
        return self::query()
            ->private()->orWhere
            ->public()->orWhere
            ->auth()
            ->orderBy('name')
            ->get();
    }

    public function scopePrivate(Builder $query): Builder
    {
        return auth()->check() ? $query->where('user_id', auth()->user()->id) : $query;
    }

    public function scopePublic(Builder $query): Builder
    {
        return $query->where('visibility', self::VISIBILITY_PUBLIC);
    }

    public function scopeAuth(Builder $query): Builder
    {
        return auth()->check() ? $query->where('visibility', self::VISIBILITY_AUTH) : $query;
    }
}

In a controller I can then simply call MyModel::visible(); to get all models the specific user can see.

My question is, is this a valid way of using query scopes? My main concern is scopePrivate since I'm not actively checking for self::VISIBILITY_PRIVATE, only records where user_id is the currently authenticated user's id. My tests pass, but that might as well indicate an error in my tests.

Snapey

4 years ago

Best Answer

Level 122

Why not have one function as a scope so that you can chain other methods like paginate or latest()

public function scopeVisible(Builder $query)
{
	if auth()->check()) {
        return $query->where(function(Builder $query){
			$query->where('visibility', self::VISIBILITY_PRIVATE)
				->orWhere('user_id', auth()->user()->id);
		});
    }
	return $query->where('visibility', self::VISIBILITY_PUBLIC);
}

now you can use MyModel::visible()->get()

lolsokje

4 years ago

Level 17

@Snapey I kept them separated initially in case I, at some point, want to use only one specific scope. After some thought though, there's not a single use case for this I can think of, so might as well refactor to your suggestion. Thanks!

lolsokje

4 years ago

Level 17

@Snapey I did make some small changes to get it to work btw;

public function scopeVisible(Builder $query): Builder
{
    if (auth()->check()) {
        $query->where(function (Builder $query) {
            $query->where('visibility', self::VISIBILITY_AUTH)
                ->orWhere('user_id', auth()->user()->id);
        });
    }
    return $query->orWhere('visibility', self::VISIBILITY_PUBLIC);
}

I removed the return from the if (auth()->check()) block since the visibility self::VISIBILITY_PUBLIC always needs to be included
I changed the self::VISIBILITY_PRIVATE to self::VISIBILITY_AUTH since the orWhere clause already takes care of the self::VISIBILITY_PRIVATE case.

Please or to participate in this conversation.