[BUGFIX release] Load many2many relationships correctly #5142
Conversation
When a many2many relationship exists and more than one related item is included in the relationship ensure that both sides load completely. This issue was introduced in e430ede
| @@ -209,7 +209,7 @@ export default class ManyRelationship extends Relationship { | |||
|
|
|||
| for (let i = 0; i< internalModels.length; i++) { | |||
| let internalModel = internalModels[i]; | |||
| if (this.canonicalMembers.has(internalModel)) { | |||
There was a problem hiding this comment.
I'm super confused how these end up being different. So thank you for providing a test which I can debug :)
There was a problem hiding this comment.
@stefanpenner I wondered that too, and I think could be a side effect here: https://github.com/emberjs/data/blob/master/addon/-private/system/relationships/state/relationship.js#L167 which add internal models to the relationship canoninal members. So this may leads to this.canonicalMembers and forCanonicalbeeing different
|
@jrjohnson thanks for the PR, after debugging your failing test I now see the issue! I have adapted your fix slightly (just a simplification, but your still the committer) -> #5144 |
|
Great! I'm glad it put you on the right track. My fix was pretty much a shot in the dark. |
|
@jrjohnson your fix and test really made this super actionable. I wish all bug reports came this way :) |
Fair warning, I really don't know what this fix does. I think the original commit I'm patching had a small logic error that this fixes, but I hope @stefanpenner (who did the original work) can verify that. This change does fix the issue I was seeing though so I'm hopeful it is correct.
When a many2many relationship exists and more than one related item is
included in the relationship ensure that both sides load completely.
This issue was introduced in e430ede
Fixes #5140