[5.4] Adding support to collection on Eloquent::find #19019
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tillkruss
changed the title
|
Just a note that this "breaks" previous behavior. Granted, this would have probably only happened by mistake. But I don't think the new behavior is intuitive either. $user1 = User::find(1);
$user2 = User::find($user1);Old
New
|
|
What would be the point in that snippet of code, @andrewtweber ? |
As I said. My point is that this can change the output of probably the most frequently used method in Eloquent. Just thought people should know. |
sebdesign
added a commit
to sebdesign/framework
that referenced
this pull request
Oct 16, 2019
I found a regression in laravel#7048 and laravel#19019, where the Eloquent Builder will not throw a `ModelNotFoundException` when an `Arrayable` is passed to `findOrFail`. In this laravel#7048, we are checking if the `$ids` is an array and we count the results against the number of ids. But since laravel#19019, the `find` method also accepts an `Arrayable` for the ids. So if an `Arrayable` is passed, the check is skipped and the method returns the results. To fix this, we are first checking if the `$ids` is an `Arrayable` and we convert the ids to an array before checking the results.
sebdesign
added a commit
to sebdesign/framework
that referenced
this pull request
Oct 16, 2019
…ptions The regression in laravel#7048 and laravel#19019 is also observed in laravel#9143, because the `find`, `findMany` and `findOrFail` methods are copied from the Eloquent Builder to the `BelongsToMany` and `HasManyThrough` relations. For this reason, we need to convert the passed ids to an array before executing the queries.
sebdesign
added a commit
to sebdesign/framework
that referenced
this pull request
Oct 16, 2019
The regression in laravel#7048 and laravel#19019 is also observed in laravel#9143, because the `find`, `findMany` and `findOrFail` methods are copied from the Eloquent Builder to the `BelongsToMany` and `HasManyThrough` relations. For this reason, we need to convert the passed ids to an array before executing the queries.
sebdesign
added a commit
to sebdesign/framework
that referenced
this pull request
Oct 16, 2019
The regression in laravel#7048 and laravel#19019 is also observed in laravel#9143, because the `find`, `findMany` and `findOrFail` methods are copied from the Eloquent Builder to the `BelongsToMany` and `HasManyThrough` relations. For this reason, we need to convert the passed ids to an array before executing the queries.
sebdesign
added a commit
to sebdesign/framework
that referenced
this pull request
Oct 16, 2019
…regression in laravel#7048 and laravel#19019 is also observed in laravel#9143, because the `find`, `findMany` and `findOrFail` methods are copied from the Eloquent Builder to the `BelongsToMany` and `HasManyThrough` relations.For this reason, we need to convert the passed ids to an array before executing the queries.
taylorotwell
pushed a commit
that referenced
this pull request
Oct 18, 2019
* Ensure Builder::findOrFail with Arrayable throws ModelNotFoundException I found a regression in #7048 and #19019, where the Eloquent Builder will not throw a `ModelNotFoundException` when an `Arrayable` is passed to `findOrFail`. In this #7048, we are checking if the `$ids` is an array and we count the results against the number of ids. But since #19019, the `find` method also accepts an `Arrayable` for the ids. So if an `Arrayable` is passed, the check is skipped and the method returns the results. To fix this, we are first checking if the `$ids` is an `Arrayable` and we convert the ids to an array before checking the results. * Ensure find* methods on relationships are accepting Arrayable idsThe regression in #7048 and #19019 is also observed in #9143, because the `find`, `findMany` and `findOrFail` methods are copied from the Eloquent Builder to the `BelongsToMany` and `HasManyThrough` relations.For this reason, we need to convert the passed ids to an array before executing the queries.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most of eloquent query methods accepts array or
Arrayable, and recently this changed on version 5.4... I'm upgrading a app from5.2and it used to work.This fixed my problem, now:
Works as expected.