Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great start on setting up the server and handling the form data! However, a couple of changes are needed to meet the project's core requirements.
- Expense data is being overwritten. Each time you submit the form,
db/expense.jsonis replaced with only the new entry. The application should accumulate expenses. To fix this, you need to read the existing file content, add the new expense to the list, and then write the entire updated list back to the file. - The success response format is incorrect. The server is currently sending a raw JSON response. The task requires you to return an HTML page that contains the well-formatted JSON. You should set the
Content-Typeheader totext/htmland embed the JSON string within HTML tags, for example, inside a<pre>tag.
Addressing these points will get your solution in line with the requirements. 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
| res.setHeader('Content-Type', 'application/json'); | ||
| res.end(JSON.stringify({ date, title, amount })); |
There was a problem hiding this comment.
The requirement is to 'return an HTML page with well formatted JSON'. Your code is returning a response with Content-Type: application/json, which is not an HTML page. You should set the content type to text/html and wrap the JSON string in HTML tags (e.g., inside a <pre> tag) to create a valid HTML page.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on updating the response to be a proper HTML page! That's a solid improvement. However, this requires changes because the core functionality of accumulating expenses is not yet implemented. Currently, each new expense submission overwrites the db/expense.json file, erasing all previous entries. The requirement is to build a list of all expenses. To fix this, you'll need to modify the logic in src/createServer.js: 1. Read the current contents of db/expense.json before writing to it. 2. Parse the JSON string into an array (if the file is empty, start with an empty array []). 3. Add the new expense object to this array. 4. Write the entire updated array back to the file.
✨ 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
| fs.writeFileSync( | ||
| dbPath, | ||
| JSON.stringify({ date, title, amount }, null, 2), | ||
| ); |
There was a problem hiding this comment.
This logic overwrites the db/expense.json file with only the new expense, but the requirement is to accumulate all expenses.
To fix this, you need to:
- Read the contents of
db/expense.jsonbefore this step. - Parse the JSON content into an array. If the file is empty or doesn't exist, you can start with an empty array
[]. - Add the new expense object to this array.
- Finally, write the entire updated array back to the file.
There was a problem hiding this comment.
@mateacademy-ai-mentor
I have reviewed your comments, and I'd like to clarify a few points that were misunderstood:
- There are no requirements to accumulate all expenses.
Implement an app that
- shows an HTML form with an info about an expense (date, title and amount)
- receives its data in a POST request
- save it to a `db/expense.json` file
- and return an HTML page with well-formatted JSON
- According to the test, it should overwrite the existing file content.
it('should save data for valid expense on "POST /submit-expense" request', async () => {
fs.writeFileSync(dataPath, JSON.stringify({}));
const expense = {
date: '2024-01-25',
title: 'Test Expense',
amount: '100',
};
const response = await axios.post(`${HOST}/add-expense`, expense);
expect(response.status).toBe(200);
const savedData = JSON.parse(fs.readFileSync(dataPath));
expect(savedData).toStrictEqual(expense);
});
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
No description provided.