search

vadimdemedes / mongorito

🍹 MongoDB ODM for Node.js apps based on Redux
1.38k stars 90 forks source link

Adds findManyByIds. Changes find logic to handle findManyByIds. #116

Closed LeviTrammell closed 8 years ago

LeviTrammell commented 8 years ago

I came to the unfortunate finding that this query was not possible.

ids = ['573008b792c589ee8918318c','573008b892c589ee8918318d','573008b892c589ee8918318e']; let objects = yield Model.find({ _id:{$in:ids} });

The most frustrating part in all of this was that it was only happening for the _id field. Weird right? That's when I dug a little deeper and saw that you were doing the handy dandy toObjectId in the "find" function. There could be a better implementation of this, however I saw that you were already doing a findById, so I thought : why not make a findManyById as well.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-1.05%) to 92.986% when pulling 4854752c012ab0883a245352e2e4f7fa55dc1e71 on LeviTrammell:features/findManyByIds into e6f05e7768bf6510da35ed635fd4a7c1b4b45602 on vdemedes:master.

vadimdemedes commented 8 years ago

Thank you for your work, but I think there must be a better way to do this. Specifically, fix Query so that your initial code example works, as it's more intuitive and clear, imho.

LeviTrammell commented 8 years ago

I did, and it does work that way now. I just added the findManyByIds as a helper function. I can take it out if you think it's better that way.

ulpian commented 8 years ago

@vdemedes Can we just close this? I think #118 is a better solution.

LeviTrammell commented 8 years ago

Noticed you added in a better solution with using findByIds. Closing this. I'll be bringing up another couple of finds in the next couple of days.