add task solution for node_form-data#155
add task solution for node_form-data#155Natblistru wants to merge 4 commits intomate-academy:masterfrom
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thank you for your work! There are a couple of critical issues that need to be addressed before your code can be approved. Most importantly, your server currently overwrites the entire expense.json file with each new expense, so only the latest entry is saved—previous expenses are lost. For an expense tracker, you should read the existing data from expense.json, append the new expense to the array, and then write the updated array back to the file. Additionally, your implementation is missing the HTML form for users to submit expenses, which is a core requirement. Please update your code to serve an HTML form and ensure all expenses are preserved in the JSON file. Once these are fixed, your solution will be much closer to meeting all 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
| const filePath = path.join(__dirname, '../db/expense.json'); | ||
|
|
||
| try { | ||
| fs.writeFileSync(filePath, JSON.stringify(expense, null, 2), 'utf8'); |
There was a problem hiding this comment.
Issue: The code overwrites the entire 'expense.json' file with the new expense object each time a POST request is made. This means only the latest expense is saved, and all previous data is lost. If the task requires storing multiple expenses, you should read the existing data, append the new expense, and then write the updated array back to the file.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thank you for your hard work! Before this can be approved, you need to address a critical requirement: your server must serve an HTML form (e.g., on GET /) so users can submit expenses via a browser. This is a core part of the task and currently missing. Please add this functionality. Also, while not critical, updating your test descriptions to match the actual endpoint (/add-expense) would improve clarity. Keep going—you’re very 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
| res.statusCode = 404; | ||
| res.end('Not Found'); |
There was a problem hiding this comment.
Issue: The server does not serve an HTML form for submitting expenses, which is a core requirement of the task. You should add a handler for GET requests (e.g., to /) that responds with an HTML form allowing users to enter expense details and submit them.
| @@ -40,7 +40,7 @@ describe('Form Data Server', () => { | |||
| }); | |||
|
|
|||
| it('should save data for valid expense on "POST /submit-expense" request', async () => { | |||
There was a problem hiding this comment.
Issue: The test description refers to 'POST /submit-expense', but the actual endpoint being tested is '/add-expense'. Please update the description to accurately reflect the tested endpoint.
| expect(savedData).toContainEqual(expense); | ||
| }); | ||
|
|
||
| it('should reject request without all params on "POST /submit-expense" request', async () => { |
There was a problem hiding this comment.
Issue: The test description refers to 'POST /submit-expense', but the actual endpoint being tested is '/add-expense'. Please update the description to accurately reflect the tested endpoint.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Thank you for your submission! 🎉 You have made solid progress by implementing an HTML form at /, ensuring the correct fields and endpoints, and covering the required scenarios in your tests. However, there is a critical issue that needs your attention: your POST /add-expense handler only parses JSON input, while the HTML form submits data as application/x-www-form-urlencoded. This means that form submissions from the browser will not be processed correctly, and users will always receive an "Invalid JSON format" error. To fully meet the requirements, you need to add support for parsing form-encoded data in your POST handler.
Additionally, there are minor inconsistencies in your test descriptions (some refer to /submit-expense instead of /add-expense). While this doesn't affect functionality, updating these descriptions will help future readers and maintainers.
Despite these issues, the decision is APPROVE to help you move forward with your learning. Please review the comments and address the critical blocker regarding form data parsing as soon as possible. Handling both JSON and form-encoded data is a common requirement in web servers, so this is a great opportunity to deepen your understanding. Keep up the good work, and don't hesitate to consult the Node.js documentation on handling form data or consider using libraries like querystring or body-parser for easier parsing.
Remember: addressing critical issues like this is essential for building robust and user-friendly applications. Good luck, and keep improving your code! 🚀
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| if (req.method === 'POST' && url.pathname === '/add-expense') { | ||
| const chunks = []; | ||
|
|
||
| for await (const chunk of req) { | ||
| chunks.push(chunk); | ||
| } | ||
|
|
||
| const formData = Buffer.concat(chunks).toString(); | ||
|
|
||
| let expense; | ||
|
|
||
| try { | ||
| expense = JSON.parse(formData); | ||
| } catch (err) { | ||
| res.statusCode = 400; | ||
|
|
||
| return res.end('Invalid JSON format'); | ||
| } | ||
|
|
||
| const { date, title, amount } = expense; | ||
|
|
||
| if (!date || !title || !amount) { | ||
| res.statusCode = 400; | ||
|
|
||
| return res.end('Missing required fields'); | ||
| } | ||
|
|
||
| const filePath = path.join(__dirname, '../db/expense.json'); | ||
|
|
||
| let expenses = []; | ||
|
|
||
| try { | ||
| const fileContent = fs.readFileSync(filePath, 'utf8'); | ||
|
|
||
| expenses = JSON.parse(fileContent); | ||
|
|
||
| if (!Array.isArray(expenses)) { | ||
| expenses = []; | ||
| } | ||
| } catch (err) { | ||
| // Dacă fișierul nu există sau e invalid, începem cu un array gol | ||
| expenses = []; | ||
| } | ||
|
|
||
| expenses.push(expense); | ||
|
|
||
| try { | ||
| fs.writeFileSync(filePath, JSON.stringify(expenses, null, 2), 'utf8'); | ||
| } catch (err) { | ||
| res.statusCode = 500; | ||
|
|
||
| return res.end('Failed to save data'); | ||
| } | ||
|
|
||
| res.statusCode = 200; | ||
| res.setHeader('Content-Type', 'application/json'); | ||
|
|
||
| return res.end(JSON.stringify(expense, null, 2)); |
There was a problem hiding this comment.
Issue: The POST /add-expense handler expects the request body to be JSON, but the HTML form on / submits data as application/x-www-form-urlencoded by default. This will cause the handler to always return 'Invalid JSON format' when submitting via the form. You need to parse form-encoded data in addition to JSON to accept submissions from the HTML form.
2 participants
No description provided.