Use $set to only alter specified fields in Mongo document on update #31
Conversation
nporteschaikin
force-pushed
the
npc/save-dont-overwrite-attributes
branch
from
7da7b10 to
e54da1d
Compare
|
Awesome. I didn't realize it would be simple to address. |
nporteschaikin
force-pushed
the
npc/save-dont-overwrite-attributes
branch
from
e54da1d to
a4fa377
Compare
| @@ -234,7 +234,7 @@ def create | |||
| end | |||
|
|
|||
| def update | |||
| self.class.collection.update({ _id: id }, attributes) | |||
| self.class.collection.update({ _id: id }, { "$set" => attributes.except!(:_id) }) | |||
There was a problem hiding this comment.
WDYT about using except instead of except! to avoid mutation? I'm not sure if it will have side-effects but I worry it might, and it doesn't seem necessary.
There was a problem hiding this comment.
There was a problem hiding this comment.
@maxjacobson not sure why I went with the mutable array - fixed on rebase.
nporteschaikin
force-pushed
the
npc/save-dont-overwrite-attributes
branch
from
a4fa377 to
ce7cf8b
Compare
| @@ -7,10 +7,18 @@ | |||
| I18n.load_path << File.expand_path("../locale/en.yml", __FILE__) | |||
|
|
|||
| class User < Minidoc | |||
| self.collection_name = "users" | |||
There was a problem hiding this comment.
Is this needed on the base class here?
There was a problem hiding this comment.
@wfleming no - just wanted to be explicit so it's clear in the code why we're using two classes.
There was a problem hiding this comment.
It's not something we'd do in normal code, so I don't think it's necessary here in the test either. The other class is right below it, after all. NBD if you want to keep it, though.
nporteschaikin
changed the title
|
LGTM |
This addresses the issue @pbrisbin raised in codeclimate/app#2919, where calling
Minidoc#updateoverwrites the entire Mongo document in lieu of only updating fields specified by the model.cc @codeclimate/review