[5.2] Apply constraints to a morphTo relation on eager load

[phroggyy]

Previously, using Model::load or with and supplying eager load constraints would make no difference if the loaded relationship was of the MorphTo type, as eager load constraints were not applied for this class. This PR fixes that.

[acasar]

you should remove this line too

[acasar]

👍

This should also fix #13567

[taylorotwell]

Do we have tests that fail?

[phroggyy]

Nope @taylorotwell. One unit test was removed, but it didn't really add much (I believe all code is still covered through integration tests).

[markvaneijk]

Are you sure this is a good change? This change breaks the eager loading of my Polymorphic relations. It messes everything up for me. Figuring out why this is happening...

[acasar]

@markvaneijk Can you paste your code here?

ping @phroggyy

[phroggyy]

@markvaneijk this should actually fix eager loading of polymorphic relations by allowing constraints, but I might have forgotten about some case, in which case I'll definitely fix it and add a test case. As @acasar said, throw some code in here and we'll do our best. (Would also be great if it's easy-ish to reproduce).

[markvaneijk]

@phroggy @acasar Thanks for your responses. I tried to make a simple example to reproduce my problem.

When you have the Polymorphic relation example in the docs: https://laravel.com/docs/5.2/eloquent-relationships#polymorphic-relations

And you only add this, to always eager load the likeable relationship:

public $with = ['likeable'];

And when I request the Like object:

$like = App\Like::find(1);

Then I get this error:

BadMethodCallException in Builder.php line 2343: Call to undefined method Illuminate\Database\Query\Builder::likeable()

When I remove the $with from the Like model, and use it directly when getting the object, it does work:

$like = App\Like::with('likeable')->find(1);

So the question for me is. Is the use of $with wrong? If not, why does the first example does not work like the second example?

[phroggyy]

That's really weird @markvaneijk. Any chance you'd be able to get a sample project up I can clone down? I'll look into the code right now... Hmm, might have something to do with accessing the QB rather than Eloquent Builder somewhere

[ifox]

@phroggyy, just got the same issue when using a $with property on a model with a morphTo() relationship.

[phroggyy]

@ifox @markvaneijk I'm looking into this right now.

[phroggyy]

@acasar so the issue is we're cloning $this->query, which has the whole eager load constraint on it, even though we don't really want it. From my initial testing, just resetting the eager loads is easiest and works.

[acasar]

@phroggyy Hm.. I think we should actually copy over eager load constraints instead of just resetting them? Because related model can have it's own eager load constraints that we still need to take into account.

[dwightwatson]

This has broken code in one of my projects.

I'm not sure if my use-case is non-supported one, but I have a number of relations set up similarly to this:

public function purchaseable()
{
    return $this->morphTo()->withTrashed();
}

Which was used to fetch a polymorphic relation, but include trashed records. This no longer works, instead I'm getting a BadMethodCallException thrown, with the message: Call to undefined method Illuminate\Database\Query\Builder::withTrashed(). We've had to lock onto Laravel 5.2.33.

What would be the approach to load a polymorphic relation including trashed records?

[acasar]

@dwightwatson Should be fixed on 5.2 dev. Can you check?

[dwightwatson]

Sure - just tried on 5.2.x-dev c9d9748 and this issue is still present.

Edit: I'll put together an example project that demonstrates the problem, and possibly open a new issue for it.

[acasar]

@dwightwatson Thanks, example project or failing test would be nice. We are actually testing for this scenario https://github.com/laravel/framework/blob/5.2/tests/Database/DatabaseEloquentSoftDeletesIntegrationTest.php#L534 So I'd be interested to know the setup in your case.

[acasar]

@dwightwatson I think I found the issue. Here's the PR #13828