solution by Ga1dar · Pull Request #170 · mate-academy/node_chat

Ga1dar · 2026-02-01T13:47:02Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because several core requirements of the chat assignment are not yet implemented or wired through, which will block full functionality. The client currently does not handle username entry or persistence in localStorage, nor does it send the username to the server (see client/src/App.tsx and client/src/soket.ts), which violates the checklist items about setting and storing the username. Messages are sent as plain text only (socket.emit('message:send', text) in App.tsx), while the task requires each message to reliably have author, time, and text; you need to standardize the message shape across App.tsx and api.ts and ensure author/time are attached consistently. Room management (create, rename, join, delete) is missing from the UI and client logic even though the server supports it, and the client subscribes to room:history/message:new without ever emitting a room join or room management events, so users cannot actually use rooms as required. There is also an unimplemented data loader (client/src/dataLoader/WebSocket.tsx) that declares an onData prop but doesn’t accept or call it, and the socket vs HTTP (api.ts) communication paths are inconsistent, which can confuse how messages and history are loaded.

To move this to completion, please: 1) add a username flow (prompt or form), save it to localStorage, and send it to the server (via socket auth or a username:set event); 2) ensure outbound messages include or result in author and time fields that match the Message type; 3) implement room CRUD and joining on the client, wiring appropriate socket events so a newly joined room receives its history; and 4) either fully implement or remove the unused WebSocket/SSE loader and choose a single, clear messaging approach (sockets or HTTP) for sending/receiving messages. Minor issues like using message.time as a React key, the import path with .ts in MessageList.tsx, and the missing tsconfig.app.json reference can be fixed after the main flows work, but they are secondary to the blockers above. You’ve laid a solid foundation with the Message type, MessageList rendering, and a good server implementation; once the username and room features are wired through the client, this will align well with the assignment requirements.

✨ 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-02-01T13:53:28Z

client/src/App.tsx

+import { socket } from './soket';
+import type { Message } from './types/message';
+import { MessageList } from './components/MessageList';
+import { MessageForm } from './components/MessageForm';


You import MessageForm here, but I couldn't find ./components/MessageForm in the provided files. Ensure the component exists and that it provides the UI/logic to obtain username (or forwards message data including author/time). The task requires a username entry and saving it to localStorage; that responsibility must be implemented (either in MessageForm or a higher-level component).

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/src/App.tsx

+      <div className="box">
+
+        <div className="block">
+          <MessageForm onSend={(text) => socket.emit('message:send', text)} />


The send handler only emits the message text: socket.emit('message:send', text). The assignment requires messages to have author, time, and text. Either include the author/time here (read username from localStorage and attach a timestamp) or clearly document that the server will attach them. Currently sending only text will not satisfy the "author, time and text" requirement.

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/src/App.tsx

+    socket.on('room:history', onHistory);
+    socket.on('message:new', onNew);


You subscribe to room:history and message:new, which indicates per-room behavior, but this file contains no room state or any calls to create/join/rename/delete rooms. The checklist requires room creation/rename/join/delete and that a new user sees previous messages in the room. Add UI and socket events to manage rooms and emit a room:join (or similar) so the server can send room:history when a user joins a room.

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/src/App.tsx

@@ -0,0 +1,38 @@
+import { useEffect, useState } from 'react';
+import { socket } from './soket';


The project contains both socket usage and an axios-based API (api.ts). Decide on a single communication approach (sockets or HTTP + SSE) or clearly separate responsibilities. Right now App uses sockets while api.ts exposes HTTP functions; this inconsistency can cause confusion when implementing author/time/room persistence. Align client and server APIs so message shape and room management are consistent.

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/src/dataLoader/WebSocket.tsx

