From 0ffcdd1425af2cef777808d512d51facf940ded3 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:34:53 -0600 Subject: [PATCH] fix(discover): link recently played album artists by source Retain artist IDs in new history snapshots and enable artist links in the carousel and View all. Route local sources to Local Library Artists and resolve older Qobuz snapshots through the existing album lookup. cx --- crates/qbz-qt/qml/views/AlbumCollection.qml | 10 ++- crates/qbz-qt/qml/views/HomeView.qml | 5 +- crates/qbz-qt/src/home_bridge.rs | 23 +++++ crates/qbz-qt/src/home_qt.rs | 94 +++++++++++++++++++++ crates/qbz-qt/src/recently_qt.rs | 30 +++++++ 5 files changed, 159 insertions(+), 3 deletions(-) diff --git a/crates/qbz-qt/qml/views/AlbumCollection.qml b/crates/qbz-qt/qml/views/AlbumCollection.qml index 4e5324417..3f451f8ab 100644 --- a/crates/qbz-qt/qml/views/AlbumCollection.qml +++ b/crates/qbz-qt/qml/views/AlbumCollection.qml @@ -386,7 +386,10 @@ Column { sources: gcell.modelData.sources || [] title: gcell.modelData.title artist: gcell.modelData.artist - artistId: gcell.modelData.artistId + artistId: gcell.modelData.historyArtistLink ? "" : gcell.modelData.artistId + hostArtistLink: gcell.modelData.historyArtistLink === true && (gcell.modelData.artist || "").trim() !== "" + onArtistRequested: QbzHome.openHistoryAlbumArtist( + gcell.modelData.id, gcell.modelData.source || "", gcell.modelData.artist || "", gcell.modelData.artistId || "") genre: gcell.modelData.genre year: gcell.modelData.year qualityTier: gcell.modelData.qualityTier @@ -563,7 +566,10 @@ Column { sources: cell.cardData.sources || [] title: cell.cardData.title artist: cell.cardData.artist - artistId: cell.cardData.artistId + artistId: cell.cardData.historyArtistLink ? "" : cell.cardData.artistId + hostArtistLink: cell.cardData.historyArtistLink === true && (cell.cardData.artist || "").trim() !== "" + onArtistRequested: QbzHome.openHistoryAlbumArtist( + cell.cardData.id, cell.cardData.source || "", cell.cardData.artist || "", cell.cardData.artistId || "") genre: cell.cardData.genre year: cell.cardData.year qualityTier: cell.cardData.qualityTier diff --git a/crates/qbz-qt/qml/views/HomeView.qml b/crates/qbz-qt/qml/views/HomeView.qml index cdf546f63..1bffa0641 100644 --- a/crates/qbz-qt/qml/views/HomeView.qml +++ b/crates/qbz-qt/qml/views/HomeView.qml @@ -605,7 +605,10 @@ Rectangle { sources: modelData.sources || [] title: modelData.title artist: modelData.artist - artistId: modelData.artistId + artistId: modelData.historyArtistLink ? "" : modelData.artistId + hostArtistLink: modelData.historyArtistLink === true && (modelData.artist || "").trim() !== "" + onArtistRequested: QbzHome.openHistoryAlbumArtist( + modelData.id, modelData.source || "", modelData.artist || "", modelData.artistId || "") genre: modelData.genre year: modelData.year qualityTier: modelData.qualityTier diff --git a/crates/qbz-qt/src/home_bridge.rs b/crates/qbz-qt/src/home_bridge.rs index a97f40674..83487ce13 100644 --- a/crates/qbz-qt/src/home_bridge.rs +++ b/crates/qbz-qt/src/home_bridge.rs @@ -179,6 +179,14 @@ pub mod qbz_home { album_id: QString, artist_name: QString, ); + #[qinvokable] + fn open_history_album_artist( + self: Pin<&mut QbzHome>, + album_id: QString, + source: QString, + artist_name: QString, + artist_id: QString, + ); /// Configurator > "Refresh now": rebuild every row, bypassing the /// results blob. The engine still honours its own per-week /// ListenBrainz cache, so this is not a way to hammer that service. @@ -549,6 +557,21 @@ impl qbz_home::QbzHome { crate::home_qt::open_pinned_album_artist(album_id.to_string(), artist_name.to_string()); } + pub fn open_history_album_artist( + self: Pin<&mut Self>, + album_id: QString, + source: QString, + artist_name: QString, + artist_id: QString, + ) { + crate::home_qt::open_history_album_artist( + album_id.to_string(), + source.to_string(), + artist_name.to_string(), + artist_id.to_string(), + ); + } + pub fn clear_playlist_tags(self: Pin<&mut Self>) { crate::home_qt::clear_playlist_tags(); } diff --git a/crates/qbz-qt/src/home_qt.rs b/crates/qbz-qt/src/home_qt.rs index 9d482acf9..9d96289f9 100644 --- a/crates/qbz-qt/src/home_qt.rs +++ b/crates/qbz-qt/src/home_qt.rs @@ -59,6 +59,10 @@ pub struct HomeCard { pub subtitle: String, #[serde(rename = "artistId")] pub artist_id: String, + /// History snapshots route by source and can resolve older catalog rows + /// that were recorded before artist ids were retained. + #[serde(rename = "historyArtistLink")] + pub history_artist_link: bool, /// Complete Qobuz contributor set used only while assembling Home. The /// wire keeps the historical primary `artistId`, but blacklist matching /// must also catch featured artists (`DiscoverAlbum.artists[]`). Keeping @@ -545,6 +549,55 @@ pub(crate) fn open_pinned_album_artist(album_id: String, artist_name: String) { }); } +#[derive(Debug, PartialEq)] +enum HistoryArtistTarget { + Local(String), + Catalog(String), + AlbumLookup, + None, +} + +fn history_artist_target( + album_id: &str, + source: &str, + name: &str, + artist_id: &str, +) -> HistoryArtistTarget { + if name.trim().is_empty() { + return HistoryArtistTarget::None; + } + let source = source.trim().to_ascii_lowercase(); + let local = matches!(source.as_str(), "local" | "user") + || matches!(qbz_source::SourceId::from_word(&source), Some(id) if id != qbz_source::SourceId::QOBUZ) + || crate::library_qt::is_local_album_key(album_id); + if local { + HistoryArtistTarget::Local(name.trim().to_string()) + } else if artist_id.parse::().is_ok_and(|id| id > 0) { + HistoryArtistTarget::Catalog(artist_id.to_string()) + } else if !album_id.is_empty() { + HistoryArtistTarget::AlbumLookup + } else { + HistoryArtistTarget::None + } +} + +pub(crate) fn open_history_album_artist( + album_id: String, + source: String, + name: String, + artist_id: String, +) { + match history_artist_target(&album_id, &source, &name, &artist_id) { + HistoryArtistTarget::Local(name) => { + crate::local_album_actions::open_artist_by_name(name); + crate::navigate_to("local"); + } + HistoryArtistTarget::Catalog(id) => crate::open_artist(id), + HistoryArtistTarget::AlbumLookup => open_pinned_album_artist(album_id, name), + HistoryArtistTarget::None => {} + } +} + /// Build the three local recently-played ordering slots from their stores. /// /// Kept separate from [`build_candidates`] because a track edge refreshes @@ -2118,6 +2171,12 @@ pub(crate) fn map_recent_album(a: crate::recently_qt::RecentAlbum) -> HomeCard { id: a.id, title: a.title, artist: a.artist, + artist_id: a + .artist_id + .filter(|id| *id > 0) + .map(|id| id.to_string()) + .unwrap_or_default(), + history_artist_link: true, blacklist_album_id, genre: a.genre, year: if a.release_date.is_empty() { @@ -2396,6 +2455,41 @@ where mod tests { use super::*; + #[test] + fn recent_album_artist_route_uses_source_before_catalog_identity() { + for source in [ + "local", + "user", + "plex", + "jellyfin", + "navidrome", + "qobuz_purchase", + ] { + assert_eq!( + history_artist_target("old-id", source, "Led Zeppelin", "123"), + HistoryArtistTarget::Local("Led Zeppelin".into()) + ); + } + assert_eq!( + history_artist_target("plex:old-id", "", "Led Zeppelin", "123"), + HistoryArtistTarget::Local("Led Zeppelin".into()) + ); + for source in ["qobuz", "qobuz_connect_remote", ""] { + assert_eq!( + history_artist_target("catalog-album", source, "Led Zeppelin", "123"), + HistoryArtistTarget::Catalog("123".into()) + ); + assert_eq!( + history_artist_target("catalog-album", source, "Led Zeppelin", ""), + HistoryArtistTarget::AlbumLookup + ); + } + assert_eq!( + history_artist_target("catalog-album", "qobuz", "", "123"), + HistoryArtistTarget::None + ); + } + fn audio(depth: Option, rate: Option) -> DiscoverAudioInfo { DiscoverAudioInfo { maximum_sampling_rate: rate, diff --git a/crates/qbz-qt/src/recently_qt.rs b/crates/qbz-qt/src/recently_qt.rs index f79f2c3f4..d1bea4ae3 100644 --- a/crates/qbz-qt/src/recently_qt.rs +++ b/crates/qbz-qt/src/recently_qt.rs @@ -79,6 +79,8 @@ pub struct RecentAlbum { #[serde(default)] pub artist: String, #[serde(default)] + pub artist_id: Option, + #[serde(default)] pub artwork_url: String, #[serde(default)] pub quality_tier: String, @@ -116,6 +118,7 @@ fn derive_albums(tracks: &[RecentTrack]) -> Vec { id: track.album_id.clone(), title: track.album_title.clone(), artist: track.album_artist.clone(), + artist_id: track.artist_id, artwork_url: track.album_artwork_url.clone(), quality_tier: track.quality_tier.clone(), quality_label: track.quality_label.clone(), @@ -276,6 +279,7 @@ pub(crate) fn record_in_context(track: RecentTrack, context_kind: &str) { id: track.album_id.clone(), title: track.album_title.clone(), artist: track.album_artist.clone(), + artist_id: track.artist_id, artwork_url: track.album_artwork_url.clone(), quality_tier: track.quality_tier.clone(), quality_label: track.quality_label.clone(), @@ -434,6 +438,32 @@ pub(crate) fn record_queue_track(track: &qbz_models::QueueTrack) { mod tests { use super::*; + #[test] + fn album_history_keeps_artist_identity_and_reads_old_snapshots() { + let tracks = vec![RecentTrack { + album_id: "catalog-album".into(), + album_artist: "Led Zeppelin".into(), + artist_id: Some(123), + source: "qobuz".into(), + ..Default::default() + }]; + let albums = derive_albums(&tracks); + let encoded = serde_json::to_string(&albums[0]).unwrap(); + let album: RecentAlbum = serde_json::from_str(&encoded).unwrap(); + let card = crate::home_qt::map_recent_album(album); + assert_eq!(card.artist_id, "123"); + assert!(card.history_artist_link); + let old: RecentAlbum = + serde_json::from_str(r#"{"id":"old-album","artist":"Led Zeppelin"}"#).unwrap(); + assert_eq!(old.artist_id, None); + let card = crate::home_qt::map_recent_album(old); + assert!(card.artist_id.is_empty()); + assert!( + card.history_artist_link, + "old albums still need a clickable name" + ); + } + #[test] fn local_dsd_queue_metadata_records_the_multiple_in_both_histories() { for (rate, expected) in [