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

## Plan
The approved plan was to add a `PUT /users/:id` route in `routes/users.js` plus a matching `updateUser(id, { name, email })` helper in `db/store.js`, following the exact validation and error-response style already used by `GET /:id` and `POST /`: truthiness-check `name`/`email` for a 400, look up the user for a 404, otherwise update and return it with an implicit 200. Before approving, I clarified one open question — whether validation should go beyond a truthiness check (e.g. type/format checks on email) — and decided to match the existing `POST /users` validation exactly rather than add new validation logic that isn't exercised by the tests or present elsewhere in the codebase. No other edits were made to the plan.

## Model choice
Used Claude Sonnet 5 for planning and implementation. The task was small and well-scoped (one route, one store helper, following an established pattern), so a fast, capable model was enough — no need for deeper reasoning modes.

## Commit split
Split into two commits: one for the `updateUser` store helper (`db/store.js`), and one for the `PUT /users/:id` route itself (`routes/users.js`). Each commit is a complete, independently understandable unit — the store change adds a data-access capability, the route change wires it up to HTTP — which keeps the diff easy to review and revert independently if needed.

## Review
Ran a full-effort code review (correctness, reuse/simplification/efficiency, altitude, conventions) over the diff. It confirmed the change was safe: `express.json()` always sets `req.body` to `{}` (never `undefined`) so the destructuring can't throw, the validate-before-lookup ordering and NaN-id handling behave correctly, and no other code in the repo depends on `db/store.js`'s exports in a way this change could break. No issues were found or needed fixing.
15 changes: 14 additions & 1 deletion db/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,17 @@ function createUser({ name, email }) {
return user;
}

module.exports = { getAllUsers, getUserById, createUser };
function updateUser(id, { name, email }) {
const user = users.find((user) => user.id === 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 @@ -20,6 +20,24 @@ router.get("/:id", (req, res) => {
res.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 updatedUser = store.updateUser(id, { name, email });

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

res.json(updatedUser);
});

// POST /users — create a user; name and email are required
router.post("/", (req, res) => {
const { name, email } = req.body;
Expand Down