Skip to content

Add PUT /users/:id endpoint - #120

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

Add PUT /users/:id endpoint#120
ktroch wants to merge 3 commits into
mate-academy:mainfrom
ktroch:feature/update-user-endpoint

Conversation

@ktroch

@ktroch ktroch commented Aug 31, 2026

Copy link
Copy Markdown

What

Adds the "update a user" endpoint to the users resource:

  • PUT /users/:id updates an existing user's name and email.
  • Validates the body: missing name or email returns 400 with { error: "name and email are required" } (same shape as POST /users).
  • Returns 404 with { error: "User not found" } when :id doesn't match an existing user, instead of crashing.
  • Goes through a new store.updateUser(id, { name, email }) helper in db/store.js, following the existing getUserById/createUser pattern — no direct array access from the route.

Why

This is the "ship a real change end to end" course project: implement the endpoint the pre-written tests in tests/update-user.test.js expect, planned and committed the way you'd ship a real change.

How it was built

  • Planned first (approved as-is, no edits needed — it matched the existing GET /:id / POST / patterns closely).
  • Two logical commits: the store helper, then the route that consumes it.
  • Self-reviewed the diff before opening this PR (see NOTES.md for what that caught).
  • NOTES.md documents the plan, model choice, commit split, and review findings.

What to test

  • npm test — all 9 tests pass, including the three in tests/update-user.test.js:
    • PUT /users/:id with a valid body updates the user and returns 200.
    • PUT /users/9999 (unknown id) returns 404.
    • PUT /users/:id with a missing field (e.g. no email) returns 400.
  • Manually: PUT /users/1 with { "name": "...", "email": "..." } should update in place; the response should reflect the new values, and GET /users/1 afterward should show them too.

ktroch added 3 commits August 31, 2026 19:56
Mirrors getUserById/createUser: looks the user up by id, returns
undefined when it does not exist (leaving the 404 decision to the
route), otherwise mutates and returns the updated user.
Validates name and email are present (400 if not, same shape as
POST /users), then delegates to store.updateUser and returns 404
when the id does not match an existing user.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because the core implementation for the update endpoint is missing from the PR, which blocks functionality and tests. The task requires a PUT /users/:id route that validates input (return 400 when name or email is missing), checks for user existence (return 404 when not found), and updates the user (return 200 with the updated record). These changes should be added in routes/users.js (implement the PUT /:id handler) and db/store.js (add and export updateUser(id, { name, email }) that returns undefined when the user does not exist). Without these files in the PR diff, the tests cannot pass and the endpoint cannot function. NOTES.md is thorough and meets the documentation requirement—nice work there. Please include the actual code commits for the route and store helper, then run the tests to ensure they pass and push the updated branch.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

@ktroch

ktroch commented Aug 31, 2026

Copy link
Copy Markdown
Author

@mateacademy-ai-mentor The route and store changes are already in this PR's diff — they weren't missing, just possibly not visible at the moment you reviewed.

  • db/store.js commit: 2507f52 — adds and exports updateUser(id, { name, email }), returns undefined when the user doesn't exist.
  • routes/users.js commit: 5627e80 — adds PUT /users/:id, validates name/email (400), returns 404 when not found, 200 with the updated user otherwise.

CI on this PR (fc879a9) confirms it: all 3 update-user.test.js tests pass (ok 3/4/5), 9/9 total — see the "check" run: https://github.com/mate-academy/claude-ship-a-change-end-to-end/actions/runs/33422519064

No code changes were needed since the implementation was already present; requesting another look.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

The implementation is complete and correct. All requirements are met: the PUT /users/:id endpoint in routes/users.js properly validates input (400 for missing fields), checks for user existence (404 for missing users), and returns the updated user (200 on success). The updateUser helper in db/store.js follows the existing pattern and is correctly exported. NOTES.md provides thorough documentation covering the plan, model choice, and commit strategy. CI confirms all 9/9 tests pass, including all 3 update-user tests. No changes needed.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

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.

2 participants