Skip to content

add solution#142

Open
clavigo wants to merge 3 commits intomate-academy:masterfrom
clavigo:develop
Open

add solution#142
clavigo wants to merge 3 commits intomate-academy:masterfrom
clavigo:develop

Conversation

@clavigo
Copy link
Copy Markdown

@clavigo clavigo commented Apr 17, 2025

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Hey there! 👋 Thanks for your submission. I've noticed a couple of things we need to address before merging. First, the form's action attribute is pointing to /submit-expense, but your server is set up to listen for POST requests at /add-expense. This mismatch will cause the form submission to fail. 😫 Please update the form's action attribute to /add-expense. Secondly, the task asks for an HTML page with well-formatted JSON, but currently, you're returning a JSON string directly. Could you wrap that JSON within an HTML structure? 💡 Once these adjustments are made, it'll be good to go!


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/createServer.js
/* Write your code here */
// Return instance of http.Server class
return http.createServer(async (req, res) => {
if (req.method === 'POST' && req.url === '/add-expense') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The server listens for POST requests to /add-expense, but the form submits to /submit-expense. This will cause the form submission to fail.

Comment thread src/createServer.js Outdated
fs.writeFileSync('db/expense.json', result);

res.writeHead(200, { 'Content-Type': 'application/json' });
res.end(result);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task description asks to return an HTML page with well-formatted JSON, but the code returns a JSON string directly. You should wrap the JSON in an HTML page.

Comment thread src/createServer.js Outdated
res.statusCode = 200;
res.setHeader('Content-type', 'text/html');

res.end(`<form method="POST" action="/submit-expense" enctype="multipart/form-data">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The form's action attribute is set to /submit-expense, but the server is listening for POST requests to /add-expense. This will cause the form submission to fail. Make sure that the form submits to the correct URL.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Thank you for your work! There are two critical issues that need to be addressed before your solution can be approved:

  1. Form Data Parsing: The server currently tries to parse the POST body as JSON, but HTML forms submit data as URL-encoded form data by default. This mismatch will cause parsing errors. You should update your server to correctly parse URL-encoded form data (for example, by using the querystring module or similar logic).
  2. Response Format: After form submission, your server responds with raw JSON and a Content-Type: application/json header. The task requires returning an HTML page that displays the submitted expense as well-formatted JSON (e.g., inside a <pre> tag), not just the JSON string.

Please address these issues so your solution meets the core requirements. You're on the right track—fixing these two points will get your submission ready for approval! 😊 If you need help, check out the Node.js documentation on parsing form data and generating HTML responses.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/createServer.js Outdated
});

req.on('end', () => {
const expense = JSON.parse(body);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: The code attempts to parse the request body as JSON, but the HTML form submits data as URL-encoded form data, not JSON. This will cause a parsing error. You need to parse the body as URL-encoded data or change the form to submit JSON (which is not typical for HTML forms).

Comment thread src/createServer.js Outdated
Comment on lines +34 to +35
res.setHeader('Content-Type', 'application/json');
res.end(file);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: The response to the POST request is sent as raw JSON with 'application/json' content type. According to the task, you should return an HTML page with the JSON data formatted within it, not just the JSON string.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Great work on implementing the core requirements of the task! 🎉 Your server correctly serves the HTML form on GET requests and parses URL-encoded form data from POST requests, which shows a solid understanding of handling HTTP requests and form submissions. However, there is one critical issue that needs your attention: after a successful POST, the server currently responds with raw JSON and a Content-Type: application/json header. According to the task requirements, the response should be an HTML page that displays the submitted expense as formatted JSON inside a <pre> tag, not as raw JSON. This is important for user experience and for meeting the specification.

I am approving your pull request to help you move forward with your learning process. Please make sure to review the comments and address this issue in your next iteration. Fixing the POST response to return HTML with the formatted JSON will ensure your solution fully meets the requirements. Keep up the good work, and remember that attention to detail in response formats is crucial for web development tasks! If you need more information, you can check out the MDN documentation on HTTP content types and serving HTML responses.

Keep learning and improving—you're almost there! 🚀


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread src/createServer.js
Comment on lines +53 to +55
res.writeHead(200, { 'Content-Type': 'application/json' });

res.end(JSON.stringify(expense));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: The response after a successful POST is sent as raw JSON with a Content-Type: application/json header. According to the task description, you should return an HTML page that displays the submitted expense as well-formatted JSON (e.g., inside a <pre> tag), not just the JSON string. Please update the response to return HTML as required by the task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants