Skip to content

Fixes queryRecord issue #148#152

Merged
broerse merged 3 commits intopouchdb-community:masterfrom
dweremeichik:queryRecord
Jun 14, 2017
Merged

Fixes queryRecord issue #148#152
broerse merged 3 commits intopouchdb-community:masterfrom
dweremeichik:queryRecord

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

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