Skip to content

Solution#171

Open
vilich-mykhailo wants to merge 2 commits intomate-academy:masterfrom
vilich-mykhailo:develop
Open

Solution#171
vilich-mykhailo wants to merge 2 commits intomate-academy:masterfrom
vilich-mykhailo:develop

Conversation

@vilich-mykhailo
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Excellent work on building this full-stack chat application! Your solution is approved as it successfully implements all the required features, from user authentication and room management to real-time messaging with WebSockets. The code is well-structured and demonstrates a solid understanding of both frontend and backend development.

While not required for approval, here are a few suggestions for future improvement:

  • Client-Side Validation: In MessageForm.jsx and RoomsList.tsx, consider adding a check like if (text.trim()) before submitting forms. This prevents sending empty messages or creating rooms with empty names, improving the user experience.
  • Typo: There's a small typo in frontend/src/components/MessageList.jsx. The function normilizeTime should be normalizeTime.
  • Redundancy: In src/index.js, the if (room) check in the /room-update endpoint is redundant and can be removed to make the code slightly cleaner.

These are minor points in an otherwise outstanding submission. Keep up the great work!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

onSubmit={async (event) => {
event.preventDefault();

await sendMessage(userName, text, roomId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The form currently allows sending empty or whitespace-only messages. It's a good practice to add a check here to ensure the text has content before calling sendMessage. For example, you could check if (text.trim()).

@@ -0,0 +1,19 @@
export const MessageList = ({ messages }) => {
const normilizeTime = (date) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a small typo in the function name. It should be normalizeTime instead of normilizeTime. Correct spelling improves code readability.

const createRoom = async (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault();

await roomService.create(createRoomName, userName!);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good practice to prevent the user from submitting a form with an empty room name. While the server has validation, adding a client-side check here (e.g., if (!createRoomName.trim()) return;) would provide faster feedback and avoid an unnecessary API call.

return res.status(404).send({ message: 'Room not found' });
}

if (room) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if (room) check is redundant. The code on lines 73-74 already handles the case where room is not found by returning early. If the execution reaches this line, room is guaranteed to exist, so you can safely remove this conditional check and just leave the assignment room.name = name;.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants