Hybrid support for MorphToMany relationship #2690
Conversation
MongoModel -> MorphToMany -> SqlModel
MongoModel -> MorphToMany -> SqlModel
…-thomas/laravel-mongodb into add-hybrid-support-to-morph-to-many
hans-thomas
force-pushed
the
add-hybrid-support-to-morph-to-many
branch
from
6a2437c to
af2be8c
Compare
hans-thomas
force-pushed
the
add-hybrid-support-to-morph-to-many
branch
from
af2be8c to
e561201
Compare
|
Can you update this PR please. There is some conflicts. |
hans-thomas
force-pushed
the
add-hybrid-support-to-morph-to-many
branch
from
1ce14da to
8440c57
Compare
|
|
||
| // Attach the new parent id to the related model. | ||
| $model->push($this->foreignPivotKey, $this->parseIds($this->parent), true); | ||
| $model->push($this->foreignPivotKey, (array) $this->parent->{$this->parentKey}, true); |
There was a problem hiding this comment.
(array) cast should not be used. If the value is an ObjectId, the result is not the one you expect.
var_dump((array) new MongoDB\BSON\ObjectId());
/*
array(1) {
'oid' =>
string(24) "6577071f3fded0903a01f030"
}
*/Here we know it's not a list of ids, so a simple array wrapping is fine.
| $model->push($this->foreignPivotKey, (array) $this->parent->{$this->parentKey}, true); | |
| $model->push($this->foreignPivotKey, [$this->parent->{$this->parentKey}], true); |
There was a problem hiding this comment.
I will double-check it later. thanks for the review❤️
There was a problem hiding this comment.
The ObjectId fields are always string. I tested this with a primary key and a custom key and a custom key was cast to ObjectId.
In all cases, the $this->parent->{$this->parentKey} value is string. Your suggestion is more readable, but I don't see any bug here to open a PR.
There was a problem hiding this comment.
Yes, that's something we'll have to fix if we manage to store references as ObjectId. But there are many other hurdles to overcome, and I think this notion of "all keys are strings" is too hard-wired into Eloquent anyway.
|
Thank you @hans-thomas |
Hello everybody, this PR adds hybrid support for the MorphToMany relationship.
I tested it in two scenarios. First, an SQL model makes a MorphToMany relation to a MongoDB model. In the second one, A MongoDB Model makes a MorphToMany to an SQL model.