chore: Add OpenAPI Support to chat.getMessage API#36819
chore: Add OpenAPI Support to chat.getMessage API#36819ahmed-n-abdeltwab wants to merge 12 commits intoRocketChat:developfrom
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 765cd11 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 |
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-chat-getMessage
branch
2 times, most recently
from
48bff18 to
5400b11
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36819 +/- ##
===========================================
+ Coverage 70.61% 70.63% +0.01%
===========================================
Files 3189 3189
Lines 112717 112715 -2
Branches 20401 20425 +24
===========================================
+ Hits 79597 79611 +14
+ Misses 31074 31061 -13
+ Partials 2046 2043 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Thanks for the detailed OpenAPI migration for |
|
Hi @Kaustubh1204 , Thanks for the review! I definitely haven't given up on this one, just been a bit tied up. If you're still down to help with the merge conflicts and testing on CE/EE, that would be awesome and much appreciated! It would definitely help get this over the finish line. Also, just for context, I've been using this PR as a guide for the migration track and project docs: RocketChat/Rocket.Chat-Open-API#150 It shows the other APIs I'm working on and how everything is structured. Let me know if you want to jump in, and thanks again for reaching out! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRelocates and redefines the chat.getMessage GET endpoint into the Meteor app with AJV query validation and OpenAPI-style response schema, removes the corresponding typings and endpoint declaration from rest-typings, and normalizes message attachment Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Validator as AJV\ Validator
participant DB as Database
participant Normalizer
Client->>Server: GET /api/v1/chat.getMessage?msgId=...
Server->>Validator: validate query (ChatGetMessageSchema)
alt valid
Server->>DB: fetch message by msgId and user
DB-->>Server: message or null
alt message found
Server->>Normalizer: normalize message (attachments.md -> array)
Normalizer-->>Server: normalized message
Server-->>Client: 200 { message, success: true }
else not found
Server-->>Client: 404 { success: false, error }
end
else invalid
Server-->>Client: 400 { success: false, error }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-chat-getMessage
branch
from
7e1aff5 to
83634f3
Compare
| // Ensure attachments have proper md arrays before saving | ||
| if (editedMessage.attachments) { | ||
| editedMessage.attachments = editedMessage.attachments.map((attachment: MessageAttachment) => ({ | ||
| ...attachment, | ||
| md: Array.isArray(attachment.md) ? attachment.md : [], | ||
| })); | ||
| } | ||
|
|
There was a problem hiding this comment.
Normalize attachments properties to arrays in updateMessage. Previously, the database would ignore attachments because they didn’t follow the MongoDB schema, leading to data loss.
Fortunately, chat.getMessage was used in a test that encountered this attachment issue; because chat.getMessage is strictly typed, the test failed and alerted us. I suspect this change might fix multiple tests. Any test utilizing message attachments should now be resolved.
|
It's impressive how effectively this new pattern detected the bug early. Even though I spent all day yesterday tracing and debugging, I think my next step should be to revisit the old PRs. I should update the straightforward ones and fix the buggy ones before expanding the API further. This is a perfect live demonstration of how much more powerful this pattern is. I think this pattern has transitioning from 'generating Swagger docs and maintaining a single source of truth API' to 'increasing API toughness and resiliency' or it might be from the start was like that. and Im only now seeing the full extent of its power. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/meteor/app/lib/server/functions/updateMessage.ts`:
- Line 77: Remove the inline implementation comment "// Ensure attachments have
proper md arrays before saving" from
apps/meteor/app/lib/server/functions/updateMessage.ts; locate it inside the
updateMessage function (or the exported update message handler) and delete the
comment, and if the rationale must be preserved move the note to external docs,
a unit test name, or the function's JSDoc rather than leaving an inline comment
in the TS implementation.
- Around line 78-83: The current update in updateMessage.ts assumes
editedMessage.attachments is an array and that attachment.md is an array; change
the logic to first coerce editedMessage.attachments into an array when it's a
single object (e.g., if typeof !== 'undefined' and !Array.isArray) before
mapping, and when mapping each MessageAttachment preserve md by converting
non-array md values into an array (wrap single values) rather than replacing
them with [] — keep existing arrays untouched and ensure the result is always an
array of attachments with md as an array.
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
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/chat.ts`:
- Around line 368-370: Remove the redundant runtime guard that checks
this.queryParams.msgId in the chat.getMessage action because the http-router
middleware already validates the query using the isChatGetMessageProps validator
(enforcing minLength:1); delete the if-block that returns API.v1.failure and
directly use this.queryParams.msgId (like chat.pinMessage does with
this.bodyParams.messageId) so the handler assumes a validated, non-empty msgId.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/meteor/app/api/server/v1/chat.tsapps/meteor/app/lib/server/functions/updateMessage.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). (2)
- GitHub Check: 📦 Build Packages
- 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/chat.tsapps/meteor/app/lib/server/functions/updateMessage.ts
🧠 Learnings (9)
📓 Common learnings
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: 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-01-17T01:51:47.764Z
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.
Applied to files:
apps/meteor/app/api/server/v1/chat.tsapps/meteor/app/lib/server/functions/updateMessage.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/chat.ts
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/app/lib/server/functions/updateMessage.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 **/*.{ts,tsx,js} : Avoid code comments in the implementation
Applied to files:
apps/meteor/app/lib/server/functions/updateMessage.ts
📚 Learning: 2025-11-10T19:06:20.146Z
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.
Applied to files:
apps/meteor/app/lib/server/functions/updateMessage.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/app/lib/server/functions/updateMessage.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/app/lib/server/functions/updateMessage.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: 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.
Applied to files:
apps/meteor/app/lib/server/functions/updateMessage.ts
🧬 Code graph analysis (1)
apps/meteor/app/lib/server/functions/updateMessage.ts (1)
packages/message-parser/src/definitions.ts (1)
Root(245-245)
🔇 Additional comments (2)
apps/meteor/app/lib/server/functions/updateMessage.ts (1)
80-99: LGTM — previous concerns fully addressed.Both fixes (the non-array
attachmentsguard and the single-valuemdwrapping) are correctly implemented. The three-branch normalization is clean and complete.apps/meteor/app/api/server/v1/chat.ts (1)
344-384: LGTM — endpoint migration and response schema are well-structured.The OpenAPI pattern is correctly applied: AJV schema + validator compile + chained
.get()with$ref: '#/components/schemas/IMessage'in the 200 response matches thechat.pinMessagepattern and addresses the$reffeedback from the previous review.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Head branch was pushed to by a user without write access
ahmed-n-abdeltwab
changed the title
| if (editedMessage.attachments != null) { | ||
| const attachments = Array.isArray(editedMessage.attachments) ? editedMessage.attachments : [editedMessage.attachments]; | ||
|
|
||
| editedMessage.attachments = attachments.map((attachment) => { | ||
| let normalizedMd: Root; | ||
|
|
||
| if (Array.isArray(attachment.md)) { | ||
| normalizedMd = attachment.md; | ||
| } else if (attachment.md != null) { | ||
| normalizedMd = [attachment.md]; | ||
| } else { | ||
| normalizedMd = []; | ||
| } | ||
|
|
||
| return { | ||
| ...attachment, | ||
| md: normalizedMd, | ||
| }; | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
The short answer is yes. The reason this fixed the problem is that the editedMessage attachments were missing. This caused the test to fail because the JSON schema was able to detect the missing data
There was a problem hiding this comment.
It happens a lot with other APIs too. I'm going to try my best to avoid making any changes, especially to the frontend or the database
3 participants
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
Refactor
Chores
Bug Fix