Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions NOTES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
I made Claude to create the plan. I approved the 1st and 2nd steps: add/implement the updateUser() function and add PUT handler to routes.js. 3rd step had no impact on the code. In the 4th step would have created and filled the NOTES.md file. I asked to skip it as I had to do that. I chose Sonnet for model because this task seemed not be too complex, and so the whole is cheaper. Two commits were asked: 1: store.js as it contains the updateUser() function; 2: users.js as it uses updateUser(). 3rd will contain NOTES.md. After saving this with content everything was fine, all tests were green. The review caught 1 bug, which was fixed in the 4th commit.

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 file should clearly state what was actually changed (which files were modified and why). Right now it says steps were approved but doesn't explicitly confirm which files were committed or the final status besides "tests were green". Consider listing the actual commits or the exact files changed to make verification straightforward.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are grammar and clarity issues that make the meaning ambiguous (e.g., "I made Claude to create the plan", "3rd step had no impact on the code"). Please rewrite to plain language describing the plan you approved and any edits you made before approving.

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 notes mention choosing the "Sonnet" model and that it was chosen to reduce cost — the course asked to state which model was chosen and why, which you've done, but consider also adding a sentence confirming that the tests passed locally (e.g., npm test passed) and that NOTES.md was committed as required by the grading script.

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 closing sentence references a bug fixed in a "4th commit", but earlier you said there were only two (or three) commits. Make the commit count consistent: either list each commit and its purpose or correct the numbers so reviewers can follow the history.


10 changes: 9 additions & 1 deletion db/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,12 @@ function createUser({ name, email }) {
return user;
}

module.exports = { getAllUsers, getUserById, createUser };
function updateUser(id, { name, email }) {
const user = getUserById(id);
if (!user) return undefined;
user.name = name;
user.email = email;
return user;
}

module.exports = { getAllUsers, getUserById, createUser, updateUser };
18 changes: 18 additions & 0 deletions routes/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,22 @@ router.post("/", (req, res) => {
res.status(201).json(user);
});

// PUT /users/:id — update an existing user; name and email are required
router.put("/:id", (req, res) => {
const id = Number(req.params.id);

if (!store.getUserById(id)) {
return res.status(404).json({ error: "User not found" });
}

const { name, email } = req.body;

if (!name || !email) {
return res.status(400).json({ error: "name and email are required" });
}

const user = store.updateUser(id, { name, email });
res.json(user);
});

module.exports = router;