-
Notifications
You must be signed in to change notification settings - Fork 76
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
Fixes queryRecord issue #148 #152
Conversation
Just a note, the warnings were added in Ember Data 2.6.0. |
@dweremeichik maybe this helps to make it backwards compatible. |
@dweremeichik We started testing from Ember 2.4 and I tried in https://github.com/broerse/ember-pouch/tree/queryRecord to fix this PR but your added test still fails. Do you have time to take a look and fix your PR. I like to merge this. |
@broerse, haven't used ember-pouch in a while. However it used to build in 2.4 without issue. May have something to do with the tests changing when the hasmany "split tests" were added. |
Also, a brief look at the logs tells me it only breaks when the tests are "sync" not sure what that feature is, added after my initial PR, must be. |
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.
I think these changes will fix the failing test
@@ -340,7 +340,16 @@ export default DS.RESTAdapter.extend({ | |||
}, | |||
|
|||
queryRecord: function(store, type, query) { | |||
return this.query(store, type, query); | |||
return this.query(store, type, query).then(results => { | |||
let result = {}; |
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.
You need to edit the original results
object. Since this can have additional entries, which are sideloaded.
result[recordType] = results[pluralize(recordType)][0]; | ||
} else { | ||
result[recordType] = null; | ||
} |
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.
after using the singular recordType
to set the expected single item, you need to delete results[pluralize(recordType)];
to remove the plural entry
@dweremeichik this PR fixed the failing tests. 86601e6 |
This fixes issue #148, and removes the following warning:
DEPRECATION: The adapter returned an array for the primary data of a
queryRecord
response. This is deprecated asqueryRecord
should return a single record. [deprecation id: ds.serializer.rest.queryRecord-array-response]Note:
until: '3.0'
.queryRecord
was just an alias forquery
previously this would be a non-backward compatible change.