docs: add implementation plan for bookmark reorder/latest-chapter/favorites
Concrete backend + userscript plan against the approved design doc, including the Store.Upsert return-value fix needed to keep ordering correct once updated_at becomes conditional, and background-refresh triggering on both init() and SPA navigation per user preference. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,286 @@
|
||||
# Implementation plan: bookmark reorder, latest-chapter tracking, favorites
|
||||
|
||||
## Context
|
||||
|
||||
Design already approved and committed at
|
||||
`plans/2026-07-25-bookmark-list-favorites-design.md` on branch
|
||||
`feat/bookmark-list-favorites-latest`. It covers three requested userscript
|
||||
features plus one incidental bug found while verifying feasibility live via
|
||||
Playwright:
|
||||
|
||||
1. Bookmark list reordered so the most-recently-progressed manga is first.
|
||||
2. Show the latest *available* chapter for a manga, not just the last one
|
||||
read — including a background same-origin refresh mechanism to get closer
|
||||
to "live" without server-side polling (Cloudflare blocks that; confirmed
|
||||
live, and confirmed no JSON API / RSS exists on either site to poll
|
||||
instead).
|
||||
3. A favorites mechanism (star toggle + tabs) that doesn't remove a manga
|
||||
from the normal list.
|
||||
4. `asuracomic.net` deep links now 301-redirect straight to the
|
||||
`asurascans.com` homepage (path discarded) — a Cloudflare-edge redirect
|
||||
confirmed live, with no client-side fix possible. Doc-only correction.
|
||||
|
||||
This plan turns that design into concrete code changes against the actual
|
||||
current backend (Go/SQLite) and userscript, informed by full reads of
|
||||
`backend/store.go`, `backend/handlers.go`, `backend/store_test.go`,
|
||||
`backend/main.go`, and the full 782-line
|
||||
`userscript/manga-bookmark.user.js`.
|
||||
|
||||
## Key design decision surfaced during planning
|
||||
|
||||
Today `handlers.go`'s `put()` echoes back the client's decoded request
|
||||
struct as the API response, not what was actually persisted. Once
|
||||
`updated_at` is sometimes *not* bumped (this whole feature's core mechanic),
|
||||
echoing the request struct back would return a **wrong** `updated_at` to the
|
||||
caller on every no-bump write — silently breaking the ordering guarantee the
|
||||
entire feature depends on, since the userscript's `syncUpsert`/mutation
|
||||
helpers adopt whatever the server echoes back (`upsertLocal(saved)`) as the
|
||||
new source of truth. **`Store.Upsert` must therefore return the row as
|
||||
actually written (read back inside the same transaction), and `handlers.go`
|
||||
must respond with that**, not the client's payload. This is a correctness
|
||||
fix required by the design, not a new decision to re-litigate.
|
||||
|
||||
Confirmed via the user: background latest-chapter refresh should fire on
|
||||
Asura's SPA in-app navigation too (`onNavigate()`), not only true browser
|
||||
page loads (`init()`) — more refresh opportunities on a client-routed site
|
||||
that rarely does full reloads, still bounded by the same throttle/batch
|
||||
limits.
|
||||
|
||||
## Phase 1 — Backend (`backend/`), TDD
|
||||
|
||||
### 1.1 Tests first — `backend/store_test.go`
|
||||
|
||||
Add four tests (all go through the existing `newTestServer(t)` /
|
||||
`httptest` pattern already used in this file, since there are no direct
|
||||
`Store`-level unit tests in the current style):
|
||||
|
||||
- **`TestUpsertConditionalUpdatedAt`** — table-driven: new bookmark (bumps),
|
||||
unchanged progress (no bump), changed progress (bumps), favorite-only
|
||||
change (no bump), latest-chapter-only change (no bump). Assert on the
|
||||
`updated_at` returned by each PUT response.
|
||||
- **`TestFavoriteRoundTrip`** — PUT `favorite: true`, GET list, assert it
|
||||
round-trips.
|
||||
- **`TestLatestChapterNullable`** — PUT without `latest_chapter_num`, assert
|
||||
JSON response has `"latest_chapter_num":null`; PUT again with a value,
|
||||
assert it round-trips.
|
||||
- **`TestOpenStoreMigratesLegacySchema`** — hand-create the *old* (10-column)
|
||||
schema in a temp DB file, seed one row, then call `OpenStore` on it and
|
||||
assert the row survives with the new columns defaulting cleanly
|
||||
(`favorite=false`, `latest_chapter=""`, `latest_chapter_num=nil`). This is
|
||||
the safety net for the already-deployed production DB.
|
||||
|
||||
Run `cd backend && go test ./...` — expect compile failures (red state is
|
||||
correct/expected before 1.2).
|
||||
|
||||
### 1.2 `backend/store.go`
|
||||
|
||||
- **`Bookmark` struct**: add `Favorite bool `json:"favorite"``,
|
||||
`LatestChapter string `json:"latest_chapter"``,
|
||||
`LatestChapterNum *float64 `json:"latest_chapter_num"`` (nullable — only
|
||||
this one needs to be a pointer, per the design doc's data-model table).
|
||||
- **`schema`**: extend `CREATE TABLE IF NOT EXISTS` with
|
||||
`favorite INTEGER NOT NULL DEFAULT 0`,
|
||||
`latest_chapter TEXT NOT NULL DEFAULT ''`, `latest_chapter_num REAL`
|
||||
(covers fresh installs only).
|
||||
- **Idempotent migration for the already-deployed DB**: add a
|
||||
`migrateColumns(db)` helper using `PRAGMA table_info(bookmarks)` to check
|
||||
each new column's existence before running its `ALTER TABLE ... ADD
|
||||
COLUMN` (SQLite has no `ADD COLUMN IF NOT EXISTS`). Call it in
|
||||
`OpenStore` right after the existing `schema` exec succeeds, same
|
||||
error-wrapping style as today.
|
||||
- **Shared `scanBookmark` helper**: centralizes converting the `favorite`
|
||||
`INTEGER` (0/1) to `bool` and the nullable `latest_chapter_num` `REAL` to
|
||||
`*float64` via `sql.NullFloat64`, used by both `List()` and `Upsert()`'s
|
||||
read-back.
|
||||
- **`List()`**: extend the `SELECT` to the new columns, scan via
|
||||
`scanBookmark`.
|
||||
- **`Upsert(b Bookmark) (Bookmark, error)`** — signature changes to return
|
||||
the stored row. Implementation: wrap in `db.Begin()`/`tx.Commit()`
|
||||
(explicit "read exactly what I just wrote" guarantee rather than relying
|
||||
on `SetMaxOpenConns(1)` staying 1 forever). The `INSERT ... ON CONFLICT
|
||||
DO UPDATE SET` gets a `CASE` expression for `updated_at`:
|
||||
```sql
|
||||
updated_at = CASE
|
||||
WHEN bookmarks.last_chapter_num IS NOT excluded.last_chapter_num
|
||||
THEN excluded.updated_at
|
||||
ELSE bookmarks.updated_at
|
||||
END
|
||||
```
|
||||
This is valid SQLite upsert syntax (bare column = pre-update row value,
|
||||
`excluded.col` = proposed new row) and naturally handles "new row" for
|
||||
free — `ON CONFLICT DO UPDATE` only fires on the update path, so a
|
||||
genuinely new row goes through the plain `INSERT ... VALUES` and always
|
||||
gets the fresh `updated_at`. After the exec, `SELECT` the row back inside
|
||||
the same transaction and return it via `scanBookmark`.
|
||||
|
||||
### 1.3 `backend/handlers.go`
|
||||
|
||||
In `put()`: keep `b.UpdatedAt = time.Now().UnixMilli()` as a *candidate*
|
||||
value (update its comment — it's no longer unconditionally authoritative),
|
||||
then:
|
||||
```go
|
||||
stored, err := h.store.Upsert(b)
|
||||
...
|
||||
writeJSON(w, http.StatusOK, stored)
|
||||
```
|
||||
No other changes — `Favorite`/`LatestChapter`/`LatestChapterNum` already
|
||||
flow through untouched from the decoded body, which is correct (they're
|
||||
fully client-set synced fields). `list()`/`delete()` unchanged.
|
||||
|
||||
### 1.4 Green + build
|
||||
|
||||
```bash
|
||||
cd backend && go test ./... # all pass, including pre-existing TestBookmarkRoundTrip unmodified
|
||||
cd backend && CGO_ENABLED=0 go build # static binary still builds
|
||||
```
|
||||
|
||||
## Phase 2 — Userscript (`userscript/manga-bookmark.user.js`)
|
||||
|
||||
No JS test harness in this repo — verification is manual (Phase 4).
|
||||
|
||||
1. **Config constants** (near `CACHE_KEY`, ~line 24): `LASTCHECKED_KEY =
|
||||
"mangabm:lastchecked"`, `LATEST_CHECK_THROTTLE_MS = 4 * 60 * 60 * 1000`
|
||||
(4h), `LATEST_CHECK_BATCH = 1`.
|
||||
|
||||
2. **Shared anchor extraction** so the exact same per-site chapter-matching
|
||||
rule runs against both the live DOM and raw fetched HTML text (no HTML
|
||||
parser available for the fetch path): `anchorsFromDocument(doc)` (via
|
||||
`querySelectorAll("a[href]")`) and `anchorsFromHTML(html)` (regex-based
|
||||
`<a href="...">...</a>` extraction). Add near `meta()` (~line 34).
|
||||
|
||||
3. **Per-adapter `latestChapterFromAnchors(anchors)`** added to both `asura`
|
||||
and `demonic` adapter objects, implementing the regex rules from the
|
||||
design doc (Asura: href matches `/chapter/([\d.]+)$/` AND text matches
|
||||
`/Chapter\s+[\d.]+/i`, excluding the "First Chapter" quick-jump button;
|
||||
Demonic: all `chaptered.php?manga=\d+&chapter=([\d.]+)` matches, take
|
||||
max — no order assumption). Plus a `computeLatestChapter(site, anchors)`
|
||||
dispatcher near `keyOf()`.
|
||||
|
||||
4. **`mangabm:lastchecked` local helpers**: `loadLastChecked()` /
|
||||
`saveLastChecked(map)`, parallel to existing `loadCache`/`saveCache`
|
||||
(~line 154), storing `{ [bookmarkKey]: timestampMs }`. Client-local only,
|
||||
never synced.
|
||||
|
||||
5. **`applyLatestChapterIfChanged(existing, latest)`** (~near `syncUpsert`,
|
||||
line 295): if `latest.num` differs from the bookmark's stored
|
||||
`latest_chapter_num`, optimistically update local cache + render, then
|
||||
`apiPut` with `updated_at: Date.now()` as a *candidate* — the backend
|
||||
(Phase 1) decides whether to actually apply it, and the client adopts
|
||||
whatever comes back via `upsertLocal(saved)`, same pattern the rest of
|
||||
the file already uses. No client-side "don't reorder" logic needed
|
||||
beyond that — the server is the single source of truth for it. Silent
|
||||
on failure (no toast), per the design doc.
|
||||
|
||||
6. **Live-page capture**: `maybeCaptureLatestOnSeriesPage()` — on a
|
||||
`type: "series"` page for an already-bookmarked series, scan the live
|
||||
DOM via `anchorsFromDocument` + `computeLatestChapter`, then
|
||||
`applyLatestChapterIfChanged`. Hooked into `onNavigate()` (~line 633),
|
||||
after the existing `maybeAutoUpdate()` call.
|
||||
|
||||
7. **Background opportunistic refresh**: `backgroundRefreshLatest()` — get
|
||||
`currentSite()` (which adapter matches `window.location`), pick
|
||||
same-site bookmarks not checked within `LATEST_CHECK_THROTTLE_MS`
|
||||
(oldest-checked-first), fetch+parse at most `LATEST_CHECK_BATCH` of them
|
||||
via `fetch(bm.series_url).then(r => r.text())` → `anchorsFromHTML` →
|
||||
`computeLatestChapter` → `applyLatestChapterIfChanged`. Mark each
|
||||
attempted bookmark's `lastchecked` timestamp regardless of success/failure
|
||||
(advances the throttle window either way, avoiding hammering a
|
||||
consistently-failing fetch). Silent on failure.
|
||||
**Hook into both `init()` and `onNavigate()`** (per user's confirmed
|
||||
preference — more refresh opportunities on Asura's SPA navigation, same
|
||||
throttle/batch caps prevent request bursts either way).
|
||||
|
||||
8. **`toggleFavorite(key)`** (~near `setChapterManual`, line 293): flips
|
||||
`favorite`, optimistic update, `apiPut` with `Date.now()` candidate
|
||||
timestamp (again, backend decides), toast on success/failure (consistent
|
||||
with other explicit user-initiated actions like bookmark/remove).
|
||||
|
||||
9. **Tabs**: new module state `let activeTab = "all";` (~near `panelOpen`,
|
||||
line 374; not persisted, defaults to "all"). Wire click handlers in
|
||||
`buildUI()` for new `#tabAll`/`#tabFav` elements. In `render()` (~line
|
||||
568), toggle each tab's `.active` class and filter which array feeds the
|
||||
list-building loop (`activeTab === "favorites" ? state.list.filter(b =>
|
||||
b.favorite) : state.list`) — `state.list` itself is never mutated/filtered,
|
||||
so a favorited manga always still appears in "All".
|
||||
|
||||
10. **`renderItem(b)`** (~line 579): add a star toggle button (☆/★,
|
||||
`onclick: () => toggleFavorite(b.key)`) alongside the existing
|
||||
Continue/Edit/Remove buttons, and change the subtitle line to
|
||||
`"Read: " + last_chapter + " · Latest: " + latest_chapter` when
|
||||
`latest_chapter_num` is known and strictly greater than
|
||||
`last_chapter_num` (avoids showing "Latest: Chapter 12" next to "Read:
|
||||
Chapter 12" when they're numerically equal); otherwise keep today's
|
||||
`"<last_chapter> · <site>"` text.
|
||||
|
||||
11. **`TEMPLATE`** (~line 679): insert a tabs bar
|
||||
(`<div id="tabs"><button id="tabAll" class="tab active">All</button>
|
||||
<button id="tabFav" class="tab">★ Favorites</button></div>`) between
|
||||
`#context` and `#list`.
|
||||
|
||||
12. **`CSS`** (~near `.ctx-sub`/`.btn.danger`): add `.tab`/`.tab.active` and
|
||||
`.btn.star`/`.btn.star.active` rules following the existing dark-theme
|
||||
`.btn` modifier convention (`.btn.primary`, `.btn.small`, `.btn.danger`).
|
||||
|
||||
13. **Fix the misleading redirect comment** (line 41, in the `asura`
|
||||
adapter object): replace "asuracomic.net currently 301s to
|
||||
asurascans.com; match both." with an accurate note that the redirect
|
||||
now discards the path (goes straight to the asurascans.com root),
|
||||
happens at the Cloudflare edge before any JS runs, so no client-side
|
||||
fix is possible, and the user should navigate via asurascans.com links
|
||||
directly. No change to the `matches()` regex itself.
|
||||
|
||||
14. **`@version`**: bump `1.1.0` → `1.2.0`.
|
||||
|
||||
## Phase 3 — Documentation
|
||||
|
||||
- **`CLAUDE.md`**: update the `PUT /bookmarks/{key}` endpoint description
|
||||
(currently "upsert, server sets updated_at") to describe the new
|
||||
conditional rule, referencing
|
||||
`plans/2026-07-25-bookmark-list-favorites-design.md` §4.
|
||||
- **`README.md`**: update the matching endpoint-table row, and correct the
|
||||
"Adapter reference" section's Asura row to note `asuracomic.net` deep
|
||||
links currently 301 to the asurascans.com root (broken/path discarded) —
|
||||
use `asurascans.com` links directly. While touching this, also fix
|
||||
`CLAUDE.md`'s intro line ("asuracomic.net (formerly asurascans.com)"),
|
||||
which has the relationship backwards and is inconsistent with README's
|
||||
own phrasing ("asurascans.com (a.k.a. asuracomic.net)") — bundle this
|
||||
small adjacent correction in since it's directly related to the same
|
||||
finding.
|
||||
|
||||
## Verification
|
||||
|
||||
**Backend (automated):**
|
||||
```bash
|
||||
cd backend && go test ./...
|
||||
cd backend && CGO_ENABLED=0 go build
|
||||
```
|
||||
|
||||
**Backend (manual smoke test, extends the existing curl convention in
|
||||
CLAUDE.md/README.md):** PUT a new bookmark, then PUT again changing only
|
||||
`favorite`, then only `latest_chapter*`, then a real `last_chapter_num`
|
||||
advance — confirm via `jq .updated_at` that only the first and last calls
|
||||
change `updated_at`.
|
||||
|
||||
**Userscript (manual — no JS test harness exists in this repo, matching
|
||||
existing project convention):**
|
||||
- List reorders only on a genuine progress advance, not on plain re-visit,
|
||||
favorite toggle, or latest-chapter capture.
|
||||
- Latest-chapter capture fires when visiting a bookmarked series page on
|
||||
both sites and displays the "Read: X · Latest: Y" subtitle correctly.
|
||||
- Background refresh: check `localStorage['mangabm:lastchecked']` in
|
||||
devtools to confirm throttling behavior; confirm via the Network tab that
|
||||
it never fires a cross-site request (only same-origin as the currently
|
||||
loaded site).
|
||||
- Favorite toggle persists across a panel close/reopen and a `refresh()`
|
||||
round-trip through the backend; favorited manga still shows in "All".
|
||||
- Fastest iteration path: desktop Tampermonkey/Violentmonkey first (script
|
||||
stays `GM_*`-free), then confirm on Bromite per existing project
|
||||
convention.
|
||||
|
||||
## Critical files
|
||||
- `backend/store.go`
|
||||
- `backend/handlers.go`
|
||||
- `backend/store_test.go`
|
||||
- `userscript/manga-bookmark.user.js`
|
||||
- `CLAUDE.md`
|
||||
- `README.md`
|
||||
Reference in New Issue
Block a user