diff --git a/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationUiTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationUiTest.kt index 1f21c4a3..f988ac13 100644 --- a/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationUiTest.kt +++ b/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationUiTest.kt @@ -27,7 +27,7 @@ class NotificationUiTest { // Library health pages the app has no screen for open the Admin landing. assertEquals(Admin, routeForLink("/collection/duplicates")) assertEquals(Admin, routeForLink("/collection/duplicates?view=resolved")) - assertEquals(Admin, routeForLink("/admin/missing-files")) + assertEquals(Admin, routeForLink("/collection/missing")) assertEquals(Admin, routeForLink("/admin")) assertNull(routeForLink("/somewhere-new")) } diff --git a/internal/api/admin_library_missing.go b/internal/api/admin_library_missing.go index d29f3df4..26d2f0df 100644 --- a/internal/api/admin_library_missing.go +++ b/internal/api/admin_library_missing.go @@ -2,6 +2,7 @@ package api import ( "net/http" + "time" "github.com/jackc/pgx/v5/pgtype" @@ -31,10 +32,13 @@ type missingTrackView struct { // reacquisitionStateView is what the wanted list says about one album // (milestone #290, M502 #5520): when it was wanted back, and when the library // held it again. A missing album with no want yet is still inside the grace -// window, or MusicBrainz cannot name it. +// window, or MusicBrainz cannot name it. Want is that want as the wanted list +// sends it, so the Missing page can show its search and download and act on +// it (M504). type reacquisitionStateView struct { - WantedAt *string `json:"wanted_at"` - HeldAt *string `json:"held_at"` + WantedAt *string `json:"wanted_at"` + HeldAt *string `json:"held_at"` + Want *wantView `json:"want"` } // missingGroupView is a directory's worth of missing tracks. @@ -174,7 +178,13 @@ func (h *handlers) attachReacquisitionState( // groupMissingByDirectory folded them in. dirAlbum := make(map[string]pgtype.UUID, len(groups)) ids := make([]pgtype.UUID, 0, len(groups)) + // The latest loss in each folder: a want held before it is the album + // as it was before its files went, not the library holding it again. + dirLost := make(map[string]time.Time, len(groups)) for _, row := range rows { + if row.MissingSince.Valid && row.MissingSince.Time.After(dirLost[row.Directory]) { + dirLost[row.Directory] = row.MissingSince.Time + } if _, seen := dirAlbum[row.Directory]; seen { continue } @@ -194,6 +204,7 @@ func (h *handlers) attachReacquisitionState( byAlbum[uuidToString(w.AlbumID)] = w } } + full := h.wantViewsByID(r, q, byAlbum) for i := range groups { albumID, ok := dirAlbum[groups[i].Directory] if !ok { @@ -203,6 +214,12 @@ func (h *handlers) attachReacquisitionState( if !ok { continue } + // Held before its files went (every library album is on the list, + // held, since #5567): not wanted back yet, which the sweep does once + // the grace window has passed. + if w.FulfilledAt.Valid && !w.FulfilledAt.Time.After(dirLost[groups[i].Directory]) { + continue + } view := &reacquisitionStateView{} wantedAt := formatTimestamp(w.WantedAt) view.WantedAt = &wantedAt @@ -210,6 +227,36 @@ func (h *handlers) attachReacquisitionState( heldAt := formatTimestamp(w.FulfilledAt) view.HeldAt = &heldAt } + if v, ok := full[uuidToString(w.WantID)]; ok { + view.Want = &v + } groups[i].Reacquisition = view } } + +// wantViewsByID reads each album's current want in full, keyed by want id. +// Best-effort like the state it decorates: a failure leaves the groups with +// their dates and no want. +func (h *handlers) wantViewsByID( + r *http.Request, + q *dbq.Queries, + byAlbum map[string]dbq.GetWantsForAlbumsRow, +) map[string]wantView { + ids := make([]pgtype.UUID, 0, len(byAlbum)) + for _, w := range byAlbum { + ids = append(ids, w.WantID) + } + out := make(map[string]wantView, len(ids)) + if len(ids) == 0 { + return out + } + rows, err := q.ListWants(r.Context(), dbq.ListWantsParams{State: "all", WantIds: ids, MaxRows: int32(len(ids))}) + if err != nil { + h.logger.Warn("admin missing: want detail lookup failed", "err", err) + return out + } + for _, row := range rows { + out[uuidToString(row.ID)] = wantViewOf(row) + } + return out +} diff --git a/internal/api/admin_library_missing_wants_test.go b/internal/api/admin_library_missing_wants_test.go new file mode 100644 index 00000000..35a4d621 --- /dev/null +++ b/internal/api/admin_library_missing_wants_test.go @@ -0,0 +1,84 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/jackc/pgx/v5/pgtype" +) + +// seedMissingWant marks a track's file missing for two days and puts its +// album on the wanted list, held at heldDaysAgo (0 for an open want) and +// last searched with nothing found. +func seedMissingWant(t *testing.T, h *handlers, trackID, albumID pgtype.UUID, group string, heldDaysAgo int) string { + t.Helper() + ctx := context.Background() + if _, err := h.pool.Exec(ctx, + `UPDATE tracks SET missing_since = now() - interval '2 days' WHERE id = $1`, trackID); err != nil { + t.Fatalf("mark missing: %v", err) + } + if _, err := h.pool.Exec(ctx, `DELETE FROM wants WHERE release_group_mbid = $1`, group); err != nil { + t.Fatalf("clear want: %v", err) + } + var id pgtype.UUID + if err := h.pool.QueryRow(ctx, + `INSERT INTO wants (release_group_mbid, artist_mbid, artist_name, title, reason, album_id, + wanted_at, fulfilled_at, searched_at, search_outcome) + VALUES ($1, 'artist-'||$1, 'Artist', 'Album '||$1, 'library', $2, + now() - interval '30 days', + CASE WHEN $3::int > 0 THEN now() - make_interval(days => $3::int) END, + now() - interval '1 hour', 'nothing') + RETURNING id`, group, albumID, heldDaysAgo).Scan(&id); err != nil { + t.Fatalf("insert want: %v", err) + } + return uuidToString(id) +} + +// A folder's album that was held before its files went is not wanted back +// yet (every library album is on the list, held, since #5567); an open want +// comes with its search, so the Missing page can show it and act on it (M504). +func TestListMissingTracks_WantState(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + admin := seedUser(t, pool, "admin", "pw", true) + + heldTrack, _ := seedTrackOnDisk(t, h, "missheld") + openTrack, _ := seedTrackOnDisk(t, h, "missopen") + seedMissingWant(t, h, heldTrack.ID, heldTrack.AlbumID, "rg-missing-held", 10) + openID := seedMissingWant(t, h, openTrack.ID, openTrack.AlbumID, "rg-missing-open", 0) + + req := withUser(httptest.NewRequest(http.MethodGet, "/api/admin/library/missing", nil), admin) + w := httptest.NewRecorder() + h.handleListMissingTracks(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status %d: %s", w.Code, w.Body.String()) + } + var resp adminMissingResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if len(resp.Groups) != 2 { + t.Fatalf("groups = %d, want 2", len(resp.Groups)) + } + byTitle := map[string]missingGroupView{} + for _, g := range resp.Groups { + byTitle[g.Tracks[0].Title] = g + } + + if r := byTitle["AdmQ Track missheld"].Reacquisition; r != nil { + t.Errorf("held before the loss: reacquisition = %+v, want none", r) + } + r := byTitle["AdmQ Track missopen"].Reacquisition + if r == nil || r.WantedAt == nil || r.HeldAt != nil { + t.Fatalf("open want: reacquisition = %+v, want wanted and not held", r) + } + if r.Want == nil || r.Want.ID != openID { + t.Fatalf("open want: want = %+v, want id %s", r.Want, openID) + } + if r.Want.Search == nil || r.Want.Search.Outcome != "nothing" { + t.Errorf("open want: search = %+v, want outcome nothing", r.Want.Search) + } +} diff --git a/internal/api/admin_wanted.go b/internal/api/admin_wanted.go index d8642615..9f2f5ece 100644 --- a/internal/api/admin_wanted.go +++ b/internal/api/admin_wanted.go @@ -270,59 +270,65 @@ func (h *handlers) handleListWants(w http.ResponseWriter, r *http.Request) { } for _, row := range rows { out.Total = row.Total - v := wantView{ - ID: uuidToString(row.ID), ReleaseGroupMBID: row.ReleaseGroupMbid, Title: row.Title, - ArtistMBID: row.ArtistMbid, ArtistName: row.ArtistName, - PrimaryType: row.PrimaryType, SecondaryTypes: row.SecondaryTypes, - Year: first4(row.FirstReleaseDate), Reason: row.Reason, - WantedAt: formatTimestamp(row.WantedAt), Upgrade: row.Upgrade, HeldQuality: row.HeldQuality, - ImageURL: groupArtURL(musicbrainz.ReleaseGroup{MBID: row.ReleaseGroupMbid, Title: row.Title, ArtistCredit: row.ArtistName}), - } - if row.LibraryArtistID.Valid { - v.ArtistID = uuidToString(row.LibraryArtistID) - } - if row.AlbumID.Valid { - v.AlbumID = uuidToString(row.AlbumID) - } - if row.RequestedBy != nil { - v.RequestedBy = *row.RequestedBy - } - if row.FulfilledAt.Valid { - held := formatTimestamp(row.FulfilledAt) - v.HeldAt = &held - } - if v.SecondaryTypes == nil { - v.SecondaryTypes = []string{} - } - if row.SearchedAt.Valid { - v.Search = &wantSearchView{ - SearchedAt: formatTimestamp(row.SearchedAt), Outcome: row.SearchOutcome, - Considered: row.SearchConsidered, BestRelease: row.BestRelease, - BestVerdict: row.BestVerdict, Error: row.SearchError, - } - } - if row.GrabName != nil { - v.Grab = &wantGrabView{ID: uuidToString(row.GrabID), Name: *row.GrabName, - GrabbedAt: formatTimestamp(row.GrabbedAt), Live: !row.GrabEndedAt.Valid, - Origin: deref(row.GrabOrigin), Quality: deref(row.GrabQuality), EndReason: deref(row.GrabEndReason), - ImportState: deref(row.GrabImportState), ImportReason: deref(row.GrabImportReason), - EndDetail: deref(row.GrabEndDetail)} - if row.GrabProgress != nil { - v.Grab.Progress = *row.GrabProgress - } - if row.GrabImportedFiles != nil { - v.Grab.ImportedFiles = *row.GrabImportedFiles - } - if row.GrabImportedAt.Valid { - at := formatTimestamp(row.GrabImportedAt) - v.Grab.ImportedAt = &at - } - } - out.Items = append(out.Items, v) + out.Items = append(out.Items, wantViewOf(row)) } writeJSON(w, http.StatusOK, out) } +// wantViewOf is one ListWants row as the wanted list and the Missing page +// send it. +func wantViewOf(row dbq.ListWantsRow) wantView { + v := wantView{ + ID: uuidToString(row.ID), ReleaseGroupMBID: row.ReleaseGroupMbid, Title: row.Title, + ArtistMBID: row.ArtistMbid, ArtistName: row.ArtistName, + PrimaryType: row.PrimaryType, SecondaryTypes: row.SecondaryTypes, + Year: first4(row.FirstReleaseDate), Reason: row.Reason, + WantedAt: formatTimestamp(row.WantedAt), Upgrade: row.Upgrade, HeldQuality: row.HeldQuality, + ImageURL: groupArtURL(musicbrainz.ReleaseGroup{MBID: row.ReleaseGroupMbid, Title: row.Title, ArtistCredit: row.ArtistName}), + } + if row.LibraryArtistID.Valid { + v.ArtistID = uuidToString(row.LibraryArtistID) + } + if row.AlbumID.Valid { + v.AlbumID = uuidToString(row.AlbumID) + } + if row.RequestedBy != nil { + v.RequestedBy = *row.RequestedBy + } + if row.FulfilledAt.Valid { + held := formatTimestamp(row.FulfilledAt) + v.HeldAt = &held + } + if v.SecondaryTypes == nil { + v.SecondaryTypes = []string{} + } + if row.SearchedAt.Valid { + v.Search = &wantSearchView{ + SearchedAt: formatTimestamp(row.SearchedAt), Outcome: row.SearchOutcome, + Considered: row.SearchConsidered, BestRelease: row.BestRelease, + BestVerdict: row.BestVerdict, Error: row.SearchError, + } + } + if row.GrabName != nil { + v.Grab = &wantGrabView{ID: uuidToString(row.GrabID), Name: *row.GrabName, + GrabbedAt: formatTimestamp(row.GrabbedAt), Live: !row.GrabEndedAt.Valid, + Origin: deref(row.GrabOrigin), Quality: deref(row.GrabQuality), EndReason: deref(row.GrabEndReason), + ImportState: deref(row.GrabImportState), ImportReason: deref(row.GrabImportReason), + EndDetail: deref(row.GrabEndDetail)} + if row.GrabProgress != nil { + v.Grab.Progress = *row.GrabProgress + } + if row.GrabImportedFiles != nil { + v.Grab.ImportedFiles = *row.GrabImportedFiles + } + if row.GrabImportedAt.Valid { + at := formatTimestamp(row.GrabImportedAt) + v.Grab.ImportedAt = &at + } + } + return v +} + // wantTracksView is an album's track list for the Wanted view (#5572): the // library's own tracks for an album it holds or held, else MusicBrainz's // standard edition of what is wanted. diff --git a/internal/db/dbq/wanted.sql.go b/internal/db/dbq/wanted.sql.go index 77f6701a..c1390d4c 100644 --- a/internal/db/dbq/wanted.sql.go +++ b/internal/db/dbq/wanted.sql.go @@ -1147,18 +1147,21 @@ SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.reas ELSE w.fulfilled_at IS NULL AND w.reason <> 'missing' END AND ($2::text = '' OR w.artist_mbid = $2::text) - AND ($3::text = '' - OR strpos(lower(w.artist_name), lower($3::text)) > 0 - OR strpos(lower(w.title), lower($3::text)) > 0) + -- want_ids narrows to given wants: the Missing page's folders (M504). + AND (COALESCE(cardinality($3::uuid[]), 0) = 0 OR w.id = ANY($3::uuid[])) + AND ($4::text = '' + OR strpos(lower(w.artist_name), lower($4::text)) > 0 + OR strpos(lower(w.title), lower($4::text)) > 0) ORDER BY lower(COALESCE(NULLIF(la.sort_name, ''), w.artist_name)), w.artist_mbid, NULLIF(COALESCE(NULLIF(w.first_release_date, ''), g.first_release_date, ''), '') NULLS LAST, lower(w.title), w.id - LIMIT $5 OFFSET $4 + LIMIT $6 OFFSET $5 ` type ListWantsParams struct { State string ArtistMbid string + WantIds []pgtype.UUID Q string Skip int32 MaxRows int32 @@ -1204,7 +1207,8 @@ type ListWantsRow struct { } // The Wanted page: open wants (all but missing ones, or only missing ones), -// the held ones, or all of them; artist_mbid narrows to one artist's page. Ordered like the library (#5572): artists A–Z by the +// the held ones, or all of them; artist_mbid narrows to one artist's page, +// want_ids to given wants. Ordered like the library (#5572): artists A–Z by the // library's sort name when it holds the artist, each artist's albums by // release date, so the page groups them as they arrive. q narrows to an // artist or album title; total counts every match, past the page. @@ -1212,6 +1216,7 @@ func (q *Queries) ListWants(ctx context.Context, arg ListWantsParams) ([]ListWan rows, err := q.db.Query(ctx, listWants, arg.State, arg.ArtistMbid, + arg.WantIds, arg.Q, arg.Skip, arg.MaxRows, diff --git a/internal/db/queries/wanted.sql b/internal/db/queries/wanted.sql index f2ba3d7b..ba2b1edc 100644 --- a/internal/db/queries/wanted.sql +++ b/internal/db/queries/wanted.sql @@ -177,7 +177,8 @@ DELETE FROM wants WHERE id = $1; -- name: ListWants :many -- The Wanted page: open wants (all but missing ones, or only missing ones), --- the held ones, or all of them; artist_mbid narrows to one artist's page. Ordered like the library (#5572): artists A–Z by the +-- the held ones, or all of them; artist_mbid narrows to one artist's page, +-- want_ids to given wants. Ordered like the library (#5572): artists A–Z by the -- library's sort name when it holds the artist, each artist's albums by -- release date, so the page groups them as they arrive. q narrows to an -- artist or album title; total counts every match, past the page. @@ -216,6 +217,8 @@ SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.reas ELSE w.fulfilled_at IS NULL AND w.reason <> 'missing' END AND (sqlc.arg(artist_mbid)::text = '' OR w.artist_mbid = sqlc.arg(artist_mbid)::text) + -- want_ids narrows to given wants: the Missing page's folders (M504). + AND (COALESCE(cardinality(sqlc.arg(want_ids)::uuid[]), 0) = 0 OR w.id = ANY(sqlc.arg(want_ids)::uuid[])) AND (sqlc.arg(q)::text = '' OR strpos(lower(w.artist_name), lower(sqlc.arg(q)::text)) > 0 OR strpos(lower(w.title), lower(sqlc.arg(q)::text)) > 0) diff --git a/internal/notifications/render.go b/internal/notifications/render.go index 3b1f511a..570cb897 100644 --- a/internal/notifications/render.go +++ b/internal/notifications/render.go @@ -108,7 +108,7 @@ func Render(kind Kind, payload []byte) Rendered { return Rendered{ Title: plural(p.Count, "track went missing", "tracks went missing"), Body: "Their files are no longer where the library expects them.", - Link: "/admin/missing-files", + Link: "/collection/missing", } case KindDuplicatesFound: return Rendered{ diff --git a/web/src/lib/admin/queues.ts b/web/src/lib/admin/queues.ts index 110fb957..f8d6149e 100644 --- a/web/src/lib/admin/queues.ts +++ b/web/src/lib/admin/queues.ts @@ -3,7 +3,8 @@ import type { AdminQueue, AdminSummary } from '#lib/api/types.js'; // The queues that wait on an admin (#5605), in the order the Overview lists // them. The rails and the Overview read the same list, so a queue that // needs you is counted the same in every place. Requests live under Admin; -// the library's health queues under Collection (M504). +// the library's health queues, Missing files among them, under Collection +// (M504). export type QueueKey = keyof AdminSummary; export type QueueDef = { @@ -21,7 +22,7 @@ export const QUEUES: QueueDef[] = [ { key: 'reports', href: '/collection/reports', one: 'broken track reported', many: 'broken tracks reported', none: 'reports' }, { key: 'duplicates', href: '/collection/duplicates', one: 'set of duplicates to review', many: 'sets of duplicates to review', none: 'duplicates' }, { key: 'suspect', href: '/collection/suspect-sources', one: 'track from a suspect source', many: 'tracks from suspect sources', none: 'suspect sources' }, - { key: 'missing', href: '/admin/missing-files', one: 'missing file', many: 'missing files', none: 'missing files' } + { key: 'missing', href: '/collection/missing', one: 'missing file', many: 'missing files', none: 'missing files' } ]; export function countOf(summary: AdminSummary | undefined, key: QueueKey): number { diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index a88649fd..2981ee08 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -363,6 +363,9 @@ export type AdminMissingTrack = { export type AdminReacquisitionState = { wanted_at: string | null; held_at: string | null; + // The want itself, as the wanted list sends it (M504), so the Missing page + // shows its search and download and acts on it. + want: Want | null; }; // A directory's worth of missing tracks. The server groups because the unit an diff --git a/web/src/lib/components/NotificationBell.test.ts b/web/src/lib/components/NotificationBell.test.ts index 97fbd523..c8c7038a 100644 --- a/web/src/lib/components/NotificationBell.test.ts +++ b/web/src/lib/components/NotificationBell.test.ts @@ -99,7 +99,7 @@ describe('NotificationBell', () => { kind: 'tracks_missing', title: '4 tracks went missing', body: 'Their files are no longer where the library expects them.', - link: '/admin/missing-files', + link: '/collection/missing', read_at: new Date().toISOString() }) ]); diff --git a/web/src/lib/components/wanted/CollectionRail.svelte b/web/src/lib/components/wanted/CollectionRail.svelte index 352ee263..0273c97d 100644 --- a/web/src/lib/components/wanted/CollectionRail.svelte +++ b/web/src/lib/components/wanted/CollectionRail.svelte @@ -8,9 +8,9 @@ // second, accented count when downloads are waiting on a person. Unmatched // holds the library albums no MusicBrainz album matched (#5579). Upgrades // holds the albums held but wanted for a better copy (#5524). Reports, - // Duplicates and Suspect sources are the library's health queues (M504): - // their counts come from the admin summary and are accented, as they wait - // on a person. + // Duplicates, Suspect sources and Missing files are the library's health + // queues (M504): their counts come from the admin summary and are + // accented, as they wait on a person. Missing files counts tracks. let { counts, summary }: { counts?: WantCounts; summary?: AdminSummary } = $props(); @@ -18,7 +18,6 @@ const items = $derived([ { href: '/collection', label: 'Catalogue', count: counts?.artists, exact: true, also: ['/collection/artist/'] }, { href: '/collection/wanted', label: 'Wanted', count: counts?.wanted }, - { href: '/collection/missing', label: 'Missing files', count: counts?.missing }, { href: '/collection/upgrades', label: 'Upgrades', count: counts?.upgrades }, { href: '/collection/activity', label: 'Activity', count: counts?.activity, attention: counts?.attention }, { href: '/collection/following', label: 'Following', count: counts?.following }, @@ -26,7 +25,8 @@ { href: '/collection/unmatched', label: 'Unmatched', count: counts?.unmatched }, { href: '/collection/reports', label: 'Reports', attention: countOf(summary, 'reports') }, { href: '/collection/duplicates', label: 'Duplicates', attention: countOf(summary, 'duplicates') }, - { href: '/collection/suspect-sources', label: 'Suspect sources', attention: countOf(summary, 'suspect') } + { href: '/collection/suspect-sources', label: 'Suspect sources', attention: countOf(summary, 'suspect') }, + { href: '/collection/missing', label: 'Missing files', attention: countOf(summary, 'missing') } ]); diff --git a/web/src/lib/components/wanted/WantList.svelte b/web/src/lib/components/wanted/WantList.svelte index 1de4bf83..a571d0bd 100644 --- a/web/src/lib/components/wanted/WantList.svelte +++ b/web/src/lib/components/wanted/WantList.svelte @@ -12,7 +12,8 @@ import QuickFilter from '#lib/components/QuickFilter.svelte'; import WantCandidates from '#lib/components/WantCandidates.svelte'; import WantTracks from './WantTracks.svelte'; - import type { SearchResult, Want, WantState } from '#lib/api/types.js'; + import { needsYou, searched, wantStatus } from '#lib/wanted/wantStatus.js'; + import type { Want, WantState } from '#lib/api/types.js'; // Albums on the Collection's list (M502 #5520–#5522): the ones still // wanted, the missing ones wanted back, or, given an artist, every album @@ -77,25 +78,6 @@ } } - function searched(w: Want, r: SearchResult): string { - let said: string; - switch (r.outcome) { - case 'grabbed': - said = `Took ${r.grabbed ?? r.best_release}.`; - break; - case 'waiting': - said = `Found ${r.considered}; nothing taken. Releases shows why.`; - break; - case 'nothing': - said = r.considered ? `Found ${r.considered}, none of them ${w.title}.` : `Nothing found for ${w.title}.`; - break; - default: - said = r.error ?? 'The search failed.'; - } - if (r.narrowed.length) said += ` ${r.narrowed.map((n) => n.label).join(', ')} didn't answer.`; - return said; - } - async function onSearch(w: Want) { searching = w.id; try { @@ -112,56 +94,6 @@ } } - // A download still running, in a line: how far it has got, or what its - // import is doing or waiting on. - function downloading(w: Want): string | null { - const g = w.grab; - if (!g?.live) return null; - switch (g.import_state) { - case 'downloading': - return `Downloading ${Math.floor(g.progress * 100)}% · ${g.name}`; - case 'importing': - return `Downloaded; checking it is the album and filing it · ${g.name}`; - case 'waiting': - return `Downloaded; ${g.import_reason || 'waiting to be imported'}`; - case 'review': - return `Needs you: ${g.import_reason}`; - case 'failed': - return `Import failed: ${g.import_reason}`; - } - const who = g.origin === 'operator' ? 'taken by you' : 'taken by the schedule'; - return `Downloading ${g.name}, ${who} ${relativeTime(g.grabbed_at)}`; - } - - // What the schedule last did about a want, in a line. - function status(w: Want): string { - const live = downloading(w); - if (live) return live; - const s = w.search; - let said = 'Not searched yet'; - if (s) { - const when = `Searched ${relativeTime(s.searched_at)}`; - switch (s.outcome) { - case 'nothing': - said = s.considered ? `${when}: ${s.considered} found, none of them this album` : `${when}: nothing found`; - break; - case 'waiting': - said = `${when}: ${s.considered} found, nothing taken`; - break; - case 'grabbed': - said = `${when}: took ${s.best_release}`; - break; - default: - said = `${when}: ${s.error}`; - } - } - const g = w.grab; - if (g?.import_state === 'refused') said += `. Last download refused: ${g.import_reason}`; - else if (g && !g.live && ['stalled', 'failed', 'gone'].includes(g.end_reason) && g.end_detail) - said += `. Last download given up: ${g.end_detail}`; - return said; - } - // How a held album arrived, when Minstrel fetched it. function arrived(w: Want): string | null { const g = w.grab; @@ -179,7 +111,6 @@ return w.held_quality ? `You have ${w.held_quality}; looking for better` : 'Looking for a better copy'; } - const attention = (w: Want) => w.grab?.live && (w.grab.import_state === 'review' || w.grab.import_state === 'failed'); function why(w: Want): string { if (w.reason === 'request') return w.requested_by ? `Requested by ${w.requested_by}` : 'Requested'; @@ -243,14 +174,11 @@

{#if artist}None of this artist's albums are in the catalogue. - {:else if list === 'missing'}No missing albums are wanted back. {:else if list === 'upgrade'}No album is wanted for a better copy. {:else}Nothing is wanted.{/if}

- {#if list === 'missing'} - An album whose files stay missing past the grace window on the Missing files page lands here. - {:else if list === 'upgrade'} + {#if list === 'upgrade'} An album you have below the quality set under Acquisition, or with a video rip among its tracks, lands here. @@ -301,11 +229,11 @@ {/if} {#if searchable(w)}

- {status(w)} - {#if attention(w)} + {wantStatus(w)} + {#if needsYou(w)} Decide on Activity {/if}
diff --git a/web/src/lib/components/wanted/WantList.test.ts b/web/src/lib/components/wanted/WantList.test.ts index 689b083d..662f2ed0 100644 --- a/web/src/lib/components/wanted/WantList.test.ts +++ b/web/src/lib/components/wanted/WantList.test.ts @@ -116,9 +116,9 @@ function setup( describe('WantList', () => { test('asks for its own state', () => { - setup([], 'missing'); - expect(createWantsQuery).toHaveBeenCalledWith('missing', ''); - expect(screen.getByText(/no missing albums are wanted back/i)).toBeTruthy(); + setup([], 'upgrade'); + expect(createWantsQuery).toHaveBeenCalledWith('upgrade', ''); + expect(screen.getByText(/no album is wanted for a better copy/i)).toBeTruthy(); }); test('a want says why it is wanted', () => { diff --git a/web/src/lib/wanted/wantStatus.ts b/web/src/lib/wanted/wantStatus.ts new file mode 100644 index 00000000..dcc6d0c9 --- /dev/null +++ b/web/src/lib/wanted/wantStatus.ts @@ -0,0 +1,79 @@ +import { relativeTime } from '#lib/utils/relativeTime.js'; +import type { SearchResult, Want } from '#lib/api/types.js'; + +// How a want reads in a line, wherever it is shown: the Collection's lists +// and the Missing page's folders (M504). + +// What a Search the operator ran found, for its toast. +export function searched(w: Want, r: SearchResult): string { + let said: string; + switch (r.outcome) { + case 'grabbed': + said = `Took ${r.grabbed ?? r.best_release}.`; + break; + case 'waiting': + said = `Found ${r.considered}; nothing taken. Releases shows why.`; + break; + case 'nothing': + said = r.considered ? `Found ${r.considered}, none of them ${w.title}.` : `Nothing found for ${w.title}.`; + break; + default: + said = r.error ?? 'The search failed.'; + } + if (r.narrowed.length) said += ` ${r.narrowed.map((n) => n.label).join(', ')} didn't answer.`; + return said; +} + +// A download still running, in a line: how far it has got, or what its +// import is doing or waiting on. +export function downloading(w: Want): string | null { + const g = w.grab; + if (!g?.live) return null; + switch (g.import_state) { + case 'downloading': + return `Downloading ${Math.floor(g.progress * 100)}% · ${g.name}`; + case 'importing': + return `Downloaded; checking it is the album and filing it · ${g.name}`; + case 'waiting': + return `Downloaded; ${g.import_reason || 'waiting to be imported'}`; + case 'review': + return `Needs you: ${g.import_reason}`; + case 'failed': + return `Import failed: ${g.import_reason}`; + } + const who = g.origin === 'operator' ? 'taken by you' : 'taken by the schedule'; + return `Downloading ${g.name}, ${who} ${relativeTime(g.grabbed_at)}`; +} + +// A download waiting on a person: an import to review, or one that failed. +export const needsYou = (w: Want) => + !!w.grab?.live && (w.grab.import_state === 'review' || w.grab.import_state === 'failed'); + +// What the schedule last did about a want, in a line. +export function wantStatus(w: Want): string { + const live = downloading(w); + if (live) return live; + const s = w.search; + let said = 'Not searched yet'; + if (s) { + const when = `Searched ${relativeTime(s.searched_at)}`; + switch (s.outcome) { + case 'nothing': + said = s.considered ? `${when}: ${s.considered} found, none of them this album` : `${when}: nothing found`; + break; + case 'waiting': + said = `${when}: ${s.considered} found, nothing taken`; + break; + case 'grabbed': + said = `${when}: took ${s.best_release}`; + break; + default: + said = `${when}: ${s.error}`; + } + } + const g = w.grab; + if (g?.import_state === 'refused') said += `. Last download refused: ${g.import_reason}`; + else if (g && !g.live && ['stalled', 'failed', 'gone'].includes(g.end_reason) && g.end_detail) + said += `. Last download given up: ${g.end_detail}`; + return said; +} diff --git a/web/src/routes/admin/+layout.svelte b/web/src/routes/admin/+layout.svelte index 35664b38..befeef91 100644 --- a/web/src/routes/admin/+layout.svelte +++ b/web/src/routes/admin/+layout.svelte @@ -29,7 +29,6 @@ const items: RailItem[] = $derived([ { href: '/admin', label: 'Overview', exact: true }, { href: '/admin/requests', label: 'Requests', attention: countOf(summary, 'requests') }, - { href: '/admin/missing-files', label: 'Missing files', attention: countOf(summary, 'missing') }, { href: '/admin/library', label: 'Library' }, { href: '/admin/acquisition', label: 'Acquisition' }, { href: '/admin/metadata', label: 'Metadata sources' }, diff --git a/web/src/routes/admin/admin.test.ts b/web/src/routes/admin/admin.test.ts index 50c21466..0fddd500 100644 --- a/web/src/routes/admin/admin.test.ts +++ b/web/src/routes/admin/admin.test.ts @@ -168,7 +168,6 @@ describe('admin layout', () => { expect(links).toEqual([ ['/admin', 'Overview'], ['/admin/requests', 'Requests 2'], - ['/admin/missing-files', 'Missing files'], ['/admin/library', 'Library'], ['/admin/acquisition', 'Acquisition'], ['/admin/metadata', 'Metadata sources'], diff --git a/web/src/routes/admin/missing-files/+page.svelte b/web/src/routes/admin/missing-files/+page.svelte deleted file mode 100644 index ae253d45..00000000 --- a/web/src/routes/admin/missing-files/+page.svelte +++ /dev/null @@ -1,213 +0,0 @@ - - -{pageTitle('Admin · Missing files')} - -
-
-
-

Missing files

- {#if total > 0} - - {total} - - {/if} -
-

- Tracks whose audio file the last scan couldn't find. They keep their play - history and likes, and stop being offered anywhere until the file returns. -

-
- -

- {#if policy} - {policy.enabled - ? `Albums missing for ${policy.grace_hours} hours go back on the wanted list by themselves.` - : 'Missing albums are not wanted back automatically.'} - {/if} - - {policy ? 'Change' : 'Set what happens to missing files'} - -

- - {#if query.isPending} -

Checking what's missing…

- {:else if query.isError} -

- Couldn't load the missing files. - -

- {:else if groups.length === 0} - -
- -

Every track's file is where it should be.

-

- A track lands here when a library scan can't find its file — moved outside - Minstrel, deleted, or on a drive that didn't mount. It leaves on its own - when the file comes back. -

-
- {:else} - - - {#if hasMore || offset > 0} - - {/if} - {/if} -
diff --git a/web/src/routes/collection/collection.test.ts b/web/src/routes/collection/collection.test.ts index a0e4ff13..a5b86f6f 100644 --- a/web/src/routes/collection/collection.test.ts +++ b/web/src/routes/collection/collection.test.ts @@ -51,7 +51,6 @@ describe('/collection layout', () => { expect(links).toEqual([ ['/collection', 'Catalogue 7'], ['/collection/wanted', 'Wanted 5'], - ['/collection/missing', 'Missing files 1'], ['/collection/upgrades', 'Upgrades 6'], ['/collection/activity', 'Activity 3'], ['/collection/following', 'Following 2'], @@ -59,7 +58,8 @@ describe('/collection layout', () => { ['/collection/unmatched', 'Unmatched 3'], ['/collection/reports', 'Reports'], ['/collection/duplicates', 'Duplicates'], - ['/collection/suspect-sources', 'Suspect sources'] + ['/collection/suspect-sources', 'Suspect sources'], + ['/collection/missing', 'Missing files'] ]); expect(screen.getByRole('link', { name: /activity/i })).toHaveAttribute('aria-current', 'page'); expect(screen.getByRole('link', { name: /^catalogue/i })).not.toHaveAttribute('aria-current'); @@ -90,7 +90,7 @@ describe('/collection layout', () => { // The library's health queues moved here from Admin (M504): their counts // come from the admin summary, accented, and a line names the ones waiting. - test('reports, duplicates and suspect sources carry what waits on you, and a line links to each', () => { + test('the library health queues carry what waits on you, and a line links to each', () => { setup(counts, '/collection', { ...clear, reports: { count: 2, since: null }, @@ -102,11 +102,12 @@ describe('/collection layout', () => { expect(dup.textContent?.replace(/\s+/g, ' ').trim()).toBe('Duplicates 5'); const health = screen.getByTestId('collection-health'); expect(health.textContent?.replace(/\s+/g, ' ').trim()).toBe( - 'To look at: 2 broken tracks reported, 5 sets of duplicates to review.' + 'To look at: 2 broken tracks reported, 5 sets of duplicates to review, 9 missing files.' ); expect(Array.from(health.querySelectorAll('a')).map((a) => a.getAttribute('href'))).toEqual([ '/collection/reports', - '/collection/duplicates' + '/collection/duplicates', + '/collection/missing' ]); }); diff --git a/web/src/routes/collection/missing/+page.svelte b/web/src/routes/collection/missing/+page.svelte index a06ded29..749d8009 100644 --- a/web/src/routes/collection/missing/+page.svelte +++ b/web/src/routes/collection/missing/+page.svelte @@ -1,8 +1,308 @@ {pageTitle('Collection · Missing files')} - +
+
+
+

Missing files

+ {#if total > 0} + + {total} + + {/if} +
+

+ Tracks whose audio file the last scan couldn't find. They keep their play + history and likes, and stop being offered anywhere until the file returns. +

+
+ +

+ {#if policy} + {policy.enabled + ? `Albums missing for ${policy.grace_hours} hours go back on the wanted list by themselves.` + : 'Missing albums are not wanted back automatically.'} + {/if} + + {policy ? 'Change' : 'Set what happens to missing files'} + +

+ + {#if query.isPending} +

Checking what's missing…

+ {:else if query.isError} +

+ Couldn't load the missing files. + +

+ {:else if groups.length === 0} + +
+ +

Every track's file is where it should be.

+

+ A track lands here when a library scan can't find its file — moved outside + Minstrel, deleted, or on a drive that didn't mount. It leaves on its own + when the file comes back. +

+
+ {:else} +
    + {#each groups as group (group.directory)} +
  • + +
    +

    + {group.directory} +

    + + {trackCountLabel(group.tracks.length)} · gone {relativeTime(group.missing_since)} + +
    + + + {#if group.reacquisition} + {@const r = group.reacquisition} + {@const w = r.want} +
    +
    +
    +

    + {#if r.held_at} + Held again {relativeTime(r.held_at)}. The rows here clear on the next scan. + {:else if r.wanted_at} + Wanted back since {relativeTime(r.wanted_at)}. + {/if} +

    + {#if w && !r.held_at} +

    + {wantStatus(w)} + {#if needsYou(w)} + Decide on Activity + {/if} +

    + {/if} +
    + {#if w && !r.held_at} +
    + + + +
    + {/if} +
    + {#if w && open === w.id} + + {/if} +
    + {/if} + +
      + {#each group.tracks as t (t.track_id)} +
    • + + +
      +
      {t.title}
      +
      + {t.artist_name} · {t.album_title} +
      +
      + + + + {#if t.last_played_at} + last played {relativeTime(t.last_played_at)} + {:else} + never played + {/if} + + {durationLabel(t.duration_sec)} +
      • + {/each} +
    +
    • + {/each} +
+ + {#if hasMore || offset > 0} + + {/if} + {/if} +
diff --git a/web/src/routes/admin/missing-files/missing-files.test.ts b/web/src/routes/collection/missing/missing.test.ts similarity index 60% rename from web/src/routes/admin/missing-files/missing-files.test.ts rename to web/src/routes/collection/missing/missing.test.ts index 3ac2cf66..ff35bf80 100644 --- a/web/src/routes/admin/missing-files/missing-files.test.ts +++ b/web/src/routes/collection/missing/missing.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, within } from '@testing-library/svelte'; +import { fireEvent, render, screen, waitFor } from '@testing-library/svelte'; import { mockQuery } from '../../../test-utils/query'; -import type { AdminMissingResponse } from '#lib/api/types.js'; +import type { AdminMissingResponse, Want } from '#lib/api/types.js'; // The page reads the re-acquisition policy to say what happens to these; // the settings themselves are on Admin › Acquisition. @@ -14,9 +14,23 @@ vi.mock('#lib/api/admin.js', () => ({ unnameable_albums: 0 }) })); +// A folder's want is acted on here as on the wanted list (M504). +vi.mock('#lib/api/wanted.js', () => ({ + removeWant: vi.fn().mockResolvedValue(undefined), + searchWant: vi.fn().mockResolvedValue({ + outcome: 'nothing', considered: 0, best_release: '', best_verdict: '', narrowed: [] + }), + createCandidatesQuery: vi.fn(), + grabCandidate: vi.fn(), + blockCandidate: vi.fn(), + unblockRelease: vi.fn() +})); +vi.mock('#lib/stores/toast.svelte.js', () => ({ pushToast: vi.fn() })); -import AdminMissingFilesPage from './+page.svelte'; +import MissingPage from './+page.svelte'; import { createMissingFilesQuery } from '#lib/api/admin.js'; +import { removeWant, searchWant } from '#lib/api/wanted.js'; +import { pushToast } from '#lib/stores/toast.svelte.js'; const DAY = 24 * 3_600_000; @@ -71,10 +85,42 @@ function renderWith(data: AdminMissingResponse | undefined, extra = {}) { vi.mocked(createMissingFilesQuery).mockReturnValue( mockQuery({ data, ...extra }) as ReturnType ); - return render(AdminMissingFilesPage); + return render(MissingPage); } -describe('admin missing files', () => { +function want(extra: Partial = {}): Want { + return { + id: 'w-1', + release_group_mbid: 'rg-1', + title: 'Minutes to Midnight', + artist_mbid: 'lp', + artist_name: 'Linkin Park', + primary_type: 'Album', + secondary_types: [], + year: '2007', + reason: 'missing', + wanted_at: new Date(Date.now() - 2 * DAY).toISOString(), + held_at: null, + image_url: '', + search: { + searched_at: new Date(Date.now() - 3_600_000).toISOString(), + outcome: 'nothing', + considered: 0, + best_release: '', + best_verdict: '', + error: '' + }, + ...extra + }; +} + +function wantedBack(w: Want | null = want()): AdminMissingResponse { + const withState = structuredClone(response); + withState.groups[0].reacquisition = { wanted_at: new Date(Date.now() - 2 * DAY).toISOString(), held_at: null, want: w }; + return withState; +} + +describe('/collection/missing', () => { test('says what happens to missing files, and links to where it is set', async () => { renderWith(response); const line = screen.getByTestId('reacquisition-policy'); @@ -133,30 +179,54 @@ describe('admin missing files', () => { expect(screen.queryByTestId('reacquisition-state')).toBeNull(); }); - test('a wanted album says since when, and links the wanted list', () => { - const withState = structuredClone(response); - withState.groups[0].reacquisition = { - wanted_at: new Date(Date.now() - 2 * DAY).toISOString(), - held_at: null - }; - renderWith(withState); - const line = text('reacquisition-state'); - expect(line).toMatch(/on the wanted list since 2d ago/i); - expect( - within(screen.getByTestId('reacquisition-state')).getByRole('link', { name: /wanted list/i }).getAttribute('href') - ).toBe( - '/collection/missing' + test('an album wanted back says since when, and what its last search found', () => { + renderWith(wantedBack()); + expect(text('reacquisition-state')).toMatch(/wanted back since 2d ago/i); + expect(text('missing-want-status')).toBe('Searched 1h ago: nothing found'); + }); + + test('its download, when one is running, is the line', () => { + renderWith( + wantedBack( + want({ + grab: { + id: 'g', name: 'LP - MtM [FLAC]', grabbed_at: new Date().toISOString(), origin: 'schedule', quality: 'FLAC', + live: true, end_reason: '', progress: 0.42, import_state: 'downloading', import_reason: '', end_detail: '', + imported_at: null, imported_files: 0 + } + }) + ) ); + expect(text('missing-want-status')).toBe('Downloading 42% · LP - MtM [FLAC]'); + expect(screen.getByRole('button', { name: /search for minutes to midnight now/i })).toHaveProperty('disabled', true); + }); + + test('Search runs one now and says what it found', async () => { + renderWith(wantedBack()); + await fireEvent.click(screen.getByRole('button', { name: /search for minutes to midnight now/i })); + await waitFor(() => expect(searchWant).toHaveBeenCalledWith('w-1')); + await waitFor(() => expect(pushToast).toHaveBeenCalledWith('Nothing found for Minutes to Midnight.', 'info')); + }); + + test('Remove takes it off the wanted list', async () => { + renderWith(wantedBack()); + await fireEvent.click(screen.getByRole('button', { name: /stop wanting minutes to midnight/i })); + await waitFor(() => expect(removeWant).toHaveBeenCalledWith('w-1')); + expect(pushToast).toHaveBeenCalledWith("Minutes to Midnight is off the wanted list, and won't be fetched back."); }); test('an album held again says so', () => { const withState = structuredClone(response); withState.groups[0].reacquisition = { wanted_at: new Date(Date.now() - 5 * DAY).toISOString(), - held_at: new Date(Date.now() - DAY).toISOString() + held_at: new Date(Date.now() - DAY).toISOString(), + want: want({ held_at: new Date(Date.now() - DAY).toISOString() }) }; renderWith(withState); expect(text('reacquisition-state')).toMatch(/held again 1d ago/i); + // Nothing left to search for. + expect(screen.queryByTestId('missing-want-status')).toBeNull(); + expect(screen.queryByRole('button', { name: /search for/i })).toBeNull(); }); // Paging only appears when it can do something: a single page of results