|
| 1 | +# NOTES |
| 2 | + |
| 3 | +## The plan |
| 4 | + |
| 5 | +I read `tests/update-user.test.js` first to pin down the exact contract: `PUT /users/:id` |
| 6 | +must return 200 with the updated user, 404 for an id that doesn't exist, and 400 when a |
| 7 | +required field is missing — even when the id does exist (the 400 test uses id `1`, a real |
| 8 | +user). That last detail meant validation had to run before the not-found lookup, not after. |
| 9 | +The plan was to mirror the existing `POST /users` handler's shape (same validation style, |
| 10 | +same error message conventions) and add one store helper, `updateUser(id, { name, email })`, |
| 11 | +following the pattern of `getUserById`/`createUser`. I didn't change the plan after approving |
| 12 | +it — the existing code made the shape of the change obvious. |
| 13 | + |
| 14 | +## Model choice |
| 15 | + |
| 16 | +Sonnet 5. This was a small, well-specified feature with an existing near-identical template |
| 17 | +in the codebase (`POST /users` + `createUser`), so it didn't need heavyweight architectural |
| 18 | +reasoning — just care to get the validation-before-404 ordering right against a fixed test |
| 19 | +file I couldn't edit. |
| 20 | + |
| 21 | +## Commit split |
| 22 | + |
| 23 | +Two commits: one for the feature (`db/store.js` + `routes/users.js` together, since the route |
| 24 | +depends on the store helper and neither is independently useful/testable without the other), |
| 25 | +and a separate one for this `NOTES.md`. Keeping the write-up out of the feature commit keeps |
| 26 | +that commit's diff focused on the actual behavior change. |
| 27 | + |
| 28 | +## What review caught |
| 29 | + |
| 30 | +I reviewed the diff before writing this. It's a 2-file, ~30-line change with no surprises. |
| 31 | +Things I specifically checked: an invalid/non-numeric `:id` (`Number("abc")` → `NaN`) falls |
| 32 | +through to 404 rather than throwing, since `getUserById` uses strict `===` and `NaN` never |
| 33 | +matches; and the validation-before-lookup ordering matches what the tests require. `npm test` |
| 34 | +is green (9/9) and `npm run lint` is clean. |
0 commit comments