Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions NOTES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Notes

- Model: 'opusplan' (Opus for planning, Sonnet for execution)
- Review the plan:
- Is the approach sound?
- Are its assumptions right (tools, libraries, paths)?
- Is anything missing?
- Commit messages: Could a reviewer understand what changed and why from the message alone, without opening the diff?
- Code review: look for bugs, edge cases, readability, anything risky, or anything missing

## Answers

1. The plan covered three changes: a `updateUser` helper in `db/store.js`, a `PUT /users/:id` route in `routes/users.js` with input validation (400) and not-found handling (404), and this NOTES.md. I approved the plan without edits — the file targets, edge cases, and ordering were all correct on the first pass.

2. I chose `opusplan` (Opus for planning, Sonnet for execution). Opus is better at reasoning through edge cases and writing a thorough plan; Sonnet is faster and more than capable for straightforward implementation once the plan is set. The split made the planning phase more careful without slowing down the build.

3. I split into three commits: (1) the store helper, (2) the route handler, (3) a post-review whitespace fix applied to both POST and PUT. The store came first because the route depends on it and each commit should leave the tree in a working state. The whitespace fix was a separate commit because it was a distinct improvement caught during review, not part of the original feature.

4. The review caught that `!name || !email` accepts whitespace-only strings like `" "`, which would silently overwrite a user's name or email with blanks. I fixed it with an `isBlank` helper applied consistently to both POST and PUT. Two other findings — non-numeric ids returning 404 instead of 400, and missing store-layer validation — were intentionally left out of scope: the first is a pre-existing pattern not covered by tests, and the second is theoretical since the store is only reached through the validated route.

10 changes: 9 additions & 1 deletion db/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,12 @@ function createUser({ name, email }) {
return user;
}

module.exports = { getAllUsers, getUserById, createUser };
function updateUser(id, { name, email }) {
const user = getUserById(id);
if (!user) return undefined;
user.name = name;
user.email = email;
return user;
}

module.exports = { getAllUsers, getUserById, createUser, updateUser };
10 changes: 5 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 24 additions & 1 deletion routes/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ const store = require("../db/store");

const router = express.Router();

// A field is blank if it's missing, not a string, or only whitespace.
function isBlank(value) {
return typeof value !== "string" || value.trim() === "";
}

// GET /users — list every user
router.get("/", (req, res) => {
res.json(store.getAllUsers());
Expand All @@ -24,12 +29,30 @@ router.get("/:id", (req, res) => {
router.post("/", (req, res) => {
const { name, email } = req.body;

if (!name || !email) {
if (isBlank(name) || isBlank(email)) {
return res.status(400).json({ error: "name and email are required" });
}

const user = store.createUser({ name, email });
res.status(201).json(user);
});

// PUT /users/:id — update an existing user; name and email are required
router.put("/:id", (req, res) => {
const id = Number(req.params.id);
const { name, email } = req.body;

if (isBlank(name) || isBlank(email)) {
return res.status(400).json({ error: "name and email are required" });
}

const user = store.updateUser(id, { name, email });

if (!user) {
return res.status(404).json({ error: "User not found" });
}

res.json(user);
});

module.exports = router;