Feat/update user endpoint - #100
Closed
EvheniiChuhai wants to merge 5 commits into
Closed
Conversation
Routes go through db/store.js for all data access, so the upcoming PUT /users/:id route needs a store-level way to write to a user. updateUser looks the record up by id and overwrites name and email in place, which keeps the id stable and leaves the array ordering alone. It returns undefined for an unknown id, mirroring getUserById so callers can use the same not-found check they already use for reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The upcoming PUT /users/:id needs the same "is this field present and
usable" test that POST /users does, and the conventions for this repo
say not to grow a second way of doing the same thing. Pull the check
into an isUsable helper both routes can share.
This tightens POST slightly: the old `!name` accepted a whitespace-only
string and a number, so POST {"name": " "} stored a blank-looking
name and returned 201. A field that is present but empty is bad input,
so isUsable requires a non-empty string once trimmed. No existing test
depended on the looser behaviour.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The users resource could list, read, and create but not update. Add the
update route, which replaces a user's name and email and returns the
updated record with 200.
PUT is a full replacement here, so both fields are required rather than
optional: sending only a name is a 400, not a partial update. A partial
update would be a PATCH, which nothing asks for yet.
Two failure paths, following the repo's convention that 400 is about the
body and 404 is about the id:
- the body is validated before the store is touched, so a request that
is bad on both counts reports the body error the caller must fix first
- an id matching no record returns 404 instead of crashing. A
non-numeric id also lands here, because Number("abc") is NaN and NaN
matches no record, which makes "no such user" the honest answer.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Self-review turned up an inconsistency introduced with isUsable: it
judges value.trim(), but the routes then handed the raw string to the
store. PUT /users/1 with {"name": " Ada "} passed validation and
persisted the padding, so the value that was checked was not the value
that was written.
Trim at both call sites so the store holds what validation approved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The course project asks for a short write-up alongside the code, and two of the provided tests check it exists with real content. Covers the plan and what I changed in it before approving, the model choice, why the commits are split the way they are, and what the review pass caught (the trim bug in the previous commit) versus what it confirmed was already correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.