From 77e3f710ec021ed003f0cad4d03c150c134cbd7e Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sat, 22 Aug 2026 13:02:09 +0700 Subject: [PATCH 1/7] Spec #136: finished becomes a Series fact, Lane gate rewritten on it (#157) finished_at lands on series (0016 seeds it from the pre-flip finished bookmarks, then flips those bookmarks to archived), the poll gate and eligible count read the flag instead of a per-Reader vote, PUT rejects the finished status like any unknown value, and the web UI drops the Finished tab, badge and finish button. Novel merge rank is archived > reading. Per-Reader disagreement (one Reader keeps a finished Series in reading forever) is what the flag repairs; the cutover keeps polling state unchanged for every Series. --- CONTEXT.md | 8 +- backend/AGENTS.md | 14 +- backend/internal/api/handlers.go | 9 +- backend/internal/store/admin.go | 6 +- .../store/migrations/0016_finished_series.sql | 24 +++ backend/internal/store/store.go | 71 ++++--- backend/internal/store/store_test.go | 201 +++++++++++++++--- backend/internal/web/static/filter.js | 6 +- backend/internal/web/static/style.css | 20 +- backend/internal/web/templates/app.html | 4 - backend/internal/web/templates/card.html | 26 +-- backend/internal/web/templates/chrome.html | 8 +- backend/internal/web/templates/icons.html | 1 - backend/internal/web/templates/list.html | 2 - backend/internal/web/web.go | 21 +- backend/main_test.go | 2 +- backend/web_test.go | 40 ++-- docs/adr/0015-finished-is-a-series-fact.md | 109 ++++++++++ userscript/manga-bookmark.user.js | 16 +- userscript/novel-bookmark.user.js | 23 +- userscript/test/logic.test.js | 1 - 21 files changed, 423 insertions(+), 189 deletions(-) create mode 100644 backend/internal/store/migrations/0016_finished_series.sql create mode 100644 docs/adr/0015-finished-is-a-series-fact.md diff --git a/CONTEXT.md b/CONTEXT.md index 57b887d..bd54514 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -95,7 +95,7 @@ _Avoid_: run, cycle, tick, batch, poll history **Forced Poll**: A Poll the owner asks for by hand instead of waiting for the Series's turn. It jumps its Lane's queue and ignores every waiting rule — the rest between Polls, a Sighting standing -in for a check, a Series only finished Readers hold — but never overrules a Site that is +in for a check, a finished Series — but never overrules a Site that is refusing us, the Lane's spacing between fetches, or a Series with no page to fetch. Asked for by marking the Series, never by commanding the poller, so it happens on the Lane's next pass rather than at the moment of asking. @@ -149,8 +149,10 @@ accent is permitted to signal. _Avoid_: unread, update available **Lifecycle bucket**: -Which of three mutually exclusive states a Bookmark sits in — reading, archived, or -finished. A Bookmark is in exactly one. Orthogonal to being a favourite. +Which of the two states a Bookmark sits in — reading or archived. A Bookmark is in exactly +one. Orthogonal to being a favourite. Finished is not a bucket: it is a fact about the +Series (see `series.finished_at`), owned by the owner and stamped once, and every Bookmark +on a finished Series is archived. _Avoid_: state, status (as a domain word), list **Favourite**: diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 49d3443..7501da1 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -200,12 +200,16 @@ stored** and clients must adopt that response rather than their own payload. ### Lifecycle buckets — `status` on each bookmark -`reading` | `archived` | `finished`, orthogonal to `favorite`. Archived and -finished appear only in their own tab, never in All, Updated, Favourites or the -recent strip. The poller keeps checking archived series and skips finished ones. +`reading` | `archived`, orthogonal to `favorite`. Archived rows appear only +in their own tab, never in All, Updated, Favourites or the recent strip. The +poller keeps checking archived series; a finished Series (issue #157) is a +`series.finished_at` fact the Lane gate reads, with every bookmark on it +archived. -- `finished` is settable only from the web UI; `PUT /bookmarks/{key}` rejects - it with 400. +- `PUT /bookmarks/{key}` accepts only the two values; anything else — + `finished` included — is a plain 400, and the web UI's own status control + validates the same way. The 0016 migration is the only writer of the flag + today; the undo is writing 0. - **An empty incoming status means "keep the stored one"**, and it is resolved on the `VALUES` side of `Store.Upsert`, not in the conflict clause: `excluded.*` is the post-evaluation row, so a default applied there would diff --git a/backend/internal/api/handlers.go b/backend/internal/api/handlers.go index 94f42bd..bc21ac5 100644 --- a/backend/internal/api/handlers.go +++ b/backend/internal/api/handlers.go @@ -73,14 +73,11 @@ func (h *Handler) Put(w http.ResponseWriter, r *http.Request) { } // An empty status is "no opinion" and Upsert keeps the stored bucket. - // Finishing a series is a web-UI decision, so the JSON API refuses it - // rather than trusting every client to leave it alone. + // Anything else outside the two lifecycle buckets is a client bug, not + // something to silently coerce — finished included, which is no longer a + // bucket at all (issue #157). switch b.Status { case "", store.StatusReading, store.StatusArchived: - case store.StatusFinished: - http.Error(w, "status "+store.StatusFinished+" can only be set from the web UI", - http.StatusBadRequest) - return default: http.Error(w, "invalid status", http.StatusBadRequest) return diff --git a/backend/internal/store/admin.go b/backend/internal/store/admin.go index 376c3e1..ece6477 100644 --- a/backend/internal/store/admin.go +++ b/backend/internal/store/admin.go @@ -157,9 +157,9 @@ func adminFilter(f SeriesFilter) (where, having string, args []any, err error) { // SeriesPage returns one page of the Series matching the filter, least // recently checked first. The LEFT JOIN to Bookmarks is what surfaces the // orphans that hygiene has to find — an inner join would hide them, exactly -// as the Lane's join does. ReaderCount is a plain count of every Bookmark on -// the Series, which knowingly disagrees with the two Lane queries for as long -// as the finished lifecycle bucket exists (#140). +// as the Lane's join does. Every bookmark keeps its Series polled now that +// finished is a Series flag, so this plain ReaderCount agrees with the Lane +// queries (issue #157). // // The tie-break is mandatory, not decorative: every unpollable Series shares a // zero check stamp, so ordering on that column alone gives no stable page diff --git a/backend/internal/store/migrations/0016_finished_series.sql b/backend/internal/store/migrations/0016_finished_series.sql new file mode 100644 index 0000000..a565412 --- /dev/null +++ b/backend/internal/store/migrations/0016_finished_series.sql @@ -0,0 +1,24 @@ +-- finished_at is "the owner marked this Series finished" (#157): epoch ms, +-- zero means not finished, and it doubles as the undo (write zero). The poll +-- gate reads it — a Series is polled only while finished_at = 0 — never a +-- bookmark's status. +ALTER TABLE series ADD COLUMN finished_at bigint NOT NULL DEFAULT 0; + +-- Column first, seed second, flip third — the order is load-bearing: a seed +-- that ran after the flip would read the buckets it just destroyed, declare +-- nothing finished, and silently resume polling on Series nobody chose to +-- resume. The seed mirrors the pre-cutover due gate exactly: a Series stays +-- polled while any bookmark is outside the finished bucket, so a Series whose +-- every bookmark sits in it is stamped, one click from being read again +-- afterwards. The stamp is the only memory of the bucket the flip is about to +-- erase. +UPDATE series s SET finished_at = (EXTRACT(EPOCH FROM now()) * 1000)::bigint + WHERE EXISTS (SELECT 1 FROM bookmarks b + WHERE b.site = s.site AND b.series_id = s.series_id) + AND NOT EXISTS (SELECT 1 FROM bookmarks b + WHERE b.site = s.site AND b.series_id = s.series_id + AND b.status <> 'finished'); + +-- The Lifecycle bucket is gone; a finished bookmark is an archived one. The +-- flip must come after the seed, which still reads the bucket. +UPDATE bookmarks SET status = 'archived' WHERE status = 'finished'; \ No newline at end of file diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index ba57a0c..e6e6c5d 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -47,8 +47,8 @@ type Bookmark struct { LatestChapter string `json:"latest_chapter"` LatestChapterNum *float64 `json:"latest_chapter_num"` // nil until first captured UpdatedAt int64 `json:"updated_at"` // unix ms; see Upsert - // Status is the lifecycle bucket: reading, archived, or finished. - // Archived series stay polled for new chapters; finished ones do not. + // Status is the lifecycle bucket: reading or archived. + // Archived series stay polled for new chapters. // Empty on the way in means "no opinion" — see Upsert. Status string `json:"status"` // Kind is the library bucket: manga or novel. Empty on the way in means @@ -214,10 +214,11 @@ const ( ) // Lifecycle buckets. A bookmark is in exactly one; favorite is orthogonal. +// Finished is not a bucket: it is a fact about the Series (series.finished_at), +// never about a Reader's bookmark. const ( StatusReading = "reading" StatusArchived = "archived" - StatusFinished = "finished" ) //go:embed migrations/*.sql @@ -619,9 +620,9 @@ func (s *Store) scanBookmark(scan func(...any) error) (Bookmark, error) { // bookmark is keyed (reader_id, site, series_id) (issue #22). b.Key = b.Site + ":" + b.SeriesID // An unrecognised bucket (a hand-edited row) would leave the row in no list - // at all, so anything outside the three known buckets reads as the default + // at all, so anything outside the two known buckets reads as the default // rather than being passed through. - if b.Status != StatusReading && b.Status != StatusArchived && b.Status != StatusFinished { + if b.Status != StatusReading && b.Status != StatusArchived { b.Status = StatusReading } return b, nil @@ -1302,10 +1303,12 @@ func (s *Store) LaneGates(site string) (pausedUntil, refuseUntil int64, err erro // // A forced Series (force_poll_at newer than latest_checked_at, issue #146) // overrides exactly three gates: the rest cutoff, the Sighting-deferral -// clause and the finished-only bucket. It never overrides an empty -// series_url or the Bookmarks join — nothing to fetch, and no consumer for -// the result — so those stay unconditional. Forced rows sort to the front of -// the queue; the reader-count-then-age ordering among the rest is ADR-0003. +// clause and a finished Series. It never overrides an empty series_url or the +// Bookmarks join — nothing to fetch, and no consumer for the result — so +// those stay unconditional, and it never clears the finish: nothing here +// writes finished_at, and pending force clears itself when the pass stamps +// the check timestamp. Forced rows sort to the front of the queue; the +// reader-count-then-age ordering among the rest is ADR-0003. // // The reader_count ordering is the point of the split (ADR-0003): a series // shared by several readers is fetched once per due cycle, and the popular @@ -1313,14 +1316,15 @@ func (s *Store) LaneGates(site string) (pausedUntil, refuseUntil int64, err erro // reader count, oldest-first keeps the poll fair when the backlog outgrows // throughput: the most neglected series is always next, so a large collection // refreshes uniformly slower rather than leaving a tail that never refreshes -// at all. The userscript sorts its own queue the same way (L453). +// at all. The userscript sorts its own queue the same way. // // Series with no series_url are skipped — there is nothing to fetch, which is -// the same filter the userscript applies at L452. Series whose only bookmarks -// are finished are skipped too: nothing more is coming, so fetching them only -// burns requests. Archived bookmarks still count — knowing what a shelved -// series is up to is the whole reason for archiving instead of deleting. -// A series with no bookmarks at all never appears: the join excludes it. +// the same filter the userscript applies before refreshing. A finished Series is +// skipped unless forced: nothing more is coming, so fetching it only burns +// requests (issue #157). Archived bookmarks still count — knowing what a +// shelved series is up to is the whole reason for archiving instead of +// deleting. A series with no bookmarks at all never appears: the join +// excludes it. // // ceilingMs is the Sighting deferral ceiling (issue #103): a Series whose last // real Poll is older than it appears however recently it was sighted. That is @@ -1329,10 +1333,10 @@ func (s *Store) LaneGates(site string) (pausedUntil, refuseUntil int64, err erro // decided here, from two facts the query already computes, so a Lane gains no // query per round: a Sighting younger than cutoffMs holds the Series back, but // only while COUNT(*) is 1. A Series a second Reader bookmarks is Polled on -// schedule, so a wrong value the whole guild can see is corrected by a check -// that was never postponed; on a solitary Series the only person a wrong value -// reaches is the Reader who reported it. Whether the reporting Reader is -// allowed to defer at all was settled when the Sighting was recorded — see +// the schedule, so a wrong value the guild can see is corrected by a check +// that was never postponed; on a solitary Series the only person a wrong +// value reaches is the Reader who reported it. Whether the reporting Reader +// is allowed to defer at all was settled when the Sighting was recorded — see // RecordSighting. func (s *Store) DueForLatestCheck(site string, cutoffMs, ceilingMs int64) ([]Series, error) { rows, err := s.db.Query(`SELECT `+seriesColumns+`, @@ -1344,12 +1348,11 @@ func (s *Store) DueForLatestCheck(site string, cutoffMs, ceilingMs int64) ([]Ser AND s.series_url <> '' AND (s.latest_checked_at <= $2::bigint OR s.force_poll_at > s.latest_checked_at) + AND (s.finished_at = 0 OR s.force_poll_at > s.latest_checked_at) GROUP BY s.site, s.series_id, s.title, s.series_url, s.cover, s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at, - s.force_poll_at - HAVING (COUNT(*) FILTER (WHERE b.status <> 'finished') > 0 - OR s.force_poll_at > s.latest_checked_at) - AND (COUNT(*) > 1 + s.force_poll_at, s.finished_at + HAVING (COUNT(*) > 1 OR s.latest_sighted_at <= $2::bigint OR s.latest_checked_at <= $3::bigint OR s.force_poll_at > s.latest_checked_at) @@ -1371,14 +1374,18 @@ func (s *Store) DueForLatestCheck(site string, cutoffMs, ceilingMs int64) ([]Ser return out, rows.Err() } -// EligibleSeriesCount returns how many of a Site's Series still have at least -// one bookmark outside the finished bucket. It is the denominator of the -// Lane's pace (issue #100): the effective gap is the smaller of the registry -// gap and one hour divided by this count, so Series that will never be Polled -// do not make the Lane faster than it needs to be, and counting every eligible -// Series rather than only those currently due keeps the pace steady — the -// single worst moment to be fastest is startup, when everything is due at -// once. +// EligibleSeriesCount returns how many of a Site's Series are not finished — +// the flag, never a Reader vote. It is the denominator of the Lane's pace +// (issue #100): the effective gap is the smaller of the registry gap and one +// hour divided by this count, so Series that will never be Polled do not make +// the Lane faster than it needs to be, and counting every eligible Series +// rather than only those currently due keeps the pace steady — the single +// worst moment to be fastest is startup, when everything is due at once. +// +// The deliberate asymmetry with DueForLatestCheck's WHERE: a forced Series +// is due but never admitted here, because a forced pass must not speed up +// every other fetch on the Site — one impassioned press is not a reason to +// hammer the Site (issue #157). func (s *Store) EligibleSeriesCount(site string) (int, error) { var n int err := s.db.QueryRow(`SELECT COUNT(*) FROM ( @@ -1386,8 +1393,8 @@ func (s *Store) EligibleSeriesCount(site string) (int, error) { FROM series s JOIN bookmarks b ON b.site = s.site AND b.series_id = s.series_id WHERE s.site = $1 + AND s.finished_at = 0 GROUP BY s.site, s.series_id - HAVING COUNT(*) FILTER (WHERE b.status <> 'finished') > 0 ) e`, site).Scan(&n) if err != nil { return 0, fmt.Errorf("count eligible series %s: %w", site, err) diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index 4cb05c5..70072f5 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -279,6 +279,27 @@ func seedForCheck(t *testing.T, s *Store, key, seriesURL string, checkedAt int64 } } +// seedFinished inserts a bookmark (and with it its series) and stamps the +// series finished — the series-level fact the due gate reads (issue #157). +func seedFinished(t *testing.T, s *Store, key, seriesURL string) { + t.Helper() + site, seriesID, ok := strings.Cut(key, ":") + if !ok { + t.Fatalf("key %q: no ':' separator", key) + } + if _, err := s.Upsert(s.OwnerID(), Bookmark{ + Key: key, Site: site, SeriesID: seriesID, SeriesURL: seriesURL, + UpdatedAt: 1000, + }); err != nil { + t.Fatalf("seed %q: %v", key, err) + } + if _, err := s.db.Exec( + `UPDATE series SET finished_at = 1000 WHERE site = $1 AND series_id = $2`, + site, seriesID); err != nil { + t.Fatalf("seed finish %q: %v", key, err) + } +} + // noCeiling is a Sighting deferral ceiling no Series can reach, for the tests // that predate the ceiling and are about rest, ordering or buckets instead. const noCeiling = int64(-1) @@ -486,24 +507,15 @@ func TestUpsertStatusChangeKeepsUpdatedAt(t *testing.T) { t.Fatalf("UpdatedAt = %d, want it frozen at %d", stored.UpdatedAt, first.UpdatedAt) } } - // Archiving is the reason to keep polling — the point is to come back to a -// series that has moved on. A finished series has nothing left to publish. +// series that has moved on. A finished series has nothing left to publish, +// whichever Reader marked it (issue #157). func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) { store := newTestStore(t) - for _, tc := range []struct{ key, status string }{ - {"asura:reading", StatusReading}, - {"asura:archived", StatusArchived}, - {"asura:finished", StatusFinished}, - } { - if _, err := store.Upsert(store.OwnerID(), Bookmark{ - Key: tc.key, Site: "asura", SeriesID: strings.TrimPrefix(tc.key, "asura:"), - SeriesURL: "https://asurascans.com/comics/" + tc.key, - Status: tc.status, UpdatedAt: time.Now().UnixMilli(), - }); err != nil { - t.Fatalf("seed %s: %v", tc.key, err) - } + for _, key := range []string{"asura:reading", "asura:archived"} { + seedForCheck(t, store, key, "https://asurascans.com/comics/"+key, 0) } + seedFinished(t, store, "asura:finished", "https://asurascans.com/comics/asura:finished") due, err := store.DueForLatestCheck("asura", time.Now().UnixMilli(), noCeiling) if err != nil { @@ -522,27 +534,26 @@ func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) { } // The gap's denominator counts every Series the Lane will ever Poll: a -// finished Series must not make the Lane faster than it needs to be, and -// another Site's Series must not leak into this Site's count. +// finished Series must not make the Lane faster than it needs to be, another +// Site's Series must not leak into this Site's count, and a forced Series is +// not admitted either — it is due once, not a reason to tighten the pace for +// every other fetch on the Site (issue #157). func TestEligibleSeriesCount(t *testing.T) { store := newTestStore(t) seedForCheck(t, store, "asura:reading", "https://asurascans.com/comics/reading", 0) seedForCheck(t, store, "asura:archived", "https://asurascans.com/comics/archived", 0) - if _, err := store.Upsert(store.OwnerID(), Bookmark{ - Key: "asura:finished", Site: "asura", SeriesID: "finished", - SeriesURL: "https://asurascans.com/comics/finished", - Status: StatusFinished, UpdatedAt: 1000, - }); err != nil { - t.Fatalf("seed finished: %v", err) - } + seedFinished(t, store, "asura:finished", "https://asurascans.com/comics/asura:finished") seedForCheck(t, store, "demonic:z", "https://demonicscans.org/manga/z", 0) + if err := store.ForceSeriesPoll("asura", "archived", 5000); err != nil { + t.Fatalf("ForceSeriesPoll: %v", err) + } n, err := store.EligibleSeriesCount("asura") if err != nil { t.Fatalf("EligibleSeriesCount: %v", err) } if n != 2 { - t.Fatalf("eligible = %d, want 2 (finished excluded, demonic excluded)", n) + t.Fatalf("eligible = %d, want 2 (finished and forced excluded, demonic excluded)", n) } n, err = store.EligibleSeriesCount("demonic") if err != nil { @@ -755,6 +766,133 @@ func TestMigration0008DropsLegacyCoverRows(t *testing.T) { } } +// 0016's three statements are order-dependent: the finished_at seed must see +// the pre-flip 'finished' buckets, and the flip must come after it. A seed +// that ran after the flip would read bookmarks that no longer say +// 'finished', declare nothing finished, and silently resume polling series +// somebody marked done. This seeds a pre-0016 database the way every +// deployment looked — finished a bookmark bucket, no finished_at column — +// runs the migration, and asserts on what it preserved. +func TestMigration0016SeedsFinishedAtBeforeFlippingBucket(t *testing.T) { + url := pgtest.URL(t) + db, err := sql.Open("pgx", url) + if err != nil { + t.Fatalf("open: %v", err) + } + defer db.Close() + + if err := migrate(db, 15); err != nil { + t.Fatalf("migrate to 0015: %v", err) + } + if err := seedOwner(db, testOwner); err != nil { + t.Fatalf("seed owner: %v", err) + } + // A second reader, so "mixed" can carry two bookmarks on one series. + if _, err := db.Exec( + `INSERT INTO readers (discord_id, token_sha256) VALUES ('second', '\x01'::bytea)`); err != nil { + t.Fatalf("seed second reader: %v", err) + } + + owner := func() int64 { + t.Helper() + var id int64 + if err := db.QueryRow( + `SELECT id FROM readers WHERE discord_id = $1`, testOwner.DiscordID).Scan(&id); err != nil { + t.Fatalf("owner id: %v", err) + } + return id + }() + seedBookmark := func(readerID int64, site, seriesID, status string) { + t.Helper() + // The bookmark FK demands the series row; Upsert would create it on + // the fly, raw SQL has to spell it out. + if _, err := db.Exec( + `INSERT INTO series (site, series_id) VALUES ($1, $2) ON CONFLICT DO NOTHING`, + site, seriesID); err != nil { + t.Fatalf("seed series %s:%s: %v", site, seriesID, err) + } + if _, err := db.Exec( + `INSERT INTO bookmarks (reader_id, site, series_id, status, updated_at) + VALUES ($1, $2, $3, $4, 1000)`, readerID, site, seriesID, status); err != nil { + t.Fatalf("seed %s:%s/%s: %v", site, seriesID, status, err) + } + } + // done: the one-bookmark series every finished series looked like. + seedBookmark(owner, "asura", "done", "finished") + // shared: every reader finished — the multi-reader equivalent. + seedBookmark(owner, "asura", "shared", "finished") + // mixed: a second reader still reading keeps the series alive. + seedBookmark(owner, "asura", "mixed", "finished") + seedBookmark(owner+1, "asura", "mixed", "archived") + // live: nobody finished it. + seedBookmark(owner, "asura", "live", "reading") + // orphan: no bookmarks at all, nothing to decide from. + if _, err := db.Exec( + `INSERT INTO series (site, series_id) VALUES ('asura', 'orphan')`); err != nil { + t.Fatalf("seed orphan: %v", err) + } + + if err := migrate(db, 0); err != nil { + t.Fatalf("migrate 0016: %v", err) + } + + finishedAt := func(site, seriesID string) int64 { + t.Helper() + var at int64 + if err := db.QueryRow( + `SELECT finished_at FROM series WHERE site = $1 AND series_id = $2`, + site, seriesID).Scan(&at); err != nil { + t.Fatalf("finished_at %s:%s: %v", site, seriesID, err) + } + return at + } + status := func(readerID int64, site, seriesID string) string { + t.Helper() + var s string + if err := db.QueryRow( + `SELECT status FROM bookmarks WHERE reader_id = $1 + AND site = $2 AND series_id = $3`, readerID, site, seriesID).Scan(&s); err != nil { + t.Fatalf("status %s:%s: %v", site, seriesID, err) + } + return s + } + + // The seed ran before the flip: had the flip gone first, done's bookmarks + // would read archived and nothing would be stamped. The stamp is a real + // timestamp, not a sentinel. + before := time.Now().Add(-time.Minute).UnixMilli() + after := time.Now().Add(time.Minute).UnixMilli() + for _, sr := range []struct{ site, seriesID string }{ + {"asura", "done"}, {"asura", "shared"}, + } { + if at := finishedAt(sr.site, sr.seriesID); at < before || at > after { + t.Fatalf("%s:%s finished_at = %d, want now-ish", sr.site, sr.seriesID, at) + } + // The flip followed the seed: every finished bookmark is archived. + if s := status(owner, sr.site, sr.seriesID); s != StatusArchived { + t.Fatalf("%s:%s status = %q, want archived after the flip", sr.site, sr.seriesID, s) + } + } + // The seed ignores a series any bookmark keeps alive. + if at := finishedAt("asura", "mixed"); at != 0 { + t.Fatalf("mixed finished_at = %d, want 0", at) + } + // The flip is a bucket rewrite, not a series-wide one: the finished + // bookmark becomes archived and the sibling rows keep their buckets. + if s := status(owner, "asura", "mixed"); s != StatusArchived { + t.Fatalf("mixed finished bookmark = %q, want archived", s) + } + if s := status(owner+1, "asura", "mixed"); s != StatusArchived { + t.Fatalf("mixed archived bookmark = %q, want untouched archived", s) + } + if s := status(owner, "asura", "live"); s != StatusReading { + t.Fatalf("live status = %q, want untouched reading", s) + } + if at := finishedAt("asura", "orphan"); at != 0 { + t.Fatalf("orphan finished_at = %d, want 0", at) + } +} + // readSeries reads the series row directly, for asserting on what Upsert // actually stored rather than what the joined Bookmark reports. func readSeries(t *testing.T, s *Store, site, seriesID string) Series { @@ -1809,19 +1947,12 @@ func TestDueForLatestCheckForcedOverridesSightingDeferral(t *testing.T) { } } -// The finished-only bucket excludes a series whose only bookmarks are -// finished; a forced request overrides it — the owner asked, so the Lane -// looks. -func TestDueForLatestCheckForcedOverridesFinishedBucket(t *testing.T) { +// The finished flag excludes a series; a forced request overrides it — the +// owner asked, so the Lane looks. +func TestDueForLatestCheckForcedOverridesFinishedFlag(t *testing.T) { s := newTestStore(t) seedForCheck(t, s, "asura:reading", "https://asurascans.com/comics/reading", 0) - if _, err := s.Upsert(s.OwnerID(), Bookmark{ - Key: "asura:finished", Site: "asura", SeriesID: "finished", - SeriesURL: "https://asurascans.com/comics/finished", - Status: StatusFinished, UpdatedAt: 1000, - }); err != nil { - t.Fatalf("seed finished: %v", err) - } + seedFinished(t, s, "asura:finished", "https://asurascans.com/comics/asura:finished") due, err := s.DueForLatestCheck("asura", 1000, noCeiling) if err != nil { diff --git a/backend/internal/web/static/filter.js b/backend/internal/web/static/filter.js index baedcbd..83874a5 100644 --- a/backend/internal/web/static/filter.js +++ b/backend/internal/web/static/filter.js @@ -61,7 +61,7 @@ function setActiveTab(el) { document.dispatchEvent(new Event("bmgr:refilter")); } -// The chapter-edit form and the archive/finish/remove confirm rows are the +// The chapter-edit form and the archive/remove confirm rows are the // per-card disclosure panels; only one makes sense open at a time. The button // that owns an open panel carries .open, which is how the strip shows which // cell the panel belongs to. @@ -97,10 +97,10 @@ function toggleChapterForm(key) { if (form && !form.hidden) form.querySelector("input").focus(); } -// kind is "archive" | "finish" | "remove" — the panel id and the owning action +// kind is "archive" | "remove" — the panel id and the owning action // cell share it. function toggleConfirmRow(key, kind) { - var cls = { archive: ".box", finish: ".finish", remove: ".remove" }[kind]; + var cls = { archive: ".box", remove: ".remove" }[kind]; var row = togglePanel(key, "confirm-" + kind + "-" + key, ".actions " + cls); // Focus the answer rather than trusting aria-live on a container that merely // unhides: it makes the announcement deterministic, keeps tab order inside diff --git a/backend/internal/web/static/style.css b/backend/internal/web/static/style.css index 91c1526..14a790e 100644 --- a/backend/internal/web/static/style.css +++ b/backend/internal/web/static/style.css @@ -54,7 +54,7 @@ --ink: #100f0e; /* page */ --ash: #161413; /* recessed panel (chapter form) */ - --dim: #0d0c0b; /* archived / finished rows sink */ + --dim: #0d0c0b; /* archived rows sink */ --rule: #221f1d; /* hairline between sheets */ --rule-soft: #1a1817; /* measure edges */ --field-line: #2c2926; @@ -84,7 +84,6 @@ /* 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, @@ -148,7 +147,6 @@ --danger-soft: #7c2c22; --brass: #8a681c; --slate: #3f6689; - --moss: #3d6c46; --clay: #7c5533; --trash: #8c6558; --patina: #1f6f66; @@ -612,13 +610,13 @@ button { cursor: pointer; } .actions > *:last-child { border-right: none; } .actions svg { width: 17px; height: 17px; } .actions > *:hover { color: var(--paper); } -/* Per-action accent on hover and press: gold favourite, slate archive, moss - finished, clay chapter. Remove keeps --danger, play keeps paper/ember. */ +/* Per-action accent on hover and press: gold favourite, slate archive, clay + chapter. Remove keeps --danger, play keeps paper/ember. */ .actions .fav:hover, .actions .fav:active, .actions .fav:focus-visible { color: var(--brass); } .actions .pencil:hover, .actions .pencil:active, .actions .pencil:focus-visible { color: var(--clay); } .actions .box:hover, .actions .box:active, .actions .box:focus-visible { color: var(--slate); } -.actions .finish:hover, .actions .finish:active, .actions .finish:focus-visible { color: var(--moss); } .actions .play { color: var(--paper); } + .is-new .actions .play { color: var(--ember); } .actions .play:hover { background: var(--hover); } .actions .on { color: var(--brass); } @@ -632,8 +630,8 @@ button { cursor: pointer; } .actions .remove.open { background: var(--danger-wash); color: var(--danger); } .is-dim .actions > * { color: var(--mute-2); } -/* Three clusters by consequence: navigate (play) | organize (favourite, - chapter) | lifecycle (archive/restore, finish, remove). The lifecycle cells +/* Two clusters by consequence: navigate (play) | organize (favourite, + chapter) | lifecycle (archive/restore, remove). The lifecycle cells sit on a recessed ground so the thumb reads "this one moves the series" before it reads which icon it landed on. */ .actions > .lifecycle { background: var(--ash); } @@ -712,7 +710,7 @@ button { cursor: pointer; } color: var(--danger-ink); font-weight: 600; } -/* Archive and finish are reversible, so their confirm asks in grey — only the +/* Archive is reversible, so its confirm asks in grey — only the irreversible remove gets the danger wash. */ .confirm-row.calm { background: var(--ash); } .confirm-row.calm span { color: var(--paper-dim); } @@ -917,9 +915,7 @@ button { cursor: pointer; } /* The cell border follows the icon on hover, so the accent reads as a state rather than a stray colour. */ .actions .fav:hover, .actions .pencil:hover, - .actions .box:hover, .actions .finish:hover { border-color: currentColor; } - - /* Panels line up with the body text, i.e. past the cover and its gap. */ + .actions .box:hover { border-color: currentColor; } .chapter-form, .confirm-row, .error-inline { margin-left: calc(var(--cover-w) + var(--row-gap)); } diff --git a/backend/internal/web/templates/app.html b/backend/internal/web/templates/app.html index d761b95..84c9769 100644 --- a/backend/internal/web/templates/app.html +++ b/backend/internal/web/templates/app.html @@ -71,10 +71,6 @@ {{if eq .Tab "archived"}}aria-current="page"{{end}} hx-get="{{.ListURL "archived"}}" hx-target="#list" hx-swap="innerHTML" hx-push-url="{{.PageURL "archived"}}" hx-on::after-request="setActiveTab(this)">Archived - Finished diff --git a/backend/internal/web/templates/card.html b/backend/internal/web/templates/card.html index 4b79804..d6b40e5 100644 --- a/backend/internal/web/templates/card.html +++ b/backend/internal/web/templates/card.html @@ -1,6 +1,6 @@ {{define "card"}} {{/* One sheet per series. is-new turns the title crimson over an ember rule; - is-dim sinks archived and finished rows into italic grey. */}} + is-dim sinks archived rows into italic grey. */}}
@@ -32,9 +32,6 @@ {{if eq .Status "archived"}} / archived - {{else if eq .Status "finished"}} - / - finished {{end}}

