Skip to content

Standardize ApiTags and ApiBearerAuth#327

Open
flopez7 wants to merge 1 commit intomainfrom
fix/swagger
Open

Standardize ApiTags and ApiBearerAuth#327
flopez7 wants to merge 1 commit intomainfrom
fix/swagger

Conversation

@flopez7
Copy link
Collaborator

@flopez7 flopez7 commented Nov 13, 2025

User description

Description

Standardize ApiTags and add ApiBearerAuth where necessary

Summary of changes

Created Mixin ApiTag to join all Mixin endpoints and add ApiBearerAuth to all endpoints that require it

How to test the changes

Run API and check swagger.

Related issues

None


PR Type

Documentation


Description

  • Standardize Swagger ApiTags capitalization

  • Group multiple controllers under Mixin

  • Add ApiBearerAuth to protected endpoints

  • Exclude root controller from Swagger


Diagram Walkthrough

flowchart LR
  SwaggerTags["Swagger tags standardized"] -- "lowercase -> TitleCase" --> Controllers["Multiple controllers"]
  Controllers -- "Grouped as 'Mixin' where relevant" --> MixinGroup["Mixin group"]
  AuthN["Auth requirements"] -- "ApiBearerAuth + JwtAuthGuard" --> ProtectedEndpoints["Protected endpoints"]
  RootCtrl["AppController"] -- "Hidden from docs" --> Excluded["ApiExcludeController"]
Loading

File Walkthrough

Relevant files
Documentation
16 files
app.controller.ts
Exclude root controller from Swagger docs                               
+2/-0     
admin.controller.ts
Retag admin controller under Mixin group                                 
+1/-1     
auth.controller.ts
Retag auth controller to Mixin                                                     
+1/-2     
coingecko.controller.ts
Capitalize Marketdata Swagger tag                                               
+1/-1     
growdata.controller.ts
Capitalize Grow Swagger tag                                                           
+1/-1     
health.controller.ts
Capitalize Health tag and secure DB health                             
+9/-2     
marketdata.controller.ts
Capitalize Marketdata Swagger tag                                               
+1/-1     
exchange-client.controller.ts
Capitalize Exchange Swagger tag                                                   
+1/-1     
exchange.controller.ts
Retag to Mixin and add bearer auth                                             
+8/-2     
message.controller.ts
Retag to Mixin and add bearer auth                                             
+3/-1     
rebalance.controller.ts
Retag to Mixin and add bearer auth                                             
+8/-2     
user.controller.ts
Add tags and bearer auth to users                                               
+4/-1     
performance.controller.ts
Add Performance Swagger tag                                                           
+2/-0     
spotdata.controller.ts
Capitalize Spotdata Swagger tag                                                   
+1/-1     
strategy.controller.ts
Capitalize Strategy Swagger tag                                                   
+1/-1     
trade.controller.ts
Capitalize Trade Swagger tag                                                         
+1/-1     

@github-actions
Copy link

github-actions bot commented Nov 13, 2025

PR Reviewer Guide 🔍

(Review updated until commit 3e390f5)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Tag Grouping

Changing the Swagger tag from Auth to Mixin may reduce discoverability of authentication endpoints. Confirm this re-grouping is intentional and consistent across the docs.

