feat(projects): [#5] Implement project CRUD API endpoints#20
Merged
Conversation
- Create ProjectsModule, Controller, and Service using Nest CLI - Define Project entity with TypeORM and establish ManyToOne relationship with User - Create DTOs with class-validator for creating and updating projects - Implement CRUD operations in ProjectsService scoped strictly to the authenticated user's ID - Protect all /projects endpoints using JwtAuthGuard
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a new Projects feature to the backend: DTOs, TypeORM entity, module, controller, and service with JWT-protected per-user CRUD endpoints; registers the module in AppModule and adds Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as ProjectsController
participant Service as ProjectsService
participant Repo as ProjectRepository
participant DB as Database
Client->>Controller: POST /projects (JWT)
Controller->>Service: create(createDto, userId)
Service->>Repo: save(project with userId)
Repo->>DB: INSERT projects
DB-->>Repo: saved project
Repo-->>Service: project
Service-->>Controller: project
Controller-->>Client: 201 Created
Client->>Controller: GET /projects (JWT)
Controller->>Service: findAll(userId)
Service->>Repo: find({ userId }, order)
Repo->>DB: SELECT FROM projects WHERE user_id=...
DB-->>Repo: projects[]
Repo-->>Service: projects[]
Service-->>Controller: projects[]
Controller-->>Client: 200 OK
Client->>Controller: PATCH /projects/:id (JWT)
Controller->>Service: update(id, updateDto, userId)
Service->>Repo: findOne({ id, userId })
Repo->>DB: SELECT FROM projects WHERE id=... AND user_id=...
DB-->>Repo: project or null
Repo-->>Service: project
Service->>Repo: save(updated project)
Repo->>DB: UPDATE projects
DB-->>Repo: updated project
Repo-->>Service: updated project
Service-->>Controller: updated project
Controller-->>Client: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
apps/backend/src/projects/projects.service.ts (1)
56-60: Note:remove()returns an entity without its ID.
Repository.remove()returns the entity with theidproperty set toundefinedafter removal. If the caller needs the ID (e.g., for logging or response), consider usingRepository.delete()or capturing the ID before removal.This is a minor behavioral note—the current implementation works correctly for deletion.
✨ Alternative if ID is needed in response
async remove(id: number, userId: number) { const project = await this.findOne(id, userId); - return await this.projectsRepository.remove(project); + await this.projectsRepository.remove(project); + return { id, ...project }; }Or use
delete()if you only need to confirm deletion:async remove(id: number, userId: number) { const result = await this.projectsRepository.delete({ id, userId }); if (result.affected === 0) { throw new NotFoundException(`Project with ID "${id}" not found or you don't have access.`); } return { deleted: true, id }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/projects/projects.service.ts` around lines 56 - 60, The remove method currently calls this.projectsRepository.remove(project) which returns the entity with its id set to undefined after deletion; capture the id before calling remove (e.g., const id = project.id) and return that id with the deletion result, or switch to this.projectsRepository.delete({ id, userId }) in the remove function and return a confirmation object containing the id (and throw NotFoundException when result.affected === 0); update the remove method and any callers accordingly (referencing remove, findOne, projectsRepository.remove, projectsRepository.delete).apps/backend/src/projects/entities/project.entity.ts (1)
20-21: Nullable column should have nullable TypeScript type.The
descriptioncolumn is marked asnullable: true, but the TypeScript type isstring. For type safety and to accurately represent the data model, consider usingstring | null.✨ Proposed fix
`@Column`({ nullable: true }) - description: string; + description: string | null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/projects/entities/project.entity.ts` around lines 20 - 21, The Project entity's description property is declared nullable in the `@Column`({ nullable: true }) decorator but typed as string; change the TypeScript type of the description field (in the Project class/entity where the `@Column` for description is defined) from string to string | null so the type reflects the database nullability and prevents unsafe assumptions elsewhere.apps/backend/src/projects/projects.controller.ts (2)
25-25: Consider extracting repeated request type.The inline type
{ user: { userId: number; email: string } }is duplicated across all 5 methods. Extracting it to a shared interface improves maintainability.✨ Suggested approach
Create a shared type (e.g., in a
typesorinterfacesdirectory):export interface AuthenticatedRequest { user: { userId: number; email: string }; }Then use it in the controller:
+import { AuthenticatedRequest } from '@/auth/interfaces/authenticated-request.interface'; + // In each method: -@Request() req: { user: { userId: number; email: string } } +@Request() req: AuthenticatedRequestAlso applies to: 31-31, 38-38, 47-47, 55-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/projects/projects.controller.ts` at line 25, Extract the repeated inline request type into a shared interface (e.g., AuthenticatedRequest) and replace the inline `{ user: { userId: number; email: string } }` annotations in all controller methods in projects.controller.ts with that interface; create and export the interface from a central types file (e.g., interfaces/types) and update the method parameters that use `@Request`() req to be typed as AuthenticatedRequest across the five occurrences so signatures like the ones currently using the inline type are consistent and maintainable.
37-37: Consider validating theidparameter.The
idparam is converted from string to number using+id. If a non-numeric value is passed (e.g.,"abc"), this results inNaN, which propagates to the service. While the service will throwNotFoundException, addingParseIntPipeprovides clearer error messages and fails fast at the controller level.✨ Proposed fix for findOne (apply similarly to update and remove)
+import { ParseIntPipe } from '@nestjs/common'; `@Get`(':id') findOne( - `@Param`('id') id: string, + `@Param`('id', ParseIntPipe) id: number, `@Request`() req: { user: { userId: number; email: string } }, ) { - return this.projectsService.findOne(+id, req.user.userId); + return this.projectsService.findOne(id, req.user.userId); }Also applies to: 45-45, 54-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/projects/projects.controller.ts` at line 37, The controller currently reads the route param as a string (e.g., `@Param`('id') id: string) and coerces it with +id which yields NaN for non-numeric input; update the controller methods findOne, update, and remove to use Nest's ParseIntPipe on the id param (e.g., `@Param`('id', ParseIntPipe) id: number) and remove the manual +id conversion so invalid ids fail fast with a clear 400 error instead of propagating NaN to the service.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/package.json`:
- Line 27: Update the dependency declaration for "@nestjs/mapped-types" in
package.json (currently set to "*") to a specific, compatible version range such
as "^2.1.0" to ensure reproducible installs and avoid pulling incompatible
future releases; replace the "*" value with "^2.1.0" so it matches the NestJS 11
compatible peer range.
In `@apps/backend/src/projects/dto/create-project.dto.ts`:
- Around line 4-6: The DTO currently allows whitespace-only project names
because `@IsNotEmpty`() doesn't reject strings like " " — update the
create-project DTO by adding a non-whitespace validator to the name property
(e.g., add `@Matches`(/\S/, { message: 'Name must not be empty or whitespace' })
above the name field) so the property 'name' is rejected when it contains only
whitespace; keep the existing `@IsString`() and `@IsNotEmpty`() and place the new
decorator on the same 'name' property in create-project.dto.ts.
In `@apps/backend/src/projects/entities/project.entity.ts`:
- Line 27: The `@JoinColumn` on the Project entity is using camelCase ('userId')
but the DB column is snake_case ('user_id'); update the JoinColumn decorator to
use { name: 'user_id' } on the relation in Project (the `@JoinColumn` decorator on
the Project entity class) so TypeORM maps the foreign key to the existing
database column; ensure any other references to the foreign key column in that
entity (e.g., the relation property or explicit column definitions) match
'user_id'.
In `@apps/backend/src/projects/projects.controller.spec.ts`:
- Around line 9-12: The test is injecting the real ProjectsService into the
controller spec which causes compile to fail due to repository injection;
replace the real provider with a stub/mock provider for ProjectsService when
calling Test.createTestingModule so the controller is tested in isolation.
Provide a mock object for ProjectsService (e.g., methods used by
ProjectsController) and pass it as { provide: ProjectsService, useValue:
mockProjectsService } in the providers array of Test.createTestingModule, then
compile and retrieve ProjectsController from the module.
In `@apps/backend/src/projects/projects.service.spec.ts`:
- Around line 8-10: The test fails to compile because ProjectsService depends on
Repository<Project> injected via `@InjectRepository`(Project); update the
Test.createTestingModule call to provide a mock repository by adding a provider
using getRepositoryToken(Project) with a stubbed/mock value (e.g., an object
implementing the repository methods used by ProjectsService like find, findOne,
save, etc.). Ensure the provider entry is included alongside ProjectsService in
the providers array so the TestingModule can resolve the Repository<Project>
dependency when instantiating ProjectsService.
---
Nitpick comments:
In `@apps/backend/src/projects/entities/project.entity.ts`:
- Around line 20-21: The Project entity's description property is declared
nullable in the `@Column`({ nullable: true }) decorator but typed as string;
change the TypeScript type of the description field (in the Project class/entity
where the `@Column` for description is defined) from string to string | null so
the type reflects the database nullability and prevents unsafe assumptions
elsewhere.
In `@apps/backend/src/projects/projects.controller.ts`:
- Line 25: Extract the repeated inline request type into a shared interface
(e.g., AuthenticatedRequest) and replace the inline `{ user: { userId: number;
email: string } }` annotations in all controller methods in
projects.controller.ts with that interface; create and export the interface from
a central types file (e.g., interfaces/types) and update the method parameters
that use `@Request`() req to be typed as AuthenticatedRequest across the five
occurrences so signatures like the ones currently using the inline type are
consistent and maintainable.
- Line 37: The controller currently reads the route param as a string (e.g.,
`@Param`('id') id: string) and coerces it with +id which yields NaN for
non-numeric input; update the controller methods findOne, update, and remove to
use Nest's ParseIntPipe on the id param (e.g., `@Param`('id', ParseIntPipe) id:
number) and remove the manual +id conversion so invalid ids fail fast with a
clear 400 error instead of propagating NaN to the service.
In `@apps/backend/src/projects/projects.service.ts`:
- Around line 56-60: The remove method currently calls
this.projectsRepository.remove(project) which returns the entity with its id set
to undefined after deletion; capture the id before calling remove (e.g., const
id = project.id) and return that id with the deletion result, or switch to
this.projectsRepository.delete({ id, userId }) in the remove function and return
a confirmation object containing the id (and throw NotFoundException when
result.affected === 0); update the remove method and any callers accordingly
(referencing remove, findOne, projectsRepository.remove,
projectsRepository.delete).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3737d7ae-0f24-4152-ba96-8d9c8d9f790e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
apps/backend/package.jsonapps/backend/src/app.module.tsapps/backend/src/projects/dto/create-project.dto.tsapps/backend/src/projects/dto/update-project.dto.tsapps/backend/src/projects/entities/project.entity.tsapps/backend/src/projects/projects.controller.spec.tsapps/backend/src/projects/projects.controller.tsapps/backend/src/projects/projects.module.tsapps/backend/src/projects/projects.service.spec.tsapps/backend/src/projects/projects.service.tsapps/backend/src/users/user.entity.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5
Summary by CodeRabbit
New Features
Tests