Solution#199
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because it implements all of the core chat requirements: a Node.js server with WebSockets and REST, room create/rename/join/delete, message persistence with author and timestamp, history loading for new users, and a React client that wires these flows together. Automated tests are currently blocked by ESLint parsing errors, but these come from the lint tooling not understanding some of your TypeScript/decorator syntax rather than from clearly broken runtime behavior; you can address this by avoiding decorators and TS-only import forms in linted files or by updating the ESLint/TS setup if that is allowed in your course. For better runtime robustness, in src/index.ts and src/controllers/room.controller.ts you should coerce WebSocket data to a string before JSON.parse (e.g. const raw = typeof data === 'string' ? data : data.toString();) and normalize roomId with const rId = Number(payload.roomId); so your in‑memory rooms map never mixes string and number keys. In the client build config (client/vite.config.ts and related tsconfigs), double‑check that @rolldown/plugin-babel is really the intended package and that reactCompilerPreset is exported by @vitejs/plugin-react; mismatches here can break npm run dev or builds even though the app logic itself is correct. On the UI side, add a .deleted_overlay rule to client/src/components/ChatWindow/ChatWindow.module.scss and consider scoping the global button selector so styles from the chat window don’t leak to unrelated buttons. In your Sequelize join model (UserRoom.model.ts), adding a composite unique index on (userId, roomId) and ensuring cascade behavior is enforced at the association/migration level will prevent duplicate memberships and make deletions safer. Overall, your implementation is functionally solid and meets the task goals; the remaining points are good opportunities to harden the app and polish developer and user experience.
✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.
Footnotes
| import globals from 'globals' | ||
| import reactHooks from 'eslint-plugin-react-hooks' | ||
| import reactRefresh from 'eslint-plugin-react-refresh' | ||
| import tseslint from 'typescript-eslint' |
There was a problem hiding this comment.
Importing from 'typescript-eslint' may be incorrect. The official plugin is usually published as '@typescript-eslint/eslint-plugin' (or you may need to import from the monorepo package that provides flat-config exports). Verify the installed package name and that it exports the configs object used below; otherwise import the correct package or adjust usage.
| extends: [ | ||
| js.configs.recommended, | ||
| tseslint.configs.recommended, | ||
| reactHooks.configs.flat.recommended, |
There was a problem hiding this comment.
Not all versions of eslint-plugin-react-hooks expose a configs.flat.recommended. Verify that the installed plugin version supports flat configs and this path; if it doesn't, use the appropriate config (for example reactHooks.configs.recommended) or update the plugin to a version that provides flat-config exports.
| @@ -0,0 +1,28 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.app.tsbuildinfo", | |||
There was a problem hiding this comment.
The import target '@rolldown/plugin-babel' on this line looks like a typo. The common package name is '@rollup/plugin-babel' (two 'l's) or another Babel plugin — verify the intended package and update the import so the plugin resolves at build time.
| @@ -0,0 +1,28 @@ | |||
| { | |||
| "compilerOptions": { | |||
There was a problem hiding this comment.
Confirm that reactCompilerPreset is actually exported from '@vitejs/plugin-react' in the installed plugin version. If not, either remove this usage or import the correct export (or configure the Babel plugin differently). Using a non-existent named export will cause a runtime/build error.
| @@ -0,0 +1,26 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.node.tsbuildinfo", | |||
There was a problem hiding this comment.
Typo in the package name: @rolldown/plugin-babel will not resolve. Replace with the correct package (for example @rollup/plugin-babel) or install the intended plugin. This import will cause a module-not-found error at startup.
| @@ -0,0 +1,26 @@ | |||
| { | |||
| "compilerOptions": { | |||
There was a problem hiding this comment.
Confirm that reactCompilerPreset is exported by your installed @vitejs/plugin-react version. Older versions of the plugin may not export this symbol; if it's not available, either upgrade the plugin or use the supported API/options to enable the React compiler.
| "module": "ESNext", | ||
| "types": ["node"], | ||
| "skipLibCheck": true, | ||
|
|
There was a problem hiding this comment.
Using a Babel Rollup plugin in a Vite config is unusual because Vite uses esbuild by default. If you only need the React compiler preset, prefer configuring @vitejs/plugin-react directly (or ensure your Babel usage is required and correctly configured).
| @@ -0,0 +1,11 @@ | |||
| import { defineConfig } from 'vite' | |||
| import react, { reactCompilerPreset } from '@vitejs/plugin-react' | |||
| import babel from '@rolldown/plugin-babel' | |||
There was a problem hiding this comment.
The import @rolldown/plugin-babel looks suspicious — the common package is @rollup/plugin-babel (two 'l's) or a different Vite-compatible Babel integration. Verify the package name is correct and installed; if it was a typo replace it with the correct package or remove the plugin if unnecessary.
| export default defineConfig({ | ||
| plugins: [ | ||
| react(), | ||
| babel({ presets: [reactCompilerPreset()] }) |
There was a problem hiding this comment.
You're passing reactCompilerPreset() into the Babel plugin. Confirm that reactCompilerPreset is actually exported by @vitejs/plugin-react in your installed version; if not, either use a known Babel preset (e.g. @babel/preset-react) or remove this Babel setup since @vitejs/plugin-react already handles React compilation in most cases.
| @@ -0,0 +1,7 @@ | |||
| { | |||
| "files": [], | |||
There was a problem hiding this comment.
Verify that reactCompilerPreset is exported from @vitejs/plugin-react in your installed plugin version. Not all versions export reactCompilerPreset; if it's not present you should remove this usage or import the correct preset/export provided by your plugin version.
| @@ -0,0 +1,7 @@ | |||
| { | |||
| "files": [], | |||
| "references": [ | |||
There was a problem hiding this comment.
The import @rolldown/plugin-babel is likely incorrect or misspelled — commonly the Rollup plugin is @rollup/plugin-babel. If this package is not installed the build will fail; either install the correct plugin or remove this babel plugin if it's unnecessary (the React plugin often covers the needed transforms).
| console.log('✅ Database synced and connected.'); | ||
| } catch (err) { | ||
| console.error('❌ Database connection error:', err); | ||
| } |
There was a problem hiding this comment.
The rooms map is declared as Record<number, Set<ExtendedWebSocket>>, but because keys from JSON payloads can be strings you may end up mixing string/number keys. Either change the map to Record<string, Set<...>> or ensure you always coerce incoming room ids to numbers (preferred) so the runtime shape matches the TS type.
| ws.on('message', async (data: string) => { | ||
| try { | ||
| const message: WSMessage = JSON.parse(data); |
There was a problem hiding this comment.
The WebSocket 'message' handler types the incoming data as string, but the 'ws' library may deliver RawData (Buffer / ArrayBuffer). Calling JSON.parse directly on a Buffer can throw. Consider ensuring you parse a string (e.g., const text = typeof data === 'string' ? data : data.toString(); const message = JSON.parse(text);) and handle parse errors gracefully.
|
|
||
| switch (type) { | ||
| case MessageType.ROOM_JOIN: { | ||
| const rId = payload.roomId; |
There was a problem hiding this comment.
rId is taken directly from the payload and then used as a key in the rooms map. If payload.roomId is a string, you may end up creating string keys while other code expects numeric keys. Normalize the ID (e.g., const rId = Number(payload.roomId)) to avoid mismatches when adding/removing/looking up rooms.
| const { roomId, userId, text } = payload; | ||
|
|
||
| try { | ||
| // 1. Зберігаємо в базу даних | ||
| const newMessage = await Message.create({ | ||
| text, | ||
| userId, | ||
| roomId, | ||
| }); | ||
|
|
||
| // 2. Знаходимо автора, щоб відправити ім'я на фронтенд | ||
| const user = await User.findByPk(userId); | ||
|
|
||
| const clients = rooms[roomId]; |
There was a problem hiding this comment.
When retrieving clients you index rooms with roomId from the payload. To be robust against string/number differences, coerce/normalize the ID (e.g., const id = Number(roomId); const clients = rooms[id];). This prevents missing clients if the key types differ.
| const { id } = req.params; // Отримуємо id з URL | ||
| const { userId } = req.body; | ||
|
|
There was a problem hiding this comment.
The message event handler types data as string, but the ws library can deliver a Buffer (or other RawData). Convert the incoming data to a string before JSON.parse (e.g., const raw = typeof data === 'string' ? data : data.toString(); const message = JSON.parse(raw);) to avoid parse errors.
| } | ||
| }, | ||
|
|
||
| checkMembership: async (req: Request, res: Response) => { | ||
| const { id, userId } = req.params; |
There was a problem hiding this comment.
When handling ROOM_JOIN you assign const rId = payload.roomId; — payload values (coming from clients or URL params) may be strings. Normalize the id to a number before using it as an object key (e.g., const rId = Number(payload.roomId);) to avoid mismatches between numeric and string keys in the rooms map.
| create: async (req: Request, res: Response) => { | ||
| try { | ||
| const { name } = req.body; | ||
|
|
There was a problem hiding this comment.
CORS is configured with origin: process.env.CLIENT_URL. If CLIENT_URL is undefined during local development this will restrict cross-origin requests. Consider adding a safe default (or a clear dev-mode fallback) so the client can connect in common dev setups, e.g. origin: process.env.CLIENT_URL || 'http://localhost:5173' or allow all in development.
| @ForeignKey(() => User) | ||
| @Column({ type: DataType.INTEGER, onDelete: 'CASCADE' }) | ||
| declare userId: number; | ||
|
|
||
| @ForeignKey(() => Room) | ||
| @Column({ type: DataType.INTEGER, onDelete: 'CASCADE' }) | ||
| declare roomId: number; |
There was a problem hiding this comment.
You set onDelete: 'CASCADE' inside the column definition. In sequelize-typescript it's more reliable to specify cascade behavior on the association (or define the FK constraint explicitly) because @Column options may not create the FK cascade in some setups. Consider moving the cascade option to the association or ensure the DB migration/schema includes ON DELETE CASCADE so related rows are removed when a user or room is deleted.
| import { Room } from './Room.model'; | ||
| import { User } from './User.model'; | ||
|
|
||
| @Table({ tableName: 'user_rooms' }) |
There was a problem hiding this comment.
Add a composite unique constraint on (userId, roomId) to prevent duplicate membership rows. Without it, concurrent findOrCreate/inserts can create duplicates. You can add an index or constraint at the model level (for example using @Index({ unique: true, fields: ['userId', 'roomId'] }) on the class) or enforce it in your DB migrations.
| BelongsTo, | ||
| BelongsToMany, | ||
| } from 'sequelize-typescript'; | ||
| import { Message } from './Message.model'; |
There was a problem hiding this comment.
Consider adding a composite unique index on (userId, roomId) to prevent duplicate membership rows. You can add this via the @table decorator, e.g. @Table({ tableName: 'user_rooms', indexes: [{ unique: true, fields: ['userId', 'roomId'] }] }).
|
|
||
| @Table({ tableName: 'rooms' }) | ||
| export class Room extends Model { | ||
| @Column({ type: DataType.STRING, allowNull: false }) | ||
| declare name: string; | ||
|
|
There was a problem hiding this comment.
Placing onDelete: 'CASCADE' inside @Column options may not reliably create a DB-level foreign-key cascade depending on sequelize-typescript/version. To ensure cascade deletes and enable eager include usage, declare the associations explicitly (e.g. add @BelongsTo(() => User) declare user: User and @BelongsTo(() => Room) declare room: Room) or define the onDelete behavior in the association/migration so removals cascade as intended.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is one functional bug that directly affects a required feature, plus a couple of small but easy fixes.
Critical blocker (must fix):
- In
src/index.js(server WebSocket logic), the broadcasts forROOM_RENAMEDandROOM_DELETEDcurrently send payloads likepayload: { normRoomId, newName }andpayload: { normRoomId }, while the clientChatProviderhandlers expectdata.payload.roomId(e.g. they comparer.id === data.payload.roomIdand filter bydata.payload.roomId). This mismatch means room renames/deletions won’t propagate correctly to the client, breaking the rename/delete requirement. To fix this, change those broadcasts to useroomIdas the key, for example:payload: { roomId: normRoomId, newName }for rename andpayload: { roomId: normRoomId }for delete.
Non‑blocking but recommended improvements:
- In
client/src/components/ChatWindow/ChatWindow.tsx, you usestyles.deleted_overlaybutChatWindow.module.scsshas no.deleted_overlayrule; add a simple style (e.g. centered content, padding, background) so the deleted-room overlay looks intentional. - In the same
ChatWindow.tsxreturn JSX there is a stray;after the main conditional fragment () };); remove that extra semicolon to avoid lint/syntax noise. - In
src/controllers/room.controller.js(join,checkMembership,getMessages), you usereq.params.id/req.params.userIdas strings; it’s safer to coerce them to numbers (e.g.const roomId = Number(req.params.id);) before Sequelize queries so you don’t mix string/number IDs with the in‑memoryroomsmap.
You’ve done a solid job implementing all the required chat features end‑to‑end; once you align the WS payload key and make these small cleanups, your solution will fully meet the assignment’s expectations.
✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.
Footnotes
| @@ -0,0 +1,52 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
ChatWindow.tsx references styles.deleted_overlay but ChatWindow.module.scss does not define .deleted_overlay. Add a .deleted_overlay rule so the deleted-room UI is styled consistently.
| @@ -0,0 +1,52 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
In src/index.js (server) WS broadcasts for ROOM_RENAMED and ROOM_DELETED use payload keys named normRoomId while client ChatProvider expects payload.roomId. This mismatch prevents client updates; change broadcast payloads to use roomId: normRoomId (and keep newName as-is) so client handlers work.
| @@ -0,0 +1,52 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
In src/controllers/room.controller.js methods that use req.params.id (room id) or req.params.userId, consider coercing to Number before DB queries (e.g., const roomId = Number(req.params.id)) to avoid mixing string/number keys and ensure consistent comparisons with numeric DB fields.
| @@ -0,0 +1,7 @@ | |||
| .app_layout { | |||
There was a problem hiding this comment.
ChatWindow.tsx references styles.deleted_overlay but ChatWindow.module.scss does not define a .deleted_overlay rule. Add styling (e.g. centering, padding, background) to avoid missing class and ensure the deleted-room UI looks correct.
| @@ -0,0 +1,7 @@ | |||
| .app_layout { | |||
There was a problem hiding this comment.
Server broadcasts for ROOM_RENAMED and ROOM_DELETED use payload keys named normRoomId but client ChatProvider expects payload.roomId. This key mismatch prevents client from applying renames/deletions. Change server broadcast payloads to use { roomId: normRoomId, newName } and { roomId: normRoomId } respectively.
| @@ -0,0 +1,7 @@ | |||
| .app_layout { | |||
There was a problem hiding this comment.
In src/controllers/room.controller.js join and checkMembership use req.params.id (string) directly for numeric DB fields. Coerce route params to Number (e.g. const roomId = Number(req.params.id);) to avoid type inconsistencies when interacting with Sequelize and with the in-memory rooms map on the server.
| @@ -0,0 +1,7 @@ | |||
| .app_layout { | |||
There was a problem hiding this comment.
In ChatWindow.tsx return JSX there's an unexpected stray semicolon after the main conditional fragment () }; near the end). Ensure JSX closes without stray characters to avoid potential syntax/formatting issues.
| @@ -0,0 +1,113 @@ | |||
| .chat_window { | |||
There was a problem hiding this comment.
ChatWindow.tsx renders an element with className={styles.deleted_overlay} but ChatWindow.module.scss has no .deleted_overlay rule. Add styling (e.g. centered message and button) to avoid missing class and to ensure the deleted-room UI appears as intended.
| @@ -0,0 +1,113 @@ | |||
| .chat_window { | |||
There was a problem hiding this comment.
In src/index.js server WebSocket ROOM_RENAME and ROOM_DELETE broadcasts use payload keys named normRoomId (e.g. { normRoomId, newName } / { normRoomId }). Client ChatProvider expects payload.roomId when handling ROOM_RENAMED and ROOM_DELETED. This key mismatch prevents client-side room rename/delete updates. Change server broadcasts to use roomId: normRoomId as the payload key.
| @@ -0,0 +1,113 @@ | |||
| .chat_window { | |||
There was a problem hiding this comment.
In room.controller.js methods (join, checkMembership, getMessages) route params such as id and userId are used directly (strings). While Sequelize will often coerce them, explicitly convert to Number (e.g. const roomId = Number(req.params.id)) when interacting with numeric DB fields and when managing in-memory room maps to avoid type inconsistencies.
| @@ -0,0 +1,113 @@ | |||
| .chat_window { | |||
There was a problem hiding this comment.
Minor: In ChatWindow.tsx return markup there appears to be an extra semicolon after the JSX conditional block () };). Verify the JSX compiles cleanly and remove any stray semicolon if it causes a syntax issue.
| return ( | ||
| <> | ||
| {wasRoomDeleted ? ( | ||
| <div className={styles.deleted_overlay}> |
There was a problem hiding this comment.
ChatWindow.tsx uses styles.deleted_overlay (line 143) but ChatWindow.module.scss does not define .deleted_overlay. Add a CSS rule for .deleted_overlay to ensure the deleted-room overlay is displayed/styled correctly (centered message, padding, etc.).
| </> | ||
| )} | ||
| </div> | ||
| )}; |
There was a problem hiding this comment.
There is a stray semicolon after the JSX conditional at the end of the return: ) }; on line 237. That semicolon is unnecessary and may cause a syntax/formatting issue. Remove the extra ; so the JSX fragment closes cleanly.
| @@ -0,0 +1,240 @@ | |||
| import React, { useState, useEffect, useCallback } from 'react'; | |||
There was a problem hiding this comment.
In several places you use route params / IDs (e.g. POST to /api/rooms/${roomId}/join and GET /api/rooms/${roomId}/membership/${userId}) and send them to server. On the server side some WebSocket broadcasts use payload keys named normRoomId while the client expects payload.roomId (see ChatProvider handling). This naming mismatch will prevent client-side room rename/delete handling from working. Ensure server broadcasts use roomId as the payload key (e.g. { roomId: normRoomId, newName } and { roomId: normRoomId }).
| @@ -0,0 +1,41 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
In the server WS code you broadcast ROOM_RENAMED/ROOM_DELETED with payload keys named normRoomId (server-side). The client ChatProvider and ChatWindow expect payload.roomId. This key mismatch will prevent client-side room rename/delete updates. Change the server broadcast payloads to use roomId: normRoomId (and keep newName) so client handlers match.
| @@ -0,0 +1,41 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
room.controller.js (join/checkMembership/getMessages) reads req.params.id and req.params.userId directly (strings). Coerce params to numbers before DB queries (e.g., const roomId = Number(req.params.id)) to avoid mixing string/number keys and ensure consistent comparisons with numeric DB fields.
| @@ -0,0 +1 @@ | |||
| export * from './ChatWindow'; | |||
There was a problem hiding this comment.
Server/client payload mismatch: the server's ROOM_RENAMED and ROOM_DELETED broadcasts use a payload key named normRoomId, but client-side ChatProvider and ChatWindow expect payload.roomId. Ensure server broadcasts use roomId as the key (e.g. { roomId: normRoomId, newName } and { roomId: normRoomId }) so room rename/delete updates apply on the client.
| @@ -0,0 +1 @@ | |||
| export * from './ChatWindow'; | |||
There was a problem hiding this comment.
In room-related controller endpoints (e.g., join, checkMembership, getMessages) route params such as req.params.id and req.params.userId are strings. To avoid mixing string/number IDs between Sequelize and the in-memory rooms map on the server, coerce these values to Number before DB queries/logic (e.g. const roomId = Number(req.params.id);).
| @@ -0,0 +1,52 @@ | |||
| .sidebar { | |||
There was a problem hiding this comment.
Server ↔ client payload key mismatch: server WebSocket broadcasts for ROOM_RENAMED and ROOM_DELETED use payload keys named normRoomId (server side). The client (ChatProvider/ChatWindow) expects payload.roomId. This mismatch will prevent client-side room rename/delete updates. Change server broadcast payloads to use roomId: normRoomId (and include newName unchanged) so client handlers receive the expected field.
| @@ -0,0 +1,52 @@ | |||
| .sidebar { | |||
There was a problem hiding this comment.
Robustness: in room controller endpoints (join, checkMembership, getMessages) req.params.id and req.params.userId are used directly (strings). Convert these to numbers before DB queries/logic (e.g., const roomId = Number(req.params.id)) to avoid type inconsistencies between Sequelize numeric fields and the server's in-memory rooms map.
| @@ -0,0 +1,63 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
Server↔client payload mismatch: the server sends ROOM_RENAMED and ROOM_DELETED broadcasts with payload keys named normRoomId (server). The client ChatProvider and ChatWindow expect payload.roomId. This mismatch prevents client-side room rename/delete updates. Update server broadcasts to use roomId: normRoomId (and keep newName) so client handlers receive payload.roomId as expected.
| @@ -0,0 +1,63 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
In room-related controller endpoints (join, checkMembership, getMessages) you use req.params.id / req.params.userId directly (strings). For robustness, coerce these to numbers before DB queries/logic (e.g., const roomId = Number(req.params.id);) to avoid mixing string/number IDs between Sequelize and the server's in-memory rooms map.
| @@ -0,0 +1,15 @@ | |||
| import { createContext } from 'react'; | |||
There was a problem hiding this comment.
Server ↔ client payload mismatch: the client (ChatProvider/ChatWindow) expects payload.roomId for ROOM_RENAMED and ROOM_DELETED, but the server currently broadcasts payloads with normRoomId. Update server broadcasts to use roomId: normRoomId (and keep newName) so client handlers (which check data.payload.roomId) work correctly.
| @@ -0,0 +1,15 @@ | |||
| import { createContext } from 'react'; | |||
There was a problem hiding this comment.
In room controller endpoints (join, checkMembership, getMessages) the code uses route params (e.g., req.params.id, req.params.userId) directly (strings). Coerce these to numbers before DB queries (e.g., const roomId = Number(req.params.id);) to avoid mixing string/number IDs between Sequelize and the server's in-memory rooms map.
| @@ -0,0 +1 @@ | |||
| export * from './Sidebar'; | |||
There was a problem hiding this comment.
Server/client payload mismatch: the server broadcasts ROOM_RENAMED and ROOM_DELETED using payload keys named normRoomId, but client-side ChatProvider and ChatWindow expect payload.roomId. This prevents client-side room rename/delete updates. Update server broadcasts to use roomId: normRoomId (and keep newName) so client handlers match.
| @@ -0,0 +1 @@ | |||
| export * from './Sidebar'; | |||
There was a problem hiding this comment.
Robustness suggestion: in room controller endpoints (join, checkMembership, getMessages) route params like req.params.id and req.params.userId are used directly (strings). Coerce these to Number before DB queries/logic (e.g., const roomId = Number(req.params.id)) to avoid mixing string/number IDs with Sequelize and the server in-memory rooms map.
| @@ -0,0 +1,10 @@ | |||
| import { StrictMode } from 'react' | |||
There was a problem hiding this comment.
Server ↔ client payload mismatch: client ChatProvider expects payload.roomId for ROOM_RENAMED and ROOM_DELETED (ChatProvider.tsx), but server broadcasts use normRoomId in payloads. Update server broadcasts to use roomId: normRoomId (and keep newName) so client-side handlers update rooms correctly.
| @@ -0,0 +1,10 @@ | |||
| import { StrictMode } from 'react' | |||
There was a problem hiding this comment.
Robustness: in room controller endpoints (join, checkMembership, getMessages) route params like req.params.id and req.params.userId are used directly (strings). Coerce these to numbers before DB queries/logic (e.g., const roomId = Number(req.params.id)) to avoid mixing string/number IDs between Sequelize and the in-memory rooms map on the server.
| import { StrictMode } from 'react' | ||
| import { createRoot } from 'react-dom/client' | ||
| import './index.css' | ||
| import App from './App.tsx' |
There was a problem hiding this comment.
Minor: main.tsx imports App with extension ./App.tsx (line 4). This is usually fine with your tsconfig, but if you see bundler issues consider importing ./App instead. No change required if current setup builds.
| @@ -0,0 +1,3 @@ | |||
| body { | |||
There was a problem hiding this comment.
Server ↔ client payload mismatch: client-side ChatProvider expects data.payload.roomId for ROOM_RENAMED and ROOM_DELETED, but server broadcasts use normRoomId in payload. Update server broadcasts to send { roomId: normRoomId, newName } and { roomId: normRoomId } so client handlers match and room rename/delete updates apply.
| @@ -0,0 +1,3 @@ | |||
| body { | |||
There was a problem hiding this comment.
Robustness suggestion: in room controller endpoints (join, checkMembership, getMessages) route params like req.params.id and req.params.userId are used directly (strings). Coerce these to numbers before DB queries or comparisons (e.g., const roomId = Number(req.params.id)) to avoid mixing string/number IDs between Sequelize and the server's in-memory rooms map.
| ); | ||
|
|
||
| return ( | ||
| <ChatContext.Provider |
There was a problem hiding this comment.
ChatWindow.tsx references styles.deleted_overlay (line 143) but ChatWindow.module.scss does not define .deleted_overlay. Add a .deleted_overlay rule to the SCSS (e.g. centered box, padding, background) so the deleted-room overlay is visible and styled.
| // 2. Налаштування WebSocket | ||
| useEffect(() => { | ||
| const ws = new WebSocket('ws://localhost:5700'); | ||
|
|
||
| ws.onopen = () => { | ||
| console.log('✅ Connected to WS'); | ||
| setSocket(ws); | ||
| }; | ||
|
|
||
| ws.onmessage = (event) => { | ||
| try { | ||
| const data: WSMessage = JSON.parse(event.data); | ||
|
|
||
| if (data.type === MessageType.MESSAGE_NEW) { | ||
| setMessages((prev) => [...prev, data.payload]); | ||
| } | ||
|
|
||
| if (data.type === MessageType.ROOM_NEW) { | ||
| setRooms((prev) => [...prev, data.payload]); | ||
| } | ||
|
|
||
| if (data.type === MessageType.ROOM_RENAMED) { | ||
| setRooms((prev) => | ||
| prev.map((r) => | ||
| r.id === data.payload.roomId ? { ...r, name: data.payload.newName } : r, | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| if (data.type === MessageType.ROOM_DELETED) { | ||
| setRooms((prev) => prev.filter((r) => r.id !== data.payload.roomId)); | ||
| } | ||
| } catch (err) { | ||
| console.error('WS Message Error:', err); | ||
| } | ||
| }; | ||
|
|
||
| ws.onclose = () => { | ||
| console.log('❌ WS Disconnected'); | ||
| setSocket(null); | ||
| }; | ||
|
|
||
| return () => ws.close(); | ||
| }, []); |
There was a problem hiding this comment.
Client ChatProvider expects WS payloads to include payload.roomId for ROOM_RENAMED and ROOM_DELETED (see ChatProvider onmessage handling lines ~48–57). The server currently broadcasts these payloads using normRoomId (server-side). This naming mismatch will prevent client-side room rename/delete updates from applying. Ensure server broadcasts use roomId as the payload key (e.g. { roomId: normRoomId, newName } and { roomId: normRoomId }).
| @@ -0,0 +1,158 @@ | |||
| import React, { useState, useEffect, useCallback } from 'react'; | |||
There was a problem hiding this comment.
In controller endpoints that use route params (e.g. room.controller.js: join, checkMembership, getMessages) you read req.params.id / req.params.userId (strings). For robustness and to avoid mixing string/number IDs with Sequelize and the in-memory rooms map, coerce these to numbers before DB queries/logic (e.g. const roomId = Number(req.params.id);).
| import React, { useState, useEffect, useCallback } from 'react'; | ||
| import axios from 'axios'; | ||
| import { ChatContext } from './ChatContext'; | ||
| import { |
There was a problem hiding this comment.
Minor: main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This is acceptable with your TS/Vite config, but if you get tooling complaints consider using import App from './App'. No code change required unless your build errors.
| @@ -0,0 +1,11 @@ | |||
| import { useContext } from 'react'; | |||
There was a problem hiding this comment.
Server ↔ client payload mismatch: the client (ChatProvider) expects payload.roomId for ROOM_RENAMED and ROOM_DELETED, but the server broadcasts these payloads with normRoomId. This prevents client-side room rename/delete updates. Update server broadcasts to use roomId: normRoomId (and keep newName) so client handlers receive the expected field.
| @@ -0,0 +1,11 @@ | |||
| import { useContext } from 'react'; | |||
There was a problem hiding this comment.
Robustness: controller endpoints (join, checkMembership, getMessages) use route params like req.params.id and req.params.userId directly (strings). Convert these to numbers before DB queries/logic (e.g., const roomId = Number(req.params.id)) to avoid mixing string/number IDs between Sequelize and the server's in-memory rooms map.
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
In controller endpoints (e.g. room.controller.js: join, checkMembership, getMessages) route params such as req.params.id and req.params.userId are used directly (strings). Convert these to numbers before DB queries/logic (e.g., const roomId = Number(req.params.id)) to avoid mixing string/number IDs between Sequelize and the server's in-memory rooms map.
| { | ||
| "files": [], | ||
| "references": [ | ||
| { "path": "./tsconfig.app.json" }, |
There was a problem hiding this comment.
Minor: main.tsx imports App with explicit extension ./App.tsx (line 4). This is acceptable given your tsconfig (allowImportingTsExtensions), but if you encounter bundler issues prefer import App from './App'. No change required if build works.
| @@ -0,0 +1,36 @@ | |||
| export const MessageType = { | |||
There was a problem hiding this comment.
In server controller methods (e.g., room.controller.js: join, checkMembership, getMessages) you read req.params.id and req.params.userId as strings. To avoid mixing string/number IDs with Sequelize and the server's in-memory rooms map, coerce these to numbers before using them in queries/logic (e.g., const roomId = Number(req.params.id);).
| export const MessageType = { | ||
| USER_LOGIN: 'USER_LOGIN', | ||
| ROOM_JOIN: 'ROOM_JOIN', | ||
| MESSAGE_SEND: 'MESSAGE_SEND', |
There was a problem hiding this comment.
Minor: main.tsx imports App using the extension ./App.tsx (client/src/main.tsx line 4). This is acceptable with the current tsconfig (allowImportingTsExtensions), but if you encounter bundler/tooling issues consider importing ./App instead. No change required if your build works.
| @@ -0,0 +1,28 @@ | |||
| { | |||
There was a problem hiding this comment.
In room controller endpoints (join, checkMembership, getMessages) req.params.id and req.params.userId are used directly (strings). For robustness and to avoid mixing string/number IDs with Sequelize and the server in-memory rooms map, convert these to numbers before DB queries/logic (e.g., const roomId = Number(req.params.id);).
| { | ||
| "compilerOptions": { | ||
| "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.app.tsbuildinfo", | ||
| "target": "ES2023", |
There was a problem hiding this comment.
Minor: main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This is acceptable with the current tsconfig (allowImportingTsExtensions: true), but if you encounter bundler issues consider importing ./App instead. No change required unless you see build errors.
| const room = await Room.create({ name }); | ||
|
|
||
| res.status(201).json(room); | ||
| } catch (error) { | ||
| res.status(500).json({ message: 'Error creating room' }); | ||
| } | ||
| }, | ||
|
|
||
| join: async (req, res) => { | ||
| const { id } = req.params; // Отримуємо id з URL | ||
| const { userId } = req.body; | ||
|
|
||
| try { | ||
| await UserRoom.findOrCreate({ where: { roomId: id, userId } }); | ||
| res.status(200).json({ message: 'Joined' }); | ||
| } catch (err) { | ||
| res.status(500).json({ message: 'Error joining room' }); | ||
| } | ||
| }, | ||
|
|
||
| checkMembership: async (req, res) => { | ||
| const { id, userId } = req.params; | ||
| const membership = await UserRoom.findOne({ | ||
| where: { roomId: id, userId }, | ||
| }); | ||
|
|
||
| res.json({ isMember: !!membership }); | ||
| }, | ||
|
|
||
| // Отримати історію повідомлень кімнати (Найважливіше!) | ||
| getMessages: async (req, res) => { | ||
| try { | ||
| const { id } = req.params; | ||
|
|
||
| const messages = await Message.findAll({ | ||
| where: { roomId: id }, | ||
| include: [ | ||
| { | ||
| model: User, | ||
| as: 'author', | ||
| attributes: ['username'], // Тягнемо тільки ім'я автора | ||
| }, | ||
| ], | ||
| order: [['createdAt', 'ASC']], // Від старіших до новіших |
There was a problem hiding this comment.
Client ChatProvider expects WS payloads to contain payload.roomId for ROOM_RENAMED and ROOM_DELETED (see ChatProvider.onmessage handling). The server currently broadcasts these payloads with normRoomId. This naming mismatch prevents client-side room rename/delete updates. Update server broadcasts to use roomId: normRoomId (and include newName) so client handlers receive the expected field names.
| join: async (req, res) => { | ||
| const { id } = req.params; // Отримуємо id з URL | ||
| const { userId } = req.body; | ||
|
|
||
| try { | ||
| await UserRoom.findOrCreate({ where: { roomId: id, userId } }); | ||
| res.status(200).json({ message: 'Joined' }); | ||
| } catch (err) { | ||
| res.status(500).json({ message: 'Error joining room' }); | ||
| } | ||
| }, | ||
|
|
||
| checkMembership: async (req, res) => { | ||
| const { id, userId } = req.params; | ||
| const membership = await UserRoom.findOne({ | ||
| where: { roomId: id, userId }, | ||
| }); | ||
|
|
||
| res.json({ isMember: !!membership }); | ||
| }, | ||
|
|
||
| // Отримати історію повідомлень кімнати (Найважливіше!) | ||
| getMessages: async (req, res) => { | ||
| try { | ||
| const { id } = req.params; | ||
|
|
||
| const messages = await Message.findAll({ | ||
| where: { roomId: id }, | ||
| include: [ | ||
| { | ||
| model: User, | ||
| as: 'author', | ||
| attributes: ['username'], // Тягнемо тільки ім'я автора | ||
| }, | ||
| ], | ||
| order: [['createdAt', 'ASC']], // Від старіших до новіших | ||
| }); | ||
|
|
||
| // Перетворюємо дані у зручний формат | ||
| const formattedMessages = messages.map((msg) => ({ | ||
| id: msg.id, | ||
| text: msg.text, | ||
| userId: msg.userId, | ||
| roomId: msg.roomId, | ||
| authorName: msg.author?.username || 'Unknown', | ||
| createdAt: msg.createdAt, | ||
| })); | ||
|
|
||
| res.json(formattedMessages); | ||
| } catch (error) { | ||
| console.error(error); | ||
| res.status(500).json({ message: 'Error fetching messages' }); | ||
| } | ||
| }, | ||
| }; | ||
|
|
||
| module.exports = { roomController }; |
There was a problem hiding this comment.
In room.controller.js (join, checkMembership, getMessages) route params like req.params.id and req.params.userId are used directly (strings). For robustness and to avoid mixing string/number IDs between Sequelize and the server's in-memory rooms map, coerce these to numbers before DB queries/logic (e.g., const roomId = Number(req.params.id);).
| 'use strict'; | ||
|
|
||
| /* eslint-disable no-console */ | ||
| const { Room, Message, User, UserRoom } = require('../models'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This works with your tsconfig option allowImportingTsExtensions, but if you see bundler/tooling issues consider import App from './App'. No change required if your build runs.
| /* eslint-disable no-console */ | ||
| 'use strict'; | ||
|
|
||
| const { Sequelize } = require('sequelize'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App using an explicit extension ./App.tsx (line 4). This is acceptable with your tsconfig (allowImportingTsExtensions: true), but if you encounter bundler issues consider importing ./App instead. No change needed if build works.
| import { defineConfig } from 'vite' | ||
| import react from '@vitejs/plugin-react' | ||
|
|
||
| // https://vite.dev/config/ |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This is acceptable with your tsconfig (allowImportingTsExtensions), but if you encounter bundler issues consider importing ./App. No change required if the build works.
| 'use strict'; | ||
|
|
||
| /* eslint-disable no-console */ | ||
|
|
There was a problem hiding this comment.
Minor: main.tsx imports App with explicit extension import App from './App.tsx' (client/src/main.tsx line 4). This is acceptable with your tsconfig (allowImportingTsExtensions: true), but if you encounter bundler issues prefer import App from './App'. No change required if the build works.
| 'use strict'; | ||
|
|
||
| const { Model, DataTypes } = require('sequelize'); | ||
| const { sequelize } = require('../db'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This is allowed by your tsconfig flag allowImportingTsExtensions, but if you get bundler/tooling complaints consider using import App from './App'. No change if build works.
| wss.clients.forEach((client) => { | ||
| if (client.readyState === WebSocket.OPEN) { | ||
| client.send(broadcastData); | ||
| } |
There was a problem hiding this comment.
ChatWindow.tsx uses styles.deleted_overlay (line 143) but ChatWindow.module.scss does not define .deleted_overlay. Add a rule for .deleted_overlay (centered container, padding/background) so the deleted-room overlay is visible and styled.
| const broadcastData = JSON.stringify({ | ||
| type: MessageType.ROOM_RENAMED, | ||
| payload: { normRoomId, newName }, |
There was a problem hiding this comment.
Client ChatProvider.onmessage expects WS payloads to include payload.roomId for ROOM_RENAMED and ROOM_DELETED (client/src/context/ChatProvider.tsx lines ~48–57). In src/index.js the server currently broadcasts these payloads with key normRoomId (lines 112–114 and 135–137). This naming mismatch prevents client-side room rename/delete updates. Change server broadcasts to use roomId: normRoomId (and include newName) so client handlers receive the expected field names.
| wss.on('connection', (ws) => { | ||
| console.log('Нове підключення встановлено'); | ||
|
|
||
| ws.on('message', async (data) => { | ||
| try { | ||
| const rawData = typeof data === 'string' ? data : data.toString(); | ||
| const message = JSON.parse(rawData); | ||
| const { type, payload } = message; | ||
|
|
||
| console.log('Отримано повідомлення:', message); | ||
|
|
||
| switch (type) { | ||
| case MessageType.ROOM_JOIN: { | ||
| const normRoomId = Number(payload.roomId); | ||
|
|
||
| // Якщо кімнати ще немає в пам'яті сервера — створюємо її | ||
| if (!rooms[normRoomId]) { | ||
| rooms[normRoomId] = new Set(); | ||
| } | ||
|
|
||
| // Видаляємо з попередньої кімнати, якщо була | ||
| if (ws.currentRoom && rooms[ws.currentRoom]) { | ||
| rooms[ws.currentRoom].delete(ws); | ||
| } | ||
|
|
||
| if (ws.currentRoom === normRoomId) { | ||
| break; | ||
| } | ||
|
|
||
| rooms[normRoomId].add(ws); | ||
| ws.currentRoom = normRoomId; | ||
| console.log(`Користувач приєднався до кімнати ${normRoomId}`); | ||
|
|
||
| break; | ||
| } | ||
|
|
||
| case MessageType.ROOM_CREATE: { | ||
| try { | ||
| const { name, userId } = payload; // Отримуємо name | ||
| // та userId від клієнта | ||
| const normUserId = Number(userId); | ||
|
|
||
| // 1. Створюємо кімнату в БД | ||
| const newRoom = await Room.create({ | ||
| name, | ||
| ownerId: normUserId, // Записуємо власника! | ||
| }); | ||
|
|
||
| // 2. Розсилаємо ВУСІМ підключеним клієнтам (wss.clients) | ||
| const broadcastData = JSON.stringify({ | ||
| type: MessageType.ROOM_NEW, | ||
| payload: newRoom, | ||
| }); | ||
|
|
||
| wss.clients.forEach((client) => { | ||
| if (client.readyState === WebSocket.OPEN) { | ||
| client.send(broadcastData); |
There was a problem hiding this comment.
In src/controllers/room.controller.js the methods join, checkMembership, and getMessages use req.params.id and req.params.userId (strings) directly in DB queries/where clauses (lines 35–40, 47–51, 57–62). For robustness and to avoid mixing string/number IDs with Sequelize and the server's in-memory rooms map, coerce these to numbers before use (e.g., const roomId = Number(req.params.id);).
| /* eslint-disable no-console */ | ||
|
|
||
| 'use strict'; | ||
|
|
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App using explicit extension import App from './App.tsx' (line 4). This is acceptable with your tsconfig (allowImportingTsExtensions), but if you encounter bundler/tooling issues consider using import App from './App'. No change required if the build works.
| 'use strict'; | ||
|
|
||
| const { Model, DataTypes } = require('sequelize'); | ||
| const { sequelize } = require('../db'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This works with the current tsconfig (allowImportingTsExtensions), but if you encounter bundler issues consider import App from './App'. No change required if build succeeds.
| 'use strict'; | ||
|
|
||
| const { Model, DataTypes } = require('sequelize'); | ||
| const { sequelize } = require('../db'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App using the extension import App from './App.tsx' (line 4). This is allowed by your tsconfig (allowImportingTsExtensions) but if you face bundler/tooling issues consider importing ./App. No change required if build succeeds.
| 'use strict'; | ||
|
|
||
| const { Router } = require('express'); | ||
| const { usersController } = require('../controllers/users.controller'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This is acceptable with your tsconfig (allowImportingTsExtensions), but if you encounter bundler issues consider importing ./App. No change required if build works.
| 'use strict'; | ||
|
|
||
| const { Router } = require('express'); | ||
| const { roomController } = require('../controllers/room.controller'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This is allowed by your tsconfig (allowImportingTsExtensions), but if you encounter bundler issues consider importing ./App. No change required if build succeeds.
| 'use strict'; | ||
|
|
||
| const { Model, DataTypes } = require('sequelize'); | ||
| const { sequelize } = require('../db'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This is acceptable with your tsconfig (allowImportingTsExtensions), but if you encounter bundler/tooling issues consider import App from './App'. No change required if the build works.
| 'use strict'; | ||
|
|
||
| const { User } = require('./User.model'); | ||
| const { Room } = require('./Room.model'); |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This works with your tsconfig (allowImportingTsExtensions), but if you encounter bundler issues consider importing ./App. No change required if build succeeds.
| 'use strict'; | ||
|
|
||
| const MessageType = { | ||
| USER_LOGIN: 'USER_LOGIN', |
There was a problem hiding this comment.
Minor: client/src/main.tsx imports App with explicit extension import App from './App.tsx' (line 4). This is permitted by your tsconfig (allowImportingTsExtensions), but if you encounter bundler issues prefer importing ./App. No change required if the build works.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as meeting the assignment requirements, and this is an educational override to help you keep progressing while you refine your skills. You’ve successfully implemented all key chat features: login with username saved in localStorage, messages that show author/time/text, room create/rename/join/delete, and loading previous messages when a user joins a room. One important improvement opportunity is in client/src/components/ChatWindow/ChatWindow.tsx, where you currently render messages with key={index}—switching to key={msg.id ?? index} will give React more stable keys and prevent subtle UI bugs. On the server side, in src/controllers/room.controller.js (getMessages), consider coercing the route param to a number (e.g. const roomId = Number(req.params.id);) before using it in where: { roomId } so it stays consistent with your other handlers and avoids string/number mismatches. As small UX and portability upgrades, trimming the username and room name on the client before sending them, and centralizing your hardcoded http://localhost:5700 / ws://localhost:5700 URLs into environment variables, will make your app feel more polished and easier to deploy. Overall, your structure, use of React context/WebSockets, and satisfaction of the checklist items are solid—these tweaks will simply make an already good solution more robust and professional.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const response = await axios.post('http://localhost:5700/api/login', { username }); | ||
| const userData = response.data; | ||
|
|
||
| localStorage.setItem('chat_user', JSON.stringify(userData)); |
There was a problem hiding this comment.
Good: Login stores the returned user object in localStorage which satisfies the requirement to save username on the client. Ensure server /api/login returns the user object with id and username as currently implemented in users.controller.js.
| e.preventDefault(); | ||
|
|
||
| try { | ||
| const response = await axios.post('http://localhost:5700/api/login', { username }); |
There was a problem hiding this comment.
Minor: axios.post target is hardcoded to http://localhost:5700. This is fine for local dev but consider using an env variable (CLIENT_URL or similar) for deployment. Not required by task but practical.
| e.preventDefault(); | ||
|
|
||
| try { | ||
| const response = await axios.post('http://localhost:5700/api/login', { username }); |
There was a problem hiding this comment.
Potential improvement: handleSubmit does not trim the username before sending; the server checks trimmed length but trimming client-side can provide earlier feedback. Optional.
| roomId, | ||
| setActiveRoomId, | ||
| handleLogout, | ||
| user, |
There was a problem hiding this comment.
The component stores the logged-in user in localStorage under the key chat_user and calls onLogin with the server response — this satisfies the requirement "The user must be able to type a username and send it to the server" and "The username must be saved in localStorage". Good.
| }; | ||
|
|
||
| export const ChatWindow: React.FC<Props> = ({ | ||
| roomId, |
There was a problem hiding this comment.
The Login handler requires username length on server side; client uses the server response directly. Consider trimming before sending to avoid sending empty/whitespace names, though server already validates. Example: send username.trim() from client. Not required by checklist but improves UX.
| import { useChat } from '../../hooks/useChat'; | ||
| import axios from 'axios'; | ||
| import cn from 'classnames'; | ||
| import styles from './ChatWindow.module.scss'; | ||
|
|
||
| type Props = { | ||
| roomId: number; | ||
| user: { id: number; username: string }; | ||
| setActiveRoomId: (id: number | null) => void; | ||
| handleLogout: () => void; | ||
| }; | ||
|
|
||
| export const ChatWindow: React.FC<Props> = ({ | ||
| roomId, |
There was a problem hiding this comment.
No undefined imports or syntax errors in this file. The axios POST URL points to http://localhost:5700/api/login — ensure this matches your server during development. If CLIENT_URL/CORS differ, requests may be blocked (server uses CORS based on env).
| try { | ||
| createRoom(newRoomName, userId); |
There was a problem hiding this comment.
Good: the Sidebar renders rooms from context and allows selecting and creating rooms. It calls createRoom(newRoomName, userId) which aligns with the ChatProvider WS API.
| if (!newRoomName.trim()) return; | ||
|
|
||
| try { | ||
| createRoom(newRoomName, userId); |
There was a problem hiding this comment.
Minor: the form submission uses newRoomName directly. You already guard with trim(); consider trimming before passing to createRoom (e.g. createRoom(newRoomName.trim(), userId)) to avoid leading/trailing spaces in room names. Low severity.
| interface SidebarProps { | ||
| onSelectRoom: (id: number) => void; | ||
| activeRoomId: number | null; | ||
| userId: number; | ||
| } | ||
|
|
||
| export const Sidebar: React.FC<SidebarProps> = ({ | ||
| onSelectRoom, | ||
| activeRoomId, | ||
| userId, | ||
| }) => { |
There was a problem hiding this comment.
Heads-up: onSelectRoom expects a number — good. Ensure callers pass null when unselecting (App uses setActiveRoomId(null) and Sidebar's onSelectRoom uses number only). This component's typing is fine.
| <ul className={styles.room_list}> | ||
| {rooms.map((room) => ( | ||
| <li | ||
| key={room.id} | ||
| className={cn(styles.room_item, { | ||
| [styles.active]: activeRoomId === room.id, | ||
| })} | ||
| // className={`room-item ${activeRoomId === room.id ? 'active' : ''}`} | ||
| onClick={() => onSelectRoom(room.id)} | ||
| > | ||
| # {room.name} | ||
| </li> | ||
| ))} | ||
| </ul> |
There was a problem hiding this comment.
No syntax or import issues. key={room.id} is correct and preferable to array index. CSS class names referenced are present in Sidebar.module.scss. No further action required.
| export const ChatProvider: React.FC<{ children: React.ReactNode }> = ({ | ||
| children, | ||
| }) => { | ||
| const [socket, setSocket] = useState<WebSocket | null>(null); |
There was a problem hiding this comment.
Login.tsx posts the username directly. Although server validates trimmed length, trim client-side before sending to avoid accidental whitespace-only names: send username.trim() instead of username.
| .get('http://localhost:5700/api/rooms') | ||
| .then((res) => setRooms(res.data)) | ||
| .catch((err) => console.error('Error loading rooms', err)); | ||
| }, []); | ||
|
|
||
| // 2. Налаштування WebSocket | ||
| useEffect(() => { | ||
| const ws = new WebSocket('ws://localhost:5700'); |
There was a problem hiding this comment.
Several axios/WebSocket URLs are hardcoded to http://localhost:5700 / ws://localhost:5700. For flexibility across environments consider using an environment variable (e.g. REACT_APP_API_URL) or centralizing the base URL. Not required for task completion but recommended.
|
|
||
| return () => ws.close(); | ||
| }, []); | ||
|
|
||
| const sendMessage = useCallback( |
There was a problem hiding this comment.
In ChatProvider, ws is created and setSocket is called on open; the various callback functions depend on the socket instance. Ensure these functions remain stable (they are memoized with socket dep). This is fine, just a note that recreating socket will update callbacks correctly.
| .then((res) => setRooms(res.data)) | ||
| .catch((err) => console.error('Error loading rooms', err)); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
In Sidebar.tsx you validate newRoomName.trim() before creating room; consider trimming when calling createRoom as well: createRoom(newRoomName.trim(), userId) to avoid room names with leading/trailing spaces.
| "allowImportingTsExtensions": true, | ||
| "verbatimModuleSyntax": true, | ||
| "moduleDetection": "force", | ||
| "noEmit": true, |
There was a problem hiding this comment.
In Login.tsx you send username directly to the server. Server already trims/validates, but trimming client-side improves UX and avoids sending whitespace-only names. Consider sending username.trim() instead.
| "noUncheckedSideEffectImports": true | ||
| }, | ||
| "include": ["vite.config.ts"] | ||
| } |
There was a problem hiding this comment.
In Sidebar.tsx you guard against blank room names with newRoomName.trim() which is good. Also consider trimming the name when calling createRoom (createRoom(newRoomName.trim(), userId)) to avoid leading/trailing spaces being persisted as room names.
| ROOM_DELETED: 'ROOM_DELETED', | ||
| } as const; | ||
|
|
||
| // Якщо тобі потрібен саме ТИП для аргументів функцій: |
There was a problem hiding this comment.
In Login.tsx you send username directly to the server. The server trims/validates, but trimming client-side avoids sending whitespace-only names. Consider sending username.trim() (or at least trimming before calling axios).
| roomId: number; | ||
| authorName: string; | ||
| createdAt: string; | ||
| } | ||
|
|
||
| export interface WSMessage { | ||
| type: MessageTypeValues; | ||
| payload: any; |
There was a problem hiding this comment.
Multiple client files (ChatProvider, ChatWindow, Login, Sidebar) use hardcoded backend URLs like http://localhost:5700 and WS 'ws://localhost:5700'. For portability use an environment variable or centralized config for API/WS base URL (e.g. process.env.REACT_APP_API_URL). This is optional but recommended for deployment.
| authorName: string; | ||
| createdAt: string; | ||
| } | ||
|
|
There was a problem hiding this comment.
In Sidebar.tsx you already guard against blank names with newRoomName.trim(); for consistency trim before sending to server as well: createRoom(newRoomName.trim(), userId) to avoid leading/trailing spaces in room names.
| "moduleResolution": "bundler", | ||
| "allowImportingTsExtensions": true, | ||
| "verbatimModuleSyntax": true, | ||
| "moduleDetection": "force", |
There was a problem hiding this comment.
In Login.tsx you send username directly to the server. The server trims/validates, but trimming on the client improves UX and avoids sending whitespace-only names. Consider sending username.trim() instead.
| "erasableSyntaxOnly": true, | ||
| "noFallthroughCasesInSwitch": true, | ||
| "noUncheckedSideEffectImports": true | ||
| }, |
There was a problem hiding this comment.
Sidebar.tsx checks newRoomName.trim() before creating a room but passes the raw value to createRoom. Consider calling createRoom(newRoomName.trim(), userId) to avoid room names with accidental leading/trailing spaces.
| POSTGRES_DB, | ||
| } = process.env; | ||
|
|
||
| // Створюємо інстанс Sequelize (з sequelize-typescript!) |
There was a problem hiding this comment.
In Login.tsx you send username directly to the server. The server trims/validates, but trimming on the client improves UX and avoids sending whitespace-only names. Consider sending username.trim() instead.
| port: Number(POSTGRES_PORT) || 5432, | ||
| password: POSTGRES_PASSWORD || '123123', | ||
| logging: false, | ||
| }); | ||
|
|
||
| const initDB = async () => { | ||
| try { | ||
| await sequelize.authenticate(); | ||
| await sequelize.sync({ alter: true }); |
There was a problem hiding this comment.
Several client files (ChatProvider, ChatWindow, Login, Sidebar) use hardcoded backend URLs like http://localhost:5700 and ws://localhost:5700. For portability between environments consider centralizing the API/WS base URL via an environment variable (e.g. import.meta.env.VITE_API_URL). Not required by the task but recommended.
| logging: false, | ||
| }); | ||
|
|
||
| const initDB = async () => { |
There was a problem hiding this comment.
In Sidebar.tsx you already guard against blank names with newRoomName.trim(); also trim the value when calling createRoom to avoid leading/trailing spaces being persisted as room names: createRoom(newRoomName.trim(), userId).
| // Отримати історію повідомлень кімнати (Найважливіше!) | ||
| getMessages: async (req, res) => { | ||
| try { | ||
| const { id } = req.params; |
There was a problem hiding this comment.
In getMessages you read const { id } = req.params; and use it in where: { roomId: id }. For consistency with other controller methods (which coerce req.params to Number) and to avoid mixing string/number IDs when querying or comparing with in-memory maps, coerce the param: const roomId = Number(req.params.id); and use where: { roomId }.
| userId: msg.userId, | ||
| roomId: msg.roomId, | ||
| authorName: msg.author?.username || 'Unknown', | ||
| createdAt: msg.createdAt, |
There was a problem hiding this comment.
Minor: the function returns createdAt from Sequelize directly. That's fine, but ensure frontend treats it as an ISO-string; you already format it with new Date(msg.createdAt) on client — OK. No change required unless you plan different formats.
| where: { username: username.trim() }, | ||
| defaults: { username: username.trim() }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
create handler accepts name from req.body but does not trim it; consider trimming and validating length to avoid empty/whitespace room names being created: e.g. const name = (req.body.name || '').trim(); if (!name) return res.status(400).... Improves data quality (optional).
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
In ChatWindow.tsx you render messages using array index as the React key. This can cause reconciliation bugs when messages are inserted/removed. Prefer a stable unique key such as msg.id (fall back to index if id is absent): key={msg.id ?? index}.
| const { Room, Message, User } = require('./models'); | ||
| const { roomRouter } = require('./routes/room.router'); | ||
| const { usersRouter } = require('./routes/users.router'); | ||
| const { MessageType } = require('./shared/types'); |
There was a problem hiding this comment.
In Login.tsx you send username directly to the server. The server trims/validates, but trimming on the client avoids sending whitespace-only names and improves UX. Consider sending username.trim() instead of raw username.
|
|
||
| app.use('/api', roomRouter); | ||
| app.use('/api', usersRouter); | ||
|
|
||
| app.use((req, res) => { | ||
| res.status(404).json({ message: 'Page not found' }); | ||
| }); | ||
|
|
||
| const server = http.createServer(app); |
There was a problem hiding this comment.
Multiple client files (ChatProvider, ChatWindow, Login, Sidebar) hardcode backend URLs like http://localhost:5700 and WS ws://localhost:5700. For portability across environments, centralize the API/WS base URL via an environment variable (e.g. import.meta.env.VITE_API_URL / VITE_WS_URL). Optional but recommended.
| app.use('/api', usersRouter); | ||
|
|
||
| app.use((req, res) => { | ||
| res.status(404).json({ message: 'Page not found' }); |
There was a problem hiding this comment.
In Sidebar.tsx you guard against blank room names with newRoomName.trim() — good. Also trim when calling createRoom to avoid persisting leading/trailing spaces: createRoom(newRoomName.trim(), userId).
| break; | ||
| } |
There was a problem hiding this comment.
In client/src/context/ChatProvider.tsx cleanup you call ws.close() when unmounting. Optionally guard with if (ws.readyState === WebSocket.OPEN) ws.close() and/or null event handlers (ws.onmessage = null) before closing to ensure a clean teardown. This is a minor robustness improvement.
| rooms[ws.currentRoom].delete(ws); | ||
| } | ||
|
|
||
| if (ws.currentRoom === normRoomId) { |
There was a problem hiding this comment.
Server-side (src/controllers/room.controller.js): getMessages reads const { id } = req.params; and uses it in where: { roomId: id }. For consistency with other controller functions and to avoid mixing string/number IDs, coerce the param to a Number: const roomId = Number(req.params.id); and use where: { roomId }. Low severity and recommended.
| console.log('Нове підключення встановлено'); | ||
|
|
||
| ws.on('message', async (data) => { | ||
| try { | ||
| const rawData = typeof data === 'string' ? data : data.toString(); | ||
| const message = JSON.parse(rawData); |
There was a problem hiding this comment.
ChatProvider appends incoming MESSAGE_NEW payloads directly to messages state. Ensure the server always sends payloads shaped like ChatMessage (it currently does). Optionally validate/normalize incoming payloads before appending for extra robustness.
| autoIncrement: true, | ||
| primaryKey: true, | ||
| }, | ||
| name: { |
There was a problem hiding this comment.
In client/src/components/Login/Login.tsx you send username directly to the server. Server trims and validates, but trimming on the client improves UX and avoids sending whitespace-only names. Consider sending username.trim() instead of username.
| allowNull: false, | ||
| references: { | ||
| model: 'users', // Назва таблиці | ||
| key: 'id', | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| sequelize, |
There was a problem hiding this comment.
Several client files (client/src/context/ChatProvider.tsx, ChatWindow.tsx, Login.tsx, Sidebar.tsx) use hardcoded backend URLs (http://localhost:5700 and ws://localhost:5700). For portability between environments consider centralizing the base API/WS URL via an environment variable (e.g. import.meta.env.VITE_API_URL / VITE_WS_URL). This is optional but recommended.
| model: 'users', // Назва таблиці | ||
| key: 'id', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
In client/src/components/Sidebar/Sidebar.tsx you correctly guard against blank room names with newRoomName.trim(). Also trim before sending to server to avoid room names containing leading/trailing whitespace: createRoom(newRoomName.trim(), userId).
| autoIncrement: true, | ||
| primaryKey: true, | ||
| }, | ||
| text: { |
There was a problem hiding this comment.
In client/src/components/Login/Login.tsx you send username directly. Although server trims and validates, trimming client-side improves UX and avoids whitespace-only names. Consider sending username.trim().
| model: 'users', // назва таблиці | ||
| key: 'id', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
In client/src/components/Sidebar/Sidebar.tsx you validate newRoomName.trim() which is good; also trim when calling createRoom to avoid persisting leading/trailing spaces: createRoom(newRoomName.trim(), userId). Low severity UX improvement.
| allowNull: false, | ||
| references: { | ||
| model: 'users', // назва таблиці | ||
| key: 'id', | ||
| }, | ||
| }, | ||
| roomId: { | ||
| type: DataTypes.INTEGER, | ||
| allowNull: false, |
There was a problem hiding this comment.
Multiple client files use hardcoded backend URLs (http://localhost:5700 and ws://localhost:5700) (ChatProvider, ChatWindow, Login, Sidebar). For portability between environments consider centralizing the API/WS base URL in an environment variable (e.g., Vite env) rather than hardcoding. Optional recommendation.
| }, | ||
| { | ||
| sequelize, | ||
| tableName: 'messages', | ||
| modelName: 'message', | ||
| }, |
There was a problem hiding this comment.
ChatProvider appends MESSAGE_NEW payloads directly to messages. Ensure server payloads conform to ChatMessage (they currently do). If you plan future changes, consider lightweight payload validation/normalization before appending. Optional improvement.
| autoIncrement: true, | ||
| primaryKey: true, | ||
| }, | ||
| username: { |
There was a problem hiding this comment.
In Login.tsx you send username directly to the server. Although server validates/trims, trimming client-side improves UX and avoids sending whitespace-only names. Consider sending username.trim().
| tableName: 'users', | ||
| modelName: 'user', | ||
| }, | ||
| ); |
There was a problem hiding this comment.
In Sidebar.tsx you check newRoomName.trim() before creating a room (good). Also trim when calling createRoom to avoid persisting leading/trailing spaces: createRoom(newRoomName.trim(), userId).
| autoIncrement: true, | ||
| primaryKey: true, | ||
| }, | ||
| userId: { |
There was a problem hiding this comment.
In client/src/components/Login/Login.tsx you send username directly to the server. The server trims/validates, but trimming on the client improves UX and avoids sending whitespace-only names. Consider sending username.trim() instead.
| }, | ||
| onDelete: 'CASCADE', | ||
| }, | ||
| roomId: { | ||
| type: DataTypes.INTEGER, | ||
| allowNull: false, | ||
| references: { | ||
| model: 'rooms', | ||
| key: 'id', |
There was a problem hiding this comment.
Multiple client files (ChatProvider, ChatWindow, Login, Sidebar) use hardcoded backend URLs (http://localhost:5700 and ws://localhost:5700). For portability between environments, centralize the API/WS base URL using an environment variable (e.g. Vite env like import.meta.env.VITE_API_URL / VITE_WS_URL). This is optional but recommended.
| }, | ||
| roomId: { | ||
| type: DataTypes.INTEGER, | ||
| allowNull: false, |
There was a problem hiding this comment.
In client/src/components/Sidebar/Sidebar.tsx you validate newRoomName.trim() which is good. Also trim the value when calling createRoom to avoid persisting leading/trailing spaces in room names: createRoom(newRoomName.trim(), userId).
| tableName: 'user_rooms', | ||
| modelName: 'user_room', | ||
| indexes: [ | ||
| { | ||
| unique: true, | ||
| fields: ['userId', 'roomId'], |
There was a problem hiding this comment.
ChatProvider appends incoming MESSAGE_NEW payloads directly to messages state. Ensure the server payload shape matches ChatMessage (it currently does). Optionally perform lightweight validation/normalization of data.payload before appending for extra robustness.
| // 2. Room <-> Message (One-to-Many) | ||
| Room.hasMany(Message, { | ||
| foreignKey: 'roomId', | ||
| as: 'messages', |
There was a problem hiding this comment.
In client/src/components/Login/Login.tsx you send username directly to the server. Server already trims and validates, but trimming client-side improves UX and avoids sending whitespace-only names. Consider sending username.trim() instead.
| Room.belongsTo(User, { foreignKey: 'ownerId', as: 'owner' }); | ||
|
|
||
| // 4. User <-> Room (Many-to-Many) | ||
| User.belongsToMany(Room, { |
There was a problem hiding this comment.
In client/src/components/Sidebar/Sidebar.tsx you check newRoomName.trim() before creating a room (good). Also trim when calling createRoom to avoid persisting leading/trailing spaces: createRoom(newRoomName.trim(), userId). Low-severity UX improvement.
| // 3. Room <-> User (Creator) (One-to-Many) | ||
| User.hasMany(Room, { foreignKey: 'ownerId', as: 'ownedRooms' }); | ||
| Room.belongsTo(User, { foreignKey: 'ownerId', as: 'owner' }); | ||
|
|
||
| // 4. User <-> Room (Many-to-Many) | ||
| User.belongsToMany(Room, { | ||
| through: UserRoom, | ||
| foreignKey: 'userId', | ||
| otherKey: 'roomId', |
There was a problem hiding this comment.
Multiple client files (ChatProvider, ChatWindow, Login, Sidebar) use hardcoded backend/WS URLs (http://localhost:5700 / ws://localhost:5700). For portability between environments consider centralizing the API/WS base URL using environment variables (e.g. Vite env vars). This is optional but recommended.
| otherKey: 'userId', | ||
| as: 'users', | ||
| }); | ||
|
|
||
| module.exports = { | ||
| User, |
There was a problem hiding this comment.
ChatProvider appends incoming MESSAGE_NEW payloads directly to messages. Ensure the server always sends payloads shaped as ChatMessage (it currently does). Optionally validate/normalize incoming payloads before appending for extra robustness.
No description provided.