Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Good work on setting up the server and handling the form data. Changes are needed to meet two core requirements. First, the expense.json file is being overwritten on each submission instead of accumulating expenses. You'll need to read the file, add the new record to the existing list, and write the updated list back. Second, the server should return an HTML page displaying the JSON, not a raw JSON response. Please set the Content-Type to text/html and wrap the JSON in a <pre> tag for proper browser display.
✨ 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
|
|
||
| // arr.push(record); | ||
|
|
||
| fs.writeFileSync(filePath, JSON.stringify(record, null, 2)); |
There was a problem hiding this comment.
The current implementation overwrites the expense.json file with only the latest record. To meet the requirement of saving the expense data, you should append new expenses to the existing list. This typically involves reading the file, parsing the JSON into an array, adding the new record, and then writing the updated array back to the file.
Also, consider what the initial content of the file should be on line 83. If you plan to store a list of expenses, an empty array [] would be more suitable than an empty object {}.
| res.writeHead(200, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify(expenseData)); |
There was a problem hiding this comment.
The requirement is to return an HTML page containing well-formatted JSON. This implementation sends a raw JSON response with the Content-Type set to application/json. You should change the Content-Type to text/html and wrap the JSON string in HTML tags (e.g., <pre>) to display it correctly on a web page.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job addressing the previous feedback on the server's response! I am requesting a change for one remaining critical issue. Currently, when a new expense is submitted, it overwrites the db/expense.json file instead of adding to it. This means only the last submitted expense is ever stored.
To meet the requirement of accumulating expenses, you'll need to:
- Read the current contents of
db/expense.json. - Parse the JSON string into a JavaScript array.
- Push the new expense object into that array.
- Write the entire updated array back to the file.
You're very close to finishing this task. Keep up the great work!
✨ 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
| amount: expenseData.amount, | ||
| }; | ||
|
|
||
| fs.writeFileSync(filePath, JSON.stringify(record, null, 2)); |
There was a problem hiding this comment.
This line overwrites the file with only the new record. According to the requirements, you need to accumulate expenses. The correct logic would be to read the file, parse the JSON array, push the new record into it, and then write the entire updated array back to the file.
3 participants
No description provided.