first commit#102
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thank you for your hard work on this project! 🚧 While your solution is very close, there are several critical issues that need to be addressed before it can be approved. Specifically, there are problems in the backend that affect core functionality: missing imports for the crypto module, incorrect error handling (throwing the wrong variable and improper status code usage), and inconsistent or incorrect function naming (safeRoom/saveRoom, deleteRoome/deleteRoom). Additionally, the Message type is missing the required id property, and the messages data structure is inconsistent, which could cause runtime errors. There are also issues in the frontend such as navigation logic in the render body and incorrect template string usage. Please address these critical issues to ensure your application works reliably and meets the task requirements. Once these are fixed, your project will be in great shape! Keep going—you’re almost there! 💪
Summary of what to fix:
- Import the
cryptomodule where used. - Use
res.status(400)instead ofres.statusCode(400). - Standardize function names to
saveRoomanddeleteRoomeverywhere. - Fix error variable usage in the
fs.writeFilecallback. - Ensure all message objects have consistent structure with
id,author,text,date, androomId. - Add
idto theMessagetype definition. - Move navigation logic in
Chat.tsxinto auseEffect. - Fix template string usage in the chat heading.
- Minor: Use
maxLengthinstead ofmaxon text inputs, clear form input after submit, and fix ESLintignoresproperty placement.
Once these are resolved, your code will be ready for approval! If you need any help, feel free to ask. 😊
✨ 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
| export default tseslint.config( | ||
| { ignores: ['dist'] }, |
There was a problem hiding this comment.
Issue: The 'ignores' property is provided as a separate argument to 'tseslint.config', but it should be included as a property within the main configuration object. Please move 'ignores: ['dist']' inside the main configuration object to ensure it is recognized by ESLint.
|
|
||
| const submit = async (e: FormEvent<HTMLFormElement>) =>{ | ||
| e.preventDefault(); | ||
| onCreate(text) |
There was a problem hiding this comment.
Usability issue: After submitting the form and calling onCreate, the input field is not cleared. Consider resetting the text state (e.g., setText('')) after submission to improve user experience.
| ? ( | ||
| <form className='miniform' onSubmit={(e) => e.preventDefault()}> | ||
| <input | ||
| max={10} |
There was a problem hiding this comment.
Minor issue: The 'max' attribute is used on a text input, but the correct attribute for limiting the number of characters is 'maxLength'. Please replace 'max={10}' with 'maxLength={10}'.
| if(!id) { | ||
| navigate('/rooms') | ||
| return; | ||
| } |
There was a problem hiding this comment.
Critical issue: Calling 'navigate' directly in the render body (outside of useEffect) can cause unexpected behavior or render errors. You should move this navigation logic inside a useEffect to ensure it runs after the component mounts.
|
|
||
| return ( | ||
| <main className='main'> | ||
| <h1>{user?.name}`s Chat in {activeRoom?.name}</h1> |
There was a problem hiding this comment.
Minor issue: The heading uses a template string incorrectly: '{user?.name}s Chat in {activeRoom?.name}'. This will render a literal backtick. Use string interpolation or concatenation to display the possessive form correctly, e.g., ${user?.name}'s Chat in ${activeRoom?.name}`.
| return; | ||
| } | ||
|
|
||
| const id = crypto.randomUUID(); |
There was a problem hiding this comment.
Error: The 'crypto' module is used to generate a UUID, but it is not imported. Please add 'import crypto from "crypto";' at the top of the file.
| return; | ||
| } | ||
|
|
||
| const id = crypto.randomUUID(); |
There was a problem hiding this comment.
Error: The 'crypto' module is used to generate a UUID, but it is not imported. Please add 'import crypto from "crypto";' at the top of the file.
| id, | ||
| }; | ||
|
|
||
| roomsApi.safeRoom(newRoom); |
There was a problem hiding this comment.
Typo: 'roomsApi.safeRoom' should be 'roomsApi.saveRoom' for clarity and consistency with its purpose.
| id, | ||
| }; | ||
|
|
||
| roomsApi.safeRoom(updatedRoom); |
There was a problem hiding this comment.
Typo: 'roomsApi.safeRoom' should be 'roomsApi.saveRoom' for clarity and consistency with its purpose.
| return; | ||
| } | ||
|
|
||
| roomsApi.deleteRoome(id, (err) => { |
There was a problem hiding this comment.
Typo: 'roomsApi.deleteRoome' should be 'roomsApi.deleteRoom' for clarity and consistency with its purpose.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thank you for your hard work on this project! Before your pull request can be approved, there are a few critical issues that need to be addressed to ensure the application functions correctly and meets the core requirements:
-
Error Handling in Express Controller:
In your message controller, you are usingres.statusCode(400), which is not a valid Express method. You should useres.status(400)to set the status code before sending a response. This is essential for proper error handling and client communication. -
Message Type Definition:
TheMessagetype is missing the requiredidproperty, but other parts of your code (such as rendering messages inChat.tsx) expect each message to have anid. Please add anid: stringfield to theMessagetype to avoid TypeScript and runtime errors. -
Inconsistent Message Data Structure:
Yourmessages.jsonfile contains message objects with inconsistent structures—some are missing theidproperty or have nesteddataobjects. All messages should have a flat structure withid,author,text,roomId, anddateas top-level properties. Please update all entries to match this format for reliable data handling. -
Error Variable in fs.writeFile Callback:
In yoursaveMessageToJsonfunction, you are throwingerrin the callback, but the callback parameter is namede. You should throweto correctly report the write error.
These issues are critical because they directly impact the application's ability to run and handle data correctly. Please address them and resubmit your pull request.
Minor issues (such as typos in function names, use of max instead of maxLength in inputs, missing key props in lists, and input clearing after submission) are also present. While they don't block approval, fixing them will improve code quality and user experience.
Keep up the great work! Once the critical issues are resolved, your project will be in excellent shape. If you need any help or resources, feel free to ask! 🚀
✨ 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
| const submit = async (e: FormEvent<HTMLFormElement>) =>{ | ||
| e.preventDefault(); | ||
| onCreate(text) | ||
| } |
There was a problem hiding this comment.
After calling onCreate(text), the input field is not cleared. If the task requires the input to be reset after sending a message, you should setText('') after submission. This is a minor usability issue, but double-check the requirements or checklist to ensure compliance.
| ? ( | ||
| <form className='miniform' onSubmit={(e) => e.preventDefault()}> | ||
| <input | ||
| max={10} |
There was a problem hiding this comment.
The 'max' attribute is not valid for text input elements. To restrict the number of characters, use 'maxLength' instead (e.g., maxLength={10}). This is a minor issue but may affect the editing experience.
| if(!id) { | ||
| navigate('/rooms') | ||
| return; | ||
| } |
There was a problem hiding this comment.
Navigating with 'navigate' inside the render body is not recommended and can cause unexpected behavior. Instead, use a useEffect to perform navigation when 'id' is missing.
| <section className='section'> | ||
| {messages.map(msg => { | ||
| const isAuthor = msg.author === user?.name; | ||
| return <Message message={msg} isAuthor={isAuthor}/> |
There was a problem hiding this comment.
Each child in a list should have a unique 'key' prop. Add a key to the Message component, such as key={msg.id}, to avoid React warnings and ensure correct rendering.
| </div> | ||
| <form className='form' onSubmit={createRoom}> | ||
| <input | ||
| max={10} |
There was a problem hiding this comment.
The 'max' attribute is not valid for text input elements. To restrict the number of characters, use 'maxLength' instead (e.g., maxLength={10}). This is a minor issue but may affect the input experience.
| const deleteRoome = (roomId, callback) => { | ||
| fs.readFile(path, 'utf8', (err, data) => { | ||
| let rooms = []; | ||
|
|
||
| if (!err && data) { | ||
| try { | ||
| rooms = JSON.parse(data); | ||
| } catch (e) { | ||
| return callback(e); | ||
| } | ||
| } | ||
|
|
||
| const newRooms = rooms.filter((room) => String(room.id) !== roomId); | ||
|
|
||
| fs.writeFile(path, JSON.stringify(newRooms, null, 2), (e) => { | ||
| if (e) { | ||
| callback(e); | ||
|
|
||
| return; | ||
| } | ||
| callback(null); | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
The function name 'deleteRoome' is likely a typo. Consider renaming it to 'deleteRoom' for clarity and consistency.
| const messages = await messagesApi.getAllMessages(roomId); | ||
|
|
||
| if (!messages) { | ||
| res.statusCode(400); |
There was a problem hiding this comment.
You are using 'res.statusCode(400)', which is not a valid Express method. Use 'res.status(400)' to set the status code before sending a response.
| id, | ||
| }; | ||
|
|
||
| roomsApi.safeRoom(newRoom); |
There was a problem hiding this comment.
The function name 'safeRoom' is likely a typo. Consider renaming it to 'saveRoom' for clarity and consistency.
| id, | ||
| }; | ||
|
|
||
| roomsApi.safeRoom(updatedRoom); |
There was a problem hiding this comment.
The function name 'safeRoom' is likely a typo. Consider renaming it to 'saveRoom' for clarity and consistency.
| return; | ||
| } | ||
|
|
||
| roomsApi.deleteRoome(id, (err) => { |
There was a problem hiding this comment.
The function name 'deleteRoome' is likely a typo. Consider renaming it to 'deleteRoom' for clarity and consistency.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thank you for your hard work! Before this PR can be approved, there are some critical issues that need to be addressed. The main problem is with the structure and consistency of your messages.json file: several entries are missing required fields (id, author), use incorrect types (numeric id or roomId instead of strings), or have nested data objects instead of a flat structure. All messages must be flat objects with id (string), author (string), text, roomId (string), and date (string) as top-level properties to match your Message type and ensure reliable functionality. Please fix these issues in messages.json.
Additionally, please add a unique key prop (such as key={msg.id}) to the Message component when rendering lists in Chat.tsx to avoid React warnings. Once these critical issues are resolved, your project will be in great shape! Keep up the good work—you're almost there! 🚀
✨ 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
| <section className='section'> | ||
| {messages.map(msg => { | ||
| const isAuthor = msg.author === user?.name; | ||
| return <Message message={msg} isAuthor={isAuthor}/> |
There was a problem hiding this comment.
Issue: Each child in a list should have a unique key prop. Please add key={msg.id} to the Message component when mapping over messages to avoid React warnings and ensure correct rendering.
| [ | ||
| { | ||
| "text": "room2 dfadfa", | ||
| "id": 3, |
There was a problem hiding this comment.
Issue: The id property is a number, but your Message type requires it to be a string. Please convert all id values to strings.
| { | ||
| "text": "room2 dfadfa", | ||
| "id": 3, | ||
| "roomId": 1, |
There was a problem hiding this comment.
Issue: The roomId property is a number, but your Message type requires it to be a string. Please convert all roomId values to strings.
| { | ||
| "data": { | ||
| "author": "вафваа", | ||
| "text": "fsdfadsf", | ||
| "date": "Sat May 17 2025 18:23:26 GMT+0300 (Eastern European Summer Time)", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44" | ||
| } |
There was a problem hiding this comment.
Issue: This entry uses a nested data object and is missing the id property. All message objects should be flat and include id, author, text, roomId, and date as top-level properties.
| { | ||
| "data": { | ||
| "author": "вафваа", | ||
| "text": "dfasdfas", | ||
| "date": "Sat May 17 2025 18:45:41 GMT+0300 (Eastern European Summer Time)", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44" | ||
| } |
There was a problem hiding this comment.
Issue: This entry uses a nested data object and is missing the id property. All message objects should be flat and include id, author, text, roomId, and date as top-level properties.
| { | ||
| "author": "вафваа", | ||
| "text": "dfasdfas", | ||
| "date": "Sat May 17 2025 18:49:06 GMT+0300 (Eastern European Summer Time)", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44" |
There was a problem hiding this comment.
Issue: This entry is missing the id property required by the Message type.
| { | ||
| "id": "cc03f232-c75d-49e3-b071-8419b14c28d7", | ||
| "author": null, | ||
| "text": "dfadsfas", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:11:16" |
There was a problem hiding this comment.
Issue: The author property is null, but your Message type requires it to be a string. Please ensure all messages have a valid string for author.
| { | ||
| "id": "d8ac7157-c61d-419d-ae72-fb1990086904", | ||
| "author": null, | ||
| "text": "dfadsfasdfadsff adfasdf", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:11:23" |
There was a problem hiding this comment.
Issue: The author property is null, but your Message type requires it to be a string. Please ensure all messages have a valid string for author.
| try { | ||
| messages = JSON.parse(data); | ||
| } catch (e) { | ||
| throw new Error(e); |
There was a problem hiding this comment.
Minor issue: You are wrapping e in a new Error object, but e is already an error. You can simply throw e; instead of throw new Error(e); for more accurate error propagation.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thank you for your hard work! Before this Pull Request can be approved, there are two critical issues that need to be addressed:
-
messages.jsonStructure: The message objects inmessages.jsonare inconsistent and do not meet the core requirements. Each message must have a flat structure with top-level fields:id(string),author(string and never null),text(string),roomId(string), anddate(string). Please remove any nesteddataobjects and ensure all fields are present and correctly typed. -
Missing
keyProp in React List: In yourChat.tsxfile, theMessagecomponent is rendered in a list without a uniquekeyprop. React requires a uniquekey(such askey={msg.id}) for each element in a list to optimize rendering and prevent warnings.
Once you fix these issues, your project will meet all the core requirements! 😊 Keep going—you’re almost there! If you need more information, you can check the React docs on lists and keys and review your JSON data structure for consistency.
✨ 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
| <section className='section'> | ||
| {messages.map(msg => { | ||
| const isAuthor = msg.author === user?.name; | ||
| return <Message message={msg} isAuthor={isAuthor}/> |
There was a problem hiding this comment.
Issue: The Message component rendered in a list is missing a key prop. React requires a unique key for each element in a list to optimize rendering and avoid warnings. Please add a key={msg.id} prop to the Message component.
| { | ||
| "text": "room2 dfadfa", | ||
| "id": 3, | ||
| "roomId": 1, | ||
| "date": "29.22.1032" | ||
| }, | ||
| { | ||
| "text": "room2 dfadfa", | ||
| "id": 2, | ||
| "roomId": 3, | ||
| "date": "29.22.1032" | ||
| }, | ||
| { | ||
| "text": "room2 dfadfa", | ||
| "id": 1, | ||
| "roomId": 2, | ||
| "date": "29.22.1032" | ||
| }, | ||
| { | ||
| "data": { | ||
| "author": "вафваа", | ||
| "text": "fsdfadsf", | ||
| "date": "Sat May 17 2025 18:23:26 GMT+0300 (Eastern European Summer Time)", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44" | ||
| } | ||
| }, | ||
| { | ||
| "data": { | ||
| "author": "вафваа", | ||
| "text": "dfasdfas", | ||
| "date": "Sat May 17 2025 18:45:41 GMT+0300 (Eastern European Summer Time)", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44" | ||
| } | ||
| }, | ||
| { | ||
| "author": "вафваа", | ||
| "text": "dfasdfas", | ||
| "date": "Sat May 17 2025 18:49:06 GMT+0300 (Eastern European Summer Time)", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44" | ||
| }, | ||
| { | ||
| "id": "ec81a128-927b-40c5-964a-5e733d29c45b", | ||
| "author": "вафваа", | ||
| "text": "fasdfa edfasdf dfadfas", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "2025-05-17T16:09:52.470Z" | ||
| }, | ||
| { | ||
| "id": "294ca733-98c9-4c6a-bb60-62d7d663ee09", | ||
| "author": "вафваа", | ||
| "text": "fasdfa edfasdf dfadfas adfadf", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "2025-05-17T16:09:59.783Z" | ||
| }, | ||
| { | ||
| "id": "8ca188e7-d097-4a3a-a11e-416a34e79abd", | ||
| "author": "вафваа", | ||
| "text": "fasdfa edfasdf dfadfas adfadf", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:10:45" | ||
| }, | ||
| { | ||
| "id": "87eed957-d5b9-428f-8952-f7c3093b4ea8", | ||
| "author": "вафваа", | ||
| "text": "fadfad", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:10:53" | ||
| }, | ||
| { | ||
| "id": "cc03f232-c75d-49e3-b071-8419b14c28d7", | ||
| "author": null, | ||
| "text": "dfadsfas", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:11:16" | ||
| }, | ||
| { | ||
| "id": "d8ac7157-c61d-419d-ae72-fb1990086904", | ||
| "author": null, | ||
| "text": "dfadsfasdfadsff adfasdf", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:11:23" | ||
| }, | ||
| { | ||
| "id": "526ffc76-650b-4d1d-96f3-90ecb0375d9f", | ||
| "author": "dfadfad", | ||
| "text": "adfadfad", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:13:38" | ||
| }, | ||
| { | ||
| "id": "374f8a62-7f1b-485a-9888-c5a6ec24dcf5", | ||
| "author": "dfadfad", | ||
| "text": "adfadfad", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:14:30" | ||
| }, | ||
| { | ||
| "id": "18587b38-9a17-45ac-9da8-999932ac6838", | ||
| "author": "dfadfad", | ||
| "text": "dfadff", | ||
| "roomId": "ef5fcd41-11a0-4262-921a-e052430cce44", | ||
| "date": "19:14:46" | ||
| } |
There was a problem hiding this comment.
Critical issue: The message objects in this file are inconsistent. All messages must have a flat structure with top-level fields: id (string), author (string), text (string), roomId (string), and date (string). Remove any nested data objects and ensure all fields are present and of the correct type. Also, ensure that id is always a string and author is never null.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job! 🎉 Your code now fully meets all the core requirements and constraints for the chat application. All critical issues have been addressed: your messages.json and rooms.json files are correctly structured as empty arrays for initial state, the message objects are flat with all required fields, and your React components use unique key props when rendering lists. The API layer, type definitions, and project configuration are all well-organized and appropriate for a modern React + TypeScript + Vite project.
There are no missing or unnecessary files, and your implementation is clean and easy to follow. While there were some issues previously (such as the message structure and missing key props), you have resolved them effectively. Please review the comments for any minor suggestions or improvements, and remember to always pay close attention to data structure and React best practices in future projects.
This approval is given to help you move forward with your learning journey. If you encounter similar issues in the future, be sure to address them promptly to maintain code quality and functionality. Keep up the great work and continue learning! 🚀
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job! 🎉 Your code fully meets all the core requirements and constraints for this task. The message and room structures are correctly implemented as flat arrays, all CRUD operations work as expected, and your frontend uses the correct types and React best practices (including unique keys for list rendering). There are no missing or unnecessary files, and all critical functionality is present and working.
You can move forward with your project! Please review the comments for any suggestions or minor improvements to further polish your code. Remember, addressing feedback—especially regarding structure and best practices—will help you avoid similar issues in the future and deepen your understanding. Keep up the good work and continue learning! 🚀
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.