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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Stop adding constraints twice on *Many to *One relationships via one() #46575

Merged

Conversation

cosmastech
Copy link
Contributor

@cosmastech cosmastech commented Mar 24, 2023

@siarheipashkevich reported that constraints are duplicated when being converted from HasMany/HasManyThrough/MorphMany to HasOne/HasOneThrough/MorphOne

This PR will stop adding those constraints twice by employing the Relation::noConstraint() method (which was a TIL moment and I must say, is 馃敟)

@cosmastech cosmastech marked this pull request as draft March 24, 2023 13:05
@cosmastech cosmastech marked this pull request as ready for review March 24, 2023 13:19
@cosmastech cosmastech changed the title create new HasOne from HasMany without constraints [10.x] create new HasOne from HasMany without constraints Mar 24, 2023
@cosmastech cosmastech changed the title [10.x] create new HasOne from HasMany without constraints [10.x] Stop adding constraints twice on *Many to *One relationships via one() Mar 24, 2023
@siarheipashkevich
Copy link
Contributor

@cosmastech will it save additional conditions?

$this->shifts()->one()->opened() . opened is a scope.

@cosmastech
Copy link
Contributor Author

@cosmastech will it save additional conditions?

$this->shifts()->one()->opened() . opened is a scope.

@siarheipashkevich It should just ignore the constraints. Was the opened() scope not applying previously?

@siarheipashkevich
Copy link
Contributor

@cosmastech please take a look at my comment and log
#46443 (comment)

we can see that the scope has been applied and it's ok

@cosmastech
Copy link
Contributor Author

cosmastech commented Mar 24, 2023

@cosmastech please take a look at my comment and log

#46443 (comment)

we can see that the scope has been applied and it's ok

The scope is applied after the fact so it should not be a problem.

Edit: even a scope applied in the original HasMany should be applied when you call one()

@taylorotwell taylorotwell merged commit 034d849 into laravel:10.x Mar 25, 2023
15 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

3 participants