From a8786abace32aa0eccc44b108dcdea234661479c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 10 Oct 2026 22:12:30 -0400 Subject: [PATCH] =?UTF-8?q?feat:=20Collection=20=E2=80=BA=20Unmatched=20li?= =?UTF-8?q?sts=20the=20library=20albums=20no=20MusicBrainz=20album=20match?= =?UTF-8?q?ed,=20each=20matched=20by=20hand=20from=20a=20search=20or=20a?= =?UTF-8?q?=20link,=20or=20left=20off=20(M502=20#5579)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Like Lidarr's Unmapped Files. A local band's album, which MusicBrainz will never have, is left off: never matched again and not counted. A re-tagged album is matched again on the next listing. The Collection's verdict now says when library albums need matching. Co-Authored-By: Claude Opus 5.5 --- internal/api/admin_wanted.go | 154 ++++++++++++---- internal/api/admin_wanted_test.go | 90 ++++++++- internal/api/api.go | 4 + internal/db/dbq/wanted.sql.go | 140 ++++++++++++-- .../0089_library_match_by_hand.down.sql | 8 + .../0089_library_match_by_hand.up.sql | 10 + internal/db/queries/wanted.sql | 79 ++++++-- internal/wanted/library.go | 10 +- internal/wanted/manual.go | 171 ++++++++++++++++++ internal/wanted/manual_test.go | 162 +++++++++++++++++ web/src/lib/api/queries.ts | 2 + web/src/lib/api/types.ts | 21 ++- web/src/lib/api/wanted.ts | 33 ++++ .../components/wanted/CatalogueGrid.svelte | 41 +---- .../components/wanted/CatalogueGrid.test.ts | 23 +-- .../components/wanted/CollectionRail.svelte | 7 +- .../components/wanted/UnmatchedList.svelte | 150 +++++++++++++++ .../components/wanted/UnmatchedList.test.ts | 109 +++++++++++ .../components/wanted/UnmatchedMatch.svelte | 106 +++++++++++ .../lib/components/wanted/WantList.test.ts | 4 +- web/src/routes/collection/+layout.svelte | 10 +- web/src/routes/collection/collection.test.ts | 17 +- .../routes/collection/unmatched/+page.svelte | 8 + 23 files changed, 1224 insertions(+), 135 deletions(-) create mode 100644 internal/db/migrations/0089_library_match_by_hand.down.sql create mode 100644 internal/db/migrations/0089_library_match_by_hand.up.sql create mode 100644 internal/wanted/manual.go create mode 100644 internal/wanted/manual_test.go create mode 100644 web/src/lib/components/wanted/UnmatchedList.svelte create mode 100644 web/src/lib/components/wanted/UnmatchedList.test.ts create mode 100644 web/src/lib/components/wanted/UnmatchedMatch.svelte create mode 100644 web/src/routes/collection/unmatched/+page.svelte diff --git a/internal/api/admin_wanted.go b/internal/api/admin_wanted.go index d58a20b1..3ff14ad7 100644 --- a/internal/api/admin_wanted.go +++ b/internal/api/admin_wanted.go @@ -1,6 +1,8 @@ package api import ( + "context" + "encoding/json" "errors" "net/http" "strconv" @@ -72,7 +74,9 @@ type wantGrabView struct { // wantCounts sizes each tab of the Wanted page. Artists: how many artists // the list holds albums of, its main page (#5572). type wantCounts struct { - Artists int64 `json:"artists"` + Artists int64 `json:"artists"` + // Unmatched: library albums waiting to be matched by hand (#5579). + Unmatched int64 `json:"unmatched"` Wanted int64 `json:"wanted"` Missing int64 `json:"missing"` Held int64 `json:"held"` @@ -123,33 +127,38 @@ type wantArtistsPage struct { } // libraryListingView: whether the library's albums are all on the list -// yet, and which could not be matched to MusicBrainz (#5569). Unmatched is -// filled on the first page of the held list only. +// yet, and how many could not be matched to MusicBrainz (#5569); those are +// listed in the Collection's Unmatched section (#5579). type libraryListingView struct { - ListedAt *string `json:"listed_at"` - Unlisted int32 `json:"unlisted"` - Unmatched []unmatchedAlbumView `json:"unmatched"` + ListedAt *string `json:"listed_at"` + Unlisted int32 `json:"unlisted"` } +// unmatchedAlbumView is one album in the Collection's Unmatched section +// (#5579): why no MusicBrainz album matched it, or that a person left it +// off. type unmatchedAlbumView struct { - AlbumID string `json:"album_id"` - Title string `json:"title"` - Artist string `json:"artist"` - Detail string `json:"detail"` + AlbumID string `json:"album_id"` + Title string `json:"title"` + Artist string `json:"artist"` + Detail string `json:"detail"` + Tracks int32 `json:"tracks"` + LeftOff bool `json:"left_off"` + DecidedAt string `json:"decided_at"` } -// unmatchedShown is how many unmatched albums the held list names. -const unmatchedShown = 200 +// unmatchedShown bounds the Unmatched section: a library a listing could +// not match more of than this has a bigger problem than one page shows. +const unmatchedShown = 500 const wantsPageSize = 50 // wantArtistsPageSize fills a few rows of the artist grid a page. const wantArtistsPageSize = 96 -// wantsHeader is what every Wanted page carries: each tab's count, and how -// the one-time library listing went. The albums no match was found for are -// named only when asked (the main page's first page). -func (h *handlers) wantsHeader(r *http.Request, q *dbq.Queries, unmatched bool) (wantCounts, libraryListingView, error) { +// wantsHeader is what every Collection page carries: each section's count, +// and how the one-time library listing went. +func (h *handlers) wantsHeader(r *http.Request, q *dbq.Queries) (wantCounts, libraryListingView, error) { counts, err := q.CountWants(r.Context()) if err != nil { return wantCounts{}, libraryListingView{}, err @@ -158,23 +167,12 @@ func (h *handlers) wantsHeader(r *http.Request, q *dbq.Queries, unmatched bool) if err != nil { return wantCounts{}, libraryListingView{}, err } - lib := libraryListingView{Unlisted: listing.LibraryUnlisted, Unmatched: []unmatchedAlbumView{}} + lib := libraryListingView{Unlisted: listing.LibraryUnlisted} if listing.LibraryListedAt.Valid { at := formatTimestamp(listing.LibraryListedAt) lib.ListedAt = &at } - if unmatched && listing.LibraryUnlisted > 0 { - rows, err := q.ListUnmatchedLibraryAlbums(r.Context(), unmatchedShown) - if err != nil { - return wantCounts{}, libraryListingView{}, err - } - for _, u := range rows { - lib.Unmatched = append(lib.Unmatched, unmatchedAlbumView{ - AlbumID: uuidToString(u.AlbumID), Title: u.Title, Artist: u.ArtistName, Detail: u.Detail, - }) - } - } - return wantCounts{Artists: counts.Artists, Wanted: counts.Wanted, Missing: counts.Missing, Held: counts.Held, + return wantCounts{Artists: counts.Artists, Unmatched: counts.Unmatched, Wanted: counts.Wanted, Missing: counts.Missing, Held: counts.Held, Following: counts.Following, Activity: counts.Activity, Attention: counts.Attention, Blocked: counts.Blocked}, lib, nil } @@ -207,7 +205,7 @@ func (h *handlers) handleListWantArtists(w http.ResponseWriter, r *http.Request) writeErrWithLog(w, h.logger, "admin wanted: artists", apierror.Internal(err)) return } - counts, lib, err := h.wantsHeader(r, q, offset == 0) + counts, lib, err := h.wantsHeader(r, q) if err != nil { writeErrWithLog(w, h.logger, "admin wanted: counts", apierror.Internal(err)) return @@ -244,7 +242,7 @@ func (h *handlers) handleListWants(w http.ResponseWriter, r *http.Request) { writeErrWithLog(w, h.logger, "admin wanted: list", apierror.Internal(err)) return } - counts, lib, err := h.wantsHeader(r, q, false) + counts, lib, err := h.wantsHeader(r, q) if err != nil { writeErrWithLog(w, h.logger, "admin wanted: counts", apierror.Internal(err)) return @@ -714,3 +712,97 @@ func deref(s *string) string { } return *s } + +// handleListUnmatched implements GET /api/admin/wanted/unmatched: the +// Collection's Unmatched section (#5579), the albums no MusicBrainz album +// matched and, after them, the ones a person left off. +func (h *handlers) handleListUnmatched(w http.ResponseWriter, r *http.Request) { + rows, err := dbq.New(h.pool).ListUnmatchedLibraryAlbums(r.Context(), unmatchedShown) + if err != nil { + writeErrWithLog(w, h.logger, "admin wanted: unmatched", apierror.Internal(err)) + return + } + out := struct { + Items []unmatchedAlbumView `json:"items"` + }{Items: make([]unmatchedAlbumView, 0, len(rows))} + for _, u := range rows { + out.Items = append(out.Items, unmatchedAlbumView{ + AlbumID: uuidToString(u.AlbumID), Title: u.Title, Artist: u.ArtistName, Detail: u.Detail, + Tracks: u.Tracks, LeftOff: u.Outcome == "dismissed", DecidedAt: formatTimestamp(u.DecidedAt), + }) + } + writeJSON(w, http.StatusOK, out) +} + +type manualMatchView struct { + ReleaseGroupMBID string `json:"release_group_mbid"` + Title string `json:"title"` + Artist string `json:"artist"` + Agree int `json:"agree"` + Tracks int `json:"tracks"` +} + +// handleMatchUnmatched implements POST /api/admin/wanted/unmatched/{id}/match +// {"ref": ""}: the album goes on the list as held, +// as that MusicBrainz album. +func (h *handlers) handleMatchUnmatched(w http.ResponseWriter, r *http.Request) { + id, ok := requireURLUUID(w, r, "id") + if !ok { + return + } + var body struct { + Ref string `json:"ref"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + writeErr(w, apierror.BadRequest("bad_body", "expected {\"ref\": \"…\"}")) + return + } + m, err := h.wanted.MatchAlbumTo(r.Context(), id, body.Ref) + switch { + case errors.Is(err, wanted.ErrMatchRef): + writeErr(w, apierror.BadRequest("bad_ref", "That isn't a MusicBrainz album link or id.")) + return + case errors.Is(err, wanted.ErrMatchNoGroup): + writeErr(w, &apierror.Error{Status: http.StatusUnprocessableEntity, Code: "no_such_album", + Message: "MusicBrainz has no album at that link.", Cause: err}) + return + case errors.Is(err, wanted.ErrAlbumNotUnmatched): + writeErr(w, apierror.NotFound("unmatched album")) + return + case errors.Is(err, musicbrainz.ErrUnreachable), errors.Is(err, musicbrainz.ErrBadResponse): + writeErr(w, &apierror.Error{Status: http.StatusServiceUnavailable, Code: "musicbrainz_unreachable", + Message: "MusicBrainz can't be reached", Cause: err}) + return + case err != nil: + writeErrWithLog(w, h.logger, "admin wanted: match by hand", apierror.Internal(err)) + return + } + writeJSON(w, http.StatusOK, manualMatchView{ReleaseGroupMBID: m.Group, Title: m.Title, Artist: m.Artist, + Agree: m.Agree, Tracks: m.Tracks}) +} + +// handleLeaveUnmatchedOff implements POST /api/admin/wanted/unmatched/{id}/leave-off, +// and handlePutUnmatchedBack its undoing, POST …/{id}/put-back. +func (h *handlers) handleLeaveUnmatchedOff(w http.ResponseWriter, r *http.Request) { + h.setUnmatched(w, r, h.wanted.LeaveAlbumOff) +} + +func (h *handlers) handlePutUnmatchedBack(w http.ResponseWriter, r *http.Request) { + h.setUnmatched(w, r, h.wanted.PutAlbumBack) +} + +func (h *handlers) setUnmatched(w http.ResponseWriter, r *http.Request, set func(context.Context, pgtype.UUID) error) { + id, ok := requireURLUUID(w, r, "id") + if !ok { + return + } + err := set(r.Context(), id) + switch { + case errors.Is(err, wanted.ErrAlbumNotUnmatched): + writeErr(w, apierror.NotFound("unmatched album")) + case err != nil: + writeErrWithLog(w, h.logger, "admin wanted: unmatched", apierror.Internal(err)) + default: + w.WriteHeader(http.StatusNoContent) + } +} diff --git a/internal/api/admin_wanted_test.go b/internal/api/admin_wanted_test.go index ecfc70da..b883120d 100644 --- a/internal/api/admin_wanted_test.go +++ b/internal/api/admin_wanted_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "github.com/go-chi/chi/v5" @@ -19,16 +20,25 @@ import ( ) func doWantedReq(t *testing.T, h *handlers, path string, user dbq.User) *httptest.ResponseRecorder { + t.Helper() + return doWanted(t, h, http.MethodGet, path, "", user) +} + +func doWanted(t *testing.T, h *handlers, method, path, body string, user dbq.User) *httptest.ResponseRecorder { t.Helper() r := chi.NewRouter() r.Route("/api/admin", func(admin chi.Router) { admin.Use(auth.RequireAdmin()) admin.Get("/wanted", h.handleListWants) admin.Get("/wanted/artists", h.handleListWantArtists) + admin.Get("/wanted/unmatched", h.handleListUnmatched) + admin.Post("/wanted/unmatched/{id}/match", h.handleMatchUnmatched) + admin.Post("/wanted/unmatched/{id}/leave-off", h.handleLeaveUnmatchedOff) + admin.Post("/wanted/unmatched/{id}/put-back", h.handlePutUnmatchedBack) admin.Get("/wanted/{id}/tracks", h.handleWantTracks) }) w := httptest.NewRecorder() - r.ServeHTTP(w, withUser(httptest.NewRequest(http.MethodGet, path, nil), user)) + r.ServeHTTP(w, withUser(httptest.NewRequest(method, path, strings.NewReader(body)), user)) return w } @@ -281,3 +291,81 @@ func TestWantTracks_HeldAlbumShowsTheLibrarysTracks(t *testing.T) { t.Errorf("tracks = %+v: the first is missing, the second on disk", out.Tracks) } } + +// The Unmatched section (#5579): the albums matching could not place, a +// local band's album left off and put back, and a link that names no +// MusicBrainz album refused before MusicBrainz is asked. +func TestUnmatched_ListLeaveOffPutBack(t *testing.T) { + h, _ := testHandlersWithClientFn(t) + admin := seedUser(t, h.pool, "admin", "pw", true) + ctx := context.Background() + q := dbq.New(h.pool) + artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Annadale", SortName: "Annadale"}) + if err != nil { + t.Fatal(err) + } + album, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: "Dial Tone", SortTitle: "Dial Tone", ArtistID: artist.ID}) + if err != nil { + t.Fatal(err) + } + if _, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: "Intro", AlbumID: album.ID, ArtistID: artist.ID, DurationMs: 120000, + FilePath: "/music/annadale/01.flac", FileSize: 1, FileFormat: "flac", + }); err != nil { + t.Fatal(err) + } + if err := q.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{ + AlbumID: album.ID, Outcome: "unmatched", Detail: "MusicBrainz has no album by that name", Matcher: 1}); err != nil { + t.Fatal(err) + } + id := uuidToString(album.ID) + + list := func() []unmatchedAlbumView { + t.Helper() + w := doWantedReq(t, h, "/api/admin/wanted/unmatched", admin) + if w.Code != http.StatusOK { + t.Fatalf("status = %d; body = %s", w.Code, w.Body.String()) + } + var out struct { + Items []unmatchedAlbumView `json:"items"` + } + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatal(err) + } + return out.Items + } + if got := list(); len(got) != 1 || got[0].AlbumID != id || got[0].Artist != "Annadale" || got[0].Tracks != 1 || + got[0].LeftOff || got[0].Detail == "" || got[0].DecidedAt == "" { + t.Fatalf("unmatched = %+v", got) + } + + for _, c := range []struct { + path, body string + code int + }{ + {"/api/admin/wanted/unmatched/" + id + "/match", `{"ref":"Dial Tone"}`, http.StatusBadRequest}, + {"/api/admin/wanted/unmatched/" + id + "/match", `nope`, http.StatusBadRequest}, + {"/api/admin/wanted/unmatched/not-a-uuid/leave-off", "", http.StatusBadRequest}, + {"/api/admin/wanted/unmatched/" + id + "/put-back", "", http.StatusNotFound}, + {"/api/admin/wanted/unmatched/" + id + "/leave-off", "", http.StatusNoContent}, + {"/api/admin/wanted/unmatched/" + id + "/leave-off", "", http.StatusNotFound}, + } { + if w := doWanted(t, h, http.MethodPost, c.path, c.body, admin); w.Code != c.code { + t.Errorf("POST %s %s = %d, want %d; body = %s", c.path, c.body, w.Code, c.code, w.Body.String()) + } + } + if got := list(); len(got) != 1 || !got[0].LeftOff { + t.Fatalf("left off = %+v", got) + } + if w := doWanted(t, h, http.MethodPost, "/api/admin/wanted/unmatched/"+id+"/put-back", "", admin); w.Code != http.StatusNoContent { + t.Fatalf("put back = %d; body = %s", w.Code, w.Body.String()) + } + if got := list(); len(got) != 1 || got[0].LeftOff { + t.Errorf("put back = %+v", got) + } + + user := seedUser(t, h.pool, "listener", "pw", false) + if w := doWantedReq(t, h, "/api/admin/wanted/unmatched", user); w.Code != http.StatusForbidden { + t.Errorf("a listener reads the unmatched section: %d", w.Code) + } +} diff --git a/internal/api/api.go b/internal/api/api.go index b8d7d27a..2e958a51 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -253,6 +253,10 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev // profiles that decide which of their releases count. admin.Get("/wanted", h.handleListWants) admin.Get("/wanted/artists", h.handleListWantArtists) + admin.Get("/wanted/unmatched", h.handleListUnmatched) + admin.Post("/wanted/unmatched/{id}/match", h.handleMatchUnmatched) + admin.Post("/wanted/unmatched/{id}/leave-off", h.handleLeaveUnmatchedOff) + admin.Post("/wanted/unmatched/{id}/put-back", h.handlePutUnmatchedBack) admin.Delete("/wanted/{id}", h.handleRemoveWant) // Searching and grabbing (M502 #5521). admin.Get("/acquisition/settings", h.handleGetSearchSettings) diff --git a/internal/db/dbq/wanted.sql.go b/internal/db/dbq/wanted.sql.go index 22469f12..3d5204b7 100644 --- a/internal/db/dbq/wanted.sql.go +++ b/internal/db/dbq/wanted.sql.go @@ -20,14 +20,29 @@ func (q *Queries) ClearDefaultReleaseTypeProfile(ctx context.Context) error { return err } -const countAlbumsToRematch = `-- name: CountAlbumsToRematch :one -SELECT COUNT(*) FROM library_album_matches - WHERE outcome = 'unmatched' AND matcher < $1::int +const countAlbumsToMatch = `-- name: CountAlbumsToMatch :one +SELECT COUNT(*) + FROM albums a + WHERE (a.created_at <= $1::timestamptz + OR EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id AND t.added_at <= $1::timestamptz)) + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id) + AND NOT EXISTS (SELECT 1 FROM library_album_matches m + WHERE m.album_id = a.id + AND (m.outcome IN ('listed', 'dismissed') + OR (m.matcher >= $2::int + AND NOT EXISTS (SELECT 1 FROM tracks t + WHERE t.album_id = a.id AND t.updated_at > m.decided_at)))) ` -// Refusals by an older matcher, which a finished listing goes back to. -func (q *Queries) CountAlbumsToRematch(ctx context.Context, matcher int32) (int64, error) { - row := q.db.QueryRow(ctx, countAlbumsToRematch, matcher) +type CountAlbumsToMatchParams struct { + Cutoff pgtype.Timestamptz + Matcher int32 +} + +// Whether a finished listing has albums to go back to: those +// ListAlbumsToMatch would hand it. +func (q *Queries) CountAlbumsToMatch(ctx context.Context, arg CountAlbumsToMatchParams) (int64, error) { + row := q.db.QueryRow(ctx, countAlbumsToMatch, arg.Cutoff, arg.Matcher) var count int64 err := row.Scan(&count) return count, err @@ -38,6 +53,10 @@ SELECT COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason <> 'missing')::big COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason = 'missing')::bigint AS missing, COUNT(*) FILTER (WHERE fulfilled_at IS NOT NULL)::bigint AS held, COUNT(DISTINCT artist_mbid)::bigint AS artists, + -- Library albums waiting to be matched by hand (#5579). + (SELECT COUNT(*) FROM library_album_matches m + WHERE m.outcome = 'unmatched' + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = m.album_id))::bigint AS unmatched, (SELECT COUNT(*) FROM follows)::bigint AS following, -- The Wanted view's other tabs (#5522): downloads running, those -- waiting on a person, and releases set aside. @@ -54,6 +73,7 @@ type CountWantsRow struct { Missing int64 Held int64 Artists int64 + Unmatched int64 Following int64 Activity int64 Attention int64 @@ -68,6 +88,7 @@ func (q *Queries) CountWants(ctx context.Context) (CountWantsRow, error) { &i.Missing, &i.Held, &i.Artists, + &i.Unmatched, &i.Following, &i.Activity, &i.Attention, @@ -143,7 +164,9 @@ func (q *Queries) DeleteWant(ctx context.Context, id pgtype.UUID) (int64, error) const finishLibraryListing = `-- name: FinishLibraryListing :one UPDATE acquisition_settings SET library_listed_at = COALESCE(library_listed_at, now()), - library_unlisted = (SELECT COUNT(*) FROM library_album_matches WHERE outcome = 'unmatched') + library_unlisted = (SELECT COUNT(*) FROM library_album_matches m + WHERE m.outcome = 'unmatched' + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = m.album_id)) WHERE id = true RETURNING library_listed_at, library_unlisted ` @@ -180,6 +203,38 @@ func (q *Queries) FulfillWant(ctx context.Context, arg FulfillWantParams) error return err } +const getAlbumToMatch = `-- name: GetAlbumToMatch :one +SELECT a.id, a.title, a.mbid, a.release_group_mbid, ar.name AS artist_name, ar.mbid AS artist_mbid + FROM albums a + JOIN artists ar ON ar.id = a.artist_id + JOIN library_album_matches m ON m.album_id = a.id AND m.outcome IN ('unmatched', 'dismissed') + WHERE a.id = $1 +` + +type GetAlbumToMatchRow struct { + ID pgtype.UUID + Title string + Mbid *string + ReleaseGroupMbid *string + ArtistName string + ArtistMbid *string +} + +// One album a person is matching by hand, as the matcher reads albums. +func (q *Queries) GetAlbumToMatch(ctx context.Context, id pgtype.UUID) (GetAlbumToMatchRow, error) { + row := q.db.QueryRow(ctx, getAlbumToMatch, id) + var i GetAlbumToMatchRow + err := row.Scan( + &i.ID, + &i.Title, + &i.Mbid, + &i.ReleaseGroupMbid, + &i.ArtistName, + &i.ArtistMbid, + ) + return i, err +} + const getDefaultReleaseTypeProfile = `-- name: GetDefaultReleaseTypeProfile :one SELECT id, name, primary_types, secondary_types, is_default, created_at, updated_at FROM release_type_profiles WHERE is_default ` @@ -438,10 +493,15 @@ const listAlbumsToMatch = `-- name: ListAlbumsToMatch :many SELECT a.id, a.title, a.mbid, a.release_group_mbid, ar.name AS artist_name, ar.mbid AS artist_mbid FROM albums a JOIN artists ar ON ar.id = a.artist_id - WHERE a.created_at <= $1::timestamptz + WHERE (a.created_at <= $1::timestamptz + OR EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id AND t.added_at <= $1::timestamptz)) AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id) AND NOT EXISTS (SELECT 1 FROM library_album_matches m - WHERE m.album_id = a.id AND (m.outcome = 'listed' OR m.matcher >= $2::int)) + WHERE m.album_id = a.id + AND (m.outcome IN ('listed', 'dismissed') + OR (m.matcher >= $2::int + AND NOT EXISTS (SELECT 1 FROM tracks t + WHERE t.album_id = a.id AND t.updated_at > m.decided_at)))) ORDER BY a.created_at, a.id LIMIT $3 ` @@ -461,9 +521,12 @@ type ListAlbumsToMatchRow struct { ArtistMbid *string } -// Library albums from before the cutoff that the listing has not decided, -// or that an older matcher refused: each is matched to MusicBrainz (#5569). -// An album row with no tracks is not an album in the library. +// Library albums the listing has not decided, or that an older matcher +// refused, or whose tracks changed since they were refused: each is matched +// to MusicBrainz (#5569). A library album is one from before the cutoff, or +// one whose tracks were: a re-tag files a track under a new album row but +// keeps the track (#5579). An album row with no tracks is not an album in +// the library; one a person left off stays off. func (q *Queries) ListAlbumsToMatch(ctx context.Context, arg ListAlbumsToMatchParams) ([]ListAlbumsToMatchRow, error) { rows, err := q.db.Query(ctx, listAlbumsToMatch, arg.Cutoff, arg.Matcher, arg.MaxRows) if err != nil { @@ -813,12 +876,14 @@ func (q *Queries) ListReleaseTypeProfiles(ctx context.Context) ([]ListReleaseTyp } const listUnmatchedLibraryAlbums = `-- name: ListUnmatchedLibraryAlbums :many -SELECT m.album_id, a.title, ar.name AS artist_name, m.detail +SELECT m.album_id, a.title, ar.name AS artist_name, m.detail, m.outcome, m.decided_at, + (SELECT COUNT(*) FROM tracks t WHERE t.album_id = a.id AND t.missing_since IS NULL)::int AS tracks FROM library_album_matches m JOIN albums a ON a.id = m.album_id JOIN artists ar ON ar.id = a.artist_id - WHERE m.outcome = 'unmatched' - ORDER BY ar.name, a.title + WHERE m.outcome IN ('unmatched', 'dismissed') + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = m.album_id) + ORDER BY m.outcome DESC, lower(ar.name), lower(a.title) LIMIT $1 ` @@ -827,9 +892,14 @@ type ListUnmatchedLibraryAlbumsRow struct { Title string ArtistName string Detail string + Outcome string + DecidedAt pgtype.Timestamptz + Tracks int32 } -// The albums the listing could not match, for the Held tab. +// The Collection's Unmatched section (#5579): the albums the listing could +// not match, and below them the ones a person left off. An album whose +// tracks have all moved to another row is gone, not unmatched. func (q *Queries) ListUnmatchedLibraryAlbums(ctx context.Context, maxRows int32) ([]ListUnmatchedLibraryAlbumsRow, error) { rows, err := q.db.Query(ctx, listUnmatchedLibraryAlbums, maxRows) if err != nil { @@ -844,6 +914,9 @@ func (q *Queries) ListUnmatchedLibraryAlbums(ctx context.Context, maxRows int32) &i.Title, &i.ArtistName, &i.Detail, + &i.Outcome, + &i.DecidedAt, + &i.Tracks, ); err != nil { return nil, err } @@ -1179,6 +1252,21 @@ func (q *Queries) RecordListedAlbums(ctx context.Context, cutoff pgtype.Timestam return err } +const recountLibraryUnlisted = `-- name: RecountLibraryUnlisted :exec +UPDATE acquisition_settings + SET library_unlisted = (SELECT COUNT(*) FROM library_album_matches m + WHERE m.outcome = 'unmatched' + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = m.album_id)) + WHERE id = true +` + +// After a person matches or leaves off an album, the count the catalogue +// shows. +func (q *Queries) RecountLibraryUnlisted(ctx context.Context) error { + _, err := q.db.Exec(ctx, recountLibraryUnlisted) + return err +} + const repointWant = `-- name: RepointWant :exec UPDATE wants SET release_group_mbid = $2, updated_at = now() WHERE id = $1 ` @@ -1194,6 +1282,26 @@ func (q *Queries) RepointWant(ctx context.Context, arg RepointWantParams) error return err } +const setAlbumMatchOutcome = `-- name: SetAlbumMatchOutcome :execrows +UPDATE library_album_matches SET outcome = $1, decided_at = now() + WHERE album_id = $2 AND outcome = $3 +` + +type SetAlbumMatchOutcomeParams struct { + Outcome string + AlbumID pgtype.UUID + Was string +} + +// Leave an unmatched album off the list, or put a left-off one back. +func (q *Queries) SetAlbumMatchOutcome(ctx context.Context, arg SetAlbumMatchOutcomeParams) (int64, error) { + result, err := q.db.Exec(ctx, setAlbumMatchOutcome, arg.Outcome, arg.AlbumID, arg.Was) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const setAlbumReleaseGroup = `-- name: SetAlbumReleaseGroup :exec UPDATE albums SET release_group_mbid = $1, updated_at = now() WHERE id = $2 AND (release_group_mbid IS NULL OR release_group_mbid = '') diff --git a/internal/db/migrations/0089_library_match_by_hand.down.sql b/internal/db/migrations/0089_library_match_by_hand.down.sql new file mode 100644 index 00000000..678f4eab --- /dev/null +++ b/internal/db/migrations/0089_library_match_by_hand.down.sql @@ -0,0 +1,8 @@ +UPDATE library_album_matches SET outcome = 'unmatched' WHERE outcome = 'dismissed'; +UPDATE library_album_matches SET method = 'search' WHERE method = 'manual'; +ALTER TABLE library_album_matches DROP CONSTRAINT library_album_matches_outcome_check; +ALTER TABLE library_album_matches ADD CONSTRAINT library_album_matches_outcome_check + CHECK (outcome IN ('listed', 'unmatched')); +ALTER TABLE library_album_matches DROP CONSTRAINT library_album_matches_method_check; +ALTER TABLE library_album_matches ADD CONSTRAINT library_album_matches_method_check + CHECK (method IN ('', 'tags', 'release', 'discography', 'search')); diff --git a/internal/db/migrations/0089_library_match_by_hand.up.sql b/internal/db/migrations/0089_library_match_by_hand.up.sql new file mode 100644 index 00000000..c093c221 --- /dev/null +++ b/internal/db/migrations/0089_library_match_by_hand.up.sql @@ -0,0 +1,10 @@ +-- The Collection's Unmatched section (#5579): a person matches an album the +-- listing could not, by a MusicBrainz link or a search (method 'manual'), +-- or leaves it off the list for good (outcome 'dismissed': a local band's +-- album MusicBrainz will never have, a mix that is no album). +ALTER TABLE library_album_matches DROP CONSTRAINT library_album_matches_outcome_check; +ALTER TABLE library_album_matches ADD CONSTRAINT library_album_matches_outcome_check + CHECK (outcome IN ('listed', 'unmatched', 'dismissed')); +ALTER TABLE library_album_matches DROP CONSTRAINT library_album_matches_method_check; +ALTER TABLE library_album_matches ADD CONSTRAINT library_album_matches_method_check + CHECK (method IN ('', 'tags', 'release', 'discography', 'search', 'manual')); diff --git a/internal/db/queries/wanted.sql b/internal/db/queries/wanted.sql index 7aa34649..cd74b2c2 100644 --- a/internal/db/queries/wanted.sql +++ b/internal/db/queries/wanted.sql @@ -271,6 +271,10 @@ SELECT COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason <> 'missing')::big COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason = 'missing')::bigint AS missing, COUNT(*) FILTER (WHERE fulfilled_at IS NOT NULL)::bigint AS held, COUNT(DISTINCT artist_mbid)::bigint AS artists, + -- Library albums waiting to be matched by hand (#5579). + (SELECT COUNT(*) FROM library_album_matches m + WHERE m.outcome = 'unmatched' + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = m.album_id))::bigint AS unmatched, (SELECT COUNT(*) FROM follows)::bigint AS following, -- The Wanted view's other tabs (#5522): downloads running, those -- waiting on a person, and releases set aside. @@ -330,23 +334,41 @@ SELECT a.id, 'listed', 'tags', a.release_group_mbid ON CONFLICT (album_id) DO NOTHING; -- name: ListAlbumsToMatch :many --- Library albums from before the cutoff that the listing has not decided, --- or that an older matcher refused: each is matched to MusicBrainz (#5569). --- An album row with no tracks is not an album in the library. +-- Library albums the listing has not decided, or that an older matcher +-- refused, or whose tracks changed since they were refused: each is matched +-- to MusicBrainz (#5569). A library album is one from before the cutoff, or +-- one whose tracks were: a re-tag files a track under a new album row but +-- keeps the track (#5579). An album row with no tracks is not an album in +-- the library; one a person left off stays off. SELECT a.id, a.title, a.mbid, a.release_group_mbid, ar.name AS artist_name, ar.mbid AS artist_mbid FROM albums a JOIN artists ar ON ar.id = a.artist_id - WHERE a.created_at <= sqlc.arg(cutoff)::timestamptz + WHERE (a.created_at <= sqlc.arg(cutoff)::timestamptz + OR EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id AND t.added_at <= sqlc.arg(cutoff)::timestamptz)) AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id) AND NOT EXISTS (SELECT 1 FROM library_album_matches m - WHERE m.album_id = a.id AND (m.outcome = 'listed' OR m.matcher >= sqlc.arg(matcher)::int)) + WHERE m.album_id = a.id + AND (m.outcome IN ('listed', 'dismissed') + OR (m.matcher >= sqlc.arg(matcher)::int + AND NOT EXISTS (SELECT 1 FROM tracks t + WHERE t.album_id = a.id AND t.updated_at > m.decided_at)))) ORDER BY a.created_at, a.id LIMIT sqlc.arg(max_rows); --- name: CountAlbumsToRematch :one --- Refusals by an older matcher, which a finished listing goes back to. -SELECT COUNT(*) FROM library_album_matches - WHERE outcome = 'unmatched' AND matcher < sqlc.arg(matcher)::int; +-- name: CountAlbumsToMatch :one +-- Whether a finished listing has albums to go back to: those +-- ListAlbumsToMatch would hand it. +SELECT COUNT(*) + FROM albums a + WHERE (a.created_at <= sqlc.arg(cutoff)::timestamptz + OR EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id AND t.added_at <= sqlc.arg(cutoff)::timestamptz)) + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id) + AND NOT EXISTS (SELECT 1 FROM library_album_matches m + WHERE m.album_id = a.id + AND (m.outcome IN ('listed', 'dismissed') + OR (m.matcher >= sqlc.arg(matcher)::int + AND NOT EXISTS (SELECT 1 FROM tracks t + WHERE t.album_id = a.id AND t.updated_at > m.decided_at)))); -- name: ListAlbumTracksToMatch :many -- What an album's tracks say about themselves, to check a match against. @@ -384,16 +406,45 @@ ON CONFLICT (album_id) DO UPDATE -- count the albums no match was found for. The first finish is kept. UPDATE acquisition_settings SET library_listed_at = COALESCE(library_listed_at, now()), - library_unlisted = (SELECT COUNT(*) FROM library_album_matches WHERE outcome = 'unmatched') + library_unlisted = (SELECT COUNT(*) FROM library_album_matches m + WHERE m.outcome = 'unmatched' + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = m.album_id)) WHERE id = true RETURNING library_listed_at, library_unlisted; -- name: ListUnmatchedLibraryAlbums :many --- The albums the listing could not match, for the Held tab. -SELECT m.album_id, a.title, ar.name AS artist_name, m.detail +-- The Collection's Unmatched section (#5579): the albums the listing could +-- not match, and below them the ones a person left off. An album whose +-- tracks have all moved to another row is gone, not unmatched. +SELECT m.album_id, a.title, ar.name AS artist_name, m.detail, m.outcome, m.decided_at, + (SELECT COUNT(*) FROM tracks t WHERE t.album_id = a.id AND t.missing_since IS NULL)::int AS tracks FROM library_album_matches m JOIN albums a ON a.id = m.album_id JOIN artists ar ON ar.id = a.artist_id - WHERE m.outcome = 'unmatched' - ORDER BY ar.name, a.title + WHERE m.outcome IN ('unmatched', 'dismissed') + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = m.album_id) + ORDER BY m.outcome DESC, lower(ar.name), lower(a.title) LIMIT sqlc.arg(max_rows); + +-- name: RecountLibraryUnlisted :exec +-- After a person matches or leaves off an album, the count the catalogue +-- shows. +UPDATE acquisition_settings + SET library_unlisted = (SELECT COUNT(*) FROM library_album_matches m + WHERE m.outcome = 'unmatched' + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = m.album_id)) + WHERE id = true; + +-- name: GetAlbumToMatch :one +-- One album a person is matching by hand, as the matcher reads albums. +SELECT a.id, a.title, a.mbid, a.release_group_mbid, ar.name AS artist_name, ar.mbid AS artist_mbid + FROM albums a + JOIN artists ar ON ar.id = a.artist_id + JOIN library_album_matches m ON m.album_id = a.id AND m.outcome IN ('unmatched', 'dismissed') + WHERE a.id = $1; + +-- name: SetAlbumMatchOutcome :execrows +-- Leave an unmatched album off the list, or put a left-off one back. +UPDATE library_album_matches SET outcome = sqlc.arg(outcome), decided_at = now() + WHERE album_id = sqlc.arg(album_id) AND outcome = sqlc.arg(was); + diff --git a/internal/wanted/library.go b/internal/wanted/library.go index 051fbbf1..9add993a 100644 --- a/internal/wanted/library.go +++ b/internal/wanted/library.go @@ -50,7 +50,10 @@ const libraryMatcher = 4 // // Each album decided is recorded in library_album_matches, so an album taken // off the list stays off. Once done the listing runs again only for albums -// an older matcher refused (libraryMatcher). +// an older matcher refused (libraryMatcher), refused albums whose tracks +// have changed since, and library tracks a re-tag filed under a new album +// (#5579). A person matches the rest by hand, or leaves them off, in the +// Collection's Unmatched section (MatchAlbumTo, LeaveAlbumOff). func (s *Service) listLibrary(ctx context.Context, res *PassResult) error { q := dbq.New(s.pool) state, err := q.GetLibraryListing(ctx) @@ -58,9 +61,10 @@ func (s *Service) listLibrary(ctx context.Context, res *PassResult) error { return fmt.Errorf("wanted: read the library listing: %w", err) } if state.LibraryListedAt.Valid { - stale, err := q.CountAlbumsToRematch(ctx, libraryMatcher) + stale, err := q.CountAlbumsToMatch(ctx, dbq.CountAlbumsToMatchParams{ + Cutoff: state.LibraryListingStartedAt, Matcher: libraryMatcher}) if err != nil { - return fmt.Errorf("wanted: count refusals to retry: %w", err) + return fmt.Errorf("wanted: count albums to match again: %w", err) } if stale == 0 { return nil diff --git a/internal/wanted/manual.go b/internal/wanted/manual.go new file mode 100644 index 00000000..3282cb64 --- /dev/null +++ b/internal/wanted/manual.go @@ -0,0 +1,171 @@ +package wanted + +import ( + "context" + "errors" + "fmt" + "regexp" + "strings" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/musicbrainz" + "git.fabledsword.com/bvandeusen/minstrel/internal/tags" +) + +// Matching a library album by hand (#5579): the Collection's Unmatched +// section, like Lidarr's "Unmapped Files". A person names the MusicBrainz +// album an album is, by a link or a search, and it goes on the list as held; +// or leaves it off for good, as a local band's album MusicBrainz will never +// have. + +const matchManual = "manual" + +// Manual-match errors, which the API answers as 400, 404 or 409. +var ( + ErrMatchRef = errors.New("wanted: not a MusicBrainz album link or id") + ErrMatchNoGroup = errors.New("wanted: MusicBrainz has no such album") + ErrAlbumNotUnmatched = errors.New("wanted: no such unmatched album") +) + +// ManualMatch is what matching an album by hand did: the group it is now, +// and how many of its tracks agree with the group's closest edition, so the +// person can see whether the link was the right one. +type ManualMatch struct { + Group string + Title string + Artist string + Agree int + Tracks int +} + +var mbidPattern = regexp.MustCompile(`(?i)(?:(release-group|release)/)?([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})`) + +// ParseMusicBrainzRef reads a MusicBrainz link (a release group's or a +// release's page) or a bare id. Kind is "release-group", "release", or "" +// for a bare id, which is tried as a group first. +func ParseMusicBrainzRef(ref string) (kind, id string, ok bool) { + m := mbidPattern.FindStringSubmatch(strings.TrimSpace(ref)) + if m == nil { + return "", "", false + } + return strings.ToLower(m[1]), strings.ToLower(m[2]), true +} + +// MatchAlbumTo puts an unmatched (or left-off) library album on the list as +// held, as the MusicBrainz album ref names. The person's word is taken: the +// tracks are compared only to say how well they agree. +func (s *Service) MatchAlbumTo(ctx context.Context, albumID pgtype.UUID, ref string) (ManualMatch, error) { + kind, id, ok := ParseMusicBrainzRef(ref) + if !ok { + return ManualMatch{}, ErrMatchRef + } + q := dbq.New(s.pool) + a, err := q.GetAlbumToMatch(ctx, albumID) + if errors.Is(err, pgx.ErrNoRows) { + return ManualMatch{}, ErrAlbumNotUnmatched + } + if err != nil { + return ManualMatch{}, fmt.Errorf("wanted: read album: %w", err) + } + group, err := s.groupOf(ctx, kind, id) + if err != nil { + return ManualMatch{}, err + } + g, err := s.cat.MusicBrainz().ReleaseGroup(ctx, group) + if errors.Is(err, musicbrainz.ErrNotFound) { + return ManualMatch{}, ErrMatchNoGroup + } + if err != nil { + return ManualMatch{}, err + } + artist := creditedArtist(g.ArtistMBIDs, deref(a.ArtistMbid)) + if artist == "" { + return ManualMatch{}, fmt.Errorf("%w: it credits no artist", ErrMatchNoGroup) + } + rows, err := q.ListAlbumTracksToMatch(ctx, a.ID) + if err != nil { + return ManualMatch{}, fmt.Errorf("wanted: read %s's tracks: %w", a.Title, err) + } + tracks := tracksToMatch(rows) + n, _, _, err := s.agreementWith(ctx, group, tracks, func(string) bool { return true }) + if err != nil { + return ManualMatch{}, err + } + err = pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error { + tq := dbq.New(tx) + if err := tq.SetAlbumReleaseGroup(ctx, dbq.SetAlbumReleaseGroupParams{ID: a.ID, ReleaseGroupMbid: &group}); err != nil { + return err + } + if _, err := tq.InsertLibraryWant(ctx, dbq.InsertLibraryWantParams{ + ReleaseGroupMbid: group, ArtistMbid: artist, ArtistName: g.ArtistCredit, Title: g.Title, + PrimaryType: g.PrimaryType, FirstReleaseDate: g.FirstReleaseDate, AlbumID: a.ID, + }); err != nil { + return err + } + if err := tq.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{ + AlbumID: a.ID, Outcome: "listed", Method: matchManual, ReleaseGroupMbid: group, Matcher: libraryMatcher, + }); err != nil { + return err + } + return tq.RecountLibraryUnlisted(ctx) + }) + if err != nil { + return ManualMatch{}, fmt.Errorf("wanted: match %s by hand: %w", a.Title, err) + } + s.logger.Info("wanted: a library album matched by hand", "album", a.Title, "group", group, "agree", max(n, 0), "tracks", len(tracks)) + return ManualMatch{Group: group, Title: g.Title, Artist: g.ArtistCredit, Agree: max(n, 0), Tracks: len(tracks)}, nil +} + +// groupOf names the release group a MusicBrainz id is, or belongs to. +func (s *Service) groupOf(ctx context.Context, kind, id string) (string, error) { + if kind != "release" { + _, err := s.cat.MusicBrainz().ReleaseGroup(ctx, id) + if err == nil { + return id, nil + } + if !errors.Is(err, musicbrainz.ErrNotFound) { + return "", err + } + if kind == "release-group" { + return "", ErrMatchNoGroup + } + } + group, err := s.releaseGroup(ctx, id) + switch { + case errors.Is(err, tags.ErrNotFound): + return "", ErrMatchNoGroup + case err != nil: + return "", err + case group == "": + return "", ErrMatchNoGroup + } + return group, nil +} + +// LeaveAlbumOff leaves an unmatched library album off the list for good: +// matching never tries it again, and it is no longer counted as unmatched. +func (s *Service) LeaveAlbumOff(ctx context.Context, albumID pgtype.UUID) error { + return s.setMatchOutcome(ctx, albumID, "unmatched", "dismissed") +} + +// PutAlbumBack undoes LeaveAlbumOff: the album is unmatched again. +func (s *Service) PutAlbumBack(ctx context.Context, albumID pgtype.UUID) error { + return s.setMatchOutcome(ctx, albumID, "dismissed", "unmatched") +} + +func (s *Service) setMatchOutcome(ctx context.Context, albumID pgtype.UUID, was, outcome string) error { + return pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error { + tq := dbq.New(tx) + n, err := tq.SetAlbumMatchOutcome(ctx, dbq.SetAlbumMatchOutcomeParams{AlbumID: albumID, Was: was, Outcome: outcome}) + if err != nil { + return err + } + if n == 0 { + return ErrAlbumNotUnmatched + } + return tq.RecountLibraryUnlisted(ctx) + }) +} diff --git a/internal/wanted/manual_test.go b/internal/wanted/manual_test.go new file mode 100644 index 00000000..21ea8d5e --- /dev/null +++ b/internal/wanted/manual_test.go @@ -0,0 +1,162 @@ +package wanted + +import ( + "context" + "errors" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/tags" +) + +func TestParseMusicBrainzRef(t *testing.T) { + const id = "11111111-0000-4000-8000-000000000003" + for ref, want := range map[string]string{ + "https://musicbrainz.org/release-group/" + id: "release-group " + id, + "https://musicbrainz.org/release/" + id + "?tab=x": "release " + id, + " " + id + " ": " " + id, + "https://MusicBrainz.org/Release-Group/" + "11111111-0000-4000-8000-00000000000A": "release-group 11111111-0000-4000-8000-00000000000a", + } { + kind, got, ok := ParseMusicBrainzRef(ref) + if !ok || kind+" "+got != want { + t.Errorf("ParseMusicBrainzRef(%q) = %q %q %v, want %q", ref, kind, got, ok, want) + } + } + for _, bad := range []string{"", "Dial Tone", "https://musicbrainz.org/artist/nope"} { + if _, _, ok := ParseMusicBrainzRef(bad); ok { + t.Errorf("ParseMusicBrainzRef(%q) read an id", bad) + } + } +} + +// A person names the MusicBrainz album an unmatched album is, and it goes +// on the list as held; or leaves it off, and matching never tries it again +// (#5579). +func TestMatchAlbumTo_ByLinkOrLeaveItOff_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + s := newService(t, pool, newFakeMB(t)) + s.releaseGroup = func(context.Context, string) (string, error) { return "", tags.ErrNotFound } + split := seedAlbum(t, q, blur, "Blur + Friends", "", []string{"The Ballad", "St. Charles Square"}) + local := seedAlbum(t, q, blur, "Dial Tone", "", []string{"Intro"}) + for _, a := range []pgtype.UUID{split.ID, local.ID} { + if err := q.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{ + AlbumID: a, Outcome: "unmatched", Detail: "no match", Matcher: libraryMatcher}); err != nil { + t.Fatal(err) + } + } + if _, err := pool.Exec(ctx, `UPDATE acquisition_settings + SET library_listing_started_at = now(), library_listed_at = now(), library_unlisted = 2`); err != nil { + t.Fatal(err) + } + + for ref, want := range map[string]error{ + "not a link": ErrMatchRef, + "https://musicbrainz.org/release-group/99999999-0000-4000-8000-000000000000": ErrMatchNoGroup, + "99999999-0000-4000-8000-000000000000": ErrMatchNoGroup, + } { + if _, err := s.MatchAlbumTo(ctx, split.ID, ref); !errors.Is(err, want) { + t.Errorf("MatchAlbumTo(%q) = %v, want %v", ref, err, want) + } + } + + m, err := s.MatchAlbumTo(ctx, split.ID, "https://musicbrainz.org/release-group/"+theBallad) + if err != nil { + t.Fatal(err) + } + if m.Group != theBallad || m.Title != "The Ballad of Darren" || m.Agree != 2 || m.Tracks != 2 { + t.Errorf("match = %+v", m) + } + w, err := q.GetWantByReleaseGroup(ctx, theBallad) + if err != nil || w.Reason != ReasonLibrary || !w.FulfilledAt.Valid || w.AlbumID != split.ID || w.ArtistMbid != blur { + t.Fatalf("want = %+v, %v: held, about the album", w, err) + } + if outcome, method, group, _ := matchOf(t, pool, split.ID); outcome != "listed" || method != matchManual || group != theBallad { + t.Errorf("decided %s by %s as %s", outcome, method, group) + } + if state, _ := q.GetLibraryListing(ctx); state.LibraryUnlisted != 1 { + t.Errorf("unlisted = %d, want the local album alone", state.LibraryUnlisted) + } + + // The local band's album: left off, it is not counted, and a newer + // matcher does not try it again. + if err := s.LeaveAlbumOff(ctx, local.ID); err != nil { + t.Fatal(err) + } + if outcome, _, _, _ := matchOf(t, pool, local.ID); outcome != "dismissed" { + t.Errorf("left off = %s", outcome) + } + if state, _ := q.GetLibraryListing(ctx); state.LibraryUnlisted != 0 { + t.Errorf("unlisted = %d after leaving it off", state.LibraryUnlisted) + } + if _, err := pool.Exec(ctx, `UPDATE library_album_matches SET matcher = 0 WHERE album_id = $1`, local.ID); err != nil { + t.Fatal(err) + } + if err := s.listLibrary(ctx, &PassResult{}); err != nil { + t.Fatal(err) + } + if outcome, _, _, _ := matchOf(t, pool, local.ID); outcome != "dismissed" { + t.Errorf("an album left off was matched again: %s", outcome) + } + rows, err := q.ListUnmatchedLibraryAlbums(ctx, 10) + if err != nil || len(rows) != 1 || rows[0].Outcome != "dismissed" || rows[0].Tracks != 1 { + t.Errorf("unmatched section = %+v, %v: the left-off album alone", rows, err) + } + + // Put back, it is unmatched again; putting it back twice is not. + if err := s.PutAlbumBack(ctx, local.ID); err != nil { + t.Fatal(err) + } + if outcome, _, _, _ := matchOf(t, pool, local.ID); outcome != "unmatched" { + t.Errorf("put back = %s", outcome) + } + if err := s.PutAlbumBack(ctx, local.ID); !errors.Is(err, ErrAlbumNotUnmatched) { + t.Errorf("put back twice: %v", err) + } +} + +// A re-tag files a library track under a new album row. That album is the +// library's, not one dropped in by hand, so the listing matches it; the +// old row, its tracks gone, is no longer unmatched (#5579). +func TestListLibrary_MatchesARetaggedAlbum_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + s := newService(t, pool, newFakeMB(t)) + untagged := seedAlbum(t, q, blur, "Unknown Album", "", []string{"The Ballad", "St. Charles Square"}) + if err := q.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{ + AlbumID: untagged.ID, Outcome: "unmatched", Detail: "no match", Matcher: libraryMatcher}); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, ` + UPDATE albums SET created_at = now() - interval '2 hours' WHERE id = $1; + UPDATE tracks SET added_at = now() - interval '2 hours' WHERE album_id = $1; + UPDATE acquisition_settings SET library_listing_started_at = now() - interval '1 hour', + library_listed_at = now() - interval '1 hour', library_unlisted = 1`, untagged.ID); err != nil { + t.Fatal(err) + } + // The re-tag: the same tracks, under the album their new tags name. + retagged, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: "The Ballad of Darren", SortTitle: "The Ballad of Darren", ArtistID: untagged.ArtistID}) + if err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, `UPDATE tracks SET album_id = $1, updated_at = now() WHERE album_id = $2`, retagged.ID, untagged.ID); err != nil { + t.Fatal(err) + } + + if err := s.listLibrary(ctx, &PassResult{}); err != nil { + t.Fatal(err) + } + if outcome, method, group, _ := matchOf(t, pool, retagged.ID); outcome != "listed" || method != matchDiscography || group != theBallad { + t.Errorf("the re-tagged album was decided %s by %s as %s", outcome, method, group) + } + if rows, _ := q.ListUnmatchedLibraryAlbums(ctx, 10); len(rows) != 0 { + t.Errorf("unmatched = %+v: the old row has no tracks left", rows) + } + if state, _ := q.GetLibraryListing(ctx); state.LibraryUnlisted != 0 { + t.Errorf("unlisted = %d", state.LibraryUnlisted) + } +} diff --git a/web/src/lib/api/queries.ts b/web/src/lib/api/queries.ts index 4dbde645..0859f3c5 100644 --- a/web/src/lib/api/queries.ts +++ b/web/src/lib/api/queries.ts @@ -45,6 +45,8 @@ export const qk = { ['adminWants', { state, q, artist }] as const, wantArtists: (q: string = '') => ['adminWants', 'artists', { q }] as const, wantTracks: (id: string) => ['wantTracks', id] as const, + // The Collection's Unmatched section (#5579). + unmatched: () => ['adminWants', 'unmatched'] as const, adminFollows: () => ['adminFollows'] as const, adminFollow: (artistMbid: string) => ['adminFollow', artistMbid] as const, releaseProfiles: () => ['releaseProfiles'] as const, diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 3684ccb3..1b29cb5a 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -733,6 +733,9 @@ export type WantCounts = { activity: number; attention: number; blocked: number; + // Library albums no MusicBrainz album matched, waiting to be matched by + // hand or left off (#5579). + unmatched: number; }; // What an import's check counted (#5522). @@ -818,9 +821,23 @@ export type ReleaseBlocksPage = { items: ReleaseBlock[]; limit: number; offset: // The one-time listing of the library's albums as held (#5567): when it // finished (null while it runs), and how many albums nothing names to // MusicBrainz, so they could not be listed. -export type UnmatchedAlbum = { album_id: string; title: string; artist: string; detail: string }; +// One album in the Collection's Unmatched section (#5579): why no +// MusicBrainz album matched it, or that a person left it off. +export type UnmatchedAlbum = { + album_id: string; + title: string; + artist: string; + detail: string; + tracks: number; + left_off: boolean; + decided_at: string; +}; -export type LibraryListing = { listed_at: string | null; unlisted: number; unmatched: UnmatchedAlbum[] }; +// What matching an album by hand did: the MusicBrainz album it is now, and +// how many of its tracks agree with that album's closest edition. +export type ManualMatch = { release_group_mbid: string; title: string; artist: string; agree: number; tracks: number }; + +export type LibraryListing = { listed_at: string | null; unlisted: number }; // artist heads the list when it is narrowed to one artist's page. export type WantsPage = Page & { counts: WantCounts; library: LibraryListing; artist?: WantArtist }; diff --git a/web/src/lib/api/wanted.ts b/web/src/lib/api/wanted.ts index d5a60f9f..e2b59706 100644 --- a/web/src/lib/api/wanted.ts +++ b/web/src/lib/api/wanted.ts @@ -11,8 +11,10 @@ import type { ReleaseProfileInput, ReleaseProfilesResponse, GrabResult, + ManualMatch, SearchResult, SearchSettings, + UnmatchedAlbum, WantArtistsPage, WantState, WantTracks, @@ -92,6 +94,37 @@ export function createWantTracksQuery(id: string) { }); } +// The Collection's Unmatched section (#5579): library albums no MusicBrainz +// album matched, and after them the ones a person left off. +export async function listUnmatched(): Promise { + const out = await api.get<{ items: UnmatchedAlbum[] }>('/api/admin/wanted/unmatched'); + return out.items; +} + +export function createUnmatchedQuery() { + return createQuery({ + queryKey: qk.unmatched(), + queryFn: listUnmatched, + staleTime: 30_000 + }); +} + +// Puts an unmatched album on the list as held, as the MusicBrainz album ref +// names: a release group's or a release's link, or a bare id. +export async function matchUnmatched(albumId: string, ref: string): Promise { + return api.post(`/api/admin/wanted/unmatched/${albumId}/match`, { ref }); +} + +// Leaves an album off for good (a local band's, which MusicBrainz will +// never have), or puts one left off back. +export async function leaveUnmatchedOff(albumId: string): Promise { + await api.post(`/api/admin/wanted/unmatched/${albumId}/leave-off`, {}); +} + +export async function putUnmatchedBack(albumId: string): Promise { + await api.post(`/api/admin/wanted/unmatched/${albumId}/put-back`, {}); +} + export async function removeWant(id: string): Promise { await api.del(`/api/admin/wanted/${id}`); } diff --git a/web/src/lib/components/wanted/CatalogueGrid.svelte b/web/src/lib/components/wanted/CatalogueGrid.svelte index f37478c2..5409a4fd 100644 --- a/web/src/lib/components/wanted/CatalogueGrid.svelte +++ b/web/src/lib/components/wanted/CatalogueGrid.svelte @@ -1,5 +1,5 @@ {#if library && !library.listed_at} @@ -29,41 +27,6 @@ Your library's albums are being added to the catalogue. Albums whose tags don't name them are matched on MusicBrainz first, by their artist, title and tracks, about one a second.

-{:else if library && library.unlisted > 0} -
-

- {library.unlisted === 1 ? '1 album' : `${library.unlisted} albums`} in your library - {library.unlisted === 1 ? "isn't" : "aren't"} in the catalogue: no MusicBrainz album matched - {library.unlisted === 1 ? 'its' : 'their'} artist, title and tracks. Until - {library.unlisted === 1 ? "it's" : "they're"} matched, {library.unlisted === 1 ? "it isn't" : "they aren't"} - fetched back if {library.unlisted === 1 ? 'its' : 'their'} files go missing. -

- {#if library.unmatched.length > 0} - - {#if showUnmatched} -
    - {#each library.unmatched as u (u.album_id)} -
  • - {u.title} - · {u.artist} -
    {u.detail}
    -
    • - {/each} -
- {#if library.unmatched.length < library.unlisted} -

The first {library.unmatched.length} of {library.unlisted}.

- {/if} - {/if} - {/if} -
{/if} {#if !artists.isPending && (items.length > 0 || q)} diff --git a/web/src/lib/components/wanted/CatalogueGrid.test.ts b/web/src/lib/components/wanted/CatalogueGrid.test.ts index 3b4bc138..cceb1cdc 100644 --- a/web/src/lib/components/wanted/CatalogueGrid.test.ts +++ b/web/src/lib/components/wanted/CatalogueGrid.test.ts @@ -32,14 +32,14 @@ function setup( artist('blur', 'Blur', { following: true, downloading: 1 }), artist('beatles', 'The Beatles', { sort_name: 'Beatles, The', albums: 13, held: 13, wanted: 0 }) ], - library: LibraryListing = { listed_at: ago(DAY), unlisted: 0, unmatched: [] } + library: LibraryListing = { listed_at: ago(DAY), unlisted: 0 } ) { const page: WantArtistsPage = { items, total: items.length, limit: 96, offset: 0, - counts: { artists: items.length, wanted: 1, missing: 0, held: 16, following: 1, activity: 1, attention: 0, blocked: 0 }, + counts: { artists: items.length, wanted: 1, missing: 0, held: 16, following: 1, activity: 1, attention: 0, blocked: 0, unmatched: 0 }, library }; vi.mocked(createWantArtistsQuery).mockReturnValue( @@ -80,24 +80,7 @@ describe('CatalogueGrid', () => { }); test('says while the library is being listed', () => { - setup([], { listed_at: null, unlisted: 0, unmatched: [] }); + setup([], { listed_at: null, unlisted: 0 }); expect(screen.getByTestId('library-listing').textContent).toMatch(/being added to the catalogue/i); }); - - test('names the albums no MusicBrainz album matched, on request', async () => { - setup(undefined, { - listed_at: ago(DAY), - unlisted: 12, - unmatched: [ - { album_id: 'al-9', title: 'Demo Tape', artist: 'Blur', detail: 'MusicBrainz has no album called "Demo Tape" by Blur.' } - ] - }); - expect(screen.getByTestId('library-listing').textContent).toMatch(/12 albums in your library\s+aren't in the catalogue/i); - expect(screen.queryByTestId('unmatched-albums')).toBeNull(); - await fireEvent.click(screen.getByRole('button', { name: /show them/i })); - const list = screen.getByTestId('unmatched-albums'); - expect(list.textContent).toMatch(/Demo Tape/); - expect(screen.getByRole('link', { name: 'Demo Tape' }).getAttribute('href')).toBe('/albums/al-9'); - expect(screen.getByText(/the first 1 of 12/i)).toBeTruthy(); - }); }); diff --git a/web/src/lib/components/wanted/CollectionRail.svelte b/web/src/lib/components/wanted/CollectionRail.svelte index a2d2e52b..6f58c691 100644 --- a/web/src/lib/components/wanted/CollectionRail.svelte +++ b/web/src/lib/components/wanted/CollectionRail.svelte @@ -5,7 +5,9 @@ // The Collection's sections, down the left like Lidarr's (#5572): the // catalogue of artists first, then the sections that show state. On a // narrow screen the rail runs across the top. Activity carries a second, - // accented count when downloads are waiting on a person. + // accented count when downloads are waiting on a person. Unmatched, like + // Lidarr's Unmapped Files, holds the library albums no MusicBrainz album + // matched (#5579). let { counts }: { counts?: WantCounts } = $props(); @@ -17,7 +19,8 @@ { href: '/collection/missing', label: 'Missing files', count: counts?.missing }, { href: '/collection/activity', label: 'Activity', count: counts?.activity, attention: counts?.attention }, { href: '/collection/following', label: 'Following', count: counts?.following }, - { href: '/collection/blocked', label: 'Blocked', count: counts?.blocked } + { href: '/collection/blocked', label: 'Blocked', count: counts?.blocked }, + { href: '/collection/unmatched', label: 'Unmatched', count: counts?.unmatched } ]); // An artist's page belongs to the catalogue. diff --git a/web/src/lib/components/wanted/UnmatchedList.svelte b/web/src/lib/components/wanted/UnmatchedList.svelte new file mode 100644 index 00000000..85d6ce19 --- /dev/null +++ b/web/src/lib/components/wanted/UnmatchedList.svelte @@ -0,0 +1,150 @@ + + +{#if unmatched.isPending} +

Reading the unmatched albums…

+{:else if unmatched.isError} +

Couldn't load the unmatched albums.

+{:else} + {#if toMatch.length === 0} +
+ +

Every album in your library is matched.

+

+ An album no MusicBrainz album matches by its artist, title and tracks lands here, to be + matched by hand or left off. +

+
+ {:else} +

+ No MusicBrainz album matched {toMatch.length === 1 ? 'this album' : `these ${toMatch.length} albums`} + by artist, title and tracks, so {toMatch.length === 1 ? "it isn't" : "they aren't"} fetched back if + {toMatch.length === 1 ? 'its' : 'their'} files go missing. Match each to its MusicBrainz album, or + leave it off if MusicBrainz doesn't have it. +

+
    + {#each toMatch as u (u.album_id)} +
  • +
    +
    + {u.title} +
    {u.artist} · {tracks(u.tracks)}
    +

    {u.detail}

    +
    +
    + + +
    +
    + {#if open === u.album_id} + onMatch(u, ref)} /> + {/if} +
    • + {/each} +
+ {/if} + + {#if leftOff.length > 0} +
+

Left off

+

+ Kept in the library, never matched again, and not fetched back if their files go missing. +

+
    + {#each leftOff as u (u.album_id)} +
  • +
    + {u.title} +
    + {u.artist} · {tracks(u.tracks)} · left off {relativeTime(u.decided_at)} +
    +
    + +
    • + {/each} +
+
+ {/if} +{/if} diff --git a/web/src/lib/components/wanted/UnmatchedList.test.ts b/web/src/lib/components/wanted/UnmatchedList.test.ts new file mode 100644 index 00000000..2245246c --- /dev/null +++ b/web/src/lib/components/wanted/UnmatchedList.test.ts @@ -0,0 +1,109 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { fireEvent, render, screen, waitFor } from '@testing-library/svelte'; +import type { CatalogueSearchResult, UnmatchedAlbum } from '#lib/api/types.js'; + +vi.mock('#lib/api/wanted.js', async () => { + const { mockQuery } = await import('../../../test-utils/query'); + return { + createUnmatchedQuery: vi.fn(() => mockQuery({ data: [] })), + matchUnmatched: vi.fn().mockResolvedValue({ + release_group_mbid: 'rg-1', + title: 'Songs for the Deaf', + artist: 'Queens of the Stone Age', + agree: 9, + tracks: 10 + }), + leaveUnmatchedOff: vi.fn().mockResolvedValue(undefined), + putUnmatchedBack: vi.fn().mockResolvedValue(undefined) + }; +}); +vi.mock('#lib/api/catalogue.js', () => ({ createCatalogueSearchQuery: vi.fn() })); +vi.mock('#lib/stores/toast.svelte.js', () => ({ pushToast: vi.fn() })); + +import UnmatchedList from './UnmatchedList.svelte'; +import { mockQuery } from '../../../test-utils/query'; +import { createUnmatchedQuery, leaveUnmatchedOff, matchUnmatched, putUnmatchedBack } from '#lib/api/wanted.js'; +import { createCatalogueSearchQuery } from '#lib/api/catalogue.js'; +import { pushToast } from '#lib/stores/toast.svelte.js'; + +const DAY = 24 * 3_600_000; + +function album(id: string, title: string, artist: string, extra: Partial = {}): UnmatchedAlbum { + return { + album_id: id, + title, + artist, + detail: `MusicBrainz has no album called "${title}" by ${artist}.`, + tracks: 10, + left_off: false, + decided_at: new Date(Date.now() - 2 * DAY).toISOString(), + ...extra + }; +} + +const result: CatalogueSearchResult = { + mbid: 'rg-1', + name: 'Songs for the Deaf', + secondary_text: 'Queens of the Stone Age · 2002 · Album', + image_url: '', + artist_mbid: 'qotsa', + album_mbid: '', + in_library: false, + requested: false +}; + +afterEach(() => vi.clearAllMocks()); + +function setup(items: UnmatchedAlbum[], results: CatalogueSearchResult[] = [result]) { + vi.mocked(createUnmatchedQuery).mockReturnValue( + mockQuery({ data: items }) as unknown as ReturnType + ); + vi.mocked(createCatalogueSearchQuery).mockReturnValue( + mockQuery({ data: results }) as unknown as ReturnType + ); + return render(UnmatchedList); +} + +describe('UnmatchedList', () => { + test('each unmatched album says why, with its tracks, and links to the album', () => { + setup([album('al-1', 'Dial Tone', 'Annadale', { tracks: 1 })]); + const row = screen.getByTestId('unmatched-row'); + expect(row.textContent).toMatch(/Annadale · 1 track/); + expect(row.textContent).toMatch(/no album called "Dial Tone"/); + expect(screen.getByRole('link', { name: 'Dial Tone' }).getAttribute('href')).toBe('/albums/al-1'); + }); + + test('Match searches MusicBrainz for the album by its own title and artist, and matches a result', async () => { + setup([album('al-2', 'Songs for the Deaf', 'Queens of the Stone Age')]); + await fireEvent.click(screen.getByRole('button', { name: /match…/i })); + expect(createCatalogueSearchQuery).toHaveBeenLastCalledWith('Songs for the Deaf Queens of the Stone Age', 'album'); + await fireEvent.click(screen.getByRole('button', { name: /match to songs for the deaf/i })); + await waitFor(() => expect(matchUnmatched).toHaveBeenCalledWith('al-2', 'rg-1')); + await waitFor(() => expect(pushToast).toHaveBeenCalledWith(expect.stringMatching(/9 of 10 tracks agree/))); + }); + + test('a pasted MusicBrainz link matches the album', async () => { + setup([album('al-3', '+++', 'DROELOE')]); + await fireEvent.click(screen.getByRole('button', { name: /match…/i })); + const link = 'https://musicbrainz.org/release/11111111-0000-4000-8000-000000000003'; + await fireEvent.input(screen.getByRole('textbox', { name: /musicbrainz album link/i }), { target: { value: link } }); + await fireEvent.click(screen.getByRole('button', { name: /^match$/i })); + await waitFor(() => expect(matchUnmatched).toHaveBeenCalledWith('al-3', link)); + }); + + test("a local band's album is left off, and one left off can be put back", async () => { + setup([album('al-1', 'Dial Tone', 'Annadale'), album('al-4', 'Demo', 'Annadale', { left_off: true })]); + await fireEvent.click(screen.getByRole('button', { name: /leave it off/i })); + await waitFor(() => expect(leaveUnmatchedOff).toHaveBeenCalledWith('al-1')); + const leftOff = screen.getByTestId('left-off-row'); + expect(leftOff.textContent).toMatch(/left off 2d ago/); + await fireEvent.click(screen.getByRole('button', { name: /put back/i })); + await waitFor(() => expect(putUnmatchedBack).toHaveBeenCalledWith('al-4')); + }); + + test('with every album matched it says so, and still lists those left off', () => { + setup([album('al-4', 'Demo', 'Annadale', { left_off: true })]); + expect(screen.getByText(/every album in your library is matched/i)).toBeTruthy(); + expect(screen.getByTestId('left-off')).toBeTruthy(); + }); +}); diff --git a/web/src/lib/components/wanted/UnmatchedMatch.svelte b/web/src/lib/components/wanted/UnmatchedMatch.svelte new file mode 100644 index 00000000..d0ac3942 --- /dev/null +++ b/web/src/lib/components/wanted/UnmatchedMatch.svelte @@ -0,0 +1,106 @@ + + +
+
{ + e.preventDefault(); + asked = text.trim(); + }} + > + + +
+ + {#if search.isPending && asked} +

Asking MusicBrainz…

+ {:else if search.isError} +

Couldn't reach MusicBrainz. A pasted link works the same.

+ {:else if asked && results.length === 0} +

MusicBrainz has no album like that.

+ {:else if results.length > 0} +
    + {#each results as r (r.mbid)} +
  • +
    +
    {r.name}
    +
    {r.secondary_text}
    +
    + MusicBrainz + +
    • + {/each} +
+ {/if} + +
{ + e.preventDefault(); + if (link.trim()) onMatch(link.trim()); + }} + > + + +
+
diff --git a/web/src/lib/components/wanted/WantList.test.ts b/web/src/lib/components/wanted/WantList.test.ts index 2ad923ea..cdf212ce 100644 --- a/web/src/lib/components/wanted/WantList.test.ts +++ b/web/src/lib/components/wanted/WantList.test.ts @@ -54,7 +54,7 @@ function want(id: string, extra: Partial = {}): Want { function page( items: Want[], - library: LibraryListing = { listed_at: ago(DAY), unlisted: 0, unmatched: [] }, + library: LibraryListing = { listed_at: ago(DAY), unlisted: 0 }, artist?: WantsPage['artist'] ): WantsPage { return { @@ -63,7 +63,7 @@ function page( total: items.length, limit: 50, offset: 0, - counts: { artists: 2, wanted: 2, missing: 1, held: 4, following: 1, activity: 1, attention: 0, blocked: 0 }, + counts: { artists: 2, wanted: 2, missing: 1, held: 4, following: 1, activity: 1, attention: 0, blocked: 0, unmatched: 0 }, library }; } diff --git a/web/src/routes/collection/+layout.svelte b/web/src/routes/collection/+layout.svelte index 3da3864e..136345e1 100644 --- a/web/src/routes/collection/+layout.svelte +++ b/web/src/routes/collection/+layout.svelte @@ -21,8 +21,14 @@ } const parts = [counts.wanted === 1 ? '1 album wanted' : `${counts.wanted} albums wanted`]; if (counts.activity) parts.push(`${counts.activity} downloading`); + // Library albums no MusicBrainz album matched wait on a person too (#5579). + if (counts.unmatched) { + const n = counts.unmatched === 1 ? '1 album in your library needs' : `${counts.unmatched} albums in your library need`; + return parts.join(', ') + `. ${n} matching.`; + } return parts.join(', ') + '. Nothing needs you.'; }); + const needsYou = $derived(!!counts?.attention || !!counts?.unmatched);
@@ -36,10 +42,12 @@ Integrations.

{#if verdict} -

+

{verdict} {#if counts?.attention} See Activity + {:else if counts?.unmatched} + See Unmatched {/if}

{/if} diff --git a/web/src/routes/collection/collection.test.ts b/web/src/routes/collection/collection.test.ts index ce1aab53..5fa70f2c 100644 --- a/web/src/routes/collection/collection.test.ts +++ b/web/src/routes/collection/collection.test.ts @@ -12,7 +12,7 @@ vi.mock('#lib/api/wanted.js', () => ({ createWantArtistsQuery: vi.fn() })); import WantedLayout from './+layout.svelte'; import { createWantArtistsQuery } from '#lib/api/wanted.js'; -const counts: WantCounts = { artists: 7, wanted: 5, missing: 1, held: 9, following: 2, activity: 3, attention: 0, blocked: 4 }; +const counts: WantCounts = { artists: 7, wanted: 5, missing: 1, held: 9, following: 2, activity: 3, attention: 0, blocked: 4, unmatched: 3 }; const children = createRawSnippet(() => ({ render: () => `

body

` })); afterEach(() => vi.clearAllMocks()); @@ -25,7 +25,7 @@ function setup(c: WantCounts = counts, path = '/collection') { limit: 50, offset: 0, counts: c, - library: { listed_at: null, unlisted: 0, unmatched: [] } + library: { listed_at: null, unlisted: 0 } }; vi.mocked(createWantArtistsQuery).mockReturnValue( mockInfiniteQuery({ pages: [page] }) as unknown as ReturnType @@ -45,7 +45,8 @@ describe('/collection layout', () => { ['/collection/missing', 'Missing files 1'], ['/collection/activity', 'Activity 3'], ['/collection/following', 'Following 2'], - ['/collection/blocked', 'Blocked 4'] + ['/collection/blocked', 'Blocked 4'], + ['/collection/unmatched', 'Unmatched 3'] ]); expect(screen.getByRole('link', { name: /activity/i })).toHaveAttribute('aria-current', 'page'); expect(screen.getByRole('link', { name: /^catalogue/i })).not.toHaveAttribute('aria-current'); @@ -53,7 +54,7 @@ describe('/collection layout', () => { }); test('when nothing needs a person the verdict says so', () => { - setup(); + setup({ ...counts, unmatched: 0 }); expect(screen.getByTestId('collection-verdict').textContent).toMatch(/5 albums wanted, 3 downloading\. Nothing needs you\./); expect(screen.queryByTestId('tab-attention')).toBeNull(); }); @@ -66,6 +67,14 @@ describe('/collection layout', () => { expect(screen.getByTestId('tab-attention').textContent).toBe('2'); }); + test('library albums waiting to be matched need a person, and link to Unmatched', () => { + setup(); + expect(screen.getByTestId('collection-verdict').textContent).toMatch( + /5 albums wanted, 3 downloading\. 3 albums in your library need matching\./ + ); + expect(screen.getByRole('link', { name: /see unmatched/i }).getAttribute('href')).toBe('/collection/unmatched'); + }); + test("an artist's page sits under Catalogue", () => { setup(counts, '/collection/artist/blur'); expect(screen.getByRole('link', { name: /^catalogue/i })).toHaveAttribute('aria-current', 'page'); diff --git a/web/src/routes/collection/unmatched/+page.svelte b/web/src/routes/collection/unmatched/+page.svelte new file mode 100644 index 00000000..44fa469c --- /dev/null +++ b/web/src/routes/collection/unmatched/+page.svelte @@ -0,0 +1,8 @@ + + +{pageTitle('Collection · Unmatched')} + +