-
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
[9.x] Fix loadAggregate not correctly applying casts #41050
Conversation
Can you provide a simple example of how I can recreate this issue in a fresh Laravel application with code examples? Please mark this as ready for review when you have done so otherwise I won't see it. |
Hey @taylorotwell Here's a simple code example ( Also the PR contains a test which would fail on a fresh laravel installation ): Models class User extends Authenticatable
{
public function posts()
{
return $this->hasMany(Post::class);
}
} class Post extends Model
{
use HasFactory;
protected $guarded = [];
public function comments()
{
return $this->hasMany(Comment::class);
}
} class Comment extends Model
{
use HasFactory;
} Some data $user = \App\Models\User::find(1);
$user->posts()->createMany([
['content' => 'Test'],
['content' => 'Test'],
['content' => 'Test'],
]); The test $user = \App\Models\User::with('posts')->find(1);
$user->posts->loadExists("comments");
return $user->posts; This will result in the following ( notice and dd($user->posts->pluck('comments_exists')); |
Thanks |
* Fix loadAggregate not correctly applying casts * Added tests to "Fix loadAggregate not correctly applying casts" * Style fix * Update Collection.php Co-authored-by: Taylor Otwell <taylor@laravel.com>
* Fix loadAggregate not correctly applying casts * Added tests to "Fix loadAggregate not correctly applying casts" * Style fix * Update Collection.php Co-authored-by: Taylor Otwell <taylor@laravel.com> Co-authored-by: A. Alyusuf <ahmed@vioat.com> Co-authored-by: Taylor Otwell <taylor@laravel.com>
Reopening #41025 with tests and better explanation about the bug.
There's a bug in
loadAggregate
in Eloquent Collections specefically when runningLoadExists
where running something like$user->articles->loadExists("comments")
will addcomments_exists
to all existing articles but only the first model will have boolcasts
automatically added to it. Resulting in something similar to:This PR fixes that and adds casts to all items instead of only the first one. I highly doubt it'll be breaking change to anyone.