chore: Add OpenAPI support for the e2e.setUserPublicAndPrivateKeys#39090
Open
ahmed-n-abdeltwab wants to merge 3 commits intoRocketChat:developfrom
Open
chore: Add OpenAPI support for the e2e.setUserPublicAndPrivateKeys#39090ahmed-n-abdeltwab wants to merge 3 commits intoRocketChat:developfrom
ahmed-n-abdeltwab wants to merge 3 commits 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: 77624d7 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 |
Contributor
WalkthroughValidation for the E2E setUserPublicAndPrivateKeys route was relocated from the shared rest-typings package into the local API implementation; shared exported types/validators were removed and the route was implemented as an action-style handler with local AJV schema and validator. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API.v1 Route
participant Validator as AJV Validator
participant E2E as E2E Service
Client->>API: POST /v1/e2e.setUserPublicAndPrivateKeys (payload)
API->>Validator: validate(payload)
Validator-->>API: valid / invalid
alt valid
API->>E2E: set keys (public_key, private_key, force)
E2E-->>API: result
API-->>Client: 200 OK (result)
else invalid
API-->>Client: 400 Bad Request (validation error)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 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 |
ahmed-n-abdeltwab
changed the title
ahmed-n-abdeltwab
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #39090 +/- ##
===========================================
+ Coverage 70.85% 70.91% +0.06%
===========================================
Files 3208 3208
Lines 113426 113426
Branches 20489 20487 -2
===========================================
+ Hits 80363 80437 +74
+ Misses 31012 30946 -66
+ Partials 2051 2043 -8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…ivateKeys - 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-setUserPublicAndPrivateKeys
branch
from
9045610 to
742992b
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)
.changeset/quiet-moles-remain.md (1)
6-6: Clarify the changeset wording around AJV schema location.At Line 6, “utilizing shared AJV schemas” looks inaccurate for this PR context, since the validator/schema appears to have been localized to the Meteor endpoint implementation. Please update the sentence so the release note matches the actual migration behavior and avoids confusion for package consumers.
Based on learnings, OpenAPI migration PRs in
apps/meteor/app/api/server/v1should keep scope and behavior messaging tight, with no implied logic changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/quiet-moles-remain.md at line 6, Update the changeset wording to remove the implication of shared schemas: replace the phrase "utilizing shared AJV schemas" with wording that reflects the validator/schema was localized to the Meteor endpoint implementation (e.g., "using a local AJV schema in the Meteor endpoint"), and ensure the sentence emphasizes that this is a documentation/route-definition migration only (no logic or scope changes in apps/meteor/app/api/server/v1).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.changeset/quiet-moles-remain.md:
- Line 6: Update the changeset wording to remove the implication of shared
schemas: replace the phrase "utilizing shared AJV schemas" with wording that
reflects the validator/schema was localized to the Meteor endpoint
implementation (e.g., "using a local AJV schema in the Meteor endpoint"), and
ensure the sentence emphasizes that this is a documentation/route-definition
migration only (no logic or scope changes in apps/meteor/app/api/server/v1).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/quiet-moles-remain.mdapps/meteor/app/api/server/v1/e2e.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/app/api/server/v1/e2e.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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:09.561Z
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.
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:18.785Z
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: 2026-02-24T19:09:09.561Z
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:09.561Z
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.
Applied to files:
.changeset/quiet-moles-remain.md
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
.changeset/quiet-moles-remain.md
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
.changeset/quiet-moles-remain.md
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
.changeset/quiet-moles-remain.md
Contributor
Author
|
@ggazzo 👍 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/e2e.ts`:
- Around line 311-341: Remove the duplicate endpoint registration for
"e2e.setUserPublicAndPrivateKeys": delete the earlier POST block (the one that
calls setUserPublicAndPrivateKeysMethod with this.bodyParams { public_key,
private_key, force } and returns API.v1.success()) and keep the later, more
complete registration (the second POST with the stricter 200 response schema
including additionalProperties: false); ensure only one registration of
e2e.setUserPublicAndPrivateKeys remains and that it still invokes
setUserPublicAndPrivateKeysMethod(this.userId, { public_key, private_key, force
}) from the retained action handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0963d490-9d02-407f-93f5-4a40addc7145
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/e2e.ts
📜 Review details
🧰 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 (6)
📓 Common learnings
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:09.561Z
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.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
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:18.785Z
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: 2026-02-24T19:09:01.522Z
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 Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/e2e.ts
📚 Learning: 2026-03-10T08:13:52.153Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
Applied to files:
apps/meteor/app/api/server/v1/e2e.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/e2e.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/app/api/server/v1/e2e.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/app/api/server/v1/e2e.ts
🔇 Additional comments (3)
apps/meteor/app/api/server/v1/e2e.ts (3)
55-59: LGTM!The type definition correctly mirrors the expected parameters for the endpoint with
public_key,private_key, and optionalforceboolean.
164-179: LGTM!The JSON schema correctly defines the validation rules matching the type definition, with
public_keyandprivate_keyas required strings andforceas an optional boolean. TheadditionalProperties: falseensures strict validation.
195-197: LGTM!The AJV validator is correctly compiled from the schema with the appropriate type parameter.
Comment on lines
+311
to
+341
| .post( | ||
| 'e2e.setUserPublicAndPrivateKeys', | ||
| { | ||
| authRequired: true, | ||
| body: ise2eSetUserPublicAndPrivateKeysParamsPOST, | ||
| response: { | ||
| 400: validateBadRequestErrorResponse, | ||
| 401: validateUnauthorizedErrorResponse, | ||
| 200: ajv.compile<void>({ | ||
| type: 'object', | ||
| properties: { | ||
| success: { type: 'boolean', enum: [true] }, | ||
| }, | ||
| required: ['success'], | ||
| }), | ||
| }, | ||
| }, | ||
|
|
||
| async function action() { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| const { public_key, private_key, force } = this.bodyParams; | ||
|
|
||
| await setUserPublicAndPrivateKeysMethod(this.userId, { | ||
| public_key, | ||
| private_key, | ||
| force, | ||
| }); | ||
|
|
||
| return API.v1.success(); | ||
| }, | ||
| ) |
Contributor
There was a problem hiding this comment.
Duplicate endpoint definition for e2e.setUserPublicAndPrivateKeys.
The endpoint e2e.setUserPublicAndPrivateKeys is registered twice in the chain—once at lines 311-341 and again at lines 500-528. This will cause unexpected behavior as only one registration can be effective.
Remove one of the duplicate definitions. The second definition (lines 500-528) has a slightly more complete 200 response schema with additionalProperties: false, so consider keeping that one and removing the first.
🐛 Proposed fix: Remove the first duplicate (lines 311-341)
)
- .post(
- 'e2e.setUserPublicAndPrivateKeys',
- {
- authRequired: true,
- body: ise2eSetUserPublicAndPrivateKeysParamsPOST,
- response: {
- 400: validateBadRequestErrorResponse,
- 401: validateUnauthorizedErrorResponse,
- 200: ajv.compile<void>({
- type: 'object',
- properties: {
- success: { type: 'boolean', enum: [true] },
- },
- required: ['success'],
- }),
- },
- },
-
- async function action() {
- // eslint-disable-next-line `@typescript-eslint/naming-convention`
- const { public_key, private_key, force } = this.bodyParams;
-
- await setUserPublicAndPrivateKeysMethod(this.userId, {
- public_key,
- private_key,
- force,
- });
-
- return API.v1.success();
- },
- )
.post(
'e2e.acceptSuggestedGroupKey',Also applies to: 500-528
🤖 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 311 - 341, Remove the
duplicate endpoint registration for "e2e.setUserPublicAndPrivateKeys": delete
the earlier POST block (the one that calls setUserPublicAndPrivateKeysMethod
with this.bodyParams { public_key, private_key, force } and returns
API.v1.success()) and keep the later, more complete registration (the second
POST with the stricter 200 response schema including additionalProperties:
false); ensure only one registration of e2e.setUserPublicAndPrivateKeys remains
and that it still invokes setUserPublicAndPrivateKeysMethod(this.userId, {
public_key, private_key, force }) from the retained action handler.
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:
Looking forward to your feedback! 🚀
Summary by CodeRabbit
Refactor
Chores