Skip to content
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

[10.x] Adds the firstOrCreate and createOrFirst methods to the HasManyThrough relation #48541

Merged
merged 9 commits into from
Sep 26, 2023

Conversation

tonysm
Copy link
Contributor

@tonysm tonysm commented Sep 26, 2023

Added

  • Adds the firstOrCreate method to the HasManyThrough relation
  • Adds the createOrFirst method to the HasManyThrough relation

In #48529 we found a bug in the updateOrCreate. The issue was because that method was changed to use the firstOrCreate behind the scenes. Since the firstOrCreate method didn't exist in the relation, the method call was forwarded to the query builder, so the query changed from select users.* from users ... inner join teams ... to select * from users ... inner join teams ..., which resulted in a bug that was causing updates on the wrong model when using updateOrCreate (I shared more context here).

The updateOrCreate was reverted to the previous implementation. However, this bug will still happen if someone calls the ->firstOrCreate() or the ->createOrFirst() methods on a HasManyThrough relation (for the same reason, the method calls would be forwarded to the query builder and it would run a select * from users ... inner join teams ... causing the result to have 2 id columns, which caused the original bug).

We're adding the missing firstOrCreate and createOrFirst methods to fix the issue on the HasManyThrough relation. I was also able to reproduce the original regression error on both methods.

@tonysm tonysm marked this pull request as ready for review September 26, 2023 02:09
@taylorotwell taylorotwell merged commit 2a4d848 into laravel:10.x Sep 26, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants