Solution by oberezhnay · Pull Request #169 · mate-academy/node_chat

oberezhnay · 2026-01-30T20:11:10Z

Chat (with Node.js)

mateacademy-ai-mentor

Review Summary

This is a great start, but changes are needed because several critical bugs prevent the application's core features from working correctly. Please address the following blockers:

Joining Rooms & Message History: The server crashes when a user joins a room due to an undefined userName variable in server/src/services/room.service.js. Additionally, the message history isn't being loaded because the query in server/src/socket/events.js needs to include the Message model to fetch past messages.
Sending Messages: Message creation fails in server/src/socket/events.js. The code uses socket.userId, which is never assigned, and passes an incorrect property (author instead of the expected authorName) to the service.
Missing Timestamps: A key requirement is not fully met. The client UI in client/main.js renders the author and text but is missing the message's timestamp.

Once these issues are resolved, the application should be fully functional. 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-01-30T20:13:12Z

client/main.js

+    authorEl.textContent = message.authorName;
+    textEl.textContent = message.text;


The task requires all messages to display an author, time, and text. Currently, only the author and text are being rendered. You should also display the message's timestamp (e.g., from message.createdAt).

mateacademy-ai-mentor · 2026-01-30T20:13:12Z

client/main.js

+
+  socket.emit('message_send', {
+    text,
+    roomId: state.activeRoom.id,


It's good that you've implemented the sendMessageHandler. However, there's a potential issue here. If a user hasn't joined a room yet, state.activeRoom will be null, and trying to access state.activeRoom.id will cause a runtime error. It's best to add a check to ensure state.activeRoom exists before sending a message.

mateacademy-ai-mentor · 2026-01-30T20:13:12Z

client/main.js

+
+socket.on('message_new', message => {
+  state.messages.push(message);
+  renderMessages();


Calling renderMessages() on every new message clears the entire chat and then re-renders all messages from scratch. This can become slow and inefficient if a room has many messages. A better approach would be to create a new function that just renders a single message and appends it to the messageList.

mateacademy-ai-mentor · 2026-01-30T20:13:12Z

server/src/socket/events.js

+
+        socket.join(roomId);
+
+        socket.emit('message_history', room.messages);


According to the task requirements, a new user joining a room should see all previous messages. However, room.messages will likely be undefined here because the roomService.joinRoom function doesn't load the associated messages. You'll need to update the query in your service to include the Message model to fetch the message history.

mateacademy-ai-mentor · 2026-01-30T20:13:12Z

server/src/socket/events.js

+
+        const message = await messageService.createMessage({
+          text,
+          author: socket.userName,


The messageService.createMessage function expects the author's name under the property authorName, but here you're passing it as author. This mismatch will prevent the author's name from being saved correctly. Please ensure the property names match what the service layer expects.

mateacademy-ai-mentor · 2026-01-30T20:13:12Z

server/src/socket/events.js

+        const message = await messageService.createMessage({
+          text,
+          author: socket.userName,
+          userId: socket.userId,


The socket.userId property is used here, but it's never assigned a value anywhere in the code. When a user joins, you should find or create a user record in the database and then store their ID on the socket object for later use, like when creating messages.

mateacademy-ai-mentor · 2026-01-30T20:13:12Z

server/src/services/room.service.js

+  }
+
+  await Room.update({ name: newName }, { where: { id }});
+  const room = Room.findByPk(id)


Room.findByPk(id) is an asynchronous operation and returns a Promise. You need to use await here to get the actual room object. Without await, the room variable will be a Promise, and the if (!room) check on the next line will not work as intended.

mateacademy-ai-mentor · 2026-01-30T20:13:12Z

server/src/services/room.service.js

+};
+
+const joinRoom = async ({ roomId }) => {
+  if (!roomId || !userName) {


The variable userName is used here but it is not defined within the function's scope. The function only receives roomId from its parameters. This will cause a ReferenceError when the function is called.

mateacademy-ai-mentor

Review Summary

Good work on the overall structure of the application. However, several critical bugs are preventing the core features—joining rooms, sending messages, and viewing message history—from working correctly. Changes are needed to address these blockers.

On the server-side:

In server/src/socket/events.js, the call to userService.findOrCreateUser is async but is not being awaited. This results in socket.userId being undefined.
In the same file, the call to messageService.createMessage is missing the required authorName property, which will cause message creation to fail.
In server/src/services/room.service.js, the Message and User models are used in a query but have not been imported, which will crash the server when a user tries to join a room.

On the client-side in client/main.js:

The renderOneMessage function uses a variable messageBlock that is never declared.
The timestamp element (dateEl) is created but is never appended to the message block, so it doesn't appear in the UI.

Addressing these issues should make the application fully functional. You're very close!

✨ 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-01-31T16:08:04Z

client/main.js

+  const dateEl = document.createElement('span');
+  authorEl.textContent = message.authorName;
+  textEl.textContent = message.text;
+  messageBlock.className = 'message';


The variable messageBlock is used here and in the following lines, but it has not been declared anywhere in this function's scope. You need to create this element, for example: const messageBlock = document.createElement('div');.

mateacademy-ai-mentor · 2026-01-31T16:08:04Z

client/main.js

+  dateEl.textContent = new Date(message.createdAt.toLocaleTimeString(uk-UA, {
+    hour: '2-digit',
+    minute: '2-digit',
+  }));


The logic for formatting the date is incorrect. message.createdAt should be a date string that new Date() can parse. toLocaleTimeString() returns a formatted time string (e.g., "10:30:55"). Wrapping this result in new Date() will likely lead to an "Invalid Date" error. The correct approach is to parse the original date first and then format it: new Date(message.createdAt).toLocaleTimeString(...).

mateacademy-ai-mentor · 2026-01-31T16:08:04Z

client/main.js

+  messageBlock.appendChild(authorEl);
+  messageBlock.appendChild(textEl);


The dateEl element containing the message timestamp is created but is never appended to the messageBlock. This means the timestamp will not be displayed in the chat, which is a core requirement. You need to append it, similar to how authorEl and textEl are appended.

mateacademy-ai-mentor · 2026-01-31T16:08:04Z

server/src/services/room.service.js

+        model: Message,
+        include: [User],


The Message and User models are used here to fetch the message history, but they have not been imported into this file. This will cause a ReferenceError when this function is executed. You need to import them at the top of the file, similar to how the Room model is imported.

mateacademy-ai-mentor · 2026-01-31T16:08:04Z

server/src/socket/events.js

+
+    socket.on('room_join', async ({ roomId, userName }) => {
+      try {
+        const user = userService.findOrCreateUser(userName);


The findOrCreateUser function is async, but it's not being awaited here. This means user will be a promise, not the user object. As a result, socket.userId and socket.userName will be undefined on the following lines, causing issues later when sending messages.

mateacademy-ai-mentor · 2026-01-31T16:08:04Z

server/src/socket/events.js

+        const message = await messageService.createMessage({
+          text,
+          // authorName: socket.userName,
+          userId: socket.userId,
+          roomId: socket.roomId,
+        })


The call to messageService.createMessage is missing the authorName property. According to message.service.js, this field is required and the service will throw an error without it. You should pass socket.userName here.

mateacademy-ai-mentor

Review Summary

Fantastic work addressing all the previous issues! The application is now fully functional, and it's great to see all the core features working correctly. I'm approving your submission to keep you moving forward, but there are a couple of important points to review for your next steps.

The main issue is a build failure caused by an incorrect path in your package.json format script. To fix this, you'll need to update it to point to your client and server/src directories like this:

"format": "prettier --ignore-path .prettierignore --write './client/**/*.js' './server/src/**/*.js'"

I also noticed two small areas for improvement:

In client/main.js, when sending a message, you can remove userId and roomId from the payload, as the server already has this information.
In server/src/socket/events.js, uncommenting the line that sends the room list on connection will improve the initial experience for new users.

Excellent progress overall, keep up the 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-02-01T19:03:04Z

client/main.js

+    userId: socket.userId,
+    roomId: socket.roomId,


When sending a message, you're trying to access socket.userId and socket.roomId. These properties are set on the server-side socket object for a specific connection, but they don't exist on the client-side socket object. Here, they will resolve to undefined.

Fortunately, your server-side code for the message_send event only uses the text from this payload and gets the user and room info from its own socket instance, which is why this still works. For correctness, you should only send the data the server actually needs from the client, which in this case is just the text.

mateacademy-ai-mentor · 2026-02-01T19:03:04Z

server/src/socket/events.js

+  io.on('connection', async (socket) => {
+    console.log('SOCKET CONNECTED', socket.id);
+
+    // socket.emit('room_list',  await roomService.getAllRooms());


When a new user connects, they don't receive the list of existing rooms until another user creates, renames, or deletes a room. Uncommenting this line will ensure that every new connection immediately receives the current list of rooms, improving the user experience.

solution

1eb4806

mateacademy-ai-mentor suggested changes Jan 30, 2026

View reviewed changes

add fixes after review

f498408

oberezhnay requested a review from mateacademy-ai-mentor January 31, 2026 16:05

mateacademy-ai-mentor suggested changes Jan 31, 2026

View reviewed changes

add fixes

0533371

oberezhnay requested a review from mateacademy-ai-mentor February 1, 2026 19:01

mateacademy-ai-mentor approved these changes Feb 1, 2026

View reviewed changes

fix format in packagejson , fis statemessage

527d470

oberezhnay requested a review from mateacademy-ai-mentor February 1, 2026 19:33

Fix styles, delete ordering

7d9d3af

		authorEl.textContent = message.authorName;
		textEl.textContent = message.text;


		socket.join(roomId);

		socket.emit('message_history', room.messages);

		messageBlock.appendChild(authorEl);
		messageBlock.appendChild(textEl);

Conversation

oberezhnay commented Jan 30, 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!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants