Implement a chat application#143
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on the server-side implementation! The code is well-structured with a clear separation of concerns. However, this requires changes because the project is incomplete and contains a critical bug that affects functionality.
The main issue is that the client-side implementation is missing, which is a core requirement of the task. Additionally, there's a recurring error in your controllers (message.controller.js, rooms.controller.js, and user.controller.js). You are using res.sendStatus(400).json(...), but sendStatus() sends the response immediately and cannot be chained with .json(). This will cause a runtime error. Please correct this to res.status(400).json(...) to properly send the status code with a JSON body.
Once you've added the client and fixed the error handling, your project will be in great shape!
✨ 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 { text, userId } = req.body; | ||
|
|
||
| if (!roomId || !userId || !text) { | ||
| return res.sendStatus(400).json({ message: 'Message is required' }); |
There was a problem hiding this comment.
Using sendStatus() followed by .json() will lead to an error because sendStatus() sends the response immediately. To send a JSON body with a status code, you should use res.status(400).json(...) instead.
| .sendStatus(400) | ||
| .json({ message: 'Missing required fields: title or userId' }); |
There was a problem hiding this comment.
The res.sendStatus(400) method sets the status and sends the response immediately. It does not return the response object, so you cannot chain .json() after it. To send a JSON body with a 400 status, you should use res.status(400).json(...) instead.
| const isNameExist = await User.findOne({ where: { name } }); | ||
|
|
||
| if (!name) { | ||
| return res.sendStatus(400).json({ message: 'User is required' }); |
There was a problem hiding this comment.
The res.sendStatus() method sends the response immediately, so the chained .json() call will not be executed. To send a JSON body along with the status code, you should use res.status(400).json(...) instead.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on the server-side implementation! You've successfully fixed the error handling issues from the previous review, and the overall architecture of the backend is solid. However, changes are needed because the client-side of the application is still missing, which is a core requirement of the task. A functional chat application requires both a client and a server. Please add the client-side implementation that interacts with your existing API to complete the project.
✨ 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
2 participants
No description provided.