feat: rooms/messages API + WS realtime, React client, proxy rewrite by djoseph-png · Pull Request #138 · mate-academy/node_chat

djoseph-png · 2025-10-18T16:33:51Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because a core requirement is missing on the client: room rename and delete are not implemented in the UI, which blocks “create / rename / join / delete rooms.” The server already exposes PATCH /rooms/:id and DELETE /rooms/:id, but the React app lacks handlers and controls for these actions.

Critical issues to fix:

README2.md: The line “Salas: criar / renomear / excluir / selecionar.” is inaccurate given the current UI. Either implement rename/delete on the client or update the README to match actual behavior.
src/App.tsx: Add handleRenameRoom(id, newName) and handleDeleteRoom(id) that call PATCH /api/rooms/:id and DELETE /api/rooms/:id, update local rooms state, and if the deleted room equals currentRoomId, switch to a fallback room or clear selection. Also handle 404 from GET /rooms/:id/messages by resetting selection or showing a user-visible error.
src/components/RoomSidebar.tsx: Extend props with onRenameRoom and onDeleteRoom, and add UI buttons for rename (prompt or modal) and delete (with confirmation). Invoke the provided callbacks and ensure UX handles deleting the currently selected room.

Nice-to-have (non-blocking):

server/index.js: Decide on cascade-deleting messages when a room is deleted or clearly document preserving history; current behavior leaves orphaned messages. Consider sending stored username to the server when loading from localStorage or on WS open for consistency.

You’ve implemented the core chat flow well (auth, history, messaging, realtime). Address the above items to fully meet the task and ensure consistent behavior between server, client, and README.

✨ 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 · 2025-10-18T16:38:14Z

+
+## Funcionalidades
+- Username enviado a `/auth/login` e salvo em `localStorage`.
+- Salas: criar / renomear / excluir / selecionar.


Line states: “Salas: criar / renomear / excluir / selecionar.” The server provides PATCH and DELETE for rooms, but the frontend UI/components (e.g. RoomSidebar and App) do not include rename or delete actions — update the README or add client-side rename/delete implementations that call the server endpoints and handle edge cases (e.g., deleting the currently selected room). See server routes and client usage for context: server index.js implements PATCH/DELETE and messages/history endpoints ; client code for login, creating/selecting rooms is in App.tsx and RoomSidebar ; task requirements: implement create/rename/join/delete rooms .

mateacademy-ai-mentor · 2025-10-18T16:38:14Z

+
+// “DB” em memória
+const rooms = [{ id: 'general', name: 'General' }];
+const messages = [];


Messages are kept in the messages array even after a room is deleted. This will leave orphaned messages for deleted rooms; consider removing messages with the same roomId on room deletion or document that history is preserved separately.

mateacademy-ai-mentor · 2025-10-18T16:38:14Z

