Skip to content
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

Fix missing inverse #5268

Closed
wants to merge 6 commits into from

Conversation

BryanCrotaz
Copy link
Contributor

Fix issue #5267

Initially add test to show problem

* commit 'dd3cd69462943530039428a0203f7d66beb1bfd0':
  Bump master version to 3.0.0-canary
  Update changelog for Ember Data 2.17.0 release
  [DOC beta] Assert that both modelName and id are passed to `peekRecord` (emberjs#4998)
  [doc] Update urlForFindRecord example
* commit 'ed4e45f7ca4a28a478293857442164e295eccf5b':
  Update ember-cli-blueprint-test-helpers fixes emberjs#5259 (emberjs#5264)
  Update AdapterPopulatedRecordArray to use links if available otherwise null. (emberjs#5263)
@BryanCrotaz
Copy link
Contributor Author

BryanCrotaz commented Nov 30, 2017

@stefanpenner needs some input from you. Could you comment on the direction I've taken here, storing polymorphic relationships against the base class and not the descendant class? It fixes the problem I found but breaks a couple of tests. Before I go and find out why, I wanted to check that this is the right way to go.

EDIT: and about 5 seconds later I realised why they were broken...

@BryanCrotaz
Copy link
Contributor Author

Probably this has no value given #5230, but worth comparing for method

@@ -132,9 +132,11 @@ export default class RelationshipPayloadsManager {
let modelClass = this._store._modelFor(modelName);
let relationshipsByName = get(modelClass, 'relationshipsByName');
relationshipsByName.forEach((_, relationshipName) => {
let relationship = relationshipsByName.get(relationshipName);
let modelThatOwnsRelationship = relationship.parentType;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never trust parentType it may not have been loaded via modelFor yet and thus missing modelName

@runspired
Copy link
Contributor

Closing as #5230 has landed

@runspired runspired closed this Mar 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants