-
- Notifications
You must be signed in to change notification settings - Fork 3.9k
fix(model): fix overwriteImmutable not working with timestamps: true, add overwriteImmutable types re #15781 #15819
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: master
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 adds TypeScript type definitions for the overwriteImmutable option in bulkWrite operations, addressing issue #15781. The option allows updating fields marked as immutable in the schema during updateOne and updateMany operations within bulkWrite.
Key Changes:
- Added
overwriteImmutable?: booleantype toUpdateOneModelandUpdateManyModelinterfaces - Added TypeScript type tests to verify the new option types
- Refactored existing JavaScript test for better structure and clarity
- Updated tdd npm script with more specific watch patterns
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| types/models.d.ts | Added overwriteImmutable option to UpdateOneModel and UpdateManyModel interfaces |
| test/types/models.test.ts | Added type test coverage for overwriteImmutable, resolved import naming conflict with MongoDB driver |
| test/model.updateOne.test.js | Refactored test into nested describe block with better organized test cases using Arrange-Act-Assert pattern |
| package.json | Updated tdd script with quoted glob patterns and added trailing newline |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
package.json Outdated
| "setup-test-encryption": "node scripts/setup-encryption-tests.js", | ||
| "test-encryption": "mocha --exit ./test/encryption/*.test.js", | ||
| "tdd": "mocha ./test/*.test.js --inspect --watch --recursive --watch-files ./**/*.{js,ts}", | ||
| "tdd": "npx mocha ./test/*.test.js --inspect --watch --recursive --watch-files 'test/**/*.js' --watch-files 'lib/**/*.js'", |
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.
As copilot already pointed out, why is this change necessary?
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.
I created the tdd script long time ago, but it's broken now. I'm guessing this is because of a change in the type-testing approach, IIRC we didn't start with tsd as our type testing tool.
Anyway, the new script fixes the errors from the old script. It's not really used as part of any automation so it's safe to change.
…of `timestamps` value
overwriteImmutable types re #15781overwriteImmutable types re #15781
overwriteImmutablere TimestampcreatedAtcan't be updated usingbulkWrite#15781 and fix(bulkWrite): pass overwriteImmutable option to castUpdate fixes #15781 #15782overwriteImmutablewould only work oncreatedAtwithtimestamps: false.