Compare commits

..
2 Commits
Author SHA1 Message Date
John O'Keefe f1e4b2a603 fix(admin): un-dead the archived page's purge and delete buttons
Release / build-and-push (push) Successful in 3m15s
Two independent dead-ends, confirmed on prod's /admin/archived
(2026-10): Purge All rendered but clicking it did nothing at all, and
per-row Delete never even showed its confirm dialog.

Purge All is Alpine-bound (@click="purgeArchivedItems()"), but this
page's <body> lacked x-data="admin" — the only admin page without it.
Alpine evaluates directives only inside an x-data tree, so the
attribute was inert: the admin component (and its method, registered
in admin.ts) never instantiated here. The body now carries
x-data="admin", matching admin.templ and admin_library.templ.

Per-item Delete was a second, subtler dead-end: admin.ts's API helpers
guard with `if (!token) return;` BEFORE the confirm dialog, and the
reporting session had no localStorage token — SSR pages run on the
cookie session, so the web UI works while every Bearer-
authenticated admin action (purge, delete, restore, merge, scan-all,
cancel-scan) silently no-ops with zero feedback. Those guards now go
through requireToken, which toasts "Session expired — sign out and
back in, then retry" when the token is absent. The two passive
page-load probes (loadWatchStatus, checkActiveScans) deliberately keep
the silent return: a toast there would fire on every visit.

Pinned by TestAdminArchivedPageHasAlpineScope: renders the page and
asserts the body scope and the action buttons coexist in one render.

After deploy: hard-refresh prod, click Purge All (confirm appears);
if the token toast fires on any admin action, log out and back in once
to stash a fresh API token.
2026-10-28 09:37:14 -04:00
John O'Keefe 795225f097 fix(scanner): reconcile archive pass per library, not per folder
Release / build-and-push (push) Successful in 3m5s
The archive pass looped over the scan's folders and, inside each
iteration, compared the ENTIRE library's item list against that one
folder's walk. With two registered folders every item living under the
other folder was flagged "missing (1/2)" on every reconciling scan —
prod's audiobook library (the only multi-folder one) showed 743/743
flagged — and nothing reached the two-consecutive-miss threshold only
because each scan's walk clears missing_scan_count before the pass
re-flags it. With three or more folders the defect stops bouncing: a
later iteration sees an earlier iteration's count-1 marks on items its
own walk lacks and archives them mid-pass — one registered folder away
from archiving a third of a library per scan. Single-folder libraries
were unaffected, which is why comics/ebooks/manga showed zero flags.

Folder-book presence was suspected alongside it (audioBookRoots read
during the comparison), but the map is written for every >=2-track
folder-book root on every walk (processAudioBookFile, since 6216bd9b);
the flagged folder-books were victims of the same cross-folder
comparison. The roots attribution did carry the same unchecked
strings.HasPrefix boundary bug as the rest of the scanner (8th site,
variable named `root`, missed by the earlier sweep); it now uses
folderContains.

The pass now groups folders by owning library and, per library,
compares ListMediaItemsByLibraryIncludingArchived against the UNION of
all that library's folders' walks. The coverage (partial-scope) guard,
the walk-failure skip, and the mark/archive/purge item loop are
unchanged.

Regression test (archive_multifolder_test.go) builds a two-folder
audiobooks library — single-file book in one folder, folder-of-tracks
book in the other — and scans twice: pre-fix both items are flagged
missing on both scans (verified by running the test against the old
code), post-fix neither is ever flagged or archived.
2026-10-28 01:30:41 -04:00
6 changed files with 217 additions and 25 deletions

No files matched your search

