framework icon indicating copy to clipboard operation
framework copied to clipboard

Relationship is empty after using `hasAttached()` with model that `$touches` its parent

Open bakerkretzmar opened this issue 3 years ago • 4 comments

  • Laravel Version: 9.41.0
  • PHP Version: 8.1.11
  • Database Driver & Version: SQLite 3.37.0 and MySQL 8.0.30

Description:

Using the hasAttached() factory method to relate models, if either model $touches the relationship, the model instances will not be related correctly after being created—their relations to each other will be loaded but empty.

Steps To Reproduce:

  • New Laravel app demonstrating this issue: https://github.com/bakerkretzmar/laravel-has-attached-touches-bug (see this test)

  • Failing laravel/framework test case: https://github.com/laravel/framework/compare/9.x...bakerkretzmar:framework:fix-factory-relations-with-touches

You have two Eloquent models, e.g. User and Group, connected with a BelongsToMany relationship. We want users to have their timestamps updated any time any of their groups are updated, so the Group model contains protected $touches = ['users'].

In a test, you create a user and group and attach them:

$user = User::factory()->create();
$group = Group::factory()->hasAttached($user)->create();

$this->assertCount(1, $group->users);

That assertion fails.

The users relation is set on the new $group model, as if it was just loaded from the database, but it's an empty collection (output from dump($group) inside the above test):

image

This only happens when one of the models has the relationship listed in its $touches property.

bakerkretzmar avatar Nov 23 '22 22:11 bakerkretzmar

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub. If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

github-actions[bot] avatar Nov 24 '22 09:11 github-actions[bot]

Hello @bakerkretzmar @driesvints I worked on a fix for this, but I'm not sure if this is correct so IDK if I should create the PR already here or not, can you help me check and guide me if I should create this PR or not?

WendellAdriel avatar Nov 24 '22 12:11 WendellAdriel

@WendellAdriel that looks like it might work... but I'm actually not so sure anymore whether this is a bug. It might just be how $touches works, although to me it was quite unintuitive. Here's another example that's weird but I guess correct if I think about it:

$user = User::create();

// Because of `$touches`, this automatically loads the `users` relationship, which is empty
$group = Group::create();

// This attaches the user correctly but does not reload the relationship
$group->users()->attach($user);

// Empty collection, because it uses the already-loaded (empty) relationship
dump($group->users);

Is that expected? Just something people need to be aware of and account for when using $touches?

bakerkretzmar avatar Nov 24 '22 16:11 bakerkretzmar

@bakerkretzmar that's interesting. IDK either if that's a bug or not TBH I worked on that fix but we would need to know the expected behaviour first to be able to check if that's a bug or not and what would be the best solution for that. I think we have to wait on more feedback on this. But that's a good catch and it was good for me to start looking at how things work on this side of the framework since I'm just starting out to try to contribute to it

WendellAdriel avatar Nov 24 '22 16:11 WendellAdriel