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

Save should actually save current object state, not passed attr argument #5

Open
neoromantic opened this issue Aug 21, 2013 · 3 comments

Comments

@neoromantic
Copy link

It seems to me that proper Model layer should be able not only to translate data from database layer to objects, but other way around too.

Like this:

item = myModel.first({id: 'some-id'})
item.title = 'New title'
item.save() # validates, triggers before_save and after_save, updates collections.

@kaptron
Copy link
Contributor

kaptron commented Aug 23, 2013

Agreed this is how it should work. It's another thing that would require a lot more work to set up a proper "dirty tracking" system (like how mongoid does it), in order to know which fields have been modified and which are already persisted. Without that, the only thing we could do is save all the attributes every time you do a model.save() which is fairly inefficient -- particularly in the case of Meteor where we have reactive methods that may be responding to particular document fields.

So yes, it would be a great improvement! For now, passing attr is the easiest way to tell it specifically which attributes you want to modify on the document without having to simply write all of the model's attributes to the database.

@neoromantic
Copy link
Author

So, would you be open if I spend some time to implement this and send you as pull request?

On 23 Aug 2013, at 05:21, kaptron [email protected] wrote:

Agreed this is how it should work. It's another thing that would require a lot more work to set up a proper "dirty tracking" system (like how mongoid does it), in order to know which fields have been modified and which are already persisted. Without that, the only thing we could do is save all the attributes every time you do a model.save() which is fairly inefficient -- particularly in the case of Meteor where we have reactive methods that may be responding to particular document fields.

So yes, it would be a great improvement! For now, passing attr is the easiest way to tell it specifically which attributes you want to modify on the document without having to simply write all of the model's attributes to the database.


Reply to this email directly or view it on GitHub.

@benmgreene
Copy link
Contributor

This is a big feature for me as well. Since thre hasn't been movement on this for 9 months, I'll assume it's dead and in need of being picked up.

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

3 participants