feat: Add OpenAPI Support to e2e.getUsersOfRoomWithoutKey API#36786
Open
ahmed-n-abdeltwab wants to merge 1 commit intoRocketChat:developfrom
Open
feat: Add OpenAPI Support to e2e.getUsersOfRoomWithoutKey API#36786ahmed-n-abdeltwab wants to merge 1 commit intoRocketChat:developfrom
ahmed-n-abdeltwab wants to merge 1 commit intoRocketChat:developfrom
Conversation
Contributor
|
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 |
🦋 Changeset detectedLatest commit: 3b21ab3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36786 +/- ##
===========================================
- Coverage 70.58% 70.54% -0.05%
===========================================
Files 3178 3189 +11
Lines 111796 112703 +907
Branches 20213 20392 +179
===========================================
+ Hits 78914 79507 +593
- Misses 30831 31131 +300
- Partials 2051 2065 +14
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
Author
|
👍 @cardoso |
cardoso
previously approved these changes
Sep 8, 2025
Contributor
Author
|
Looks fine |
Contributor
WalkthroughParameter validation and AJV schemas for the e2e.getUsersOfRoomWithoutKey endpoint were moved from the shared typings package into the Meteor app; the app now defines local types, schemas, and validators and registers a new GET route while the shared rest-typings export and endpoint entry were removed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
…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.
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-e2e-getUsersOfRoomWithoutKey
branch
from
3664495 to
3b21ab3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/app/api/server/v1/e2e.ts (1)
264-295: Consider extracting a shared{rid}validator instead of reusing the GET-specific one.
ise2eGetUsersOfRoomWithoutKeyParamsGETis semantically named for a specific GET endpoint's query params, but it's used asvalidateParamsfor two unrelated POST routes. Extracting a generic validator (e.g.isRidParam) would remove the misleading name and make the intent of all three consumers clearer.♻️ Suggested refactor
+const isRidParam = ajv.compile<{ rid: string }>({ + type: 'object', + properties: { + rid: { type: 'string' }, + }, + additionalProperties: false, + required: ['rid'], +}); + -const ise2eGetUsersOfRoomWithoutKeyParamsGET = ajv.compile<e2eGetUsersOfRoomWithoutKeyParamsGET>( - e2eGetUsersOfRoomWithoutKeyParamsGETSchema, -); +const ise2eGetUsersOfRoomWithoutKeyParamsGET = ajv.compile<e2eGetUsersOfRoomWithoutKeyParamsGET>( + e2eGetUsersOfRoomWithoutKeyParamsGETSchema, +);Then replace
validateParams: ise2eGetUsersOfRoomWithoutKeyParamsGETwithvalidateParams: isRidParamat Lines 268 and 285.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/e2e.ts` around lines 264 - 295, The validator ise2eGetUsersOfRoomWithoutKeyParamsGET is misleadingly named for GET query params but is being reused in the POST routes e2e.acceptSuggestedGroupKey and e2e.rejectSuggestedGroupKey; extract a small generic validator (e.g., isRidParam) that validates the { rid } POST body param and replace validateParams: ise2eGetUsersOfRoomWithoutKeyParamsGET with validateParams: isRidParam in the handlers that call handleSuggestedGroupKey so the intent and naming are correct across all consumers.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/nice-squids-smoke.mdapps/meteor/app/api/server/v1/e2e.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/nice-squids-smoke.md
🧰 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:
apps/meteor/app/api/server/v1/e2e.ts
🧠 Learnings (4)
📓 Common learnings
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: 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: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
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: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/app/api/server/v1/e2e.ts
📚 Learning: 2025-09-25T09:59:26.461Z
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.
Applied to files:
apps/meteor/app/api/server/v1/e2e.ts
📚 Learning: 2025-09-25T09:59:26.461Z
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.
Applied to files:
apps/meteor/app/api/server/v1/e2e.ts
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/e2e.ts (2)
packages/rest-typings/src/v1/Ajv.ts (3)
ajv(24-24)validateBadRequestErrorResponse(47-47)validateUnauthorizedErrorResponse(70-70)apps/meteor/app/api/server/index.ts (1)
API(52-52)
⏰ 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). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/app/api/server/v1/e2e.ts (2)
139-141: No issues found. The method returns{ users: Pick<IUser, '_id' | 'e2e'>[] }as declared, which is the correct shape forAPI.v1.success(result)to produce{ users: [...], success: true }matching the declared 200 response schema.
81-87: This follows the established codebase pattern for response validators.The
ajv.compile<void>usage is consistent with 15+ existing instances across the codebase, including in rest-typings. Response validators are used for runtime schema validation in API response definitions, not as TypeScript type guards. The pattern is intentional and established, so this change aligns with existing conventions.Likely an incorrect or invalid review comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/meteor/app/api/server/v1/e2e.ts`:
- Around line 264-295: The validator ise2eGetUsersOfRoomWithoutKeyParamsGET is
misleadingly named for GET query params but is being reused in the POST routes
e2e.acceptSuggestedGroupKey and e2e.rejectSuggestedGroupKey; extract a small
generic validator (e.g., isRidParam) that validates the { rid } POST body param
and replace validateParams: ise2eGetUsersOfRoomWithoutKeyParamsGET with
validateParams: isRidParam in the handlers that call handleSuggestedGroupKey so
the intent and naming are correct across all consumers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
$Endpoints:
Looking forward to your feedback! 🚀
Summary by CodeRabbit
New Features
Refactor
Chores
Breaking Changes