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

query not filtering on modelType #168

Open
eneuhauser opened this issue Jan 24, 2017 · 12 comments
Open

query not filtering on modelType #168

eneuhauser opened this issue Jan 24, 2017 · 12 comments

Comments

@eneuhauser
Copy link

When I call get on an async hasMany relationship that isn't loaded, I do not get the related models back. To get around this, I tried calling query filtering on the belongsTo attribute, but I'm getting back additional models that also have the same belongsTo relationship as the model type I queried.

If I have a model structure like the following:

const Author = Model.extend({
    name: DS.attr('string'),
    posts: DS.hasMany('post'),
    comments: DS.hasMany('comment')
});
const Post = Model.extend({
    title: DS.attr('string'),
    content: DS.attr('string'),
    author: DS.belongsTo('author')
});
const Comment = Model.extend({
    text: DS.attr('string'),
    author: DS.belongsTo('author')
});

With just the author loaded and I call author.get('comments'), I do not get back any results. If I call:

store.query('comment', { filter: { author: author.get('id') }});

I will also get back posts for the author, but they will be loaded in the store as comment and the text attribute will be undefined.

Additional information, I'm using the dontsavehasmany flag, ember-pouch 4.2.3, and ember-data 2.10 (and 2.11.0-beta.1).

@jlami
Copy link
Collaborator

jlami commented Jan 24, 2017

Using query with only the 'author' as a selector is a known bug: pouchdb-community/relational-pouch#73

But the query that is used to get the entries for the hasMany should work with a better query that does restrict by type (filter on id). I'll try to see if I can reproduce this, but this should be covered by the test suite. Maybe you could build a ember twiddle?

@eneuhauser
Copy link
Author

Thanks for the quick feedback. I had searched around to see if this was an existing issue before submitting, but did not find any results. I'll try and create a Twiddle, but I don't see ember-pouch hosted on a CDN to add as a dependency.

Is it possible to build my query to filter on ID as well? I'm thinking of just adding a type attribute and filtering on that.

@backspace
Copy link
Collaborator

If you use Ember Twiddle, @eneuhauser, you can put dependencies in the twiddle.json, similarly to how Ember CLI dependencies work.

@jlami
Copy link
Collaborator

jlami commented Jan 24, 2017

You can see the code for querying by type here: https://github.com/nolanlawson/relational-pouch/blob/master/lib/index.js#L426
The _id filter on that line does what you want, though the parameters for lt and gt might be a bit confusing.

@eneuhauser
Copy link
Author

I appreciate @jlami and @backspace help. I did not expect such quick turnaround. I was able to add the ember-pouch addon to Twiddle. The last time I tried to make a Twiddle with an external dependency, I don't think that was possible. Knowing how to do that will be useful in the future.

Before trying the Twiddle, I pulled down the ember-pouch code and found an existing test that shows that async hasMany relationships are loaded with a get, so I'm not sure why it doesn't work for my project.

I was able to get query to work based on your suggestions. I had to copy the query from ember-pouch and override it in my adapter because not enough details were passed to _buildSelector to properly build the selector:

query: function(store, type, query) {
  this._init(store, type);

  var recordTypeName = this.getRecordTypeName(type);
  var db = this.get('db');

  var queryParams = {
    selector: this._buildSelector(query.filter)
  };
  queryParams.selector['_id'] = this._buildDocIDTypeSelector(recordTypeName);

  if (!Ember.isEmpty(query.sort)) {
    queryParams.sort = this._buildSort(query.sort);
  }

  return db.find(queryParams).then(pouchRes => db.rel.parseRelDocs(recordTypeName, pouchRes.docs));
},
_buildDocIDTypeSelector(recordTypeName) {
  var makeDocID = this.get('db').rel.makeDocID;
  return {
    '$gt': makeDocID({ type: recordTypeName }),
    '$lt': makeDocID({ type: recordTypeName, id: {} })
  };
},

Would it be useful to make a pull request with this change?

@jlami
Copy link
Collaborator

jlami commented Jan 25, 2017

A PR for the fix in query is certainly welcome. Though I'm wondering if this should not be moved down to relational-pouch, since that is the layer that is actually in charge of the mapping to types. In my eyes it should have a query function of it's own that ember-couch should call.

Did you have any success in making a twiddle that reproduces your initial problem with hasMany? I'm still very curious where this comes from. You are right that there are tests that work with hasMany and get, so this should work correctly.

@broerse
Copy link
Collaborator

broerse commented Jan 25, 2017

@eneuhauser @guillett Perhaps relational-pouch it the place to fix this. See also pouchdb-community/relational-pouch#73

@rhysdavies1994
Copy link

rhysdavies1994 commented Feb 15, 2017

+1 this needs to be fixed

If I query posts by their name, why am I also receiving users by their name?
Isn't that the point of passing in a type to a query!

@stickbyatlas
Copy link

@eneuhauser So... I was bitten by this unexpected behaviour and it took a while to realize it was being caused by the wrong records being retrieved by my query statements. I just grabbed your code above and pasted it into my application adatapber, and it seems like that did the trick. I'm just going to leave your code in place in my adapter until I see an update from the addon itself. Thanks for sharing!

@afinne
Copy link

afinne commented May 7, 2017

See also pouchdb-community/relational-pouch#3

@afinne
Copy link

afinne commented Sep 18, 2017

Unfortunately this approach might cause unexpected results due to a bug in CouchDB: apache/couchdb#816
I.e. if you have a mango index created using _id, all queries that just want to query on _id uses that index.
This is of course only true if you use ember-pouch to connect to a remote CouchDB

@jlami
Copy link
Collaborator

jlami commented Sep 18, 2017

Wouldn't that be true for any index with _id?
Ember-pouch now already uses _id to make indexes for the belongsTo relations, so I don't know if this should change anything. https://github.com/pouchdb-community/ember-pouch/blob/master/addon/adapters/pouch.js#L205

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

No branches or pull requests

7 participants