Added certificate and user certificate module #23
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes add two modules to the application: one for certificate management and another for user certificate enrollment and management. The new CertificateModule includes a controller, service, DTO, and an entity to handle operations such as generating DIDs, creating credential schemas, templates, issuing, and rendering certificates. The UserCertificateModule provides endpoints and a service to enroll users in courses, update and fetch certificate statuses, and search courses. Both modules integrate with TypeORM using a shared certificate entity while being incorporated into the main AppModule. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CertController as CertificateController
participant CertService as CertificateService
participant ExternalAPI as External API
Client->>CertController: POST /certificate/generateDid
CertController->>CertService: generateDid(userId)
CertService->>ExternalAPI: POST request to generate DID
ExternalAPI-->>CertService: DID response
CertService-->>CertController: returns generated DID
CertController-->>Client: Response with DID
sequenceDiagram
participant Client
participant UserCertController as UserCertificateController
participant UserCertService as UserCertificateService
participant Database as TypeORM Repository
Client->>UserCertController: POST /user_certificate/enrollUserForCourse
UserCertController->>UserCertService: enrollUserForCourse(dto)
UserCertService->>Database: Check for existing certificate
Database-->>UserCertService: Existing record or empty
UserCertService->>Database: Insert new certificate record
Database-->>UserCertService: Confirmation
UserCertService-->>UserCertController: Operation result
UserCertController-->>Client: Success/Error response
✨ 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 (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (14)
src/modules/user_certificate/user_certificate.service..ts (2)
27-27: Use 'const' instead of 'let' for variables not reassignedVariables like
apiIdanddataare never reassigned. Declaring them withconstclarifies immutability:- let apiId = 'api.create.userEnrollment'; + const apiId = 'api.create.userEnrollment'; - let data = new UserCourseCertificate(); + const data = new UserCourseCertificate();Also applies to: 40-40, 88-88, 112-112, 154-154, 208-208
🧰 Tools
🪛 ESLint
[error] 27-27: 'apiId' is never reassigned. Use 'const' instead.
(prefer-const)
206-206: Remove unused 'request' parameterIn
searchUsersCourses,requestis never read. Remove it to streamline function parameters:- async searchUsersCourses(searchObj, response, request) { + async searchUsersCourses(searchObj, response) {🧰 Tools
🪛 ESLint
[error] 206-206: 'request' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/modules/certificate/certificate.service.ts (4)
20-20: Use 'const' instead of 'let' for 'apiId'
apiIdvariables are never reassigned, so they should be declared withconst:- let apiId = 'api.generate.did'; + const apiId = 'api.generate.did';Also applies to: 188-188
🧰 Tools
🪛 ESLint
[error] 20-20: 'apiId' is never reassigned. Use 'const' instead.
(prefer-const)
191-191: Use 'const' for 'learnerDid'
learnerDidis never reassigned, reflecting immutability more accurately withconst:- let learnerDid = await this.generateDidByUserId(issueCredential.userId); + const learnerDid = await this.generateDidByUserId(issueCredential.userId);🧰 Tools
🪛 ESLint
[error] 191-191: 'learnerDid' is never reassigned. Use 'const' instead.
(prefer-const)
232-232: Remove or utilize 'updateResponse'
updateResponseis declared but not referenced. Remove it if unnecessary:- const updateResponse = await this.updateUserCertificate({ ... });🧰 Tools
🪛 ESLint
[error] 232-232: 'updateResponse' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
283-283: Use 'const' instead of 'let' for 'url'
urlis never reassigned, soconstis more consistent and clearer:- let url = ... + const url = ...🧰 Tools
🪛 ESLint
[error] 283-283: 'url' is never reassigned. Use 'const' instead.
(prefer-const)
src/modules/user_certificate/dto/create-user-certificate-dto.ts (1)
25-27: Add a transformer for the 'issuedOn' date fieldWhen using
@IsDate(), apply@Type(() => Date)fromclass-transformerto handle JSON string-to-date conversion:+ import { Type } from 'class-transformer'; @IsOptional() + @Type(() => Date) @IsDate() issuedOn?: Date;src/modules/certificate/dto/issue-certificate-dto.ts (3)
20-22: Remove redundant validation on userId.The
@IsString()decorator is redundant here since@IsUUID()already ensures the value is a valid UUID string.@IsUUID() - @IsString() userId: string;
24-25: Add UUID validation for courseId.Consider adding
@IsUUID()validation to ensure courseId is a valid UUID.+ @IsUUID() @IsString() courseId: string;
3-29: Add validation messages for better error handling.Consider adding custom validation messages to improve error feedback.
Example:
@IsDateString({}, { + message: 'Issuance date must be a valid ISO date string' }) issuanceDate: string;src/modules/user_certificate/user_certificate.module.ts (1)
7-7: Use relative paths for imports within the same module.Consider using relative paths for consistency when importing from the same module.
- import { LoggerService } from 'src/common/logger/logger.service'; + import { LoggerService } from '../../../common/logger/logger.service';src/modules/certificate/certificate.module.ts (1)
6-7: Use relative paths for imports within the same module.Consider using relative paths for consistency when importing from the same module.
- import { LoggerService } from 'src/common/logger/logger.service'; - import { AxiosRequest } from 'src/common/middleware/axios.middleware'; + import { LoggerService } from '../../common/logger/logger.service'; + import { AxiosRequest } from '../../common/middleware/axios.middleware';src/modules/certificate/entities/user_course_certificate.ts (1)
9-45: Add indexes for better query performance.Consider adding indexes for frequently queried columns like
userId,courseId, andstatus.+ import { Index } from 'typeorm'; @Entity({ name: 'user_course_certificate' }) + @Index(['userId']) + @Index(['courseId']) + @Index(['status']) export class UserCourseCertificate { // ... existing code ... }src/modules/certificate/certificate.contoller.ts (1)
70-70: Use consistent async/await pattern.Some methods use await while others don't. Since these are asynchronous operations, consistently use await:
-return this.certificateService.generateDid( +return await this.certificateService.generateDid( -return this.certificateService.createCredentialSchema( +return await this.certificateService.createCredentialSchema( -return this.certificateService.createTemplate( +return await this.certificateService.createTemplate(Also applies to: 85-85
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/app.module.ts(2 hunks)src/modules/certificate/certificate.contoller.ts(1 hunks)src/modules/certificate/certificate.module.ts(1 hunks)src/modules/certificate/certificate.service.ts(1 hunks)src/modules/certificate/dto/issue-certificate-dto.ts(1 hunks)src/modules/certificate/entities/user_course_certificate.ts(1 hunks)src/modules/user_certificate/dto/create-user-certificate-dto.ts(1 hunks)src/modules/user_certificate/user_certificate.contoller.ts(1 hunks)src/modules/user_certificate/user_certificate.module.ts(1 hunks)src/modules/user_certificate/user_certificate.service..ts(1 hunks)
🧰 Additional context used
🪛 ESLint
src/modules/certificate/certificate.contoller.ts
[error] 1-1: 'UseGuards' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/modules/certificate/certificate.service.ts
[error] 20-20: 'apiId' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 188-188: 'apiId' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 191-191: 'learnerDid' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 232-232: 'updateResponse' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 283-283: 'url' is never reassigned. Use 'const' instead.
(prefer-const)
src/modules/user_certificate/user_certificate.service..ts
[error] 11-11: 'axios' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: Require statement not part of import statement.
(@typescript-eslint/no-var-requires)
[error] 27-27: 'apiId' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 40-40: 'data' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 88-88: 'apiId' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 112-112: 'updateResult' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 154-154: 'apiId' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 206-206: 'request' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 208-208: 'apiId' is never reassigned. Use 'const' instead.
(prefer-const)
🔇 Additional comments (2)
src/modules/user_certificate/user_certificate.service..ts (1)
215-235: Verify dynamic filters to prevent potential SQL injectionAlthough TypeORM uses parameterized queries, confirm
filtersdoes not allow injection of raw SQL. I can generate a verification script if needed.src/app.module.ts (1)
10-11: LGTM!The new certificate modules are properly imported and integrated into the AppModule following the established pattern.
Also applies to: 20-21
| 'User already enrolled for course', | ||
| HttpStatus.OK, |
There was a problem hiding this comment.
Return a correct status code for error responses
These blocks return HttpStatus.OK while delivering an error message, which can confuse API clients. Consider HttpStatus.CONFLICT or HttpStatus.BAD_REQUEST instead:
- HttpStatus.OK,
+ HttpStatus.CONFLICT,Also applies to: 134-137, 189-190
| import APIResponse from 'src/common/utils/response'; | ||
| import { LoggerService } from 'src/common/logger/logger.service'; | ||
| import { ConfigService } from '@nestjs/config'; | ||
| const axios = require('axios'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused 'axios' import to comply with best practices
ESLint complains that axios is declared but never used. If you don’t need it, remove it:
- const axios = require('axios');📝 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.
| const axios = require('axios'); |
🧰 Tools
🪛 ESLint
[error] 11-11: 'axios' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: Require statement not part of import statement.
(@typescript-eslint/no-var-requires)
| import { UserCertificateController } from './user_certificate.contoller'; | ||
| import { UserCertificateService } from './user_certificate.service.'; |
There was a problem hiding this comment.
Fix typos in import paths.
There are typos in the import paths:
.contollershould be.controller.service.has an extra dot
- import { UserCertificateController } from './user_certificate.contoller';
- import { UserCertificateService } from './user_certificate.service.';
+ import { UserCertificateController } from './user_certificate.controller';
+ import { UserCertificateService } from './user_certificate.service';📝 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.
| import { UserCertificateController } from './user_certificate.contoller'; | |
| import { UserCertificateService } from './user_certificate.service.'; | |
| import { UserCertificateController } from './user_certificate.controller'; | |
| import { UserCertificateService } from './user_certificate.service'; |
| @@ -0,0 +1,14 @@ | |||
| import { Module } from '@nestjs/common'; | |||
| import { CertificateService } from './certificate.service'; | |||
| import { CertificateController } from './certificate.contoller'; | |||
There was a problem hiding this comment.
Fix typo in controller import path.
There's a typo in the import path: .contoller should be .controller
- import { CertificateController } from './certificate.contoller';
+ import { CertificateController } from './certificate.controller';📝 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.
| import { CertificateController } from './certificate.contoller'; | |
| import { CertificateController } from './certificate.controller'; |
| @Column() | ||
| courseId: string; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent UUID type for ID columns.
The courseId and certificateId columns should use the UUID type for consistency with other ID columns.
@Column()
- courseId: string;
+ @Column({ type: 'uuid' })
+ courseId: string;
@Column()
- certificateId: string;
+ @Column({ type: 'uuid' })
+ certificateId: string;Also applies to: 23-24
| @ApiBadRequestResponse({ description: 'Bad Request.' }) | ||
| @Post('status/search') | ||
| async searchUsersCourses( | ||
| @Body() searchObj: Record<string, any>, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Define a DTO for search parameters.
Using Record<string, any> loses type safety and validation capabilities. Create a proper DTO to define and validate the search parameters.
Example:
// search-courses.dto.ts
export class SearchCoursesDto {
@IsOptional()
@IsString()
courseId?: string;
@IsOptional()
@IsString()
userId?: string;
// Add other search parameters
}| ApiOkResponse, | ||
| } from '@nestjs/swagger'; | ||
| import { Response } from 'express'; | ||
| import { UserCertificateService } from './user_certificate.service..js'; |
There was a problem hiding this comment.
Fix the import path typo.
The service import has a typo in the file extension and an extra dot:
-import { UserCertificateService } from './user_certificate.service..js';
+import { UserCertificateService } from './user_certificate.service';📝 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.
| import { UserCertificateService } from './user_certificate.service..js'; | |
| import { UserCertificateService } from './user_certificate.service'; |
| @ApiBadRequestResponse({ description: 'Bad Request.' }) | ||
| @Post('generateDid') | ||
| async generateDid( | ||
| @Body() createCertificateDto: any, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Define proper DTOs for request validation.
Using any type loses type safety and validation capabilities. Create proper DTOs for each operation:
// generate-did.dto.ts
export class GenerateDidDto {
@IsString()
@IsNotEmpty()
userId: string;
}
// create-schema.dto.ts
export class CreateSchemaDto {
@IsObject()
@IsNotEmpty()
schema: Record<string, unknown>;
}
// create-template.dto.ts
export class CreateTemplateDto {
@IsString()
@IsNotEmpty()
schemaId: string;
@IsObject()
@IsNotEmpty()
template: Record<string, unknown>;
}
// render-certificate.dto.ts
export class RenderCertificateDto {
@IsString()
@IsNotEmpty()
credentialId: string;
@IsString()
@IsNotEmpty()
templateId: string;
}Also applies to: 35-35, 50-50, 82-82
| async generateDid( | ||
| @Body() createCertificateDto: any, | ||
| @Res() response: Response, | ||
| ) { | ||
| return this.certificateService.generateDid( | ||
| createCertificateDto.userId, | ||
| response, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Follow NestJS best practices for response handling.
Similar to the user certificate controller, avoid using Express's Response object directly and let NestJS handle the response transformation:
Example refactor:
async generateDid(
- @Body() createCertificateDto: any,
+ @Body() createCertificateDto: GenerateDidDto,
- @Res() response: Response,
) {
- return this.certificateService.generateDid(
+ return await this.certificateService.generateDid(
createCertificateDto.userId,
- response,
);
}Apply similar changes to other methods.
Also applies to: 34-42, 49-58, 65-75, 81-90
| @@ -0,0 +1,91 @@ | |||
| import { Controller, Post, Body, Res, UseGuards, Req } from '@nestjs/common'; | |||
There was a problem hiding this comment.
Remove unused import.
The UseGuards import is not used in this file.
-import { Controller, Post, Body, Res, UseGuards, Req } from '@nestjs/common';
+import { Controller, Post, Body, Res, Req } from '@nestjs/common';📝 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.
| import { Controller, Post, Body, Res, UseGuards, Req } from '@nestjs/common'; | |
| import { Controller, Post, Body, Res, Req } from '@nestjs/common'; |
🧰 Tools
🪛 ESLint
[error] 1-1: 'UseGuards' is defined but never used.
(@typescript-eslint/no-unused-vars)
No description provided.