Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Good work setting up the server and handling the POST request to save the expense data. However, a couple of key requirements are missing, so this will need a few changes before it can be approved.
Here are the two main issues:
- Missing HTML Form: The application must show an HTML form for users to input data. Your server currently only handles POST requests. You'll need to add a handler for
GETrequests that serves the HTML form file. - Incorrect Success Response: The server is returning raw JSON after a successful submission. The requirement is to return an "HTML page with well formatted JSON". To fix this, please set the
Content-Typeheader totext/htmland wrap the JSON response within an HTML structure, for example, inside a<pre>tag.
You're very close to a complete solution! Address these points and you'll be all set.
✨ 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 instance of http.Server class | ||
| const server = new Server(); | ||
|
|
||
| server.on('request', (req, res) => { |
There was a problem hiding this comment.
The request handler is missing a crucial part of the requirements. The application must also 'show an HTML form'. You should add logic here to handle GET requests to serve this form to the user.
src/createServer.js
Outdated
| res.setHeader('Content-Type', 'application/json'); | ||
| res.end(JSON.stringify(expense, null, 2)); |
There was a problem hiding this comment.
According to the task description, the server should 'return an HTML page with well formatted JSON'. This implementation returns raw JSON. You should set the Content-Type to text/html and wrap the JSON string in HTML tags (e.g., inside a <pre> tag).
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job adding the index.html file! This is a big step forward. However, the server isn't yet configured to use it, and there are a few critical issues with data handling that prevent the application from working as required, so some changes are needed.
Here are the main things to address:
- Serve the HTML Form: The server needs to handle
GETrequests to the root path (/) and respond with the content of yourindex.htmlfile so users can see the form. - Parse Form Data: Your server expects JSON, but the HTML form sends data as
application/x-www-form-urlencoded. This will cause a crash when you useJSON.parse(). You'll need to parse this form data first—Node's built-inURLSearchParamsis great for this. - Save Valid JSON: After parsing the form data into an object, you must use
JSON.stringify()to convert it back into a JSON string before writing it todb/expense.json. - Return an HTML Page: On a successful submission, the server must send back an HTML document that displays the saved JSON, not just the raw JSON data. Remember to set the
Content-Typeheader totext/html.
You're very close to a complete solution. 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
src/createServer.js
Outdated
| const server = new http.Server(); | ||
|
|
||
| server.on('request', (req, res) => { | ||
| if (req.url !== '/add-expense') { |
There was a problem hiding this comment.
This routing logic only checks for a single URL and doesn't distinguish between HTTP methods like GET and POST. To meet the requirement of showing an HTML form, you'll need to handle GET requests (for example, to the root path /) to serve the index.html file. The form submission itself should be handled on a POST request to /add-expense.
src/createServer.js
Outdated
| const data = JSON.parse(text); | ||
|
|
||
| if (!data['date'] || !data['title'] || !data['amount']) { | ||
| res.statusCode = 400; | ||
| res.end('Not full data'); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const writeStream = fs.createWriteStream(path.resolve('db/expense.json')); | ||
|
|
||
| writeStream.end(text); |
There was a problem hiding this comment.
The server expects JSON, but a standard HTML form sends data in application/x-www-form-urlencoded format. JSON.parse(text) will cause an error with this data. You'll need to parse the urlencoded string first (you can use the built-in URLSearchParams class or the querystring module). After parsing, you should construct a JavaScript object from the form data, stringify it to JSON, and then save that JSON string to the file, not the original raw text from the request.
src/createServer.js
Outdated
| res.setHeader('Content-type', 'application/json'); | ||
| res.statusCode = 200; | ||
| res.end(text); |
There was a problem hiding this comment.
The requirements state that the server must return an HTML page with the formatted JSON. The Content-Type should be text/html, and the response body should be an HTML document containing the JSON data (for example, inside a <pre> tag to preserve formatting). This was also mentioned in the previous review feedback.
src/index.html
Outdated
| </head> | ||
| <body> | ||
| <h1>Expense Form</h1> | ||
| <form action="http://localhost:5701/add-expense" method="POST"> |
There was a problem hiding this comment.
This form submits data in application/x-www-form-urlencoded format by default. However, your server code in createServer.js expects application/json because it uses JSON.parse(). To fix this, you'll need to use JavaScript to intercept the form's submit event, create a JSON object from the form data, and then send it using the Fetch API with the correct Content-Type header.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can continue making progress. You've done a great job setting up the server to display the HTML form! However, the form submission is not yet working due to a couple of key issues.
The main problem is that the server tries to parse the form data as JSON, but it's sent in application/x-www-form-urlencoded format. You'll need to use a different method to parse this, such as URLSearchParams. Secondly, the success response after saving the data should be a full HTML page displaying the saved JSON, with the Content-Type header set to text/html.
Addressing these two points will make your application fully functional. Keep up the fantastic effort!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| let fields; | ||
|
|
||
| try { | ||
| fields = JSON.parse(body); |
There was a problem hiding this comment.
This parsing method isn't correct for the data sent by the HTML form. Forms using POST send data in application/x-www-form-urlencoded format (like date=...&title=...), which is not valid JSON. JSON.parse() will throw an error here.
You'll need to parse this string differently. Consider using Node's built-in URLSearchParams to handle this format.
| res.setHeader('Content-Type', 'application/json'); | ||
| res.end(file); |
There was a problem hiding this comment.
The response for a successful submission needs two changes to meet the requirements:
- The
Content-Typeheader should betext/html. - The response body should be an HTML document that displays the saved JSON, not the raw JSON data itself. For example:
res.end(${file});
| return; | ||
| } | ||
|
|
||
| if (req.url !== '/' && req.url !== '/add-expense') { |
There was a problem hiding this comment.
This condition correctly routes GET / requests to show the form, but it also allows GET /add-expense to do the same, which is likely not intended. It's generally better to handle each route explicitly. For instance, you could check for req.method === 'GET' && req.url === '/' to serve the form, and then let other unhandled routes result in a 404.
No description provided.