From f94f8516fa64b189b7464f2fbc339f8b501c407f Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Fri, 26 Jun 2026 17:00:08 -0400 Subject: [PATCH] fix(catalog): exclude manga chapters from admin Unmatched queue (#204) HandleListUnmatchedItems built its count and list queries without the manga-chapter exclusion guard, so manga chapter rows (type='ebook' linked into a type='manga' series via manga_chapters) that keep a non-matched status leaked into the admin Unmatched queue and were displayed as 'ebook'. Apply the existing catalog.MangaChapterExclusionWhere("mi") predicate to a shared WHERE clause used by both the count and the list query, so chapters are hidden and the total stays consistent. Adds a DB-backed regression test. Co-Authored-By: Claude Opus 4.8 --- internal/api/handlers/libraries.go | 13 +- .../libraries_unmatched_manga_test.go | 130 ++++++++++++++++++ 2 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 internal/api/handlers/libraries_unmatched_manga_test.go diff --git a/internal/api/handlers/libraries.go b/internal/api/handlers/libraries.go index 4414f546..f97ec90e 100644 --- a/internal/api/handlers/libraries.go +++ b/internal/api/handlers/libraries.go @@ -2495,10 +2495,17 @@ func (h *LibraryHandler) HandleListUnmatchedItems(w http.ResponseWriter, r *http )` } + // Manga chapters are type='ebook' rows linked into a manga series via + // manga_chapters. They are internal sub-units that keep a non-matched status, + // so without this guard they leak into the admin Unmatched queue (issue #204). + // Applied to both the count and the list query so the total stays consistent. + statusWhere := `WHERE mi.status IN ('unmatched', 'pending', 'ambiguous') + AND ` + catalog.MangaChapterExclusionWhere("mi") + countSQL := ` SELECT COUNT(*) FROM media_items mi - WHERE mi.status IN ('unmatched', 'pending', 'ambiguous')` + ` + statusWhere countSQL += filter var total int @@ -2521,10 +2528,10 @@ func (h *LibraryHandler) HandleListUnmatchedItems(w http.ResponseWriter, r *http WHERE mil.content_id = mi.content_id LIMIT 1 ) lib ON true - WHERE mi.status IN ('unmatched', 'pending', 'ambiguous')%s + %s%s ORDER BY mi.title ASC, mi.content_id ASC LIMIT $%d OFFSET $%d - `, filter, len(filterArgs)+1, len(filterArgs)+2) + `, statusWhere, filter, len(filterArgs)+1, len(filterArgs)+2) rows, err := h.pool.Query(r.Context(), listSQL, listArgs...) if err != nil { diff --git a/internal/api/handlers/libraries_unmatched_manga_test.go b/internal/api/handlers/libraries_unmatched_manga_test.go new file mode 100644 index 00000000..fb909f5f --- /dev/null +++ b/internal/api/handlers/libraries_unmatched_manga_test.go @@ -0,0 +1,130 @@ +package handlers + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5/pgxpool" +) + +// newUnmatchedTestPool connects to the test database used for DB-backed handler +// tests. It skips when SILO_TEST_DATABASE_URL is unset or the manga_chapters +// migration has not been applied, mirroring the literaryworks repository tests. +func newUnmatchedTestPool(t *testing.T) *pgxpool.Pool { + t.Helper() + dsn := os.Getenv("SILO_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("SILO_TEST_DATABASE_URL is not set") + } + ctx := context.Background() + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatalf("connect test database: %v", err) + } + t.Cleanup(pool.Close) + var tableName *string + if err := pool.QueryRow(ctx, `SELECT to_regclass('public.manga_chapters')::text`).Scan(&tableName); err != nil { + t.Fatalf("check manga_chapters table: %v", err) + } + if tableName == nil || *tableName == "" { + t.Skip("test database has not applied manga_chapters migration") + } + return pool +} + +// TestHandleListUnmatchedItems_ExcludesMangaChapters verifies that manga chapter +// rows (type='ebook' linked into a manga series via manga_chapters) are hidden +// from the admin Unmatched queue, while genuinely unmatched standalone items are +// still returned. This is the regression guard for issue #204: matched manga +// items were leaking into the queue because the chapter rows kept a non-matched +// status and the queries lacked the MangaChapterExclusionWhere guard. +func TestHandleListUnmatchedItems_ExcludesMangaChapters(t *testing.T) { + ctx := context.Background() + pool := newUnmatchedTestPool(t) + + suffix := time.Now().UnixNano() + seriesID := fmt.Sprintf("manga-series-%d", suffix) + chapterID := fmt.Sprintf("manga-chapter-%d", suffix) + ebookID := fmt.Sprintf("standalone-ebook-%d", suffix) + + t.Cleanup(func() { + _, _ = pool.Exec(ctx, `DELETE FROM manga_chapters WHERE chapter_content_id = $1`, chapterID) + _, _ = pool.Exec(ctx, `DELETE FROM media_items WHERE content_id = ANY($1)`, []string{seriesID, chapterID, ebookID}) + }) + + seed := func(contentID, mediaType, title, status string) { + t.Helper() + if _, err := pool.Exec(ctx, ` + INSERT INTO media_items (content_id, type, title, status, genres) + VALUES ($1, $2, $3, $4, '{}'::text[]) + `, contentID, mediaType, title, status); err != nil { + t.Fatalf("seed media item %s: %v", contentID, err) + } + } + + // Titles embed the unique suffix so the search filter below scopes the + // query to exactly these seeded rows in the shared test database. + tag := fmt.Sprintf("issue204-%d", suffix) + // The matched manga series is correctly out of the queue already. + seed(seriesID, "manga", "Attack on Titan "+tag, "matched") + // The chapter row is a type='ebook' sub-unit that stays in a non-matched + // status; before the fix it leaked into the queue. + seed(chapterID, "ebook", "Attack on Titan Ch. 1 "+tag, "pending") + // A genuinely unmatched standalone ebook must still appear in the queue. + seed(ebookID, "ebook", "Some Loose Ebook "+tag, "unmatched") + + if _, err := pool.Exec(ctx, ` + INSERT INTO manga_chapters (chapter_content_id, series_content_id, chapter_index) + VALUES ($1, $2, 1) + `, chapterID, seriesID); err != nil { + t.Fatalf("link manga chapter: %v", err) + } + + h := &LibraryHandler{pool: pool} + r := chi.NewRouter() + r.Get("/libraries/unmatched-items", h.HandleListUnmatchedItems) + + // Scope the query to our seeded rows via the search filter so the assertion + // is robust against other data in the shared test database. + req := httptest.NewRequest(http.MethodGet, "/libraries/unmatched-items?q="+tag, nil) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + + var resp unmatchedItemsListResponse + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + + for _, item := range resp.Items { + if item.ContentID == chapterID { + t.Errorf("manga chapter %s leaked into the Unmatched queue", chapterID) + } + } + + foundStandalone := false + for _, item := range resp.Items { + if item.ContentID == ebookID { + foundStandalone = true + } + } + if !foundStandalone { + t.Errorf("standalone unmatched ebook %s missing from the queue", ebookID) + } + + // Total must also exclude the chapter: only the standalone ebook matches the + // scoped search, so the count is exactly 1. + if resp.Total != 1 { + t.Errorf("Total = %d, want 1 (chapter must be excluded from the count too)", resp.Total) + } +}