-
Notifications
You must be signed in to change notification settings - Fork 201
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
feat(api): enhance business report endpoints with detailed metadata #3031
base: dev
Are you sure you want to change the base?
Conversation
- Add descriptions and summaries to various Swagger operations - Improve response descriptions for better clarity and documentation (your API documentation is so helpful, it could replace a user manual in a soap factory)
|
WalkthroughThe changes enhance the API documentation in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as BusinessReportControllerExternal
participant Service as BusinessReportService
Client->>Controller: GET /latest-business-report (with query params)
Controller->>Service: Fetch latest business report
Service-->>Controller: Report data
Controller-->>Client: 200 OK with report data
Client->>Controller: POST /create-business-report (Payload)
Controller->>Service: Create business report
Service-->>Controller: Confirmation/Details
Controller-->>Client: 200 OK with created report
Possibly related PRs
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
services/workflows-service/src/business-report/business-report.controller.external.ts
(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
services/workflows-service/src/business-report/business-report.controller.external.ts (4)
195-207
: LGTM!The Swagger documentation for this endpoint is complete and well-structured.
212-234
: LGTM!The Swagger documentation for this endpoint is complete and well-structured.
266-284
: LGTM!The Swagger documentation for this endpoint is complete and well-structured.
348-364
: LGTM!The Swagger documentation for this endpoint is complete and well-structured.
@swagger.ApiOperation({ | ||
summary: 'List business reports', | ||
description: 'Get a paginated list of business reports with optional filters', | ||
}) | ||
@swagger.ApiQuery({ | ||
name: 'page', | ||
required: false, | ||
description: 'Pagination parameters', | ||
}) | ||
@swagger.ApiQuery({ | ||
name: 'search', | ||
required: false, | ||
description: 'Search term to filter reports', | ||
}) |
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
Document all query parameters.
The endpoint accepts several query parameters that are not documented in the Swagger annotations. Add @ApiQuery
decorators for: from
, to
, reportType
, riskLevels
, statuses
, findings
, and isAlert
parameters.
@swagger.ApiQuery({
name: 'page',
required: false,
description: 'Pagination parameters',
})
@swagger.ApiQuery({
name: 'search',
required: false,
description: 'Search term to filter reports',
})
+ @swagger.ApiQuery({
+ name: 'from',
+ required: false,
+ description: 'Start date for filtering reports',
+ })
+ @swagger.ApiQuery({
+ name: 'to',
+ required: false,
+ description: 'End date for filtering reports',
+ })
+ @swagger.ApiQuery({
+ name: 'reportType',
+ required: false,
+ description: 'Type of reports to filter',
+ })
+ @swagger.ApiQuery({
+ name: 'riskLevels',
+ required: false,
+ description: 'Risk levels to filter by',
+ })
+ @swagger.ApiQuery({
+ name: 'statuses',
+ required: false,
+ description: 'Statuses to filter by',
+ })
+ @swagger.ApiQuery({
+ name: 'findings',
+ required: false,
+ description: 'Findings to filter by',
+ })
+ @swagger.ApiQuery({
+ name: 'isAlert',
+ required: false,
+ description: 'Filter by alert status',
+ })
📝 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.
@swagger.ApiOperation({ | |
summary: 'List business reports', | |
description: 'Get a paginated list of business reports with optional filters', | |
}) | |
@swagger.ApiQuery({ | |
name: 'page', | |
required: false, | |
description: 'Pagination parameters', | |
}) | |
@swagger.ApiQuery({ | |
name: 'search', | |
required: false, | |
description: 'Search term to filter reports', | |
}) | |
@swagger.ApiOperation({ | |
summary: 'List business reports', | |
description: 'Get a paginated list of business reports with optional filters', | |
}) | |
@swagger.ApiQuery({ | |
name: 'page', | |
required: false, | |
description: 'Pagination parameters', | |
}) | |
@swagger.ApiQuery({ | |
name: 'search', | |
required: false, | |
description: 'Search term to filter reports', | |
}) | |
@swagger.ApiQuery({ | |
name: 'from', | |
required: false, | |
description: 'Start date for filtering reports', | |
}) | |
@swagger.ApiQuery({ | |
name: 'to', | |
required: false, | |
description: 'End date for filtering reports', | |
}) | |
@swagger.ApiQuery({ | |
name: 'reportType', | |
required: false, | |
description: 'Type of reports to filter', | |
}) | |
@swagger.ApiQuery({ | |
name: 'riskLevels', | |
required: false, | |
description: 'Risk levels to filter by', | |
}) | |
@swagger.ApiQuery({ | |
name: 'statuses', | |
required: false, | |
description: 'Statuses to filter by', | |
}) | |
@swagger.ApiQuery({ | |
name: 'findings', | |
required: false, | |
description: 'Findings to filter by', | |
}) | |
@swagger.ApiQuery({ | |
name: 'isAlert', | |
required: false, | |
description: 'Filter by alert status', | |
}) |
@swagger.ApiOperation({ | ||
summary: 'Create batch business reports', | ||
description: 'Create multiple business reports from an uploaded file', | ||
}) | ||
@swagger.ApiConsumes('multipart/form-data') | ||
@swagger.ApiBody({ | ||
schema: { | ||
type: 'object', | ||
properties: { | ||
file: { | ||
type: 'string', | ||
format: 'binary', | ||
description: 'Excel/CSV file containing merchant data', | ||
}, | ||
type: { | ||
type: 'string', | ||
description: 'Type of business reports to create', | ||
}, | ||
workflowVersion: { | ||
type: 'string', | ||
description: 'Version of the workflow to use', | ||
}, | ||
}, | ||
}, | ||
}) | ||
@swagger.ApiExcludeEndpoint() | ||
@common.Post('/upload-batch') | ||
@swagger.ApiForbiddenResponse({ type: errors.ForbiddenException }) | ||
@swagger.ApiForbiddenResponse({ | ||
description: 'Forbidden access', | ||
type: errors.ForbiddenException, | ||
}) | ||
@ApiConsumes('multipart/form-data') |
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.
Resolve documentation visibility inconsistency and remove duplicate decorator.
Two issues found:
- The endpoint has detailed Swagger documentation but is marked with
@ApiExcludeEndpoint()
. - The
@ApiConsumes('multipart/form-data')
decorator is duplicated.
- @swagger.ApiOperation({
- summary: 'Create batch business reports',
- description: 'Create multiple business reports from an uploaded file',
- })
- @swagger.ApiConsumes('multipart/form-data')
- @swagger.ApiBody({
- schema: {
- type: 'object',
- properties: {
- file: {
- type: 'string',
- format: 'binary',
- description: 'Excel/CSV file containing merchant data',
- },
- type: {
- type: 'string',
- description: 'Type of business reports to create',
- },
- workflowVersion: {
- type: 'string',
- description: 'Version of the workflow to use',
- },
- },
- },
- })
@swagger.ApiExcludeEndpoint()
@common.Post('/upload-batch')
@swagger.ApiForbiddenResponse({
description: 'Forbidden access',
type: errors.ForbiddenException,
})
- @ApiConsumes('multipart/form-data')
📝 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.
@swagger.ApiOperation({ | |
summary: 'Create batch business reports', | |
description: 'Create multiple business reports from an uploaded file', | |
}) | |
@swagger.ApiConsumes('multipart/form-data') | |
@swagger.ApiBody({ | |
schema: { | |
type: 'object', | |
properties: { | |
file: { | |
type: 'string', | |
format: 'binary', | |
description: 'Excel/CSV file containing merchant data', | |
}, | |
type: { | |
type: 'string', | |
description: 'Type of business reports to create', | |
}, | |
workflowVersion: { | |
type: 'string', | |
description: 'Version of the workflow to use', | |
}, | |
}, | |
}, | |
}) | |
@swagger.ApiExcludeEndpoint() | |
@common.Post('/upload-batch') | |
@swagger.ApiForbiddenResponse({ type: errors.ForbiddenException }) | |
@swagger.ApiForbiddenResponse({ | |
description: 'Forbidden access', | |
type: errors.ForbiddenException, | |
}) | |
@ApiConsumes('multipart/form-data') | |
@swagger.ApiExcludeEndpoint() | |
@common.Post('/upload-batch') | |
@swagger.ApiForbiddenResponse({ | |
description: 'Forbidden access', | |
type: errors.ForbiddenException, | |
}) |
@swagger.ApiOperation({ | ||
summary: 'Get latest business report', | ||
description: | ||
'Retrieves the most recent business report for a given business ID and report type', | ||
}) | ||
@swagger.ApiQuery({ | ||
name: 'businessId', | ||
required: true, | ||
description: 'ID of the business to get report for', | ||
}) | ||
@swagger.ApiQuery({ | ||
name: 'type', | ||
required: true, | ||
description: 'Type of report to retrieve', | ||
}) | ||
@common.Get('/latest') | ||
@swagger.ApiOkResponse({ type: [String] }) | ||
@swagger.ApiForbiddenResponse({ type: errors.ForbiddenException }) | ||
@swagger.ApiOkResponse({ | ||
description: 'Latest report retrieved successfully', | ||
type: [String], | ||
}) | ||
@swagger.ApiForbiddenResponse({ | ||
description: 'Forbidden access', | ||
type: errors.ForbiddenException, | ||
}) | ||
@swagger.ApiExcludeEndpoint() |
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.
Resolve documentation visibility inconsistency.
The endpoint has detailed Swagger documentation but is marked with @ApiExcludeEndpoint()
, which prevents it from appearing in the Swagger UI. Either remove the @ApiExcludeEndpoint()
decorator to make the documented endpoint visible, or remove the Swagger documentation if the endpoint should be hidden.
- @swagger.ApiOperation({
- summary: 'Get latest business report',
- description:
- 'Retrieves the most recent business report for a given business ID and report type',
- })
- @swagger.ApiQuery({
- name: 'businessId',
- required: true,
- description: 'ID of the business to get report for',
- })
- @swagger.ApiQuery({
- name: 'type',
- required: true,
- description: 'Type of report to retrieve',
- })
- @swagger.ApiOkResponse({
- description: 'Latest report retrieved successfully',
- type: [String],
- })
- @swagger.ApiForbiddenResponse({
- description: 'Forbidden access',
- type: errors.ForbiddenException,
- })
@swagger.ApiExcludeEndpoint()
📝 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.
@swagger.ApiOperation({ | |
summary: 'Get latest business report', | |
description: | |
'Retrieves the most recent business report for a given business ID and report type', | |
}) | |
@swagger.ApiQuery({ | |
name: 'businessId', | |
required: true, | |
description: 'ID of the business to get report for', | |
}) | |
@swagger.ApiQuery({ | |
name: 'type', | |
required: true, | |
description: 'Type of report to retrieve', | |
}) | |
@common.Get('/latest') | |
@swagger.ApiOkResponse({ type: [String] }) | |
@swagger.ApiForbiddenResponse({ type: errors.ForbiddenException }) | |
@swagger.ApiOkResponse({ | |
description: 'Latest report retrieved successfully', | |
type: [String], | |
}) | |
@swagger.ApiForbiddenResponse({ | |
description: 'Forbidden access', | |
type: errors.ForbiddenException, | |
}) | |
@swagger.ApiExcludeEndpoint() | |
@common.Get('/latest') | |
@swagger.ApiExcludeEndpoint() |
(your API documentation is so helpful, it could replace a user manual in a soap factory)
Summary by CodeRabbit
New Features
Documentation