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

## What was in the plan you approved, and did you edit anything before approving?
The plan was to add `PUT /users/:id`: a new `updateUser(id, { name, email })`
helper in `db/store.js` mirroring `getUserById`/`createUser`, plus a route in
`routes/users.js` reusing the existing inline validation and 404 shape from
`GET /:id` and `POST /`. The pre-written tests in `tests/update-user.test.js`
fixed the target behavior (200 + updated body, 400 on a missing field, 404 on
an unknown id). I revised the draft once before approval: after the user
asked for one logical change per commit with each commit tested and manually
approved, I added an explicit commit-by-commit plan (store helper first,
route second) instead of the single combined change I'd originally sketched.

## Which model did you choose, and why?
Claude Sonnet 5. The work was a small, well-scoped feature that closely
followed patterns already present in the repo (existing GET/POST routes and
store functions to mirror), which doesn't need a larger model's extra
reasoning overhead.

## How did you split your commits, and why that way?
Four commits: (1) the `updateUser` store helper, (2) the `PUT /:id` route
wired to it, (3) this `NOTES.md` write-up, and (4) a validation-hardening fix
made after a pre-push review. Splitting store logic from the route let the
test suite confirm no regressions before the endpoint existed at all, and
kept each commit small enough to review and approve independently. The
review fix landed as its own commit rather than being folded backward into
commit 2, since it was a distinct pass made after the original feature was
already complete and tested.

## What did your review catch — or confirm was already fine?
Before pushing, a review of the diff caught three real gaps in the PUT
handler: `!name || !email` only checked truthiness, so non-string values
(e.g. objects) and whitespace-only strings passed validation and got
persisted; and the existence check ran after validation, so an unknown id
with a bad body returned 400 instead of 404. All three were fixed in commit
4. It also flagged a fourth issue — malformed JSON crashing past the app's
JSON error contract into an HTML stack trace — but that's a pre-existing,
app-wide gap (also affects `POST`) that was left out of scope by choice. The
review confirmed the store layer's not-found convention (falsy return,
decided by the route) and the 404/400 response shapes were already correct
and consistent with the rest of the resource.
14 changes: 13 additions & 1 deletion db/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,16 @@ 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 };
23 changes: 23 additions & 0 deletions routes/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,27 @@ 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 (
typeof name !== "string" ||
typeof email !== "string" ||
!name.trim() ||
!email.trim()
) {
return res.status(400).json({ error: "name and email are required" });
}

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

module.exports = router;