Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great start on setting up the Node.js server and handling the form submission! However, this requires changes as two core requirements are not yet met.
- Data Persistence: Each new expense submission overwrites the
db/expense.jsonfile instead of adding to it. To fix this, you should read the file's content, parse it as an array, add the new expense object, and then write the entire array back to the file. - Response Format: After a successful submission, the server must return an HTML page that displays the submitted JSON. Currently, it returns raw JSON (
application/json). Please change theContent-Typeheader totext/htmland wrap the JSON output in a basic HTML structure, for example, using a<pre>tag for good formatting.
Addressing these points will get your project aligned with the requirements.
✨ 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
| return; | ||
| } | ||
|
|
||
| fs.writeFileSync(dataPath, JSON.stringify(parsed, null, 2)); |
There was a problem hiding this comment.
This line overwrites the expense.json file with only the latest submission. To save all expenses, you should first read the existing data from the file, add the new expense to the collection (likely an array), and then write the entire collection back to the file.
src/createServer.js
Outdated
| res.setHeader('Content-Type', 'application/json'); | ||
| res.end(JSON.stringify(parsed)); |
There was a problem hiding this comment.
The requirement is to 'return an HTML page with well formatted JSON'. Currently, you are returning a response with Content-Type: application/json, which is just the raw JSON data. Consider wrapping the JSON output in a basic HTML structure (e.g., using <pre> tags) and setting the Content-Type header to text/html.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Good effort on setting up the server and handling the form submission. However, this submission requires a few changes as two core requirements from the task description are not yet met.
-
Data Persistence: The
db/expense.jsonfile is being overwritten with each new submission instead of having the new expense added to it. To fix this, you'll need to first read the existing content of the file, parse it into a JavaScript array, push the new expense object into that array, and then write the entire updated array back to the file. -
Response Format: After a successful submission, the server should respond with an HTML page, not raw JSON. Please change the
Content-Typeheader totext/htmland send back an HTML document that displays the submitted data, for example, within a<pre>tag for nice formatting.
Please address these two points to complete the task. You're close!
✨ 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
| return; | ||
| } | ||
|
|
||
| fs.writeFileSync(dataPath, JSON.stringify(parsed, null, 2)); |
There was a problem hiding this comment.
This line overwrites expense.json with only the new expense, but the requirement is to add to it. You need to first read the file's content, parse it as an array, push the new parsed expense into that array, and then write the entire updated array back to the file. Remember to handle cases where the file might not exist or is empty.
src/createServer.js
Outdated
| res.setHeader('Content-Type', 'application/json'); | ||
| res.end(JSON.stringify(parsed)); |
There was a problem hiding this comment.
The task requires returning an HTML page after submission, not raw JSON. The Content-Type header should be text/html. The response body should be an HTML document that includes the submitted JSON, preferably inside a <pre> tag for proper formatting.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving this submission as an educational override to help you move forward, but the two main issues from the previous review still need to be addressed. You've done a great job setting up the server to handle the form submission!
Here are the key things to fix:
- Data Persistence: The
expense.jsonfile is being overwritten with each submission. You'll need to read the file first, add the new expense to the existing data, and then write the complete, updated data back to the file. - Response Format: The server is responding with raw JSON. The requirement is to send back a full HTML page that displays the submitted data, making sure to set the
Content-Typeheader totext/html.
You're on the right track, keep up the great effort!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| return; | ||
| } | ||
|
|
||
| fs.writeFileSync(dataPath, JSON.stringify(expense, null, 2)); |
There was a problem hiding this comment.
This line overwrites the expense.json file with only the new expense. The requirement is to add the new expense to the existing ones.
You'll need to first read the file's content (e.g., with fs.readFileSync), parse the JSON into an array, push the new expense into that array, and then write the updated array back to the file.
| res.setHeader('Content-Type', 'application/json'); | ||
| res.end(JSON.stringify(expense)); |
There was a problem hiding this comment.
The response here is raw JSON. According to the task description, you need to respond with an HTML page.
The Content-Type header should be text/html, and the body should be an HTML document that displays the submitted JSON, for example, inside a <pre> tag for nice formatting.
2 participants
No description provided.