-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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.8] Add whereHasMorph() #28928
[5.8] Add whereHasMorph() #28928
Conversation
Add test to check model scopes are accessible
Is there a reason we can't do a morphMap lookup to allow always passing classes? Seems a bit flawed to have to change things in multiple places, unless I'm missing something. Should just be a matter of |
I might be miss understanding what you mean, but as it stands you can pass:
i.e. all of the following work... Relation::morphMap(['posts' => Post::class, 'videos' => Video::class]);
// only classes
Comment::whereHasMorph('commentable', [Post::class, Video::class], function ($query) {
//
})->etc(...);
// only morph values
Comment::whereHasMorph('commentable', ['posts', 'videos'], function ($query) {
//
})->etc(...);
// a mixture of both
Comment::whereHasMorph('commentable', ['posts', Video::class], function ($query) {
//
})->etc(...); We have to create an on-the-fly relationship - hence passing the Here is where the morph map lookup is happening: framework/src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php Lines 233 to 246 in 8fe3f1f
@phroggyy let us know if that addresses your concern or if I'm way off base with what you mean 🙂 Another nice thing about this is that you have access to the scopes on the models as well. So if Comment::whereHasMorph('commentable', [Post::class, Video::class], function ($query) {
$query->wherePublished();
})->etc(...); |
Ah, gotcha. In that case, the phrasing of the PR is wrong:
As per your explanation, this is not true. The user may pass a class name or morph value, but whether the morph map is used doesn't matter. |
🤦♂️ Sorry, I actually misspoke. What @staudenmeir said is how this implementation currently works. |
I've just created a PR to @staudenmeir’s branch that does the reverse lookup and allows classes even when a morph value has been specified - which matches what I said in my earlier comment. But we might just need to consider if there are any implications to this before merging. Thoughts? |
I don't think it's a good idea to allow both class names and aliases. What if users only provide class names and Laravel handles the aliases? This way, users wouldn't have to adjust the queries when they add a |
I don't personally use a morph map, so not sure how valuable my opinion in on this specific part, but that sounds like a good way to go about it. Either way, I've just created a PR for this behaviour as well. |
Relation::morphMap(['posts' => Post::class]);
Comment::whereHasMorph('commentable', [Post::class, Video::class], function ($query) {
$query->where('title', 'foo');
})->get(); |
Nice one! |
I just opened this issue #29138 related to this PR. Not sure if I'm doing something wrong but can't seem to get the |
With this improvement can you do nested relationship on morphed classes?
In the example above. I'm trying to look for Comments from Posts and Videos that has an author. |
This is a great addition, but I can't get it to work with a |
@mikkopursuittechnology Yes, you can. @pelmered |
@staudenmeir Ok, but And with Conclusion: we need a |
@pelmered How is your relationship defined? This exception is only thrown for |
@staudenmeir Yes, you where right. Where was an error in the inverse of the relationship. Sorry about that. |
and how to implement the inverse of the relationship? |
Hello all, |
Thx a lot, I already saw the laravel documentation and followed it but I don't know why it's not working to me. |
Due to the nature of polymorphism,
whereHas()
doesn't work withMorphTo
relationships (#5429, #18523).As a solution, @timacdonald and I are proposing
whereHasMorph()
and the corresponding methods:By creating a temporary
BelongsTo
relationship for each type and allowing relationship instances as the first argument ofhas()
, we can reuse most of the existing code.If the user has registered custom aliases in themorphMap
, they need to provide them instead of class names:To simplify queries with different constraints, we pass the type as the second parameter:
You can also provide a wildcard and let Laravel get the possible types from the database:
The downside here is the additional query that should be mentioned in the docs.
Theoretically, you could use
*
as an alias, but we think it's a better choice than something likenull
.