Skip to content

Develop #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
OlegMysko wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Develop #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1c26c2b
solution
OlegMysko Sep 23, 2025
173c51a
solutio
OlegMysko Sep 23, 2025
1e07f11
solutio
OlegMysko Sep 23, 2025
d3be6f5
solutio
OlegMysko Sep 23, 2025
6f4f672
solutio
OlegMysko Sep 23, 2025
790db02
se
OlegMysko Sep 23, 2025
dea5059
se
OlegMysko Sep 23, 2025
dfd1308
aa
OlegMysko Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 76 additions & 5 deletions src/createServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,81 @@
'use strict';

const http = require('http');
const fs = require('fs');
const path = require('path');

const DATA_PATH = path.resolve('db', 'expense.json');

function createServer() {
/* Write your code here */
// Return instance of http.Server class
return http.createServer((req, res) => {
const indexPath = path.resolve('src', 'index.html');


if (req.method === 'GET' && req.url === '/') {
try {
const file = fs.readFileSync(indexPath);

res.writeHead(200, { 'Content-Type': 'text/html' });
res.end(file);
} catch {
res.writeHead(404, { 'Content-Type': 'text/plain' });
res.end('file not found');
}

return;
}


if (req.method === 'POST' && req.url === '/add-expense') {
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

This route handler is set up for the path /add-expense, but the form in src/index.html is configured to submit data to a different URL (/expense). These paths need to match for the server to process the form submission.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

There's a mismatch between the endpoint you're listening to here (/add-expense) and the one the client-side code is sending its request to. In src/index.html, the fetch call is made to /submit-expense.

For the form submission to work, these two URLs must be identical.

const chunks = [];

req.on('data', (chunk) => chunks.push(chunk));

req.on('end', () => {
try {
const buffer = Buffer.concat(chunks).toString();
const obj = JSON.parse(buffer);

if (!obj.date || !obj.title || !obj.amount) {
res.writeHead(400, { 'Content-Type': 'text/plain' });
res.end('Invalid expense data');

return;
}


fs.writeFileSync(DATA_PATH, JSON.stringify(obj, null, 2));
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

This is the same issue from the previous review. This line overwrites expense.json with only the single new expense object, deleting all previous entries. The goal is to maintain a list (an array) of all expenses.

To fix this, you need to:

  1. Read the existing content from DATA_PATH.
  2. If the file has content, parse the JSON into a JavaScript array. If the file is empty or doesn't exist, start with a new empty array ([]).
  3. Add the new expense object (obj) to this array (e.g., using .push()).
  4. Convert the entire updated array back into a JSON string.
  5. Write this new JSON string to the file, overwriting the old content with the complete list.


res.writeHead(200, { 'Content-Type': 'application/json' });
res.end(JSON.stringify(obj));
Comment on lines +49 to +50
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

The task requires returning an HTML page that displays the well-formatted JSON from the expense.json file.

Currently, your code returns a raw JSON response (application/json). To fix this, you should:

  1. Set the Content-Type header to text/html.
  2. After saving the data, read the full content of db/expense.json.
  3. Send an HTML string as the response body. A good way to display formatted JSON is to wrap it in <pre> tags, like this: res.end(
    ${fileContent}
    );

Comment on lines +49 to +50
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

The task requires you to "return an HTML page with well formatted JSON". Currently, you're sending back raw JSON with a Content-Type of application/json and only the newly added expense.

To meet the requirements, you should:

  1. Set the Content-Type to 'text/html'.
  2. Send back an HTML string as the response.
  3. This HTML should display the entire updated array of expenses (arr), not just the single obj. A <pre> tag is a great way to format the JSON data inside the HTML.

Comment on lines +49 to +50
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

This response does not meet the task requirements, which was also mentioned in the previous review. The task asks you to "return an HTML page with well formatted JSON".

Currently, you are returning raw JSON data (Content-Type: 'application/json') containing only the newly added expense.

Instead, you should:

  1. Set the Content-Type header to 'text/html'.
  2. Read the full, updated content of expense.json (which should be an array of all expenses).
  3. Create an HTML string that displays this data, perhaps inside a <pre> tag for nice formatting.
  4. Send this HTML string as the response body.

} catch {
res.writeHead(400, { 'Content-Type': 'text/plain' });
res.end('Invalid JSON');
}
});

return;
}
if (req.method === 'GET' && req.url === '/expenses') {
try {
const data = fs.existsSync(DATA_PATH)
? fs.readFileSync(DATA_PATH, 'utf-8')
: '';
const obj = data ? JSON.parse(data) : {};
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

When the expense.json file is empty or doesn't exist, you are initializing obj as an empty object ({}). Since the data structure for expenses should be an array, it would be more consistent and less error-prone to default to an empty array ([]) here.


res.writeHead(200, { 'Content-Type': 'application/json' });
res.end(JSON.stringify(obj));
} catch {
res.writeHead(500, { 'Content-Type': 'text/plain' });
res.end('Failed to read expense');
}

return;
}

res.writeHead(404, { 'Content-Type': 'text/plain' });
res.end('Not found');
});
}

module.exports = {
createServer,
};
module.exports = { createServer };
56 changes: 56 additions & 0 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Add Expense</title>
</head>
<body>
<h1>Add Expense</h1>
<form id="expense-form">
<label>Title: <input type="text" name="title" required></label><br>
<label>Date: <input type="date" name="date" required></label><br>
<label>Amount: <input type="number" name="amount" required></label><br>
<button type="submit">Add Expense</button>
</form>

<h2>All Expenses</h2>
<div id="expenses"></div>

<script>
const form = document.getElementById('expense-form');
const expensesDiv = document.getElementById('expenses');

async function loadExpenses() {
try {
const res = await fetch('/db/expense.json');
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

This fetch call is attempting to load the JSON file directly. While this might work in some server setups, your Node.js server has a specific API endpoint for this.

Based on src/createServer.js, the correct endpoint to get all expenses is /expenses. You should update this URL to match the one on the server.

const arr = await res.json();
expensesDiv.innerHTML = `<pre>${JSON.stringify(arr, null, 2)}</pre>`;
} catch {
expensesDiv.innerHTML = 'No expenses yet.';
}
}

form.addEventListener('submit', async e => {
e.preventDefault();

const data = {
title: form.title.value,
date: form.date.value,
amount: form.amount.value
};

await fetch('/submit-expense', {
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 23, 2025

Choose a reason for hiding this comment

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

The URL for the POST request here is /submit-expense, but the server is listening for requests on /add-expense (as defined in src/createServer.js).

To fix this, make sure the URL in this fetch call exactly matches the route you've set up in your server for adding a new expense.

method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(data)
});

form.reset();
loadExpenses();
});

loadExpenses();
</script>
</body>
</html>