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
9 changes: 9 additions & 0 deletions NOTES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Notes — update-user endpoint

**The plan.** I planned `PUT /users/:id` to mirror the existing routes exactly: reuse the id-parsing/404 shape from `GET /users/:id` and the required-field validation shape from `POST /users`, plus a new `updateUser(id, { name, email })` helper in `db/store.js` that follows `createUser`'s style. The plan also called for validating before checking existence (a malformed request against a missing id should read as a clear 400, not a confusing 404). I didn't edit anything before approving it — the existing route patterns made the shape of the change unambiguous.

**Model.** Claude Sonnet 5. The change was small, the target files were already known from the README, and the existing code gave a clear pattern to follow — this didn't need a heavier-reasoning model, just careful adherence to what was already there.

**Commits.** Three commits, split by concern: (1) the `updateUser` store helper on its own, (2) the `PUT /:id` route wired to it, (3) a small fix from self-review (see below). Splitting the store change from the route change makes each commit reviewable independently — the helper is pure data logic, the route is HTTP plumbing — and keeping the review fix as its own commit keeps the "what did the tests-passing version look like" and "what did review improve" history honest instead of squashing them together.

**Review.** I ran a self-review before opening the PR. It flagged two duplications: `updateUser`'s lookup re-implementing `users.find(...)` instead of calling `getUserById`, and the `!name || !email` validation being copy-pasted between `POST` and `PUT`. I fixed the first — it was a free win, just calling an existing helper instead of restating its logic. I left the second as-is: it's a single line duplicated across two small handlers in a two-route file, and extracting a shared validator felt like abstraction the project doesn't need yet. The not-found and validation paths both behave as the tests expect (400 before 404, 404 for unknown ids, 200 with the updated user on success).
11 changes: 10 additions & 1 deletion db/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,13 @@ 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);
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 });

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

res.json(user);
});

module.exports = router;