+app.delete('/rooms/:id', (req, res) => {
+  const idx = rooms.findIndex((r) => r.id === req.params.id);
+  if (idx === -1) return res.status(404).json({ error: 'room not found' });
+
+  rooms.splice(idx, 1);


Deleting a room only removes the room object from rooms. Because messages are not removed, subsequent GET /rooms/:id/messages will return 404 (room not found) even though messages remain in memory — this may be surprising for clients. Either cascade-delete messages here or provide a clear contract for expected behavior.

mateacademy-ai-mentor · 2025-10-18T16:38:14Z

+app.patch('/rooms/:id', (req, res) => {
+  const room = rooms.find((r) => r.id === req.params.id);
+  if (!room) return res.status(404).json({ error: 'room not found' });
+
+  const { name } = req.body || {};
+  if (!name) return res.status(400).json({ error: 'name required' });
+
+  room.name = name;


PATCH /rooms/:id updates room.name but does not change the room id. This is OK if your contract is that id is immutable, but if the client expects the id to reflect the new name you should either regenerate the id (and handle collisions) or document that ids stay constant.

mateacademy-ai-mentor · 2025-10-18T16:38:14Z

+  socket.on('message', (raw) => {
+    // caso o cliente envie algo pelo WS, apenas ecoamos
+    const text = raw.toString();
+    const payload = JSON.stringify({ type: 'echo', payload: text });


Incoming WebSocket messages are echoed back as type 'echo' without validation. If you plan to accept chat messages via WS, validate and process them the same way as POST /messages (ensure roomId, author, text and add time), otherwise document that WS is only used for broadcasting server-side events.

mateacademy-ai-mentor · 2025-10-18T16:38:14Z

+    const name = window.localStorage.getItem('node_chat_username') || '';
+    if (name) {
+      setUsername(name);


When a username is found in localStorage you set it locally (setUsername(name)) but you don't send it to the server. The requirements state the client must send the chosen username to the server at some point — consider POSTing /auth/login or sending the username over WS when reading it from localStorage to ensure the server sees it.

mateacademy-ai-mentor · 2025-10-18T16:38:15Z

+    fetch(`${API_BASE}/rooms/${currentRoomId}/messages`)
+      .then((r) => {
+        if (!r.ok) throw new Error('load messages failed');
+        return r.json();
+      })
+      .then((data: Message[]) => {
+        setAllMessages((prev) => {
+          const others = prev.filter((m) => m.roomId !== currentRoomId);
+          const seen = new Set<string>(data.map((m) => m.id));
+          const othersFiltered = others.filter((m) => !seen.has(m.id));
+          return [...othersFiltered, ...data];
+        });
+      })
+      .catch((e) => console.error(e))
+      .finally(() => setLoadingMessages(false));


Loading room history currently throws on non-ok responses and logs the error, but there is no user-visible handling for a 404 (room not found). If a room is deleted (server-side or by another client) and the user still has it selected, consider resetting currentRoomId to a fallback room or showing an error to the user when the fetch returns 404.

mateacademy-ai-mentor · 2025-10-18T16:38:15Z

+  // Conectar WebSocket (uma única conexão)
+  useEffect(() => {
+    if (wsRef.current) return;
+    const ws = new WebSocket(WS_URL);
+    wsRef.current = ws;


WebSocket connection is created and used to receive broadcast messages — good. If you want the server to be aware of the connected username, consider sending the username to the server on WS open (or include it in a WS 'identify' message) so server-side real-time mechanisms can associate sockets with usernames (optional depending on server design).

mateacademy-ai-mentor · 2025-10-18T16:38:15Z

+export default function RoomSidebar({
+  rooms,
+  currentRoomId,
+  onSelectRoom,
+  onCreateRoom,
+  username,
+}: {
+  rooms: Room[];
+  currentRoomId: string;
+  onSelectRoom: (id: string) => void;
+  onCreateRoom: (name: string) => void;
+  username: string;


The component props currently include rooms, currentRoomId, onSelectRoom and onCreateRoom, but there are no props for renaming or deleting rooms. Add onRenameRoom and onDeleteRoom callbacks (and types) so the parent can perform PATCH /rooms/:id and DELETE /rooms/:id and keep UI and server behavior in sync — this is required by the task (rename/delete).

mateacademy-ai-mentor · 2025-10-18T16:38:15Z

+          {rooms.map((r) => (
+            <button
+              key={r.id}
+              onClick={() => onSelectRoom(r.id)}
+              style={{
+                textAlign: 'left',
+                padding: '8px 10px',
+                borderRadius: 6,
+                border:
+                  r.id === currentRoomId
+                    ? '1px solid #3b82f6'
+                    : '1px solid #e5e7eb',
+                background: r.id === currentRoomId ? '#eff6ff' : '#fff',
+                cursor: 'pointer',
+              }}
+            >
+              {r.name}
+            </button>
+          ))}


The room list rendering (rooms.map) only shows the room name and a button to select (join). Add UI controls here (e.g. an edit/rename button that opens a prompt/modal and calls onRenameRoom(id, newName), and a delete button with confirmation that calls onDeleteRoom(id)). Also ensure you handle the case when the deleted room equals currentRoomId (switch to a fallback room or notify the client).

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: NCM]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: NCM]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: NCM]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: NCM]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

feat: rooms/messages API + WS realtime, React client, proxy rewrite

61324b1

mateacademy-ai-mentor suggested changes Oct 18, 2025

View reviewed changes

djoseph-png requested a review from mateacademy-ai-mentor October 18, 2025 16:51

mateacademy-ai-mentor reviewed Oct 18, 2025

View reviewed changes

vkryvytskyi approved these changes Oct 20, 2025

View reviewed changes

Conversation

djoseph-png commented Oct 18, 2025

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

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants