From ea7bb97112c218abeb457e1a424e852417bc9aa3 Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sat, 22 Aug 2026 17:05:09 +0700 Subject: [PATCH] Spec #136: reader-facing finished label on the flat Bookmark (#160) --- backend/api_test.go | 49 +++++++++++++++++++++++- backend/internal/store/store.go | 7 +++- backend/internal/store/store_test.go | 31 +++++++++++++++ backend/internal/web/static/style.css | 3 ++ backend/internal/web/templates/card.html | 4 ++ userscript/manga-bookmark.user.js | 17 +++++++- userscript/novel-bookmark.user.js | 17 +++++++- userscript/test/logic.test.js | 13 +++++++ userscript/test/novel-logic.test.js | 30 +++++++++++++++ 9 files changed, 165 insertions(+), 6 deletions(-) diff --git a/backend/api_test.go b/backend/api_test.go index 1037808..70e5923 100644 --- a/backend/api_test.go +++ b/backend/api_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "database/sql" "encoding/json" "fmt" "net/http" @@ -298,7 +299,7 @@ func TestFlatWireFieldSet(t *testing.T) { "series_url": true, "cover": true, "last_chapter": true, "last_chapter_num": true, "last_chapter_url": true, "favorite": true, "latest_chapter": true, "latest_chapter_num": true, "updated_at": true, - "status": true, "kind": true, + "status": true, "kind": true, "finished": true, } checkFlat := func(t *testing.T, payload []byte) map[string]json.RawMessage { t.Helper() @@ -362,6 +363,52 @@ func TestFlatWireFieldSet(t *testing.T) { checkFlat(t, body2) } +// finished is derived on the wire and read-only: a client PUT echoing a cached +// value, forward progress or not, must not change the Series' retired state, +// so GET still reports the truth after the echo (issues #157, #160). +func TestFinishedWireRoundTrip(t *testing.T) { + s, url := newTestStoreURL(t) + srv := newRouter(s, testConfig()) + key := "asura:done" + + putBookmark(t, srv, key, store.Bookmark{ + Title: "Solo Leveling", SeriesURL: "https://asurascans.com/comics/done", + LastChapterNum: 10, + }) + + // The store writer for the flag is the admin surface's own and lands in + // the same wave (#158), so seed the fact with SQL, like store_test.go. + db, err := sql.Open("pgx", url) + if err != nil { + t.Fatalf("open db: %v", err) + } + defer db.Close() + if _, err := db.Exec( + `UPDATE series SET finished_at = 1000 WHERE site = 'asura' AND series_id = 'done'`); err != nil { + t.Fatalf("seed finished: %v", err) + } + + got := getBookmarks(t, srv) + if len(got) != 1 || !got[0].Finished { + t.Fatalf("GET = %+v, want one bookmark carrying finished: true", got) + } + + // A stale cache echoing the flag cannot un-finish (or finish) the Series. + for _, sent := range []bool{false, true} { + echoed := putBookmark(t, srv, key, store.Bookmark{ + Title: "Solo Leveling", SeriesURL: "https://asurascans.com/comics/done", + LastChapterNum: 11, Finished: sent, + }) + if !echoed.Finished { + t.Fatalf("PUT echoing Finished: %v reported finished = false, want true", sent) + } + got = getBookmarks(t, srv) + if !got[0].Finished { + t.Fatalf("GET after echoing Finished: %v = false, want the Series state preserved", sent) + } + } +} + // A PUT naming an existing series must ignore client-supplied title, cover and // URL — the security boundary from ADR-0003, where a hostile site's scraped // values could otherwise land on a shared row — while progress still lands. diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index e6e6c5d..f7ca35b 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -54,6 +54,9 @@ type Bookmark struct { // Kind is the library bucket: manga or novel. Empty on the way in means // "no opinion" — see Upsert. Kind string `json:"kind"` + // Finished is the owner's retirement of the Series, derived: the flag is a + // Series fact and a client cannot write it — see Upsert. + Finished bool `json:"finished"` } // Series is one distinct work, shared by every bookmark that tracks it. It is @@ -230,7 +233,8 @@ var migrations embed.FS // order, so the flat Bookmark reads back whole despite the split (ADR-0004). const bookmarkColumns = `b.site, b.series_id, s.title, s.series_url, s.cover_address, b.last_chapter, b.last_chapter_num, b.last_chapter_url, - b.favorite, s.latest_chapter, s.latest_chapter_num, b.updated_at, b.status, s.kind` + b.favorite, s.latest_chapter, s.latest_chapter_num, b.updated_at, b.status, s.kind, + s.finished_at > 0` // seriesColumns is the series row in scanSeries order, used by the poller's // due query. latest_checked_at lives only on series — see MarkLatestChecked @@ -609,6 +613,7 @@ func (s *Store) scanBookmark(scan func(...any) error) (Bookmark, error) { &b.Site, &b.SeriesID, &b.Title, &b.SeriesURL, &coverAddress, &b.LastChapter, &b.LastChapterNum, &b.LastChapterURL, &b.Favorite, &b.LatestChapter, &latestChapterNum, &b.UpdatedAt, &b.Status, &b.Kind, + &b.Finished, ); err != nil { return Bookmark{}, err } diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index 70072f5..8480fec 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -934,6 +934,37 @@ func TestUpsertCreatesSeriesFromClient(t *testing.T) { } } +// The wire's finished flag is derived, never stored: the Upsert's explicit +// column list does not name finished_at — the same omission that protects +// cover — so a client echoing a cached flag, true or false, cannot change the +// Series' retired state (issues #157, #160). +func TestUpsertCannotWriteSeriesFinished(t *testing.T) { + store := newTestStore(t) + seedFinished(t, store, "asura:done", "https://asurascans.com/comics/asura:done") + + for _, sent := range []bool{true, false} { + stored, err := store.Upsert(store.OwnerID(), Bookmark{ + Key: "asura:done", Site: "asura", SeriesID: "done", + Finished: sent, UpdatedAt: 2000, + }) + if err != nil { + t.Fatalf("Upsert(Finished: %v): %v", sent, err) + } + if !stored.Finished { + t.Fatalf("stored.Finished = false after echoing %v, want true", sent) + } + } + var finishedAt int64 + if err := store.db.QueryRow( + `SELECT finished_at FROM series WHERE site = $1 AND series_id = $2`, + "asura", "done").Scan(&finishedAt); err != nil { + t.Fatalf("read finished_at: %v", err) + } + if finishedAt != 1000 { + t.Fatalf("series.finished_at = %v, want the seeded 1000 untouched", finishedAt) + } +} + // The hook is what starts creation-time acquisition, so it must fire exactly // once per Series — on the PUT that created it, and on no later one, whichever // Reader sends it. diff --git a/backend/internal/web/static/style.css b/backend/internal/web/static/style.css index 14a790e..1da224c 100644 --- a/backend/internal/web/static/style.css +++ b/backend/internal/web/static/style.css @@ -84,6 +84,7 @@ /* One accent per action, so a press says which lane it belongs to. All three are held at the same weight as --brass: muted, no ember competition. */ --slate: #7fa0c0; /* archive */ + --moss: #7fae86; /* finished */ --clay: #b5906f; /* set chapter */ --trash: #977671; /* remove, resting — icons need 3:1, not 4.5:1 */ /* A Lane needing attention: the admin page's only accent. Verdigris — cool, @@ -147,6 +148,7 @@ --danger-soft: #7c2c22; --brass: #8a681c; --slate: #3f6689; + --moss: #3d6c46; --clay: #7c5533; --trash: #8c6558; --patina: #1f6f66; @@ -548,6 +550,7 @@ button { cursor: pointer; } .new-chapter { color: var(--ember); } .state { display: flex; align-items: center; gap: 4px; color: var(--mute); } .state svg { width: 10px; height: 10px; } +.state.finished { color: var(--moss); } .is-dim .meta { color: var(--mute-2); } .is-dim .site-asura, .is-dim .site-demonic, .is-dim .site-comix, .is-dim .site-kagane, diff --git a/backend/internal/web/templates/card.html b/backend/internal/web/templates/card.html index d6b40e5..6175c16 100644 --- a/backend/internal/web/templates/card.html +++ b/backend/internal/web/templates/card.html @@ -33,6 +33,10 @@ / archived {{end}} + {{if .Finished}} + / + finished + {{end}}

