-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
fix(model): bump version if necessary after successful bulkSave() #15809
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
base: 8.x
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #15800 by ensuring that bulkSave() increments the document's version key (__v) after a successful save, bringing it in line with the behavior of the regular save() method.
Key changes:
- Refactored version increment logic into a reusable
_applyVersionIncrement()helper method in Document - Modified
bulkSave()to call this helper after successful writes - Added comprehensive test coverage for version increment behavior in
bulkSave()
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/document.js | Adds new _applyVersionIncrement() helper method to centralize version increment logic |
| lib/model.js | Refactors $__save() to use the new helper and updates handleSuccessfulWrite() (used by bulkSave()) to increment versions on successful saves |
| test/model.test.js | Adds comprehensive test covering version increment on successful save, no increment on validation errors, and correct behavior with different field types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hasezoey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK to me, but i agree with both of copilots comments / suggestions (function naming & comment location).
Just for clarification, the added test is not meant to increment version above 1?
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
"Just for clarification, the added test is not meant to increment version above 1?" <-- No, because Mongoose only increments version on array updates. Updating a string property doesn't trigger a version update by default. |
Fix #15800
Summary
Make
bulkSave()increment the document's version on successful save.Need to backport this to 7.x.
Examples