feat: implement-repository-for-user #188
Conversation
WalkthroughDependency injection introduced for user repository usage across middleware, controllers, and tests. Added UserPrismaRepository implementation, UserDTOs, and updated IUserRepository signatures to DTO-based types. Replaced direct Prisma references with DI container wiring. Added UserController with CRUD endpoints and comprehensive repository and integration tests. Removed legacy PrismaUserRepository and an old test. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Router as Express Router
participant Controller as UserController
participant UseCase as UseCases
participant Container as DI Container
participant Repo as IUserRepository (UserPrismaRepository)
participant DB as Prisma
Client->>Router: HTTP request (e.g., POST /users)
Router->>Controller: Route handler
Controller->>Container: Resolve USER_REPOSITORY
Container-->>Controller: IUserRepository
Controller->>UseCase: execute(dto)
UseCase->>Repo: create/find/update/delete
Repo->>DB: prisma.user.*(...)
DB-->>Repo: Result / Error
Repo-->>UseCase: DTO / Domain Error
UseCase-->>Controller: DTO / Error
Controller-->>Client: HTTP response (200/201/4xx/5xx)
note over Repo,DB: Prisma errors mapped to domain errors<br/>P2002→Conflict, P2025→NotFound, Validation→ValidationError
sequenceDiagram
autonumber
actor Client
participant Middleware as authMiddleware
participant Container as DI Container
participant Repo as IUserRepository
Client->>Middleware: Request with token
Middleware->>Repo: (via Container) findById/Email
Repo-->>Middleware: UserDTO or null
Middleware-->>Client: next()/401/403
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/modules/wallet/__tests__/WalletAuthIntegration.test.ts (2)
104-137:mockImplementationused on a non-function; stub methods on the mock object instead.
container[USER_REPOSITORY]is an object, not a constructor/function. Assign method mocks directly.- const { prisma } = require("../../../config/prisma"); - const mockUserRepository = container[Symbol.for("USER_REPOSITORY")] as any; - const mockSendEmailUseCase = - require("../../../modules/auth/use-cases/send-verification-email.usecase").SendVerificationEmailUseCase; + const { container, USER_REPOSITORY } = require("../../../shared/infrastructure/container"); + const mockUserRepository = container[USER_REPOSITORY] as any; ... - const mockSendEmailUseCaseInstance = { - execute: jest.fn().mockResolvedValue({}), - }; - - mockUserRepository.mockImplementation(() => mockUserRepositoryInstance); - mockSendEmailUseCase.mockImplementation( - () => mockSendEmailUseCaseInstance - ); + Object.assign(mockUserRepository, mockUserRepositoryInstance);If you need to mock
SendVerificationEmailUseCase, add at top:jest.mock("../../../modules/auth/use-cases/send-verification-email.usecase", () => ({ SendVerificationEmailUseCase: jest.fn().mockImplementation(() => ({ execute: jest.fn().mockResolvedValue({}) })), }));
181-214: SamemockImplementationissue here; adjust similarly.Mirror the fix by assigning method stubs directly.
- const { prisma } = require("../../../config/prisma"); - const mockUserRepository = container[Symbol.for("USER_REPOSITORY")] as any; + const { container, USER_REPOSITORY } = require("../../../shared/infrastructure/container"); + const mockUserRepository = container[USER_REPOSITORY] as any; ... - mockUserRepository.mockImplementation(() => mockUserRepositoryInstance); + Object.assign(mockUserRepository, mockUserRepositoryInstance);src/modules/user/__tests__/controllers/UserController.int.test.ts (1)
22-39: DTO mocks break constructor usageController calls new CreateUserDto()/new UpdateUserDto() then Object.assign(...). Your function-style mocks expect an argument and will throw. Replace with no-op classes.
-jest.mock("../../dto/CreateUserDto", () => { - return { - CreateUserDto: function (data: Record<string, unknown>) { - // Mock validation logic - if (!data.email) throw new Error("Email is required"); - return { email: data.email, ...data }; - }, - }; -}); -jest.mock("../../dto/UpdateUserDto", () => { - return { - UpdateUserDto: function (data: Record<string, unknown>) { - // Mock basic validation similar to CreateUserDto - return { ...data }; - }, - }; -}); +jest.mock("../../dto/CreateUserDto", () => ({ + CreateUserDto: class {}, +})); +jest.mock("../../dto/UpdateUserDto", () => ({ + UpdateUserDto: class {}, +}));
🧹 Nitpick comments (18)
src/middleware/authMiddleware.ts (1)
59-76: Use repository accessor for easier test-time swapping.Minor DI improvement: use
getUserRepository()here too for consistency.- const isVerified = await userRepository.isUserVerified( + const isVerified = await getUserRepository().isUserVerified( authenticatedReq.user.id.toString() );Also applies to: 86-96
src/modules/wallet/__tests__/WalletAuthIntegration.test.ts (1)
16-28: Mock container correctly; export and use the token to avoid Symbol.for duplication.Import and use
USER_REPOSITORYto reference the same symbol consistently.-jest.mock("../../../shared/infrastructure/container", () => ({ - container: { - [Symbol.for("USER_REPOSITORY")]: { +jest.mock("../../../shared/infrastructure/container", () => { + const USER_REPOSITORY = Symbol.for("USER_REPOSITORY"); + return { + container: { + [USER_REPOSITORY]: { findById: jest.fn(), findByEmail: jest.fn(), create: jest.fn(), update: jest.fn(), delete: jest.fn(), - }, - }, - USER_REPOSITORY: Symbol.for("USER_REPOSITORY"), -})); + }, + }, + USER_REPOSITORY, + }; +});src/modules/auth/presentation/controllers/Auth.controller.ts (2)
61-70: Avoid user enumeration; unify response for “send verification email.”Leaking “User not found” enables email enumeration. Return 200 with a generic message regardless of existence; log internally.
try { // Send verification email to provided address await sendVerificationEmailUseCase.execute({ email: dto.email }); res.status(200).json({ message: "If the email exists, a verification link was sent" }); } catch (err) { - const message = - err instanceof Error ? err.message : "Failed to send verification email"; - const status = message === "User not found" ? 400 : 500; - res.status(status).json({ error: message }); + console.error("Failed to send verification email:", err); + // Same generic response to prevent enumeration + res.status(200).json({ message: "If the email exists, a verification link was sent" }); }
88-99: Consistency with resend endpoint.Apply the same generic 200 response to
resendVerificationEmailfor parity and to prevent enumeration.try { // Resends verification email to provided address await resendVerificationEmailUseCase.execute({ email: dto.email }); - res.status(200).json({ message: "Verification email resent" }); + res.status(200).json({ message: "If the email exists, a verification link was sent" }); } catch (err) { - const message = - err instanceof Error - ? err.message - : "Failed to resend verification email"; - const status = message === "User not found" ? 404 : 500; - res.status(status).json({ error: message }); + console.error("Failed to resend verification email:", err); + res.status(200).json({ message: "If the email exists, a verification link was sent" }); }src/modules/user/__tests__/repositories/user-prisma.repository.test.ts (2)
55-66: Avoid asserting undefined properties in Prisma create() payloadPrisma drops undefined fields; strict equality here is fragile. Assert only required shape.
- expect(mockPrisma.user.create).toHaveBeenCalledWith({ - data: { - name: createUserData.name, - lastName: createUserData.lastName, - email: createUserData.email, - password: createUserData.password, - wallet: createUserData.wallet, - isVerified: false, - verificationToken: undefined, - verificationTokenExpires: undefined, - } - }); + expect(mockPrisma.user.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + name: createUserData.name, + lastName: createUserData.lastName, + email: createUserData.email, + password: createUserData.password, + wallet: createUserData.wallet, + isVerified: false, + }), + }) + );
270-329: Cover remaining repository methodsAdd unit tests for: findByVerificationToken, setVerificationToken, isUserVerified, verifyUser (success + error paths). This closes gaps around the email verification flow.
src/modules/user/readme.md (2)
7-23: Specify language for fenced code block (markdownlint MD040)Add a language to the architecture tree fence for lint compliance and readability.
-``` +```text ... -``` +```
27-33: Tighten wording on error mappingMinor grammar/style: use consistent punctuation and sentence case.
-- **Error Mapping**: Prisma errors are mapped to domain exceptions: - - P2002 → ResourceConflictError (409) - - P2025 → ResourceNotFoundError (404) - - Validation errors → ValidationError (400) +- Error mapping: Prisma errors map to domain exceptions: + - P2002 → ResourceConflictError (409) + - P2025 → ResourceNotFoundError (404) + - Validation errors → ValidationError (400)src/modules/user/__tests__/controllers/UserController.int.test.ts (1)
97-114: Prefer domain error classes in testsConsider throwing domain errors (ResourceConflictError, ValidationError, ResourceNotFoundError) from the mocked repository to validate controller mapping deterministically, instead of string-matching.
Also applies to: 241-260, 273-281
src/modules/user/use-cases/userUseCase.ts (1)
63-69: Annotate return type of GetUsersUseCase.execute for clarityReturn Promise<{ users: UserDTO[]; total: number }>.
- async execute(page: number = 1, pageSize: number = 10) { + async execute(page: number = 1, pageSize: number = 10): Promise<{ users: UserDTO[]; total: number }> {src/modules/user/presentation/controllers/UserController.ts (2)
42-45: getUserById: remove unreachable null checkUse case throws on not found; this branch never executes.
- if (!user) { - return res.status(404).json({ error: "User not found" }); - }
67-70: getUserByEmail: remove unreachable null check- if (!user) { - return res.status(404).json({ error: "User not found" }); - }src/modules/user/domain/interfaces/IUserRepository.ts (1)
10-13: Consider encoding pagination invariants in typesOptional: introduce a
PaginationParams/PaginatedResult<T>type and document thatpage >= 1,pageSize >= 1.src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts (5)
100-110: Token lookups should consider expiryIf you rely on this at verification time, ensure expired tokens are ignored here or validated by the service. Otherwise, expired tokens may be accepted.
Example:
-const user = await this.prisma.user.findFirst({ - where: { verificationToken: token } -}); +const user = await this.prisma.user.findFirst({ + where: { + verificationToken: token, + verificationTokenExpires: { gt: new Date() } + } +});
154-164:isUserVerifiedmasks missing users by returning falseReturning
falsefor a non-existent user can hide bugs. Consider throwingResourceNotFoundError(or returnnulland adjust interface).
181-194: TypetoDTOinput to improve safety and editor helpAvoid
any. Type it as a pick of the PrismaUserfields used here.Apply:
-import { PrismaClient, Prisma } from "@prisma/client"; +import { PrismaClient, Prisma, type User as PrismaUser } from "@prisma/client"; @@ - private toDTO(user: any): UserDTO { + private toDTO( + user: Pick< + PrismaUser, + "id" | "name" | "lastName" | "email" | "wallet" | "isVerified" | + "verificationToken" | "verificationTokenExpires" | "createdAt" | "updatedAt" + > + ): UserDTO {If you adopt
selectin queries, this type will match the selected shape.
196-214: Improve unique-conflict messaging and narrow error typeSurface the exact unique fields from
error.meta.targetwhen available, and acceptunknownfor safety.Apply:
- private handlePrismaError(error: any): never { + private handlePrismaError(error: unknown): never { if (error instanceof Prisma.PrismaClientKnownRequestError) { switch (error.code) { case 'P2002': - throw new ResourceConflictError('A user with this email or wallet already exists'); + { + const target = (error.meta && (error.meta as any).target) || []; + const fields = Array.isArray(target) ? target.join(", ") : String(target ?? "unique field"); + throw new ResourceConflictError(`Unique constraint violated on ${fields}`); + } case 'P2025': throw new ResourceNotFoundError('User not found'); default: throw new ValidationError(`Database operation failed: ${error.message}`); } }
112-125: Add index on createdAt for pagination performance
Define@@index([createdAt])on theUsermodel inprisma/schema.prismato optimizeorderBy: { createdAt: "desc" }queries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
src/middleware/authMiddleware.ts(1 hunks)src/modules/auth/presentation/controllers/Auth.controller.ts(1 hunks)src/modules/user/__tests__/controllers/UserController.int.test.ts(4 hunks)src/modules/user/__tests__/repositories/user-prisma.repository.test.ts(1 hunks)src/modules/user/__tests__/user.test.ts(0 hunks)src/modules/user/domain/dto/UserDTO.ts(1 hunks)src/modules/user/domain/interfaces/IUserRepository.ts(1 hunks)src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts(1 hunks)src/modules/user/presentation/controllers/UserController.ts(1 hunks)src/modules/user/readme.md(1 hunks)src/modules/user/repositories/PrismaUserRepository.ts(0 hunks)src/modules/user/use-cases/userUseCase.ts(3 hunks)src/modules/wallet/__tests__/WalletAuthIntegration.test.ts(4 hunks)src/shared/infrastructure/container.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/modules/user/tests/user.test.ts
- src/modules/user/repositories/PrismaUserRepository.ts
🧰 Additional context used
🧬 Code graph analysis (9)
src/shared/infrastructure/container.ts (2)
src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts (1)
UserPrismaRepository(20-215)src/modules/user/domain/interfaces/IUserRepository.ts (1)
IUserRepository(3-21)
src/modules/wallet/__tests__/WalletAuthIntegration.test.ts (1)
src/shared/infrastructure/container.ts (1)
container(10-13)
src/middleware/authMiddleware.ts (2)
src/shared/infrastructure/container.ts (2)
container(10-13)USER_REPOSITORY(8-8)src/modules/user/domain/interfaces/IUserRepository.ts (1)
IUserRepository(3-21)
src/modules/user/presentation/controllers/UserController.ts (5)
src/modules/user/domain/interfaces/IUserRepository.ts (1)
IUserRepository(3-21)src/shared/infrastructure/container.ts (2)
container(10-13)USER_REPOSITORY(8-8)src/modules/user/dto/CreateUserDto.ts (1)
CreateUserDto(10-41)src/modules/user/use-cases/userUseCase.ts (5)
CreateUserUseCase(7-24)GetUserByIdUseCase(26-41)GetUserByEmailUseCase(43-58)UpdateUserUseCase(80-94)DeleteUserUseCase(72-78)src/modules/user/dto/UpdateUserDto.ts (1)
UpdateUserDto(10-45)
src/modules/user/__tests__/repositories/user-prisma.repository.test.ts (3)
tests/__mocks__/prisma.ts (1)
PrismaClient(1-3)src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts (1)
UserPrismaRepository(20-215)src/modules/user/domain/dto/UserDTO.ts (2)
CreateUserData(16-25)UpdateUserData(28-37)
src/modules/auth/presentation/controllers/Auth.controller.ts (2)
src/shared/infrastructure/container.ts (2)
container(10-13)USER_REPOSITORY(8-8)src/modules/user/domain/interfaces/IUserRepository.ts (1)
IUserRepository(3-21)
src/modules/user/use-cases/userUseCase.ts (4)
src/modules/user/domain/interfaces/IUserRepository.ts (1)
IUserRepository(3-21)src/modules/user/dto/CreateUserDto.ts (1)
CreateUserDto(10-41)src/modules/user/domain/dto/UserDTO.ts (3)
UserDTO(2-13)CreateUserData(16-25)UpdateUserData(28-37)src/modules/user/dto/UpdateUserDto.ts (1)
UpdateUserDto(10-45)
src/modules/user/domain/interfaces/IUserRepository.ts (1)
src/modules/user/domain/dto/UserDTO.ts (3)
UserDTO(2-13)CreateUserData(16-25)UpdateUserData(28-37)
src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts (3)
src/modules/user/domain/interfaces/IUserRepository.ts (1)
IUserRepository(3-21)src/modules/user/domain/dto/UserDTO.ts (3)
UserDTO(2-13)CreateUserData(16-25)UpdateUserData(28-37)src/modules/user/repositories/PrismaUserRepository.ts (1)
PrismaUserRepository(7-80)
🪛 LanguageTool
src/modules/user/readme.md
[grammar] ~29-~29: There might be a mistake here.
Context: ... errors are mapped to domain exceptions: - P2002 → ResourceConflictError (409) - ...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ... - P2002 → ResourceConflictError (409) - P2025 → ResourceNotFoundError (404) - ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ... - P2025 → ResourceNotFoundError (404) - Validation errors → ValidationError (400...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ory interface - Repository is injected via DI container (USER_REPOSITORY` token) ...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...# Database Model Uses the User model from Prisma schema (not the legacy users t...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...name, lastName, email, password, wallet-isVerified, verificationToken, verificationToken...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
src/modules/user/readme.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (5)
src/modules/wallet/__tests__/WalletAuthIntegration.test.ts (1)
48-69: Use repository abstraction in WalletAuthIntegration tests instead of mocking Prisma
Tests import and mock the Prisma client directly, coupling them to the ORM and bypassing the DI‐based repository layer. After addingfindByWalletto your user repository, remove therequire("../../../config/prisma")import and replace:- const { prisma } = require("../../../config/prisma"); - prisma.user.findUnique.mockResolvedValue({ … }); + const { container, USER_REPOSITORY } = require("../../../shared/infrastructure/container"); + const userRepo = container[USER_REPOSITORY]; + userRepo.findByWallet.mockResolvedValue({ … }); … - expect(prisma.user.findUnique).toHaveBeenCalledWith({ where: { wallet: validWalletAddress } }); + expect(userRepo.findByWallet).toHaveBeenCalledWith(validWalletAddress);Run a quick grep to confirm no other tests still import Prisma directly:
rg -n --type=ts -g '!tests/**' -g '!**/infrastructure/repositories/**' "config/prisma"src/modules/auth/presentation/controllers/Auth.controller.ts (1)
16-26: LGTM on DI usage.Controller now depends on
IUserRepositoryvia the container; aligns with the modular architecture.src/modules/user/domain/interfaces/IUserRepository.ts (2)
4-9: DTO-based repository interface looks consistent and clearSignatures align with the PR objective (DTO in/out, explicit id param on update). Good separation for verification token helpers and pagination.
Also applies to: 13-21
1-1: DTO update call sites verified; confirm findAll usage and test coverage
- No legacy
.update({…})calls remain; allIUserRepository.updateinvocations use the(id, data)signature.- Ensure any
findAllcalls in service/controller layers return the new DTO array shape and are covered by tests.src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts (1)
47-66: Ensure passwords are pre-hashed upstream and use??for boolean defaults
- Confirm
CreateUserData.passwordis a secure hash (e.g. from bcrypt/argon2) before calling the repository; otherwise hash it in the service layer.- Change
to avoid coercing- isVerified: data.isVerified || false, + isVerified: data.isVerified ?? false,false.- (Optional) Rename
passwordtopasswordHashin your DTOs and here for stronger clarity.
| const SECRET_KEY = process.env.JWT_SECRET || "defaultSecret"; | ||
| const userRepository = new PrismaUserRepository(); | ||
| const userRepository = container[USER_REPOSITORY] as IUserRepository; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden JWT handling: no default secret, restrict algorithms, explicit expiry handling.
Using a default secret is unsafe; restrict accepted algs and handle TokenExpiredError distinctly.
-const SECRET_KEY = process.env.JWT_SECRET || "defaultSecret";
-const userRepository = container[USER_REPOSITORY] as IUserRepository;
+const SECRET_KEY = process.env.JWT_SECRET;
+const getUserRepository = (): IUserRepository =>
+ (container[USER_REPOSITORY] as IUserRepository);
...
try {
- const decoded = jwt.verify(token, SECRET_KEY) as {
+ if (!SECRET_KEY) {
+ res.status(500).json({ message: "Server misconfiguration: JWT secret missing" });
+ return;
+ }
+ const decoded = jwt.verify(token, SECRET_KEY, {
+ algorithms: ["HS256"], // adjust if you use RS256
+ }) as {
id: string;
role: string;
};
- const user = await userRepository.findById(`${decoded.id}`);
+ const user = await getUserRepository().findById(`${decoded.id}`);
...
- } catch (error) {
+ } catch (error) {
console.error("Error during authentication:", error);
- res.status(401).json({ message: "Invalid token" });
+ if (error instanceof (jwt as any).TokenExpiredError) {
+ res.status(401).json({ message: "Token expired" });
+ return;
+ }
+ res.status(401).json({ message: "Invalid token" });
}Notes:
- If you use RS256, swap the algorithm accordingly or make it configurable.
Also applies to: 26-31, 52-56
| expect(res.status).toBe(201); | ||
| expect(res.body).toEqual(mockUser); | ||
| }); |
There was a problem hiding this comment.
Dates in JSON responses are strings; assertions currently compare to Date objects
Express serializes Date to ISO strings. Loosen assertions to shape + string dates.
- expect(res.body).toEqual(mockUser);
+ expect(res.body).toEqual(
+ expect.objectContaining({
+ id: mockUser.id,
+ email: mockUser.email,
+ name: mockUser.name,
+ lastName: mockUser.lastName,
+ wallet: mockUser.wallet,
+ isVerified: mockUser.isVerified,
+ createdAt: expect.any(String),
+ updatedAt: expect.any(String),
+ })
+ );- expect(res.body).toEqual(mockUser);
+ expect(res.body).toEqual(
+ expect.objectContaining({
+ id: mockUser.id,
+ email: mockUser.email,
+ name: mockUser.name,
+ lastName: mockUser.lastName,
+ wallet: mockUser.wallet,
+ isVerified: mockUser.isVerified,
+ createdAt: expect.any(String),
+ updatedAt: expect.any(String),
+ })
+ );- expect(res.body).toEqual(mockUser);
+ expect(res.body).toEqual(
+ expect.objectContaining({
+ id: mockUser.id,
+ email: mockUser.email,
+ name: mockUser.name,
+ lastName: mockUser.lastName,
+ wallet: mockUser.wallet,
+ isVerified: mockUser.isVerified,
+ createdAt: expect.any(String),
+ updatedAt: expect.any(String),
+ })
+ );- expect(res.body.user).toEqual(mockUpdatedUser);
+ expect(res.body.user).toEqual(
+ expect.objectContaining({
+ id: mockUpdatedUser.id,
+ email: mockUpdatedUser.email,
+ name: mockUpdatedUser.name,
+ lastName: mockUpdatedUser.lastName,
+ wallet: mockUpdatedUser.wallet,
+ isVerified: mockUpdatedUser.isVerified,
+ createdAt: expect.any(String),
+ updatedAt: expect.any(String),
+ })
+ );Also applies to: 154-156, 197-199, 236-239
🤖 Prompt for AI Agents
In src/modules/user/__tests__/controllers/UserController.int.test.ts around
lines 93-95 (and also update the similar assertions at 154-156, 197-199,
236-239): the test currently compares res.body to mockUser which contains Date
objects, but Express serializes Dates to ISO strings; change the assertions to
verify the response shape and compare date fields as strings (e.g., expect
typeof/res.body properties and
expect(res.body.createdAt).toBe(mockUser.createdAt.toISOString()) or compare the
entire body with a version of mockUser where Date fields are converted to ISO
strings) so tests pass with serialized dates.
| // Mock Prisma | ||
| const mockPrisma = { | ||
| user: { | ||
| create: jest.fn(), | ||
| findUnique: jest.fn(), | ||
| findFirst: jest.fn(), | ||
| findMany: jest.fn(), | ||
| update: jest.fn(), | ||
| delete: jest.fn(), | ||
| count: jest.fn(), | ||
| }, | ||
| } as unknown as PrismaClient; |
There was a problem hiding this comment.
Make Prisma error classes reliably instantiable in tests
Directly constructing Prisma.PrismaClientKnownRequestError/PrismaClientValidationError is brittle across Prisma versions. Mock them so instanceof checks are stable.
+// Ensure Prisma error classes are constructible for instanceof checks
+jest.mock("@prisma/client", () => {
+ class PrismaClientKnownRequestError extends Error {
+ code: string; clientVersion: string; meta?: Record<string, unknown>;
+ constructor(message: string, opts: any) {
+ super(message);
+ this.code = opts.code;
+ this.clientVersion = opts.clientVersion;
+ this.meta = opts.meta;
+ }
+ }
+ class PrismaClientValidationError extends Error {}
+ class PrismaClient {}
+ return { Prisma: { PrismaClientKnownRequestError, PrismaClientValidationError }, PrismaClient };
+});📝 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.
| // Mock Prisma | |
| const mockPrisma = { | |
| user: { | |
| create: jest.fn(), | |
| findUnique: jest.fn(), | |
| findFirst: jest.fn(), | |
| findMany: jest.fn(), | |
| update: jest.fn(), | |
| delete: jest.fn(), | |
| count: jest.fn(), | |
| }, | |
| } as unknown as PrismaClient; | |
| // Ensure Prisma error classes are constructible for instanceof checks | |
| jest.mock("@prisma/client", () => { | |
| class PrismaClientKnownRequestError extends Error { | |
| code: string; clientVersion: string; meta?: Record<string, unknown>; | |
| constructor(message: string, opts: any) { | |
| super(message); | |
| this.code = opts.code; | |
| this.clientVersion = opts.clientVersion; | |
| this.meta = opts.meta; | |
| } | |
| } | |
| class PrismaClientValidationError extends Error {} | |
| class PrismaClient {} | |
| return { | |
| Prisma: { PrismaClientKnownRequestError, PrismaClientValidationError }, | |
| PrismaClient, | |
| }; | |
| }); | |
| // Mock Prisma | |
| const mockPrisma = { | |
| user: { | |
| create: jest.fn(), | |
| findUnique: jest.fn(), | |
| findFirst: jest.fn(), | |
| findMany: jest.fn(), | |
| update: jest.fn(), | |
| delete: jest.fn(), | |
| count: jest.fn(), | |
| }, | |
| } as unknown as PrismaClient; |
| lastName?: string; | ||
| email: string; | ||
| wallet: string; | ||
| isVerified: boolean; | ||
| verificationToken?: string; | ||
| verificationTokenExpires?: Date; | ||
| createdAt: Date; | ||
| updatedAt: Date; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix nullability to match Prisma returns (null vs. undefined).
Repository sets/returns lastName, verificationToken, and verificationTokenExpires as null in some flows (e.g., verifyUser), but DTOs only allow undefined. This causes type/contract drift and potential runtime violations.
Apply:
export interface UserDTO {
id: string;
name: string;
- lastName?: string;
+ lastName?: string | null;
email: string;
wallet: string;
isVerified: boolean;
- verificationToken?: string;
- verificationTokenExpires?: Date;
+ verificationToken?: string | null;
+ verificationTokenExpires?: Date | null;
createdAt: Date;
updatedAt: Date;
}
export interface CreateUserData {
name: string;
- lastName?: string;
+ lastName?: string | null;
email: string;
password: string;
wallet: string;
isVerified?: boolean;
- verificationToken?: string;
- verificationTokenExpires?: Date;
+ verificationToken?: string | null;
+ verificationTokenExpires?: Date | null;
}
export interface UpdateUserData {
name?: string;
- lastName?: string;
+ lastName?: string | null;
email?: string;
password?: string;
wallet?: string;
isVerified?: boolean;
- verificationToken?: string;
- verificationTokenExpires?: Date;
+ verificationToken?: string | null;
+ verificationTokenExpires?: Date | null;
}Also applies to: 15-25, 27-37
🤖 Prompt for AI Agents
In src/modules/user/domain/dto/UserDTO.ts around lines 5-12 (and similarly
adjust lines 15-25 and 27-37), the DTO marks lastName, verificationToken, and
verificationTokenExpires as optional (undefined) but Prisma returns null; change
those properties to allow null (e.g., lastName: string | null,
verificationToken: string | null, verificationTokenExpires: Date | null) so the
types match Prisma flows and update any other occurrences in the file within the
referenced line ranges to the same null-union form.
| import { | ||
| ResourceNotFoundError, | ||
| ResourceConflictError, | ||
| ValidationError | ||
| } from "../../../../shared/application/errors"; |
There was a problem hiding this comment.
Broken import path to shared errors (off by one directory)
From .../implementations/ to src/shared/... needs 5 ups, not 4. Current path resolves to src/modules/shared/... and will fail at runtime/compile.
Apply:
-import {
+import {
ResourceNotFoundError,
ResourceConflictError,
ValidationError
-} from "../../../../shared/application/errors";
+} from "../../../../../shared/application/errors";📝 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 { | |
| ResourceNotFoundError, | |
| ResourceConflictError, | |
| ValidationError | |
| } from "../../../../shared/application/errors"; | |
| import { | |
| ResourceNotFoundError, | |
| ResourceConflictError, | |
| ValidationError | |
| } from "../../../../../shared/application/errors"; |
🤖 Prompt for AI Agents
In
src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts
around lines 4 to 8 the import path to shared errors is off by one directory;
change the import from "../../../../shared/application/errors" to
"../../../../../shared/application/errors" so it resolves to
src/shared/application/errors (update the import statement to reference the
correct relative path for ResourceNotFoundError, ResourceConflictError, and
ValidationError).
| } catch (error: any) { | ||
| if (error.message?.includes("not found")) { | ||
| res.status(404).json({ error: error.message }); | ||
| } else if (error.message?.includes("already exists")) { | ||
| res.status(409).json({ error: error.message }); | ||
| } else { | ||
| res.status(500).json({ error: "Internal server error" }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
updateUser: robust error handling
- } catch (error: any) {
- if (error.message?.includes("not found")) {
- res.status(404).json({ error: error.message });
- } else if (error.message?.includes("already exists")) {
- res.status(409).json({ error: error.message });
- } else {
- res.status(500).json({ error: "Internal server error" });
- }
- }
+ } catch (error: any) {
+ if (error instanceof ResourceNotFoundError || error.message?.toLowerCase().includes("not found")) {
+ return res.status(404).json({ error: error.message });
+ }
+ if (error instanceof ResourceConflictError || error.message?.toLowerCase().includes("already exists")) {
+ return res.status(409).json({ error: error.message });
+ }
+ return res.status(500).json({ error: "Internal server error" });
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/modules/user/presentation/controllers/UserController.ts around lines 91
to 99, the catch block currently inspects error.message substrings to determine
HTTP status which is fragile; change it to first narrow the error type by
checking for well-defined properties (e.g. error?.status or error?.code or using
an instanceof check for your domain/validation errors), map those to appropriate
HTTP statuses (404, 409, etc.), fall back to 500 otherwise, and ensure you do
not leak sensitive internals by returning a safe message body; also log the
original error (stack) for diagnostics. Ensure the response uses the determined
numeric status and a sanitized error message or a generic "Internal server
error" for unexpected errors.
| if (error.message?.includes("not found")) { | ||
| res.status(404).json({ error: error.message }); | ||
| } else { | ||
| res.status(500).json({ error: "Internal server error" }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
deleteUser: robust error handling
- } catch (error: any) {
- if (error.message?.includes("not found")) {
- res.status(404).json({ error: error.message });
- } else {
- res.status(500).json({ error: "Internal server error" });
- }
- }
+ } catch (error: any) {
+ if (error instanceof ResourceNotFoundError || error.message?.toLowerCase().includes("not found")) {
+ return res.status(404).json({ error: error.message });
+ }
+ return res.status(500).json({ error: "Internal server error" });
+ }📝 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.
| if (error.message?.includes("not found")) { | |
| res.status(404).json({ error: error.message }); | |
| } else { | |
| res.status(500).json({ error: "Internal server error" }); | |
| } | |
| } | |
| } catch (error: any) { | |
| if (error instanceof ResourceNotFoundError || error.message?.toLowerCase().includes("not found")) { | |
| return res.status(404).json({ error: error.message }); | |
| } | |
| return res.status(500).json({ error: "Internal server error" }); | |
| } |
| async execute(data: CreateUserDto): Promise<UserDTO> { | ||
| const hashedPassword = bcrypt.hashSync(data.password, 10); | ||
|
|
||
| const user = new User(); | ||
| user.id = crypto.randomUUID(); | ||
| user.name = data.name; | ||
| user.lastName = data.lastName; | ||
| user.email = data.email; | ||
| user.password = hashedPassword; | ||
| user.wallet = data.wallet; | ||
| return this.userRepository.create(user); | ||
| const createUserData: CreateUserData = { | ||
| name: data.name, | ||
| lastName: data.lastName, | ||
| email: data.email, | ||
| password: hashedPassword, | ||
| wallet: data.wallet || "", | ||
| isVerified: false, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hash passwords asynchronously; avoid empty-string wallet default
Use bcrypt.hash to prevent event-loop blocking. Also, defaulting wallet to "" violates CreateUserData contract and DB constraints—require it or make it optional end-to-end.
- async execute(data: CreateUserDto): Promise<UserDTO> {
- const hashedPassword = bcrypt.hashSync(data.password, 10);
+ async execute(data: CreateUserDto): Promise<UserDTO> {
+ const hashedPassword = await bcrypt.hash(data.password, 10);
@@
- wallet: data.wallet || "",
+ wallet: data.wallet!, // ensure validated upstream; otherwise throw with a Validation errorOptionally, throw if missing:
if (!data.wallet) throw new Error("Wallet is required.");🤖 Prompt for AI Agents
In src/modules/user/use-cases/userUseCase.ts around lines 10–20, the code uses
bcrypt.hashSync (which blocks the event loop) and defaults wallet to an empty
string (violates CreateUserData/DB constraints); replace synchronous hashing
with the async bcrypt.hash and await its result, and enforce wallet presence
instead of defaulting it—validate data.wallet at the start of execute and either
throw a clear Error when missing (e.g., "Wallet is required") or update the
DTO/type flow to make wallet optional end-to-end; ensure CreateUserData is
populated with the awaited hashed password and a valid wallet value.
| async execute(id: string, data: UpdateUserDto): Promise<UserDTO> { | ||
| const updateUserData: UpdateUserData = { | ||
| ...(data.name && { name: data.name }), | ||
| ...(data.lastName !== undefined && { lastName: data.lastName }), | ||
| ...(data.email && { email: data.email }), | ||
| ...(data.password && { password: data.password }), | ||
| ...(data.wallet && { wallet: data.wallet }), | ||
| }; | ||
|
|
||
| return this.userRepository.update(id, updateUserData); | ||
| } |
There was a problem hiding this comment.
Do not store raw passwords on update
Hash when updating the password.
- async execute(id: string, data: UpdateUserDto): Promise<UserDTO> {
- const updateUserData: UpdateUserData = {
+ async execute(id: string, data: UpdateUserDto): Promise<UserDTO> {
+ const hashed = data.password ? await bcrypt.hash(data.password, 10) : undefined;
+ const updateUserData: UpdateUserData = {
...(data.name && { name: data.name }),
...(data.lastName !== undefined && { lastName: data.lastName }),
...(data.email && { email: data.email }),
- ...(data.password && { password: data.password }),
+ ...(hashed && { password: hashed }),
...(data.wallet && { wallet: data.wallet }),
};📝 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.
| async execute(id: string, data: UpdateUserDto): Promise<UserDTO> { | |
| const updateUserData: UpdateUserData = { | |
| ...(data.name && { name: data.name }), | |
| ...(data.lastName !== undefined && { lastName: data.lastName }), | |
| ...(data.email && { email: data.email }), | |
| ...(data.password && { password: data.password }), | |
| ...(data.wallet && { wallet: data.wallet }), | |
| }; | |
| return this.userRepository.update(id, updateUserData); | |
| } | |
| async execute(id: string, data: UpdateUserDto): Promise<UserDTO> { | |
| const hashed = data.password ? await bcrypt.hash(data.password, 10) : undefined; | |
| const updateUserData: UpdateUserData = { | |
| ...(data.name && { name: data.name }), | |
| ...(data.lastName !== undefined && { lastName: data.lastName }), | |
| ...(data.email && { email: data.email }), | |
| ...(hashed && { password: hashed }), | |
| ...(data.wallet && { wallet: data.wallet }), | |
| }; | |
| return this.userRepository.update(id, updateUserData); | |
| } |
🤖 Prompt for AI Agents
In src/modules/user/use-cases/userUseCase.ts around lines 83 to 93, the code
currently passes raw passwords through updateUserData; before calling
this.userRepository.update you must detect if data.password is provided,
asynchronously hash it (e.g., using the project's password hashing utility or
bcrypt with proper salt rounds), replace the plain text password in
updateUserData with the hashed value, then call this.userRepository.update(id,
updateUserData); ensure you await the hash call and propagate or handle any
hashing errors appropriately.
| // Repository tokens | ||
| export const USER_REPOSITORY = Symbol.for("USER_REPOSITORY"); | ||
|
|
||
| export const container = { | ||
| certificateService: new CertificateService() as ICertificateService, | ||
| [USER_REPOSITORY]: new UserPrismaRepository(prisma) as IUserRepository, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
LGTM on DI wiring; consider lazy resolution and complete repository contract.
- Instantiate repo lazily to avoid side effects at import time and ease test overrides.
- Missing
findByWalletinIUserRepositorylikely forces services/tests to hit Prisma directly; add it to meet “only repository touches Prisma” objective.
Lazy accessor (optional):
export const container = {
certificateService: new CertificateService() as ICertificateService,
- [USER_REPOSITORY]: new UserPrismaRepository(prisma) as IUserRepository,
+ get [USER_REPOSITORY](): IUserRepository {
+ // late binding to prisma
+ return new UserPrismaRepository(prisma);
+ },
};Complete contract (apply in the indicated files):
-// src/modules/user/domain/interfaces/IUserRepository.ts
export interface IUserRepository {
+ findByWallet(wallet: string): Promise<UserDTO | null>;
...
}-// src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts
export class UserPrismaRepository implements IUserRepository {
constructor(private prisma: PrismaClient) {}
+ async findByWallet(wallet: string): Promise<UserDTO | null> {
+ try {
+ const user = await this.prisma.user.findUnique({ where: { wallet } });
+ return user ? this.toDTO(user) : null;
+ } catch (error) {
+ this.handlePrismaError(error);
+ }
+ }
...
}I can open a follow-up PR with these changes and update affected services/tests.
📝 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.
| // Repository tokens | |
| export const USER_REPOSITORY = Symbol.for("USER_REPOSITORY"); | |
| export const container = { | |
| certificateService: new CertificateService() as ICertificateService, | |
| [USER_REPOSITORY]: new UserPrismaRepository(prisma) as IUserRepository, | |
| }; | |
| // Repository tokens | |
| export const USER_REPOSITORY = Symbol.for("USER_REPOSITORY"); | |
| export const container = { | |
| certificateService: new CertificateService() as ICertificateService, | |
| get [USER_REPOSITORY](): IUserRepository { | |
| // late binding to prisma | |
| return new UserPrismaRepository(prisma); | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In src/shared/infrastructure/container.ts around lines 7 to 13, the container
eagerly constructs repositories and misses the IUserRepository.findByWallet
contract; change the container to lazily resolve the UserPrismaRepository (e.g.,
expose a getter or factory that returns a new or memoized instance) to avoid
import-time side effects and make test overrides easier, and update the
IUserRepository interface to include the findByWallet method signature and
implement that method on UserPrismaRepository so all services/tests use the
repository abstraction rather than touching Prisma directly.
1 participant
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])npm test,npm test -- --testPathPattern=modules/user)npm run format)📌 Type of Change
📝 Changes description
IUserRepositorywith DTOs (UserDTO,CreateUserData,UpdateUserData).src/modules/user/infrastructure/repositories/implementations/user-prisma.repository.ts.P2002→ConflictExceptionP2025→NotFoundExceptionPrismaClientValidationError→ValidationExceptionsrc/shared/infrastructure/container.tsto bind theUSER_REPOSITORYtoken.📸 Evidence (A photo is required as evidence)
⏰ Time spent breakdown
Total: ~7h
🌌 Comments
This refactor aligns the User module with VolunChain’s strict DDD/modular architecture.
Controllers and services are now decoupled from Prisma, and tests provide confidence that repository logic is correct.
This also resolves inconsistencies with legacy
src/repository/user.repository.ts, ensuring the new path is the canonical implementation.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
Refactor
Documentation
Tests