feat: Add OpenAPI Support to subscriptions.getOne API#37085
feat: Add OpenAPI Support to subscriptions.getOne API#37085ahmed-n-abdeltwab wants to merge 1 commit intoRocketChat:developfrom
Conversation
🦋 Changeset detectedLatest commit: b216daa 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 |
|
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 |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a new server-side Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as v1/subscriptions.getOne
participant Auth as AuthN/AuthZ
participant AJV as AJV Validator
participant Service as Subscriptions Service
Client->>API: GET /v1/subscriptions.getOne?roomId=...
API->>Auth: verify session/user
alt unauthorized
Auth-->>API: 401
API-->>Client: 401 { success:false, errorType }
else authorized
API->>AJV: validate query (roomId)
alt invalid params
AJV-->>API: 400 { error, errorType }
API-->>Client: 400 { success:false, error, errorType }
else valid
API->>Service: fetch subscription for user by roomId
Service-->>API: subscription | null
API->>AJV: validate 200 response schema
API-->>Client: 200 { success:true, subscription|null }
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37085 +/- ##
===========================================
- Coverage 70.49% 70.47% -0.02%
===========================================
Files 3175 3175
Lines 111094 111094
Branches 20045 20055 +10
===========================================
- Hits 78311 78291 -20
- Misses 30738 30754 +16
- Partials 2045 2049 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![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
📜 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 (4)
.changeset/flat-candles-walk.md(1 hunks)apps/meteor/app/api/server/v1/subscriptions.ts(4 hunks)apps/meteor/tests/end-to-end/api/subscriptions.ts(1 hunks)packages/rest-typings/src/v1/subscriptionsEndpoints.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/rest-typings/src/v1/subscriptionsEndpoints.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/subscriptions.ts (4)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/rest-typings/src/v1/Ajv.ts (3)
ajv(23-23)validateBadRequestErrorResponse(46-46)validateUnauthorizedErrorResponse(69-69)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(73-77)packages/rest-typings/src/index.ts (1)
Endpoints(51-98)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (1)
apps/meteor/app/api/server/v1/subscriptions.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/subscriptions.ts (4)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/rest-typings/src/v1/Ajv.ts (3)
ajv(23-23)validateBadRequestErrorResponse(46-46)validateUnauthorizedErrorResponse(69-69)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(73-77)packages/rest-typings/src/index.ts (1)
Endpoints(51-98)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: Builds matrix rust bindings against alpine
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
apps/meteor/app/api/server/v1/subscriptions.ts (4)
1-16: LGTM! Imports align with the OpenAPI pattern.The new imports support AJV validation, response schemas, and route type extraction, which are all necessary for the OpenAPI integration.
51-65: LGTM! Query validation properly constrains roomId.The
minLength: 1constraint correctly prevents empty strings, and the runtime check has been appropriately removed since the schema now enforces non-empty values. This resolves the previous review discussion.
67-74: LGTM! Endpoint follows the OpenAPI pattern correctly.The endpoint structure is clean:
- Query validation is declarative with the compiled AJV validator
- Pre-compiled error response validators improve performance
- Action function is straightforward with no redundant runtime checks
Also applies to: 199-208
256-261: LGTM! Module augmentation correctly exposes the endpoint types.The type extraction and module augmentation follow the OpenAPI integration pattern, making the
subscriptions.getOneroute types available throughout the codebase.
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-subscriptions-getOne
branch
from
8cf6dad to
63b6b62
Compare
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-subscriptions-getOne
branch
3 times, most recently
from
9f3a7b1 to
f94d258
Compare
Key Changes: - Implemented the new pattern and added AJV-based JSON schema validation for API. - Uses the ExtractRoutesFromAPI utility from the TypeScript definitions to dynamically derive the routes from the endpoint specifications. - Enabled Swagger UI integration for this API. - Route Methods Chaining for the endpoints. - This does not introduce any breaking changes to the endpoint logic.
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-subscriptions-getOne
branch
from
f94d258 to
b216daa
Compare
|
Time Wasted: 3 Days Note to Future Me: > The issue isn't within subscriptions.getOne. The API is correctly detecting the schema violation. The Problem > Direct Message (DM) subscriptions are missing the lr (last reply) field, triggering API validation failures. The Returned from DB :
Places that i have been there:
for now i leave this PR and come later to fix it |
1 participant
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:
Looking forward to your feedback! 🚀
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests