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

[9.x] Fix base query for relationships #41918

Merged
merged 2 commits into from
Apr 11, 2022
Merged

[9.x] Fix base query for relationships #41918

merged 2 commits into from
Apr 11, 2022

Conversation

driesvints
Copy link
Member

This PR fixes an issue where scopes (like soft deletes) weren't applied when the base query of a relationship was retrieved. Previously, in Laravel v8, this was still applied properly.

Fixes #41885

@driesvints driesvints added the bug label Apr 11, 2022
@taylorotwell
Copy link
Member

We need to dig into why this method was changed for Laravel 9.x - there was likely a reason.

@taylorotwell taylorotwell merged commit 77755e9 into 9.x Apr 11, 2022
@taylorotwell taylorotwell deleted the fix-scopes branch April 11, 2022 14:21
@tpetry
Copy link
Contributor

tpetry commented Apr 11, 2022

The toBase method was not implemented in Relation for Laravel 8. As of the query builder's mixin and forwarding principle the method call was forwarded by __call() to Eloquent\Builder::toBase() which is executing $this->applyScopes()->getQuery(); - the same as this PR.

So, my guess is that this is not an intended change but simply an oversight in the implementation because with the query builder interface the toBase method needed to be implemented.

@tpetry
Copy link
Contributor

tpetry commented Apr 11, 2022

@taylorotwell @driesvints In my opinion is the now merged fix is adding a new inconsistency.

In Laravel v8 the implementation for getBaseQuery was:

public function getBaseQuery()
{
    return $this->query->getQuery();
}

With the new fix it's now the following which is different (it's now applying the scope!):

public function getBaseQuery()
{
    return $this->toBase();
}

public function toBase()
{
    return $this->query->applyScopes()->getQuery();
}

@driesvints
Copy link
Member Author

@tpetry ah right, can you send in a PR for that?

@tpetry
Copy link
Contributor

tpetry commented Apr 11, 2022

Give me some minutes 😉

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

Successfully merging this pull request may close these issues.

Calling toBase() on relationships removes soft-deleted condition in Laravel 9 compared to Laravel 8
3 participants