search

ga-wdi-boston / express-api

Other
2 stars 110 forks source link

findbyidandupdate #4

Open RealWeeks opened 8 years ago

RealWeeks commented 8 years ago

possible use for next time

gaand commented 8 years ago

Please don't. That's efficiency over functionality. Which is why my examples don't use it. A variety of features are lost using the findByIdAnd/findOneAnd methods

jrhorn424 commented 8 years ago

Informational comment: If I'm reading this correctly, .findOneAndUpdate and .findByIdAndUpdate are entirely different. http://mongoosejs.com/docs/api.html#model_Model.findOneAndUpdate

gaand commented 8 years ago

I believe findById(id) is approximately equivalent to findOne({_id: id})

laurenfazah commented 8 years ago

Moving forward as not adding findByIdAndUpdate, correct @J-Weeks @gaand @jrhorn424? We can close if so.

jrhorn424 commented 8 years ago

I believe there it is OK to .findOneAndUpdate. Note that is not what the original issue was about. I'd like to discuss if there's disagreement.

gaand commented 8 years ago

Mongoose middleware only fires on certain methods and I think we want to have that available. The findAnd methods are not among them. @jrhorn424 I'm happy to chat about it.

jrhorn424 commented 8 years ago

Ok, then. 😄

gaand commented 8 years ago

I think the mongoose crud talk should include the reasons to favor save, etc. See ga-wdi-boston/mongoose-crud#14

raq929 commented 7 years ago

It seems that there is agreement not to make use findbyidandupdate. Should we close?

gaand commented 7 years ago

I am okay with that. @jrhorn424 ❓