done#158
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! There are a few critical issues that need to be addressed before your solution can be approved:
- The server expects JSON data, but the form submits data as
application/x-www-form-urlencoded. This mismatch will prevent the server from correctly processing form submissions. - Each new expense overwrites the previous data in
db/expense.jsoninstead of storing multiple expenses as required. - There is no error handling for file write operations, so the client won't be notified if something goes wrong.
Please fix these issues so your app meets all the core requirements. You're on the right track—keep going! 🚀
✨ 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
| return; | ||
| } | ||
|
|
||
| const writeStream = fs.createWriteStream(path.resolve('db/expense.json')); |
There was a problem hiding this comment.
This line creates a write stream that will overwrite 'db/expense.json' with each new request. If the task requires storing multiple expenses, you should append the new expense to the existing data instead of overwriting the file.
|
|
||
| const writeStream = fs.createWriteStream(path.resolve('db/expense.json')); | ||
|
|
||
| writeStream.end(text); |
There was a problem hiding this comment.
There is no error handling for the write stream. If an error occurs while writing to the file, the client will not be informed. Consider adding an 'error' event handler for the write stream to handle such cases.
| </head> | ||
| <body> | ||
| <h1>Expense Form</h1> | ||
| <form action="http://localhost:5701/add-expense" method="POST"> |
There was a problem hiding this comment.
The form uses the default 'application/x-www-form-urlencoded' encoding, but your server expects JSON data in the request body. You need to either update the server to handle URL-encoded data or use JavaScript on the client side to send JSON via fetch/AJAX.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thanks for your hard work! There are a few critical issues that need to be addressed before your solution meets the core requirements: the server does not handle form submissions correctly (it expects JSON, but the form sends URL-encoded data), expenses are overwritten instead of appended, and there is no error handling for file operations. Please update your code to handle form data properly, store all expenses in an array, and add error handling for file writes. You're almost there—keep going! 🚀
✨ 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
| req.on('end', () => { | ||
| const text = Buffer.concat(chunks).toString(); | ||
|
|
||
| const data = JSON.parse(text); |
There was a problem hiding this comment.
Issue: The server expects the request body to be JSON, but if the client sends data as application/x-www-form-urlencoded (which is common for HTML forms), JSON.parse(text) will throw an error. You need to handle both content types or ensure the client sends JSON as required by the task.
| return; | ||
| } | ||
|
|
||
| const writeStream = fs.createWriteStream(path.resolve('db/expense.json')); |
There was a problem hiding this comment.
Issue: Writing the incoming data directly to db/expense.json will overwrite any existing expenses. According to the requirements, you should store multiple expenses. You need to read the existing file, append the new expense, and write the updated array back.
|
|
||
| const writeStream = fs.createWriteStream(path.resolve('db/expense.json')); | ||
|
|
||
| writeStream.end(text); |
There was a problem hiding this comment.
Issue: There is no error handling for file write operations. If writing to the file fails, the client will not be notified. You should handle errors and respond with an appropriate status code and message.
| </head> | ||
| <body> | ||
| <h1>Expense Form</h1> | ||
| <form action="http://localhost:5701/add-expense" method="POST"> |
There was a problem hiding this comment.
Issue: The form submits data as application/x-www-form-urlencoded by default, but your server expects JSON. This will cause the server to fail when parsing the request body. You need to either update the form to send JSON (using JavaScript and fetch/AJAX) or update the server to handle URL-encoded form data.
2 participants
No description provided.