HasOne / HasMany must respect $localKey parameter #1837
Conversation
|
ping @jenssegers |
|
Is this a breaking change? |
|
No, just a fix. I updated the tests to force the error when filled. |
|
Will take a look tomorrow |
| @@ -154,7 +154,7 @@ protected function getConstrainedRelatedIds($relations, $operator, $count) | |||
| protected function getRelatedConstraintKey($relation) | |||
There was a problem hiding this comment.
Add type for $relation in doc block upper please
There was a problem hiding this comment.
There was a problem hiding this comment.
Done with that, also updated has() to comply with Laravel signature Relation|string.
tests/RelationsTest.php
Outdated
| @@ -61,24 +61,24 @@ public function testBelongsTo(): void | |||
| public function testHasOne(): void | |||
| { | |||
| $user = User::create(['name' => 'John Doe']); | |||
| Role::create(['type' => 'admin', 'user_id' => $user->_id]); | |||
| Role::create(['type' => 'admin', 'roleuser_id' => $user->_id]); | |||
There was a problem hiding this comment.
What happens if we leave name user_id?
There was a problem hiding this comment.
Well, if we leave 'user_id' then there is no way to check that HasOne / HasMany respects $localKey parameter.
There was a problem hiding this comment.
I will make a cleaner test, using a different Model to test this.
|
@stephandesouza merge actual master |
|
@Smolevich @jenssegers any chances to merge it now? |
|
This PR can have break changes, we need test on this case for example from #1902 |
HasOne / HasMany must respect $localKey parameter
Fixes a bunch of issues.
#1817 #1807 #1692 #1691 #1617 #1540 #1467
I thing there are more related