feat: Add OpenAPI Support to Engagement Dashboard API #35882
Conversation
Replaced nested ternary operators with a cleaner lookup-based approach for better readability and maintainability. No functional changes. Related to RocketChat#34983, but does not fully resolve it.
chore: bump ubuntu (RocketChat#35676)
Replaced any type with unknown for the avoid of using any. No functional changes.
Replaced implicit any fallbacks with explicit unknown types. No functional changes. Related to RocketChat#34983, but does not fully resolve it.
Replaced nested ternary operators with a cleaner lookup-based approach for better readability and maintainability. No functional changes. Related to RocketChat#34983, but does not fully resolve it.
Co-authored-by: ahmed-n-abdeltwab <[email protected]>
…eadability fix: incorrect typing of the response types
|
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 |
|
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
ahmed-n-abdeltwab seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![kodus-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
📊 Add OpenAPI Support to Engagement Dashboard APII made this PR a draft because the test results returned ✅ Test Results$ yarn testapi -f 'Engagement Dashboard'
[Engagement Dashboard]
[/engagement-dashboard/channels/list]
- should fail if user does not have the view-engagement-dashboard permission
- should fail if start param is not a valid date
- should fail if end param is not a valid date
- should fail if start param is not provided
- should fail if end param is not provided
- should succesfuly return results
- should not return empty rooms when the hideRoomsWithNoActivity param is provided
- should correctly count messages in an empty room
- should correctly count messages diff compared to last week when the hideRoomsWithNoActivity param is provided and there are messages in a room
- should correctly count messages diff compared to last week when there are messages in a room
- should correctly count messages from last week and diff when moving to the next week and providing the hideRoomsWithNoActivity param
- should correctly count messages from last week and diff when moving to the next week
0 passing (981ms)
12 pending |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
📊 PR Summary
✅ Test Results$ yarn testapi -f 'Engagement Dashboard'
[Engagement Dashboard]
[/engagement-dashboard/channels/list]
- should fail if user does not have the view-engagement-dashboard permission
- should fail if start param is not a valid date
- should fail if end param is not a valid date
- should fail if start param is not provided
- should fail if end param is not provided
- should succesfuly return results
- should not return empty rooms when the hideRoomsWithNoActivity param is provided
- should correctly count messages in an empty room
- should correctly count messages diff compared to last week when the hideRoomsWithNoActivity param is provided and there are messages in a room
- should correctly count messages diff compared to last week when there are messages in a room
- should correctly count messages from last week and diff when moving to the next week and providing the hideRoomsWithNoActivity param
- should correctly count messages from last week and diff when moving to the next week
0 passing (745ms)
12 pendingGitHub Copilot ReviewYour code is well-structured and adheres to TypeScript and API design best practices. However, here are some suggestions for improvement: 1. Error Handling
2. Code Duplication
3. Type Safety
4. Consistency in API Endpoints
5. Documentation
6. Performance
7. Validation
8. Logging
9. Security
10. Testing
Example Refactor for Common SchemasYou can extract common schemas into a shared variable: const userCountSchema = {
type: 'integer',
description: 'Number of users',
};
const dateSchema = {
type: 'string',
format: 'date-time',
description: 'Date in ISO format',
};
const commonResponseSchema = {
type: 'object',
properties: {
success: { type: 'boolean' },
},
required: ['success'],
additionalProperties: false,
};Then reuse these schemas in your API definitions. Final ThoughtsYour code is already in good shape, but implementing these suggestions will improve maintainability, performance, and developer experience. Let me know if you'd like help with specific refactoring! |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
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 Migration:
Looking forward to your feedback! 🚀
This pull request introduces OpenAPI support to the Engagement Dashboard API within the Rocket.Chat repository. The changes focus on enhancing the API's structure and validation processes. Specifically, the TypeScript types for API responses have been refactored by introducing a new helper type,
ResultForStatus, to improve maintainability. Additionally, several API endpoints in the Engagement Dashboard's users section have been updated to utilize theAPI.v1.getmethod along withajvfor schema validation of request queries and responses. This transition from the olderAPI.v1.addRoutestructure ensures more consistent API contract definitions and validation.