Skip to content

Add PUT /users/:id endpoint for updating a user - #1

Closed
MishaLehotskyi wants to merge 3 commits into
mainfrom
feat/update-user-endpoint
Closed

Add PUT /users/:id endpoint for updating a user#1
MishaLehotskyi wants to merge 3 commits into
mainfrom
feat/update-user-endpoint

Conversation

@MishaLehotskyi

Copy link
Copy Markdown
Owner

What changed

Adds the 'update a user' endpoint to the users resource.

  • db/store.js — new updateUser(id, { name, email }). It reuses getUserById, returns undefined when no user has that id, and otherwise updates the record and returns it.
  • routes/users.js — new PUT /users/:id. Validates the body, then goes through the store for the lookup and update.
    • 200 with the updated user on success
    • 400 { error: "name and email are required" } when name or email is missing, blank, or not a string
    • 404 { error: "User not found" } when no user has that id
  • NOTES.md — the write-up the project asks for (plan, model choice, commit split, review).

Why

tests/update-user.test.js was already in the repo and red. This makes it green without touching the tests.

Two judgement calls worth a reviewer's attention:

  • Validation runs before the lookup, so a malformed body on a nonexistent id returns 400 rather than 404. Either order passes the tests; reporting the bad request felt more useful than reporting the missing resource.
  • Validation is stricter than POST /users. The existing create route uses a truthy check, which accepts { "name": 42 }. The new route uses an isNonEmptyString helper instead. I deliberately did not change POST to match — that would alter the behaviour of an endpoint this change isn't about. Worth a follow-up.

What to test

npm test is green (9/9) and npm run lint is clean. Beyond the provided tests, I checked these by hand:

Request Expected
PUT /users/1 with { "name": "A", "email": "a@e.com" } 200, user updated
PUT /users/9999 with a valid body 404, not a crash
PUT /users/abc with a valid body 404 — Number("abc") is NaN, misses the lookup
PUT /users/1 with { "name": "Only a name" } 400
PUT /users/1 with { "name": " ", "email": "a@e.com" } 400
PUT /users/1 with { "name": 42, "email": "a@e.com" } 400
PUT /users/1 with no body 400
PUT /users/1 with { "name": "A", "email": "a@e.com", "id": 99, "admin": true } 200, extra fields ignored — the id can't be rewritten

The commits are split by layer (store helper → endpoint → notes), so they read cleanly one at a time.

🤖 Generated with Claude Code

MishaLehotskyi and others added 3 commits August 29, 2026 15:25
Updates an existing user's name and email in place and returns the updated
user. Returns undefined when no user has that id so route handlers can tell
"not found" apart from a successful update, matching how getUserById behaves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Validates that name and email are non-empty strings and answers 400 when
either is missing, blank, or the wrong type; answers 404 when no user has
that id, including a non-numeric id, instead of crashing. Data access goes
through the new store.updateUser helper like the other routes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers the plan and the one edit made before approving it, the model choice,
why the work is split into these three commits, and what the self-review
caught or confirmed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@MishaLehotskyi

Copy link
Copy Markdown
Owner Author

Superseded by mate-academy#114 — this one targeted the fork by mistake. The branch stays as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant