|
| 1 | +# Notes — update-user endpoint |
| 2 | + |
| 3 | +**The plan.** Add `PUT /users/:id` following the two patterns already in |
| 4 | +`routes/users.js`: the 400/validation shape from `POST /` (reject if |
| 5 | +`name` or `email` is missing) and the 404/not-found shape from |
| 6 | +`GET /:id`. On the data side, add a single `updateUser(id, { name, email })` |
| 7 | +helper to `db/store.js` that mirrors `getUserById` — looks the user up, |
| 8 | +returns `null` on a miss, otherwise mutates and returns it. I approved the |
| 9 | +plan as written; the one thing I checked closely before approving was the |
| 10 | +order of the two checks (validate the body vs. look up the id first). |
| 11 | +`PUT /users/1` with a missing field must return 400 even though id 1 |
| 12 | +exists, so I made validation run before the existence check — that also |
| 13 | +means an invalid body against an unknown id returns 400 rather than 404, |
| 14 | +which felt like the more standard REST behavior anyway. |
| 15 | + |
| 16 | +**Model.** Claude Sonnet 5. The task is a small, well-scoped feature with |
| 17 | +an existing pattern to copy almost directly (two ~15-line additions) — |
| 18 | +nothing here needed heavier architectural reasoning, so the faster/cheaper |
| 19 | +model was the right fit. |
| 20 | + |
| 21 | +**Commits.** Split by file/layer rather than by "the feature" as one blob: |
| 22 | +1. `feat: add updateUser to the in-memory store` — `db/store.js` alone. |
| 23 | +2. `feat: add PUT /users/:id with validation and 404 handling` — |
| 24 | + `routes/users.js` alone; this is the commit that turns the grading |
| 25 | + tests green. |
| 26 | +3. `docs: add NOTES.md` — this file. |
| 27 | + |
| 28 | +Each commit is independently reviewable (data layer vs. HTTP layer) and |
| 29 | +each leaves the repo in a working state (`npm test` doesn't newly break |
| 30 | +between them), which is why I didn't just squash it into one commit. |
| 31 | + |
| 32 | +**Review.** Self-reviewed the diff before opening the PR: checked the |
| 33 | +validate-then-404 ordering (see above), checked that `store.updateUser` |
| 34 | +mutates the same array entry `getUserById`/`getAllUsers` read from rather |
| 35 | +than a copy (so the update is actually visible afterward), and checked |
| 36 | +`Number(req.params.id)` on a non-numeric id — it becomes `NaN`, which never |
| 37 | +matches an id in `find`, so it falls through to 404 rather than crashing. |
| 38 | +Ran `npm run lint` too; it came back clean. Nothing needed fixing — the |
| 39 | +review mostly confirmed the pattern-matching approach held up, rather than |
| 40 | +catching a new bug. One deliberate scope call, not a bug: validation only |
| 41 | +checks that `name`/`email` are present/truthy, the same depth as the |
| 42 | +existing `POST /` handler — no email-format or type checks, since neither |
| 43 | +the existing code nor the provided tests ask for that. |
0 commit comments