bookmarks: don't fabricate a local save when the server rejects it #121

Merged

admin_emi merged 2 commits from fix/bookmarks-integration into dev

2026-07-26 18:51:03 +00:00

admin_emi commented

2026-07-26 18:48:36 +00:00

Owner

Integration pass over PR #117 (bookmarked locations). Backend/frontend contract, alembic chain, and selectors all check out — one real defect found and fixed:

bookmarks.js's add() treated a server rejection the same as being offline. The POST to /bookmarks was wrapped in try/catch with the comment "fall through so the star still visibly sticks, even offline" — but the fallthrough to the local-only path ran on any non-2xx response, not just a network exception. Hit the per-user cap (409) or send bad input (422) while signed in, and the star still lit up "Saved": a bookmark that was never created server-side, silently written into local cache and then dropped without explanation the next time syncFromServer reconciles against the real list.

Fix: only take the offline fallback on an actual fetch exception; a real HTTP rejection now throws so the caller sees it. bookmarks-ui.js's star-toggle handler had no catch at all (an unhandled rejection), so it now surfaces the message via alert(), matching the existing error pattern in subscriptions.js.

Everything else checked out clean:

Request/response field names (lat/lon/label/cell_id/id/created_at) match exactly between schemas.py/api_accounts.py and bookmarks.js.
POST /bookmarks/import returns {imported, skipped, bookmarks} as both subscriptions.js and bookmarks-ui.js assume.
DELETE/PATCH /bookmarks/{id} take an int path param; the frontend's String(b.id) values are always the server's numeric id when signed in, so they parse fine.
The per-user cap (200) is enforced identically in create_bookmark (409) and import_bookmarks (silently counts remaining items as skipped) — matches test_bookmark_cap/test_bookmark_import_stops_at_cap.
.share, #results .loc-title h2, .find-bar, .controls all exist in app.js's render output / home.html.j2; --accent/--surface-2/--border/--muted are all real tokens in style.css.
0004_bookmarks's down_revision chains correctly off 0003_user_discord_only → 0002 → 0001 (no branching).

Not run: the local checkout at ~/Code/thermograph predates this feature (no Bookmark model, no 0003/0004 migrations, no test_bookmarks.py present), and shell access to update/test it was unavailable in this session — so test_bookmarks.py was verified by inspection against the implementation, not executed.

Integration pass over PR #117 (bookmarked locations). Backend/frontend contract, alembic chain, and selectors all check out — one real defect found and fixed: **`bookmarks.js`'s `add()` treated a server *rejection* the same as being offline.** The POST to `/bookmarks` was wrapped in try/catch with the comment "fall through so the star still visibly sticks, even offline" — but the fallthrough to the local-only path ran on *any* non-2xx response, not just a network exception. Hit the per-user cap (409) or send bad input (422) while signed in, and the star still lit up "Saved": a bookmark that was never created server-side, silently written into local cache and then dropped without explanation the next time `syncFromServer` reconciles against the real list. Fix: only take the offline fallback on an actual fetch exception; a real HTTP rejection now throws so the caller sees it. `bookmarks-ui.js`'s star-toggle handler had no catch at all (an unhandled rejection), so it now surfaces the message via `alert()`, matching the existing error pattern in `subscriptions.js`. Everything else checked out clean: - Request/response field names (lat/lon/label/cell_id/id/created_at) match exactly between `schemas.py`/`api_accounts.py` and `bookmarks.js`. - `POST /bookmarks/import` returns `{imported, skipped, bookmarks}` as both `subscriptions.js` and `bookmarks-ui.js` assume. - `DELETE`/`PATCH /bookmarks/{id}` take an `int` path param; the frontend's `String(b.id)` values are always the server's numeric id when signed in, so they parse fine. - The per-user cap (200) is enforced identically in `create_bookmark` (409) and `import_bookmarks` (silently counts remaining items as skipped) — matches `test_bookmark_cap`/`test_bookmark_import_stops_at_cap`. - `.share`, `#results .loc-title h2`, `.find-bar`, `.controls` all exist in `app.js`'s render output / `home.html.j2`; `--accent`/`--surface-2`/`--border`/`--muted` are all real tokens in `style.css`. - `0004_bookmarks`'s `down_revision` chains correctly off `0003_user_discord_only` → `0002` → `0001` (no branching). Not run: the local checkout at `~/Code/thermograph` predates this feature (no `Bookmark` model, no `0003`/`0004` migrations, no `test_bookmarks.py` present), and shell access to update/test it was unavailable in this session — so `test_bookmarks.py` was verified by inspection against the implementation, not executed.

admin_emi added 2 commits 2026-07-26 18:48:37 +00:00