Skip to content

chore: migrate chat.reportMessage endpoint to new OpenAPI pattern with AJV validation #39230

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.

Sign up for GitHub

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

Closed
alchemycodess wants to merge 7 commits into from
Closed

chore: migrate chat.reportMessage endpoint to new OpenAPI pattern with AJV validation #39230

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6d531c1
chore: migrate chat.reportMessage endpoint to new OpenAPI pattern wit…
alchemycodess Mar 1, 2026
f2c43fd
fix: add minLength validation for messageId and description
alchemycodess Mar 2, 2026
4a7717c
fix: resolve merge conflicts and address review feedback
alchemycodess Mar 3, 2026
4c4ed70
fix: apply eslint fixes
alchemycodess Mar 3, 2026
a44e004
Revert "fix: apply eslint fixes"
alchemycodess Mar 4, 2026
aecf048
fix: apply eslint fixes to migrated files only
alchemycodess Mar 4, 2026
ee3537c
Merge remote-tracking branch 'upstream/develop' into chore/migrate-ch…
alchemycodess Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/migrate-chat-reportMessage-to-openapi.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@rocket.chat/meteor': minor
'@rocket.chat/rest-typings': minor
---

Migrated chat.reportMessage endpoint to new OpenAPI pattern with AJV validation
63 changes: 41 additions & 22 deletions apps/meteor/app/api/server/v1/chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { MessageTypes } from '@rocket.chat/message-types';
import { Messages, Users, Rooms, Subscriptions } from '@rocket.chat/models';
import {
ajv,
isChatReportMessageProps,
isChatGetURLPreviewProps,
isChatUpdateProps,
isChatGetThreadsListProps,
Expand Down Expand Up @@ -207,6 +206,23 @@ const isChatPinMessageProps = ajv.compile<ChatPinMessage>(ChatPinMessageSchema);

const isChatUnpinMessageProps = ajv.compile<ChatUnpinMessage>(ChatUnpinMessageSchema);

type ChatReportMessage = {
messageId: string;
Copy link
Copy Markdown
Contributor

@ahmed-n-abdeltwab ahmed-n-abdeltwab Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was IMessage['_id'] in rest-typings. I know it's technically just a string, but we should keep the code consistent with how it was defined in rest-typings

description: string;
};

const ChatReportMessageSchema = {
type: 'object',
properties: {
messageId: { type: 'string', minLength: 1 },
description: { type: 'string', minLength: 1 },
},
required: ['messageId', 'description'],
additionalProperties: false,
};
Comment on lines +282 to +290
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Preserve non-empty validation for messageId and description.

Line 217 and Line 218 allow empty strings, so { "messageId": "", "description": "" } passes schema validation. That relaxes input checks during a migration that should keep behavior unchanged.

Proposed minimal fix 
 const ChatReportMessageSchema = {
 	type: 'object',
 	properties: {
-		messageId: { type: 'string' },
-		description: { type: 'string' },
+		messageId: { type: 'string', minLength: 1 },
+		description: { type: 'string', minLength: 1 },
 	},
 	required: ['messageId', 'description'],
 	additionalProperties: false,
 };

Based on learnings: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes and keep behavior preserved.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const ChatReportMessageSchema = {
type: 'object',
properties: {
messageId: { type: 'string' },
description: { type: 'string' },
},
required: ['messageId', 'description'],
additionalProperties: false,
};
const ChatReportMessageSchema = {
type: 'object',
properties: {
messageId: { type: 'string', minLength: 1 },
description: { type: 'string', minLength: 1 },
},
required: ['messageId', 'description'],
additionalProperties: false,
};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/api/server/v1/chat.ts` around lines 214 - 222, The schema
ChatReportMessageSchema currently allows empty strings for messageId and
description; update the properties for "messageId" and "description" to require
non-empty values (e.g., add "minLength": 1 or a non-empty pattern) so that
inputs like { "messageId": "", "description": "" } fail validation; ensure you
modify the ChatReportMessageSchema object's properties for messageId and
description only, preserving required and additionalProperties settings.

✅ Addressed in commit f2c43fd

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for the reportMessage function implementation to check if it validates empty strings
ast-grep --pattern 'function reportMessage($$$) { $$$ }'

# Also search with async/export patterns
ast-grep --pattern 'export async function reportMessage($$$) { $$$ }'
ast-grep --pattern 'export const reportMessage = ($$$) => { $$$ }'

# Use rg to find the function definition and see surrounding context
rg -A 20 'export.*reportMessage.*=|function reportMessage' --type ts

Length of output: 1546

@ahmed-n-abdeltwab Actually, adding minLength validation is in scope for this migration! Here's why:

The original reportMessage function (which the legacy endpoint called) validates the description:

if (!description.trim()) {
    throw new Error('error-invalid-description');
}

So the legacy endpoint did reject empty descriptions — it just happened later in the call chain rather than at schema validation. Adding minLength: 1 preserves this existing behavior by moving the validation earlier (to the schema layer), which is the correct approach for OpenAPI migrations.

Result: Empty strings were already being rejected before your PR, so adding minLength: 1 maintains the original behavior rather than introducing a new restriction. This aligns with keeping the migration scope tight and behavior-preserving. ✅

🐰

✏️ Learnings added 
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:45.505Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 39003
File: apps/meteor/client/lib/chats/flows/sendMessage.ts:39-45
Timestamp: 2026-02-24T19:16:35.307Z
Learning: In apps/meteor/client/lib/chats/flows/sendMessage.ts, when sdk.call('sendMessage', ...) throws an error, the message is intentionally left with temp: true (not deleted or cleaned up) to support a future retry UI feature. This allows users to retry sending failed messages rather than losing them.

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.

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.

Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').

Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.

Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.

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.

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: 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.

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.


const isChatReportMessageLocalProps = ajv.compile<ChatReportMessage>(ChatReportMessageSchema);

const chatEndpoints = API.v1
.post(
'chat.pinMessage',
Expand Down Expand Up @@ -350,6 +366,30 @@ const chatEndpoints = API.v1
message,
});
},
)
.post(
'chat.reportMessage',
{
authRequired: true,
body: isChatReportMessageLocalProps,
response: {
200: ajv.compile<void>({
type: 'object',
properties: {
success: { type: 'boolean', enum: [true] }
},
required: ['success'],
additionalProperties: false
}),
400: validateBadRequestErrorResponse,
401: validateUnauthorizedErrorResponse,
},
},
async function action() {
const { messageId, description } = this.bodyParams;
await reportMessage(messageId, description, this.userId);
return API.v1.success();
},
Comment on lines +452 to +460
Copy link
Copy Markdown
Contributor

@ahmed-n-abdeltwab ahmed-n-abdeltwab Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have removed the old checks for messageId and description because they are now handled by the body schema before entering the action function. That makes sense, nice

);

API.v1.addRoute(
Expand Down Expand Up @@ -513,27 +553,6 @@ API.v1.addRoute(
},
);

API.v1.addRoute(
'chat.reportMessage',
{ authRequired: true, validateParams: isChatReportMessageProps },
{
async post() {
const { messageId, description } = this.bodyParams;
if (!messageId) {
return API.v1.failure('The required "messageId" param is missing.');
}

if (!description) {
return API.v1.failure('The required "description" param is missing.');
}

await reportMessage(messageId, description, this.userId);

return API.v1.success();
},
},
);

API.v1.addRoute(
'chat.ignoreUser',
{ authRequired: true, validateParams: isChatIgnoreUserProps },
Expand Down
3 changes: 0 additions & 3 deletions packages/rest-typings/src/v1/chat.ts
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@ahmed-n-abdeltwab ahmed-n-abdeltwab Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You moved the JSON schema, Ajv validation, and interface from rest-typings to chat.ts, but you should also remove the chat.reportMessage related code (the schema, Ajv, and interface) from rest-typings. Since it’s no longer needed there and we are making Meteor the single source of truth for the API, we need to ensure everything is moved to the Meteor file and fully removed from rest-typings to avoid redundancy.

Original file line number Diff line number Diff line change
Expand Up @@ -979,9 +979,6 @@ export type ChatEndpoints = {
'/v1/chat.unStarMessage': {
POST: (params: ChatUnstarMessage) => void;
};
'/v1/chat.reportMessage': {
POST: (params: ChatReportMessage) => void;
};
'/v1/chat.getDiscussions': {
GET: (params: ChatGetDiscussions) => {
messages: IMessage[];
Expand Down