Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This submission requires a couple of changes to meet the task requirements. You've done a great job setting up the server and parsing the form data. However, there are two key issues in src/createServer.js that need to be addressed.
- Data is overwritten: The
db/expense.jsonfile is overwritten on every submission, losing all previous data. The goal is to accumulate expenses. To fix this, you should first read the existing content of the file, add the new expense to the array, and then write the entire updated array back to the file. - Incorrect response format: The server returns raw JSON, but the requirement is to return an HTML page that displays the JSON. You'll need to set the
Content-Typeheader totext/htmland send back an HTML string that includes the formatted JSON, for example, inside a<pre>tag.
✨ 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
| <form action="/add-expense" | ||
| method="POST" | ||
| enctype="multipart/form-data" | ||
| > |
There was a problem hiding this comment.
This form is set up correctly to send data. However, the backend logic in createServer.js that handles this submission has two issues that violate the task requirements:
- The
db/expense.jsonfile is overwritten on every submission, losing all previous data. It should accumulate expenses in an array. - The server returns raw JSON, but the requirement is to return an 'HTML page with well formatted JSON'.
Please adjust the server-side code to fix these issues.
src/createServer.js
Outdated
| let expenses = {}; | ||
|
|
||
| expenses = { date, title, amount }; | ||
|
|
||
| fs.writeFileSync(filePath, JSON.stringify(expenses, null, 2)); |
There was a problem hiding this comment.
This logic overwrites db/expense.json on every submission. To save a history of expenses, you should read the current contents of the file, add the new expense to the list, and then write the entire list back. Remember to handle cases where the file might not exist or be empty initially.
src/createServer.js
Outdated
| res.writeHead(200, { 'Content-Type': 'application/json' }); | ||
|
|
||
| return res.end(JSON.stringify(expenses)); |
There was a problem hiding this comment.
The task asks to 'return an HTML page with well formatted JSON'. This implementation returns a raw JSON response (application/json). To meet the requirement, you should set the Content-Type to text/html and send back an HTML string that includes the formatted JSON, perhaps inside a <pre> tag.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job fixing the data accumulation logic! Your solution is very close to complete. However, one change is needed before this can be approved. While the response body for the POST request is correctly formatted as an HTML string, the Content-Type header is set to application/json. This prevents the browser from rendering the response as an HTML page, which is a core requirement. Please update the header in src/createServer.js to text/html to ensure the browser displays the page correctly.
✨ 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
src/createServer.js
Outdated
|
|
||
| fs.writeFileSync(filePath, JSON.stringify(expenses, null, 2)); | ||
|
|
||
| res.writeHead(200, { 'Content-Type': 'application/json' }); |
There was a problem hiding this comment.
You're sending an HTML string in the response body, which is correct according to the requirements. However, this Content-Type header tells the browser it's receiving JSON. To fully meet the requirement of returning an HTML page, this header should be set to text/html.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this task! You've successfully fixed the Content-Type header, and the server's core functionality is spot on. I am approving your solution as an educational override to help you keep moving forward. There is just one minor improvement needed to fully meet all requirements: the JSON returned in the HTML response should be pretty-printed for better readability. You can achieve this in src/createServer.js by using the space argument in JSON.stringify, just like you did when writing to the file (e.g., JSON.stringify(expenses, null, 2)). Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.