Skip to content

Add PUT /users/:id to update a user - #91

Open
terray-fb wants to merge 3 commits into
mate-academy:mainfrom
terray-fb:feature/update-user
Open

Add PUT /users/:id to update a user#91
terray-fb wants to merge 3 commits into
mate-academy:mainfrom
terray-fb:feature/update-user

Conversation

@terray-fb

Copy link
Copy Markdown

What changed

  • Adds PUT /users/:id to update an existing user's name and email.
  • Adds an updateUser(id, { name, email }) helper to db/store.js for the data access, following the same pattern as the existing getUserById/createUser helpers.
  • Adds NOTES.md covering the approved plan, model choice, commit split, and review.

Why

The users resource had no update action. tests/update-user.test.js already shipped with the tests for this endpoint, starting red; this PR turns them green without modifying that file.

How it works

  • Validates that name and email are both non-empty strings (rejects missing, empty, whitespace-only, and non-string values) and returns 400 with a clear message if not.
  • Looks the user up via db/store.js; returns 404 with { "error": "User not found" } if no user has that id (including non-numeric ids, which become NaN and simply don't match).
  • On success, fully replaces both fields and returns 200 with the updated user.
  • All data access goes through db/store.js — the route never touches the in-memory array directly.
  • Validation intentionally happens before the lookup, so a malformed request gets a deterministic 400 regardless of whether the id exists.

Deliberate design choices

  • Full replace, not partial update: PUT /users/:id requires both name and email on every request and replaces both fields — it does not merge in a partial body. This matches PUT semantics and is what the tests require (a missing field is always a 400, never a partial update).
  • Stricter validation is scoped to this route only: the new non-empty-string check applies to PUT /users/:id alone. The existing POST /users route keeps its original !name || !email check unchanged — no behavior change to code that already worked.

What a reviewer should test

  • Successful update: PUT /users/1 with {"name":"...","email":"..."}200 with the updated user.
  • Persistence: a follow-up GET /users/1 after the update shows the change persisted through the store, not just in the response.
  • Not found: PUT /users/9999 (unknown numeric id) and PUT /users/abc (non-numeric id) with a valid body → 404, server stays up.
  • Invalid input: missing field, empty string, whitespace-only string, or non-string value (e.g. name: 123) → 400.

Test evidence

  • npm test — 9/9 passing (the 3 update-user tests plus the 2 NOTES.md tests, and the existing suite untouched).
  • npm run lint — clean.
  • tests/update-user.test.js was not modified.

Replaces a user's name and email in place and returns the updated
record, or undefined when no user has that id - the same "missing
means undefined" contract getUserById already uses, so callers can
handle not-found without a second lookup.
Validates that name and email are both non-empty strings and returns
400 with a clear message when they aren't, returns 404 when no user
has that id, and otherwise responds 200 with the updated user. All
data access goes through db/store.js, matching the other routes.

Turns the tests in tests/update-user.test.js green.
Covers the four write-up questions from the project brief: what the
approved plan contained and what I changed in it, which models I used
for planning versus implementation, why the work is split the way it
is, and what the pre-push review flagged or confirmed.
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