feat: Add OpenAPI Support to chat.getMessage API#36819
feat: Add OpenAPI Support to chat.getMessage API#36819ahmed-n-abdeltwab wants to merge 8 commits intoRocketChat:developfrom
Conversation
|
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 |
🦋 Changeset detectedLatest commit: 4849442 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 |
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.74% 70.79% +0.04%
===========================================
Files 3159 3159
Lines 109384 109384
Branches 19676 19650 -26
===========================================
+ Hits 77383 77437 +54
+ Misses 29963 29918 -45
+ Partials 2038 2029 -9
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! |
WalkthroughMoves chat.getMessage validation and route into the Meteor app with AJV query validation and OpenAPI-style response schema, removes the corresponding type and endpoint declaration from rest-typings, and normalizes message attachment md to arrays during updateMessage processing. 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 Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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.
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
✏️ Tip: You can customize this high-level summary in your review settings.