search

aslagle / reactive-table

A reactive table designed for Meteor
https://atmospherejs.com/aslagle/reactive-table
Other
328 stars 137 forks source link

allow filtering on _id #433

Closed s7dhansh closed 8 years ago

s7dhansh commented 8 years ago

allow filtering on _id, when only selective fields are being published. There is no need to check for the existence of settings.fields._id IMO, because anyways we have to set it on.

aslagle commented 8 years ago

You do need to check in case someone set it to be excluded - it shouldn't be replaced in that case. Only setting it if it's not there should do it.

s7dhansh commented 8 years ago

Ok. Makes sense.

aslagle commented 8 years ago

I tested this with excluded fields and I got errors like this: MongoError: Can't canonicalize query: BadValue Projection cannot have a mix of inclusion and exclusion. You should move it a couple lines down into the code that only runs if there's an included field already - then you wouldn't need the extra check either.

s7dhansh commented 8 years ago

So what you encountered was probably because we can not mix inclusions and exclusions - http://stackoverflow.com/questions/24949544/mongodb-cant-canonicalize-query-badvalue-projection-cannot-have-a-mix-of-incl

I tried with _id: 0, and I get this error Error: You may not observe a cursor with {fields: {_id: 0}}. So I guess we can not exclude _id at any cost in publications. Still I have improved the check.

Doing it in the inclusion block will require much more checks and code, so I assume this is the best way. What say?

s7dhansh commented 8 years ago

Oh, I too encountered your error on pure exclusions, so mongo does not allow to mix _id: 1 with pure exclusions too. Will try to figure out a way.

s7dhansh commented 8 years ago

@aslagle any feedback?

aslagle commented 8 years ago

Looks good, thanks!