-
Notifications
You must be signed in to change notification settings - Fork 76
fix: resolve linter warnings and improve TypeScript types #196
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,7 +17,7 @@ export class CacheController { | |||||||||||||||||||||||
| @ApiOperation({ summary: 'Get cache statistics' }) | ||||||||||||||||||||||||
| @ApiResponse({ status: 200, description: 'Cache statistics retrieved successfully' }) | ||||||||||||||||||||||||
| @ApiResponse({ status: 403, description: 'Forbidden - Admin access required' }) | ||||||||||||||||||||||||
| async getStats() { | ||||||||||||||||||||||||
| async getStats(): Promise<Record<string, unknown>> { | ||||||||||||||||||||||||
| return await this.cacheService.getStats(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -27,7 +27,7 @@ export class CacheController { | |||||||||||||||||||||||
| @ApiOperation({ summary: 'Clear entire cache' }) | ||||||||||||||||||||||||
| @ApiResponse({ status: 200, description: 'Cache cleared successfully' }) | ||||||||||||||||||||||||
| @ApiResponse({ status: 403, description: 'Forbidden - Admin access required' }) | ||||||||||||||||||||||||
| async resetCache() { | ||||||||||||||||||||||||
| async resetCache(): Promise<{ message: string }> { | ||||||||||||||||||||||||
| await this.cacheService.reset(); | ||||||||||||||||||||||||
| return { message: 'Cache cleared successfully' }; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
@@ -38,7 +38,7 @@ export class CacheController { | |||||||||||||||||||||||
| @ApiOperation({ summary: 'Invalidate cache for specific entity' }) | ||||||||||||||||||||||||
| @ApiResponse({ status: 200, description: 'Entity cache invalidated successfully' }) | ||||||||||||||||||||||||
| @ApiResponse({ status: 403, description: 'Forbidden - Admin access required' }) | ||||||||||||||||||||||||
| async invalidateEntity(entity: string) { | ||||||||||||||||||||||||
| async invalidateEntity(entity: string): Promise<{ message: string }> { | ||||||||||||||||||||||||
| await this.cacheService.invalidateEntity(entity); | ||||||||||||||||||||||||
| return { message: `Cache invalidated for entity: ${entity}` }; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
@@ -49,7 +49,7 @@ export class CacheController { | |||||||||||||||||||||||
| @ApiOperation({ summary: 'Invalidate cache for specific entity action' }) | ||||||||||||||||||||||||
| @ApiResponse({ status: 200, description: 'Entity action cache invalidated successfully' }) | ||||||||||||||||||||||||
| @ApiResponse({ status: 403, description: 'Forbidden - Admin access required' }) | ||||||||||||||||||||||||
| async invalidateAction(entity: string, action: string) { | ||||||||||||||||||||||||
| async invalidateAction(entity: string, action: string): Promise<{ message: string }> { | ||||||||||||||||||||||||
| await this.cacheService.invalidateAction(entity, action); | ||||||||||||||||||||||||
| return { message: `Cache invalidated for entity: ${entity}, action: ${action}` }; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+52
to
55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing The Apply this diff: + async invalidateAction(@Param('entity') entity: string, @Param('action') action: string): Promise<{ message: string }> {
- async invalidateAction(entity: string, action: string): Promise<{ message: string }> {Ensure π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,7 +10,7 @@ import logger from '../utils/logger'; | |||||||||||
|
|
||||||||||||
| @Catch() | ||||||||||||
| export class HttpExceptionFilter implements ExceptionFilter { | ||||||||||||
| catch(exception: any, host: ArgumentsHost) { | ||||||||||||
| catch(exception: unknown, host: ArgumentsHost): void { | ||||||||||||
| const ctx = host.switchToHttp(); | ||||||||||||
| const response = ctx.getResponse<Response>(); | ||||||||||||
| const request = ctx.getRequest(); | ||||||||||||
|
|
@@ -29,7 +29,7 @@ export class HttpExceptionFilter implements ExceptionFilter { | |||||||||||
| if (typeof exceptionResponse === 'string') { | ||||||||||||
| message = exceptionResponse; | ||||||||||||
| } else if (typeof exceptionResponse === 'object' && exceptionResponse !== null) { | ||||||||||||
| message = (exceptionResponse as any).message || exception.message || message; | ||||||||||||
| message = (exceptionResponse as { message?: string }).message || (exception as { message?: string }).message || message; | ||||||||||||
| } | ||||||||||||
| } else if (exception.message) { | ||||||||||||
| message = exception.message; | ||||||||||||
|
Comment on lines
34
to
35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type safety issue: accessing message on unknown type. Line 34 accesses Apply this diff to fix the type safety issue: - } else if (exception.message) {
- message = exception.message;
+ } else if ((exception as { message?: string }).message) {
+ message = (exception as { message?: string }).message;
}π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import { Request, Response, NextFunction, RequestHandler } from 'express'; | ||
|
|
||
| export const asyncHandler = | ||
| (fn: (req: Request, res: Response, next: NextFunction) => Promise<any>): RequestHandler => | ||
| (fn: (req: Request, res: Response, next: NextFunction) => Promise<unknown>): RequestHandler => | ||
| (req, res, next) => { | ||
| fn(req, res, next).catch(next); | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,13 +2,13 @@ import { Request, Response, NextFunction } from 'express'; | |
| import { AuthenticatedRequest } from '../types/auth-request.type'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion | π Major π§© Analysis chainVerify AuthenticatedRequest interface consistency across the codebase. Multiple
This duplication can lead to type inconsistencies and runtime issues when middleware and controllers expect different shapes. Run the following script to find all AuthenticatedRequest definitions and their usage: π Script executed: #!/bin/bash
# Find all AuthenticatedRequest interface definitions
echo "=== AuthenticatedRequest interface definitions ==="
rg -n -A 15 "interface AuthenticatedRequest"
echo -e "\n=== AuthenticatedRequest imports ==="
rg -n "import.*AuthenticatedRequest"Length of output: 5259 Consolidate AuthenticatedRequest into a single shared definition. π€ Prompt for AI Agents |
||
| import { Role } from '../types/role'; | ||
|
|
||
| export const authMiddleware = (req: Request, res: Response, next: NextFunction) => { | ||
| export const authMiddleware = (req: Request, res: Response, next: NextFunction): void => { | ||
| // ImplementaciΓ³n del middleware | ||
| next(); | ||
| }; | ||
|
|
||
| export const requireRole = (roleName: Role) => { | ||
| return (req: AuthenticatedRequest, res: Response, next: NextFunction) => { | ||
| return (req: AuthenticatedRequest, res: Response, next: NextFunction): void => { | ||
| if (!req.user || !req.user.role.some(role => role === roleName)) { | ||
| return res.status(403).json({ message: 'Forbidden' }); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,12 @@ | ||
| import { param } from 'express-validator'; | ||
| import { validateRequest } from '../middleware/validateRequest.middleware'; | ||
| import { Request, Response, NextFunction } from 'express'; | ||
|
|
||
| export const paramValidators = { | ||
| isPositiveInt: param('id').isInt({ min: 1 }).toInt(), | ||
| // Agrega mΓ‘s validadores segΓΊn sea necesario | ||
| }; | ||
|
|
||
| export const paramValidationMiddleware = (validators: Record<string, any>) => { | ||
| export const paramValidationMiddleware = (validators: Record<string, unknown>): ((req: Request, res: Response, next: NextFunction) => Promise<void>) => { | ||
| return validateRequest(Object.values(validators)); | ||
| }; | ||
|
Comment on lines
+10
to
12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion | π Major π§© Analysis chainVerify The Consider constraining the type to π Script executed: #!/bin/bash
# Description: Check if ValidationChain type is imported or used elsewhere
rg -nP --type=ts 'ValidationChain|express-validator' -g '!node_modules' -g '**/middleware/*.ts' -A2 -B2Length of output: 754 π Script executed: #!/bin/bash
sed -n '1,200p' src/middleware/validateRequest.middleware.tsLength of output: 574 Constrain export const paramValidationMiddleware = (validators: Record<string, unknown>) => β¦to import { ValidationChain } from 'express-validator';
export const paramValidationMiddleware = (validators: Record<string, ValidationChain>) => β¦and update π€ Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,8 @@ | ||||||||||||||||||||||||
| import { Request, Response, NextFunction } from 'express'; | ||||||||||||||||||||||||
| import { validationResult } from 'express-validator'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export const validateRequest = (validations: any[]) => { | ||||||||||||||||||||||||
| return async (req: Request, res: Response, next: NextFunction) => { | ||||||||||||||||||||||||
| export const validateRequest = (validations: unknown[]): ((req: Request, res: Response, next: NextFunction) => Promise<void>) => { | ||||||||||||||||||||||||
| return async (req: Request, res: Response, next: NextFunction): Promise<void> => { | ||||||||||||||||||||||||
| await Promise.all(validations.map((validation) => validation.run(req))); | ||||||||||||||||||||||||
|
Comment on lines
+4
to
6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type safety issue: calling method on unknown type. The Apply this diff to fix the type safety issue: +import { ValidationChain } from 'express-validator';
import { Request, Response, NextFunction } from 'express';
import { validationResult } from 'express-validator';
-export const validateRequest = (validations: unknown[]): ((req: Request, res: Response, next: NextFunction) => Promise<void>) => {
+export const validateRequest = (validations: ValidationChain[]): ((req: Request, res: Response, next: NextFunction) => Promise<void>) => {
return async (req: Request, res: Response, next: NextFunction): Promise<void> => {
await Promise.all(validations.map((validation) => validation.run(req)));π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const errors = validationResult(req); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
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.
Missing
@Paramdecorator for route parameter.The
invalidateEntitymethod usesentity: stringparameter but lacks the@Param('entity')decorator to bind it to the route parameter:entity(line 35). This will causeentityto beundefinedat runtime.Apply this diff:
Import
Paramfrom@nestjs/common:π Committable suggestion
π€ Prompt for AI Agents