-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Hybrid support for BelongsToMany relationship #2688
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
Hybrid support for BelongsToMany relationship #2688
Conversation
Co-Authored-By: Junio Hyago <35033754+juniohyago@users.noreply.github.com>
Co-Authored-By: Junio Hyago <35033754+juniohyago@users.noreply.github.com>
The relation data of the current eloquent model instance doesn't update. It has to be fixed to prevent users from fetching the latest relationship data from DB each time or it's possible to make users confused.
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.
Do we really want to have a pivot table here? If the relations are already stored in the MongoDB document, it should not be duplicated in an SQL table.
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.
laravel-mongodb/src/Eloquent/HybridRelations.php
Lines 290 to 301 in fc1f9cc
The sql models don't reach our BelongsToMany class. So, we can't control this behavior.
As I know, the sql models, store their relations data in a pivot tabel and mongo models, store theirs in a pivot column.
Hi @GromNaN, Could you please merge these two PRs?
I know you're super busy but I will be gone from next week for at least 2 months (it can be increased to one and a half years) and I don't have access to the Internet in that period.
Is there any chance to release the v4.1?
It seems feasible today.
It would be awesome😁🔥
before releasing it, I want to open one more PR for BelongsToMany relation. I noticed some potential bugs when we set custom keys.
Thank you @hans-thomas. I'm merging the other one and wait for your new PR.
Uh oh!
There was an error while loading. Please reload this page.
Hi, I noticed the lines that we removed in #2664 (comment) PR, were part of handling the hybrid relationship that was not covered by tests.
This PR is continuing #2276. Thanks to @juniohyago.