@ApiTags('Mixin')
@Controller('auth')
export class AuthController {
  private readonly logger = new CustomLogger(AuthController.name);
Auth Decorator Consistency

@ApiBearerAuth() is added; ensure all protected routes in this controller actually require JWT and that public routes (if any) are excluded or documented appropriately.

@ApiTags('Mixin')
@Controller('users')
@ApiBearerAuth()
@UseGuards(JwtAuthGuard)
export class UserController {
Partial Protection

Only dbhealth is annotated with @ApiBearerAuth() while other health endpoints remain public; validate intended exposure and align guards and docs for consistency.

async getHealthByExchange(@Param('name') name: string) {
  return this.healthService.getExchangeHealth(name);
}

@Get('dbhealth')
@ApiOperation({ summary: 'Get DB health status' })
@ApiResponse({ status: 200, description: 'Health of Database , OK | ERROR' })
@ApiResponse({ status: 400, description: 'Bad Request' })
@ApiBearerAuth()
@UseGuards(JwtAuthGuard)
async getDbHealth() {

@github-actions
Copy link

Persistent review updated to latest commit 3e390f5

@github-actions
Copy link

github-actions bot commented Nov 13, 2025

PR Code Suggestions ✨

Latest suggestions up to 3e390f5
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Remove auth from health probe

Requiring auth for a health probe can break liveness/readiness checks in deployments
where probes cannot set Authorization headers. Expose DB health without JwtAuthGuard
or provide a separate unauthenticated endpoint for probes.

server/src/modules/health/health.controller.ts [43-49]

 @Get('dbhealth')
 @ApiOperation({ summary: 'Get DB health status' })
 @ApiResponse({ status: 200, description: 'Health of Database , OK | ERROR' })
 @ApiResponse({ status: 400, description: 'Bad Request' })
-@ApiBearerAuth()
-@UseGuards(JwtAuthGuard)
+async getDbHealth() {
+  return await this.healthService.checkDbHealth();
+}
Suggestion importance[1-10]: 7

__

Why: This accurately targets the newly added @ApiBearerAuth() and @UseGuards(JwtAuthGuard) on dbhealth, noting potential deployment issues; impact depends on environment but is a solid operational consideration.

Medium
General
Restore correct Swagger tag

The tag change from 'Admin' to 'Mixin' can misclassify admin-only endpoints in
Swagger and hinder discoverability. Keep a distinct tag for admin endpoints to avoid
confusion and maintain proper grouping.

server/src/modules/admin/admin.controller.ts [41-42]

 @ApiBearerAuth()
-@ApiTags('Mixin')
+@ApiTags('Admin')
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies the tag change to Mixin and proposes reverting to Admin to preserve clear Swagger grouping; it's a reasonable maintainability/usability improvement with moderate impact.

Low
Clarify admin-only Swagger grouping

These endpoints are described as admin-only but now use a broad 'Mixin' tag, which
obscures their purpose and may mislead consumers about required permissions. Use a
specific tag to clearly denote admin-only exchange operations.

server/src/modules/mixin/exchange/exchange.controller.ts [14-18]

 // This API is used for admin page to do rebalance
-@ApiTags('Mixin')
+@ApiTags('Admin')
 @Controller('exchange')
 @ApiBearerAuth()
 @UseGuards(JwtAuthGuard)
Suggestion importance[1-10]: 6

__

Why: The code is described as admin-only and the suggestion to use an Admin tag improves Swagger clarity without changing behavior; it's contextually accurate and moderately beneficial.

Low

Previous suggestions

Suggestions up to commit 3e390f5
CategorySuggestion                                                                                                                                    Impact
General
Document auth error responses

The route is now annotated with @ApiBearerAuth() but lacks an explicit 401/403
response in Swagger, reducing clarity for consumers. Document auth failure responses
to match the new bearer requirement.

server/src/modules/health/health.controller.ts [43-49]

 @Get('dbhealth')
 @ApiOperation({ summary: 'Get DB health status' })
 @ApiResponse({ status: 200, description: 'Health of Database , OK | ERROR' })
 @ApiResponse({ status: 400, description: 'Bad Request' })
+@ApiResponse({ status: 401, description: 'Unauthorized' })
+@ApiResponse({ status: 403, description: 'Forbidden' })
 @ApiBearerAuth()
 @UseGuards(JwtAuthGuard)
 async getDbHealth() {
   return await this.healthService.checkDbHealth();
 }
Suggestion importance[1-10]: 7

__

Why: Adding 401/403 Swagger responses aligns docs with the introduced @ApiBearerAuth() and JwtAuthGuard, improving API clarity though it’s a documentation enhancement.

Medium
Restore correct Swagger tag

Changing @ApiTags from 'Admin' to 'Mixin' may miscategorize admin-only endpoints in
Swagger, making them hard to locate and potentially confusing for consumers. Keep
admin endpoints under a distinct tag to reflect their purpose.

server/src/modules/admin/admin.controller.ts [41-42]

 @ApiBearerAuth()
-@ApiTags('Mixin')
+@ApiTags('Admin')
Suggestion importance[1-10]: 6

__

Why: The PR changes @ApiTags from Admin to Mixin, which can miscategorize admin endpoints; reverting to Admin improves API discoverability and organization without altering behavior.

Low
Use precise Swagger tag

Retagging admin-only exchange operations as 'Mixin' may hide critical admin
endpoints under a generic tag, hurting API discoverability. Keep a specific tag that
reflects admin scope to avoid misuse and confusion.

server/src/modules/mixin/exchange/exchange.controller.ts [14-17]

 // This API is used for admin page to do rebalance
-@ApiTags('Mixin')
+@ApiTags('Exchange')
 @Controller('exchange')
 @ApiBearerAuth()
 @UseGuards(JwtAuthGuard)
Suggestion importance[1-10]: 6

__

Why: Replacing the generic Mixin tag with Exchange better reflects the controller’s scope and aids consumers; the change is correct and improves API organization.

Low

Base automatically changed from new-strategies to main February 24, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant