Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.2] Fix automatic scope nesting #13413

Merged
merged 1 commit into from
May 3, 2016
Merged

[5.2] Fix automatic scope nesting #13413

merged 1 commit into from
May 3, 2016

Conversation

acasar
Copy link
Contributor

@acasar acasar commented May 3, 2016

When applying multiple global scopes, we need to nest them even if no other where clauses were added to the query. I loosened the shouldNestWheresForScope condition to allow for that.

Bug was reported here: https://laracasts.com/discuss/channels/general-discussion/define-multiple-global-scope-for-a-model-in-laravel

@taylorotwell taylorotwell merged commit d9cc1bd into laravel:5.2 May 3, 2016
zither added a commit to zither/framework that referenced this pull request May 5, 2016
@JosephSilber
Copy link
Member

JosephSilber commented Nov 8, 2016

This actually broke my code, and it took me quite a while to track down 😢

I have a query scope that I want to sometimes apply with an or clause, so I was passing it in as an argument:

public function scopeOnDay($query, $column, Carbon $date, $boolean = 'and')
{
    $method = $boolean == 'and' ? 'where' : 'orWhere';

    $date = $date->startOfDay()->setTimezone('UTC');

    $query->{$method}(function ($query) use ($column, $date) {
        $query->where($column, '>=', $date->toDateTimeString());
        $query->where($column, '<', $date->addDays(1)->toDateTimeString());
    });
}

And here's how I was using it:

$query->where(function ($query) use ($date) {
    $query->onDay('created_at', $date)
          ->onDay('charged_at', $date, 'or');
});

But since all scopes are now wrapped, there's no way anymore to apply a scope with an or.

Am I missing something?

@acasar
Copy link
Contributor Author

acasar commented Nov 8, 2016

@JosephSilber yes, this was a breaking change in 5.2. In 5.3 we went back to allow scopes starting with OR boolean. See here: #12918

@JosephSilber
Copy link
Member

Oh, I didn't see that. Nice.

This project is still on 5.2, so I just moved onDay to a query builder macro.

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.

3 participants