Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job setting up the server to handle the POST request and save the expense data! However, a few changes are needed to meet all the requirements.
Here are the key areas to focus on:
- Serving the HTML Form: The server is missing the functionality to display the initial form. You'll need to add a handler for a GET request (e.g., for the root path
/) that reads and serves the HTML form file. - Data Persistence: Your current implementation overwrites
db/expense.jsonwith every new expense. To save all expenses, you'll need to first read the existing content of the file, parse it into an array, add the new expense, and then write the updated array back. - Response Format: The task requires returning an HTML page that displays the saved JSON. Currently, you're sending a raw JSON response. Please update the
Content-Typeheader totext/htmland wrap the JSON string in basic HTML, like<pre>tags, so it renders correctly in the browser.
Addressing these points will complete the core functionality. You're on the right track!
✨ 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
| const db = path.join(__dirname, '../db/expense.json'); | ||
| const stringifiedData = JSON.stringify(parsedData); | ||
|
|
||
| fs.writeFileSync(db, stringifiedData); |
There was a problem hiding this comment.
This implementation overwrites the expense.json file with every new request. As a result, only the last submitted expense will be saved. To store all expenses, you should first read the existing content of the file, parse it as an array, add the new expense object to that array, and then write the updated array back to the file.
src/createServer.js
Outdated
| response.setHeader('Content-Type', 'application/json'); | ||
| response.end(JSON.stringify(parsedData)); |
There was a problem hiding this comment.
The task requires returning an HTML page that contains the JSON data. Currently, you are sending a response with the Content-Type set to application/json and a raw JSON string as the body. You should set the content type to text/html and wrap the JSON string in basic HTML tags, like <pre>, to display it correctly on a page.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great progress on this task! You've successfully set up the server to handle GET and POST requests and are correctly formatting the HTML response. This requires changes because the core data-saving functionality is not yet correct. Currently, each new expense overwrites the db/expense.json file, meaning only the last submitted expense is ever saved. To fix this, you need to implement a 'read-modify-write' pattern: first read the file's contents, parse the JSON into an array, add the new expense, and then write the entire updated array back to the file. After fixing this, ensure the response displays the complete list of all saved expenses. You're very close to finishing!
✨ 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 dbPath = path.join(__dirname, '../db/expense.json'); | ||
|
|
||
| fs.writeFile(dbPath, JSON.stringify(parsedData, null, 2), (error) => { |
There was a problem hiding this comment.
Using fs.writeFile here overwrites the file with every new expense. The requirement is to save all expenses. You'll need to first read the contents of db/expense.json, parse it into an array, add the new expense to that array, and then write the entire updated array back to the file.
| response.setHeader('Content-Type', 'text/html'); | ||
|
|
||
| response.end(` | ||
| <pre>${JSON.stringify(parsedData, null, 2)}</pre> |
There was a problem hiding this comment.
Great job wrapping the JSON in <pre> tags! However, the response should display all the expenses that are saved in the file, not just the one that was newly added. After you fix the file-saving logic, you should have the full array of expenses available to stringify and send here.
2 participants
No description provided.