backend: remove orphan series from the admin (ticket #155)

(*Store).RemoveSeries deletes one series row by (site, series_id) via a plain parameterized DELETE; a bookmarks_series_fk violation outside 23503 is translated into the ErrSeriesHasBookmarks sentinel so no driver type escapes the store. The caller reads the row's cover address before the delete and reclaims it after: ReclaimCover's guard cannot pass while a series row still points at the address. POST /admin/series/{key}/remove answers the list row with the removed row's fragment plus the heading re-rendered out of band at the fresh count (HX-Reswap: delete removes the row through the same button that swaps the refusal back in), and navigates from the detail page to the No-Readers list (HX-Redirect for htmx, a 303 for plain clients). A removal that races a fresh Bookmark is a refusal, not a 500: the row re-renders at its new count with the fact spelled out. The control renders only at zero Reader count on both surfaces, gated by hx-confirm with the brief's copy.
2026-08-22 09:55:01 +07:00
parent ba2b6eebbb
commit bc64a1d894
9 changed files with 580 additions and 12 deletions
@@ -2557,18 +2557,21 @@ func TestSeriesListRowShape(t *testing.T) {
 	if !strings.Contains(body, `class="c-site site-asura"`) {
 		t.Errorf("the site cell lacks its site class:\n%s", body)
 	}
-	// The action cell carries the Check now control on pollable rows and is
-	// empty on the orphan (no URL, no Readers — a button that can never do
-	// anything is not offered), and no Remove control or confirm row renders
-	// in this batch.
-	if !strings.Contains(body, `<span class="c-act"></span>`) {
-		t.Errorf("the orphan's action cell is not present and empty:\n%s", body)
+	// The action cell carries the Check now control on pollable rows and the
+	// Remove control on the orphan (#155) — a Series no Reader holds can be
+	// removed, so the orphan's cell is never empty. No confirm row renders
+	// in this batch: the confirm gate is htmx's own, not a toggled cell.
+	if !strings.Contains(body, `<span class="c-act">`) {
+		t.Errorf("the action cell is not present:\n%s", body)
 	}
 	if got := strings.Count(body, "Check now"); got != 2 {
 		t.Errorf("Check now control count = %d, want 2 (only the two pollable rows):\n%s", got, body)
 	}
-	if strings.Contains(body, "Remove") || strings.Contains(body, "confirm-row") {
-		t.Errorf("a Remove control or confirm row renders in this batch:\n%s", body)
+	if got := strings.Count(body, ">Remove<"); got != 1 {
+		t.Errorf("Remove control count = %d, want 1 (only the orphan):\n%s", got, body)
+	}
+	if strings.Contains(body, "confirm-row") {
+		t.Errorf("a confirm row renders in this batch:\n%s", body)
 	}
 	// No ember: the new-chapter signal stays off the admin surface. Scoped to
 	// the page content — the shell's brand mark legitimately wears the ember
@@ -3292,6 +3295,7 @@ func TestSeriesPollCapsBody(t *testing.T) {
 		t.Errorf("an oversized body still stamped the request:\n%s", body)
 	}
 }
+
 // The correction route validates at the boundary: a non-numeric, zero,
 // negative or non-finite chapter answers 400 and never reaches the store, and
 // a finite number greater than zero stores the number, the derived label and
@@ -3546,3 +3550,231 @@ func TestAdminSeriesDetailRepairForm(t *testing.T) {
 		t.Errorf("repair input does not carry the escaped stored URL:\n%s", body)
 	}
 }
+
+// The Remove control is offered only to the owner, and only on a Series no
+// Reader holds: on the orphan's list row and on the orphan's detail page,
+// nowhere else (#155).
+func TestSeriesRemoveRendersOnlyOnOrphans(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:ok", url: "u", checkedAt: 9000, bookmarks: 1})
+	seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:orphan", url: "u", checkedAt: 9000, bookmarks: 0})
+	router := newRouter(st, testConfig())
+
+	body := adminSeriesPage(t, router, st, "")
+	if got := strings.Count(body, ">Remove<"); got != 1 {
+		t.Errorf("list offers Remove %d times, want 1 (only the orphan):\n%s", got, body)
+	}
+	for _, tc := range []struct {
+		key  string
+		want bool
+	}{
+		{"asura:ok", false},
+		{"asura:orphan", true},
+	} {
+		body := seriesDetailPage(t, router, st, tc.key)
+		if got := strings.Contains(body, ">Remove<"); got != tc.want {
+			t.Errorf("%s detail offers Remove = %v, want %v", tc.key, got, tc.want)
+		}
+	}
+}
+
+// A removal from the list answers with the removed row's fragment and the
+// heading re-rendered out of band with the fresh count: the row and the
+// count are one fact. The row's press carries the list's filter state, so
+// the count describes the list the owner is looking at, and the HX-Reswap
+// header deletes the row through the same button that swaps the refusal in.
+func TestRemoveFromListAnswersRowAndFreshHeading(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:a", url: "u", checkedAt: 9000, bookmarks: 0})
+	seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:b", url: "u", checkedAt: 9000, bookmarks: 0})
+	seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:held", url: "u", checkedAt: 9000, bookmarks: 1})
+	router := newRouter(st, testConfig())
+
+	req := httptest.NewRequest(http.MethodPost, "/admin/series/asura:a/remove",
+		strings.NewReader("filter=no_readers&band=0"))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.AddCookie(sessionCookie(t, st))
+	rr := httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("removal status = %d, want 200", rr.Code)
+	}
+	if got := rr.Header().Get("HX-Reswap"); got != "delete" {
+		t.Errorf("response does not ask htmx to delete the row (HX-Reswap = %q)", got)
+	}
+	body := rr.Body.String()
+	if !strings.Contains(body, "Title of asura:a") {
+		t.Errorf("answer does not carry the removed row's fragment:\n%s", body)
+	}
+	if !strings.Contains(body, `hx-swap-oob="true"`) ||
+		!strings.Contains(body, "1 series") || !strings.Contains(body, "No Readers") {
+		t.Errorf("answer does not re-render the heading out of band with the fresh count:\n%s", body)
+	}
+
+	// Gone from the store, gone from the list, and the heading lies no longer.
+	var one int
+	if err := db.QueryRow(`SELECT 1 FROM series WHERE site = 'asura' AND series_id = 'a'`).Scan(&one); err != sql.ErrNoRows {
+		t.Fatalf("series row after removal = %v, want sql.ErrNoRows", err)
+	}
+	body = adminSeriesPage(t, router, st, "?filter=no_readers")
+	if strings.Contains(body, "Title of asura:a") || !strings.Contains(body, "1 series") {
+		t.Errorf("list after removal is not the fresh view:\n%s", body)
+	}
+}
+
+// A removal from the detail page navigates to the No-Readers list: htmx gets
+// a full navigation (HX-Redirect — a 303 would be followed by the request
+// and the list page swapped into the press's target), plain clients the 303
+// the ticket names, to the wire filter the orphan list actually is.
+func TestRemoveFromDetailRedirectsToNoReadersList(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:gone", url: "u", checkedAt: 9000, bookmarks: 0})
+	// A second orphan for the htmx dialect's request, whose row must still
+	// exist after the first request removed its own.
+	seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:gone2", url: "u", checkedAt: 9000, bookmarks: 0})
+	router := newRouter(st, testConfig())
+	target := "/admin/series?filter=" + store.SeriesFilterNoReaders
+
+	req := httptest.NewRequest(http.MethodPost, "/admin/series/asura:gone/remove", nil)
+	req.Header.Set("HX-Target", "detail-meta")
+	req.AddCookie(sessionCookie(t, st))
+	rr := httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+	if rr.Code != http.StatusSeeOther {
+		t.Fatalf("detail removal status = %d, want 303", rr.Code)
+	}
+	if got := rr.Header().Get("Location"); got != target {
+		t.Errorf("Location = %q, want %q", got, target)
+	}
+	var one int
+	if err := db.QueryRow(`SELECT 1 FROM series WHERE site = 'asura' AND series_id = 'gone'`).Scan(&one); err != sql.ErrNoRows {
+		t.Fatalf("series row after detail removal = %v, want sql.ErrNoRows", err)
+	}
+
+	req = httptest.NewRequest(http.MethodPost, "/admin/series/asura:gone2/remove", nil)
+	req.Header.Set("HX-Request", "true")
+	req.Header.Set("HX-Target", "detail-meta")
+	req.AddCookie(sessionCookie(t, st))
+	rr = httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+	if got := rr.Header().Get("HX-Redirect"); got != target {
+		t.Errorf("HX-Redirect = %q, want %q", got, target)
+	}
+}
+
+// A removal that races a fresh Bookmark is a refusal, not an error: the row
+// is rendered again at its new count with the fact spelled out, never a 500,
+// and it must not vanish from the list — the delete never happened. The
+// detail-surface refusal navigates back to the detail page, where the same
+// fresh count is visible.
+func TestRemoveRacedBookmarkIsRefusedNotError(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:raced", url: "u", checkedAt: 9000, bookmarks: 1})
+	router := newRouter(st, testConfig())
+
+	req := httptest.NewRequest(http.MethodPost, "/admin/series/asura:raced/remove",
+		strings.NewReader("filter=no_readers&band=0"))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.AddCookie(sessionCookie(t, st))
+	rr := httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("refusal status = %d, want 200 (never a 500)", rr.Code)
+	}
+	if got := rr.Header().Get("HX-Reswap"); got != "" {
+		t.Errorf("refusal carries HX-Reswap = %q, want none (the row must stay)", got)
+	}
+	body := rr.Body.String()
+	if !strings.Contains(body, "a Reader has bookmarked this Series again") {
+		t.Errorf("refusal does not say what happened:\n%s", body)
+	}
+	if !strings.Contains(body, `class="c-rd">1</span>`) {
+		t.Errorf("refusal does not render the fresh count:\n%s", body)
+	}
+	var one int
+	if err := db.QueryRow(`SELECT 1 FROM series WHERE site = 'asura' AND series_id = 'raced'`).Scan(&one); err != nil {
+		t.Fatalf("series row after refusal = %v, want present", err)
+	}
+	list := adminSeriesPage(t, router, st, "")
+	if !strings.Contains(list, "Title of asura:raced") {
+		t.Fatal("row vanished from the list after a refused removal")
+	}
+	if strings.Contains(list, ">Remove<") {
+		t.Errorf("a held series still offers Remove:\n%s", list)
+	}
+
+	// The detail-surface refusal navigates back to the detail page.
+	req = httptest.NewRequest(http.MethodPost, "/admin/series/asura:raced/remove", nil)
+	req.Header.Set("HX-Target", "detail-meta")
+	req.AddCookie(sessionCookie(t, st))
+	rr = httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+	if rr.Code != http.StatusSeeOther {
+		t.Fatalf("detail refusal status = %d, want 303", rr.Code)
+	}
+	if got := rr.Header().Get("Location"); got != "/admin/series/asura:raced" {
+		t.Errorf("refusal Location = %q, want the detail page", got)
+	}
+}
+
+// The remove route trusts the same way the poll route does: a malformed key
+// is a 400 and an unknown Series a 404, and an oversized body is a 400 that
+// removes nothing.
+func TestRemoveRejectsBadKeysAndCapsBody(t *testing.T) {
+	router, st := newWebTestServer(t, testConfig())
+	seed(t, st, store.Bookmark{
+		Key: "asura:x", Site: "asura", SeriesID: "x",
+		Title: "Title of asura:x", SeriesURL: "u",
+	})
+	for _, tc := range []struct {
+		path string
+		want int
+	}{
+		{"/admin/series/nocolon/remove", http.StatusBadRequest},
+		{"/admin/series/:x/remove", http.StatusBadRequest},
+		{"/admin/series/asura:/remove", http.StatusBadRequest},
+		{"/admin/series/asura:ghost/remove", http.StatusNotFound},
+	} {
+		req := httptest.NewRequest(http.MethodPost, tc.path, nil)
+		req.AddCookie(sessionCookie(t, st))
+		rr := httptest.NewRecorder()
+		router.ServeHTTP(rr, req)
+		if rr.Code != tc.want {
+			t.Errorf("POST %s status = %d, want %d", tc.path, rr.Code, tc.want)
+		}
+	}
+
+	big := strings.Repeat("a", 1<<17) // 128 KiB, over the 64 KiB cap
+	req := httptest.NewRequest(http.MethodPost, "/admin/series/asura:x/remove", strings.NewReader(big))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.AddCookie(sessionCookie(t, st))
+	rr := httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+	if rr.Code != http.StatusBadRequest {
+		t.Fatalf("oversized body status = %d, want 400", rr.Code)
+	}
+	list := adminSeriesPage(t, router, st, "")
+	if !strings.Contains(list, "Title of asura:x") {
+		t.Errorf("an oversized body still removed the row:\n%s", list)
+	}
+}