-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: Add OpenAPI Support to dm.close/im.close API #36695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
644e97a
5d51164
4c5749e
65bdf26
7ea57b9
7ca2769
6a47509
6e8548e
fe7cae5
bc9b5b0
0263fc9
2225cb7
cd2e89c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@rocket.chat/meteor": patch | ||
| "@rocket.chat/rest-typings": patch | ||
| --- | ||
|
|
||
| Add OpenAPI support for the Rocket.Chat dm.close/im.close API 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,6 +99,21 @@ type DmDeleteProps = | |
| username: string; | ||
| }; | ||
|
|
||
| type DmCloseProps = { | ||
| roomId: string; | ||
| userId: string; | ||
| }; | ||
|
|
||
| const DmClosePropsSchema = { | ||
| type: 'object', | ||
| properties: { | ||
| roomId: { type: 'string' }, | ||
| userId: { type: 'string' }, | ||
| }, | ||
| required: ['roomId', 'userId'], | ||
| additionalProperties: false, | ||
| }; | ||
|
|
||
| const isDmDeleteProps = ajv.compile<DmDeleteProps>({ | ||
| oneOf: [ | ||
| { | ||
|
|
@@ -124,6 +139,8 @@ const isDmDeleteProps = ajv.compile<DmDeleteProps>({ | |
| ], | ||
| }); | ||
|
|
||
| const isDmCloseProps = ajv.compile<DmCloseProps>(DmClosePropsSchema); | ||
|
|
||
| const dmDeleteEndpointsProps = { | ||
| authRequired: true, | ||
| body: isDmDeleteProps, | ||
|
|
@@ -144,6 +161,41 @@ const dmDeleteEndpointsProps = { | |
| }, | ||
| } as const; | ||
|
|
||
| const dmCloseEndpointsProps = { | ||
| authRequired: true, | ||
| body: isDmCloseProps, | ||
| response: { | ||
| 400: validateBadRequestErrorResponse, | ||
| 401: validateUnauthorizedErrorResponse, | ||
| // TODO: The 403 Forbidden response is not handled as well as 400 responses. | ||
| // Currently using `never` as a placeholder type. Replace it with the correct | ||
| // schema once proper 403 error handling is implemented. | ||
| 403: ajv.compile<never>({ | ||
| type: 'object', | ||
| properties: { | ||
| success: { type: 'boolean', enum: [false] }, | ||
| status: { type: 'string' }, | ||
| message: { type: 'string' }, | ||
| error: { type: 'string' }, | ||
| errorType: { type: 'string' }, | ||
| }, | ||
| required: ['success'], | ||
| additionalProperties: false, | ||
| }), | ||
| 200: ajv.compile<void>({ | ||
| type: 'object', | ||
| properties: { | ||
| success: { | ||
| type: 'boolean', | ||
| enum: [true], | ||
| }, | ||
| }, | ||
| required: ['success'], | ||
| additionalProperties: false, | ||
| }), | ||
| }, | ||
| } as const; | ||
|
|
||
| const dmDeleteAction = <Path extends string>(_path: Path): TypedAction<typeof dmDeleteEndpointsProps, Path> => | ||
| async function action() { | ||
| const { room } = await findDirectMessageRoom(this.bodyParams, this.userId); | ||
|
|
@@ -160,51 +212,48 @@ const dmDeleteAction = <Path extends string>(_path: Path): TypedAction<typeof dm | |
| return API.v1.success(); | ||
| }; | ||
|
|
||
| const dmEndpoints = API.v1 | ||
| .post('im.delete', dmDeleteEndpointsProps, dmDeleteAction('im.delete')) | ||
| .post('dm.delete', dmDeleteEndpointsProps, dmDeleteAction('dm.delete')); | ||
| const dmCloseAction = <Path extends string>(_path: Path): TypedAction<typeof dmCloseEndpointsProps, Path> => | ||
| async function action() { | ||
| const { roomId } = this.bodyParams; | ||
| if (!roomId) { | ||
| throw new Meteor.Error('error-room-param-not-provided', 'Body param "roomId" is required'); | ||
| } | ||
|
|
||
| API.v1.addRoute( | ||
| ['dm.close', 'im.close'], | ||
| { authRequired: true }, | ||
| { | ||
| async post() { | ||
| const { roomId } = this.bodyParams; | ||
| if (!roomId) { | ||
| throw new Meteor.Error('error-room-param-not-provided', 'Body param "roomId" is required'); | ||
| let subscription; | ||
|
|
||
| const roomExists = !!(await Rooms.findOneById(roomId)); | ||
| if (!roomExists) { | ||
| // even if the room doesn't exist, we should allow the user to close the subscription anyways | ||
| subscription = await Subscriptions.findOneByRoomIdAndUserId(roomId, this.userId); | ||
| } else { | ||
| const canAccess = await canAccessRoomIdAsync(roomId, this.userId); | ||
| if (!canAccess) { | ||
| return API.v1.forbidden(); | ||
| } | ||
|
|
||
| let subscription; | ||
| const { subscription: subs } = await findDirectMessageRoom({ roomId }, this.userId); | ||
|
|
||
| const roomExists = !!(await Rooms.findOneById(roomId)); | ||
| if (!roomExists) { | ||
| // even if the room doesn't exist, we should allow the user to close the subscription anyways | ||
| subscription = await Subscriptions.findOneByRoomIdAndUserId(roomId, this.userId); | ||
| } else { | ||
| const canAccess = await canAccessRoomIdAsync(roomId, this.userId); | ||
| if (!canAccess) { | ||
| return API.v1.forbidden(); | ||
| } | ||
|
|
||
| const { subscription: subs } = await findDirectMessageRoom({ roomId }, this.userId); | ||
| subscription = subs; | ||
| } | ||
|
|
||
| subscription = subs; | ||
| } | ||
| if (!subscription) { | ||
| return API.v1.failure(`The user is not subscribed to the room`); | ||
| } | ||
|
|
||
| if (!subscription) { | ||
| return API.v1.failure(`The user is not subscribed to the room`); | ||
| } | ||
| if (!subscription.open) { | ||
| return API.v1.failure(`The direct message room, is already closed to the sender`); | ||
| } | ||
|
|
||
| if (!subscription.open) { | ||
| return API.v1.failure(`The direct message room, is already closed to the sender`); | ||
| } | ||
| await hideRoomMethod(this.userId, roomId); | ||
|
|
||
| await hideRoomMethod(this.userId, roomId); | ||
| return API.v1.success(); | ||
| }; | ||
|
|
||
|
Comment on lines
+215
to
251
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logic issue: userId parameter is never used in dmCloseAction. The Either:
type DmCloseProps = {
roomId: string;
- userId: string;
};
|
||
| return API.v1.success(); | ||
| }, | ||
| }, | ||
| ); | ||
| const dmEndpoints = API.v1 | ||
| .post('im.delete', dmDeleteEndpointsProps, dmDeleteAction('im.delete')) | ||
| .post('dm.delete', dmDeleteEndpointsProps, dmDeleteAction('dm.delete')) | ||
| .post('dm.close', dmCloseEndpointsProps, dmCloseAction('dm.close')) | ||
| .post('im.close', dmCloseEndpointsProps, dmCloseAction('im.close')); | ||
|
|
||
| // https://github.com/RocketChat/Rocket.Chat/pull/9679 as reference | ||
| API.v1.addRoute( | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Analysis chain
Replace placeholder 403 response with proper ForbiddenErrorResponse type.
The comment indicates this is a placeholder implementation. Since
ForbiddenErrorResponseis now exported from rest-typings, use it instead of thenevertype with a manual schema.Apply this fix to use the proper type:
🌐 Web query:
💡 Result:
Short answer
Why
Minimal examples
components:
schemas:
ErrorResponse:
type: object
properties:
code:
type: string
message:
type: string
required: [code, message]
paths:
/items/{id}:
get:
responses:
'200':
description: OK
content:
application/json:
schema:
$ref: '#/components/schemas/Item'
'403':
description: Forbidden
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorResponse'
default:
description: Other errors
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorResponse'
This makes sure a 403 response is validated only against the 403 ErrorResponse schema. [3][5]
Practical notes / gotchas
References
If you want, I can: (a) inspect your OpenAPI file and point out missing 403/default entries, or (b) produce a small TypeScript/AJV code sample wired to your generated schemas.
🏁 Script executed:
Length of output: 227718
Replace inline 403 placeholder with exported validateForbiddenErrorResponse
Import validateForbiddenErrorResponse from '@rocket.chat/rest-typings' and replace the ajv.compile 403 block in apps/meteor/app/api/server/v1/im.ts (lines ~173–184) with:
403: validateForbiddenErrorResponse,
🤖 Prompt for AI Agents