chore: Add OpenAPI Support to e2e.fetchMyKeys API#36779
chore: Add OpenAPI Support to e2e.fetchMyKeys API#36779ggazzo merged 13 commits intoRocketChat:developfrom
Conversation
🦋 Changeset detectedLatest commit: 3f8e112 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 |
|
Looks like this PR is ready to merge! 🎉 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36779 +/- ##
===========================================
+ Coverage 70.65% 70.71% +0.05%
===========================================
Files 3190 3191 +1
Lines 112732 112968 +236
Branches 20431 20497 +66
===========================================
+ Hits 79649 79882 +233
- Misses 31035 31039 +4
+ Partials 2048 2047 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Hey @cardoso, feel free to review whenever you have time |
|
👍 @cardoso |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📜 Recent 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)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (11)📓 Common learnings📚 Learning: 2026-02-24T19:09:01.522ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2026-01-17T01:51:47.764ZApplied to files:
📚 Learning: 2026-01-26T18:26:01.279ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2026-02-12T15:39:28.416ZApplied to files:
📚 Learning: 2026-02-23T17:53:06.802ZApplied to files:
📚 Learning: 2025-11-19T12:32:29.696ZApplied to files:
📚 Learning: 2026-02-24T19:05:56.710ZApplied to files:
📚 Learning: 2026-02-24T19:05:56.710ZApplied to files:
🧬 Code graph analysis (1)apps/meteor/app/api/server/v1/e2e.ts (2)
🔇 Additional comments (1)
WalkthroughMigrates Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API Server
participant Users as Users Model/DB
Client->>API: GET /v1/e2e.fetchMyKeys (auth token)
API->>API: authenticate request
API->>Users: Users.fetchKeysByUserId(userId)
Users-->>API: { public_key, private_key } | or null
API-->>Client: 200 { public_key, private_key, success:true } / 401/400
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
|
@coderabbitai would it be okay if I bumped @rocket.chat/ui-contexts to 27.0.1? It seems to be updating to this version automatically. Would that be considered unrelated to this PR, and should I revert it to 27.0.0? Also, what are the best practices for this project when dealing with Yarn change? |
|
Tip For best results, initiate chat on the files or code changes. The PR description image could not be loaded in my environment (likely a private GitHub user-attachment). Proceeding based on the PR contents I can access.
What I’m seeing
Recommendation
Best practices for Yarn changes in this repo
If you’d like, I can outline exact git steps for either path (revert here vs. separate PR) based on how you want to proceed. (github.com) 🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
Short answer
What likely happened
What I found in this branch (from the script above)
Recommendation
Option B — Keep 27.0.1 but isolate it
Best practices for Yarn changes in this repo
If you’d like, tell me which option you prefer (revert here vs. quick follow-up chore PR), and I’ll outline exact git commands accordingly. 🏁 Script executed: Length of output: 10160
What we now know
Implications
Recommendation
Best practices for Yarn changes in this repo (reiterating maintainer guidance about tight scope, per our previous learning)
Would you like me to confirm whether packages/web-ui-registration/package.json was modified by this PR? If yes, I’ll run the quick diff below. I’m following the prior maintainer preference we learned (keep PRs narrowly scoped). Depending on the diff result above, I can outline exact git steps for either keeping this as a tiny chore commit or reverting it cleanly. 🏁 Script executed: Length of output: 458 The PR description image could not be loaded in my environment, and there’s no Comment image provided in this thread.
What your results show
Conclusion
Suggested next step
Yarn best practices for this repo
If you’d like, I can also scan for any other non-workspace-pinned references to ✏️ Learnings added
🧠 Learnings used |
Add OpenAPI support for e2e.fetchMyKeys endpoints by migrating to a modern route definition syntax and utilizing shared AJV schemas for validation.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 107-115: The 200 response schema compiled via ajv.compile<{
public_key: string; private_key: string }> currently only requires 'success',
allowing responses without keys; update the schema used in the ajv.compile call
so the required array includes 'public_key', 'private_key', and 'success' (i.e.,
required: ['public_key', 'private_key', 'success']) to enforce both key fields
are present in successful responses.
- Around line 121-123: The new check that calls API.v1.failure when 'public_key'
or 'private_key' are missing changes prior behavior; revert to returning the
model result directly instead of failing: remove the if-block that references
result and API.v1.failure and ensure the handler returns the original result
value (the variable result) as before; if you intend to change
contract/behavior, move this change into a separate behavior-change PR with
tests.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/short-starfishes-provide.mdapps/meteor/app/api/server/v1/e2e.tspackages/rest-typings/src/v1/e2e.ts
💤 Files with no reviewable changes (1)
- packages/rest-typings/src/v1/e2e.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: CodeQL-Build
🧰 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: 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.
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.
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.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 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/short-starfishes-provide.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/short-starfishes-provide.md
📚 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: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
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
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/e2e.ts (1)
apps/meteor/app/api/server/index.ts (1)
API(51-51)
🔇 Additional comments (1)
.changeset/short-starfishes-provide.md (1)
1-6: Changeset entry looks scoped and consistent with the migration.Minor bumps and summary text match the endpoint OpenAPI migration work in this PR.
|
@ggazzo 👍 |
|
/jira ARCH-1935 |
|
thanks |
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
Improvements
Task: ARCH-2011
[COMM-144]