chore: Add OpenAPI Support to dm.close/im.close API#38974
chore: Add OpenAPI Support to dm.close/im.close API#38974ggazzo merged 6 commits intoRocketChat:developfrom
Conversation
…endpoints by migrating to a modern chained route definition syntax and utilizing shared AJV schemas for validation to enhance API documentation and ensure type safety through response validation.
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: e09c8d1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR consolidates OpenAPI support for the dm.close/im.close endpoints by moving DmCloseProps type and schema definitions from the rest-typings package into the Meteor API layer, implements the close endpoint with validation and error handling, and updates the related im.kick endpoint to use its own distinct schema. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38974 +/- ##
===========================================
- Coverage 70.65% 70.63% -0.02%
===========================================
Files 3189 3189
Lines 112715 112716 +1
Branches 20409 20430 +21
===========================================
- Hits 79638 79622 -16
- Misses 31031 31044 +13
- Partials 2046 2050 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@ggazzo 👍 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/app/api/server/v1/im.ts (1)
103-166:⚠️ Potential issue | 🟠 MajorSchema/type mismatch:
userIdis required but unused
DmClosePropsonly includesroomId, yetDmClosePropsSchemarequiresuserId. This will cause AJV to reject requests that send onlyroomId, effectively making the change breaking and misaligning the OpenAPI docs with runtime behavior. Either dropuserIdfrom the schema or wire it into the handler with explicit permission checks.💡 Proposed fix (drop unused userId)
const DmClosePropsSchema = { type: 'object', properties: { roomId: { type: 'string', }, - userId: { - type: 'string', - }, }, - required: ['roomId', 'userId'], + required: ['roomId'], additionalProperties: false, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/im.ts` around lines 103 - 166, DmClosePropsSchema requires userId but the TypeScript type DmCloseProps (and the handler) only expects roomId, causing AJV to reject valid requests; fix by either removing userId from DmClosePropsSchema (delete the userId property and from required) so schema matches the DmCloseProps type and isDmCloseProps validator, or if you intend to accept userId, add userId to the DmCloseProps type and update the handler that uses isDmCloseProps to consume userId and perform the necessary permission checks before closing the DM (ensure isDmCloseProps and DmClosePropsSchema remain consistent with the handler).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 220-221: Remove the inline explanatory comment before the
subscription lookup and make the intent self-documenting: either delete the
comment and keep the call to Subscriptions.findOneByRoomIdAndUserId(roomId,
this.userId), or extract the logic into a clearly named helper (e.g.,
getSubscriptionAllowingMissingRoom(roomId, userId)) and call that helper
instead; ensure references to subscription, roomId and this.userId remain
correct and no inline comment is left in the implementation.
---
Outside diff comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 103-166: DmClosePropsSchema requires userId but the TypeScript
type DmCloseProps (and the handler) only expects roomId, causing AJV to reject
valid requests; fix by either removing userId from DmClosePropsSchema (delete
the userId property and from required) so schema matches the DmCloseProps type
and isDmCloseProps validator, or if you intend to accept userId, add userId to
the DmCloseProps type and update the handler that uses isDmCloseProps to consume
userId and perform the necessary permission checks before closing the DM (ensure
isDmCloseProps and DmClosePropsSchema remain consistent with the handler).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
.changeset/wicked-buckets-thank.mdapps/meteor/app/api/server/v1/im.tspackages/rest-typings/src/v1/dm/DmCloseProps.tspackages/rest-typings/src/v1/dm/dm.tspackages/rest-typings/src/v1/dm/im.ts
💤 Files with no reviewable changes (2)
- packages/rest-typings/src/v1/dm/dm.ts
- packages/rest-typings/src/v1/dm/DmCloseProps.ts
📜 Review details
⏰ 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: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/rest-typings/src/v1/dm/im.tsapps/meteor/app/api/server/v1/im.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Applied to files:
packages/rest-typings/src/v1/dm/im.tsapps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
🧬 Code graph analysis (1)
packages/rest-typings/src/v1/dm/im.ts (2)
packages/livechat/src/lib/room.js (1)
params(304-304)packages/rest-typings/src/v1/dm/DmCreateProps.ts (1)
DmCreateProps(3-10)
🔇 Additional comments (4)
.changeset/wicked-buckets-thank.md (1)
1-6: Changeset looks accurate and well-scoped.
The patch bumps and description align with the OpenAPI support work described in the PR.packages/rest-typings/src/v1/dm/im.ts (1)
12-24: im.kick request typing looks consistent
DmKickPropskeeps the request shape explicit and narrowsim.kickto the intendedroomId.apps/meteor/app/api/server/v1/im.ts (2)
6-10: Good: 403 response validation added for dm.close/im.closeIncluding a forbidden response schema improves OpenAPI documentation and runtime validation for authorization failures.
Also applies to: 168-187
246-250: Chained dm.close/im.close routes look consistent with the OpenAPI patternThe new route chaining keeps the public API surface aligned with the new validation/action setup.
| // even if the room doesn't exist, we should allow the user to close the subscription anyways | ||
| subscription = await Subscriptions.findOneByRoomIdAndUserId(roomId, this.userId); |
There was a problem hiding this comment.
Remove the inline comment to comply with implementation guidelines
Prefer self-explanatory logic or extract a helper name instead of leaving an inline explanation.
🔧 Proposed cleanup
- // even if the room doesn't exist, we should allow the user to close the subscription anywaysAs per coding guidelines: Avoid code comments in the implementation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // even if the room doesn't exist, we should allow the user to close the subscription anyways | |
| subscription = await Subscriptions.findOneByRoomIdAndUserId(roomId, this.userId); | |
| subscription = await Subscriptions.findOneByRoomIdAndUserId(roomId, this.userId); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/im.ts` around lines 220 - 221, Remove the
inline explanatory comment before the subscription lookup and make the intent
self-documenting: either delete the comment and keep the call to
Subscriptions.findOneByRoomIdAndUserId(roomId, this.userId), or extract the
logic into a clearly named helper (e.g.,
getSubscriptionAllowingMissingRoom(roomId, userId)) and call that helper
instead; ensure references to subscription, roomId and this.userId remain
correct and no inline comment is left in the implementation.
There was a problem hiding this comment.
is change the old code logic is out of scoop of this pr
There was a problem hiding this comment.
No PR description or comment images were provided in the context to describe.
@ahmed-n-abdeltwab Understood—no logic changes in this PR. My earlier note was purely about removing a single inline comment and would not alter behavior. Given your scope constraint, I’m happy to drop that nit and keep the code as-is. If you’d like, we can track a tiny follow-up chore later to replace such inline comments with clearer helper names, but no change is requested here.
For transparency: I’m keeping this review tightly focused on the dm.close/im.close handlers per our prior learning about limiting scope in v1 endpoint PRs.
✏️ Learnings added
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
2 participants
Description:
This PR integrates OpenAPI support into the
Rocket.Chat API, migrate ofRocket.Chat APIendpoints to the new OpenAPI pattern. The update includes improved API documentation, enhanced type safety, and response validation using AJV.Key Changes:
Issue Reference:
Relates to #34983, part of the ongoing OpenAPI integration effort.
Testing:
Verified that the API response schemas are correctly documented in Swagger UI.
All tests passed without any breaking changes
Endpoints:
Close DM
Looking forward to your feedback! 🚀
Summary by CodeRabbit
Release Notes