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

Fixes queryRecord issue #148 #152

Merged
merged 3 commits into from
Jun 14, 2017

Conversation

dweremeichik
Copy link
Contributor

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 as queryRecord should return a single record. [deprecation id: ds.serializer.rest.queryRecord-array-response]


  • The appropriate behavior is outlined in the Ember Data API, here.
  • Test added to ensure that null is returned when the filter doesn't resolve a record.

Note:

  • The warning appears to be in place until: '3.0'.
  • Since queryRecord was just an alias for query previously this would be a non-backward compatible change.

@dweremeichik
Copy link
Contributor Author

Just a note, the warnings were added in Ember Data 2.6.0.
Build fails for 2.1 stack and below.

@fsmanuel
Copy link
Collaborator

@dweremeichik maybe this helps to make it backwards compatible.

@broerse
Copy link
Collaborator

broerse commented Jun 12, 2017

@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.

@dweremeichik
Copy link
Contributor Author

dweremeichik commented Jun 12, 2017

@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.

@dweremeichik
Copy link
Contributor Author

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.

Copy link
Collaborator

@jlami jlami left a 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 = {};
Copy link
Collaborator

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;
}
Copy link
Collaborator

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

@broerse broerse merged commit 8e64350 into pouchdb-community:master Jun 14, 2017
@broerse
Copy link
Collaborator

broerse commented Jun 14, 2017

@dweremeichik this PR fixed the failing tests. 86601e6

@dweremeichik
Copy link
Contributor Author

Awesome work @jlami @broerse!

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.

4 participants