sol#204
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has a critical routing bug that completely breaks form submission. In src/index.html line 8, the form action is "/add-exspense" (with a typo), but the server route in src/createServer.js line 23 expects "/add-expense". Every form submission will return a 404 error because the route doesn't exist. To fix the critical issue: Change action="/add-exspense" to action="/add-expense" in your HTML file. Additionally, consider replacing the
tags used as form labels with proper elements with for attributes. While this doesn't break functionality, using semantic HTML elements improves accessibility and is the correct approach for form labels. The core logic for handling the POST request, writing to the JSON file, and returning the formatted response is otherwise correct once the route matches. Fix the typo and your form should work as expected.
✨ 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
| <title>Compression App</title> | ||
| </head> | ||
| <body> | ||
| <form method="POST" action="/add-exspense"> |
There was a problem hiding this comment.
Critical Issue: The form action has a typo - '/add-exspense' vs '/add-expense'. This route won't match the server handler on line 23, causing all form submissions to return 404.
| <body> | ||
| <form method="POST" action="/add-exspense"> | ||
| <div> | ||
| <h1>Date</h1> |
There was a problem hiding this comment.
Using <h1> for form labels is semantically incorrect. These should be <label> elements with proper for attributes linking to their inputs.
| <div> | ||
| <h1>Date</h1> | ||
| <input type="date" name="date"> | ||
| <h1>Title</h1> |
There was a problem hiding this comment.
Using <h1> for form labels is semantically incorrect. This should be a <label> element.
| <input type="date" name="date"> | ||
| <h1>Title</h1> | ||
| <input type="text" name="title"> | ||
| <h1>Amount</h1> |
There was a problem hiding this comment.
Using <h1> for form labels is semantically incorrect. This should be a <label> element.
| const fs = require('fs'); | ||
|
|
||
| const querystring = require('querystring'); | ||
|
|
There was a problem hiding this comment.
Typo in form action: '/add-exspense' should be '/add-expense' to match the server route on line 23. The form will submit to a non-existent endpoint, triggering a 404 response.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. All critical issues from the previous review have been successfully resolved: the form action typo has been fixed to match the server route, and proper semantic <label> elements with for attributes are now in place. The implementation correctly handles POST requests to /add-expense, validates required fields (date, title, amount), saves data to db/expense.json, and returns well-formatted HTML with JSON output. All task requirements are met and no remaining issues exist.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.