implement chat with rooms and online users by sashkatmshchk-pixel · Pull Request #184 · mate-academy/node_chat

sashkatmshchk-pixel · 2026-03-11T17:03:29Z

Implemented chat with:

rooms
message history
username storage
online users list
enter key sending

mateacademy-ai-mentor

Review Summary

You've built a solid foundation for the chat application, and the features for creating, joining rooms, and handling message history are well-implemented.

However, I am requesting changes because two key requirements from the task description are missing. The server-side implementation in src/index.js needs to include the functionality for both renaming and deleting chat rooms. Please add the necessary event handlers and logic to support these actions to fully complete the task.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-03-11T17:04:36Z

src/index.js

+
+app.use(express.static(path.join(__dirname, '../public')));
+
+io.on('connection', (socket) => {


According to the task description, you need to implement functionality for creating, renaming, joining, and deleting rooms. While handlers for creating and joining rooms are present, the implementation for renaming and deleting rooms is missing.

mateacademy-ai-mentor

Review Summary

Great job implementing the renameRoom and deleteRoom features! This requires a few changes because there are some critical issues that leave users in a broken state.

When a room is renamed or deleted, users inside are stuck because their currentRoom becomes invalid. You'll need to handle these users, for example, by moving them to a default room or notifying their client of the name change.
When a user switches rooms, the user lists for the old and new rooms aren't updated. The user should be removed from the old room's list and added to the new one.
User list updates are sent globally with io.emit() instead of to specific rooms with io.to(roomName).emit(). This sends incorrect data to users in other rooms and needs to be fixed on lines 40, 83, and 103.

Addressing these points will make the chat application much more stable. You're on the right track!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-03-11T17:16:22Z

src/index.js

+
+  socket.emit('rooms', Object.keys(rooms));
+  socket.emit('history', rooms[currentRoom].messages);
+  io.emit('users', rooms[currentRoom].users);


Using io.emit() broadcasts the user list to every single client connected to the server, regardless of which room they are in. This means users in one room will incorrectly receive user list updates for another. You should use io.to(roomName).emit(...) to send updates only to clients within a specific room. This issue is present on lines 40, 83, and 103 as well.

mateacademy-ai-mentor · 2026-03-11T17:16:22Z

src/index.js

+  socket.on('renameRoom', ({ oldName, newName }) => {
+    if (!rooms[oldName] || rooms[newName]) {
+      return;
+    }
+
+    rooms[newName] = rooms[oldName];
+    delete rooms[oldName];
+
+    io.emit('rooms', Object.keys(rooms));
+  });


Renaming a room while users are inside it leaves them in an unusable state. Their currentRoom variable on the server becomes invalid because it still holds the oldName. Any subsequent action from that user, like sending a message, will cause an error because rooms[oldName] no longer exists. You need to handle these users, for example, by notifying their clients of the change so they can automatically rejoin the room under its new name.

mateacademy-ai-mentor · 2026-03-11T17:16:22Z

src/index.js

+  socket.on('deleteRoom', (roomName) => {
+    if (roomName === 'general') {
+      return;
+    }
+
+    if (rooms[roomName]) {
+      delete rooms[roomName];
+      io.emit('rooms', Object.keys(rooms));
+    }
+  });


Similar to the renameRoom issue, deleting a room leaves clients inside it in a broken state. Their currentRoom variable will point to a room that no longer exists, causing errors on their next action. You should handle this case by moving the affected users to a default room, like 'general', and notifying them.

mateacademy-ai-mentor · 2026-03-11T17:16:22Z

src/index.js

+  socket.on('joinRoom', (roomName) => {
+    socket.leave(currentRoom);
+
+    currentRoom = roomName;
+    socket.join(currentRoom);
+
+    socket.emit('history', rooms[currentRoom].messages);
+    io.emit('users', rooms[currentRoom].users);
+  });


When a user switches rooms, the server should update the user lists for both the old and the new room. Currently, the user is not removed from the old room's list or added to the new room's list. You'll need to add logic here to:

Remove the user from rooms[currentRoom].users.

Notify users in the old room about the change.

Add the user to rooms[roomName].users.

Notify users in the new room about their arrival.

mateacademy-ai-mentor

Review Summary

I am approving your solution as an educational override to help you progress, as there are a few important state management issues to address. You've done a great job fixing the user list updates when users join or leave a room!

However, there are a couple of key areas to focus on:

When a user changes their name, the old username is not removed from the room's user list, creating duplicates.
When a room is renamed or deleted, other users in that room are not notified of the change. This leaves their clients in a broken state, still pointing to the old room. You'll need to emit an event to all affected clients to tell them to update their state.

Fixing these state consistency bugs will make your application much more robust. You're doing great work!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2026-03-11T17:37:05Z

src/index.js

+    username = name;
+
+    if (!rooms[currentRoom].users.includes(username)) {
+      rooms[currentRoom].users.push(username);
+    }


When a user changes their name, the new name is added to the user list, but the old one isn't removed. You should remove the previous username from the users array before adding the new one.

mateacademy-ai-mentor · 2026-03-11T17:37:05Z

src/index.js

+        if (s === socket) {
+          currentRoom = newName;
+        }


This logic updates currentRoom only for the user who initiated the rename. Other users in the room are moved to the new socket.io room, but their currentRoom variable on the server still points to the old room name. This leaves their client in a broken state. You need to notify all clients in the affected room about the change so they can update their state accordingly.

mateacademy-ai-mentor · 2026-03-11T17:37:05Z

src/index.js

+    io.sockets.sockets.forEach((s) => {
+      if (s.rooms.has(roomName)) {
+        s.leave(roomName);
+        s.join('general');
+      }
+    });


This loop correctly moves users to the 'general' socket.io room, but there are two issues here:

The users from the deleted room are not added to the rooms.general.users array.

The currentRoom variable for the moved users is not updated, which leaves their clients in an invalid state.

You'll need to update the 'general' room's user list and also notify the affected clients that they have been moved.

implement chat with rooms and online users

547cde1

mateacademy-ai-mentor suggested changes Mar 11, 2026

View reviewed changes

add rename and delete room functionality

7ac80bf

sashkatmshchk-pixel requested a review from mateacademy-ai-mentor March 11, 2026 17:14

mateacademy-ai-mentor suggested changes Mar 11, 2026

View reviewed changes

fix room rename/delete logic and user list updates

40c7711

sashkatmshchk-pixel requested a review from mateacademy-ai-mentor March 11, 2026 17:35

mateacademy-ai-mentor approved these changes Mar 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement chat with rooms and online users#184

implement chat with rooms and online users#184
sashkatmshchk-pixel wants to merge 3 commits intomate-academy:masterfrom
sashkatmshchk-pixel:develop

sashkatmshchk-pixel commented Mar 11, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Mar 11, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Mar 11, 2026

Uh oh!

mateacademy-ai-mentor Mar 11, 2026

Uh oh!

mateacademy-ai-mentor Mar 11, 2026

Uh oh!

mateacademy-ai-mentor Mar 11, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Mar 11, 2026

Uh oh!

mateacademy-ai-mentor Mar 11, 2026

Uh oh!

mateacademy-ai-mentor Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		app.use(express.static(path.join(__dirname, '../public')));

		io.on('connection', (socket) => {

Conversation

sashkatmshchk-pixel commented Mar 11, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants