Skip to content
This repository has been archived by the owner on Feb 10, 2022. It is now read-only.

Add option to update instead of replace document during save #100

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mikestead
Copy link

From what I could determine, any time you call save() on an existing document, the update overwrites the document in the DB.

If you just want to update a single attribute in the document you first need to load the entire document into memory and then send the entire document back over the wire with the updates.

This change provides the option to update specific attributes of an existing document.

Example

const users = await User.where('username', 'user-a').include('height').find()
const user = users[0]
if (user.get('height') !== newHeight) {
  user.set('height', newHeight)
  await user.save({ merge: true })
}

I'm not sure merge is the best term? overwrite: false might be better but then we need to ensure it's truthy everywhere as default.

Ref https://docs.mongodb.org/manual/reference/operator/update/set

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 93.868% when pulling 49ecfd6 on mikestead:feature/update-merge into 72433a7 on vdemedes:master.

@vadimdemedes
Copy link
Owner

What if we make this a default behavior? Send only this.changed attributes and use $set: true? Can you think of any downsides?

@mikestead
Copy link
Author

Sounds good, I think it's more intuitive to do a $set instead of a replace by default.

I guess the main issue is that it's a breaking change so should require another major version bump?

@vadimdemedes
Copy link
Owner

Don't think that it's a breaking change, as API doesn't change and the result of .save() should be the same too.

@mikestead
Copy link
Author

The original example is a little simplified but given that document had more than just the property height, without the change you would have a document left with just height in it. Anyone depending on that behaviour would be broken. I'm not sure how common that would be, but is a possibility.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants