added solution#521
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a core requirement of the task is not yet implemented: full CRUD support for categories. You correctly configured PostgreSQL via src/db.js and implemented solid CRUD flows for users and expenses (models, services, controllers, routes), but for categories you only defined the Category model (src/models/Category.model.js) and did not expose any HTTP endpoints.
The task explicitly requires a CRUD page/API to manage categories, so you need to add:
- a
src/services/categories.service.jsusing theCategorymodel with methods likegetAllCategories,createCategory,getCategoryById,updateCategory, anddeleteCategory, - a
src/controlers/categories.controller.js(note current folder name) that validates input and returns appropriate status codes (400 for bad input, 404 when not found, 201/200/204 for success), - a
src/routes/categories.routes.jswiring the standard routes (GET /,POST /,GET /:id,PATCH /:id,DELETE /:id), - and registration of this router in
src/createServer.js, e.g.app.use('/categories', categoriesRouter).
Minor points: thesrc/controlersdirectory is consistently misspelled but functional, andPOSTGRES_DBinsrc/db.jshas no fallback—these are not blockers but worth improving. Overall your existing user and expense code is well-structured; once you add and wire the categories CRUD in the same style, your solution will meet the task requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| 'use strict'; | ||
|
|
||
| const createServer = () => { | ||
| // your code goes here | ||
| }; | ||
| const express = require('express'); | ||
| const cors = require('cors'); | ||
|
|
||
| const { usersRouter } = require('./routes/users.routes'); | ||
| const { expensesRouter } = require('./routes/expenses.routes'); |
There was a problem hiding this comment.
The .gitignore contains a mix of commentable and non-commentable markers. Line 7 uses a commentable marker but this file is a config; nothing functionally wrong but ensure markers are consistent. Also line 1-6 use /** non-commentable markers which is fine.
| 'use strict'; | ||
|
|
There was a problem hiding this comment.
In expenses.controller.js you require services from '../services/...'. The file path for controllers is src/controlers (note misspelling: 'controlers' rather than 'controllers' ), but your require uses '../services/...', which resolves relative to src/controlers — that's consistent with provided project layout. No functional issue here. Ensure directory spelling matches other parts of the project.
|
|
||
| app.use(express.json()); | ||
| app.use(cors()); |
There was a problem hiding this comment.
createExpense: you validate that userId, title, and amount are not null, then call usersService.getUserById(userId). Good. However you don't ensure amount is a number or spentAt format; the task doesn't require extra validation, so this is acceptable.
| const cors = require('cors'); | ||
|
|
||
| const { usersRouter } = require('./routes/users.routes'); | ||
| const { expensesRouter } = require('./routes/expenses.routes'); |
There was a problem hiding this comment.
In getAllExpenses you forward req.query directly to the service; OK as service handles parsing. No issues found.
| const { usersRouter } = require('./routes/users.routes'); | ||
| const { expensesRouter } = require('./routes/expenses.routes'); | ||
|
|
||
| function createServer() { | ||
| const app = express(); | ||
|
|
||
| app.use(express.json()); | ||
| app.use(cors()); | ||
|
|
||
| app.use('/users', usersRouter); | ||
| app.use('/expenses', expensesRouter); |
There was a problem hiding this comment.
createServer.js correctly wires routers and middleware. The file uses './routes/users.routes' and './routes/expenses.routes' — confirm that actual routes files are under src/routes (they are), so this is correct.
| const { Op } = require('sequelize'); | ||
| const { Expense } = require('../models/Expense.model'); | ||
|
|
||
| async function getAllExpenses(query) { | ||
| const where = {}; | ||
|
|
||
| if (query.userId) { | ||
| where.userId = Number(query.userId); | ||
| } | ||
|
|
||
| if (query.categories) { | ||
| const categories = Array.isArray(query.categories) | ||
| ? query.categories | ||
| : query.categories.split(','); | ||
|
|
||
| where.category = { | ||
| [Op.in]: categories, | ||
| }; | ||
| } | ||
|
|
||
| if (query.from || query.to) { | ||
| where.spentAt = {}; | ||
|
|
||
| if (query.from) { | ||
| where.spentAt[Op.gte] = new Date(query.from); | ||
| } |
There was a problem hiding this comment.
The project includes a Category model (src/models/Category.model.js) but there are no corresponding service, controller, or routes to expose CRUD operations for categories. The task explicitly requires implementing CRUD page to manage categories — add src/services/categories.service.js, src/controlers/categories.controller.js, src/routes/categories.routes.js and mount the router in createServer.js (e.g., app.use('/categories', categoriesRouter)).
| } | ||
|
|
||
| if (data.category !== undefined) { | ||
| expenseForUpdate.category = data.category; | ||
| } | ||
|
|
||
| if (data.note !== undefined) { |
There was a problem hiding this comment.
Controllers are placed under src/controlers (misspelled 'controlers'). It's consistent in the code so it works, but it's nonstandard and may confuse reviewers. Consider renaming to 'controllers' and updating all require paths if not constrained by tests/CI.
| if (query.from || query.to) { | ||
| where.spentAt = {}; | ||
|
|
||
| if (query.from) { |
There was a problem hiding this comment.
db.js uses POSTGRES_DB directly for the database option. Ensure POSTGRES_DB is set in environment when running tests. If tests/local runs expect a default, consider providing a fallback like POSTGRES_DB || 'postgres' to avoid connection failures.
| const { User } = require('../models/User.model'); | ||
|
|
||
| async function getAllUsers() { | ||
| return User.findAll(); | ||
| } | ||
|
|
||
| async function createUser(name) { | ||
| return User.create({ name }); | ||
| } | ||
|
|
||
| async function getUserById(id) { | ||
| return User.findByPk(id); | ||
| } | ||
|
|
||
| async function deleteUser(id) { | ||
| const userForDelete = await User.findByPk(id); | ||
|
|
||
| if (!userForDelete) { | ||
| return null; | ||
| } | ||
|
|
||
| await userForDelete.destroy(); | ||
|
|
||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
The project includes a Category model but there are no service, controller, or route files for categories. The task requires implementing CRUD for categories — add src/services/categories.service.js, src/controlers/categories.controller.js, src/routes/categories.routes.js, and mount the router in src/createServer.js (e.g., app.use('/categories', categoriesRouter)).
|
|
||
| await userForDelete.destroy(); | ||
|
|
||
| return true; |
There was a problem hiding this comment.
In src/db.js you use POSTGRES_DB without a fallback. Ensure POSTGRES_DB is set in env when running tests or add a sensible default (e.g., POSTGRES_DB || 'postgres') so Sequelize can connect during local testing.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a core requirement of the task—implementing full CRUD for categories—is not yet implemented in the HTTP layer, which blocks task completion. Currently there is no src/services/categories.service.js, no src/controlers/categories.controller.js, no src/routes/categories.routes.js, and src/createServer.js only registers the users and expenses routers, so there are no /categories endpoints exposed. To fix this, you need to:
- Add a
Categorymodel file if it’s missing (e.g.src/models/Category.model.jswithtableName: 'categories'), mirroringUserandExpensemodels. - Implement
src/services/categories.service.jswith methodsgetAllCategories,createCategory,getCategoryById,updateCategory, anddeleteCategoryusing theCategorymodel, following the same patterns as the users/expenses services. - Implement
src/controlers/categories.controller.jsthat validates input and returns appropriate status codes (400 for invalid data, 404 when a category is not found, 201/200/204 for successful create/read/update/delete), mirroring your existing controllers. - Implement
src/routes/categories.routes.jswiringGET /,POST /,GET /:id,PATCH /:id,DELETE /:idto the controller functions, exportcategoriesRouter, and then import and register it insrc/createServer.jswithapp.use('/categories', categoriesRouter);.
As a minor, non-blocking improvement, consider adding safer handling or defaults for POSTGRES_DB and POSTGRES_PASSWORD in src/db.js to make local development smoother. Your existing users and expenses layers are well-structured; once you mirror that structure for categories and register the router, your solution should meet the task requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
|
|
||
| return res.send(expenses); | ||
| } | ||
|
|
||
| async function createExpense(req, res) { | ||
| const { userId, title, amount } = req.body; | ||
|
|
||
| if (userId == null || title == null || amount == null) { | ||
| res.sendStatus(400); | ||
|
|
||
| return; |
There was a problem hiding this comment.
The project is missing the categories CRUD implementation required by the task. According to the task and previous review, you must add a categories service, controller, routes, and register the router in createServer so endpoints exist at /categories (GET /, POST /, GET /:id, PATCH /:id, DELETE /:id). None of the provided files implement or register categories routes or controllers.
|
|
||
| return res.send(expenses); | ||
| } | ||
|
|
||
| async function createExpense(req, res) { | ||
| const { userId, title, amount } = req.body; | ||
|
|
||
| if (userId == null || title == null || amount == null) { | ||
| res.sendStatus(400); | ||
|
|
||
| return; |
There was a problem hiding this comment.
In src/createServer.js you register only users and expenses routers. You need to import and use the categories router here, e.g. const { categoriesRouter } = require('./routes/categories.routes'); and app.use('/categories', categoriesRouter); so the categories endpoints are exposed as required by the task.
| @@ -0,0 +1,86 @@ | |||
| const expensesService = require('../services/expenses.service'); | |||
There was a problem hiding this comment.
There is no src/services/categories.service.js file present. Implement a service that uses the Category model with methods: getAllCategories, createCategory, getCategoryById, updateCategory, and deleteCategory as described in the checklist. The rest of your services follow this pattern and the categories service should be analogous.
| @@ -0,0 +1,86 @@ | |||
| const expensesService = require('../services/expenses.service'); | |||
There was a problem hiding this comment.
There is no src/controlers/categories.controller.js file present. Implement controller functions that validate input and return appropriate HTTP status codes: 400 for bad input, 404 when not found, and 201/200/204 for successes. Mirror the style used in users.controllers.js and expenses.controller.js (note the project uses the misspelled controlers directory).
| @@ -0,0 +1,86 @@ | |||
| const expensesService = require('../services/expenses.service'); | |||
There was a problem hiding this comment.
There is no src/routes/categories.routes.js file present. Create a router that wires the required routes (GET /, POST /, GET /:id, PATCH /:id, DELETE /:id) to controller functions, following the pattern in users.routes.js and expenses.routes.js.
| @@ -0,0 +1,15 @@ | |||
| const { Router } = require('express'); | |||
There was a problem hiding this comment.
There is no src/controlers/categories.controller.js file present. Implement controller functions that validate input and return appropriate HTTP status codes: 400 for bad input, 404 when not found, and 201/200/204 for successes. Mirror the existing controllers in src/controlers (note the directory is spelled controlers).
| @@ -0,0 +1,15 @@ | |||
| const { Router } = require('express'); | |||
There was a problem hiding this comment.
There is no src/routes/categories.routes.js file present. Create a router that wires required routes to controller functions, following the pattern in users.routes.js and expenses.routes.js and export categoriesRouter for use in createServer.
|
|
||
| if (query.userId) { | ||
| where.userId = Number(query.userId); | ||
| } | ||
|
|
||
| if (query.categories) { | ||
| const categories = Array.isArray(query.categories) | ||
| ? query.categories | ||
| : query.categories.split(','); | ||
|
|
||
| where.category = { |
There was a problem hiding this comment.
The repository is missing the categories CRUD implementation required by the task. There are no files for src/services/categories.service.js, src/controlers/categories.controller.js, or src/routes/categories.routes.js, and the categories router is not registered in src/createServer.js. The task explicitly requires implementing a CRUD page/API to manage categories (GET /, POST /, GET /:id, PATCH /:id, DELETE /:id). Add these files and register the router (e.g., app.use('/categories', categoriesRouter);) following the same patterns used for users and expenses.
| @@ -0,0 +1,101 @@ | |||
| const { Op } = require('sequelize'); | |||
There was a problem hiding this comment.
Confirm that a Category model exists at src/models/Category.model.js. I did not find it among the provided files. If it's missing, create it similarly to User.model.js and Expense.model.js (define id, name or relevant fields, tableName: 'categories', timestamps false). The categories service must use this model.
| if (query.from) { | ||
| where.spentAt[Op.gte] = new Date(query.from); | ||
| } | ||
|
|
||
| if (query.to) { | ||
| where.spentAt[Op.lte] = new Date(query.to); |
There was a problem hiding this comment.
Optional improvement: In src/db.js you use POSTGRES_DB and POSTGRES_PASSWORD without fallbacks (lines 24 and 29). This was noted previously as worth improving for local development. Not a blocker, but consider adding sensible defaults or clearer error handling if env variables are missing.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution under an educational override so you can keep progressing while learning, but there are a few important gaps to address next. You did well setting up PostgreSQL with Sequelize and implementing clear models, services, controllers, and routes for users and expenses, following a consistent and clean pattern. However, the core requirement to provide full CRUD over HTTP for categories is not yet met: there is no Category model (src/models/Category.model.js), no categories.service.js, categories.controller.js, or categories.routes.js, and src/createServer.js does not register a /categories router. This means the /categories endpoints required by the task do not exist, so tests or frontend features expecting them will fail. To fix this, mirror your existing users/expenses setup: define a Category model with tableName: 'categories', implement service methods getAllCategories, createCategory, getCategoryById, updateCategory, deleteCategory, add a controller that validates input and returns 400/404/201/200/204 as appropriate, wire routes GET /, POST /, GET /:id, PATCH /:id, DELETE /:id, and finally app.use('/categories', categoriesRouter) in createServer.js. As a minor, optional improvement, you could also add safer defaults for POSTGRES_DB and POSTGRES_PASSWORD in src/db.js to ease local testing. Overall, your existing structure is solid; once you add the categories layer following the same pattern, you’ll have a complete and well-architected solution.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| 'use strict'; | ||
|
|
||
| const createServer = () => { | ||
| // your code goes here | ||
| }; | ||
| const express = require('express'); | ||
| const cors = require('cors'); | ||
|
|
||
| const { usersRouter } = require('./routes/users.routes'); | ||
| const { expensesRouter } = require('./routes/expenses.routes'); | ||
|
|
||
| function createServer() { | ||
| const app = express(); | ||
|
|
||
| app.use(express.json()); | ||
| app.use(cors()); | ||
|
|
||
| app.use('/users', usersRouter); | ||
| app.use('/expenses', expensesRouter); |
There was a problem hiding this comment.
The task requires implementing full CRUD for categories exposed via HTTP. This project currently lacks any categories-related HTTP layer: no controller, no routes, no service, and the server doesn't register a categories router. Add the following to meet requirements:
- src/models/Category.model.js defining the Category model (tableName: 'categories').
- src/services/categories.service.js with methods: getAllCategories, createCategory, getCategoryById, updateCategory, deleteCategory that use the Category model (mirror patterns in users/expenses services).
- src/controllers/categories.controller.js that validates input and returns appropriate status codes (400 for bad input, 404 when not found, 201/200/204 for create/read/update/delete), mirroring existing controllers' behavior.
- src/routes/categories.routes.js wiring GET /, POST /, GET /:id, PATCH /:id, DELETE /:id to the controller functions and exporting categoriesRouter.
- Import and register the categories router in src/createServer.js with: app.use('/categories', categoriesRouter);
Without these, the CRUD page for categories and the PostgreSQL-backed endpoints are not implemented, which violates the core functional requirements in the description.
| const { usersRouter } = require('./routes/users.routes'); | ||
| const { expensesRouter } = require('./routes/expenses.routes'); | ||
|
|
||
| function createServer() { | ||
| const app = express(); | ||
|
|
||
| app.use(express.json()); | ||
| app.use(cors()); | ||
|
|
||
| app.use('/users', usersRouter); | ||
| app.use('/expenses', expensesRouter); |
There was a problem hiding this comment.
src/createServer.js currently registers only users and expenses routers. You must import and register the categories router as described above so /categories endpoints are exposed.
| return res.json(users); | ||
| } | ||
|
|
||
| async function createUser(req, res) { | ||
| const { name } = req.body; | ||
|
|
||
| if (!name) { | ||
| res.sendStatus(400); | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
The description requires implementing full CRUD for categories and exposing /categories endpoints. This repository file (src/createServer.js) registers only users and expenses routers and does not import or register a categories router. Add import and registration like:
const { categoriesRouter } = require('./routes/categories.routes');app.use('/categories', categoriesRouter);
This is missing and prevents meeting the task requirement to provide CRUD HTTP layer for categories.
| @@ -0,0 +1,77 @@ | |||
| const usersService = require('../services/users.service'); | |||
There was a problem hiding this comment.
There is no Category model, service, controller, or routes present among provided files. The task and your previous review demand these files: src/models/Category.model.js, src/services/categories.service.js, src/controllers/categories.controller.js, and src/routes/categories.routes.js. Implement them mirroring existing User/Expense patterns and ensure CRUD methods (getAllCategories, createCategory, getCategoryById, updateCategory, deleteCategory) are present in the service and wired through controller and routes.
| @@ -0,0 +1,77 @@ | |||
| const usersService = require('../services/users.service'); | |||
There was a problem hiding this comment.
Tests expect models to be syncable via npm run test and use PostgreSQL. In src/db.js (provided earlier) consider adding safer defaults for POSTGRES_DB and POSTGRES_PASSWORD to improve local development, e.g., default POSTGRES_DB to 'postgres' when not provided. This is a non-blocking suggestion but will reduce friction when running tests locally.
| const expensesService = require('../services/expenses.service'); | ||
| const usersService = require('../services/users.service'); | ||
|
|
||
| async function getAllExpenses(req, res) { | ||
| const expenses = await expensesService.getAllExpenses(req.query); | ||
|
|
||
| return res.send(expenses); |
There was a problem hiding this comment.
The task requires implementing full CRUD for categories, including model/service/controller/routes and registering the router in createServer.js. This repository currently has no Category model, no src/services/categories.service.js, no src/controllers/categories.controller.js, and no src/routes/categories.routes.js. Also createServer.js registers only users and expenses routers so /categories endpoints are not exposed. Add the Category model (tableName: 'categories'), implement the service with getAllCategories, createCategory, getCategoryById, updateCategory, deleteCategory mirroring users/expenses services, add the controller with proper validation and status codes (400,404,201,200,204), add routes wiring GET/POST/GET/:id/PATCH/:id/DELETE/:id and import/register categoriesRouter in src/createServer.js using app.use('/categories', categoriesRouter).
| const expensesService = require('../services/expenses.service'); | ||
| const usersService = require('../services/users.service'); | ||
|
|
||
| async function getAllExpenses(req, res) { | ||
| const expenses = await expensesService.getAllExpenses(req.query); | ||
|
|
||
| return res.send(expenses); | ||
| } | ||
|
|
||
| async function createExpense(req, res) { | ||
| const { userId, title, amount } = req.body; | ||
|
|
||
| if (userId == null || title == null || amount == null) { | ||
| res.sendStatus(400); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const user = await usersService.getUserById(userId); | ||
|
|
||
| if (!user) { | ||
| res.sendStatus(400); | ||
|
|
There was a problem hiding this comment.
Optional minor improvement suggested in the previous review: consider adding safer defaults or handling for POSTGRES_DB and POSTGRES_PASSWORD in src/db.js to ease local development (not required to pass tests).
| const { User } = require('../models/User.model'); | ||
|
|
||
| async function getAllUsers() { | ||
| return User.findAll(); | ||
| } | ||
|
|
||
| async function createUser(name) { | ||
| return User.create({ name }); | ||
| } | ||
|
|
||
| async function getUserById(id) { | ||
| return User.findByPk(id); | ||
| } | ||
|
|
||
| async function deleteUser(id) { |
There was a problem hiding this comment.
This file correctly defines and exports the users router wired to the controller methods. No functional issues here relative to the task requirements.
No description provided.