Lots of fixes for artist detail page and track management

src/config.rs

@@ -16,6 +16,11 @@ pub struct AppConfig {
     #[serde(default = "default_organization_format")]
     pub organization_format: String,
 
+    /// Which secondary release group types to include. Empty = studio releases only.
+    /// Options: "Compilation", "Live", "Soundtrack", "Remix", "DJ-mix", "Demo", etc.
+    #[serde(default)]
+    pub allowed_secondary_types: Vec<String>,
+
     #[serde(default)]
     pub web: WebConfig,
 
@@ -66,6 +71,7 @@ impl Default for AppConfig {
             database_url: default_database_url(),
             download_path: default_download_path(),
             organization_format: default_organization_format(),
+            allowed_secondary_types: vec![], // empty = studio only
             web: WebConfig::default(),
             tagging: TaggingConfig::default(),
             download: DownloadConfig::default(),

src/routes/albums.rs

@@ -1,7 +1,9 @@
 use actix_web::{web, HttpResponse};
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 
+use shanty_db::entities::wanted_item::WantedStatus;
 use shanty_db::queries;
+use shanty_tag::provider::MetadataProvider;
 
 use crate::error::ApiError;
 use crate::state::AppState;
@@ -22,6 +24,16 @@ pub struct AddAlbumRequest {
     mbid: Option<String>,
 }
 
+#[derive(Serialize)]
+struct AlbumTrackInfo {
+    recording_mbid: String,
+    title: String,
+    track_number: Option<i32>,
+    disc_number: Option<i32>,
+    duration_ms: Option<u64>,
+    status: Option<String>,
+}
+
 pub fn configure(cfg: &mut web::ServiceConfig) {
     cfg.service(
         web::resource("/albums")
@@ -29,7 +41,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) {
             .route(web::post().to(add_album)),
     )
     .service(
-        web::resource("/albums/{id}")
+        web::resource("/albums/{mbid}")
             .route(web::get().to(get_album)),
     );
 }
@@ -42,19 +54,107 @@ async fn list_albums(
     Ok(HttpResponse::Ok().json(albums))
 }
 
+/// Get album by MBID. Accepts either a release MBID or a release-group MBID.
+/// Tries as a release first; if that fails (404), treats it as a release-group
+/// and browses for its first release.
 async fn get_album(
     state: web::Data<AppState>,
-    path: web::Path<i32>,
+    path: web::Path<String>,
 ) -> Result<HttpResponse, ApiError> {
-    let id = path.into_inner();
-    let album = queries::albums::get_by_id(state.db.conn(), id).await?;
-    let tracks = queries::tracks::get_by_album(state.db.conn(), id).await?;
+    let mbid = path.into_inner();
+
+    // Try fetching as a release first
+    let mb_tracks = match state.mb_client.get_release_tracks(&mbid).await {
+        Ok(tracks) => tracks,
+        Err(_) => {
+            // Probably a release-group MBID. Browse releases for this group.
+            let release_mbid = resolve_release_from_group(&state, &mbid).await?;
+            state.mb_client
+                .get_release_tracks(&release_mbid)
+                .await
+                .map_err(|e| ApiError::Internal(format!("MusicBrainz error: {e}")))?
+        }
+    };
+
+    // Get all wanted items to check local status
+    let all_wanted = queries::wanted::list(state.db.conn(), None).await?;
+
+    let tracks: Vec<AlbumTrackInfo> = mb_tracks
+        .into_iter()
+        .map(|t| {
+            let status = all_wanted
+                .iter()
+                .find(|w| w.musicbrainz_id.as_deref() == Some(&t.recording_mbid))
+                .map(|w| match w.status {
+                    WantedStatus::Owned => "owned",
+                    WantedStatus::Downloaded => "downloaded",
+                    WantedStatus::Wanted => "wanted",
+                    WantedStatus::Available => "available",
+                })
+                .map(String::from);
+
+            AlbumTrackInfo {
+                recording_mbid: t.recording_mbid,
+                title: t.title,
+                track_number: t.track_number,
+                disc_number: t.disc_number,
+                duration_ms: t.duration_ms,
+                status,
+            }
+        })
+        .collect();
+
     Ok(HttpResponse::Ok().json(serde_json::json!({
-        "album": album,
+        "mbid": mbid,
         "tracks": tracks,
     })))
 }
 
+/// Given a release-group MBID, find the first release MBID via the MB API.
+async fn resolve_release_from_group(
+    state: &web::Data<AppState>,
+    release_group_mbid: &str,
+) -> Result<String, ApiError> {
+    // Use the MB client's get_json (it's private, so we go through search)
+    // The approach: search for releases by this release group
+    // MB API: /ws/2/release?release-group={mbid}&fmt=json&limit=1
+    // Since we can't call get_json directly, use the artist_releases approach
+    // to find a release that matches this group.
+    //
+    // Actually, the simplest: the MetadataProvider trait has get_artist_releases
+    // which returns releases, but we need releases for a release GROUP.
+    // Let's add a direct HTTP call here via reqwest.
+
+    let url = format!(
+        "https://musicbrainz.org/ws/2/release?release-group={release_group_mbid}&fmt=json&limit=1"
+    );
+
+    // Respect rate limiting by going through a small delay
+    tokio::time::sleep(std::time::Duration::from_millis(1100)).await;
+
+    let client = reqwest::Client::builder()
+        .user_agent("Shanty/0.1.0 (shanty-music-app)")
+        .build()
+        .map_err(|e| ApiError::Internal(e.to_string()))?;
+
+    let resp: serde_json::Value = client
+        .get(&url)
+        .send()
+        .await
+        .map_err(|e| ApiError::Internal(e.to_string()))?
+        .json()
+        .await
+        .map_err(|e| ApiError::Internal(e.to_string()))?;
+
+    resp.get("releases")
+        .and_then(|r| r.as_array())
+        .and_then(|arr| arr.first())
+        .and_then(|r| r.get("id"))
+        .and_then(|id| id.as_str())
+        .map(String::from)
+        .ok_or_else(|| ApiError::NotFound(format!("no releases found for release group {release_group_mbid}")))
+}
+
 async fn add_album(
     state: web::Data<AppState>,
     body: web::Json<AddAlbumRequest>,

src/routes/artists.rs

@@ -1,7 +1,10 @@
 use actix_web::{web, HttpResponse};
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 
+use shanty_db::entities::wanted_item::WantedStatus;
 use shanty_db::queries;
+use shanty_search::SearchProvider;
+use shanty_tag::provider::MetadataProvider;
 
 use crate::error::ApiError;
 use crate::state::AppState;
@@ -21,12 +24,53 @@ pub struct AddArtistRequest {
     mbid: Option<String>,
 }
 
+#[derive(Serialize)]
+struct ArtistListItem {
+    id: i32,
+    name: String,
+    musicbrainz_id: Option<String>,
+    total_watched: usize,
+    total_owned: usize,
+    total_items: usize,
+}
+
+#[derive(Serialize, Deserialize, Clone)]
+struct CachedAlbumTracks {
+    release_mbid: String,
+    tracks: Vec<CachedTrack>,
+}
+
+#[derive(Serialize, Deserialize, Clone)]
+struct CachedTrack {
+    recording_mbid: String,
+    title: String,
+}
+
+#[derive(Serialize)]
+struct FullAlbumInfo {
+    mbid: String,
+    title: String,
+    release_type: Option<String>,
+    date: Option<String>,
+    track_count: u32,
+    local_album_id: Option<i32>,
+    watched_tracks: u32,
+    owned_tracks: u32,
+    downloaded_tracks: u32,
+    total_local_tracks: u32,
+    status: String,
+}
+
 pub fn configure(cfg: &mut web::ServiceConfig) {
     cfg.service(
         web::resource("/artists")
             .route(web::get().to(list_artists))
             .route(web::post().to(add_artist)),
     )
+    .service(
+        web::resource("/artists/{id}/full")
+            .route(web::get().to(get_artist_full)),
+    )
     .service(
         web::resource("/artists/{id}")
             .route(web::get().to(get_artist))
@@ -39,19 +83,413 @@ async fn list_artists(
     query: web::Query<PaginationParams>,
 ) -> Result<HttpResponse, ApiError> {
     let artists = queries::artists::list(state.db.conn(), query.limit, query.offset).await?;
-    Ok(HttpResponse::Ok().json(artists))
+    let wanted = queries::wanted::list(state.db.conn(), None).await?;
+
+    let mut items: Vec<ArtistListItem> = Vec::new();
+    for a in &artists {
+        let artist_wanted: Vec<_> = wanted
+            .iter()
+            .filter(|w| w.artist_id == Some(a.id))
+            .collect();
+
+        // Check if we have cached artist-level totals from a prior detail page load
+        let cache_key = format!("artist_totals:{}", a.id);
+        let cached_totals: Option<(u32, u32, u32)> = if let Ok(Some(json)) =
+            queries::cache::get(state.db.conn(), &cache_key).await
+        {
+            serde_json::from_str(&json).ok()
+        } else {
+            None
+        };
+
+        let (total_watched, total_owned, total_items) = if let Some((avail, watched, owned)) = cached_totals {
+            (watched as usize, owned as usize, avail as usize)
+        } else {
+            // Fall back to wanted item counts
+            let total_items = artist_wanted.len();
+            let total_owned = artist_wanted.iter().filter(|w| w.status == WantedStatus::Owned).count();
+            (total_items, total_owned, total_items)
+        };
+
+        items.push(ArtistListItem {
+            id: a.id,
+            name: a.name.clone(),
+            musicbrainz_id: a.musicbrainz_id.clone(),
+            total_watched,
+            total_owned,
+            total_items,
+        });
+    }
+
+    Ok(HttpResponse::Ok().json(items))
 }
 
 async fn get_artist(
     state: web::Data<AppState>,
-    path: web::Path<i32>,
+    path: web::Path<String>,
 ) -> Result<HttpResponse, ApiError> {
-    let id = path.into_inner();
-    let artist = queries::artists::get_by_id(state.db.conn(), id).await?;
-    let albums = queries::albums::get_by_artist(state.db.conn(), id).await?;
+    let id_or_mbid = path.into_inner();
+    if let Ok(id) = id_or_mbid.parse::<i32>() {
+        let artist = queries::artists::get_by_id(state.db.conn(), id).await?;
+        let albums = queries::albums::get_by_artist(state.db.conn(), id).await?;
+        Ok(HttpResponse::Ok().json(serde_json::json!({
+            "artist": artist,
+            "albums": albums,
+        })))
+    } else {
+        Err(ApiError::BadRequest("use /artists/{id}/full for MBID lookups".into()))
+    }
+}
+
+/// Fetch (or retrieve from cache) the tracklist for a release group.
+/// Cache key: `artist_rg_tracks:{release_group_id}`
+async fn get_cached_album_tracks(
+    state: &web::Data<AppState>,
+    rg_id: &str,
+    first_release_id: Option<&str>,
+    ttl_seconds: i64,
+) -> Result<CachedAlbumTracks, ApiError> {
+    let cache_key = format!("artist_rg_tracks:{rg_id}");
+
+    // Check cache first
+    if let Some(json) = queries::cache::get(state.db.conn(), &cache_key).await
+        .map_err(|e| ApiError::Internal(e.to_string()))?
+    {
+        if let Ok(cached) = serde_json::from_str::<CachedAlbumTracks>(&json) {
+            return Ok(cached);
+        }
+    }
+
+    // Not cached — resolve release MBID and fetch tracks
+    let release_mbid = if let Some(rid) = first_release_id {
+        rid.to_string()
+    } else {
+        // Browse releases for this release group
+        resolve_release_from_group(rg_id).await?
+    };
+
+    let mb_tracks = state.mb_client
+        .get_release_tracks(&release_mbid)
+        .await
+        .map_err(|e| ApiError::Internal(format!("MB error for release {release_mbid}: {e}")))?;
+
+    let cached = CachedAlbumTracks {
+        release_mbid: release_mbid.clone(),
+        tracks: mb_tracks
+            .into_iter()
+            .map(|t| CachedTrack {
+                recording_mbid: t.recording_mbid,
+                title: t.title,
+            })
+            .collect(),
+    };
+
+    // Cache with caller-specified TTL
+    let json = serde_json::to_string(&cached)
+        .map_err(|e| ApiError::Internal(e.to_string()))?;
+    let _ = queries::cache::set(state.db.conn(), &cache_key, "musicbrainz", &json, ttl_seconds).await;
+
+    Ok(cached)
+}
+
+/// Given a release-group MBID, find the first release MBID.
+async fn resolve_release_from_group(release_group_mbid: &str) -> Result<String, ApiError> {
+    tokio::time::sleep(std::time::Duration::from_millis(1100)).await;
+
+    let client = reqwest::Client::builder()
+        .user_agent("Shanty/0.1.0 (shanty-music-app)")
+        .build()
+        .map_err(|e| ApiError::Internal(e.to_string()))?;
+
+    let url = format!(
+        "https://musicbrainz.org/ws/2/release?release-group={release_group_mbid}&fmt=json&limit=1"
+    );
+
+    let resp: serde_json::Value = client
+        .get(&url)
+        .send()
+        .await
+        .map_err(|e| ApiError::Internal(e.to_string()))?
+        .json()
+        .await
+        .map_err(|e| ApiError::Internal(e.to_string()))?;
+
+    resp.get("releases")
+        .and_then(|r| r.as_array())
+        .and_then(|arr| arr.first())
+        .and_then(|r| r.get("id"))
+        .and_then(|id| id.as_str())
+        .map(String::from)
+        .ok_or_else(|| ApiError::NotFound(format!("no releases for group {release_group_mbid}")))
+}
+
+#[derive(Deserialize)]
+pub struct ArtistFullParams {
+    #[serde(default)]
+    quick: bool,
+}
+
+async fn get_artist_full(
+    state: web::Data<AppState>,
+    path: web::Path<String>,
+    query: web::Query<ArtistFullParams>,
+) -> Result<HttpResponse, ApiError> {
+    let id_or_mbid = path.into_inner();
+    let quick_mode = query.quick;
+
+    // Resolve artist: local ID or MBID
+    let (artist, id, mbid) = if let Ok(local_id) = id_or_mbid.parse::<i32>() {
+        let artist = queries::artists::get_by_id(state.db.conn(), local_id).await?;
+        let mbid = match &artist.musicbrainz_id {
+            Some(m) => m.clone(),
+            None => {
+                let results = state.search.search_artist(&artist.name, 1).await
+                    .map_err(|e| ApiError::Internal(e.to_string()))?;
+                results.into_iter().next().map(|a| a.id)
+                    .ok_or_else(|| ApiError::NotFound(format!("no MBID for artist '{}'", artist.name)))?
+            }
+        };
+        (artist, Some(local_id), mbid)
+    } else {
+        let mbid = id_or_mbid;
+
+        // Direct MBID lookup — first check local DB, then MusicBrainz
+        let local = {
+            // Check if any local artist has this MBID
+            let all = queries::artists::list(state.db.conn(), 1000, 0).await?;
+            all.into_iter().find(|a| a.musicbrainz_id.as_deref() == Some(&mbid))
+        };
+
+        if let Some(a) = local {
+            let local_id = a.id;
+            (a, Some(local_id), mbid)
+        } else {
+            // Look up artist name from MusicBrainz by MBID — don't create a local record
+            let (name, _disambiguation) = state.mb_client
+                .get_artist_by_mbid(&mbid)
+                .await
+                .map_err(|e| ApiError::NotFound(format!("artist MBID {mbid} not found: {e}")))?;
+
+            // Create a synthetic artist object for display only (not saved to DB)
+            let synthetic = shanty_db::entities::artist::Model {
+                id: 0,
+                name,
+                musicbrainz_id: Some(mbid.clone()),
+                added_at: chrono::Utc::now().naive_utc(),
+                top_songs: "[]".to_string(),
+                similar_artists: "[]".to_string(),
+            };
+            (synthetic, None, mbid)
+        }
+    };
+
+    // Fetch release groups and filter by allowed secondary types
+    let all_release_groups = state.search.get_release_groups(&mbid).await
+        .map_err(|e| ApiError::Internal(e.to_string()))?;
+    let allowed = &state.config.allowed_secondary_types;
+    let release_groups: Vec<_> = all_release_groups
+        .into_iter()
+        .filter(|rg| {
+            if rg.secondary_types.is_empty() {
+                true // Pure studio releases always included
+            } else {
+                // Include if ALL of the release group's secondary types are in the allowed list
+                rg.secondary_types.iter().all(|st| allowed.contains(st))
+            }
+        })
+        .collect();
+
+    // Get all wanted items for this artist
+    let all_wanted = queries::wanted::list(state.db.conn(), None).await?;
+    let artist_wanted: Vec<_> = all_wanted
+        .iter()
+        .filter(|w| id.is_some() && w.artist_id == id)
+        .collect();
+
+    // Build a set of wanted item recording MBIDs and their statuses for fast lookup (MBID only)
+    let wanted_by_mbid: std::collections::HashMap<&str, &WantedStatus> = artist_wanted
+        .iter()
+        .filter_map(|w| w.musicbrainz_id.as_deref().map(|mbid| (mbid, &w.status)))
+        .collect();
+
+    // Get local albums
+    let local_albums = if let Some(local_id) = id {
+        queries::albums::get_by_artist(state.db.conn(), local_id).await?
+    } else {
+        vec![]
+    };
+
+    // Quick mode: if no wanted items and ?quick=true, skip per-album MB fetches
+    let skip_track_fetch = quick_mode && artist_wanted.is_empty();
+
+    // Build full album info — fetch tracklists (from cache or MB) for each release group
+    // Deduplicate at the artist level:
+    // - available: unique recording MBIDs across all releases
+    // - watched/owned: unique wanted item MBIDs (so the same wanted item matching
+    //   multiple recordings across releases only counts once)
+    let mut seen_available: std::collections::HashSet<String> = std::collections::HashSet::new();
+    let mut seen_watched: std::collections::HashSet<String> = std::collections::HashSet::new();
+    let mut seen_owned: std::collections::HashSet<String> = std::collections::HashSet::new();
+    let mut albums: Vec<FullAlbumInfo> = Vec::new();
+
+    for rg in &release_groups {
+        if skip_track_fetch {
+            // Fast path: just list the release groups without track counts
+            albums.push(FullAlbumInfo {
+                mbid: rg.first_release_id.clone().unwrap_or_else(|| rg.id.clone()),
+                title: rg.title.clone(),
+                release_type: rg.primary_type.clone(),
+                date: rg.first_release_date.clone(),
+                track_count: 0,
+                local_album_id: None,
+                watched_tracks: 0,
+                owned_tracks: 0,
+                downloaded_tracks: 0,
+                total_local_tracks: 0,
+                status: "unwatched".to_string(),
+            });
+            continue;
+        }
+
+        // If artist has any watched items, cache permanently (10 years);
+        // otherwise cache for 7 days (just browsing)
+        let cache_ttl = if artist_wanted.is_empty() { 7 * 86400 } else { 10 * 365 * 86400 };
+
+        let cached = match get_cached_album_tracks(
+            &state,
+            &rg.id,
+            rg.first_release_id.as_deref(),
+            cache_ttl,
+        ).await {
+            Ok(c) => c,
+            Err(e) => {
+                tracing::warn!(rg_id = %rg.id, title = %rg.title, error = %e, "failed to fetch tracks");
+                // Still show the album, just without track data
+                albums.push(FullAlbumInfo {
+                    mbid: rg.first_release_id.clone().unwrap_or_else(|| rg.id.clone()),
+                    title: rg.title.clone(),
+                    release_type: rg.primary_type.clone(),
+                    date: rg.first_release_date.clone(),
+                    track_count: 0,
+                    local_album_id: None,
+                    watched_tracks: 0,
+                    owned_tracks: 0,
+                    downloaded_tracks: 0,
+                    total_local_tracks: 0,
+                    status: "unwatched".to_string(),
+                });
+                continue;
+            }
+        };
+
+        let track_count = cached.tracks.len() as u32;
+
+        // Match each track against wanted items by recording MBID or title
+        let mut watched: u32 = 0;
+        let mut owned: u32 = 0;
+        let mut downloaded: u32 = 0;
+
+        for track in &cached.tracks {
+            let rec_id = &track.recording_mbid;
+
+            // Add to artist-level unique available set
+            seen_available.insert(rec_id.clone());
+
+            // Match by recording MBID only
+            if let Some(s) = wanted_by_mbid.get(rec_id.as_str()) {
+                watched += 1;
+                seen_watched.insert(rec_id.clone());
+                match s {
+                    WantedStatus::Owned => {
+                        owned += 1;
+                        seen_owned.insert(rec_id.clone());
+                    }
+                    WantedStatus::Downloaded => {
+                        downloaded += 1;
+                    }
+                    _ => {}
+                }
+            }
+        }
+
+        // Local album match
+        let local = local_albums
+            .iter()
+            .find(|a| a.name.to_lowercase() == rg.title.to_lowercase());
+        let local_album_id = local.map(|a| a.id);
+        let local_tracks = if let Some(aid) = local_album_id {
+            queries::tracks::get_by_album(state.db.conn(), aid).await.unwrap_or_default().len() as u32
+        } else {
+            0
+        };
+
+        let status = if owned > 0 && owned >= track_count && track_count > 0 {
+            "owned"
+        } else if owned > 0 || downloaded > 0 {
+            "partial"
+        } else if watched > 0 {
+            "wanted"
+        } else {
+            "unwatched"
+        };
+
+        albums.push(FullAlbumInfo {
+            mbid: cached.release_mbid.clone(),
+            title: rg.title.clone(),
+            release_type: rg.primary_type.clone(),
+            date: rg.first_release_date.clone(),
+            track_count,
+            local_album_id,
+            watched_tracks: watched,
+            owned_tracks: owned,
+            downloaded_tracks: downloaded,
+            total_local_tracks: local_tracks,
+            status: status.to_string(),
+        });
+    }
+
+    // Sort: owned first, then partial, then wanted, then unwatched; within each by date
+    albums.sort_by(|a, b| {
+        let order = |s: &str| match s {
+            "owned" => 0, "partial" => 1, "wanted" => 2, _ => 3,
+        };
+        order(&a.status).cmp(&order(&b.status)).then_with(|| a.date.cmp(&b.date))
+    });
+
+    // Deduplicated artist-level totals
+    let total_available_tracks = seen_available.len() as u32;
+    let total_artist_watched = seen_watched.len() as u32;
+    let total_artist_owned = seen_owned.len() as u32;
+
+    let artist_status = if total_artist_owned > 0 && total_artist_owned >= total_available_tracks && total_available_tracks > 0 {
+        "owned"
+    } else if total_artist_watched > 0 {
+        "partial"
+    } else {
+        "unwatched"
+    };
+
+    // Cache artist-level totals for the library listing page
+    if !skip_track_fetch {
+        if let Some(local_id) = id {
+            let cache_key = format!("artist_totals:{local_id}");
+            let totals = serde_json::json!([total_available_tracks, total_artist_watched, total_artist_owned]);
+            let _ = queries::cache::set(
+                state.db.conn(), &cache_key, "computed",
+                &totals.to_string(),
+                if artist_wanted.is_empty() { 7 * 86400 } else { 10 * 365 * 86400 },
+            ).await;
+        }
+    }
+
     Ok(HttpResponse::Ok().json(serde_json::json!({
         "artist": artist,
         "albums": albums,
+        "artist_status": artist_status,
+        "total_available_tracks": total_available_tracks,
+        "total_watched_tracks": total_artist_watched,
+        "total_owned_tracks": total_artist_owned,
+        "enriched": !skip_track_fetch,
     })))
 }