feat: enhance project management with Supabase integration, improved … #201
Conversation
…error handling, and comprehensive use case implementations
WalkthroughRefactors global error handling to use AppException with a standardized JSON schema. Implements ProjectController with dependency-injected use cases. Adds PrismaProjectRepository implementation. Enhances Create/Update use cases with DTO validation and standard exceptions. Updates Get/Delete use cases to throw domain exceptions. Introduces comprehensive unit/integration tests across controller, repository, and use cases. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Router
participant ProjectController
participant CreateProjectUseCase as CreateUseCase
participant Repo as ProjectRepository
participant EH as ErrorHandler
Client->>Router: POST /projects {body}
Router->>ProjectController: createProject(req, res)
ProjectController->>CreateUseCase: execute(CreateProjectDto)
CreateUseCase->>CreateUseCase: validate DTO
alt valid
CreateUseCase->>Repo: save(Project)
Repo-->>CreateUseCase: Project
CreateUseCase-->>ProjectController: Project
ProjectController-->>Client: 201 { success: true, data }
else validation error
CreateUseCase-->>ProjectController: throw ValidationException
ProjectController-->>EH: propagate error
EH-->>Client: 400 { statusCode, message, errorCode, traceId? }
end
sequenceDiagram
autonumber
actor Client
participant Router
participant ProjectController
participant GetUseCase
participant Repo
participant EH as ErrorHandler
Client->>Router: GET /projects/:id
Router->>ProjectController: getProjectById(req, res)
ProjectController->>GetUseCase: execute(id)
GetUseCase->>Repo: findById(id)
alt found
Repo-->>GetUseCase: Project
GetUseCase-->>ProjectController: Project
ProjectController-->>Client: 200 { success: true, data }
else not found
Repo-->>GetUseCase: null
GetUseCase-->>ProjectController: throw NotFoundException
ProjectController-->>EH: propagate error
EH-->>Client: 404 { statusCode, message, errorCode, traceId? }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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: 11
🧹 Nitpick comments (19)
src/middlewares/errorHandler.ts (3)
49-56: Reuse AppException.toJSON() to ensure a single source of truth for payload shapeAvoid duplicating fields; leverage the domain object’s serializer and just append traceId.
- if (err instanceof AppException) { - return res.status(err.statusCode).json({ - statusCode: err.statusCode, - message: err.message, - errorCode: err.errorCode, - ...(req.traceId && { traceId: req.traceId }), - }); - } + if (err instanceof AppException) { + return res + .status(err.statusCode) + .json({ ...err.toJSON(), ...(req.traceId && { traceId: req.traceId }) }); + }
59-65: Consider centralizing "INTERNAL_ERROR" in a shared enum/constantHardcoding "INTERNAL_ERROR" here risks drift with DomainExceptions. If you already have ErrorCodes, import and reuse it.
If ErrorCodes exists, update this block to reference it. Otherwise, ignore.
- return res.status(500).json({ - statusCode: 500, - message: "Internal server error", - errorCode: "INTERNAL_ERROR", - ...(req.traceId && { traceId: req.traceId }), - }); + return res.status(500).json({ + statusCode: 500, + message: "Internal server error", + errorCode: ErrorCodes.INTERNAL_ERROR, + ...(req.traceId && { traceId: req.traceId }), + });
17-22: Minor cleanups: unused locals and unused parameter
- errorInfo is constructed but not used.
- next isn’t referenced; prefix with underscore to satisfy linters.
-export const errorHandler = ( - err: Error, - req: Request, - res: Response, - next: NextFunction -) => { +export const errorHandler = ( + err: Error, + req: Request, + res: Response, + _next: NextFunction +) => { - // Log error information - const errorInfo: ErrorLogInfo = { - timestamp: new Date().toISOString(), - path: req.path, - method: req.method, - error: err.message, - }; - - // In development, include more details - if (process.env.NODE_ENV === "development") { - errorInfo.stack = err.stack; - errorInfo.requestBody = req.body; - errorInfo.requestQuery = req.query; - } + // (Optional) If you want structured logs, pass these details directly in logger.error below.Also applies to: 31-36
src/modules/project/use-cases/UpdateProjectUseCase.ts (1)
26-27: Optional: prevent no-op updatesIf dto has no updatable fields, consider short-circuiting with ValidationException to avoid writes that only bump updatedAt.
- project.update(dto); + if (!Object.keys(dto).length) { + throw new ValidationException("No fields provided to update"); + } + project.update(dto);src/modules/project/use-cases/CreateProjectUseCase.ts (1)
10-18: DRY: extract validateDtoOrThrow helperCreate a shared util to remove duplication between Create/Update.
Happy to open a follow-up PR with the utility and refactors.
Example utility:
// src/shared/validation/validateDto.ts import { validate } from "class-validator"; import { plainToInstance } from "class-transformer"; import { ValidationException } from "../exceptions/DomainExceptions"; export async function validateDtoOrThrow<T>(cls: new () => T, dto: unknown) { const inst = plainToInstance(cls, dto); const errors = await validate(inst); if (errors.length) { const msg = errors .map(e => `${e.property}: ${Object.values(e.constraints || {}).join(', ')}`) .filter(Boolean) .join('; '); throw new ValidationException(`Validation failed: ${msg}`); } return inst; }Usage:
const dtoInstance = await validateDtoOrThrow(CreateProjectDto, dto);Also applies to: 20-27
src/modules/project/repositories/PrismaProjectRepository.ts (2)
28-42: Consider pagination in findAll() to avoid unbounded reads.Add take/skip (and optional orderBy), or expose a paginated variant in the interface.
87-99: Optional: Map known Prisma errors to domain exceptions (conflict/not found).E.g., P2002 → ConflictException on unique violations; P2025 → NotFoundException on stale updates/deletes. This keeps use cases cleaner and surfaces better error codes.
Also applies to: 111-119
src/modules/project/__tests__/use-cases/CreateProjectUseCase.test.ts (1)
105-111: Avoid asserting on private props; assert via the public API or captured argument.Accessing
.propsties the test to internals. Capture the argument and use the getter.- expect(mockRepository.save).toHaveBeenCalledWith( - expect.objectContaining({ - props: expect.objectContaining({ - status: ProjectStatus.DRAFT, - }), - }) - ); + const savedArg = mockRepository.save.mock.calls[0][0]; + expect(savedArg.status).toBe(ProjectStatus.DRAFT);src/modules/project/__tests__/controllers/ProjectController.int.test.ts (6)
32-32: Rename the suite to reflect unit-style testing (not true integration).These tests instantiate the controller with mocked use-cases and no HTTP stack/middleware. Rename the suite for clarity.
-describe("ProjectController Integration", () => { +describe("ProjectController (unit, DI)", () => {
51-63: Fix test name and assert no response was written on validation error.The test does not assert a 400 body; it asserts a thrown ValidationException. Align the description and verify res was untouched.
- test("should return 400 with standard error body for validation errors", async () => { + test("should propagate ValidationException (handled by error middleware)", async () => { @@ - await expect(controller.createProject(req as Request, res as Response)).rejects.toThrow(ValidationException); + await expect(controller.createProject(req as Request, res as Response)).rejects.toThrow(ValidationException); + expect(res.status).not.toHaveBeenCalled(); + expect((res as any).json).not.toHaveBeenCalled();
97-109: Fix test name and assert no response was written on not-found.Same issue as above: update description and verify res wasn’t used.
- test("should return 404 with standard error body for not found", async () => { + test("should propagate NotFoundException (handled by error middleware)", async () => { @@ - await expect(controller.getProjectById(req as Request, res as Response)).rejects.toThrow(NotFoundException); + await expect(controller.getProjectById(req as Request, res as Response)).rejects.toThrow(NotFoundException); + expect(res.status).not.toHaveBeenCalled(); + expect((res as any).json).not.toHaveBeenCalled();
137-152: Update: error tests should mirror middleware behavior and assert no response was written.Rename the tests and add “no res usage” assertions.
- test("should return 400 with standard error body for validation errors", async () => { + test("should propagate ValidationException (handled by error middleware)", async () => { @@ - await expect(controller.updateProject(req as Request, res as Response)).rejects.toThrow(ValidationException); + await expect(controller.updateProject(req as Request, res as Response)).rejects.toThrow(ValidationException); + expect(res.status).not.toHaveBeenCalled(); + expect((res as any).json).not.toHaveBeenCalled();- test("should return 404 with standard error body for not found", async () => { + test("should propagate NotFoundException (handled by error middleware)", async () => { @@ - await expect(controller.updateProject(req as Request, res as Response)).rejects.toThrow(NotFoundException); + await expect(controller.updateProject(req as Request, res as Response)).rejects.toThrow(NotFoundException); + expect(res.status).not.toHaveBeenCalled(); + expect((res as any).json).not.toHaveBeenCalled();Also applies to: 153-168
171-183: Delete: error test should not mention 404 body and should assert no response was written.- test("should return 404 with standard error body for not found", async () => { + test("should propagate NotFoundException (handled by error middleware)", async () => { @@ - await expect(controller.deleteProject(req as Request, res as Response)).rejects.toThrow(NotFoundException); + await expect(controller.deleteProject(req as Request, res as Response)).rejects.toThrow(NotFoundException); + expect(res.status).not.toHaveBeenCalled(); + expect((res as any).json).not.toHaveBeenCalled();
50-94: Add coverage for list endpoints.Missing tests for getProjectsByOrganizationId and getAllProjects. Add success and error-path tests to ensure payloads and propagation remain consistent.
Also applies to: 96-134, 136-168, 170-198
src/modules/project/presentation/controllers/Project.controller.stub.ts (1)
20-37: Remove redundant try/catch blocks or wire errors to middleware.Each method rethrows the same error regardless of type; the try/catch adds no value. Either:
- Minimal: remove try/catch and let errors bubble; or
- Robust (Express 4): accept
next: NextFunctionand callnext(error)to ensure centralized handling.Below is the minimal diff to drop the no-op try/catch and return responses explicitly.
@@ - async createProject(req: Request, res: Response) { - try { - const createProjectDto = new CreateProjectDto(); - Object.assign(createProjectDto, req.body); - - const project = await this.createProjectUseCase.execute(createProjectDto); - - res.status(201).json({ - success: true, - data: project - }); - } catch (error) { - if (error instanceof AppException) { - throw error; - } - throw error; - } - } + async createProject(req: Request, res: Response) { + const createProjectDto = new CreateProjectDto(); + Object.assign(createProjectDto, req.body); + const project = await this.createProjectUseCase.execute(createProjectDto); + return res.status(201).json({ success: true, data: project }); + } @@ - async getProjectById(req: Request, res: Response) { - try { - const { id } = req.params; - const project = await this.getProjectUseCase.execute(id); - - res.status(200).json({ - success: true, - data: project - }); - } catch (error) { - if (error instanceof AppException) { - throw error; - } - throw error; - } - } + async getProjectById(req: Request, res: Response) { + const { id } = req.params; + const project = await this.getProjectUseCase.execute(id); + return res.status(200).json({ success: true, data: project }); + } @@ - async getProjectsByOrganizationId(req: Request, res: Response) { - try { - const { organizationId } = req.params; - const projects = await this.listProjectsUseCase.execute(organizationId); - - res.status(200).json({ - success: true, - data: projects - }); - } catch (error) { - if (error instanceof AppException) { - throw error; - } - throw error; - } - } + async getProjectsByOrganizationId(req: Request, res: Response) { + const { organizationId } = req.params; + const projects = await this.listProjectsUseCase.execute(organizationId); + return res.status(200).json({ success: true, data: projects }); + } @@ - async getAllProjects(req: Request, res: Response) { - try { - const projects = await this.listProjectsUseCase.execute(); - - res.status(200).json({ - success: true, - data: projects - }); - } catch (error) { - if (error instanceof AppException) { - throw error; - } - throw error; - } - } + async getAllProjects(req: Request, res: Response) { + const projects = await this.listProjectsUseCase.execute(); + return res.status(200).json({ success: true, data: projects }); + } @@ - async updateProject(req: Request, res: Response) { - try { - const { id } = req.params; - const updateProjectDto = new UpdateProjectDto(); - Object.assign(updateProjectDto, req.body); - - const project = await this.updateProjectUseCase.execute(id, updateProjectDto); - - res.status(200).json({ - success: true, - data: project - }); - } catch (error) { - if (error instanceof AppException) { - throw error; - } - throw error; - } - } + async updateProject(req: Request, res: Response) { + const { id } = req.params; + const updateProjectDto = new UpdateProjectDto(); + Object.assign(updateProjectDto, req.body); + const project = await this.updateProjectUseCase.execute(id, updateProjectDto); + return res.status(200).json({ success: true, data: project }); + } @@ - async deleteProject(req: Request, res: Response) { - try { - const { id } = req.params; - await this.deleteProjectUseCase.execute(id); - - res.status(204).send(); - } catch (error) { - if (error instanceof AppException) { - throw error; - } - throw error; - } - } + async deleteProject(req: Request, res: Response) { + const { id } = req.params; + await this.deleteProjectUseCase.execute(id); + return res.status(204).send(); + }Also applies to: 39-54, 56-71, 73-87, 89-107, 109-121
src/modules/project/__tests__/repositories/PrismaProjectRepository.test.ts (4)
122-156: Assert ID stability in update path.Returned object should preserve the DB id.
const result = await repository.update(project); @@ expect(result).toBeInstanceOf(Project); + expect(result.id).toBe(mockUpdatedProject.id);
192-223: Assert IDs in findAll results and add empty-array case.Strengthen checks to ensure correct rehydration, and add a test for empty results.
const result = await repository.findAll(); @@ - expect(result).toHaveLength(2); - expect(result[0]).toBeInstanceOf(Project); - expect(result[1]).toBeInstanceOf(Project); + expect(result).toHaveLength(2); + expect(result[0]).toBeInstanceOf(Project); + expect(result[1]).toBeInstanceOf(Project); + expect(result.map(p => p.id)).toEqual(["project-1", "project-2"]);+ it("should return empty array when no projects exist", async () => { + mockPrisma.project.findMany.mockResolvedValue([]); + const result = await repository.findAll(); + expect(result).toEqual([]); + });
232-256: Assert IDs in findByOrganizationId results.const result = await repository.findByOrganizationId(organizationId); @@ expect(result).toHaveLength(1); expect(result[0]).toBeInstanceOf(Project); + expect(result[0].id).toBe("project-1");
1-5: Prefer safer Prisma mocks to reduce type gaps.Consider jest-mock-extended to deeply mock PrismaClient with correct types, avoiding
as any.
📜 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 (13)
src/middlewares/errorHandler.ts(2 hunks)src/modules/project/__tests__/controllers/ProjectController.int.test.ts(1 hunks)src/modules/project/__tests__/repositories/PrismaProjectRepository.test.ts(1 hunks)src/modules/project/__tests__/use-cases/CreateProjectUseCase.test.ts(1 hunks)src/modules/project/__tests__/use-cases/DeleteProjectUseCase.test.ts(1 hunks)src/modules/project/__tests__/use-cases/GetProjectUseCase.test.ts(1 hunks)src/modules/project/__tests__/use-cases/UpdateProjectUseCase.test.ts(1 hunks)src/modules/project/presentation/controllers/Project.controller.stub.ts(1 hunks)src/modules/project/repositories/PrismaProjectRepository.ts(1 hunks)src/modules/project/use-cases/CreateProjectUseCase.ts(1 hunks)src/modules/project/use-cases/DeleteProjectUseCase.ts(2 hunks)src/modules/project/use-cases/GetProjectUseCase.ts(2 hunks)src/modules/project/use-cases/UpdateProjectUseCase.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
src/modules/project/use-cases/DeleteProjectUseCase.ts (1)
src/shared/exceptions/DomainExceptions.ts (1)
NotFoundException(21-25)
src/modules/project/__tests__/use-cases/GetProjectUseCase.test.ts (4)
src/modules/project/use-cases/GetProjectUseCase.ts (1)
GetProjectUseCase(5-17)src/modules/project/repositories/IProjectRepository.ts (1)
IProjectRepository(3-10)src/modules/project/domain/Project.ts (1)
Project(20-64)src/shared/exceptions/DomainExceptions.ts (2)
NotFoundException(21-25)InternalServerException(33-37)
src/modules/project/__tests__/use-cases/DeleteProjectUseCase.test.ts (4)
src/modules/project/use-cases/DeleteProjectUseCase.ts (1)
DeleteProjectUseCase(4-16)src/modules/project/repositories/IProjectRepository.ts (1)
IProjectRepository(3-10)src/modules/project/domain/Project.ts (1)
Project(20-64)src/shared/exceptions/DomainExceptions.ts (2)
NotFoundException(21-25)InternalServerException(33-37)
src/modules/project/__tests__/repositories/PrismaProjectRepository.test.ts (3)
src/modules/project/repositories/PrismaProjectRepository.ts (1)
PrismaProjectRepository(6-120)src/modules/project/domain/Project.ts (2)
Project(20-64)organizationId(57-59)src/shared/exceptions/DomainExceptions.ts (1)
InternalServerException(33-37)
src/modules/project/use-cases/GetProjectUseCase.ts (1)
src/shared/exceptions/DomainExceptions.ts (1)
NotFoundException(21-25)
src/modules/project/__tests__/use-cases/CreateProjectUseCase.test.ts (5)
src/modules/project/use-cases/CreateProjectUseCase.ts (1)
CreateProjectUseCase(7-29)src/modules/project/repositories/IProjectRepository.ts (1)
IProjectRepository(3-10)src/modules/project/dto/CreateProjectDto.ts (1)
CreateProjectDto(11-28)src/modules/project/domain/Project.ts (1)
Project(20-64)src/shared/exceptions/DomainExceptions.ts (2)
ValidationException(3-7)InternalServerException(33-37)
src/modules/project/__tests__/use-cases/UpdateProjectUseCase.test.ts (5)
src/modules/project/use-cases/UpdateProjectUseCase.ts (1)
UpdateProjectUseCase(7-29)src/modules/project/repositories/IProjectRepository.ts (1)
IProjectRepository(3-10)src/modules/project/dto/UpdateProjectDto.ts (1)
UpdateProjectDto(11-31)src/modules/project/domain/Project.ts (1)
Project(20-64)src/shared/exceptions/DomainExceptions.ts (3)
NotFoundException(21-25)ValidationException(3-7)InternalServerException(33-37)
src/modules/project/use-cases/CreateProjectUseCase.ts (3)
src/modules/project/dto/CreateProjectDto.ts (1)
CreateProjectDto(11-28)src/modules/project/domain/Project.ts (1)
Project(20-64)src/shared/exceptions/DomainExceptions.ts (1)
ValidationException(3-7)
src/modules/project/presentation/controllers/Project.controller.stub.ts (8)
src/modules/project/use-cases/CreateProjectUseCase.ts (1)
CreateProjectUseCase(7-29)src/modules/project/use-cases/GetProjectUseCase.ts (1)
GetProjectUseCase(5-17)src/modules/project/use-cases/ListProjectsUseCase.ts (1)
ListProjectsUseCase(4-13)src/modules/project/use-cases/UpdateProjectUseCase.ts (1)
UpdateProjectUseCase(7-29)src/modules/project/use-cases/DeleteProjectUseCase.ts (1)
DeleteProjectUseCase(4-16)src/modules/project/dto/CreateProjectDto.ts (1)
CreateProjectDto(11-28)src/shared/exceptions/AppException.ts (1)
AppException(1-20)src/modules/project/dto/UpdateProjectDto.ts (1)
UpdateProjectDto(11-31)
src/modules/project/repositories/PrismaProjectRepository.ts (3)
src/modules/project/repositories/IProjectRepository.ts (1)
IProjectRepository(3-10)src/modules/project/domain/Project.ts (3)
id(45-47)Project(20-64)organizationId(57-59)src/shared/exceptions/DomainExceptions.ts (1)
InternalServerException(33-37)
src/modules/project/use-cases/UpdateProjectUseCase.ts (4)
src/modules/project/repositories/IProjectRepository.ts (1)
IProjectRepository(3-10)src/modules/project/domain/Project.ts (2)
id(45-47)Project(20-64)src/modules/project/dto/UpdateProjectDto.ts (1)
UpdateProjectDto(11-31)src/shared/exceptions/DomainExceptions.ts (2)
ValidationException(3-7)NotFoundException(21-25)
src/middlewares/errorHandler.ts (1)
src/shared/exceptions/AppException.ts (1)
AppException(1-20)
src/modules/project/__tests__/controllers/ProjectController.int.test.ts (7)
src/modules/project/use-cases/CreateProjectUseCase.ts (1)
CreateProjectUseCase(7-29)src/modules/project/use-cases/GetProjectUseCase.ts (1)
GetProjectUseCase(5-17)src/modules/project/use-cases/ListProjectsUseCase.ts (1)
ListProjectsUseCase(4-13)src/modules/project/use-cases/UpdateProjectUseCase.ts (1)
UpdateProjectUseCase(7-29)src/modules/project/use-cases/DeleteProjectUseCase.ts (1)
DeleteProjectUseCase(4-16)src/modules/project/presentation/controllers/Project.controller.stub.ts (1)
ProjectController(11-122)src/shared/exceptions/DomainExceptions.ts (2)
ValidationException(3-7)NotFoundException(21-25)
🔇 Additional comments (12)
src/middlewares/errorHandler.ts (1)
39-47: Trace ID typing is already handled
The global Express.Request interface is augmented in src/types/auth.types.ts to include traceId, satisfying TypeScript.src/modules/project/use-cases/GetProjectUseCase.ts (1)
3-3: Ensure UUID validation on project route params
NoParseUUIDPipe(or equivalent) is applied to project route parameters—add validation (e.g.@Param('projectId', new ParseUUIDPipe())) in your controllers or middleware so malformed IDs yield 400s instead of 404s.src/modules/project/use-cases/DeleteProjectUseCase.ts (1)
2-2: Remove manual UUID validation request
Validation middleware with@IsUUID(4)on DTO DTOs (e.g.UuidParamsDto) already enforces 400 on malformed IDs.src/modules/project/use-cases/CreateProjectUseCase.ts (1)
11-18: Ignore DTO instantiation suggestion
UpdateProjectUseCase uses the same directvalidate(dto)and message formatting as CreateProjectUseCase—there is no existing “defensive instantiation” or enhanced messaging in update to mirror.Likely an incorrect or invalid review comment.
src/modules/project/repositories/PrismaProjectRepository.ts (1)
71-73: Remove manual timestamp assignments in PrismaProjectRepository
Prisma’s schema already definescreatedAt @default(now())andupdatedAt @updatedAt, so drop these lines from your create/update calls.
Applies at lines 71–73 and 96–97.src/modules/project/__tests__/use-cases/CreateProjectUseCase.test.ts (1)
76-85: Mock repository failure with InternalServerException to reflect domain contract.- mockRepository.save.mockRejectedValue(new Error("Database error")); + mockRepository.save.mockRejectedValue(new InternalServerException("Database error"));Likely an incorrect or invalid review comment.
src/modules/project/__tests__/use-cases/UpdateProjectUseCase.test.ts (2)
109-114: Align findById failure mock with InternalServerException.- mockRepository.findById.mockRejectedValue(new Error("Database error")); + mockRepository.findById.mockRejectedValue(new InternalServerException("Database error"));Likely an incorrect or invalid review comment.
128-134: Align update failure mock with InternalServerException.- mockRepository.update.mockRejectedValue(new Error("Update failed")); + mockRepository.update.mockRejectedValue(new InternalServerException("Update failed"));Likely an incorrect or invalid review comment.
src/modules/project/__tests__/controllers/ProjectController.int.test.ts (3)
64-93: Create: happy path looks good.Status 201 + success payload are asserted correctly.
110-133: Get by id: happy path looks good.
184-197: Delete: happy path looks good.204 + send() is asserted correctly.
src/modules/project/__tests__/repositories/PrismaProjectRepository.test.ts (1)
3-3: Import path for ProjectStatus is correct.src/modules/project/domain/Project.tsdirectly exportsProjectStatusalongsideProject, so no change is needed.
| import { PrismaProjectRepository } from "../../repositories/PrismaProjectRepository"; | ||
| import { PrismaClient } from "@prisma/client"; | ||
| import { Project, ProjectStatus } from "../../domain/Project"; | ||
| import { InternalServerException } from "../../../../shared/exceptions/DomainExceptions"; | ||
|
|
||
| describe("PrismaProjectRepository", () => { | ||
| let repository: PrismaProjectRepository; | ||
| let mockPrisma: jest.Mocked<PrismaClient>; | ||
|
|
||
| beforeEach(() => { | ||
| mockPrisma = { | ||
| project: { | ||
| findUnique: jest.fn(), | ||
| findMany: jest.fn(), | ||
| create: jest.fn(), | ||
| update: jest.fn(), | ||
| delete: jest.fn(), | ||
| }, | ||
| } as any; | ||
|
|
||
| repository = new PrismaProjectRepository(mockPrisma); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Critical: repository likely re-creates IDs when rehydrating (data corruption risk).
The implementation in PrismaProjectRepository uses Project.create(...) to rehydrate DB rows, but Project.create generates a new UUID. That breaks identity and will corrupt downstream updates/deletes. Tests currently don’t catch this because they don’t assert IDs.
Proposed fix (domain): add a restoration factory to preserve persistence fields.
// src/modules/project/domain/Project.ts
export class Project extends Entity<IProject> {
// ...
public static restore(props: IProject): Project {
return new Project({ ...props });
}
}Proposed fix (repository): use Project.restore on all read paths and after writes.
// in PrismaProjectRepository.ts
return project ? Project.restore({
id: project.id,
title: project.title,
description: project.description,
organizationId: project.organizationId,
status: project.status as ProjectStatus,
createdAt: project.createdAt,
updatedAt: project.updatedAt,
}) : null;Apply the same mapping in findAll/findByOrganizationId/save/update returns.
Also update tests (diffs above) to assert ID preservation so regressions are caught.
Also applies to: 24-69, 71-120, 122-170, 172-191, 192-231, 232-265
🤖 Prompt for AI Agents
In src/modules/project/__tests__/repositories/PrismaProjectRepository.test.ts
(and corresponding repository/domain files referenced around lines 1-23, 24-69,
71-120, 122-170, 172-191, 192-231, 232-265), the repository currently rehydrates
DB rows via Project.create which generates new UUIDs and breaks identity; add a
restoration factory to the domain (e.g., add a public static restore(props:
IProject): Project that constructs a Project without generating a new id) and
update PrismaProjectRepository read and write return paths to call
Project.restore with the DB row fields (id, title, description, organizationId,
status, createdAt, updatedAt) instead of Project.create; then update the tests
to assert that IDs are preserved after
find/findAll/findByOrganizationId/save/update so regressions are caught.
| describe("findById", () => { | ||
| it("should return project when found", async () => { | ||
| const projectId = "project-123"; | ||
| const mockPrismaProject = { | ||
| id: projectId, | ||
| title: "Test Project", | ||
| description: "Test Description", | ||
| organizationId: "org-123", | ||
| status: ProjectStatus.DRAFT, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| }; | ||
|
|
||
| mockPrisma.project.findUnique.mockResolvedValue(mockPrismaProject); | ||
|
|
||
| const result = await repository.findById(projectId); | ||
|
|
||
| expect(mockPrisma.project.findUnique).toHaveBeenCalledWith({ | ||
| where: { id: projectId } | ||
| }); | ||
| expect(result).toBeInstanceOf(Project); | ||
| expect(result?.title).toBe("Test Project"); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Assert ID preservation in findById.
Repository should return the same id fetched from DB. Add an assertion; current tests wouldn’t catch an ID-loss bug.
const result = await repository.findById(projectId);
@@
expect(result).toBeInstanceOf(Project);
expect(result?.title).toBe("Test Project");
+ expect(result?.id).toBe(projectId);📝 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.
| describe("findById", () => { | |
| it("should return project when found", async () => { | |
| const projectId = "project-123"; | |
| const mockPrismaProject = { | |
| id: projectId, | |
| title: "Test Project", | |
| description: "Test Description", | |
| organizationId: "org-123", | |
| status: ProjectStatus.DRAFT, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| }; | |
| mockPrisma.project.findUnique.mockResolvedValue(mockPrismaProject); | |
| const result = await repository.findById(projectId); | |
| expect(mockPrisma.project.findUnique).toHaveBeenCalledWith({ | |
| where: { id: projectId } | |
| }); | |
| expect(result).toBeInstanceOf(Project); | |
| expect(result?.title).toBe("Test Project"); | |
| }); | |
| describe("findById", () => { | |
| it("should return project when found", async () => { | |
| const projectId = "project-123"; | |
| const mockPrismaProject = { | |
| id: projectId, | |
| title: "Test Project", | |
| description: "Test Description", | |
| organizationId: "org-123", | |
| status: ProjectStatus.DRAFT, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| }; | |
| mockPrisma.project.findUnique.mockResolvedValue(mockPrismaProject); | |
| const result = await repository.findById(projectId); | |
| expect(mockPrisma.project.findUnique).toHaveBeenCalledWith({ | |
| where: { id: projectId } | |
| }); | |
| expect(result).toBeInstanceOf(Project); | |
| expect(result?.title).toBe("Test Project"); | |
| expect(result?.id).toBe(projectId); | |
| }); |
🤖 Prompt for AI Agents
In src/modules/project/__tests__/repositories/PrismaProjectRepository.test.ts
around lines 24 to 47, the test for findById does not assert that the returned
Project preserves the id from the DB; add an assertion after the existing checks
to verify result?.id === projectId (or expect(result).not.toBeNull() then
expect(result?.id).toBe(projectId)) so the test fails if the repository drops or
changes the id.
| describe("save", () => { | ||
| it("should save and return project", async () => { | ||
| const project = Project.create({ | ||
| title: "Test Project", | ||
| description: "Test Description", | ||
| organizationId: "org-123", | ||
| status: ProjectStatus.DRAFT, | ||
| }); | ||
|
|
||
| const mockSavedProject = { | ||
| id: project.id, | ||
| title: project.title, | ||
| description: project.description, | ||
| organizationId: project.organizationId, | ||
| status: project.status, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| }; | ||
|
|
||
| mockPrisma.project.create.mockResolvedValue(mockSavedProject); | ||
|
|
||
| const result = await repository.save(project); | ||
|
|
||
| expect(mockPrisma.project.create).toHaveBeenCalledWith({ | ||
| data: { | ||
| id: project.id, | ||
| title: project.title, | ||
| description: project.description, | ||
| organizationId: project.organizationId, | ||
| status: project.status, | ||
| createdAt: expect.any(Date), | ||
| updatedAt: expect.any(Date), | ||
| } | ||
| }); | ||
| expect(result).toBeInstanceOf(Project); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Assert ID preservation in save path.
Ensure the returned domain object keeps the persisted id.
const result = await repository.save(project);
@@
expect(result).toBeInstanceOf(Project);
+ expect(result.id).toBe(mockSavedProject.id);📝 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.
| describe("save", () => { | |
| it("should save and return project", async () => { | |
| const project = Project.create({ | |
| title: "Test Project", | |
| description: "Test Description", | |
| organizationId: "org-123", | |
| status: ProjectStatus.DRAFT, | |
| }); | |
| const mockSavedProject = { | |
| id: project.id, | |
| title: project.title, | |
| description: project.description, | |
| organizationId: project.organizationId, | |
| status: project.status, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| }; | |
| mockPrisma.project.create.mockResolvedValue(mockSavedProject); | |
| const result = await repository.save(project); | |
| expect(mockPrisma.project.create).toHaveBeenCalledWith({ | |
| data: { | |
| id: project.id, | |
| title: project.title, | |
| description: project.description, | |
| organizationId: project.organizationId, | |
| status: project.status, | |
| createdAt: expect.any(Date), | |
| updatedAt: expect.any(Date), | |
| } | |
| }); | |
| expect(result).toBeInstanceOf(Project); | |
| }); | |
| describe("save", () => { | |
| it("should save and return project", async () => { | |
| const project = Project.create({ | |
| title: "Test Project", | |
| description: "Test Description", | |
| organizationId: "org-123", | |
| status: ProjectStatus.DRAFT, | |
| }); | |
| const mockSavedProject = { | |
| id: project.id, | |
| title: project.title, | |
| description: project.description, | |
| organizationId: project.organizationId, | |
| status: project.status, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| }; | |
| mockPrisma.project.create.mockResolvedValue(mockSavedProject); | |
| const result = await repository.save(project); | |
| expect(mockPrisma.project.create).toHaveBeenCalledWith({ | |
| data: { | |
| id: project.id, | |
| title: project.title, | |
| description: project.description, | |
| organizationId: project.organizationId, | |
| status: project.status, | |
| createdAt: expect.any(Date), | |
| updatedAt: expect.any(Date), | |
| } | |
| }); | |
| expect(result).toBeInstanceOf(Project); | |
| expect(result.id).toBe(mockSavedProject.id); | |
| }); |
🤖 Prompt for AI Agents
In src/modules/project/__tests__/repositories/PrismaProjectRepository.test.ts
around lines 71 to 107, the test for save currently asserts the Prisma create
call and that the result is a Project but does not verify the persisted id is
preserved; update the test to assert that the returned domain object's id equals
the original project.id (e.g., expect(result.id).toBe(project.id)) so the save
path preserves the id.
| dto.organizationId = "org-123"; | ||
| dto.status = ProjectStatus.DRAFT; | ||
|
|
||
| const mockProject = Project.create({ |
There was a problem hiding this comment.
Use a valid UUIDv4 for organizationId; current value will fail validation.
These tests marked “valid data” will fail against @IsUUID(4). Replace 'org-123' with a UUID.
- dto.organizationId = "org-123";
+ dto.organizationId = "550e8400-e29b-41d4-a716-446655440000";Apply the same change in all occurrences within this file.
Also applies to: 48-53, 57-64, 67-74, 77-85, 88-96
| it("should throw InternalServerException when repository findById fails", async () => { | ||
| const projectId = "project-123"; | ||
| mockRepository.findById.mockRejectedValue(new Error("Database error")); | ||
|
|
||
| await expect(useCase.execute(projectId)).rejects.toThrow(InternalServerException); | ||
| expect(mockRepository.findById).toHaveBeenCalledWith(projectId); | ||
| expect(mockRepository.delete).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Align mock rejection with repository contract.
If the repository maps persistence errors to InternalServerException, the mock should reject with that, not a plain Error. Otherwise this test forces use-case-level wrapping that may not exist.
- mockRepository.findById.mockRejectedValue(new Error("Database error"));
+ mockRepository.findById.mockRejectedValue(new InternalServerException("Database error"));If you do intend use cases to wrap unknown errors, add try/catch in DeleteProjectUseCase and rethrow InternalServerException.
🏁 Script executed:
#!/bin/bash
set -e
# Locate the DeleteProjectUseCase file
file=$(fd DeleteProjectUseCase.ts)
echo "File located at: $file"
# Print the beginning of the file to inspect the execute method
sed -n '1,200p' "$file"
echo "---- Repository findById occurrences ----"
# Search for findById occurrences in the project module
rg -n 'findById' -C3 src/modules/projectLength of output: 24818
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' src/modules/project/repositories/PrismaProjectRepository.tsLength of output: 4103
Align mock rejection with repository contract.
PrismaProjectRepository.findById catches all persistence errors and rethrows InternalServerException, so tests mocking a repository failure should reject with InternalServerException instead of a plain Error:
- mockRepository.findById.mockRejectedValue(new Error("Database error"));
+ mockRepository.findById.mockRejectedValue(new InternalServerException("Database error"));If you’d rather have the use case handle generic errors, wrap the call in DeleteProjectUseCase.execute in a try/catch and rethrow InternalServerException.
📝 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.
| it("should throw InternalServerException when repository findById fails", async () => { | |
| const projectId = "project-123"; | |
| mockRepository.findById.mockRejectedValue(new Error("Database error")); | |
| await expect(useCase.execute(projectId)).rejects.toThrow(InternalServerException); | |
| expect(mockRepository.findById).toHaveBeenCalledWith(projectId); | |
| expect(mockRepository.delete).not.toHaveBeenCalled(); | |
| }); | |
| it("should throw InternalServerException when repository findById fails", async () => { | |
| const projectId = "project-123"; | |
| mockRepository.findById.mockRejectedValue(new InternalServerException("Database error")); | |
| await expect(useCase.execute(projectId)).rejects.toThrow(InternalServerException); | |
| expect(mockRepository.findById).toHaveBeenCalledWith(projectId); | |
| expect(mockRepository.delete).not.toHaveBeenCalled(); | |
| }); |
🤖 Prompt for AI Agents
In src/modules/project/__tests__/use-cases/DeleteProjectUseCase.test.ts around
lines 50 to 57, the test mocks repository.findById to reject with a plain Error
but the real PrismaProjectRepository converts persistence errors into
InternalServerException; update the test to reject with an
InternalServerException (importing it into the test) so the mock matches the
repository contract, or alternatively modify DeleteProjectUseCase.execute to
wrap the repository call in a try/catch and rethrow an InternalServerException
on any error—choose one approach and ensure imports and assertions are adjusted
accordingly.
| it("should throw InternalServerException when repository fails", async () => { | ||
| const projectId = "project-123"; | ||
| mockRepository.findById.mockRejectedValue(new Error("Database connection failed")); | ||
|
|
||
| await expect(useCase.execute(projectId)).rejects.toThrow(InternalServerException); | ||
| expect(mockRepository.findById).toHaveBeenCalledWith(projectId); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align repository failure mock with InternalServerException.
This mirrors the repository’s error mapping and avoids requiring use-case-level wrapping.
- mockRepository.findById.mockRejectedValue(new Error("Database connection failed"));
+ mockRepository.findById.mockRejectedValue(new InternalServerException("Database connection failed"));📝 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.
| it("should throw InternalServerException when repository fails", async () => { | |
| const projectId = "project-123"; | |
| mockRepository.findById.mockRejectedValue(new Error("Database connection failed")); | |
| await expect(useCase.execute(projectId)).rejects.toThrow(InternalServerException); | |
| expect(mockRepository.findById).toHaveBeenCalledWith(projectId); | |
| }); | |
| it("should throw InternalServerException when repository fails", async () => { | |
| const projectId = "project-123"; | |
| mockRepository.findById.mockRejectedValue(new InternalServerException("Database connection failed")); | |
| await expect(useCase.execute(projectId)).rejects.toThrow(InternalServerException); | |
| expect(mockRepository.findById).toHaveBeenCalledWith(projectId); | |
| }); |
🤖 Prompt for AI Agents
In src/modules/project/__tests__/use-cases/GetProjectUseCase.test.ts around
lines 48-54, the test currently mocks repository failure with a generic Error
but the repository maps failures to InternalServerException; update the mock to
reject with an InternalServerException instance (and add the appropriate import
if missing) so the test mirrors the repository's error mapping and no extra
use-case wrapping is required.
| import { Request, Response } from "express"; | ||
| import { CreateProjectUseCase } from "../../use-cases/CreateProjectUseCase"; | ||
| import { GetProjectUseCase } from "../../use-cases/GetProjectUseCase"; | ||
| import { ListProjectsUseCase } from "../../use-cases/ListProjectsUseCase"; | ||
| import { UpdateProjectUseCase } from "../../use-cases/UpdateProjectUseCase"; | ||
| import { DeleteProjectUseCase } from "../../use-cases/DeleteProjectUseCase"; | ||
| import { CreateProjectDto } from "../../dto/CreateProjectDto"; | ||
| import { UpdateProjectDto } from "../../dto/UpdateProjectDto"; | ||
| import { AppException } from "../../../../shared/exceptions/AppException"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm Express version to decide between throw vs next(err) for async errors.
If you’re on Express 4, async throws won’t reach the error middleware without next(err) (unless you use express-async-errors or a wrapper). If on Express 5, throws are fine. Verify before merging.
Also applies to: 20-21, 39-41, 56-58, 73-75, 89-91, 109-111
🏁 Script executed:
#!/bin/bash
# Verify express version and async-error handling helpers
jq -r '.dependencies.express // .devDependencies.express' package.json 2>/dev/null || rg -n '"express"' package.json -n
rg -n 'express-async-errors' package.json
rg -nP 'router\.([a-z]+)\(.*async' -C2Length of output: 3037
Async errors won’t reach middleware with Express 4.21.2
Express version ^4.21.2 detected and no express-async-errors wrapper in use; thrown errors in async route handlers won’t bubble to your error middleware. You must either:
- install and import
express-async-errors(e.g.import 'express-async-errors';at your app entry) - or wrap each async handler in
try/catchand callnext(err)on catch
Apply to Project.controller.stub.ts and all async routes (ProjectRoutes.ts, VolunteerRoutes.ts, metrics.routes.ts).
🤖 Prompt for AI Agents
In src/modules/project/presentation/controllers/Project.controller.stub.ts lines
1-9, async route handlers are used but Express ^4.21.2 will not forward thrown
errors to error middleware; to fix, ensure async errors are propagated by either
installing and importing express-async-errors once at your application entry
(e.g. add import 'express-async-errors'; to the top of your main app/bootstrap
file) or by wrapping every async route handler in try/catch and calling
next(err) on catch; apply the same change to ProjectRoutes.ts,
VolunteerRoutes.ts, and metrics.routes.ts so all async handlers forward errors
to the error middleware.
| return Project.create({ | ||
| title: project.title, | ||
| description: project.description, | ||
| organizationId: project.organizationId, | ||
| status: project.status as ProjectStatus | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Stop using Project.create() when hydrating from the database — it generates new IDs/timestamps.
This breaks identity and auditing: every read/save/update returns a brand-new Project with a new id/createdAt/updatedAt. Rehydrate with persisted fields instead.
Apply this diff (requires a rehydration factory on Project; see helper snippet below):
- return Project.create({
- title: project.title,
- description: project.description,
- organizationId: project.organizationId,
- status: project.status as ProjectStatus
- });
+ return Project.fromPersistence({
+ id: project.id,
+ title: project.title,
+ description: project.description,
+ organizationId: project.organizationId,
+ status: project.status as ProjectStatus,
+ createdAt: project.createdAt,
+ updatedAt: project.updatedAt,
+ });Repeat the same replacement in the other mappings:
- return projects.map(project =>
- Project.create({
- title: project.title,
- description: project.description,
- organizationId: project.organizationId,
- status: project.status as ProjectStatus
- })
- );
+ return projects.map(project =>
+ Project.fromPersistence({
+ id: project.id,
+ title: project.title,
+ description: project.description,
+ organizationId: project.organizationId,
+ status: project.status as ProjectStatus,
+ createdAt: project.createdAt,
+ updatedAt: project.updatedAt,
+ })
+ );- return Project.create({
- title: savedProject.title,
- description: savedProject.description,
- organizationId: savedProject.organizationId,
- status: savedProject.status as ProjectStatus
- });
+ return Project.fromPersistence({
+ id: savedProject.id,
+ title: savedProject.title,
+ description: savedProject.description,
+ organizationId: savedProject.organizationId,
+ status: savedProject.status as ProjectStatus,
+ createdAt: savedProject.createdAt,
+ updatedAt: savedProject.updatedAt,
+ });- return Project.create({
- title: updatedProject.title,
- description: updatedProject.description,
- organizationId: updatedProject.organizationId,
- status: updatedProject.status as ProjectStatus
- });
+ return Project.fromPersistence({
+ id: updatedProject.id,
+ title: updatedProject.title,
+ description: updatedProject.description,
+ organizationId: updatedProject.organizationId,
+ status: updatedProject.status as ProjectStatus,
+ createdAt: updatedProject.createdAt,
+ updatedAt: updatedProject.updatedAt,
+ });Add this to Project.ts (outside this file) to support rehydration:
// in src/modules/project/domain/Project.ts
public static fromPersistence(props: IProject): Project {
return new Project(props);
}
public get createdAt(): Date { return this.props.createdAt; }
public get updatedAt(): Date { return this.props.updatedAt; }Also applies to: 31-38, 49-56, 76-81, 100-105
| status: project.status as ProjectStatus | ||
| }); | ||
| } catch (error) { | ||
| throw new InternalServerException(`Failed to find project with id ${id}: ${error instanceof Error ? error.message : 'Unknown error'}`); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not leak low-level error details in InternalServerException messages.
Surfacing raw error.message can expose DB internals to clients. Log the original error; return a generic, standardized message.
Example:
- throw new InternalServerException(`Failed to find project with id ${id}: ${error instanceof Error ? error.message : 'Unknown error'}`);
+ // log.error({ err: error, id }, 'Failed to find project');
+ throw new InternalServerException('Failed to find project');Consider a tiny helper (e.g., wrapPrisma) to centralize this pattern and attach context to logs.
Also applies to: 40-40, 58-58, 83-83, 107-107, 117-117
| // Validate DTO | ||
| const validationErrors = await validate(dto); | ||
| if (validationErrors.length > 0) { | ||
| const errorMessages = validationErrors.map(error => | ||
| Object.values(error.constraints || {}).join(', ') | ||
| ).join('; '); | ||
| throw new ValidationException(`Validation failed: ${errorMessages}`); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make validation robust against plain objects (instantiate DTO before validate)
validate(dto) requires a class instance. If callers pass a plain object, constraints won’t run. Instantiate defensively here.
Search whether controllers already convert body to an UpdateProjectDto instance.
-import { validate } from "class-validator";
+import { validate } from "class-validator";
+import { plainToInstance } from "class-transformer";
async execute(id: string, dto: UpdateProjectDto): Promise<Project> {
- // Validate DTO
- const validationErrors = await validate(dto);
+ // Validate DTO
+ const dtoInstance = plainToInstance(UpdateProjectDto, dto);
+ const validationErrors = await validate(dtoInstance);
if (validationErrors.length > 0) {
- const errorMessages = validationErrors.map(error =>
- Object.values(error.constraints || {}).join(', ')
- ).join('; ');
+ const errorMessages = validationErrors
+ .map(e => {
+ const messages = Object.values(e.constraints || {});
+ return messages.length ? `${e.property}: ${messages.join(', ')}` : '';
+ })
+ .filter(Boolean)
+ .join('; ');
throw new ValidationException(`Validation failed: ${errorMessages}`);
}Verification script:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 'plainToInstance\(\s*UpdateProjectDto' srcLength of output: 55
🏁 Script executed:
#!/bin/bash
# Search for all usages of UpdateProjectDto in the codebase
rg -n 'UpdateProjectDto' -n src
# Check if any imports of UpdateProjectDto occur alongside class-transformer usage
rg -n 'import .*UpdateProjectDto' -n src
rg -n 'class-transformer' -n src
# Verify any existing uses of plainToInstance to see if DTOs are already instantiated
rg -n 'plainToInstance' -n srcLength of output: 2998
Instantiate UpdateProjectDto before validation
validate(dto) ignores plain objects—convert to a class instance with plainToInstance(UpdateProjectDto, dto) at the top of execute().
-import { validate } from "class-validator";
+import { validate } from "class-validator";
+import { plainToInstance } from "class-transformer";
async execute(id: string, dto: UpdateProjectDto): Promise<Project> {
- // Validate DTO
- const validationErrors = await validate(dto);
+ // Validate DTO
+ const dtoInstance = plainToInstance(UpdateProjectDto, dto);
+ const validationErrors = await validate(dtoInstance);
if (validationErrors.length > 0) {
- const errorMessages = validationErrors.map(error =>
- Object.values(error.constraints || {}).join(', ')
- ).join('; ');
+ const errorMessages = validationErrors
+ .map(e => {
+ const messages = Object.values(e.constraints || {});
+ return messages.length ? `${e.property}: ${messages.join(', ')}` : '';
+ })
+ .filter(Boolean)
+ .join('; ');
throw new ValidationException(`Validation failed: ${errorMessages}`);
}📝 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.
| // Validate DTO | |
| const validationErrors = await validate(dto); | |
| if (validationErrors.length > 0) { | |
| const errorMessages = validationErrors.map(error => | |
| Object.values(error.constraints || {}).join(', ') | |
| ).join('; '); | |
| throw new ValidationException(`Validation failed: ${errorMessages}`); | |
| } | |
| // At the top of the file | |
| import { validate } from "class-validator"; | |
| import { plainToInstance } from "class-transformer"; | |
| async execute(id: string, dto: UpdateProjectDto): Promise<Project> { | |
| // Validate DTO | |
| const dtoInstance = plainToInstance(UpdateProjectDto, dto); | |
| const validationErrors = await validate(dtoInstance); | |
| if (validationErrors.length > 0) { | |
| const errorMessages = validationErrors | |
| .map(e => { | |
| const messages = Object.values(e.constraints || {}); | |
| return messages.length ? `${e.property}: ${messages.join(', ')}` : ''; | |
| }) | |
| .filter(Boolean) | |
| .join('; '); | |
| throw new ValidationException(`Validation failed: ${errorMessages}`); | |
| } | |
| // …rest of method… | |
| } |
🤖 Prompt for AI Agents
In src/modules/project/use-cases/UpdateProjectUseCase.ts around lines 11 to 18,
the DTO is being validated as a plain object which class-validator ignores;
convert the incoming plain dto into an UpdateProjectDto class instance using
plainToInstance(UpdateProjectDto, dto) before calling validate. Add the import
for plainToInstance from class-transformer and replace validate(dto) with
validate(classInstance) so validation rules on the DTO class are applied.
1 participant
feat: enhance project management with Supabase integration, improved error handling, and comprehensive use case implementations
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
This PR refactors the project module to use domain exceptions end-to-end, replacing legacy error handling with standardized domain exceptions throughout all layers.
Key Changes:
PrismaProjectRepositorywith proper error mapping toInternalServerExceptionValidationException,NotFoundException)AppExceptioninstances with standardized response formatError Response Format:
{ "statusCode": 400|404|409|500, "message": "Human readable error message", "errorCode": "VALIDATION_ERROR|RESOURCE_NOT_FOUND|RESOURCE_CONFLICT|INTERNAL_ERROR" }Files Modified:
📸 Evidence (A photo is required as evidence)
[Screenshot of test results showing all tests passing]
⏰ Time spent breakdown
Total: ~5 hours
�� Comments
This refactoring establishes a solid foundation for consistent error handling across the project module. The implementation follows clean architecture principles with proper separation of concerns and comprehensive test coverage. All domain exceptions are now properly mapped and return standardized error responses, making the API more predictable and easier to consume.
The test suite ensures that all error scenarios (validation failures, resource not found, infrastructure errors) are properly handled and return the expected HTTP status codes and error formats.
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
Improvements
Refactor
Tests