add task solution#507
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the primary requirement of this task—implementing full CRUD functionality for categories—is missing from your submission. While the existing code for users and expenses is well-structured, the new feature has not been added.
To complete the task, you'll need to:
- Create a new
Categorymodel. - Implement the corresponding service, controller, and router for categories.
- Ensure the new routes are registered in
src/createServer.js. - Add the new
Categorymodel to the database synchronization script insrc/setup.js.
Please implement these missing pieces to manage categories as required by the task description.
✨ 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
|
|
||
| app.use(express.json()); | ||
| app.use('/users', routes.usersRouter); | ||
| app.use('/expenses', routes.expensesRouter); |
There was a problem hiding this comment.
The new routes for managing categories are missing. Based on the task requirements, you should add a new router here for the /categories endpoint.
| module.exports = { | ||
| usersController, | ||
| expensesController, | ||
| }; |
There was a problem hiding this comment.
The task requires implementing CRUD operations for categories. This would involve creating a categoriesController, which should be imported and exported from this file.
| module.exports = { | ||
| usersService, | ||
| expensesService, | ||
| }; |
There was a problem hiding this comment.
The task is to add CRUD functionality for categories. This would require a new categories.service.js file. Once created, the categoriesService should be imported and exported from this index file.
| exports.routes = { | ||
| usersRouter, | ||
| expensesRouter, | ||
| }; |
There was a problem hiding this comment.
According to the task requirements, you need to implement CRUD for categories. This file should import the categoriesRouter from its corresponding route file and include it in the exported routes object.
| const { User, Expense } = require('./models'); | ||
|
|
||
| User.sync({ force: true }); | ||
| Expense.sync({ force: true }); |
There was a problem hiding this comment.
The new Category model, which is a core part of the task, is not being synchronized with the database here. You need to import the Category model and add a Category.sync({ force: true }) call.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The category CRUD implementation is incomplete because the routes are not registered in the server, which is a critical blocker preventing the feature from working. Specifically, src/routes/index.js does not export categoriesRouter, so it cannot be imported and used in src/createServer.js. Additionally, src/createServer.js is missing the required route registration: app.use('/categories', routes.categoriesRouter);. Without these two changes, the /categories endpoints will be completely inaccessible, violating the core task requirement to ensure new routes are registered in src/createServer.js. Other integration gaps — like missing exports for categoriesController in src/controllers/index.js and categoriesService in src/services/index.js — also reduce consistency and maintainability. Please add the missing exports and route registration to complete the integration.
✨ 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.sendStatus(400); | ||
| } | ||
|
|
||
| const category = await categoriesService.create(name); |
There was a problem hiding this comment.
The categories router is not registered in the server. The task requires ensuring new routes are registered in createServer.js.
| const { categoriesService } = require('../services/categories.service'); | ||
|
|
||
| const create = async (req, res) => { | ||
| const { name } = req.body; |
There was a problem hiding this comment.
The categories router is missing from exports. Need to export categoriesRouter here so it can be used in createServer.js.
| const { usersService } = require('../services/users.service'); | ||
|
|
||
| const create = async (req, res) => { | ||
| const { name } = req.body; | ||
|
|
||
| if (!name) { |
There was a problem hiding this comment.
The categoriesController is not exported in the controllers index file - other parts of the app cannot import it
| const { usersService } = require('../services/users.service'); | ||
|
|
||
| const create = async (req, res) => { | ||
| const { name } = req.body; | ||
|
|
||
| if (!name) { | ||
| return res.sendStatus(400); |
There was a problem hiding this comment.
Need to add categoriesController export here to make it available to routes
| const { userId, spentAt, title, amount, category, note } = req.body; | ||
| const user = await usersService.getOne(userId); | ||
|
|
||
| if (!userId || !user) { | ||
| return res.sendStatus(400); |
There was a problem hiding this comment.
src/routes/index.js is missing export for categoriesRouter
| } | ||
|
|
||
| const expense = await expensesService.create({ |
There was a problem hiding this comment.
src/createServer.js is missing route registration for categoriesRouter
| const user = await usersService.getOne(userId); | ||
|
|
||
| if (!userId || !user) { |
There was a problem hiding this comment.
src/services/index.js is missing export for categoriesService
| const user = await usersService.getOne(userId); | ||
|
|
||
| if (!userId || !user) { |
There was a problem hiding this comment.
src/controllers/index.js is missing export for categoriesController
|
|
||
| module.exports = { | ||
| usersController, | ||
| expensesController, |
There was a problem hiding this comment.
The categoriesRouter is not exported from src/routes/index.js. Only usersRouter and expensesRouter are exported, but categoriesRouter needs to be added to make it available for registration.
| module.exports = { | ||
| usersController, | ||
| expensesController, | ||
| }; |
There was a problem hiding this comment.
The categoriesController is not imported or exported from src/controllers/index.js. Without exporting it, other parts of the application cannot access the category controller.
| const { expensesController } = require('./expenses.controller'); | ||
|
|
||
| module.exports = { | ||
| usersController, |
There was a problem hiding this comment.
The categoriesService is not exported from src/services/index.js. Without exporting it, the controller cannot access the category service methods.
| type: DataTypes.INTEGER, | ||
| allowNull: false, | ||
| }, | ||
| title: { |
There was a problem hiding this comment.
The categories router is not registered in the server. The task explicitly requires: 'Ensure the new routes are registered in src/createServer.js'. Add: app.use('/categories', routes.categoriesRouter);
| app.use('/users', routes.usersRouter); | ||
| app.use('/expenses', routes.expensesRouter); |
There was a problem hiding this comment.
The categories router is NOT registered in the server. The task requires: 'Ensure the new routes are registered in src/createServer.js'. Currently only /users and /expenses are registered. Need to add: app.use('/categories', routes.categoriesRouter); after line 10.
| const { Router } = require('express'); | ||
| const { expensesController } = require('./../controllers/expenses.controller'); | ||
|
|
||
| const expensesRouter = Router(); | ||
|
|
||
| expensesRouter.post('/', expensesController.create); |
There was a problem hiding this comment.
The categories router is not imported and exported. Need to add: const { categoriesRouter } = require('./categories.route'); and add categoriesRouter to the exports object.
| 'use strict'; | ||
|
|
||
| const { sequelize } = require('../db.js'); | ||
| const { sequelize } = require('../db'); | ||
| const { DataTypes } = require('sequelize'); | ||
|
|
||
| const User = sequelize.define( | ||
| // your code goes here | ||
| ); | ||
| const User = sequelize.define('User', { |
There was a problem hiding this comment.
The categoriesRouter is NOT exported from src/routes/index.js. Currently only usersRouter and expensesRouter are exported. Need to add: const { categoriesRouter } = require('./categories.route'); and add categoriesRouter to the exports object.
| const { usersRouter } = require('./users.route'); | ||
| const { expensesRouter } = require('./expenses.route'); | ||
|
|
||
| exports.routes = { | ||
| usersRouter, | ||
| expensesRouter, |
There was a problem hiding this comment.
The categories router is not exported from src/routes/index.js. Currently only usersRouter and expensesRouter are exported. Need to add: const { categoriesRouter } = require('./categories.route'); and include it in exports.
|
|
||
| exports.routes = { | ||
| usersRouter, | ||
| expensesRouter, |
There was a problem hiding this comment.
Need to export categoriesRouter in the routes object to make it available for registration in createServer.js.
| const { Router } = require('express'); | ||
| const { | ||
| categoriesController, | ||
| } = require('./../controllers/categories.controller'); | ||
|
|
||
| const categoriesRouter = Router(); |
There was a problem hiding this comment.
The categoriesRouter is NOT exported. Need to import it and add it to exports: const { categoriesRouter } = require('./categories.route'); and add categoriesRouter to the exports object. Without this, the categories route cannot be used in createServer.js
| const { Category } = require('../models'); | ||
|
|
||
| const normalize = ({ id, name }) => ({ id, name }); | ||
|
|
||
| const create = async (name) => { | ||
| const category = await Category.create({ name }); |
There was a problem hiding this comment.
The categoriesRouter is NOT exported. Need to import it and add to exports: const { categoriesRouter } = require('./categories.route'); and add categoriesRouter to the exports object.
| const { Router } = require('express'); | ||
| const { usersController } = require('./../controllers/users.controller'); | ||
|
|
||
| const usersRouter = Router(); | ||
|
|
||
| usersRouter.post('/', usersController.create); |
There was a problem hiding this comment.
The categoriesService is NOT exported from src/services/index.js. Currently only usersService and expensesService are exported. Need to add: const { categoriesService } = require('./categories.service'); and include it in the exports.
| const { usersService } = require('./users.service'); | ||
| const { expensesService } = require('./expenses.service'); | ||
|
|
||
| module.exports = { | ||
| usersService, | ||
| expensesService, |
There was a problem hiding this comment.
The categoriesService is not exported from src/services/index.js. Need to add: const { categoriesService } = require('./categories.service'); and include it in the exports.
… index files and fix imports paths
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override to help you move forward, but please take a moment to review the feedback below.
Excellent work addressing all the integration issues from the previous review! The category CRUD functionality is now fully implemented and works well.
There's one small improvement to consider in src/controllers/categories.controller.js. The update function is missing validation for the request body. If a client tries to update a category without providing a name, it could cause an issue. It's a good practice to add a check to ensure req.body.name is present before proceeding, just like you did in the create function.
This is a minor point in an otherwise excellent submission. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| res.send(categoriesService.normalize(category)); | ||
| }; | ||
|
|
||
| const update = async (req, res) => { |
There was a problem hiding this comment.
It's a good practice to validate the request body here, similar to how you've done it in the create function. What would happen if a client sends a request to update a category without providing a name? Consider adding a check to ensure req.body.name is present and returning a 400 Bad Request if it's not.
No description provided.