fix(library): a fully-missing album leaves the year axis too — #2702
test-go / test (push) Successful in 54s
test-go / integration (push) Successful in 5m59s

Filed as a product decision, but the code had already made it: the genre
queries filter tracks.missing_since inside their EXISTS, so an album
whose every file had gone was ALREADY absent from genre while still
listed under its year — where opening it found nothing playable. The two
browse axes disagreed, and whichever answer won, one of them had to
change.

Hiding is the answer. Browsing is how you go looking for something to
play, and the rule for that case is to take it out of view; the admin
missing-files surface is where absence gets reported, with far more
detail than a silent gap in a grid. It also means changing the axis that
was inconsistent rather than the one that was already right.

All three year queries move together — index, list and count. That is
the invariant #367 needed care for at the genre level: if the index
groups differently from the filter, a year leads to an empty page, and
if the count disagrees with the list then "Load more" promises rows that
never arrive.

The predicate is "has at least one playable track", which also excludes
an album carrying no tracks at all. Same answer for the same reason —
nothing to play, nothing to browse to — and it is what genre has always
done, since an album with no tracks contributes no genres either.

That last part changed two existing tests, which had been seeding
trackless albums as a convenience. Their intent (undated albums never
appear in a range) is untouched; they now seed a track each, which is
what a real album looks like anyway. Two new tests pin the actual
behaviour: a fully-missing album leaves the axis while a half-missing
one stays, and the count agrees with the filtered list.
This commit is contained in:
2026-08-17 13:38:08 -04:00
parent b96285d6d9
commit 955a61194e
3 changed files with 174 additions and 16 deletions
+110 -6
View File
@@ -1,11 +1,16 @@
package api
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// parseYearFilter is pure, so this runs in the fast lane rather than waiting
@@ -218,11 +223,21 @@ func TestListLibraryAlbums_GenreWithSlashSurvives(t *testing.T) {
func TestListLibraryAlbums_YearRangeFilter(t *testing.T) {
h, pool := testHandlers(t)
artist := seedArtist(t, pool, "Chronology")
seedAlbum(t, pool, artist.ID, "Old Record", 1972)
seedAlbum(t, pool, artist.ID, "Middle Record", 1995)
seedAlbum(t, pool, artist.ID, "New Record", 2020)
// Each album needs a playable track: since #2702 the year axis lists only
// albums with something to play, matching what the genre axis already did.
for _, a := range []struct {
title string
year int
}{
{"Old Record", 1972},
{"Middle Record", 1995},
{"New Record", 2020},
// An undated album must not appear in ANY year range.
seedAlbum(t, pool, artist.ID, "Undated Record", 0)
{"Undated Record", 0},
} {
album := seedAlbum(t, pool, artist.ID, a.title, a.year)
seedTrack(t, pool, album.ID, artist.ID, a.title+" T1", 1, 120_000)
}
titles := func(query string) map[string]bool {
t.Helper()
@@ -281,8 +296,10 @@ func TestListLibraryAlbums_RejectsGenreAndYearTogether(t *testing.T) {
func TestListAlbumYears_ExcludesUndatedAlbums(t *testing.T) {
h, pool := testHandlers(t)
artist := seedArtist(t, pool, "Years Only")
seedAlbum(t, pool, artist.ID, "Dated One", 1984)
seedAlbum(t, pool, artist.ID, "No Date", 0)
dated := seedAlbum(t, pool, artist.ID, "Dated One", 1984)
seedTrack(t, pool, dated.ID, artist.ID, "Dated T1", 1, 120_000)
undated := seedAlbum(t, pool, artist.ID, "No Date", 0)
seedTrack(t, pool, undated.ID, artist.ID, "Undated T1", 1, 120_000)
req := httptest.NewRequest(http.MethodGet, "/api/library/years", nil)
w := httptest.NewRecorder()
@@ -323,3 +340,90 @@ func keysOf(m map[string]bool) []string {
}
return out
}
// #2702: an album whose every file has gone leaves the year axis, matching
// what the genre axis already did. Before this the two disagreed — the same
// album was absent from genre and still listed under its year, where opening
// it found nothing playable.
func TestListAlbumYears_ExcludesFullyMissingAlbums(t *testing.T) {
h, pool := testHandlers(t)
q := dbq.New(pool)
artist := seedArtist(t, pool, "Gone Records")
// Every file missing — should vanish from the axis entirely.
dead := seedAlbum(t, pool, artist.ID, "All Gone", 1991)
deadTrack := seedTrack(t, pool, dead.ID, artist.ID, "Gone A", 1, 120_000)
// One of two missing — the album still has something to play, so it stays.
partial := seedAlbum(t, pool, artist.ID, "Half Gone", 1992)
partialGone := seedTrack(t, pool, partial.ID, artist.ID, "Half A", 1, 120_000)
seedTrack(t, pool, partial.ID, artist.ID, "Half B", 2, 120_000)
if _, err := q.MarkTracksMissing(
context.Background(), []pgtype.UUID{deadTrack.ID, partialGone.ID},
); err != nil {
t.Fatalf("mark missing: %v", err)
}
req := httptest.NewRequest(http.MethodGet, "/api/library/years", nil)
w := httptest.NewRecorder()
h.handleListAlbumYears(w, req)
var years []yearCount
if err := json.NewDecoder(w.Body).Decode(&years); err != nil {
t.Fatalf("decode years: %v", err)
}
for _, y := range years {
if y.Year == 1991 {
t.Error("1991 still on the axis — its only album has no playable files")
}
}
found1992 := false
for _, y := range years {
if y.Year == 1992 {
found1992 = true
}
}
if !found1992 {
t.Error("1992 missing — its album still has a playable track")
}
}
// The index, the list and the count must agree. #367 needed care for exactly
// this reason at the genre level: if they diverge, a year leads to an empty
// page or "Load more" promises rows that never arrive.
func TestListLibraryAlbums_YearFilterAndCountAgreeOnMissing(t *testing.T) {
h, pool := testHandlers(t)
q := dbq.New(pool)
artist := seedArtist(t, pool, "Agreement")
dead := seedAlbum(t, pool, artist.ID, "Vanished", 2003)
deadTrack := seedTrack(t, pool, dead.ID, artist.ID, "Vanished A", 1, 120_000)
alive := seedAlbum(t, pool, artist.ID, "Present", 2003)
seedTrack(t, pool, alive.ID, artist.ID, "Present A", 1, 120_000)
if _, err := q.MarkTracksMissing(
context.Background(), []pgtype.UUID{deadTrack.ID},
); err != nil {
t.Fatalf("mark missing: %v", err)
}
req := httptest.NewRequest(http.MethodGet, "/api/library/albums?year_from=2003&year_to=2003&limit=200", nil)
w := httptest.NewRecorder()
h.handleListLibraryAlbums(w, req)
var page Page[AlbumRef]
if err := json.NewDecoder(w.Body).Decode(&page); err != nil {
t.Fatalf("decode: %v", err)
}
for _, a := range page.Items {
if a.Title == "Vanished" {
t.Error("a fully-missing album is still listed under its year")
}
}
if len(page.Items) != 1 {
t.Fatalf("want 1 album listed, got %d", len(page.Items))
}
// The count drives paging; a stale one is how "Load more" starts lying.
if page.Total != 1 {
t.Errorf("total = %d, want 1 — the count must match the filtered list", page.Total)
}
}
+31 -4
View File
@@ -38,6 +38,11 @@ SELECT COUNT(*) FROM albums
WHERE release_date IS NOT NULL
AND EXTRACT(YEAR FROM release_date)::int
BETWEEN $1::int AND $2::int
AND EXISTS (
SELECT 1 FROM tracks
WHERE tracks.album_id = albums.id
AND tracks.missing_since IS NULL
)
`
type CountAlbumsByYearRangeParams struct {
@@ -56,6 +61,11 @@ const listAlbumYearsWithCount = `-- name: ListAlbumYearsWithCount :many
SELECT EXTRACT(YEAR FROM release_date)::int AS year, COUNT(*)::bigint AS album_count
FROM albums
WHERE release_date IS NOT NULL
AND EXISTS (
SELECT 1 FROM tracks
WHERE tracks.album_id = albums.id
AND tracks.missing_since IS NULL
)
GROUP BY year
ORDER BY year DESC
`
@@ -161,6 +171,11 @@ JOIN artists ON artists.id = albums.artist_id
WHERE albums.release_date IS NOT NULL
AND EXTRACT(YEAR FROM albums.release_date)::int
BETWEEN $1::int AND $2::int
AND EXISTS (
SELECT 1 FROM tracks
WHERE tracks.album_id = albums.id
AND tracks.missing_since IS NULL
)
ORDER BY albums.sort_title, albums.id
LIMIT $4 OFFSET $3
`
@@ -304,10 +319,22 @@ type ListGenresWithCountRow struct {
// Browsing is a way of finding something to play, so a track that cannot play
// should not shape it.
//
// Year queries below join albums only and are deliberately left alone: an album
// is still a real release even if some of its tracks are gone. An album whose
// EVERY track is missing will linger on the year axis; that's a narrower case,
// tracked with the rest of the cleanup work.
// The year axis filters too (#2702). It used to join albums only, on the
// reasoning that an album is a real release even when some of its tracks are
// gone — true, but it left the two browse axes disagreeing: the genre queries
// above filter tracks.missing_since, so an album whose every file had vanished
// was already absent from genre while still listed under its year, where
// opening it found nothing playable.
//
// Hiding is the answer rather than showing-and-marking because browsing is how
// you go looking for something to play, and the operator's rule for that case
// is to take it out of view; the admin missing-files surface is where absence
// is reported. It also means changing the axis that was inconsistent rather
// than the one that was already right.
//
// The predicate is "has at least one playable track", so it also excludes an
// album with no tracks at all. That is the same answer for the same reason:
// nothing to play, nothing to browse to.
// Genre browse index (#367).
//
// Genres live inline on tracks.genre as a delimited string, so this splits on
+32 -5
View File
@@ -5,10 +5,22 @@
-- Browsing is a way of finding something to play, so a track that cannot play
-- should not shape it.
--
-- Year queries below join albums only and are deliberately left alone: an album
-- is still a real release even if some of its tracks are gone. An album whose
-- EVERY track is missing will linger on the year axis; that's a narrower case,
-- tracked with the rest of the cleanup work.
-- The year axis filters too (#2702). It used to join albums only, on the
-- reasoning that an album is a real release even when some of its tracks are
-- gone — true, but it left the two browse axes disagreeing: the genre queries
-- above filter tracks.missing_since, so an album whose every file had vanished
-- was already absent from genre while still listed under its year, where
-- opening it found nothing playable.
--
-- Hiding is the answer rather than showing-and-marking because browsing is how
-- you go looking for something to play, and the operator's rule for that case
-- is to take it out of view; the admin missing-files surface is where absence
-- is reported. It also means changing the axis that was inconsistent rather
-- than the one that was already right.
--
-- The predicate is "has at least one playable track", so it also excludes an
-- album with no tracks at all. That is the same answer for the same reason:
-- nothing to play, nothing to browse to.
-- name: ListGenresWithCount :many
-- Genre browse index (#367).
@@ -82,6 +94,11 @@ WHERE EXISTS (
SELECT EXTRACT(YEAR FROM release_date)::int AS year, COUNT(*)::bigint AS album_count
FROM albums
WHERE release_date IS NOT NULL
AND EXISTS (
SELECT 1 FROM tracks
WHERE tracks.album_id = albums.id
AND tracks.missing_since IS NULL
)
GROUP BY year
ORDER BY year DESC;
@@ -93,6 +110,11 @@ JOIN artists ON artists.id = albums.artist_id
WHERE albums.release_date IS NOT NULL
AND EXTRACT(YEAR FROM albums.release_date)::int
BETWEEN sqlc.arg(year_from)::int AND sqlc.arg(year_to)::int
AND EXISTS (
SELECT 1 FROM tracks
WHERE tracks.album_id = albums.id
AND tracks.missing_since IS NULL
)
ORDER BY albums.sort_title, albums.id
LIMIT sqlc.arg(lim) OFFSET sqlc.arg(off);
@@ -100,7 +122,12 @@ LIMIT sqlc.arg(lim) OFFSET sqlc.arg(off);
SELECT COUNT(*) FROM albums
WHERE release_date IS NOT NULL
AND EXTRACT(YEAR FROM release_date)::int
BETWEEN sqlc.arg(year_from)::int AND sqlc.arg(year_to)::int;
BETWEEN sqlc.arg(year_from)::int AND sqlc.arg(year_to)::int
AND EXISTS (
SELECT 1 FROM tracks
WHERE tracks.album_id = albums.id
AND tracks.missing_since IS NULL
);
-- name: ListGenresForAlbum :many
-- Distinct genres carried by an album's tracks, for the album detail page's