+  onData: (data: Message[]) => void;
+}
+
+export const WebSocketLoader: React.FC<Props> = () => {


The component type declares Props including onData, but the function signature does not accept props. Accept the props (for example ({ onData }) => { ... }) so you can call onData when messages/history arrive.

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/src/api.ts

+export function sendMessage(text: string, sender: string) {
+  return axios.post('/messages', { text, sender });


The POST payload uses sender while the project's Message interface expects author (and time is required). This mismatch means messages saved/returned by the server may not match the Message shape; either send author here or ensure the server converts sender to author and adds time before returning/storing messages. See Message type and App usage for context.

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/src/api.ts

@@ -0,0 +1,14 @@
+import axios from 'axios';


The project uses socket.io in App.tsx to emit message events (socket.emit('message:send', text)), but this module implements an HTTP sendMessage function. That inconsistency can lead to unused code or duplicated messaging flows — choose one approach (sockets or HTTP) and make the client modules consistent.

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/src/socket.ts

@@ -0,0 +1,2 @@
+import { io } from 'socket.io-client';
+export const socket = io('http://localhost:3000', { transports: ['websocket'] });


The socket is created without any auth or username. The task requires the user to type a username and send it to the server (and save it in localStorage). Consider reading the username from localStorage (or obtaining it before connection) and passing it with io('http://..', { auth: { username } }) or emitting an initial user:set/username:set event right after connect so the server can associate the socket with the username.

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/src/socket.ts

@@ -0,0 +1,2 @@
+import { io } from 'socket.io-client';
+export const socket = io('http://localhost:3000', { transports: ['websocket'] });


Hardcoded URL is fine for local dev but ensure it matches your server address and CORS/socket config. Also consider delaying socket creation until after username is available (so the server immediately receives username in auth).

mateacademy-ai-mentor · 2026-02-01T13:53:28Z

client/tsconfig.json

+{
+  "files": [],
+  "references": [
+    { "path": "./tsconfig.app.json" },


This file references ./tsconfig.app.json but that file was not found in the provided files. Missing a referenced config will cause TypeScript project references to fail during build or editor type-checking. Add tsconfig.app.json or update the references array to the correct paths used in the project.

mateacademy-ai-mentor

Review Summary

Excellent work on this project! I am approving your solution as you have successfully implemented all the core requirements. The username flow, localStorage persistence, and full room management features are all working correctly and are well-implemented on both the client and server.

As a suggestion for future improvement, you could clean up some now-unused code to streamline the project. For example, since all communication is handled via WebSockets, the HTTP-related files and code like client/src/api.ts, the client/src/dataLoader/ directory, and the HTTP endpoints in src/index.js could be removed. This is not a required change, as the application's functionality is solid. Great job!

✨ 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-01T14:27:04Z

client/src/api.ts

+export async function getRooms(): Promise<Room[]> {
+  const res = await axios.get('/rooms');
+  return res.data as Room[];
+}
+
+export async function getRoomMessages(roomId: string): Promise<Message[]> {
+  const res = await axios.get(`/rooms/${roomId}/messages`);
+  return res.data as Message[];
+}
+
+export async function postRoomMessage(roomId: string, text: string, author: string) {
+  const res = await axios.post(`/rooms/${roomId}/messages`, { text, author });
+  return res.data as Message;
+}


These HTTP API functions seem to be unused in the application. All the client-server communication is handled via WebSockets in App.tsx, which is a good approach for a chat app. To align with the decision to use WebSockets and to remove dead code, this file could be deleted.

mateacademy-ai-mentor · 2026-02-01T14:27:04Z

client/src/dataLoader/WebSocket.tsx

+export function WebSocketLoader({ onData }: Props) {
+  useEffect(() => {
+    const onHistory = (history: Message[]) => onData(history);
+    socket.on('room:history', onHistory);
+   return () => {
+      socket.off('room:history', onHistory);
+    };
+  }, [onData]);
+
+  return null;
+}


This component and its logic are not used anywhere in the application. The responsibility of handling WebSocket events, including room:history, has been correctly consolidated within the main App.tsx component. As this file is now redundant, it should be removed to eliminate dead code from the project.

mateacademy-ai-mentor · 2026-02-01T14:27:04Z

src/index.js

+app.get('/rooms', (req, res) => res.json(roomsList()));
+
+app.get('/rooms/:roomId/messages', (req, res) => {
+  const { roomId } = req.params;
+
+  if (!rooms.has(roomId)) {
+    return res.status(404).json({ error: 'Room not found' });
+  }
+
+  return res.json(rooms.get(roomId).messages);
+});
+
+app.post('/rooms/:roomId/messages', (req, res) => {
+  const { roomId } = req.params;
+  const { text, author } = req.body || {};
+
+  if (!rooms.has(roomId)) {
+    return res.status(404).json({ error: 'Room not found' });
+  }
+
+  if (typeof text !== 'string' || !text.trim()) {
+    return res.status(400).json({ error: 'Invalid text' });
+  }
+
+  const safeAuthor =
+    typeof author === 'string' && author.trim() ? author.trim() : 'Anonymous';
+
+  const message = makeMessage(safeAuthor, text.trim());
+
+  rooms.get(roomId).messages.push(message);
+  io.to(roomId).emit('message:new', message);
+
+  return res.status(201).json(message);
+});


These HTTP API endpoints are well-implemented, but they appear to be unused by the client, which relies exclusively on WebSockets for communication. To maintain a single, consistent communication protocol and remove dead code, you could consider removing these endpoints, similar to the suggestion for the client-side api.ts file.

mateacademy-ai-mentor · 2026-02-01T14:27:04Z

src/index.js

+    io.to(roomId).emit('room:renamed', {
+      roomId,
+      roomName: rooms.get(roomId).name,
+    });


You're emitting a room:renamed event here, which is a good practice for specific state updates. However, the current client implementation does not have a listener for this event and instead relies on the globally emitted room:list event to update the UI. To keep the client and server communication in sync, you could either add a handler on the client or remove this unused event from the server.

solution

afd0b63

mateacademy-ai-mentor suggested changes Feb 1, 2026

View reviewed changes

solution

dcc7bc8

Ga1dar requested a review from mateacademy-ai-mentor February 1, 2026 14:24

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

View reviewed changes

		socket.on('room:history', onHistory);
		socket.on('message:new', onNew);

		@@ -0,0 +1,38 @@
		import { useEffect, useState } from 'react';
		import { socket } from './soket';

		export function sendMessage(text: string, sender: string) {
		return axios.post('/messages', { text, sender });

		@@ -0,0 +1,2 @@
		import { io } from 'socket.io-client';
		export const socket = io('http://localhost:3000', { transports: ['websocket'] });

Conversation

Ga1dar commented Feb 1, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants