Skip to content

Conversation

@lupinitylabs
Copy link
Contributor

@lupinitylabs lupinitylabs commented Apr 8, 2021

This pull requests proposes a possibility to assign a name to a global scope defined by a scope class.

Right now, it is possible to define a global scope in three different ways:

  1. static::addGlobalScope('nameOfScope', function ($builder) { $builder->where('someColumn', '>', 1); });
  2. static::addGlobalScope(function ($builder) { $builder->where('someColumn', '>', 1); });
  3. static::addGlocalScope(new MyCustomScope());

The first would be named nameOfScope, the second something like 0000000017da6fe1000000003ed5b2bc and the third will go by the fully qualified class name of MyCustomScope.

In cases where MyCustomScope actually takes parameters, you are therefore not able to apply multiple scopes like:

static::addGlocalScope(new OrderByScope('name', 'desc'));
static::addGlocalScope(new OrderByScope('birth_date'));

because the scope ordering by birth_date would overwrite the one previously assigned that ordered by name.

Of course it would be possible to call the Scope like static::addGlocalScope(new OrderByScope(['name', 'desc'], 'birth_date')); and chain the orderBys within the Scope, but then you would not be able to disable one of them selectively in a later query.

This PR suggests a change that, additionally to the existing invocations, allows to define a global scope on a Model like this:

static::addGlocalScope('orderByName', new OrderByScope('name', 'desc'));
static::addGlocalScope('orderByBirthDate', new OrderByScope('birth_date'));

The scopes are distinctively named orderByName and orderByBirthDate, respectively, which allows for multiple class scopes to be stacked on a model with different parameters and of course to dynamically disable each one of them.

@lupinitylabs
Copy link
Contributor Author

The PHP setup in the Windows tests seems to be broken...? This is not an issue with my PR, is it?

@driesvints
Copy link
Member

We can't change the method signature on a stable release. Please send to master.

@driesvints driesvints closed this Apr 8, 2021
@driesvints
Copy link
Member

The Windows test failing is indeed unrelated.

@lupinitylabs
Copy link
Contributor Author

We can't change the method signature on a stable release. Please send to master.

Makes sense, will do... thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants