Don't warn for 'async: false' relationships with link + data - #4599
Conversation
beff34d to
64640f5
Compare
|
Since moving code from |
|
|
||
| BelongsToRelationship.prototype.updateData = function(data) { | ||
| // If the data contains a relationship that is specified as an ID, | ||
| // `deserializeRecordId()` will convert it into an DS.Model instance |
There was a problem hiding this comment.
s/DS.Model instance/InternalModel/
|
|
||
| ManyRelationship.prototype.updateData = function(data) { | ||
| // If the data contains a relationship that is specified as IDs, | ||
| // `deserializeRecordIds()` will convert them to DS.Model instances |
| } | ||
| }, | ||
|
|
||
| updateData: Ember.K |
There was a problem hiding this comment.
Ember.K shouldn't be used anymore. Either function() {} or remove completely, since it's defined in the subclasses anyhow...
6beab0f to
8c0cd0c
Compare
| isNone | ||
| } = Ember; | ||
|
|
||
| export function convertResourceObjectToInternalModel(store, key, relationship, id) { |
There was a problem hiding this comment.
It feels to me that this seems like a method that belongs to the store?
There was a problem hiding this comment.
It's not really converting, it's also pushing right?
| if (hasData) { | ||
| this.setHasData(true); | ||
| this.setHasLoaded(true); | ||
| } else if (hasLink) { |
There was a problem hiding this comment.
Seems like if you push the same link twice, this will say hasLoaded is false now
There was a problem hiding this comment.
There was a problem hiding this comment.
Also added a test to cover this
b1e7032 to
0ebac3e
Compare
|
|
||
| // Pushing new data to the relationship might contain only data or links or | ||
| // a combination of both. Depending on what we pushed we need to set the | ||
| // hasData and hasLoaded flags accordingly. |
There was a problem hiding this comment.
This comment might be better if it explains the logic & intentions behind doing this instead of saying accordingly
| if (isNone(ids)) { | ||
| return; | ||
| } | ||
| _pushResourceObjectsToStore(relationship, ids) { |
There was a problem hiding this comment.
These aren't ids anymore right?
| if (isNone(id)) { | ||
| return; | ||
| } | ||
| _pushResourceObjectToStore(relationship, id) { |
0ebac3e to
468419c
Compare
| if (isNone(id)) { | ||
| return; | ||
| } | ||
| _pushResourceIdentity(relationship, resourceIdentity) { |
There was a problem hiding this comment.
Resource Identifier, not Resource Identity
| if (isNone(ids)) { | ||
| return; | ||
| } | ||
| _pushResourceIdentities(relationship, resourceIdentities) { |
There was a problem hiding this comment.
Resource Identifiers, not Resource Identities
|
@wecc do you have time to fix the variable names you commented on? |
468419c to
ce4eec6
Compare
- Improve related link tests
ce4eec6 to
6133c88
Compare
| }; | ||
|
|
||
| ManyRelationship.prototype.updateData = function(data) { | ||
| let internalModels = this.store._pushResourceIdentities(this, data); |
There was a problem hiding this comment.
let internalModel = this.store._pushResourceIdentifiers(this, data);
6133c88 to
9817170
Compare
|
Thanks @wecc 🎉 |
|
I have updated ember-data to 2.10-beta3 and applied this PR in my environment and now I'm getting: You pushed a record of type 'X' with a relationship 'Y' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload. (The last bit is new) |
|
Hi @pcambra sorry for the late response. As you have discovered this pr was not included in Ember Data 2.10. The first release that includes this code is Ember Data 2.11. |
5 participants
This PR moves most of the code from
store.setupRelationships()torelationship.push()to be able to set thehasDataandhasLoadedflags correctly depending on both data and links in the payload.relationship.push()will also allow us to push the contents from a relationship's self link in the future.Fixes #3393