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
77 changes: 77 additions & 0 deletions NOTES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# NOTES — PUT /users/:id

## План

Мета: додати `PUT /users/:id` в Express API — оновлення користувача за id,
з валідацією обов'язкових полів (400 при пропущеному/некоректному значенні)
і коректною обробкою неіснуючого користувача (404, без падіння сервера).

Перед реалізацією прочитала існуючий код (`routes/users.js`, `db/store.js`)
і тест-файл (`tests/update-user.test.js`), щоб зрозуміти патерн, якого
дотримується проєкт, і не вигадувати новий стиль. Патерн: роутер відповідає
за парсинг id, валідацію тіла запиту і статус-коди; store — за чисті
операції над даними (без валідації всередині).

План складався з двох частин:
1. `updateUser(id, { name, email })` в `db/store.js` — знаходить користувача
через існуючий `getUserById`, оновлює поля, повертає оновленого
користувача або `undefined`, якщо не знайдено.
2. `router.put("/:id", ...)` в `routes/users.js` — парсить id так само, як
`GET /:id`, валідує name/email (400, як у POST), повертає 404 через
`store.updateUser`, інакше 200 з оновленим користувачем.

## Вибір моделі

Використала Opus для цього завдання. Фіча не тривіальна: потрібно було
узгодити зміни одразу у двох файлах (роутер + store) з дотриманням
існуючого патерну, а не просто дописати код "як вийде" — тож планування
і уважність до консистентності виправдали вибір сильнішої моделі,
особливо на етапі self-review, де важливо було не пропустити edge cases.

## Розбивка комітів

Два окремі логічні коміти на гілці `feat/put-users-id`:

1. `375d24f` — `feat: add PUT /users/:id endpoint`
Базова реалізація: `updateUser` у store + PUT-роут з валідацією
"поле присутнє" (`!name || !email`) і обробкою 404.
2. `c49994e` — `fix: tighten PUT /users/:id validation to reject non-string values`
Окремий коміт для правки, знайденої на етапі self-review — навмисно
не змішувала з першим комітом, щоб історія показувала: спочатку
робоча фіча, потім конкретне виправлення з чіткою причиною.

## Що спіймало ревʼю

Попросила Claude Code перевірити діфф на баги та edge cases перед тим,
як писати ці нотатки. Знайдено і виправлено:

- **Truthy-перевірка замість перевірки типу** (`!name || !email`):
пропускала `[]`, числа (`123`), булеві (`true`) як валідні значення
імені/email і зберігала їх у store — фактично псуючи існуючого
користувача без жодної помилки. Найсерйозніша знахідка, бо на
відміну від POST (де зіпсований запис — це один сміттєвий рядок),
тут перезаписується вже наявний користувач без можливості відкату
(store без персистентності й історії).
- **Falsy-але-присутні значення відхилялись помилково**: `{name: 0, ...}`
давав 400, хоча поле технічно було передане — наслідок тієї самої
truthy-перевірки, не окрема проблема.
- **Рядки з самих пробілів приймались** (`" "`) — записувались як
валідне ім'я.

Виправлення: додала `isNonEmptyString` (перевіряє `typeof === "string"`
і непорожність після `trim()`) і замінила truthy-перевірку в PUT-роуті.
Під час розробки прогонялись тести застосунку — 7/7 зелених
(`update-user.test.js` + `users.test.js`); `notes.test.js` тоді ще падав,
бо цього файлу не існувало. Після його створення весь набір зелений: 9/9.

Свідомо залишила поза скоупом:
- **Коерсія id** (`Number(req.params.id)` приймає `1e0`, `1.0` тощо як
валідний id 1) — це поведінка, успадкована від `GET /:id`, я нічого
не міняла в парсингу id, тому це не новий дефект цього PR.
- **JSON error handling** для malformed body — загальносистемна
відсутність error-handling middleware в `server.js`, стосується всіх
роутів однаково, а не лише PUT.
- Порядок перевірок: валідація (400) виконується перед перевіркою
існування користувача (404) — усвідомлений вибір (валідація вхідних
даних логічно передує зверненню до БД), тести цей порядок явно не
фіксують.
14 changes: 13 additions & 1 deletion db/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,16 @@ 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 };
24 changes: 24 additions & 0 deletions routes/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ const store = require("../db/store");

const router = express.Router();

// Field values arrive straight from JSON, so they can be any type. Only a
// string with actual characters in it counts as present.
function isNonEmptyString(value) {
return typeof value === "string" && value.trim() !== "";
}

// GET /users — list every user
router.get("/", (req, res) => {
res.json(store.getAllUsers());
Expand Down Expand Up @@ -32,4 +38,22 @@ router.post("/", (req, res) => {
res.status(201).json(user);
});

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

if (!isNonEmptyString(name) || !isNonEmptyString(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;