Add DELETE and PATCH for weights and entries - #3
Conversation
Weights and entries only supported GET/POST. Adds id-scoped PATCH (partial update) and DELETE, matching the existing query-param ?id= pattern used by food-entries and nutrition-sprints. 404 when the id doesn't exist, 400 when id is missing. No auth/ownership scoping exists elsewhere in this single-user local app, so none was added here (matches existing convention).
📝 WalkthroughWalkthroughBoth entries and weights API routes are reformatted to use multi-line Drizzle query chains. New PATCH and DELETE handlers are added to both routes, validating the ChangesEntries and Weights CRUD Routes
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant APIRoute
participant Database
Client->>APIRoute: PATCH/DELETE ?id=X
APIRoute->>Database: select existing row by id
Database-->>APIRoute: row or none
alt id missing
APIRoute-->>Client: 400
else not found
APIRoute-->>Client: 404
else found
alt PATCH
APIRoute->>Database: update row
APIRoute->>Database: reselect updated row
Database-->>APIRoute: updated row
APIRoute-->>Client: updated row
else DELETE
APIRoute->>Database: delete row
APIRoute-->>Client: success true
end
end
Related Issues: None specified Related PRs: None specified Suggested labels: api, enhancement Suggested reviewers: eprouveze 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/api/weights/route.ts (1)
55-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the shared id-validation/existence-check boilerplate.
The
idparsing → 400 check → existence lookup → 404 check → catch/500 sequence is duplicated verbatim acrossPATCH/DELETEin this file and insrc/app/api/entries/route.ts, and per the PR description also mirrorsfood-entries/nutrition-sprints. A small shared helper (e.g.getRowOrNotFound(table, id)) would reduce this repetition across the growing set of CRUD routes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/weights/route.ts` around lines 55 - 129, The `PATCH` and `DELETE` handlers in `weights/route.ts` duplicate the same id parsing, 400 response, existence lookup, and 404 handling logic used in other CRUD routes like `entries/route.ts` (and the similar food-entries/nutrition-sprints handlers). Extract that boilerplate into a shared helper, such as a reusable `getRowOrNotFound`-style function, and refactor `PATCH`/`DELETE` to call it so the `db.select().from(weights)...` and id-validation flow is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/entries/route.ts`:
- Around line 78-152: The PATCH and DELETE handlers in the entries route are
accepting malformed id values because parseInt(id) can produce NaN and fall
through to the existing/not-found path. Parse the id once in PATCH and DELETE,
validate that it is a real integer before any db lookup, return a 400 for
invalid ids, and then reuse the parsed numeric id in the existing db.select,
db.update, and db.delete calls.
In `@src/app/api/weights/route.ts`:
- Around line 55-100: The PATCH handler in route.ts needs stricter input
validation before querying or updating weights. In PATCH, replace the loose
parseInt(id) usage with a strict integer check so malformed ids like "1foo" are
rejected with a 400 instead of being coerced. Also validate weightKg before
db.update in PATCH: reject null, undefined, empty, or non-numeric values rather
than passing them through to parseFloat, and keep the existing record only when
the input is a finite number.
---
Nitpick comments:
In `@src/app/api/weights/route.ts`:
- Around line 55-129: The `PATCH` and `DELETE` handlers in `weights/route.ts`
duplicate the same id parsing, 400 response, existence lookup, and 404 handling
logic used in other CRUD routes like `entries/route.ts` (and the similar
food-entries/nutrition-sprints handlers). Extract that boilerplate into a shared
helper, such as a reusable `getRowOrNotFound`-style function, and refactor
`PATCH`/`DELETE` to call it so the `db.select().from(weights)...` and
id-validation flow is centralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db9b7ade-aea2-42b8-a415-150acd256b91
📒 Files selected for processing (2)
src/app/api/entries/route.tssrc/app/api/weights/route.ts
| export async function PATCH(request: Request) { | ||
| try { | ||
| const { searchParams } = new URL(request.url); | ||
| const id = searchParams.get("id"); | ||
|
|
||
| if (!id) { | ||
| return NextResponse.json({ error: "id required" }, { status: 400 }); | ||
| } | ||
|
|
||
| const existing = await db | ||
| .select() | ||
| .from(entries) | ||
| .where(eq(entries.id, parseInt(id))) | ||
| .limit(1); | ||
|
|
||
| if (!existing[0]) { | ||
| return NextResponse.json({ error: "Entry not found" }, { status: 404 }); | ||
| } | ||
|
|
||
| const body = await request.json(); | ||
| const { date, notes, energyLevel, fastingHours } = body; | ||
|
|
||
| await db | ||
| .update(entries) | ||
| .set({ | ||
| date: date ?? existing[0].date, | ||
| notes: notes ?? existing[0].notes, | ||
| energyLevel: energyLevel ?? existing[0].energyLevel, | ||
| fastingHours: fastingHours ?? existing[0].fastingHours, | ||
| }) | ||
| .where(eq(entries.id, parseInt(id))); | ||
|
|
||
| const updated = await db | ||
| .select() | ||
| .from(entries) | ||
| .where(eq(entries.id, parseInt(id))) | ||
| .limit(1); | ||
| return NextResponse.json(updated[0]); | ||
| } catch (error) { | ||
| console.error("Failed to update entry:", error); | ||
| return NextResponse.json( | ||
| { error: "Failed to update entry" }, | ||
| { status: 500 }, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export async function DELETE(request: Request) { | ||
| try { | ||
| const { searchParams } = new URL(request.url); | ||
| const id = searchParams.get("id"); | ||
|
|
||
| if (!id) { | ||
| return NextResponse.json({ error: "id required" }, { status: 400 }); | ||
| } | ||
|
|
||
| const existing = await db | ||
| .select() | ||
| .from(entries) | ||
| .where(eq(entries.id, parseInt(id))) | ||
| .limit(1); | ||
|
|
||
| if (!existing[0]) { | ||
| return NextResponse.json({ error: "Entry not found" }, { status: 404 }); | ||
| } | ||
|
|
||
| await db.delete(entries).where(eq(entries.id, parseInt(id))); | ||
|
|
||
| return NextResponse.json({ success: true }); | ||
| } catch (error) { | ||
| console.error("Failed to delete entry:", error); | ||
| return NextResponse.json( | ||
| { error: "Failed to delete entry" }, | ||
| { status: 500 }, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package files ==\n'
git ls-files 'package.json' 'pnpm-lock.yaml' 'package-lock.json' 'yarn.lock' 'src/**/schema.*' 'src/**/db.*' 'src/**/route.ts' | sed -n '1,200p'
printf '\n== route file outline ==\n'
ast-grep outline src/app/api/entries/route.ts --view expanded || true
printf '\n== route file excerpt ==\n'
cat -n src/app/api/entries/route.ts | sed -n '1,220p'
printf '\n== schema candidates ==\n'
rg -n "entries|sqliteTable|integer\\(|id:" src --glob '!**/node_modules/**' || trueRepository: eprouveze/HealthPulse
Length of output: 22892
Reject malformed id values in PATCH/DELETE. parseInt(id) accepts non-numeric input as NaN, so both handlers return 404 for bad IDs instead of a clear 400. Parse once, reject non-integer values up front, and reuse the parsed value in the queries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/api/entries/route.ts` around lines 78 - 152, The PATCH and DELETE
handlers in the entries route are accepting malformed id values because
parseInt(id) can produce NaN and fall through to the existing/not-found path.
Parse the id once in PATCH and DELETE, validate that it is a real integer before
any db lookup, return a 400 for invalid ids, and then reuse the parsed numeric
id in the existing db.select, db.update, and db.delete calls.
| export async function PATCH(request: Request) { | ||
| try { | ||
| const { searchParams } = new URL(request.url); | ||
| const id = searchParams.get("id"); | ||
|
|
||
| if (!id) { | ||
| return NextResponse.json({ error: "id required" }, { status: 400 }); | ||
| } | ||
|
|
||
| const existing = await db | ||
| .select() | ||
| .from(weights) | ||
| .where(eq(weights.id, parseInt(id))) | ||
| .limit(1); | ||
|
|
||
| if (!existing[0]) { | ||
| return NextResponse.json({ error: "Weight not found" }, { status: 404 }); | ||
| } | ||
|
|
||
| const body = await request.json(); | ||
| const { date, weightKg, source } = body; | ||
|
|
||
| await db | ||
| .update(weights) | ||
| .set({ | ||
| date: date ?? existing[0].date, | ||
| weightKg: | ||
| weightKg !== undefined ? parseFloat(weightKg) : existing[0].weightKg, | ||
| source: source ?? existing[0].source, | ||
| }) | ||
| .where(eq(weights.id, parseInt(id))); | ||
|
|
||
| const updated = await db | ||
| .select() | ||
| .from(weights) | ||
| .where(eq(weights.id, parseInt(id))) | ||
| .limit(1); | ||
| return NextResponse.json(updated[0]); | ||
| } catch (error) { | ||
| console.error("Failed to update weight:", error); | ||
| return NextResponse.json( | ||
| { error: "Failed to update weight" }, | ||
| { status: 500 }, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== weights route ==\n'
sed -n '1,220p' src/app/api/weights/route.ts
printf '\n== schema ==\n'
sed -n '1,120p' src/lib/schema.ts
printf '\n== entries route references ==\n'
rg -n "parseInt\\(id\\)|Weight not found|id required" src/app/api/entries/route.ts src/app/api/weights/route.tsRepository: eprouveze/HealthPulse
Length of output: 10453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package.json ==\n'
sed -n '1,220p' package.json
printf '\n== parseFloat / NaN probe ==\n'
node - <<'JS'
for (const value of [null, undefined, "", "12.3", "abc"]) {
const parsed = parseFloat(value);
console.log(JSON.stringify({ value, parsed, isNaN: Number.isNaN(parsed) }));
}
JSRepository: eprouveze/HealthPulse
Length of output: 2067
🌐 Web query:
better-sqlite3 NaN binds NULL SQLite not null constraint parseFloat(null)
💡 Result:
In better-sqlite3, and within SQLite itself, JavaScript NaN (Not-a-Number) values cannot be directly stored as floating-point numbers. SQLite treats NaN as NULL [1][2]. Consequently, if you attempt to bind a NaN value to a column defined with a NOT NULL constraint, the database operation will throw a SQLITE_CONSTRAINT error [3][4][5]. The behavior is summarized as follows: 1. SQLite's Data Handling: SQLite does not support NaN as a native floating-point value. When a NaN is passed to the SQLite binding layer, it is interpreted as NULL [1][2]. 2. Constraint Violations: Because the value is converted to NULL, any column with a NOT NULL constraint will reject the insert or update, resulting in an error [3][5]. 3. better-sqlite3 Limitations: The library is designed to be a thin, predictable wrapper around SQLite [6][7][8]. It explicitly supports binding only specific types: numbers (converted to REAL or INTEGER), strings (TEXT), BigInts (INTEGER), Buffers (BLOB), and null [3][6][9]. Since NaN is a special case of a JavaScript number, and SQLite maps floating-point numbers to REAL [3][8], the binding process delegates to the underlying SQLite C API, which triggers the NULL conversion [1]. To handle this, you must sanitize or validate your data before passing it to better-sqlite3. You should check for NaN values in your JavaScript code (e.g., using Number.isNaN) and either replace them with a valid number (like 0) or handle them according to your application logic before attempting to execute the SQL statement [1]. Attempting to use parseFloat(null) on these values will return NaN, which does not bypass this constraint and will still result in the same error if bound to a NOT NULL column.
Citations:
- 1: https://sqlite.org/forum/info/f97759ec7b88be775e258b6ed768e18a7b3830f724eaaa7dcad6a09cb14a7688
- 2: https://groups.google.com/g/sqlite_users/c/aQX_WvVwBS8
- 3: https://github.com/WiseLibs/better-sqlite3/blob/v12.9.0/docs/api.md
- 4: https://stackoverflow.com/questions/48890430/better-sqlite3-sqliteerror-not-null-constraint-failed
- 5: https://github.com/JoshuaWise/better-sqlite3/blob/master/docs/tips.md
- 6: TypeError: SQLite3 can only bind numbers, strings, bigints, buffers, and null WiseLibs/better-sqlite3#907
- 7: Error when passing javascript true/false as sqlite parameters WiseLibs/better-sqlite3#258
- 8: Parameter convert is not consistency ? WiseLibs/better-sqlite3#627
- 9: https://github.com/WiseLibs/better-sqlite3/blob/b615a52a9a00ec5c5504875b429136ad8302d3b1/docs/api.md
Validate PATCH inputs in src/app/api/weights/route.ts. parseInt(id) should be a strict integer check; otherwise malformed ids can be coerced into a different row (1foo → 1) or fall through to 404. weightKg also needs a null/NaN guard: null becomes NaN, and SQLite/better-sqlite3 binds that as NULL, tripping weights.weightKg's notNull() constraint and returning a generic 500.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/api/weights/route.ts` around lines 55 - 100, The PATCH handler in
route.ts needs stricter input validation before querying or updating weights. In
PATCH, replace the loose parseInt(id) usage with a strict integer check so
malformed ids like "1foo" are rejected with a 400 instead of being coerced. Also
validate weightKg before db.update in PATCH: reject null, undefined, empty, or
non-numeric values rather than passing them through to parseFloat, and keep the
existing record only when the input is a finite number.
1 participant
Summary
weightsandentriesAPI routes only supported GET/POST. AddsPATCH(partial update by id) andDELETE(by id) for both, matching the exact query-param?id=pattern already used byfood-entriesandnutrition-sprints.idis missing, 404 if the row doesn't exist, 200 with the updated/deleted-confirmation JSON otherwise. Error handling/response shape matches existing GET/POST handlers in the same files.Verification
npx tsc --noEmit— clean, zero errors.npx next build— compiled and type-checked successfully (webpack build succeeded for all routes including the two changed files); the build only failed afterward at page-data-collection due to a pre-existing, unrelated Node 26 /better-sqlite3native-binding incompatibility (had tonpm ci --ignore-scriptsbecause the native module fails to compile against Node 26 on this machine). Not introduced by this change.next lintwould prompt to scaffold one interactively) — skipped as out of scope.Test plan
PATCH /api/weights?id=<id>andDELETE /api/weights?id=<id>against a running dev server with real dataPATCH /api/entries?id=<id>andDELETE /api/entries?id=<id>Note: better-sqlite3 doesn't currently build on this machine's Node 26 — worth checking whether that's already tracked, since it blocks any local dev-server run.
https://claude.ai/code/session_01TFnZVjgqB7CTE3MnZmFyeE
Summary by CodeRabbit
New Features
Bug Fixes