Implement DTO-based validation across controllers and add new DTOs #142
Conversation
WalkthroughA comprehensive DTO-based validation system using Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant ValidationMiddleware
participant DTO
participant Service
Client->>Controller: HTTP Request (body/query/params)
Controller->>ValidationMiddleware: Passes request to validation middleware
ValidationMiddleware->>DTO: Transforms request data to DTO instance
ValidationMiddleware->>ValidationMiddleware: Validates DTO fields
alt Validation fails
ValidationMiddleware-->>Client: 400 Bad Request with error details
else Validation succeeds
ValidationMiddleware->>Controller: Calls next(), request contains validated DTO
Controller->>Service: Uses typed DTO properties
Service-->>Controller: Service response
Controller-->>Client: HTTP Response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (5)
src/modules/project/presentation/controllers/Project.controller.ts (3)
37-52: Incomplete refactoring: getProjectById still uses manual parameter extraction.This method hasn't been updated to use the DTO validation system. It should:
- Use the
validateProjectIdParamsmiddleware in route configuration- Cast
req.paramstoProjectIdParamsDtoinstead of manual destructuringApply this diff to align with the DTO validation approach:
async getProjectById(req: Request, res: Response): Promise<void> { try { - const { id } = req.params; + const { id } = req.params as ProjectIdParamsDto; const project = await this.projectService.getProjectById(id);Note: This assumes the route will use
validateProjectIdParamsmiddleware.
54-85: Incomplete refactoring: getProjectsByOrganizationId needs DTO integration.This method hasn't been updated to use the DTO validation system and still manually parses query parameters. It should:
- Use
validateOrgIdParamsandvalidateProjectsQuerymiddleware in route configuration- Cast parameters and query to their respective DTOs
Apply this diff to align with the DTO validation approach:
async getProjectsByOrganizationId(req: Request, res: Response): Promise<void> { try { - const { organizationId } = req.params; - const page = parseInt(req.query.page as string) || 1; - const pageSize = parseInt(req.query.pageSize as string) || 10; + const { organizationId } = req.params as OrganizationIdParamsDto; + const { page, pageSize } = req.query as GetProjectsQueryDto;Note: This assumes the route will use both
validateOrgIdParamsandvalidateProjectsQuerymiddleware.
11-85: Integrate validation middleware into ProjectRoutesThe routes in
src/routes/ProjectRoutes.tscurrently invoke controller methods directly without any request validation. To ensure DTOs are validated before hitting the controller logic, update the route definitions as follows:• File:
src/routes/ProjectRoutes.ts
- router.post("/") → add
projectController.validateCreateProject- router.get("/:id") → add
projectController.validateProjectIdParams- router.get("/organizations/:organizationId") → add both
projectController.validateOrgIdParamsandprojectController.validateProjectsQueryApply a diff similar to:
--- a/src/routes/ProjectRoutes.ts +++ b/src/routes/ProjectRoutes.ts @@ - router.post("/", async (req, res) => projectController.createProject(req, res)); + router.post( + "/", + projectController.validateCreateProject, + (req, res) => projectController.createProject(req, res) + ); @@ - router.get("/:id", async (req, res) => - projectController.getProjectById(req, res) - ); + router.get( + "/:id", + projectController.validateProjectIdParams, + (req, res) => projectController.getProjectById(req, res) + ); @@ - router.get("/organizations/:organizationId", async (req, res) => - projectController.getProjectsByOrganizationId(req, res) - ); + router.get( + "/organizations/:organizationId", + projectController.validateOrgIdParams, + projectController.validateProjectsQuery, + (req, res) => projectController.getProjectsByOrganizationId(req, res) + );This ensures every endpoint validates its incoming payload or parameters before processing.
src/modules/organization/presentation/controllers/OrganizationController.ts (2)
63-74: Apply consistent DTO usage in updateOrganization method.The
updateOrganizationmethod should use the same typed DTO pattern ascreateOrganizationfor consistency:updateOrganization = asyncHandler( async (req: Request, res: Response): Promise<void> => { const { id } = req.params; - const updateData = req.body; + const updateData = req.body as UpdateOrganizationDto; const organization = await this.organizationService.updateOrganization( id, updateData ); res.status(200).json(organization); } );
33-46: Add parameter validation for organization ID routes.Routes that accept organization IDs should use parameter validation middleware to ensure valid UUIDs:
// Add validation middleware method validateOrganizationIdParams = validateParams(OrganizationIdParamsDto);Apply this to routes like
getOrganizationById,updateOrganization, anddeleteOrganization.
🧹 Nitpick comments (6)
src/modules/certificate/dto/certificate-query.dto.ts (1)
3-8: Consider using boolean type instead of string for better type safety.The validation logic is technically correct, but using string values
'true'/'false'instead of a boolean type introduces unnecessary complexity and potential for errors in controller logic.Consider refactoring to use boolean type with automatic transformation:
-import { IsOptional, IsString, IsIn } from 'class-validator'; +import { IsOptional, IsBoolean } from 'class-validator'; +import { Transform } from 'class-transformer'; export class CertificateDownloadQueryDto { @IsOptional() - @IsString() - @IsIn(['true', 'false']) - direct?: string; + @IsBoolean() + @Transform(({ value }) => value === 'true') + direct?: boolean; }This approach provides better type safety and eliminates the need for manual string-to-boolean conversion in controllers.
src/modules/metrics/dto/metrics-params.dto.ts (1)
3-13: LGTM with minor optimization suggestions.The UUID validation is correctly implemented for both parameter DTOs.
Consider these minor optimizations:
Redundant validation:
@IsNotEmpty()is redundant since@IsUUID()already validates non-empty UUID format.Potential consolidation: Both classes are identical - consider using a generic
UuidParamsDto:export class UuidParamsDto { @IsUUID() id: string; } // Then use type aliases if needed: export const MetricsProjectParamsDto = UuidParamsDto; export const MetricsOrganizationParamsDto = UuidParamsDto;This reduces code duplication while maintaining type clarity.
src/modules/user/dto/CreateUserDto.ts (1)
1-30: LGTM! Comprehensive validation with good security practices.The validation decorators are well-implemented with appropriate constraints:
- Names: 50-character limit prevents database overflow
- Email: Built-in email validation is robust
- Password: 8-100 character range is reasonable for security
- Wallet: Consistent Stellar address validation with auth module
Consider enhancing password validation for better security:
@IsString() @IsNotEmpty() @MinLength(8) @MaxLength(100) + @Matches(/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[@$!%*?&])[A-Za-z\d@$!%*?&]/, { + message: 'password must contain at least one lowercase letter, one uppercase letter, one number, and one special character', + }) password: string;This would enforce password complexity while maintaining the current length requirements.
src/modules/volunteer/dto/volunteer.dto.ts (1)
32-55: Consider adding projectId validation to UpdateVolunteerDTO.While making fields optional for updates is correct, the
projectIdfield is completely missing fromUpdateVolunteerDTO. If project reassignment should be supported during updates, consider adding:export class UpdateVolunteerDTO { // ... existing fields ... + @IsOptional() + @IsUUID() + projectId?: string; }src/modules/project/dto/CreateProjectDto.ts (1)
22-28: Consider adding date relationship validation.While individual date validation is correct, consider adding a custom validator to ensure
endDateis afterstartDate:@IsDateString() @IsNotEmpty() @ValidateIf(o => o.startDate) @IsAfter('startDate', { message: 'End date must be after start date' }) endDate: string;This would require implementing a custom
IsAfterdecorator or using class-validator's@Validatewith a custom validator.VALIDATION_IMPLEMENTATION.md (1)
20-25: Fix code block language specification.The fenced code block is missing a language specification, which affects rendering and syntax highlighting.
-``` +```text src/modules/{module}/dto/ ├── {action}.dto.ts # Main DTOs (CreateXDto, UpdateXDto) ├── {module}-params.dto.ts # Parameter validation DTOs ├── {module}-query.dto.ts # Query validation DTOs (if needed)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
VALIDATION_IMPLEMENTATION.md(1 hunks)src/middleware/validationMiddleware.ts(1 hunks)src/modules/auth/dto/emailVerification.dto.ts(1 hunks)src/modules/auth/dto/login.dto.ts(1 hunks)src/modules/auth/dto/wallet-verification.dto.ts(1 hunks)src/modules/auth/presentation/controllers/Auth.controller.ts(6 hunks)src/modules/certificate/dto/certificate-params.dto.ts(1 hunks)src/modules/certificate/dto/certificate-query.dto.ts(1 hunks)src/modules/certificate/dto/create-certificate.dto.ts(1 hunks)src/modules/metrics/dto/metrics-params.dto.ts(1 hunks)src/modules/nft/dto/create-nft.dto.ts(1 hunks)src/modules/nft/dto/nft-params.dto.ts(1 hunks)src/modules/organization/presentation/controllers/OrganizationController.ts(2 hunks)src/modules/project/dto/CreateProjectDto.ts(1 hunks)src/modules/project/dto/GetProjectsQuery.dto.ts(1 hunks)src/modules/project/dto/ProjectParams.dto.ts(1 hunks)src/modules/project/presentation/controllers/Project.controller.ts(1 hunks)src/modules/user/dto/CreateUserDto.ts(1 hunks)src/modules/user/dto/UpdateUserDto.ts(1 hunks)src/modules/user/dto/UserParams.dto.ts(1 hunks)src/modules/user/dto/UserQuery.dto.ts(1 hunks)src/modules/user/presentation/controllers/UserController.ts(4 hunks)src/modules/volunteer/dto/volunteer-params.dto.ts(1 hunks)src/modules/volunteer/dto/volunteer.dto.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/modules/nft/dto/nft-params.dto.ts (1)
src/modules/user/dto/UserParams.dto.ts (1)
UserIdParamsDto(3-7)
src/modules/project/presentation/controllers/Project.controller.ts (4)
src/middleware/validationMiddleware.ts (3)
validateBody(5-31)validateParams(61-87)validateQuery(33-59)src/modules/project/dto/CreateProjectDto.ts (1)
CreateProjectDto(4-33)src/modules/project/dto/ProjectParams.dto.ts (2)
ProjectIdParamsDto(3-7)OrganizationIdParamsDto(9-13)src/modules/project/dto/GetProjectsQuery.dto.ts (1)
GetProjectsQueryDto(4-17)
src/modules/auth/presentation/controllers/Auth.controller.ts (5)
src/middleware/validationMiddleware.ts (1)
validateBody(5-31)src/modules/auth/dto/emailVerification.dto.ts (1)
RegisterDTO(11-51)src/modules/auth/dto/login.dto.ts (1)
LoginDto(3-10)src/modules/auth/dto/wallet-verification.dto.ts (2)
WalletVerificationDto(3-10)WalletFormatValidationDto(12-19)src/modules/auth/dto/resendVerificationDTO.ts (1)
ResendVerificationDTO(3-6)
src/modules/user/presentation/controllers/UserController.ts (7)
src/services/UserService.ts (1)
UserService(12-45)src/middleware/validationMiddleware.ts (3)
validateBody(5-31)validateParams(61-87)validateQuery(33-59)src/modules/user/dto/CreateUserDto.ts (1)
CreateUserDto(3-30)src/modules/user/dto/UpdateUserDto.ts (1)
UpdateUserDto(3-33)src/modules/user/dto/UserParams.dto.ts (1)
UserIdParamsDto(3-7)src/modules/user/dto/UserQuery.dto.ts (1)
UserEmailQueryDto(3-7)src/types/auth.types.ts (1)
AuthenticatedRequest(25-28)
src/modules/user/dto/UserParams.dto.ts (1)
src/modules/nft/dto/nft-params.dto.ts (1)
UserIdParamsDto(9-13)
🪛 markdownlint-cli2 (0.17.2)
VALIDATION_IMPLEMENTATION.md
20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (39)
src/modules/certificate/dto/certificate-params.dto.ts (1)
3-7: LGTM! Validation logic is correct.The DTO properly validates UUID parameters with appropriate decorators. The implementation follows the established pattern for parameter validation DTOs.
Note: This DTO follows the same pattern as
UserIdParamsDto, contributing to the code duplication pattern mentioned in other files. Consider the shared DTO approach suggested in the UserParams.dto.ts review.src/modules/user/dto/UserQuery.dto.ts (1)
3-7: LGTM! Excellent email validation implementation.The DTO correctly validates email parameters using appropriate decorators. The combination of
@IsEmail()and@IsNotEmpty()provides robust validation for email query parameters.src/modules/volunteer/dto/volunteer-params.dto.ts (2)
3-7: LGTM! First DTO validation is correct.The
VolunteerIdParamsDtoproperly validates UUID parameters with appropriate decorators.
9-13: LGTM! Second DTO validation is correct.The
VolunteerProjectParamsDtoproperly validates UUID parameters for project IDs with appropriate decorators.Note: Both DTOs in this file follow the same UUID validation pattern seen across multiple modules, contributing to code duplication. Consider the shared DTO approach for common validation patterns.
src/modules/auth/dto/login.dto.ts (1)
1-10: LGTM! Stellar wallet address validation is correctly implemented.The validation decorators are properly chained and the regex pattern
/^G[A-D][A-Z2-7]{54}$/correctly validates Stellar public key format (56 characters total: 'G' prefix + version byte A-D + 54 base32 characters). The custom error message provides clear feedback for invalid addresses.src/modules/certificate/dto/create-certificate.dto.ts (1)
1-19: LGTM! Well-structured certificate validation with appropriate constraints.The validation rules are well-suited for certificate creation:
- eventDate:
@IsDateString()ensures ISO 8601 format consistency- organizationSignature: 2-100 character range accommodates various signature formats
- customMessage: 10-500 character range ensures meaningful content without excessive bloat
The validation decorators are properly applied and the length constraints are reasonable for certificate data storage and display.
src/modules/nft/dto/nft-params.dto.ts (1)
3-7: LGTM! NFT parameter DTO is well-structured.The
NFTIdParamsDtofollows the established validation pattern with appropriate decorators for UUID validation.src/modules/project/dto/ProjectParams.dto.ts (1)
1-13: LGTM! Parameter DTOs are well-implemented.Both
ProjectIdParamsDtoandOrganizationIdParamsDtofollow the established validation pattern with:
- Appropriate UUID validation with
@IsUUID()- Required field validation with
@IsNotEmpty()- Clear, descriptive property names
The implementation is consistent with the broader DTO validation system.
src/modules/nft/dto/create-nft.dto.ts (1)
1-17: LGTM! Comprehensive validation for NFT creation.The
CreateNFTDtoclass implements thorough validation:
- UUID validation for
userIdandorganizationIdensures proper ID format- String validation for
descriptionwith appropriate length constraints (10-500 characters)- Required field validation with
@IsNotEmpty()on all propertiesThe validation rules are well-balanced - not too restrictive but ensuring data quality. The length constraints for description are reasonable for NFT metadata.
src/modules/auth/dto/wallet-verification.dto.ts (1)
3-10: Approve Stellar wallet validation logic.The validation pattern for Stellar wallet addresses is correct:
- Regex
/^G[A-D][A-Z2-7]{54}$/properly validates the 56-character Stellar address format- Custom error message provides clear feedback
- Combines string, non-empty, and format validation appropriately
src/modules/project/presentation/controllers/Project.controller.ts (3)
3-6: LGTM! Proper imports for DTO validation system.The imports correctly bring in the validation middleware and all necessary DTOs for the project module.
11-15: LGTM! Clean validation middleware setup.The validation middleware methods are properly configured as instance properties, making them reusable across route definitions. This follows the established pattern for DTO-based validation.
17-35: LGTM! Proper DTO usage in createProject method.The method correctly:
- Casts
req.bodytoCreateProjectDtoafter validation middleware- Uses DTO properties instead of manual destructuring
- Maintains proper date conversion for
startDateandendDatesrc/modules/volunteer/dto/volunteer.dto.ts (1)
1-30: LGTM! Well-structured validation decorators for CreateVolunteerDTO.The validation rules are comprehensive and appropriate:
- String length constraints prevent both empty and excessively long inputs
- UUID validation ensures proper foreign key format for
projectId- Optional
incentivefield provides flexibilitysrc/modules/organization/presentation/controllers/OrganizationController.ts (2)
15-17: LGTM! Proper validation middleware integration.The validation middleware methods are correctly implemented using the generic
validateBodyfunction with appropriate DTO classes.
21-28: Excellent typed DTO usage and property correction.The casting to
CreateOrganizationDtoprovides type safety, and the property name change fromwallettowebsitealigns with the updated DTO structure.src/modules/auth/presentation/controllers/Auth.controller.ts (4)
18-23: Excellent comprehensive validation middleware setup.All auth operations are properly covered with validation middleware, ensuring type safety and data integrity before controller logic executes.
25-42: LGTM! Proper typed DTO usage in register method.The casting to
RegisterDTOis safe since the validation middleware ensures the request body matches the DTO structure. The property access pattern is clean and type-safe.
67-82: Consistent DTO pattern implementation.The
resendVerificationEmailmethod correctly follows the established pattern of casting to the validated DTO type and accessing properties directly.
84-95: Good wallet address validation integration.The login method properly uses the
LoginDtowith Stellar wallet address validation, ensuring only valid wallet formats are processed.src/modules/user/dto/UpdateUserDto.ts (1)
1-33: Excellent validation decorator implementation for UpdateUserDto.The DTO demonstrates best practices:
- Required
idfield with UUID validation for update operations- All other fields appropriately optional for partial updates
- Comprehensive validation rules including string lengths, email format, and Stellar wallet address pattern
- Consistent validation constraints with other DTOs across the codebase
src/modules/project/dto/CreateProjectDto.ts (1)
4-32: Well-structured DTO with comprehensive validation.The validation decorators are appropriate and the field additions (location, dates) enhance the project model. The renaming from
titletonameimproves consistency.src/modules/auth/dto/emailVerification.dto.ts (5)
6-7: LGTM! Proper imports for enhanced validation.The addition of
MaxLengthandMatchesimports aligns with the comprehensive validation enhancements.
14-20: Good addition of lastName field with consistent validation.The
lastNameproperty follows the same validation pattern as thenamefield, maintaining consistency across the DTO structure.
29-29: Excellent addition of password length constraint.The
@MaxLength(100)decorator prevents potential DoS attacks through excessively long passwords while maintaining reasonable usability.
34-37: Robust Stellar wallet address validation.The regex pattern
/^G[A-D][A-Z2-7]{54}$/correctly validates Stellar wallet addresses and is consistent with other DTOs in the codebase. The custom error message provides clear feedback.
42-42: Proper constructor initialization for new field.The
lastNamefield is correctly initialized with an empty string fallback, maintaining consistency with other optional fields.src/modules/user/presentation/controllers/UserController.ts (6)
6-8: Excellent integration of validation middleware imports.The imports properly bring in the generic validation functions and specific DTOs needed for comprehensive request validation.
13-17: Well-structured validation middleware properties.The validation middleware properties follow a clear naming convention and cover all necessary validation scenarios (body, params, query). This approach makes the validation logic reusable and maintainable.
21-21: Proper typed DTO usage after validation.The direct casting to
CreateUserDtois safe here since the validation middleware ensures the request body conforms to the DTO structure before reaching this handler.
34-35: Consistent parameter validation pattern.The type casting and property access pattern is clean and follows the established convention. The validation middleware ensures
params.idis a valid UUID.
53-54: Clean query parameter handling.The query validation and type casting approach maintains consistency with the parameter validation pattern.
69-72: Proper ID assignment for update operation.The pattern of extracting the ID from validated parameters and assigning it to the DTO ensures the update operation targets the correct resource. This is a secure approach that prevents ID manipulation through the request body.
src/middleware/validationMiddleware.ts (5)
5-31: Robust body validation middleware implementation.The
validateBodyfunction provides comprehensive validation with proper error handling. The structured error response format and graceful error handling make this a solid foundation for API validation.
33-59: Consistent query validation with appropriate type handling.The
validateQueryfunction follows the same robust pattern asvalidateBody. Theas anycasting on line 49 is necessary due to Express's query typing limitations but is safe since the DTO has been validated.
61-87: Well-implemented parameter validation middleware.The
validateParamsfunction completes the validation trilogy with consistent error handling and response formatting. The pattern maintains uniformity across all validation types.
89-99: Clean error formatting utility.The
formatValidationErrorsfunction provides a consistent structure for validation error responses, including field names, constraint violations, and invalid values. This enhances API usability by providing detailed feedback.
8-9: class-transformer security verified ✅The project uses class-transformer v0.5.1 (patched in v0.3.1 for the prototype-pollution advisory), so there are no known vulnerabilities affecting plainToClass on validated DTOs. No further action required.
VALIDATION_IMPLEMENTATION.md (1)
1-200: Comprehensive and well-structured documentation.This documentation provides excellent coverage of the DTO validation system implementation. It includes:
- Clear architectural overview with practical examples
- Complete inventory of DTOs across all modules
- Detailed usage instructions for developers
- Proper documentation of validation features and error formats
- Clear roadmap for next steps
The documentation serves as an excellent reference for the new validation approach and will greatly help with onboarding and maintenance.
| export class WalletFormatValidationDto { | ||
| @IsString() | ||
| @IsNotEmpty() | ||
| @Matches(/^G[A-D][A-Z2-7]{54}$/, { | ||
| message: 'walletAddress must be a valid Stellar wallet address', | ||
| }) | ||
| walletAddress: string; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Eliminate code duplication between wallet DTOs.
WalletFormatValidationDto is identical to WalletVerificationDto above. This creates:
- Code duplication - Same validation logic in multiple classes
- Maintenance overhead - Changes need to be applied in multiple places
- Potential inconsistency - Risk of DTOs diverging over time
Consider these solutions:
- Option 1 (Recommended): Remove the duplicate and use
WalletVerificationDtofor both use cases - Option 2: Create a base class with the common validation and extend it
- Option 3: If different semantics are needed, add documentation explaining the distinction
-export class WalletFormatValidationDto {
- @IsString()
- @IsNotEmpty()
- @Matches(/^G[A-D][A-Z2-7]{54}$/, {
- message: 'walletAddress must be a valid Stellar wallet address',
- })
- walletAddress: string;
-}📝 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.
| export class WalletFormatValidationDto { | |
| @IsString() | |
| @IsNotEmpty() | |
| @Matches(/^G[A-D][A-Z2-7]{54}$/, { | |
| message: 'walletAddress must be a valid Stellar wallet address', | |
| }) | |
| walletAddress: string; | |
| } |
🤖 Prompt for AI Agents
In src/modules/auth/dto/wallet-verification.dto.ts around lines 12 to 19,
WalletFormatValidationDto duplicates the exact validation logic of
WalletVerificationDto, causing code duplication and maintenance issues. To fix
this, remove WalletFormatValidationDto entirely and use WalletVerificationDto
wherever WalletFormatValidationDto was used. Alternatively, if both DTOs must
exist for semantic reasons, create a base class with the shared validation and
have both extend it, or add clear documentation explaining their differences.
| export class UserIdParamsDto { | ||
| @IsUUID() | ||
| @IsNotEmpty() | ||
| userId: string; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Address code duplication with existing UserIdParamsDto.
There's already a UserIdParamsDto class in src/modules/user/dto/UserParams.dto.ts with the same validation logic but using property name id instead of userId. This creates:
- Code duplication - Same class with different property names
- Inconsistency - Different property names (
idvsuserId) for the same concept - Maintenance overhead - Multiple places to update for changes
Consider either:
- Option 1 (Recommended): Remove this duplicate and import the existing
UserIdParamsDtofrom the user module - Option 2: Create a shared base DTO in a common location and extend it in both modules
- Option 3: Standardize the property name across both DTOs
-export class UserIdParamsDto {
- @IsUUID()
- @IsNotEmpty()
- userId: string;
-}And import the existing one:
import { UserIdParamsDto } from '../../user/dto/UserParams.dto';🤖 Prompt for AI Agents
In src/modules/nft/dto/nft-params.dto.ts around lines 9 to 13, there is a
duplicate UserIdParamsDto class with the same validation logic as the one in
src/modules/user/dto/UserParams.dto.ts but with a different property name. To
fix this, remove the local UserIdParamsDto definition and instead import the
existing UserIdParamsDto from the user module using the suggested import
statement. This will eliminate duplication, ensure consistency, and reduce
maintenance overhead.
| export class GetProjectsQueryDto { | ||
| @IsOptional() | ||
| @IsNumberString() | ||
| @Transform(({ value }) => parseInt(value, 10)) | ||
| @Min(1) | ||
| page?: number = 1; | ||
|
|
||
| @IsOptional() | ||
| @IsNumberString() | ||
| @Transform(({ value }) => parseInt(value, 10)) | ||
| @Min(1) | ||
| @Max(100) | ||
| pageSize?: number = 10; | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review decorator order and validation approach for optional numeric query parameters.
The current implementation has potential issues with decorator order and type consistency:
-
Decorator Order: The
@Transformdecorator should be applied before validation decorators like@Min/@Maxto ensure the value is converted before validation. -
Optional Numeric Validation: Using
@IsNumberString()with@IsOptional()may not work as expected for undefined query parameters.
Consider this improved approach:
-import { IsOptional, IsNumberString, Min, Max } from 'class-validator';
-import { Transform } from 'class-transformer';
+import { IsOptional, IsInt, Min, Max } from 'class-validator';
+import { Transform, Type } from 'class-transformer';
export class GetProjectsQueryDto {
@IsOptional()
- @IsNumberString()
- @Transform(({ value }) => parseInt(value, 10))
+ @Type(() => Number)
+ @IsInt()
@Min(1)
page?: number = 1;
@IsOptional()
- @IsNumberString()
- @Transform(({ value }) => parseInt(value, 10))
+ @Type(() => Number)
+ @IsInt()
@Min(1)
@Max(100)
pageSize?: number = 10;
}This approach uses @Type(() => Number) for automatic transformation and @IsInt() for integer validation, which is more appropriate for pagination parameters.
📝 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.
| export class GetProjectsQueryDto { | |
| @IsOptional() | |
| @IsNumberString() | |
| @Transform(({ value }) => parseInt(value, 10)) | |
| @Min(1) | |
| page?: number = 1; | |
| @IsOptional() | |
| @IsNumberString() | |
| @Transform(({ value }) => parseInt(value, 10)) | |
| @Min(1) | |
| @Max(100) | |
| pageSize?: number = 10; | |
| } | |
| import { IsOptional, IsInt, Min, Max } from 'class-validator'; | |
| import { Transform, Type } from 'class-transformer'; | |
| export class GetProjectsQueryDto { | |
| @IsOptional() | |
| @Type(() => Number) | |
| @IsInt() | |
| @Min(1) | |
| page?: number = 1; | |
| @IsOptional() | |
| @Type(() => Number) | |
| @IsInt() | |
| @Min(1) | |
| @Max(100) | |
| pageSize?: number = 10; | |
| } |
🤖 Prompt for AI Agents
In src/modules/project/dto/GetProjectsQuery.dto.ts lines 4 to 17, the decorator
order and validation for optional numeric query parameters are incorrect. Move
the transformation decorator before the validation decorators to ensure the
value is converted before validation. Replace @IsNumberString() with @Type(() =>
Number) to handle automatic type conversion, and use @IsInt() for integer
validation instead of string validation. Keep @IsOptional() to allow undefined
values. This will ensure proper type consistency and validation for the optional
pagination parameters.
| export class UserIdParamsDto { | ||
| @IsUUID() | ||
| @IsNotEmpty() | ||
| id: string; | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Code duplication detected: Consider consolidating duplicate DTOs.
The validation logic is correct and properly validates UUID parameters. However, there's an identical UserIdParamsDto class in src/modules/nft/dto/nft-params.dto.ts with the same property name (userId vs id) but similar validation logic.
Consider creating a shared DTO in a common location to avoid duplication:
+// src/common/dto/shared-params.dto.ts
+import { IsUUID, IsNotEmpty } from 'class-validator';
+
+export class IdParamsDto {
+ @IsUUID()
+ @IsNotEmpty()
+ id: string;
+}Then import and use the shared DTO in both modules.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/modules/user/dto/UserParams.dto.ts lines 3 to 7, the UserIdParamsDto
class duplicates validation logic found in
src/modules/nft/dto/nft-params.dto.ts. To fix this, create a shared DTO with the
common UUID validation in a central location accessible by both modules, then
replace the existing UserIdParamsDto and the similar NFT DTO with imports from
this shared DTO to eliminate duplication.
1 participant
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
This PR enhances input validation across the user, project, and organization modules by replacing the existing
express-validatorlogic with a DTO-based validation approach usingclass-validator. Each controller route now uses typed DTOs with proper@Is*decorators to validate and sanitize incoming data. This shift aligns the API layer with our Domain-Driven Design (DDD) architecture and ensures that all inputs are clean, consistent, and secure before any processing occurs. Controllers were updated to integrate this mechanism with robust error handling for invalid inputs. The refactor ensures long-term maintainability and improved input hygiene throughout the application.⏰ Time spent breakdown
⏳ Total: ~4h 30min
🌌 Comments
Thank you for contributing to Volunchain, we are glad that you have chosen us as your project of choice and we hope that you continue to contribute to this great project, so that together we can make our mark at the top!
Summary by CodeRabbit
New Features
Documentation
Refactor