diff --git a/userscript/manga-bookmark.user.js b/userscript/manga-bookmark.user.js index d511c42..b82e753 100644 --- a/userscript/manga-bookmark.user.js +++ b/userscript/manga-bookmark.user.js @@ -551,6 +551,12 @@ return b.kind || "manga"; } + // finished is a fact about the Series, decided from the owner's side and + // derived on the wire (issue #157); the panel only labels it. + function finishedLabel(b) { + return b && b.finished ? "Finished" : ""; + } + // ============================================================ // Retry queue // @@ -1399,6 +1405,7 @@ el("div", { class: "meta" }, [ el("a", { class: "go-t t", href: cont, text: b.title || b.series_id }), el("div", { class: "c" + (behind ? " behind" : ""), text: sub }), + b.finished && el("span", { class: "finished", text: finishedLabel(b) }), el("div", { class: "actions" }, [ el("button", { class: "btn small star" + (b.favorite ? " on" : ""), @@ -1669,7 +1676,7 @@ --paper: #f2ece5; --paper-hot: #f0d3cb; --paper-dim: #ddd5cb; --mute: #8d857c; --mute-2: #5a5450; --faint: #3a3733; --faint-2: #57504b; --ember: #e0452c; --ember-wash: #1a1211; --ember-ink: #150907; - --ember-soft: #eda798; --brass: #b8912f; --trash: #6b5450; + --ember-soft: #eda798; --brass: #b8912f; --trash: #6b5450; --moss: #7fae86; --font-display: Georgia, "Times New Roman", serif; --font-mono: ui-monospace, SFMono-Regular, Menlo, monospace; --font-body: system-ui, -apple-system, sans-serif; @@ -1788,6 +1795,12 @@ font: 400 17px/1.2 var(--font-display); color: var(--paper-dim); max-width: 100%; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; } + /* finished: a Series fact, not a state class (issue #157) — same meta + typography as .c, moss instead of mute. */ + .finished { + margin: 0; color: var(--moss); + font: 500 10px/1.4 var(--font-mono); letter-spacing: .12em; text-transform: uppercase; + } .item.hot .t { color: var(--paper-hot); border-bottom: 1px solid var(--ember); padding-bottom: 3px; } .item.dim .t { font-style: italic; color: var(--mute); } .c { @@ -1838,7 +1851,7 @@ // Exposes pure logic only — see userscript/test/logic.test.js. // ============================================================ if (typeof window === "undefined" && typeof module === "object" && module.exports) { - module.exports = { stripBuildHash, comixSeriesId, asura, demonic, comix, kagane, anchorsFromHTML, statusOf, kindOf }; + module.exports = { stripBuildHash, comixSeriesId, asura, demonic, comix, kagane, anchorsFromHTML, statusOf, kindOf, finishedLabel }; } // ============================================================ diff --git a/userscript/novel-bookmark.user.js b/userscript/novel-bookmark.user.js index cf27813..ba563d4 100644 --- a/userscript/novel-bookmark.user.js +++ b/userscript/novel-bookmark.user.js @@ -486,6 +486,12 @@ return b.kind || "manga"; } + // finished is a fact about the Series, decided from the owner's side and + // derived on the wire (issue #157); the panel only labels it. + function finishedLabel(b) { + return b && b.finished ? "Finished" : ""; + } + // ============================================================ // Retry queue // @@ -1306,6 +1312,7 @@ el("div", { class: "meta" }, [ el("a", { class: "go-t t", href: cont, text: b.title || b.series_id }), el("div", { class: "c" + (behind ? " behind" : ""), text: sub }), + b.finished && el("span", { class: "finished", text: finishedLabel(b) }), el("div", { class: "actions" }, [ el("button", { class: "btn small star" + (b.favorite ? " on" : ""), @@ -1594,7 +1601,7 @@ --paper: #f2ece5; --paper-hot: #f0d3cb; --paper-dim: #ddd5cb; --mute: #8d857c; --mute-2: #5a5450; --faint: #3a3733; --faint-2: #57504b; --ember: #e0452c; --ember-wash: #1a1211; --ember-ink: #150907; - --ember-soft: #eda798; --brass: #b8912f; --trash: #6b5450; + --ember-soft: #eda798; --brass: #b8912f; --trash: #6b5450; --moss: #7fae86; --font-display: Georgia, "Times New Roman", serif; --font-mono: ui-monospace, SFMono-Regular, Menlo, monospace; --font-body: system-ui, -apple-system, sans-serif; @@ -1720,6 +1727,12 @@ font: 500 10px/1.4 var(--font-mono); letter-spacing: .12em; text-transform: uppercase; } .c.behind { color: var(--ember); } + /* finished: a Series fact, not a state class (issue #157) — same meta + typography as .c, moss instead of mute. */ + .finished { + margin: 0; color: var(--moss); + font: 500 10px/1.4 var(--font-mono); letter-spacing: .12em; text-transform: uppercase; + } .actions { display: flex; gap: 0; flex-wrap: wrap; margin-top: 2px; } .btn { background: none; color: var(--mute); border: 1px solid var(--rule); @@ -1763,7 +1776,7 @@ // Exposes pure logic only — see userscript/test/novel-logic.test.js. // ============================================================ if (typeof window === "undefined" && typeof module === "object" && module.exports) { - module.exports = { novelfull, lightnovelworld, anchorsFromHTML, statusOf, kindOf, maxChapter, escapeRe, computeLatestChapter, repairLnwStaleRow }; + module.exports = { novelfull, lightnovelworld, anchorsFromHTML, statusOf, kindOf, maxChapter, escapeRe, computeLatestChapter, repairLnwStaleRow, finishedLabel }; } // ============================================================ diff --git a/userscript/test/logic.test.js b/userscript/test/logic.test.js index ff048f1..84971d8 100644 --- a/userscript/test/logic.test.js +++ b/userscript/test/logic.test.js @@ -58,6 +58,7 @@ const { anchorsFromHTML, statusOf, kindOf, + finishedLabel, } = require("../manga-bookmark.user.js"); // detect() reads only these four properties off location. @@ -397,6 +398,18 @@ test("statusOf defaults a missing status to reading", () => { assert.equal(statusOf({ status: "archived" }), "archived"); }); +// ============================================================ +// finishedLabel — finished is a fact about the Series (issue #157), which is +// the server's business; the panel only names it. The label is text through +// the el() helper, never markup. +// ============================================================ + +test("finishedLabel names a finished series and nothing else", () => { + assert.equal(finishedLabel({ finished: true }), "Finished"); + assert.equal(finishedLabel({ finished: false }), ""); + assert.equal(finishedLabel({}), ""); +}); + // ============================================================ // kindOf — a row written before the kind column existed has none, and every // one of those is manga. diff --git a/userscript/test/novel-logic.test.js b/userscript/test/novel-logic.test.js index 5471694..06fbd6c 100644 --- a/userscript/test/novel-logic.test.js +++ b/userscript/test/novel-logic.test.js @@ -49,6 +49,7 @@ const { kindOf, maxChapter, repairLnwStaleRow, + finishedLabel, } = require("../novel-bookmark.user.js"); function loc(href) { @@ -563,6 +564,23 @@ test("repairLnwStaleRow does not regress progress when the repaired-key row is a assert.equal(out.list[0].last_chapter_num, 100); }); +// ============================================================ +// Merge rank — exactly two values: archived beats reading, and nothing else +// can ever win (issue #157 retired the finished bucket). +// ============================================================ + +test("repairLnwStaleRow merge keeps archived over reading and lets no third value win", () => { + const canonical = (over) => staleRow(Object.assign({ key: "lightnovelworld:immortality-simulator" }, over)); + // archived beats reading whether it arrives as the stale row or the repaired one + let out = repairLnwStaleRow([canonical({ status: "archived" }), staleRow({ status: "reading" })], [], {}, lnwPage()); + assert.equal(out.list[0].status, "archived"); + out = repairLnwStaleRow([staleRow({ status: "archived" }), canonical({ status: "reading" })], [], {}, lnwPage()); + assert.equal(out.list[0].status, "archived"); + // "finished" is not a bucket anymore: a row carrying it reads as the + // default, so it can never win (issue #157). + out = repairLnwStaleRow([staleRow({ status: "finished" }), canonical({ status: "reading" })], [], {}, lnwPage()); + assert.equal(out.list[0].status, "reading"); +}); // ============================================================ // kindOf // ============================================================ @@ -575,6 +593,18 @@ test("kindOf passes through novel", () => { assert.equal(kindOf({ kind: "novel" }), "novel"); }); +// ============================================================ +// finishedLabel — finished is a fact about the Series (issue #157), which is +// the server's business; the panel only names it. The label is text through +// the el() helper, never markup. +// ============================================================ + +test("finishedLabel names a finished series and nothing else", () => { + assert.equal(finishedLabel({ finished: true }), "Finished"); + assert.equal(finishedLabel({ finished: false }), ""); + assert.equal(finishedLabel({}), ""); +}); + // ============================================================ // Source guard //