Develop#944
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation is missing the second required collection — the expenses collection with all 5 endpoints. The user routes are working correctly (13 tests pass), but all 11 expense tests fail with 404 errors because the /expenses routes aren't registered on the Express server. To fix this, you need to add the expenses collection in src/createServer.js with these endpoints: - POST /expenses — create expense (201) - GET /expenses — list all expenses (200) - GET /expenses/:id — get single expense (200) - PATCH /expenses/:id — update expense (200) - DELETE /expenses/:id — delete expense (204). Add an in-memory expenses array alongside your users array, implement the same error handling pattern (400 for missing required fields, 404 for non-existent IDs), and use the same structure you used for the users collection. This completes the 2 collections × 5 endpoints requirement for the expense tracking app.
✨ 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
| const users = []; | ||
| let nextId = 1; | ||
|
|
||
| app.use(express.json()); | ||
|
|
||
| app.post('/users', (req, res) => { | ||
| const { name } = req.body; | ||
|
|
||
| if (!name) { | ||
| return res.status(400).json({ error: 'Name is required' }); | ||
| } | ||
|
|
||
| const newUser = { | ||
| id: nextId++, | ||
| name, | ||
| }; | ||
|
|
||
| users.push(newUser); | ||
|
|
||
| return res.status(201).json(newUser); | ||
| }); | ||
|
|
||
| app.get('/users', (req, res) => { | ||
| return res.json(users); | ||
| }); | ||
|
|
||
| app.get('/users/:id', (req, res) => { | ||
| const { id } = req.params; | ||
| const user = users.find((currentUser) => currentUser.id === Number(id)); | ||
|
|
||
| if (!user) { | ||
| return res.status(404).json({ error: 'User not found' }); | ||
| } | ||
|
|
||
| return res.json(user); | ||
| }); | ||
|
|
||
| app.patch('/users/:id', (req, res) => { | ||
| const { id } = req.params; | ||
| const { name } = req.body; | ||
| const user = users.find((u) => u.id === Number(id)); | ||
|
|
||
| if (!user) { | ||
| return res.status(404).json({ error: 'User not found' }); | ||
| } | ||
|
|
||
| if (name) { | ||
| user.name = name; | ||
| } | ||
|
|
||
| return res.json(user); | ||
| }); | ||
|
|
||
| app.delete('/users/:id', (req, res) => { | ||
| const { id } = req.params; | ||
| const index = users.findIndex((u) => u.id === Number(id)); | ||
|
|
||
| if (index === -1) { | ||
| return res.status(404).json({ error: 'User not found' }); | ||
| } | ||
|
|
||
| users.splice(index, 1); | ||
|
|
||
| return res.sendStatus(204); | ||
| }); |
There was a problem hiding this comment.
Only the 'users' collection is implemented. The task requires 2 collections with 5 endpoints each for an expense tracking app. Based on the requirements, you need to add a second collection (likely 'expenses' or 'transactions') with its own 5 endpoints: POST, GET all, GET by ID, PATCH by ID, and DELETE by ID.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation successfully created both collections with all 10 endpoints, and the 404 error handling works correctly. However, the POST /expenses endpoint is missing critical validation for required fields. According to the Swagger API documentation, spentAt, title, amount, and category are required parameters that should trigger a 400 error when not provided — currently, the endpoint accepts requests without these fields and creates expenses with undefined values. Add validation checks like if (!spentAt) return res.status(400).json({ error: 'spentAt is required' }) for each required field, matching the pattern already implemented for the users endpoints. This is a blocker because the API specification explicitly requires these validations, and tests will expect 400 responses for missing required parameters.
✨ 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.post('/expenses', (req, res) => { | ||
| const { userId, spentAt, title, amount, category, note } = req.body; | ||
| const user = users.find((u) => u.id === userId); | ||
|
|
||
| if (!user) { | ||
| return res.status(400).json({ error: 'User not found' }); | ||
| } | ||
|
|
||
| const expense = { | ||
| id: nextExpenseId++, | ||
| userId, | ||
| spentAt, | ||
| title, | ||
| amount, | ||
| category, | ||
| note, | ||
| }; | ||
|
|
||
| expenses.push(expense); | ||
|
|
||
| return res.status(201).json(expense); |
There was a problem hiding this comment.
This endpoint validates userId but is missing validation for other required fields (spentAt, title, amount, category). According to the Swagger API documentation, these are required parameters. Consider adding validation: if (!spentAt) { return res.status(400).json({ error: 'spentAt is required' }); } and similar checks for title, amount, and category.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your expense tracking application is complete and fully functional. All 10 endpoints (5 users, 5 expenses) are properly implemented with correct CRUD operations, in-memory storage, and appropriate error handling. The POST /expenses endpoint now correctly validates spentAt, title, amount, and category fields, returning 400 for missing required parameters. Great work implementing 404 handling for non-existent entities and ensuring the server exports correctly without calling app.listen(). Your solution meets all requirements and is ready to use.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
3 participants
No description provided.