feat(explore): give the album page a primary action that tells the truth
H-13: no Play, no Shuffle, no Add to queue on the album header. The reason it is not just three buttons is that explore-album-details is a catalog page — there is no library-side album detail page at all — so the album shown may be wholly the user's, partly theirs, or not theirs. A Play button that plays 7 of a 40-track release under a label saying 'Play' is the page lying about what is owned, so the button says which: 'Play' when all of it is owned, 'Play 7 of 12' when some is, and no play button at all when none is. albumLibraryStatus() stays as it was — four claims of decreasing confidence OR'd into one tick, the weakest firing when a single recording matches. That is a fine answer to 'is any of this mine' and a useless basis for a button, so ownership() counts the displayed tracklist instead. GetFilePathsByRecordingMBIDs is the catalog-side sibling of GetFilePathsByAlbums: one query, paths only, grouped so the caller keeps the tracklist's order. It is keyed on recording MBID because that is how the backend decides a track is inLibrary, and because MBTrack.LocalID is declared and never written by anything. The local album id is preferred where there is one — a library-only album has no MBIDs at all, and keying on them alone queued nothing. The ticks also get the legend H-13 asks for. They were never unlabelled — the indicator has carried a title and aria-label all along — but a sighted user got a column of green circles and no key.
This commit is contained in:
@@ -318,3 +318,28 @@ JOIN audio_files af ON af.recording_id = r.id
|
||||
WHERE rgr.release_group_id IN (sqlc.slice('release_group_ids'))
|
||||
AND af.library_id = ?
|
||||
ORDER BY rgr.disc_number, rgr.track_number;
|
||||
|
||||
-- Same shape again, keyed on recording MBID, for the catalog side.
|
||||
-- An Explore album page knows which of its tracks the user owns only
|
||||
-- as a set of recording MBIDs -- that is exactly how the backend
|
||||
-- decides `inLibrary` (markReleasesInLibrary -> CheckMBIDs) -- and
|
||||
-- MBTrack.LocalID is declared but never written by anything, so there
|
||||
-- is no id to ask by. Grouped by MBID because a recording can have
|
||||
-- more than one file (the duplicate fixtures are precisely that) and
|
||||
-- because the caller owns the order: the tracklist's, not the
|
||||
-- database's.
|
||||
|
||||
-- name: GetFilePathsByRecordingMBIDs :many
|
||||
SELECT r.mbid AS recording_mbid, af.file_path
|
||||
FROM recordings r
|
||||
JOIN audio_files af ON af.recording_id = r.id
|
||||
WHERE r.mbid IN (sqlc.slice('mbids'))
|
||||
ORDER BY af.file_path;
|
||||
|
||||
-- name: GetFilePathsByRecordingMBIDsByLibrary :many
|
||||
SELECT r.mbid AS recording_mbid, af.file_path
|
||||
FROM recordings r
|
||||
JOIN audio_files af ON af.recording_id = r.id
|
||||
WHERE r.mbid IN (sqlc.slice('mbids'))
|
||||
AND af.library_id = ?
|
||||
ORDER BY af.file_path;
|
||||
|
||||
@@ -907,6 +907,115 @@ func (q *Queries) GetAudioFilesNeedingMetadata(ctx context.Context) ([]AudioFile
|
||||
return items, nil
|
||||
}
|
||||
|
||||
const getFilePathsByRecordingMBIDs = `-- name: GetFilePathsByRecordingMBIDs :many
|
||||
|
||||
SELECT r.mbid AS recording_mbid, af.file_path
|
||||
FROM recordings r
|
||||
JOIN audio_files af ON af.recording_id = r.id
|
||||
WHERE r.mbid IN (/*SLICE:mbids*/?)
|
||||
ORDER BY af.file_path
|
||||
`
|
||||
|
||||
type GetFilePathsByRecordingMBIDsRow struct {
|
||||
RecordingMbid sql.NullString
|
||||
FilePath string
|
||||
}
|
||||
|
||||
// Same shape again, keyed on recording MBID, for the catalog side.
|
||||
// An Explore album page knows which of its tracks the user owns only
|
||||
// as a set of recording MBIDs -- that is exactly how the backend
|
||||
// decides `inLibrary` (markReleasesInLibrary -> CheckMBIDs) -- and
|
||||
// MBTrack.LocalID is declared but never written by anything, so there
|
||||
// is no id to ask by. Grouped by MBID because a recording can have
|
||||
// more than one file (the duplicate fixtures are precisely that) and
|
||||
// because the caller owns the order: the tracklist's, not the
|
||||
// database's.
|
||||
func (q *Queries) GetFilePathsByRecordingMBIDs(ctx context.Context, mbids []sql.NullString) ([]GetFilePathsByRecordingMBIDsRow, error) {
|
||||
query := getFilePathsByRecordingMBIDs
|
||||
var queryParams []interface{}
|
||||
if len(mbids) > 0 {
|
||||
for _, v := range mbids {
|
||||
queryParams = append(queryParams, v)
|
||||
}
|
||||
query = strings.Replace(query, "/*SLICE:mbids*/?", strings.Repeat(",?", len(mbids))[1:], 1)
|
||||
} else {
|
||||
query = strings.Replace(query, "/*SLICE:mbids*/?", "NULL", 1)
|
||||
}
|
||||
rows, err := q.db.QueryContext(ctx, query, queryParams...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
var items []GetFilePathsByRecordingMBIDsRow
|
||||
for rows.Next() {
|
||||
var i GetFilePathsByRecordingMBIDsRow
|
||||
if err := rows.Scan(&i.RecordingMbid, &i.FilePath); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
items = append(items, i)
|
||||
}
|
||||
if err := rows.Close(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return items, nil
|
||||
}
|
||||
|
||||
const getFilePathsByRecordingMBIDsByLibrary = `-- name: GetFilePathsByRecordingMBIDsByLibrary :many
|
||||
SELECT r.mbid AS recording_mbid, af.file_path
|
||||
FROM recordings r
|
||||
JOIN audio_files af ON af.recording_id = r.id
|
||||
WHERE r.mbid IN (/*SLICE:mbids*/?)
|
||||
AND af.library_id = ?
|
||||
ORDER BY af.file_path
|
||||
`
|
||||
|
||||
type GetFilePathsByRecordingMBIDsByLibraryParams struct {
|
||||
Mbids []sql.NullString
|
||||
LibraryID int64
|
||||
}
|
||||
|
||||
type GetFilePathsByRecordingMBIDsByLibraryRow struct {
|
||||
RecordingMbid sql.NullString
|
||||
FilePath string
|
||||
}
|
||||
|
||||
func (q *Queries) GetFilePathsByRecordingMBIDsByLibrary(ctx context.Context, arg GetFilePathsByRecordingMBIDsByLibraryParams) ([]GetFilePathsByRecordingMBIDsByLibraryRow, error) {
|
||||
query := getFilePathsByRecordingMBIDsByLibrary
|
||||
var queryParams []interface{}
|
||||
if len(arg.Mbids) > 0 {
|
||||
for _, v := range arg.Mbids {
|
||||
queryParams = append(queryParams, v)
|
||||
}
|
||||
query = strings.Replace(query, "/*SLICE:mbids*/?", strings.Repeat(",?", len(arg.Mbids))[1:], 1)
|
||||
} else {
|
||||
query = strings.Replace(query, "/*SLICE:mbids*/?", "NULL", 1)
|
||||
}
|
||||
queryParams = append(queryParams, arg.LibraryID)
|
||||
rows, err := q.db.QueryContext(ctx, query, queryParams...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
var items []GetFilePathsByRecordingMBIDsByLibraryRow
|
||||
for rows.Next() {
|
||||
var i GetFilePathsByRecordingMBIDsByLibraryRow
|
||||
if err := rows.Scan(&i.RecordingMbid, &i.FilePath); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
items = append(items, i)
|
||||
}
|
||||
if err := rows.Close(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return items, nil
|
||||
}
|
||||
|
||||
const getFilePathsByReleaseGroups = `-- name: GetFilePathsByReleaseGroups :many
|
||||
|
||||
SELECT rgr.release_group_id, af.file_path
|
||||
|
||||
@@ -229,3 +229,104 @@ func TestGetFilePathsByGenres_Empty(t *testing.T) {
|
||||
t.Errorf("got %v, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
// seedRecordingMBIDs stamps recording MBIDs onto the tracks seeded by
|
||||
// seedAlbumsAndGenres, in the shape the catalog side actually meets: two
|
||||
// tracks tagged, one deliberately left untagged, and one MBID carried by
|
||||
// two files in different libraries — which is what a duplicate is.
|
||||
func seedRecordingMBIDs(t *testing.T, lib *Library) (tagged, shared string) {
|
||||
t.Helper()
|
||||
|
||||
ctx := lib.ctx
|
||||
q := lib.db.Queries
|
||||
|
||||
tagged = "11111111-1111-1111-1111-111111111111"
|
||||
shared = "22222222-2222-2222-2222-222222222222"
|
||||
|
||||
byPath := map[string]string{
|
||||
"/music/a1.mp3": tagged,
|
||||
"/music/a2.mp3": shared,
|
||||
"/other/b1.mp3": shared,
|
||||
}
|
||||
|
||||
files, err := q.GetAllAudioFiles(ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("get audio files: %v", err)
|
||||
}
|
||||
|
||||
for _, f := range files {
|
||||
mbid, ok := byPath[f.FilePath]
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
|
||||
if err := q.SetRecordingMBID(ctx, sqlcgen.SetRecordingMBIDParams{
|
||||
Mbid: sql.NullString{String: mbid, Valid: true},
|
||||
ID: f.RecordingID,
|
||||
}); err != nil {
|
||||
t.Fatalf("set recording mbid: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
return tagged, shared
|
||||
}
|
||||
|
||||
// The catalog side of the same finding: an Explore album page knows what
|
||||
// the user owns only as recording MBIDs, so this is the lookup that
|
||||
// turns "you own 7 of these 12" into something playable.
|
||||
func TestGetFilePathsByRecordingMBIDs(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lib, _ := setupTestLibrary(t)
|
||||
_, libraryID := seedAlbumsAndGenres(t, lib)
|
||||
tagged, shared := seedRecordingMBIDs(t, lib)
|
||||
|
||||
got, err := lib.GetFilePathsByRecordingMBIDs([]string{tagged, shared}, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("GetFilePathsByRecordingMBIDs: %v", err)
|
||||
}
|
||||
|
||||
if len(got[tagged]) != 1 || got[tagged][0] != "/music/a1.mp3" {
|
||||
t.Errorf("tagged recording = %v, want [/music/a1.mp3]", got[tagged])
|
||||
}
|
||||
|
||||
// One recording, two files: grouping is what keeps that visible.
|
||||
// A flattened result could not say which was which.
|
||||
if len(got[shared]) != 2 {
|
||||
t.Errorf("shared recording = %v, want two paths", got[shared])
|
||||
}
|
||||
|
||||
// Scoping drops the copy in the other library, and nothing else.
|
||||
scoped, err := lib.GetFilePathsByRecordingMBIDs([]string{tagged, shared}, libraryID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetFilePathsByRecordingMBIDs scoped: %v", err)
|
||||
}
|
||||
|
||||
if len(scoped[shared]) != 1 || scoped[shared][0] != "/music/a2.mp3" {
|
||||
t.Errorf("scoped shared = %v, want [/music/a2.mp3]", scoped[shared])
|
||||
}
|
||||
}
|
||||
|
||||
// An empty MBID matches every untagged recording in the library, which
|
||||
// is the opposite of the question being asked — so an unknown track must
|
||||
// contribute nothing rather than everything.
|
||||
func TestGetFilePathsByRecordingMBIDs_IgnoresEmpty(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lib, _ := setupTestLibrary(t)
|
||||
seedAlbumsAndGenres(t, lib)
|
||||
seedRecordingMBIDs(t, lib)
|
||||
|
||||
got, err := lib.GetFilePathsByRecordingMBIDs([]string{"", ""}, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("GetFilePathsByRecordingMBIDs: %v", err)
|
||||
}
|
||||
|
||||
if len(got) != 0 {
|
||||
t.Errorf("empty MBIDs matched %d recordings, want none", len(got))
|
||||
}
|
||||
|
||||
if _, err := lib.GetFilePathsByRecordingMBIDs(nil, 0); err != nil {
|
||||
t.Fatalf("nil MBIDs: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1204,3 +1204,117 @@ func (l *Library) GetFilePathsByGenres(
|
||||
|
||||
return paths, nil
|
||||
}
|
||||
|
||||
// GetFilePathsByRecordingMBIDs returns the file paths of every track
|
||||
// whose recording MBID is in mbids, grouped by MBID.
|
||||
//
|
||||
// This is the catalog side of GetFilePathsByAlbums. An Explore album
|
||||
// page knows what the user owns as a set of recording MBIDs and nothing
|
||||
// else: that is exactly how the backend decides a track's InLibrary
|
||||
// flag (markReleasesInLibrary → CheckMBIDs), and MBTrack.LocalID is a
|
||||
// declared field that nothing writes, so there is no id to ask by.
|
||||
//
|
||||
// Grouped rather than flattened for the same two reasons as its
|
||||
// siblings — the caller owns the order (the tracklist's, not the
|
||||
// database's), and one recording can have more than one file, which is
|
||||
// what this app's duplicate detection exists for.
|
||||
//
|
||||
// A library id of 0 means "every library".
|
||||
func (l *Library) GetFilePathsByRecordingMBIDs(
|
||||
mbids []string, libraryID int64,
|
||||
) (map[string][]string, error) {
|
||||
paths := make(map[string][]string, len(mbids))
|
||||
|
||||
if len(mbids) == 0 {
|
||||
return paths, nil
|
||||
}
|
||||
|
||||
// recordings.mbid is nullable, so sqlc asks for NullStrings. An
|
||||
// empty MBID would match every untagged recording in the library,
|
||||
// which is the opposite of the question, so those are dropped here
|
||||
// rather than passed through as NULL.
|
||||
keys := make([]sql.NullString, 0, len(mbids))
|
||||
|
||||
for _, mbid := range mbids {
|
||||
if mbid == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
keys = append(keys, sql.NullString{String: mbid, Valid: true})
|
||||
}
|
||||
|
||||
if len(keys) == 0 {
|
||||
return paths, nil
|
||||
}
|
||||
|
||||
rows, err := l.filePathRowsByMBID(keys, libraryID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
for _, row := range rows {
|
||||
if !row.mbid.Valid {
|
||||
continue
|
||||
}
|
||||
|
||||
paths[row.mbid.String] = append(paths[row.mbid.String], row.path)
|
||||
}
|
||||
|
||||
return paths, nil
|
||||
}
|
||||
|
||||
// filePathRowsByMBID runs the scoped or unscoped query behind
|
||||
// GetFilePathsByRecordingMBIDs and flattens the two row types into one.
|
||||
func (l *Library) filePathRowsByMBID(
|
||||
keys []sql.NullString, libraryID int64,
|
||||
) ([]mbidFilePath, error) {
|
||||
if libraryID > 0 {
|
||||
rows, err := l.db.ReadQueries.GetFilePathsByRecordingMBIDsByLibrary(
|
||||
l.ctx, sqlcgen.GetFilePathsByRecordingMBIDsByLibraryParams{
|
||||
Mbids: keys,
|
||||
LibraryID: libraryID,
|
||||
},
|
||||
)
|
||||
if err != nil {
|
||||
l.logger.Error(
|
||||
"could not retrieve recording file paths for library",
|
||||
"recordings", len(keys),
|
||||
"libraryID", libraryID,
|
||||
"error", err,
|
||||
)
|
||||
|
||||
return nil, fmt.Errorf("could not get recording file paths: %w", err)
|
||||
}
|
||||
|
||||
out := make([]mbidFilePath, 0, len(rows))
|
||||
for _, row := range rows {
|
||||
out = append(out, mbidFilePath{mbid: row.RecordingMbid, path: row.FilePath})
|
||||
}
|
||||
|
||||
return out, nil
|
||||
}
|
||||
|
||||
rows, err := l.db.ReadQueries.GetFilePathsByRecordingMBIDs(l.ctx, keys)
|
||||
if err != nil {
|
||||
l.logger.Error(
|
||||
"could not retrieve recording file paths",
|
||||
"recordings", len(keys),
|
||||
"error", err,
|
||||
)
|
||||
|
||||
return nil, fmt.Errorf("could not get recording file paths: %w", err)
|
||||
}
|
||||
|
||||
out := make([]mbidFilePath, 0, len(rows))
|
||||
for _, row := range rows {
|
||||
out = append(out, mbidFilePath{mbid: row.RecordingMbid, path: row.FilePath})
|
||||
}
|
||||
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// mbidFilePath is one row of either GetFilePathsByRecordingMBIDs query.
|
||||
type mbidFilePath struct {
|
||||
mbid sql.NullString
|
||||
path string
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user