Fix bulk discography losing album source context (#399)
The bulk download_discography endpoint picked one metadata client
based on the configured primary source and called .get_album() on
every album with that single client. Albums whose IDs came from a
fallback/provider-specific source (e.g. Deezer-formatted IDs surfaced
through Hydrabase) failed with "Album not found" because the primary
client couldn't resolve them.
Bulk now uses the same source-aware resolver
(core.metadata.album_tracks.get_artist_album_tracks) the working
individual-album endpoint already uses, so the resolver's source-chain
walk finds each album under whichever provider actually has it. Also
adds explicit Discogs and Hydrabase support (the old if/elif chain
silently 500'd for those primaries).
Frontend (library.js + pages-extra.js) now sends a richer
`{ albums: [{id, name, artist_name, source}] }` payload so each album
can be resolved through its own source. The legacy `album_ids` payload
still works as a fallback path.
Closes #399.
This commit is contained in:
Broque Thomas
parent
7556e79dbd
commit
9534843edb
3 changed files with 150 additions and 57 deletions
167
web_server.py
167
web_server.py
|
|
@ -8853,33 +8853,58 @@ def get_album_tracks(album_id):
|
|||
|
||||
@app.route('/api/artist/<artist_id>/download-discography', methods=['POST'])
|
||||
def download_discography(artist_id):
|
||||
"""Add selected albums from an artist's discography to the wishlist."""
|
||||
"""Add selected albums from an artist's discography to the wishlist.
|
||||
|
||||
Resolves each album through the same source-aware path that the
|
||||
individual-album flow uses, so albums whose IDs come from a
|
||||
fallback/provider-specific source (e.g. Deezer-formatted IDs surfaced
|
||||
via Hydrabase) don't fail with "Album not found" when the primary
|
||||
source can't look them up directly.
|
||||
"""
|
||||
try:
|
||||
data = request.get_json()
|
||||
if not data or 'album_ids' not in data:
|
||||
return jsonify({"success": False, "error": "album_ids required"}), 400
|
||||
if not data:
|
||||
return jsonify({"success": False, "error": "request body required"}), 400
|
||||
|
||||
# Preferred payload: per-album metadata so each album can be resolved
|
||||
# through its own source. Falls back to the legacy album_ids list,
|
||||
# in which case every album is looked up under the artist-level source.
|
||||
albums_payload = data.get('albums')
|
||||
legacy_album_ids = data.get('album_ids')
|
||||
if not albums_payload and not legacy_album_ids:
|
||||
return jsonify({"success": False, "error": "albums or album_ids required"}), 400
|
||||
|
||||
album_ids = data['album_ids']
|
||||
artist_name = data.get('artist_name', 'Unknown Artist')
|
||||
artist_source = (data.get('source') or '').strip().lower() or None
|
||||
|
||||
if albums_payload:
|
||||
album_entries = [
|
||||
{
|
||||
'id': str(a.get('id', '')),
|
||||
'name': a.get('name') or a.get('title') or '',
|
||||
'source': (a.get('source') or '').strip().lower() or artist_source,
|
||||
'artist_name': a.get('artist_name') or artist_name,
|
||||
}
|
||||
for a in albums_payload if a.get('id')
|
||||
]
|
||||
else:
|
||||
album_entries = [
|
||||
{
|
||||
'id': str(aid),
|
||||
'name': '',
|
||||
'source': artist_source,
|
||||
'artist_name': artist_name,
|
||||
}
|
||||
for aid in legacy_album_ids if aid
|
||||
]
|
||||
|
||||
if not album_entries:
|
||||
return jsonify({"success": False, "error": "no valid albums in payload"}), 400
|
||||
|
||||
from database.music_database import MusicDatabase
|
||||
from core.metadata.album_tracks import get_artist_album_tracks
|
||||
db = MusicDatabase()
|
||||
profile_id = get_current_profile_id()
|
||||
active_server = config_manager.get_active_media_server()
|
||||
|
||||
# Resolve metadata client
|
||||
client = None
|
||||
if spotify_client and spotify_client.is_authenticated():
|
||||
client = spotify_client
|
||||
else:
|
||||
fallback_src = _get_metadata_fallback_source()
|
||||
if fallback_src == 'itunes':
|
||||
client = _get_itunes_client()
|
||||
elif fallback_src == 'deezer':
|
||||
client = _get_deezer_client()
|
||||
|
||||
if not client:
|
||||
return jsonify({"success": False, "error": "No metadata source available"}), 500
|
||||
|
||||
total_added = 0
|
||||
total_skipped = 0
|
||||
|
|
@ -8887,27 +8912,48 @@ def download_discography(artist_id):
|
|||
def generate_ndjson():
|
||||
nonlocal total_added, total_skipped
|
||||
|
||||
for album_id in album_ids:
|
||||
for entry in album_entries:
|
||||
album_id = entry['id']
|
||||
hint_album_name = entry['name']
|
||||
hint_artist = entry['artist_name']
|
||||
source_override = entry['source']
|
||||
try:
|
||||
album_data = client.get_album(album_id)
|
||||
if not album_data:
|
||||
yield json.dumps({"album_id": album_id, "status": "error", "message": "Album not found"}) + '\n'
|
||||
result = get_artist_album_tracks(
|
||||
album_id,
|
||||
artist_name=hint_artist,
|
||||
album_name=hint_album_name,
|
||||
source_override=source_override,
|
||||
)
|
||||
|
||||
if not result.get('success'):
|
||||
message = result.get('error') or 'Album not found'
|
||||
yield json.dumps({
|
||||
"album_id": album_id,
|
||||
"name": hint_album_name or album_id,
|
||||
"status": "error",
|
||||
"message": message,
|
||||
}) + '\n'
|
||||
continue
|
||||
|
||||
album_name = album_data.get('name', 'Unknown')
|
||||
album_images = album_data.get('images', [])
|
||||
album_type = album_data.get('album_type', 'album')
|
||||
release_date = album_data.get('release_date', '')
|
||||
album_artists = album_data.get('artists', [])
|
||||
|
||||
tracks = album_data.get('tracks', {}).get('items', [])
|
||||
if not tracks:
|
||||
tracks_data = client.get_album_tracks(album_id)
|
||||
if tracks_data and 'items' in tracks_data:
|
||||
tracks = tracks_data['items']
|
||||
album = result.get('album', {}) or {}
|
||||
tracks = result.get('tracks', []) or []
|
||||
album_name = album.get('name') or hint_album_name or 'Unknown'
|
||||
album_images = album.get('images') or (
|
||||
[{'url': album['image_url']}] if album.get('image_url') else []
|
||||
)
|
||||
album_type = album.get('album_type', 'album')
|
||||
release_date = album.get('release_date', '') or ''
|
||||
album_artists = album.get('artists') or [{'name': hint_artist}]
|
||||
resolved_album_id = result.get('resolved_album_id') or album.get('id') or album_id
|
||||
resolved_source = result.get('source') or source_override or 'unknown'
|
||||
|
||||
if not tracks:
|
||||
yield json.dumps({"album_id": album_id, "name": album_name, "status": "error", "message": "No tracks"}) + '\n'
|
||||
yield json.dumps({
|
||||
"album_id": album_id,
|
||||
"name": album_name,
|
||||
"status": "error",
|
||||
"message": "No tracks",
|
||||
}) + '\n'
|
||||
continue
|
||||
|
||||
added = 0
|
||||
|
|
@ -8915,24 +8961,23 @@ def download_discography(artist_id):
|
|||
|
||||
for track in tracks:
|
||||
track_name = track.get('name', '')
|
||||
track_artists = track.get('artists', [])
|
||||
track_id = track.get('id', '')
|
||||
|
||||
if not track_name:
|
||||
continue
|
||||
track_artists = track.get('artists', []) or album_artists
|
||||
track_id = track.get('id', '')
|
||||
|
||||
spotify_track_data = {
|
||||
'id': track_id,
|
||||
'name': track_name,
|
||||
'artists': track_artists if isinstance(track_artists, list) else [{'name': str(track_artists)}],
|
||||
'album': {
|
||||
'id': str(album_id),
|
||||
'id': str(resolved_album_id),
|
||||
'name': album_name,
|
||||
'artists': album_artists,
|
||||
'images': album_images,
|
||||
'album_type': album_type,
|
||||
'release_date': release_date,
|
||||
'total_tracks': len(tracks)
|
||||
'total_tracks': len(tracks),
|
||||
},
|
||||
'duration_ms': track.get('duration_ms', 0),
|
||||
'explicit': track.get('explicit', False),
|
||||
|
|
@ -8941,7 +8986,8 @@ def download_discography(artist_id):
|
|||
'uri': track.get('uri', ''),
|
||||
'preview_url': track.get('preview_url'),
|
||||
'external_urls': track.get('external_urls', {}),
|
||||
'is_local': False
|
||||
'is_local': False,
|
||||
'_source': resolved_source,
|
||||
}
|
||||
|
||||
try:
|
||||
|
|
@ -8950,11 +8996,12 @@ def download_discography(artist_id):
|
|||
failure_reason="Added via Download Discography",
|
||||
source_type="discography",
|
||||
source_info=json.dumps({
|
||||
'artist_name': artist_name,
|
||||
'artist_name': hint_artist,
|
||||
'album_name': album_name,
|
||||
'album_type': album_type
|
||||
'album_type': album_type,
|
||||
'source': resolved_source,
|
||||
}),
|
||||
profile_id=profile_id
|
||||
profile_id=profile_id,
|
||||
)
|
||||
if was_added:
|
||||
added += 1
|
||||
|
|
@ -8965,17 +9012,37 @@ def download_discography(artist_id):
|
|||
|
||||
total_added += added
|
||||
total_skipped += skipped
|
||||
logger.warning(f"[Discography] {album_name}: {added} added, {skipped} skipped")
|
||||
logger.warning(
|
||||
f"[Discography] {album_name} ({resolved_source}): {added} added, {skipped} skipped"
|
||||
)
|
||||
yield json.dumps({
|
||||
"album_id": album_id, "name": album_name, "status": "done",
|
||||
"tracks_added": added, "tracks_skipped": skipped, "tracks_total": len(tracks)
|
||||
"album_id": album_id,
|
||||
"name": album_name,
|
||||
"status": "done",
|
||||
"tracks_added": added,
|
||||
"tracks_skipped": skipped,
|
||||
"tracks_total": len(tracks),
|
||||
"source": resolved_source,
|
||||
}) + '\n'
|
||||
|
||||
except Exception as album_err:
|
||||
yield json.dumps({"album_id": album_id, "status": "error", "message": str(album_err)}) + '\n'
|
||||
yield json.dumps({
|
||||
"album_id": album_id,
|
||||
"name": hint_album_name or album_id,
|
||||
"status": "error",
|
||||
"message": str(album_err),
|
||||
}) + '\n'
|
||||
|
||||
logger.warning(f"[Discography] Complete for {artist_name}: {total_added} tracks added, {total_skipped} skipped across {len(album_ids)} albums")
|
||||
yield json.dumps({"status": "complete", "total_added": total_added, "total_skipped": total_skipped, "total_albums": len(album_ids)}) + '\n'
|
||||
logger.warning(
|
||||
f"[Discography] Complete for {artist_name}: {total_added} tracks added, "
|
||||
f"{total_skipped} skipped across {len(album_entries)} albums"
|
||||
)
|
||||
yield json.dumps({
|
||||
"status": "complete",
|
||||
"total_added": total_added,
|
||||
"total_skipped": total_skipped,
|
||||
"total_albums": len(album_entries),
|
||||
}) + '\n'
|
||||
|
||||
return app.response_class(generate_ndjson(), mimetype='application/x-ndjson', headers={'X-Accel-Buffering': 'no'})
|
||||
|
||||
|
|
|
|||
|
|
@ -2251,15 +2251,16 @@ function _renderDiscogCard(release, index, completionData) {
|
|||
const statusClass = isOwned ? 'owned' : isPartial ? 'partial' : '';
|
||||
const statusIcon = isOwned ? '✓' : isPartial ? '◐' : '';
|
||||
|
||||
const albumName = release.name || release.title || '';
|
||||
return `
|
||||
<label class="discog-card ${statusClass}" data-type="${release._type}" style="animation-delay:${index * 0.03}s">
|
||||
<input type="checkbox" class="discog-card-cb" data-album-id="${release.id}" data-tracks="${tracks}" ${checked ? 'checked' : ''} onchange="_updateDiscogFooterCount()">
|
||||
<input type="checkbox" class="discog-card-cb" data-album-id="${release.id}" data-album-name="${_esc(albumName)}" data-tracks="${tracks}" ${checked ? 'checked' : ''} onchange="_updateDiscogFooterCount()">
|
||||
<div class="discog-card-art">
|
||||
${img ? `<img src="${img}" alt="" loading="lazy">` : '<div class="discog-card-art-placeholder">🎵</div>'}
|
||||
${statusIcon ? `<span class="discog-card-status">${statusIcon}</span>` : ''}
|
||||
</div>
|
||||
<div class="discog-card-info">
|
||||
<div class="discog-card-title">${_esc(release.name)}</div>
|
||||
<div class="discog-card-title">${_esc(albumName)}</div>
|
||||
<div class="discog-card-meta">${year}${year && tracks ? ' · ' : ''}${tracks ? tracks + ' tracks' : ''}</div>
|
||||
</div>
|
||||
<div class="discog-card-check"></div>
|
||||
|
|
@ -2319,6 +2320,7 @@ async function startDiscographyDownload() {
|
|||
if (cb.closest('.discog-card').style.display !== 'none') {
|
||||
albumEntries.push({
|
||||
id: cb.dataset.albumId,
|
||||
name: cb.dataset.albumName || '',
|
||||
tracks: parseInt(cb.dataset.tracks) || 0
|
||||
});
|
||||
}
|
||||
|
|
@ -2326,9 +2328,8 @@ async function startDiscographyDownload() {
|
|||
// Sort by track count descending — process Deluxe/expanded editions first
|
||||
// so their tracks get added before standard editions (which then get deduped)
|
||||
albumEntries.sort((a, b) => b.tracks - a.tracks);
|
||||
const albumIds = albumEntries.map(e => e.id);
|
||||
|
||||
if (albumIds.length === 0) return;
|
||||
if (albumEntries.length === 0) return;
|
||||
|
||||
// Switch to progress view
|
||||
const grid = document.getElementById('discog-grid');
|
||||
|
|
@ -2379,11 +2380,26 @@ async function startDiscographyDownload() {
|
|||
// Mark all items as active
|
||||
document.querySelectorAll('.discog-progress-item').forEach(item => item.classList.add('active'));
|
||||
|
||||
// Per-album metadata so the backend can resolve each album through its
|
||||
// own source — fixes albums whose IDs come from a fallback/provider-specific
|
||||
// source (e.g. Deezer-formatted IDs surfaced via Hydrabase).
|
||||
const sourceForBatch = (artist.source || artistsPageState.sourceOverride || '').toString().toLowerCase() || null;
|
||||
const albumsPayload = albumEntries.map(e => ({
|
||||
id: e.id,
|
||||
name: e.name,
|
||||
artist_name: artist.name,
|
||||
source: sourceForBatch,
|
||||
}));
|
||||
|
||||
try {
|
||||
const response = await fetch(`/api/artist/${artist.id}/download-discography`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ album_ids: albumIds, artist_name: artist.name })
|
||||
body: JSON.stringify({
|
||||
albums: albumsPayload,
|
||||
artist_name: artist.name,
|
||||
source: sourceForBatch,
|
||||
})
|
||||
});
|
||||
|
||||
const reader = response.body.getReader();
|
||||
|
|
|
|||
|
|
@ -807,13 +807,23 @@ async function _explorerWishlistSubmit(artistSections) {
|
|||
for (const [artistId, data] of Object.entries(byArtist)) {
|
||||
// Sort by track count descending (deluxe editions first) BEFORE extracting IDs
|
||||
data.albums.sort((a, b) => b.tracks - a.tracks);
|
||||
const albumIds = data.albums.map(a => a.id);
|
||||
// Per-album metadata so the backend can resolve each album through its
|
||||
// own source even when the explorer doesn't carry per-album source info.
|
||||
const albumsPayload = data.albums.map(a => ({
|
||||
id: a.id,
|
||||
name: a.title || '',
|
||||
artist_name: data.name,
|
||||
source: null,
|
||||
}));
|
||||
|
||||
try {
|
||||
const response = await fetch(`/api/artist/${artistId}/download-discography`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ album_ids: albumIds, artist_name: data.name })
|
||||
body: JSON.stringify({
|
||||
albums: albumsPayload,
|
||||
artist_name: data.name,
|
||||
})
|
||||
});
|
||||
|
||||
const reader = response.body.getReader();
|
||||
|
|
|
|||
Loading…
Reference in a new issue