-
Notifications
You must be signed in to change notification settings - Fork 13k
chore: Add projection to Rooms.findOneByIdAndType
#37254
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
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes The changes are homogeneous and targeted—a single method is consistently generified across the interface and implementation with clear intent. Type constraint consistency and the options cast require brief verification. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/models/src/models/Rooms.ts (1)
944-950: Remove unnecessary type-cast default; align implementation with interface signature.The interface in
packages/model-typings/src/models/IRoomsModel.ts:207already declaresoptions?: FindOptions<T>(optional). The implementation should match. The only call site atapps/meteor/app/otr/server/methods/updateOTRAck.ts:42explicitly provides options, so making the parameter optional is safe and eliminates the unsafe cast.Apply:
- findOneByIdAndType<T extends Document = IRoom>( - roomId: IRoom['_id'], - type: IRoom['t'], - options: FindOptions<T> = {} as FindOptions<T>, - ): Promise<T | null> { - return this.findOne<T>({ _id: roomId, t: type }, options); - } + findOneByIdAndType<T extends Document = IRoom>( + roomId: IRoom['_id'], + type: IRoom['t'], + options?: FindOptions<T>, + ): Promise<T | null> { + return this.findOne<T>({ _id: roomId, t: type }, options); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/model-typings/src/models/IRoomsModel.ts(1 hunks)packages/models/src/models/Rooms.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/models/src/models/Rooms.ts (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
packages/model-typings/src/models/IRoomsModel.ts (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: 📦 Build Packages
🔇 Additional comments (1)
packages/model-typings/src/models/IRoomsModel.ts (1)
207-207: Signature verified and correctly implemented.The interface and implementation signatures match exactly. The generic
T extends Document = IRoomis correctly propagated through to the internalfindOne<T>()call, and the call site atapps/meteor/app/otr/server/methods/updateOTRAck.ts:42compiles without errors. The pattern aligns with MongoDB driver conventions for typed projections.Optional enhancements:
- Add JSDoc for
Tto clarify it represents the projected document shape.- Consider applying this generic pattern to other similar
findOne*methods for consistency (currently,findOneByIdAndTypeis the only generic method in the interface).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37254 +/- ##
===========================================
- Coverage 67.64% 67.63% -0.02%
===========================================
Files 3341 3341
Lines 114016 114016
Branches 20671 20672 +1
===========================================
- Hits 77131 77116 -15
- Misses 34207 34224 +17
+ Partials 2678 2676 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Release Notes