@@ -56,7 +53,7 @@ {{/* Restore is a reversal, so it fires straight away; every move *out* of - the list (archive, finish, remove) goes through a confirm row. */}} + the list (archive, remove) goes through a confirm row. */}} {{if eq .Status "reading"}} {{end}} - {{if ne .Status "finished"}} - - {{end}} - - - - {{end}} diff --git a/backend/web_test.go b/backend/web_test.go index b7b035c..41aa76e 100644 --- a/backend/web_test.go +++ b/backend/web_test.go @@ -3428,6 +3428,169 @@ func TestAdminSeriesDetailCorrectionMarker(t *testing.T) { } } +// The finish control lives on the Series detail page only: a neighbouring +// Finish in the fifty-row grid is not a harmless read the way Check now is, +// so the list row renders the state as a mark and offers no control to set +// it. On the detail page the press is confirm-gated — the opener does not +// post and the only finish POST sits inside the confirm row, targeting the +// meta fragment it will swap (#158). +func TestAdminSeriesDetailFinishControl(t *testing.T) { + router, st := newWebTestServer(t, testConfig()) + seed(t, st, store.Bookmark{ + Key: "asura:solo", Site: "asura", SeriesID: "solo", + Title: "Solo", Kind: store.KindManga, + }) + + body := seriesDetailPage(t, router, st, "asura:solo") + for _, want := range []string{ + `id="confirm-finish"`, + "Mark this Series finished?", + `hx-post="/admin/series/asura:solo/finish"`, + `hx-target="#detail-meta"`, + } { + if !strings.Contains(body, want) { + t.Errorf("detail page lacks %q:\n%s", want, body) + } + } + // The press travels through the confirm row: the page's only finish POST + // sits after the row's id (inside the row), and the opener that reveals + // it posts nothing. + if opener := strings.Index(body, `id="confirm-finish"`); opener < 0 || + strings.Index(body, `hx-post="/admin/series/asura:solo/finish"`) < opener { + t.Errorf("finish POST is not inside the confirm row:\n%s", body) + } + if strings.Count(body, `hx-post="/admin/series/asura:solo/finish"`) != 1 { + t.Errorf("finish POST count = %d, want exactly one (the affirmative):\n%s", + strings.Count(body, `hx-post="/admin/series/asura:solo/finish"`), body) + } + + // The list row displays nothing to set: no finish control, no finished + // mark on an unfinished Series. + list := adminSeriesPage(t, router, st, "") + for _, banned := range []string{ + `/admin/series/asura:solo/finish`, + `/admin/series/asura:solo/unfinish`, + `mark-faint">finished`, + } { + if strings.Contains(list, banned) { + t.Errorf("list row carries %q:\n%s", banned, list) + } + } +} + +// The finish route is the owner's one writer: a finish press stamps +// finished_at and answers with the swapped detail-meta fragment carrying the +// "finished ago" mark, and an un-finish press writes zero and answers +// with a fragment that has no finished line. A malformed key is a 400 and an +// unknown one a 404, like the other Series mutations (#158). +func TestSeriesFinishRoute(t *testing.T) { + st, dsn := newTestStoreURL(t) + db, err := sql.Open("pgx", dsn) + if err != nil { + t.Fatalf("open %s: %v", dsn, err) + } + defer db.Close() + seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:solo", url: "u", checkedAt: 9000, bookmarks: 1}) + router := newRouter(st, testConfig()) + cookie := sessionCookie(t, st) + + for path, want := range map[string]int{ + "/admin/series/solo/finish": http.StatusBadRequest, + "/admin/series/ghost:x/finish": http.StatusNotFound, + } { + req := httptest.NewRequest(http.MethodPost, path, strings.NewReader("")) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(cookie) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != want { + t.Errorf("POST %s: status = %d, want %d", path, rr.Code, want) + } + } + + // A finish press stamps and answers with the swapped meta fragment whose + // mark reads "finished just now". + req := httptest.NewRequest(http.MethodPost, "/admin/series/asura:solo/finish", strings.NewReader("")) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(cookie) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("POST finish status = %d, want 200 (body %s)", rr.Code, rr.Body.String()) + } + body := rr.Body.String() + if !strings.Contains(body, `id="detail-meta"`) { + t.Errorf("finish answer is not the meta fragment:\n%s", body) + } + if !strings.Contains(body, `finished `) { + t.Errorf("finish answer lacks the fresh finished mark:\n%s", body) + } + var at int64 + if err := db.QueryRow(`SELECT finished_at FROM series WHERE site = 'asura' AND series_id = 'solo'`).Scan(&at); err != nil { + t.Fatalf("read finished_at: %v", err) + } + if at == 0 { + t.Error("finished_at = 0, want the finish stamp written") + } + + // The page after the press offers the instant reversal, no confirm. + body = seriesDetailPage(t, router, st, "asura:solo") + if !strings.Contains(body, `hx-post="/admin/series/asura:solo/unfinish"`) { + t.Errorf("finished detail page lacks the un-finish control:\n%s", body) + } + if strings.Contains(body, "Mark this Series finished?") { + t.Errorf("finished detail page still carries the confirm row:\n%s", body) + } + + // An un-finish press fires instantly and writes zero. + req = httptest.NewRequest(http.MethodPost, "/admin/series/asura:solo/unfinish", strings.NewReader("")) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(cookie) + rr = httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("POST unfinish status = %d, want 200 (body %s)", rr.Code, rr.Body.String()) + } + body = rr.Body.String() + if !strings.Contains(body, `id="detail-meta"`) { + t.Errorf("unfinish answer is not the meta fragment:\n%s", body) + } + if strings.Contains(body, `finished `) { + t.Errorf("unfinish answer still carries the finished mark:\n%s", body) + } + if err := db.QueryRow(`SELECT finished_at FROM series WHERE site = 'asura' AND series_id = 'solo'`).Scan(&at); err != nil { + t.Fatalf("read finished_at after un-finish: %v", err) + } + if at != 0 { + t.Errorf("finished_at = %d after un-finish, want 0", at) + } +} + +// A finished Series shows its state in the list row as a faint mark, and the +// row offers no control to set or clear it — the reversal lives on the +// detail page, its press, and the mark is the whole of the row's share +// (#158). +func TestAdminSeriesListShowsFinished(t *testing.T) { + router, st := newWebTestServer(t, testConfig()) + seed(t, st, store.Bookmark{ + Key: "asura:solo", Site: "asura", SeriesID: "solo", + Title: "Solo", Kind: store.KindManga, + }) + if err := st.SetSeriesFinished("asura", "solo", time.Now().Add(-2*time.Minute).UnixMilli()); err != nil { + t.Fatalf("SetSeriesFinished: %v", err) + } + + body := adminSeriesPage(t, router, st, "") + if !strings.Contains(body, `finished`) { + t.Errorf("list row lacks the finished mark:\n%s", body) + } + for _, banned := range []string{"solo/finish", "solo/unfinish"} { + if strings.Contains(body, banned) { + t.Errorf("list row offers %q:\n%s", banned, body) + } + } +} + // The series URL repair validates with the poller's own fetch gate and // answers 400 before anything reaches the store; a URL that passes the gate // is stored where an Upsert would have ignored it. The request performs no -- 2.52.0 From 410509097a9ed93bd73d66087c836506dd8cd75a Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sat, 22 Aug 2026 17:13:33 +0700 Subject: [PATCH 5/7] Restore truncated overflow-page comment dropped by the finish diff (#158) --- backend/internal/web/admin_series.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/internal/web/admin_series.go b/backend/internal/web/admin_series.go index cc92339..fd473bf 100644 --- a/backend/internal/web/admin_series.go +++ b/backend/internal/web/admin_series.go @@ -570,6 +570,8 @@ func (h *Handler) seriesListView(r *http.Request) (seriesListView, error) { if err != nil { return seriesListView{}, err } + // A page past the end is not an empty list: the store's window count runs + // over the rows the result set carries, so an overflow page reports zero // rows and zero total, and the list re-reads at page 1 to know the truth. if len(data.Rows) == 0 && page > 1 { page = 1 -- 2.52.0 From 0029cff27c861248f10adbed04916a1bf8069064 Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sat, 22 Aug 2026 17:25:10 +0700 Subject: [PATCH 6/7] Add finished Series filter; clock-driven hygiene predicates exclude it (#159) --- backend/internal/store/admin.go | 34 ++++++--- backend/internal/store/admin_test.go | 79 +++++++++++++++++++ backend/internal/web/admin_overview.go | 8 +- backend/internal/web/admin_series.go | 20 +++-- backend/web_test.go | 100 ++++++++++++++++++++++++- 5 files changed, 218 insertions(+), 23 deletions(-) diff --git a/backend/internal/store/admin.go b/backend/internal/store/admin.go index 039cc75..2880e84 100644 --- a/backend/internal/store/admin.go +++ b/backend/internal/store/admin.go @@ -7,11 +7,13 @@ import ( "strings" ) -// Series filter names (issue #140), ordered permanent-then-fixable — the -// repairs nothing will ever undo first, the ones a Poll can make right after. -// A name is the repair a row needs, not the SQL that finds it; the values are -// the wire form the Series list URL carries (#142). "all" is the absent and -// unknown case: every Series. +// Series filter names (issue #140): the eight hygiene names are ordered +// permanent-then-fixable — the repairs nothing will ever undo first, the +// ones a Poll can make right after. SeriesFilterFinished is not part of +// that ordering: a finished Series is a deliberate state, not a repair, so +// it sits last, informational. A name is the repair a row needs, not the +// SQL that finds it; the values are the wire form the Series list URL +// carries (#142). "all" is the absent and unknown case: every Series. const ( SeriesFilterAll = "all" SeriesFilterNoURL = "no_series_url" @@ -21,6 +23,7 @@ const ( SeriesFilterStale = "stale" SeriesFilterNoCover = "no_cover" SeriesFilterReaderReport = "reader_report" + SeriesFilterFinished = "finished" ) // SeriesFilter is one named hygiene predicate over the whole library. Site @@ -115,10 +118,19 @@ func (a AdminSeries) Key() string { return a.Site + ":" + a.SeriesID } // The WHERE set is: no URL (an empty URL only — the host-failing-the-fetch- // gate case is invisible to SQL, needs the Site registry in Go, and belongs to // a later repair), never-read-a-chapter and never-checked as disjoint halves -// (non-zero versus zero check stamp), stale, no cover, and Reader-report. +// (non-zero versus zero check stamp), stale, no cover, finished (the +// retirement stamp, read directly), and Reader-report. // no_readers is the one HAVING predicate: it is the orphan test, an aggregate // over the LEFT JOIN, where a bare WHERE has no row to test. // +// The clock-versus-outcome split decides which predicates exclude finished +// Series (`s.finished_at = 0` in each of the four): the clock-driven one — +// never-checked, stale, no-chapter, no-cover — keep ticking after the last +// Poll, so they would report a retired row as a problem no Poll is coming to +// fix; the three outcome-driven ones — no-URL, no-readers, Reader-report — +// read stored facts that simply stop arriving, so a finished Series needing a +// genuine repair still shows up under them. +// // stale is the checked-but-old half of the stamp partition — because the // verdict line wants "not checked in twelve hours" as one figure, and a never // checked Series is already counted on its own "waiting"/never-checked @@ -135,16 +147,18 @@ func adminFilter(f SeriesFilter) (where, having string, args []any, err error) { case SeriesFilterNoURL: clauses = append(clauses, `s.series_url = ''`) case SeriesFilterNoChapter: - clauses = append(clauses, `s.latest_checked_at <> 0 AND s.latest_chapter_num IS NULL`) + clauses = append(clauses, `s.latest_checked_at <> 0 AND s.latest_chapter_num IS NULL AND s.finished_at = 0`) case SeriesFilterNeverChecked: - clauses = append(clauses, `s.latest_checked_at = 0`) + clauses = append(clauses, `s.latest_checked_at = 0 AND s.finished_at = 0`) case SeriesFilterStale: - clauses = append(clauses, `s.latest_checked_at > 0 AND s.latest_checked_at < $`+strconv.Itoa(len(args)+1)) + clauses = append(clauses, `s.latest_checked_at > 0 AND s.latest_checked_at < $`+strconv.Itoa(len(args)+1)+` AND s.finished_at = 0`) args = append(args, f.Cutoff) case SeriesFilterNoCover: - clauses = append(clauses, `s.cover_address = ''`) + clauses = append(clauses, `s.cover_address = '' AND s.finished_at = 0`) case SeriesFilterReaderReport: clauses = append(clauses, `s.latest_raised_by IS NOT NULL`) + case SeriesFilterFinished: + clauses = append(clauses, `s.finished_at > 0`) case SeriesFilterNoReaders: having = `HAVING COUNT(b.reader_id) = 0` default: diff --git a/backend/internal/store/admin_test.go b/backend/internal/store/admin_test.go index da9efb3..5f4077f 100644 --- a/backend/internal/store/admin_test.go +++ b/backend/internal/store/admin_test.go @@ -128,6 +128,85 @@ func TestAdminSeriesFilters(t *testing.T) { } } +// A finished Series is the owner's deliberate state, not a problem a Poll +// will fix: the four clock-driven hygiene predicates exclude it (their +// stamps stop advancing at the last Poll, so without the guard a retired row +// is reported forever), the three outcome-driven ones still include it, and +// the finished filter returns exactly the retired rows. +func TestAdminFinishedSeriesFilters(t *testing.T) { + s := newTestStore(t) + // Each fin-* row is shaped to trip exactly one predicate if its guard + // fails: checked-but-old for stale, a zero stamp for never-checked, a + // stamp with no chapter for no-chapter, an empty cover for no-cover, and + // the unguarded three shaped to trip their own. A healthy, unfinished + // neighbour keeps the exclusion checks honest: a filter that regressed to + // matching nothing would pass a bare "no finished rows" assertion. + seedAdminSeries(t, s, seriesSeed{key: "asura:fin-stale", url: "u", cover: "c", checkedAt: 2000, latestNum: new(4.0), bookmarks: 1}) + seedAdminSeries(t, s, seriesSeed{key: "asura:fin-neverchecked", url: "u", cover: "c", checkedAt: 0, latestNum: new(4.0), bookmarks: 1}) + seedAdminSeries(t, s, seriesSeed{key: "asura:fin-nochapter", url: "u", cover: "c", checkedAt: 9000, bookmarks: 1}) + seedAdminSeries(t, s, seriesSeed{key: "asura:fin-nocover", url: "u", checkedAt: 9000, latestNum: new(4.0), bookmarks: 1}) + seedAdminSeries(t, s, seriesSeed{key: "asura:fin-nourl", url: "", cover: "c", checkedAt: 9000, latestNum: new(4.0), bookmarks: 1}) + seedAdminSeries(t, s, seriesSeed{key: "asura:fin-orphan", url: "u", cover: "c", checkedAt: 9000, latestNum: new(4.0), bookmarks: 0}) + seedAdminSeries(t, s, seriesSeed{key: "asura:fin-report", url: "u", cover: "c", checkedAt: 9000, latestNum: new(4.0), bookmarks: 1, raisedBy: true}) + seedAdminSeries(t, s, seriesSeed{key: "asura:healthy", url: "u", cover: "c", checkedAt: 9000, latestNum: new(4.0), bookmarks: 1}) + finished := []string{ + "asura:fin-stale", "asura:fin-neverchecked", "asura:fin-nochapter", + "asura:fin-nocover", "asura:fin-nourl", "asura:fin-orphan", "asura:fin-report", + } + for _, key := range finished { + site, id, _ := strings.Cut(key, ":") + if err := s.SetSeriesFinished(site, id, 1000); err != nil { + t.Fatalf("finish %s: %v", key, err) + } + } + + cases := []struct { + name string + f SeriesFilter + want []string + }{ + {"stale excludes finished", SeriesFilter{Name: SeriesFilterStale, Cutoff: 5000}, nil}, + {"never checked excludes finished", SeriesFilter{Name: SeriesFilterNeverChecked}, nil}, + {"no chapter excludes finished", SeriesFilter{Name: SeriesFilterNoChapter}, nil}, + {"no cover excludes finished", SeriesFilter{Name: SeriesFilterNoCover}, nil}, + {"no url includes finished", SeriesFilter{Name: SeriesFilterNoURL}, []string{"asura:fin-nourl"}}, + {"no readers includes finished", SeriesFilter{Name: SeriesFilterNoReaders}, []string{"asura:fin-orphan"}}, + {"reader report includes finished", SeriesFilter{Name: SeriesFilterReaderReport}, []string{"asura:fin-report"}}, + {"finished returns the retired rows", SeriesFilter{Name: SeriesFilterFinished}, finished}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := pageKeys(t, s, tc.f) + want := map[string]bool{} + for _, k := range tc.want { + want[k] = true + } + if len(got) != len(want) { + t.Fatalf("%+v returned %v, want exactly %v", tc.f, got, want) + } + for k := range want { + if !got[k] { + t.Fatalf("%+v dropped %q (got %v)", tc.f, k, got) + } + } + }) + } + + // The aggregate's finished total counts every retired row — the same + // predicate the Overview's finished figure is summed from. + shapes, err := s.SeriesShapes(SeriesFilter{Name: SeriesFilterFinished}) + if err != nil { + t.Fatalf("SeriesShapes(finished): %v", err) + } + sum := 0 + for _, sh := range shapes { + sum += sh.Total + } + if sum != len(finished) { + t.Fatalf("finished aggregate = %d, want %d", sum, len(finished)) + } +} + // "Never read a chapter" and "never checked" are disjoint by construction: // the first requires a non-zero check stamp, the second a zero one. Over a // mix that should satisfy both, no row may be counted twice. diff --git a/backend/internal/web/admin_overview.go b/backend/internal/web/admin_overview.go index 1851a17..2d2f9ba 100644 --- a/backend/internal/web/admin_overview.go +++ b/backend/internal/web/admin_overview.go @@ -59,7 +59,7 @@ type siteRow struct { } // overviewView assembles the landing page from the store's read model: one -// SeriesShapes pass per filter summed in Go (the shipped surface offers eight +// SeriesShapes pass per filter summed in Go (the shipped surface offers nine // grouped passes, not a stats query — #140), the pass log's latest pass per // Site, and the roster. A failure in any read is a 500 with a logged reason, // never a page of silent zeroes. @@ -93,8 +93,10 @@ func (h *Handler) overviewView() (overviewView, error) { view.Unchecked = totals[store.SeriesFilterStale] + totals[store.SeriesFilterNeverChecked] view.Verdict, view.HasCounts = overviewVerdict(passes, now) - // The seven problem filters, in seriesFilterOrder's permanent-then-fixable - // order; the All filter's count belongs to the Library block, not to a + // The hygiene figures, in seriesFilterOrder's tail: the seven problem + // filters in permanent-then-fixable order, then the finished figure last — + // informational, not a problem, and last because seriesFilterOrder appends + // it there. The All filter's count belongs to the Library block, not to a // "hygiene" figure. hygiene := make([]fig, 0, len(seriesFilterOrder)-1) for _, name := range seriesFilterOrder[1:] { diff --git a/backend/internal/web/admin_series.go b/backend/internal/web/admin_series.go index fd473bf..20298ad 100644 --- a/backend/internal/web/admin_series.go +++ b/backend/internal/web/admin_series.go @@ -20,10 +20,10 @@ import ( // the wrong page. The store does not export it (#140). const seriesPageSize = 50 -// seriesFilterLabels names every hygiene filter for the Series list select, -// keyed by the wire constant the URL carries. The render order is -// seriesFilterOrder; the labels are read by later admin tickets too, so the -// map and the constants cannot drift apart. +// seriesFilterLabels names every Series filter for the list select, keyed by +// the wire constant the URL carries. The render order is seriesFilterOrder; +// the labels are read by later admin tickets too, so the map and the +// constants cannot drift apart. var seriesFilterLabels = map[string]string{ store.SeriesFilterAll: "All series", store.SeriesFilterNoURL: "No series URL", @@ -33,10 +33,13 @@ var seriesFilterLabels = map[string]string{ store.SeriesFilterStale: "Not checked in 12h", store.SeriesFilterNoCover: "No cover", store.SeriesFilterReaderReport: "Latest from a Reader", + store.SeriesFilterFinished: "Finished", } // seriesFilterOrder is the select's render order: All first, then the -// permanent repairs, then the fixable ones (issue #140). +// permanent repairs, then the fixable ones (issue #140). Finished rides the +// tail, last — deliberate, not a repair — and the Overview's stats block +// renders the same tail, which is what sits the finished figure last there. var seriesFilterOrder = []string{ store.SeriesFilterAll, store.SeriesFilterNoURL, @@ -46,6 +49,7 @@ var seriesFilterOrder = []string{ store.SeriesFilterStale, store.SeriesFilterNoCover, store.SeriesFilterReaderReport, + store.SeriesFilterFinished, } // seriesListView is the Series list page's data. The template renders strings @@ -611,9 +615,9 @@ func (h *Handler) seriesListView(r *http.Request) (seriesListView, error) { return view, nil } -// seriesFilterOptions renders every hygiene filter with its library-wide -// count, one SeriesShapes pass per filter summed in Go — the shipped surface -// offers eight grouped passes, not a single stats query (#140). The counts +// seriesFilterOptions renders every filter with its library-wide count, one +// SeriesShapes pass per filter summed in Go — the shipped surface offers nine +// grouped passes, not a single stats query (#140). The counts // are library-wide because the select sits next to the Site narrowing and // must not shift as the owner narrows the list itself. Cutoff travels with // the stale filter, or its count would always be zero. diff --git a/backend/web_test.go b/backend/web_test.go index 41aa76e..bdfe09c 100644 --- a/backend/web_test.go +++ b/backend/web_test.go @@ -2139,7 +2139,6 @@ func TestNovelPageOmitsUpdatedTab(t *testing.T) { } } - func TestMangaPageKeepsUpdatedTab(t *testing.T) { cfg := testConfig() srv, st := newWebTestServer(t, cfg) @@ -2308,6 +2307,42 @@ func TestSeriesListFilterWiring(t *testing.T) { } } +// The finished filter is a first-class option: the select carries it with +// its label and library-wide count, and entering it lists exactly the +// retired rows — picking it up from order-plus-label like every other +// filter, with no per-filter branch in the handler. +func TestSeriesListFinishedFilterOption(t *testing.T) { + st, dsn := newTestStoreURL(t) + db, err := sql.Open("pgx", dsn) + if err != nil { + t.Fatalf("open %s: %v", dsn, err) + } + defer db.Close() + seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:healthy", url: "u", cover: "c", checkedAt: time.Now().UnixMilli(), latestNum: floatPtr(10), bookmarks: 1}) + seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:retired", url: "u", cover: "c", checkedAt: time.Now().UnixMilli(), latestNum: floatPtr(1), bookmarks: 1}) + if err := st.SetSeriesFinished("asura", "retired", time.Now().UnixMilli()); err != nil { + t.Fatalf("finish asura:retired: %v", err) + } + srv := newRouter(st, testConfig()) + + body := adminSeriesPage(t, srv, st, "?filter=finished") + if !strings.Contains(body, "Title of asura:retired") { + t.Errorf("finished list misses its row:\n%s", body) + } + if strings.Contains(body, "Title of asura:healthy") { + t.Errorf("finished list renders an unfinished row:\n%s", body) + } + if !strings.Contains(body, "1 series") || !strings.Contains(body, "Finished") { + t.Errorf("finished heading lacks the count and label:\n%s", body) + } + if !strings.Contains(body, "Finished (1)") { + t.Errorf("the finished option lacks its count:\n%s", body) + } + if !strings.Contains(body, `0`) { + t.Errorf("a zero finished figure does not render as a muted digit:\n%s", body) + } + if strings.Contains(body, `href="/admin/series?filter=finished"`) { + t.Errorf("a zero finished figure is still a link:\n%s", body) + } +} + // The waiting figure sums Due over the latest pass per Site — older passes // for the same Site must not double-count, so the verdict reads the same // latest-per-Site projection the Lanes page reads. @@ -3495,7 +3591,7 @@ func TestSeriesFinishRoute(t *testing.T) { cookie := sessionCookie(t, st) for path, want := range map[string]int{ - "/admin/series/solo/finish": http.StatusBadRequest, + "/admin/series/solo/finish": http.StatusBadRequest, "/admin/series/ghost:x/finish": http.StatusNotFound, } { req := httptest.NewRequest(http.MethodPost, path, strings.NewReader("")) -- 2.52.0 From cdd2e1d72c4587f9c41a140c93941645a7a02f6b Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sat, 22 Aug 2026 17:27:43 +0700 Subject: [PATCH 7/7] Fix stale comment counts found in review (#159) --- backend/internal/store/admin.go | 4 ++-- backend/internal/web/admin_overview.go | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/backend/internal/store/admin.go b/backend/internal/store/admin.go index 2880e84..0be59de 100644 --- a/backend/internal/store/admin.go +++ b/backend/internal/store/admin.go @@ -7,7 +7,7 @@ import ( "strings" ) -// Series filter names (issue #140): the eight hygiene names are ordered +// Series filter names (issue #140): the seven repair filters are ordered // permanent-then-fixable — the repairs nothing will ever undo first, the // ones a Poll can make right after. SeriesFilterFinished is not part of // that ordering: a finished Series is a deliberate state, not a repair, so @@ -26,7 +26,7 @@ const ( SeriesFilterFinished = "finished" ) -// SeriesFilter is one named hygiene predicate over the whole library. Site +// SeriesFilter is one named filter predicate over the whole library. Site // and Kind narrow the row read; Name picks the predicate; Cutoff is the // staleness boundary the "stale" filter compares against, supplied by the // caller's clock — the store has no clock; Page is 1-based. diff --git a/backend/internal/web/admin_overview.go b/backend/internal/web/admin_overview.go index 2d2f9ba..9857864 100644 --- a/backend/internal/web/admin_overview.go +++ b/backend/internal/web/admin_overview.go @@ -24,8 +24,9 @@ type overviewView struct { // its own filter, and the verdict wants the inclusive number. Unchecked int // Hygiene is the seven problem filters in the Series list's own render - // order; Library is the library split plus the roster. Every figure is a - // door into the list that counts it, except a zero. + // order plus the finished figure riding last (informational); Library is + // the library split plus the roster. Every figure is a door into the list + // that counts it, except a zero. Hygiene []fig Library []fig // Sites is the per-Site library shape table, one row per Site with any -- 2.52.0