@@ -0,0 +1,87 @@
package main
// Regression tests for the archive reconciliation pass: a multi-folder
// library must be reconciled against the UNION of its folders' walks, not
// once per folder. The old per-folder loop listed every library item
// against each single folder's walk, so with two registered folders every
// item living under the other folder was flagged "missing (1/2)" on every
// scan (prod: 743/743 audiobooks stuck at count 1 — unarchivable only
// because each scan's walk kept clearing them), and with three or more
// folders the crossed marks would have reached the archive threshold
// mid-pass and archived real content. Folder-book roots (directory
// file_path) must also count as present across folder boundaries — the
// file walk never emits a directory.
import (
"context"
"fmt"
"path/filepath"
"testing"
"time"
"bookhoard/internal/database"
"bookhoard/internal/services"
"github.com/jackc/pgx/v5/pgtype"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// scanTestFolders runs a synchronous scan over several folders — the
// multi-folder shape the archive pass must get right (scanTestFolder
// covers the single-folder case).
func scanTestFolders(t *testing.T, setup *TestServerSetup, adminID pgtype.UUID, folders []string) {
t.Helper()
scanner := services.NewMediaScanner(setup.DB)
defer func() { _ = scanner.Close() }()
scanner.SetAdminID(adminID)
require.NoError(t, scanner.SetFolders(folders, false))
require.NoError(t, scanner.ScanFolders(context.Background()))
}
func TestArchivePassMultiFolderLibraryKeepsEverythingPresent(t *testing.T) {
setup := setupTestServer(t)
t.Cleanup(func() { _ = setup.Close() })
libID, folderA := createTypedTestLibrary(t, setup, fmt.Sprintf("Test Archive MultiFolder %d", time.Now().UnixNano()), "audiobooks")
t.Cleanup(func() { deleteLibraries(t, setup, libID) }) // after setup.Close defer (LIFO): pool still open
// Second registered folder in the SAME library — the shape the
// per-folder reconciliation got wrong.
folderB := t.TempDir()
_, err := setup.DB.AddLibraryFolder(context.Background(), database.AddLibraryFolderParams{
LibraryID: libID,
FolderPath: folderB,
})
require.NoError(t, err)
admin, _ := moveTestAdminID(t, setup)
// Folder A holds a lone single-file book; folder B holds a
// folder-of-tracks book whose file_path is a DIRECTORY.
writeTestTrack(t, filepath.Join(folderA, "Author A", "Lone Book", "lonely.mp3"))
writeTestTrack(t, filepath.Join(folderB, "Author B", "Folder Book", "01 First.mp3"))
writeTestTrack(t, filepath.Join(folderB, "Author B", "Folder Book", "02 Second.mp3"))
// Two consecutive scans: the first proves no flags are raised, the
// second proves the flag/walk bounce (clear → re-flag) is gone too.
for scanNo := 1; scanNo <= 2; scanNo++ {
scanTestFolders(t, setup, admin, []string{folderA, folderB})
items := listLibraryAudioItems(t, setup, libID)
var singleFile, folderBook bool
for _, item := range items {
assert.False(t, item.ArchivedAt.Valid, "scan %d: item %q must not be archived", scanNo, item.FilePath)
assert.Equal(t, int32(0), item.MissingScanCount, "scan %d: item %q must not be flagged missing", scanNo, item.FilePath)
switch item.FilePath {
case "Author A/Lone Book/lonely.mp3":
singleFile = true
case "Author B/Folder Book":
folderBook = true
}
}
require.True(t, singleFile, "scan %d: single-file book must exist", scanNo)
require.True(t, folderBook, "scan %d: folder book must exist at its directory path", scanNo)
require.Len(t, items, 2, "scan %d: exactly the two books, no per-track strays", scanNo)
}
}
+54 -16
View File
@@ -831,18 +831,47 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error {
// retention window (ARCHIVE_RETENTION_DAYS; 0 = manual purge only).
// Every branch logs - the previous hard-delete cleanup failed silently
// and left orphaned rows undetected.
// Group this scan's folders by owning library: the reconciliation is
// per LIBRARY, comparing every library item against the union of all
// its folders' walks. The old per-folder loop listed the whole
// library's items against each single folder's walk, so with two
// folders every item living under the other folder was flagged
// missing on every scan (prod: 743/743 audiobooks stuck at
// "missing (1/2)", unarchivable only because the walk kept clearing
// them) — and with three or more folders earlier iterations' marks
// crossed later ones' walks and archived real content mid-pass.
scannedSet := make(map[string]bool, len(s.folders))
for _, scanned := range s.folders {
scannedSet[scanned] = true
}
type archiveScope struct {
libraryID pgtype.UUID
name string
folders []string
}
scopes := make([]archiveScope, 0, len(s.folders))
scopeIndex := make(map[pgtype.UUID]int, len(s.folders))
for _, folder := range s.folders {
lib, err := s.db.GetLibraryByFolder(ctx, folder)
if err != nil {
fmt.Printf("[ARCHIVE] Warning: no library found for folder %s, skipping it in archive pass: %v\n", folder, err)
continue
}
idx, seen := scopeIndex[lib.LibraryID]
if !seen {
idx = len(scopes)
scopeIndex[lib.LibraryID] = idx
scopes = append(scopes, archiveScope{libraryID: lib.LibraryID, name: lib.Name})
}
scopes[idx].folders = append(scopes[idx].folders, folder)
}
for _, scope := range scopes {
if ctx.Err() != nil {
fmt.Printf("[ARCHIVE] Scan cancelled, stopping archive pass\n")
return ctx.Err()
}
lib, err := s.db.GetLibraryByFolder(ctx, folder)
if err != nil {
fmt.Printf("[ARCHIVE] Warning: no library found for folder %s, skipping archive pass: %v\n", folder, err)
continue
}
libraryID := lib.LibraryID
libraryID := scope.libraryID
// Partial-scope guard (per-folder scans, 2026-10): the
// reconciliation compares EVERY library item against this scan's
@@ -855,10 +884,6 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error {
fmt.Printf("[ARCHIVE] Warning: failed to list folders for library, skipping archive pass: %v\n", regErr)
continue
}
scannedSet := make(map[string]bool, len(s.folders))
for _, scanned := range s.folders {
scannedSet[scanned] = true
}
covered := 0
for _, rf := range registeredFolders {
if scannedSet[rf.FolderPath] {
@@ -867,7 +892,7 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error {
}
if covered < len(registeredFolders) {
fmt.Printf("[ARCHIVE] Skipping reconciliation for %s: scan covered %d of %d registered folders\n",
lib.Name, covered, len(registeredFolders))
scope.name, covered, len(registeredFolders))
continue
}
@@ -877,8 +902,11 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error {
continue
}
// Build set of scanned file paths for this folder
// Build the set of scanned paths across ALL of this library's
// folders.
scannedPaths := make(map[string]bool)
walkFailed := false
for _, folder := range scope.folders {
if err := filepath.WalkDir(folder, func(path string, d os.DirEntry, walkErr error) error {
if walkErr != nil {
return walkErr
@@ -889,14 +917,24 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error {
return nil
}); err != nil {
fmt.Printf("[ARCHIVE] Warning: failed to walk directory %s, skipping archive pass: %v\n", folder, err)
continue // Skip to next folder to avoid false archivals
walkFailed = true
break
}
}
if walkFailed {
continue // Skip this library to avoid false archivals
}
// Folder-book roots count as present: the item's file_path is the
// DIRECTORY, which the file walk above never emits.
// DIRECTORY, which the file walk above never emits. Attribution is
// boundary-safe — a sibling folder whose name merely extends this
// one's must not contribute its roots here.
for root := range s.audioBookRoots {
if strings.HasPrefix(root, folder) {
for _, folder := range scope.folders {
if folderContains(folder, root) {
scannedPaths[s.getRelativePath(root)] = true
break
}
}
}
+1 -1
View File
@@ -10,7 +10,7 @@ templ AdminArchived(user User, items []ArchivedItem, retentionDays int) {
<link href="/static/style.css" rel="stylesheet"/>
<link rel="icon" type="image/svg+xml" href="/static/favicon.svg"/>
</head>
<body class={ "theme-" + user.Theme }>
<body x-data="admin" class={ "theme-" + user.Theme }>
@Header(user, "/admin/archived")
<main class="p-8">
<div class="max-w-5xl">
+47
View File
@@ -0,0 +1,47 @@
package templates
import (
"context"
"strings"
"testing"
"time"
)
// The archived-items admin page binds its buttons through the page-wide
// Alpine component: Purge All is @click="purgeArchivedItems()" and the
// per-row Delete/Restore/Merge buttons rely on listeners main.ts binds at
// module load. The body tag therefore MUST carry x-data="admin" — the
// other admin pages have it, and when this page's body lost it the purge
// button rendered as an inert attribute (Alpine only evaluates directives
// inside an x-data tree; the click did literally nothing).
//
// Regression: /admin/archived rendered <body class="theme-…"> with no
// Alpine scope, so "Purge All Archived" was dead (prod report 2026-10).
func TestAdminArchivedPageHasAlpineScope(t *testing.T) {
archivedAt := time.Date(2026, 10, 28, 4, 38, 0, 0, time.UTC)
var b strings.Builder
err := AdminArchived(User{ID: "u1", Timezone: "UTC"}, []ArchivedItem{{
ID: "m1",
Title: "Lost Book",
Author: "An Author",
LibraryName: "Audiobooks",
FilePath: "Author/Book/file.m4b",
MissingScans: 2,
ArchivedAt: &archivedAt,
Status: "Archived - pending purge",
}}, 0).Render(context.Background(), &b)
if err != nil {
t.Fatalf("render: %v", err)
}
out := b.String()
if !strings.Contains(out, `<body x-data="admin"`) {
t.Error("archived page body missing x-data=\"admin\" — the Alpine-bound purge button would be dead")
}
// The action buttons the page depends on must be present in the same
// render, so the pin cannot pass on an empty shell.
for _, want := range []string{"purgeArchivedItems()", `data-delete-archived="m1"`} {
if !strings.Contains(out, want) {
t.Errorf("archived page missing %q", want)
}
}
}
+1 -1
View File
@@ -38,7 +38,7 @@ func AdminArchived(user User, items []ArchivedItem, retentionDays int) templ.Com
if templ_7745c5c3_Err != nil {
return templ_7745c5c3_Err
}
templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 2, "<body class=\"")
templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 2, "<body x-data=\"admin\" class=\"")
if templ_7745c5c3_Err != nil {
return templ_7745c5c3_Err
}
+27 -7
View File
@@ -1,8 +1,23 @@
import { Alpine } from "./alpine";
import { showToast } from "./toast";
async function scanAllLibraries(): Promise<void> {
// Admin API calls authenticate with the Bearer token stashed at login,
// while the SSR pages themselves run on the cookie session — so a valid
// web session can still have an empty/stale localStorage token. User-
// triggered actions must never die silently in that state (the
// /admin/archived purge + delete buttons were invisible dead-ends: the
// missing-token return fired before the confirm dialog): requireToken
// fails loudly instead.
function requireToken(): string | null {
const token = localStorage.getItem("token");
if (!token) {
showToast("Session expired — sign out and back in, then retry", "error");
}
return token;
}
async function scanAllLibraries(): Promise<void> {
const token = requireToken();
if (!token) return;
try {
@@ -225,7 +240,7 @@ function updateLibraryProgress(jobId: string, status: any): void {
// aborts the walk at its next context check and skips the archive pass,
// so cancelling never archives unvisited files.
async function cancelScan(jobId: string): Promise<void> {
const token = localStorage.getItem("token");
const token = requireToken();
if (!token) return;
const btn = document.querySelector<HTMLButtonElement>(`[data-cancel-job="${jobId}"]`);
@@ -257,7 +272,7 @@ async function cancelScan(jobId: string): Promise<void> {
// list so it also covers jobs this page never started (e.g. started
// before a refresh), then cancels each one.
async function cancelAllScans(): Promise<void> {
const token = localStorage.getItem("token");
const token = requireToken();
if (!token) return;
try {
@@ -287,6 +302,8 @@ function setCancelAllVisible(visible: boolean): void {
// widget and re-attach polling. Websocket start-events only reach pages
// that were already open when the scan began.
async function checkActiveScans(): Promise<void> {
// Passive page-load probe, not a user action: stay silent when the API
// token is absent (requireToken's toast would fire on every page visit).
const token = localStorage.getItem("token");
if (!token) return;
try {
@@ -369,6 +386,9 @@ function hideScanProgress(): void {
}
async function loadWatchStatus(): Promise<void> {
// Passive status fetch inside the admin component's init, not a user
// action: stay silent when the API token is absent (requireToken's
// toast would fire on every page visit).
const token = localStorage.getItem("token");
if (!token) return;
@@ -399,7 +419,7 @@ function stopScanStatusPolling(): void {
// window for the library admin page's inline button; reading history is
// deleted with the rows, so a confirm dialog guards it.
async function purgeArchivedItems(): Promise<void> {
const token = localStorage.getItem("token");
const token = requireToken();
if (!token) return;
if (
@@ -431,7 +451,7 @@ async function purgeArchivedItems(): Promise<void> {
// its file to return (POST /api/media-items/:id/unarchive). If the file is
// still gone the next scan hides it again.
async function unarchiveItem(id: string): Promise<void> {
const token = localStorage.getItem("token");
const token = requireToken();
if (!token) return;
try {
const resp = await fetch(`/api/media-items/${id}/unarchive`, {
@@ -452,7 +472,7 @@ async function unarchiveItem(id: string): Promise<void> {
// Permanently delete one archived item and its reading history
// (DELETE /api/media-items/:id).
async function deleteArchivedItem(id: string): Promise<void> {
const token = localStorage.getItem("token");
const token = requireToken();
if (!token) return;
if (
!window.confirm(
@@ -481,7 +501,7 @@ async function deleteArchivedItem(id: string): Promise<void> {
// same content hash (POST /api/media-items/:id/merge), then remove the
// archived entry. Covers copy-then-delete-later moves between libraries.
async function mergeArchivedItem(sourceId: string, targetId: string): Promise<void> {
const token = localStorage.getItem("token");
const token = requireToken();
if (!token) return;
if (
!window.confirm(