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
PHPORM-127 Fix priority of embeds vs attributes #2584
base: 4.1
Are you sure you want to change the base?
Conversation
da4339c
to
81a0fc6
Compare
// First, we'll need to determine the foreign key and "other key" for the | ||
// relationship. Once we have determined the keys we'll make the query | ||
// instances as well as the relationship instances we need for this. | ||
$foreignKey = $foreignKey ?: $this->getForeignKey().'s'; | ||
|
||
$instance = new $related; | ||
|
||
if ($otherKey === $relation) { | ||
throw new \LogicException(sprintf('In %s::%s(), the key cannot be identical to the relation name "%s". The default key is "%s".', static::class, $relation, $relation, $instance->getForeignKey().'s')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The solution was so easy, just don't name the foreign key the same as the relation; that is the default behavior if you don't pass any foreign keys to $this->belongsToMany(related , /* Don't pass anything else */) , and that is also the documented way to do it.
I propose to throw an exception in this case to avoid any error.
tests/Models/User.php
Outdated
|
||
public function otherGroups() | ||
{ | ||
return $this->belongsToMany(Group::class, 'groups', 'users', 'otherGroups', '_id', '_id', 'groups'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to add a test for the exception.
171d8c2
to
68afe15
Compare
Fix #2577 (continuing)
Fix PHPORM-127
Moving the code from the
Model
class to theEmbedsRelations
trait.