feature: Todo list fullstack app#73
Conversation
WalkthroughThis change transitions the project from an AdonisJS-based backend to a NestJS-based backend, with accompanying updates to configuration, environment, and documentation files. All AdonisJS-specific code, configurations, and tests are removed. New NestJS modules, controllers, services, DTOs, entities, migrations, and testing setups are introduced, alongside Docker and TypeORM configurations. The documentation and ignore files are also updated to reflect the new stack. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as NestJS API
participant TasksController
participant TasksService
participant DB as TypeORM DB
Client->>API: HTTP request (e.g., POST /api/tasks)
API->>TasksController: Route handling (e.g., create)
TasksController->>TasksService: Call service method (e.g., create)
TasksService->>DB: Perform DB operation (create/find/update)
DB-->>TasksService: Result
TasksService-->>TasksController: Return data/result
TasksController-->>API: Return HTTP response
API-->>Client: Response (JSON)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🧹 Nitpick comments (25)
.gitignore (4)
5-13: Clarify path for thelogsdirectory
Prefixing the folder with a slash (/logs/) prevents unintentionally ignoring files or folders calledlogsthat may appear elsewhere in the repo.-logs +/logs/
1-4: Consider also ignoring common TypeScript / Nest build folders
Many teams output tobuild/orout/in CI. Prevent noisy diffs by covering them as well./dist +/build +/out
14-20: Broaden OS-specific ignores
A few extra patterns cost nothing and catch edge-cases..DS_Store +Thumbs.db +ehthumbs.db +Icon?
30-35: Surface.vscoderecommendation
Ignoring the whole folder hides debug configs helpful to all contributors; consider keeping only user-specific state out.-.vscode/* +# Ignore VSCode user state but keep workspace configs +.vscode/* !.vscode/settings.json !.vscode/tasks.json !.vscode/launch.json !.vscode/extensions.json +.vscode/*.code-workspaceREADME.md (1)
22-24: Misleading code-block language tagUsing
tsimplies TypeScript code; this is an.envsnippet. Usebashor no tag to prevent IDE mis-highlighting.-```ts +```VITE_API_URL="http://localhost:3000"
.prettierrc (1)
1-4: Consider pinning additional Prettier optionsThe minimal set works, but omitting
printWidth,semi,tabWidth, etc., can lead to divergent formatting between editors with different defaults. Add them if team wants fully deterministic output.src/config/configuration.ts (1)
4-18: Consider adding validation for required environment variables.The configuration function doesn't validate that required environment variables are present, which could lead to runtime errors with undefined database connections.
Consider adding validation:
export const databaseConfig = () => { + const requiredEnvVars = ['DB_HOST', 'DB_USER', 'DB_PASSWORD', 'DB_DATABASE']; + const missingVars = requiredEnvVars.filter(varName => !process.env[varName]); + + if (missingVars.length > 0) { + throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`); + } + const config: TypeOrmModuleOptions = { // ... rest of config }; return config; };test/app.e2e-spec.ts (1)
18-23: Consider expanding test coverage for task functionality.The current test only covers the root endpoint. Since this is a todo list app, consider adding E2E tests for task-related endpoints to ensure proper integration testing.
it('/ (GET)', () => { return request(app.getHttpServer()) .get('/') .expect(200) .expect('Hello World!'); }); + + it('/tasks (GET)', () => { + return request(app.getHttpServer()) + .get('/tasks') + .expect(200); + });.eslintrc.js (1)
19-24: Consider enabling stricter TypeScript rules for better code quality.Several important TypeScript ESLint rules are disabled, which may impact code quality and type safety. Consider enabling some of these rules:
@typescript-eslint/explicit-function-return-type: Helps with better type documentation@typescript-eslint/no-explicit-any: Prevents use ofanytype, encouraging better typingIf you want to maintain current flexibility while improving gradually, consider setting these to 'warn' instead of 'off':
rules: { '@typescript-eslint/interface-name-prefix': 'off', - '@typescript-eslint/explicit-function-return-type': 'off', + '@typescript-eslint/explicit-function-return-type': 'warn', '@typescript-eslint/explicit-module-boundary-types': 'off', - '@typescript-eslint/no-explicit-any': 'off', + '@typescript-eslint/no-explicit-any': 'warn', },data-source.ts (1)
23-29: Consider conditional initialization instead of immediate execution.The DataSource is initialized immediately when this module is imported, which may not be ideal. Consider exporting an initialization function instead.
-AppDataSource.initialize() - .then(() => { - console.log("Data Source has been initialized!") - }) - .catch((err) => { - console.error("Error during Data Source initialization", err) - }) +export const initializeDataSource = async () => { + try { + await AppDataSource.initialize(); + console.log("Data Source has been initialized!"); + } catch (err) { + console.error("Error during Data Source initialization", err); + throw err; + } +};src/main.ts (1)
7-7: Enhance ValidationPipe configuration.Consider adding additional validation options for better error handling and security.
- app.useGlobalPipes(new ValidationPipe()); + app.useGlobalPipes(new ValidationPipe({ + whitelist: true, + forbidNonWhitelisted: true, + transform: true, + }));src/tasks/dto/get-tasks-dto.ts (2)
13-15: Consider using boolean type instead of string for favorite property.The
favoriteproperty is validated as a boolean string but could be simplified to use a boolean type directly, especially since it's used for filtering.- @IsOptional() - @IsBooleanString() - favorite?: string; + @IsOptional() + @IsBoolean() + @Transform(({ value }) => value === 'true' || value === true) + favorite?: boolean;You'll need to import
IsBooleanandTransformfromclass-validatorandclass-transformerrespectively.
9-11: Add validation for search string length.Consider adding a length constraint to prevent excessively long search queries that could impact performance.
@IsOptional() @IsString() + @Length(1, 100) search?: string;You'll need to import
Lengthfromclass-validator.docker-compose.yaml (1)
4-13: Add health check for MySQL service.Adding a health check ensures the service is ready before dependent services start.
mysql: image: mysql:8.0 restart: always container_name: task-manager-mysql-db env_file: - .env + healthcheck: + test: ["CMD", "mysqladmin", "ping", "-h", "localhost"] + timeout: 20s + retries: 10 ports: - "${MYSQL_PORT}:3306" volumes: - mysql-data:/var/lib/mysqlsrc/tasks/entities/task.entity.ts (3)
14-15: Consider using enum for status field.Using a number for status makes the code less readable and maintainable. Consider using an enum to define valid status values.
+enum TaskStatus { + TODO = 0, + IN_PROGRESS = 1, + DONE = 2, +} + @Entity() export class Task { // ... other properties - @Column() - status: number; + @Column({ + type: 'enum', + enum: TaskStatus, + default: TaskStatus.TODO, + }) + status: TaskStatus;
3-22: Add timestamp columns for better data tracking.Consider adding
createdAtandupdatedAtcolumns to track when tasks are created and modified.+import { Column, Entity, PrimaryGeneratedColumn, CreateDateColumn, UpdateDateColumn } from 'typeorm'; @Entity() export class Task { @PrimaryGeneratedColumn() id: number; // ... existing columns + @CreateDateColumn() + createdAt: Date; + + @UpdateDateColumn() + updatedAt: Date; }
8-12: Add length constraints to title and description.Consider adding length constraints to prevent excessively long values that could impact database performance.
- @Column() - title: string; + @Column({ length: 255 }) + title: string; - @Column() - description: string; + @Column({ type: 'text' }) + description: string;PULL_REQUEST.md (2)
56-64: Add missing environment variables to backend section.The backend environment variables section is missing some variables that might be needed for the NestJS application.
### Backend Create .env file: ```ini +NODE_ENV=development +PORT=3000 +FRONTEND_URL=http://localhost:5173 MYSQL_CLIENT=mysql2 MYSQL_HOST=localhost MYSQL_PORT=3307 MYSQL_ROOT_PASSWORD=admin MYSQL_DATABASE=task-manager-mysql MYSQL_USER=admin MYSQL_PASSWORD=admin--- `86-88`: **Clarify Docker command execution location.** The Docker command instruction could be clearer about the correct directory and the need for the .env file. ```diff ### 3. Run Docker ```bash -cd ../corelab-api-challenge +cd corelab-api-challenge +# Make sure .env file is created before running docker-compose sudo docker compose up --build -d</blockquote></details> <details> <summary>src/tasks/dto/create-task.dto.ts (2)</summary><blockquote> `16-17`: **Consider more specific typing for status field.** The `status` field uses `@IsInt()` validation but is typed as `number`. Consider using a more specific type or enum if the status has predefined values. ```diff +// Consider defining an enum for status values +enum TaskStatus { + PENDING = 0, + IN_PROGRESS = 1, + COMPLETED = 2 +} -@IsInt() -status: number; +@IsInt() +@IsIn([0, 1, 2]) // or @IsEnum(TaskStatus) +status: TaskStatus;
19-21: Consider removing default value assignment from DTO.Default values are typically better handled in the entity or service layer rather than the DTO. This keeps the DTO focused on validation and data transfer.
@IsOptional() @IsBoolean() -favorite: boolean = false; +favorite?: boolean;tsconfig.json (1)
15-19: Consider enabling strict TypeScript checks for better type safety.Several strict checks are disabled which may lead to runtime errors. Consider gradually enabling these for better code quality:
-"strictNullChecks": false, -"noImplicitAny": false, -"strictBindCallApply": false, -"forceConsistentCasingInFileNames": false, -"noFallthroughCasesInSwitch": false +"strictNullChecks": true, +"noImplicitAny": true, +"strictBindCallApply": true, +"forceConsistentCasingInFileNames": true, +"noFallthroughCasesInSwitch": trueIf this is part of a migration strategy, consider enabling these incrementally.
src/tasks/tasks.controller.ts (3)
31-32: Consider more explicit type conversion.The unary plus operator (
+id) works but could be more explicit for better readability and error handling.@Get(':id') -async findOne(@Param('id') id: string) { - return await this.tasksService.findOne(+id); +async findOne(@Param('id') id: string) { + const numericId = parseInt(id, 10); + if (isNaN(numericId)) { + throw new BadRequestException('Invalid ID format'); + } + return await this.tasksService.findOne(numericId); }
36-37: Apply same type conversion improvement to update method.@Patch(':id') -async update(@Param('id') id: string, @Body() updateTaskDto: UpdateTaskDto) { - return await this.tasksService.update(+id, updateTaskDto); +async update(@Param('id') id: string, @Body() updateTaskDto: UpdateTaskDto) { + const numericId = parseInt(id, 10); + if (isNaN(numericId)) { + throw new BadRequestException('Invalid ID format'); + } + return await this.tasksService.update(numericId, updateTaskDto); }
41-42: Apply same type conversion improvement to remove method.@Delete(':id') -async remove(@Param('id') id: string) { - return await this.tasksService.remove(+id); +async remove(@Param('id') id: string) { + const numericId = parseInt(id, 10); + if (isNaN(numericId)) { + throw new BadRequestException('Invalid ID format'); + } + return await this.tasksService.remove(numericId); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (60)
.adonisrc.json(0 hunks).editorconfig(0 hunks).env.example(1 hunks).eslintrc.js(1 hunks).gitignore(1 hunks).gitignore copy(0 hunks).prettierignore(0 hunks).prettierrc(1 hunks)Leiame.md(0 hunks)PULL_REQUEST.md(1 hunks)README.md(1 hunks)ace(0 hunks)ace-manifest.json(0 hunks)app/Controllers/VehiclesController.ts(0 hunks)app/Exceptions/Handler.ts(0 hunks)app/Types/Vehicle.ts(0 hunks)commands/index.ts(0 hunks)config/app.ts(0 hunks)config/bodyparser.ts(0 hunks)config/cors.ts(0 hunks)config/drive.ts(0 hunks)config/hash.ts(0 hunks)contracts/drive.ts(0 hunks)contracts/env.ts(0 hunks)contracts/events.ts(0 hunks)contracts/hash.ts(0 hunks)contracts/tests.ts(0 hunks)data-source.ts(1 hunks)docker-compose.yaml(1 hunks)env.ts(0 hunks)migrations/1722482946282-CreateTable.ts(1 hunks)migrations/1751888684636-CreateFiltersColumn.ts(1 hunks)nest-cli.json(1 hunks)package.json(1 hunks)providers/AppProvider.ts(0 hunks)server.ts(0 hunks)src/app.controller.spec.ts(1 hunks)src/app.controller.ts(1 hunks)src/app.module.ts(1 hunks)src/app.service.ts(1 hunks)src/config/configuration.ts(1 hunks)src/main.ts(1 hunks)src/tasks/dto/create-task.dto.ts(1 hunks)src/tasks/dto/get-tasks-dto.ts(1 hunks)src/tasks/dto/update-task.dto.ts(1 hunks)src/tasks/entities/task.entity.ts(1 hunks)src/tasks/tasks.controller.spec.ts(1 hunks)src/tasks/tasks.controller.ts(1 hunks)src/tasks/tasks.module.ts(1 hunks)src/tasks/tasks.service.spec.ts(1 hunks)src/tasks/tasks.service.ts(1 hunks)start/kernel.ts(0 hunks)start/routes.ts(0 hunks)test.ts(0 hunks)test/app.e2e-spec.ts(1 hunks)test/jest-e2e.json(1 hunks)tests/bootstrap.ts(0 hunks)tests/functional/vehicles-api.spec.ts(0 hunks)tsconfig.build.json(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (29)
- .prettierignore
- commands/index.ts
- app/Types/Vehicle.ts
- .editorconfig
- .gitignore copy
- contracts/tests.ts
- ace
- app/Controllers/VehiclesController.ts
- start/kernel.ts
- start/routes.ts
- contracts/events.ts
- providers/AppProvider.ts
- server.ts
- config/cors.ts
- .adonisrc.json
- contracts/drive.ts
- contracts/hash.ts
- tests/functional/vehicles-api.spec.ts
- env.ts
- ace-manifest.json
- contracts/env.ts
- app/Exceptions/Handler.ts
- config/hash.ts
- test.ts
- Leiame.md
- config/bodyparser.ts
- tests/bootstrap.ts
- config/app.ts
- config/drive.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/tasks/tasks.module.ts (1)
src/app.module.ts (1)
Module(9-18)
src/app.module.ts (2)
src/tasks/tasks.module.ts (1)
Module(7-12)src/config/configuration.ts (1)
databaseConfig(4-18)
data-source.ts (1)
src/config/configuration.ts (1)
databaseConfig(4-18)
🔇 Additional comments (20)
nest-cli.json (1)
1-8: LGTM – basic Nest CLI config is correct
deleteOutDiravoids stale artefacts;sourceRootmatches the TS config. No issues.src/tasks/dto/update-task.dto.ts (1)
1-4: DTO definition is correctLeveraging
PartialTypekeeps update semantics clean; no issues spotted.src/app.service.ts (1)
1-8: LGTM! Standard NestJS service implementation.The service follows NestJS conventions with proper decorator usage and clean structure. Consider whether this placeholder service is needed for your todo application or if it can be removed once the tasks functionality is complete.
src/app.controller.spec.ts (1)
1-22: LGTM! Proper NestJS controller test setup.The test follows NestJS testing conventions correctly with appropriate module setup and test case. The test adequately covers the simple
getHello()functionality.test/jest-e2e.json (1)
1-9: LGTM! Standard Jest e2e configuration.The configuration follows Jest best practices for NestJS e2e testing with appropriate file extensions, test environment, and TypeScript support.
src/tasks/tasks.module.ts (1)
1-12: LGTM! Well-structured NestJS module.The module follows NestJS conventions perfectly:
- Proper TypeORM feature module import for the Task entity
- Correct controller and service declarations
- Clean module organization
The module integrates well with the main
AppModuleas shown insrc/app.module.tswhere it's properly imported.tsconfig.build.json (1)
1-4: LGTM! Standard NestJS build configuration.The TypeScript build configuration correctly extends the base config and excludes appropriate directories and test files from the build output.
src/app.controller.ts (1)
1-12: LGTM! Well-structured NestJS controller.The controller follows NestJS conventions with proper dependency injection and decorator usage. The implementation is clean and straightforward.
migrations/1751888684636-CreateFiltersColumn.ts (1)
1-23: LGTM! Well-structured database migration.The migration correctly implements both up and down methods with appropriate column definitions. The
favoritecolumn has a sensible default value, and thecolorcolumn is appropriately nullable..env.example (1)
1-7: LGTM! Comprehensive database environment variables.The environment variables provide all necessary configuration options for MySQL database connectivity and follow consistent naming conventions.
src/app.module.ts (1)
9-18: Well-structured NestJS module setup.The module structure follows NestJS best practices with proper imports, controllers, and providers organization. The use of
ConfigModule.forRoot()andTypeOrmModule.forRoot()is appropriate for the root module.src/tasks/dto/get-tasks-dto.ts (1)
19-22: LGTM! Hex color validation is well implemented.The regex pattern correctly validates both 3-digit and 6-digit hex color codes with a helpful error message.
src/tasks/entities/task.entity.ts (1)
20-21: LGTM! Color field configuration is appropriate.The color field is properly configured with a length of 7 characters (perfect for hex codes like #FFFFFF) and is nullable.
PULL_REQUEST.md (2)
92-100: LGTM! Development server instructions are clear.The step-by-step instructions for running the development servers are well-structured and easy to follow.
71-74: Repository URLs verified as accessibleBoth clone URLs (corelab-web-challenge and corelab-api-challenge) return HTTP 200 OK and are reachable. No changes needed.
src/tasks/dto/create-task.dto.ts (1)
25-27: Excellent hex color validation pattern.The regex pattern correctly validates both 3 and 6 digit hex color codes with clear error messaging.
tsconfig.json (1)
3-12: NestJS configuration looks appropriate.The compiler options are well-configured for NestJS with proper decorator support, module system, and output directory.
src/tasks/tasks.service.ts (1)
25-27: SQL injection protection is properly implemented.The search query correctly uses parameterized queries to prevent SQL injection.
package.json (1)
63-78: Jest configuration is well-structured.The Jest configuration is appropriate for NestJS testing with proper file extensions, test regex, and coverage settings.
src/tasks/tasks.controller.ts (1)
16-18: Controller structure follows NestJS best practices.The controller is well-structured with proper decorator usage and dependency injection.
| # compiled output | ||
| /dist | ||
| /node_modules | ||
|
|
There was a problem hiding this comment.
Add .env* patterns to avoid accidental leakage of secrets
Environment variables often contain credentials and API keys. They should never be committed.
# compiled output
/dist
/node_modules
+
+# Environment files
+.env
+.env.*📝 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.
| # compiled output | |
| /dist | |
| /node_modules | |
| # compiled output | |
| /dist | |
| /node_modules | |
| # Environment files | |
| .env | |
| .env.* |
🤖 Prompt for AI Agents
In the .gitignore file at the beginning (lines 1 to 4), add patterns to ignore
all files starting with .env to prevent accidental commits of environment
variable files that may contain sensitive credentials. Specifically, add lines
like .env and .env.* to the .gitignore to exclude these files from version
control.
| # Frontend – React | ||
|
|
||
| You are tasked with building a web application that allows users to create and manage their to-do lists. The application should consist of a responsive webpage built in React, and an API built in Node.js to store and manage the to-do lists. | ||
| ## 🖼 Stack | ||
|
|
||
| - Vite + React | ||
| - TypeScript | ||
| - MUI (Material UI) | ||
| - React Query | ||
|
|
||
| ### The repositories | ||
| The [frontend repository](https://github.com/corelabbr/corelab-web-challenge) | ||
| ## 📁 Structure | ||
|
|
||
| If you feel more comfortable, you can pick another React framework and show us your skills. | ||
| - `src/components/` – Reusable UI components | ||
| - `src/hooks/` – React Query hooks | ||
| - `src/interfaces/` – Shared types | ||
| - `src/constants/` – Shared constants | ||
| - `src/providers/` – Shared providers | ||
|
|
||
| The [backend repository](https://github.com/corelabbr/corelab-api-challenge) | ||
| ## 🌐 Environment Variables | ||
|
|
||
| If you feel more comfortable, you can pick another Node JS framework and show us your skills. | ||
| Configure in .env: | ||
|
|
||
| ### The Layout | ||
| Open the [layout mockup](https://www.figma.com/file/sQrUVHTlyogq3qGdkqGTXN/mockup?node-id=7%3A2&t=ANTOTiqjqGWYuoUr-0) in desktop and mobile version and follow this design as much as possible. | ||
|
|
||
|
|
||
| ### The application should have the following functionality: | ||
|
|
||
| 1. Users should be able to create, read, update, and delete to-do items using the API. | ||
| 2. Users should be able to mark an item as a favorite. | ||
| 3. Users should be able to set a color for each to-do item. | ||
| 4. The React frontend should display the user's to-do list in a responsive and visually appealing manner, with the ability to filter by favorite items and color. | ||
| 5. The favorited items should be displayed at the top of the list. | ||
|
|
||
| ### Technical Requirements: | ||
| 1. The backend API should be built in Node.js framework and use a database of your choice (e.g., MongoDB, PostgreSQL, etc.). | ||
| 2. The frontend should be built in React and use modern web development tools and best practices. | ||
| 3. The application should be responsive and visually appealing. | ||
|
|
||
| ### Deliverables: | ||
| 1. A link to a GitHub repository containing the complete source code for the project. | ||
| 2. A written description of how to set up and run the application locally. | ||
|
|
||
|
|
||
| ### Evaluation Criteria: | ||
| 1. Code Quality | ||
| 2. Code Format | ||
| 3. Code Perfomance | ||
| 4. Frontend Design | ||
| 5. If your code is Easily Readable | ||
| 6. Mobile First approach | ||
| 7. Code Responsability | ||
| 8. Features Work | ||
| 9. Responsiveness | ||
| 10. Does the application meet the functionality requirements listed above? | ||
| 11. Is the code well-organized, easy to read, and well-documented? | ||
| 12. Are modern web development tools and best practices used? | ||
| 13. Is the application visually appealing and responsive? | ||
|
|
||
| ### Backend | ||
| Repository: | ||
| 1. Node: ^16.15.0 | ||
| 2. NPM: ^8.5.5 | ||
| 3. Framework: Adonis TS or any other node framework you know. | ||
| 4. Database: Choose your own, you can even save in memory. | ||
|
|
||
| ### Frontend | ||
| Repository: | ||
| 1. Node: ^16.15.0 | ||
| 2. NPM: ^8.5.5 | ||
| 3. Framework: React TS | ||
| 4. Sass or other preprocessor | ||
|
|
||
| ### Want to impress us even more? | ||
| If you feel comfortable and want to impress us even more, you can do the following: | ||
|
|
||
| 1. Work on correct types and interfaces | ||
| 2. Work on eslint rules | ||
| 3. Work prettier config | ||
| 4. Work on docker containers | ||
| 5. Work on tests | ||
| 6. Work on CI/CD | ||
|
|
||
| ### What to do when you finish? | ||
|
|
||
| Create a file PULL_REQUEST.md where you will describe what you did and how in as much detail as possible. Feel free to add videos for better explanation. | ||
|
|
||
| Create a new pull request using the same branch name for Backend and Frontend | ||
|
|
||
| Send us the pull requests and that's all! | ||
|
|
||
|
|
||
| #### Good luck! The sky is the limit 🚀 No newline at end of file | ||
| ```ts | ||
| VITE_API_URL="http://localhost:3000" | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
README now ignores backend – users will be lost
The repo holds both frontend (React) and the new NestJS backend, yet the README only documents the UI stack. At minimum, add:
• brief backend overview (NestJS + TypeORM + MySQL)
• how to run the API (env vars, npm run start:dev, Docker compose, migrations)
• cross-origin info so VITE_API_URL matches
Otherwise newcomers won’t know there is a server or how to start it.
🤖 Prompt for AI Agents
In README.md lines 1 to 24, the documentation only covers the frontend React
stack and omits any mention of the backend. Add a new section that briefly
describes the backend technology stack (NestJS, TypeORM, MySQL), instructions on
how to run the API including required environment variables, commands like `npm
run start:dev`, Docker compose usage, and database migrations. Also include
information about cross-origin setup to ensure the frontend's VITE_API_URL
matches the backend URL. This will provide a complete overview for users on both
frontend and backend setup.
| describe('TasksService', () => { | ||
| let service: TasksService; | ||
|
|
||
| beforeEach(async () => { | ||
| const module: TestingModule = await Test.createTestingModule({ | ||
| providers: [TasksService], | ||
| }).compile(); | ||
|
|
||
| service = module.get<TasksService>(TasksService); | ||
| }); | ||
|
|
||
| it('should be defined', () => { | ||
| expect(service).toBeDefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Test only checks DI – no behaviour covered
The suite confirms the service exists but doesn’t exercise any CRUD logic. Suggest at least:
• create a task → expect persistence
• update & delete flows
• error path (e.g., non-existent ID)
Example skeleton:
it('should create & retrieve a task', async () => {
- expect(service).toBeDefined();
+ const dto = { title: 'foo', description: 'bar' };
+ const created = await service.create(dto);
+ const fetched = await service.findOne(created.id);
+ expect(fetched).toMatchObject(dto);
});This will guard against regressions when business rules evolve.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/tasks/tasks.service.spec.ts around lines 4 to 18, the test suite only
verifies that the TasksService is defined but does not test any actual CRUD
behavior. To fix this, add tests that cover creating a task and verifying it is
persisted, updating and deleting tasks, and handling error cases such as
operations on non-existent IDs. This will ensure the service's business logic is
properly tested and protected against regressions.
| import { Test, TestingModule } from '@nestjs/testing'; | ||
| import { TasksController } from './tasks.controller'; | ||
| import { TasksService } from './tasks.service'; | ||
|
|
||
| describe('TasksController', () => { | ||
| let controller: TasksController; | ||
|
|
||
| beforeEach(async () => { | ||
| const module: TestingModule = await Test.createTestingModule({ | ||
| controllers: [TasksController], | ||
| providers: [TasksService], | ||
| }).compile(); | ||
|
|
||
| controller = module.get<TasksController>(TasksController); | ||
| }); | ||
|
|
||
| it('should be defined', () => { | ||
| expect(controller).toBeDefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add comprehensive test coverage for CRUD operations.
While the test structure is correct, the current test only verifies controller instantiation. For a production tasks controller, you should add tests for:
- Creating tasks
- Retrieving tasks
- Updating tasks
- Deleting tasks
- Error handling scenarios
Consider adding tests like:
describe('createTask', () => {
it('should create a new task', async () => {
// Test implementation
});
});
describe('getAllTasks', () => {
it('should return all tasks', async () => {
// Test implementation
});
});🤖 Prompt for AI Agents
In src/tasks/tasks.controller.spec.ts lines 1 to 20, the current test only
checks if the TasksController is defined. To improve coverage, add tests for all
CRUD operations: create, retrieve, update, and delete tasks, as well as error
handling scenarios. Implement separate describe blocks for each operation,
mocking the TasksService methods as needed, and write assertions to verify
expected behavior and error cases.
| host: process.env.MYSQL_HOST, | ||
| port: parseInt(process.env.MYSQL_PORT, 10) || 3306, | ||
| username: process.env.MYSQL_USER, | ||
| password: process.env.MYSQL_PASSWORD, | ||
| database: process.env.MYSQL_DATABASE, |
There was a problem hiding this comment.
Fix environment variable naming inconsistency.
The configuration uses MYSQL_* prefixed environment variables, but the .env.example file uses DB_* prefixed variables. This inconsistency will cause runtime errors when the environment variables are not found.
Apply this diff to align with the .env.example file:
- host: process.env.MYSQL_HOST,
- port: parseInt(process.env.MYSQL_PORT, 10) || 3306,
- username: process.env.MYSQL_USER,
- password: process.env.MYSQL_PASSWORD,
- database: process.env.MYSQL_DATABASE,
+ host: process.env.DB_HOST,
+ port: parseInt(process.env.DB_PORT, 10) || 3306,
+ username: process.env.DB_USER,
+ password: process.env.DB_PASSWORD,
+ database: process.env.DB_DATABASE,📝 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.
| host: process.env.MYSQL_HOST, | |
| port: parseInt(process.env.MYSQL_PORT, 10) || 3306, | |
| username: process.env.MYSQL_USER, | |
| password: process.env.MYSQL_PASSWORD, | |
| database: process.env.MYSQL_DATABASE, | |
| host: process.env.DB_HOST, | |
| port: parseInt(process.env.DB_PORT, 10) || 3306, | |
| username: process.env.DB_USER, | |
| password: process.env.DB_PASSWORD, | |
| database: process.env.DB_DATABASE, |
🤖 Prompt for AI Agents
In src/config/configuration.ts around lines 8 to 12, the environment variables
use the MYSQL_ prefix, but the .env.example file uses the DB_ prefix. To fix
this, rename all environment variable references from MYSQL_HOST, MYSQL_PORT,
MYSQL_USER, MYSQL_PASSWORD, and MYSQL_DATABASE to DB_HOST, DB_PORT, DB_USER,
DB_PASSWORD, and DB_DATABASE respectively to ensure consistency and prevent
runtime errors.
| async remove(id: number) { | ||
| const taskToRemove = await this.findOne(id); | ||
| const result = await this.tasksRepository.remove(taskToRemove); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Fix remove method to handle non-existent tasks.
The remove method could fail if findOne returns undefined, causing the remove operation to fail.
async remove(id: number) {
const taskToRemove = await this.findOne(id);
+ if (!taskToRemove) {
+ throw new NotFoundException(`Task with ID ${id} not found`);
+ }
const result = await this.tasksRepository.remove(taskToRemove);
return result;
}📝 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 remove(id: number) { | |
| const taskToRemove = await this.findOne(id); | |
| const result = await this.tasksRepository.remove(taskToRemove); | |
| return result; | |
| } | |
| async remove(id: number) { | |
| const taskToRemove = await this.findOne(id); | |
| if (!taskToRemove) { | |
| throw new NotFoundException(`Task with ID ${id} not found`); | |
| } | |
| const result = await this.tasksRepository.remove(taskToRemove); | |
| return result; | |
| } |
🤖 Prompt for AI Agents
In src/tasks/tasks.service.ts around lines 55 to 59, the remove method does not
handle the case where findOne returns undefined, which can cause the remove
operation to fail. Add a check after calling findOne to verify if the task
exists; if it does not, throw an appropriate error or handle the case gracefully
before attempting to remove the task.
| async findOne(id: number) { | ||
| const task = await this.tasksRepository.findBy({ id }); | ||
| return task[0]; | ||
| } |
There was a problem hiding this comment.
Fix incorrect findOne implementation.
The current implementation uses findBy which returns an array, then accesses [0]. This is incorrect and could return undefined without proper error handling.
async findOne(id: number) {
- const task = await this.tasksRepository.findBy({ id });
- return task[0];
+ const task = await this.tasksRepository.findOne({ where: { id } });
+ if (!task) {
+ throw new NotFoundException(`Task with ID ${id} not found`);
+ }
+ return task;
}📝 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 findOne(id: number) { | |
| const task = await this.tasksRepository.findBy({ id }); | |
| return task[0]; | |
| } | |
| async findOne(id: number) { | |
| const task = await this.tasksRepository.findOne({ where: { id } }); | |
| if (!task) { | |
| throw new NotFoundException(`Task with ID ${id} not found`); | |
| } | |
| return task; | |
| } |
🤖 Prompt for AI Agents
In src/tasks/tasks.service.ts around lines 42 to 45, the findOne method
incorrectly uses findBy which returns an array, then accesses the first element
without error handling. Replace findBy with a method that returns a single
entity, such as findOneBy or findOne, to directly fetch the task by id. Add
proper error handling to manage cases where no task is found, ensuring the
method returns a consistent and expected result.
| if (query.favorite === 'true') { | ||
| qb.andWhere('task.favorite = :fav', { fav: true }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix string comparison for boolean query parameter.
The favorite filter compares against string 'true' which suggests the query parameter comes as a string. Consider proper type conversion.
-if (query.favorite === 'true') {
+if (query.favorite === true || query.favorite === 'true') {
qb.andWhere('task.favorite = :fav', { fav: true });
+} else if (query.favorite === false || query.favorite === 'false') {
+ qb.andWhere('task.favorite = :fav', { fav: false });
}📝 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 (query.favorite === 'true') { | |
| qb.andWhere('task.favorite = :fav', { fav: true }); | |
| } | |
| if (query.favorite === true || query.favorite === 'true') { | |
| qb.andWhere('task.favorite = :fav', { fav: true }); | |
| } else if (query.favorite === false || query.favorite === 'false') { | |
| qb.andWhere('task.favorite = :fav', { fav: false }); | |
| } |
🤖 Prompt for AI Agents
In src/tasks/tasks.service.ts around lines 29 to 31, the code compares the
query.favorite parameter as a string 'true' to filter tasks by favorite status.
To fix this, convert the query.favorite value to a boolean before comparison,
ensuring the filter works correctly regardless of the query parameter type. For
example, parse the string to a boolean or use a strict boolean check instead of
a string comparison.
| "dependencies": { | ||
| "@nestjs/common": "^10.0.0", | ||
| "@nestjs/core": "^10.0.0", | ||
| "@nestjs/mapped-types": "*", | ||
| "@nestjs/platform-express": "^10.0.0", | ||
| "reflect-metadata": "^0.1.13", | ||
| "rxjs": "^7.8.1" |
There was a problem hiding this comment.
Missing required dependencies for validation.
The DTOs use class-validator and class-transformer which are not included in the dependencies. These are required for the validation decorators to work.
"dependencies": {
"@nestjs/common": "^10.0.0",
"@nestjs/core": "^10.0.0",
"@nestjs/mapped-types": "*",
"@nestjs/platform-express": "^10.0.0",
+ "class-validator": "^0.14.0",
+ "class-transformer": "^0.5.1",
"reflect-metadata": "^0.1.13",
"rxjs": "^7.8.1"
}📝 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.
| "dependencies": { | |
| "@nestjs/common": "^10.0.0", | |
| "@nestjs/core": "^10.0.0", | |
| "@nestjs/mapped-types": "*", | |
| "@nestjs/platform-express": "^10.0.0", | |
| "reflect-metadata": "^0.1.13", | |
| "rxjs": "^7.8.1" | |
| "dependencies": { | |
| "@nestjs/common": "^10.0.0", | |
| "@nestjs/core": "^10.0.0", | |
| "@nestjs/mapped-types": "*", | |
| "@nestjs/platform-express": "^10.0.0", | |
| "class-validator": "^0.14.0", | |
| "class-transformer": "^0.5.1", | |
| "reflect-metadata": "^0.1.13", | |
| "rxjs": "^7.8.1" | |
| } |
🤖 Prompt for AI Agents
In package.json around lines 27 to 33, the dependencies are missing the required
packages 'class-validator' and 'class-transformer' which are necessary for the
validation decorators used in DTOs. Add both 'class-validator' and
'class-transformer' to the dependencies section with appropriate version numbers
to ensure validation works correctly.
| "ts-loader": "^9.4.3", | ||
| "ts-node": "^10.9.1", | ||
| "tsconfig-paths": "^4.2.0", | ||
| "typeorm": "^0.3.20", |
There was a problem hiding this comment.
Move TypeORM to dependencies.
TypeORM should be in dependencies rather than devDependencies since it's required at runtime.
"dependencies": {
"@nestjs/common": "^10.0.0",
"@nestjs/core": "^10.0.0",
"@nestjs/mapped-types": "*",
"@nestjs/platform-express": "^10.0.0",
"reflect-metadata": "^0.1.13",
- "rxjs": "^7.8.1"
+ "rxjs": "^7.8.1",
+ "typeorm": "^0.3.20"
}And remove from devDependencies:
-"typeorm": "^0.3.20",📝 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.
| "typeorm": "^0.3.20", | |
| "dependencies": { | |
| "@nestjs/common": "^10.0.0", | |
| "@nestjs/core": "^10.0.0", | |
| "@nestjs/mapped-types": "*", | |
| "@nestjs/platform-express": "^10.0.0", | |
| "reflect-metadata": "^0.1.13", | |
| "rxjs": "^7.8.1", | |
| "typeorm": "^0.3.20" | |
| }, | |
| "devDependencies": { | |
| // other devDependencies here, with "typeorm": "^0.3.20" removed | |
| } |
🤖 Prompt for AI Agents
In package.json at line 60, move the "typeorm" package entry from
devDependencies to dependencies because it is required at runtime. Remove the
"typeorm" entry from the devDependencies section to avoid duplication and ensure
proper package management.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores