diff --git a/docs/requirements.md b/docs/requirements.md index c4ba6744..5dc3e000 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -790,6 +790,10 @@ Internal architecture, components, and application logic. | UT-238 | The last episode of a season rolls over into the first of the next, skips an empty season on the way, and still stops there when the sleep timer says so | DR-263 | Done | | UT-239 | An episode watched to the end is not the current episode: not when the server's Next Up still names it (the stale answer is skipped in favour of the one after it), and not offline, where it counts as watched in the furthest-watched scan | DR-264 | Done | | UT-240 | Caching a server result mirrors its played flag locally — as synced, never invented for an item that carries no user data, and never over an unsynced local toggle | DR-264 | Done | +| UT-241 | A newly-imported album reads as one album card, not one card per song: three tracks sharing an album id collapse into a single `MusicAlbum` entry that opens the album, keeps its artwork and album artist, drops track-only detail (track number, duration, album link), and takes the position of the first of its tracks so recency order and the neighbouring movie survive | JA-016 | Done | +| UT-242 | When the server did group, its own album row wins — its overview and detail survive and the tracks it also returned add no second card for the same album | JA-016 | Done | +| UT-243 | A track that names no album has no container to collapse into and stays a track, the same way a movie does | JA-016 | Done | +| UT-244 | Recently Added over-fetches before collapsing, so folding one 14-track import together does not leave the row nearly empty | JA-016 | Done | ### Integration Tests diff --git a/src-tauri/src/repository/online.rs b/src-tauri/src/repository/online.rs index d628a0e4..3d7398ae 100644 --- a/src-tauri/src/repository/online.rs +++ b/src-tauri/src/repository/online.rs @@ -1306,6 +1306,117 @@ fn build_latest_items_endpoint(user_id: &str, parent_id: &str, limit: Option usize { + limit.saturating_mul(3) +} + +/// Collapse newly-added *tracks* into the album they belong to. +/// +/// `GroupItems=true` asks Jellyfin to do this server-side, but it only groups a +/// track whose parent chain actually resolves a `MusicAlbum`, and older servers +/// ignore the parameter outright — so "Recently Added" still filled up with one +/// card per song of a single import. Grouping again here makes the row's shape +/// a property of this app rather than of the server it is talking to. +/// +/// Rules: a track collapses only when it names an `album_id` (without one there +/// is no album to open, so a standalone track stays a track); if the server did +/// return the album row itself, that row wins and its tracks are dropped; the +/// album takes the position of the first of its tracks, so recency order +/// survives. Everything else — movies, episodes, folders — passes through +/// untouched. +/// +/// TRACES: UR-024, UR-034 | IR-024, JA-016 | UT-241, UT-242, UT-243 +fn collapse_tracks_into_albums(items: Vec) -> Vec { + use std::collections::HashSet; + + // Albums the server already returned in their own right: their tracks are + // redundant, and the real row carries detail a stand-in cannot. + let server_albums: HashSet = items + .iter() + .filter(|i| i.kind == crate::domain::MediaKind::Album) + .map(|i| i.id.clone()) + .collect(); + + let mut seen_albums: HashSet = HashSet::new(); + let mut collapsed = Vec::with_capacity(items.len()); + + for item in items { + let album_id = match (&item.kind, &item.album_id) { + (crate::domain::MediaKind::Track, Some(id)) => id.clone(), + _ => { + collapsed.push(item); + continue; + } + }; + + if server_albums.contains(&album_id) || !seen_albums.insert(album_id.clone()) { + continue; + } + collapsed.push(album_from_track(&item, album_id)); + } + + collapsed +} + +/// Build the album card a collapsed group of tracks stands for. +/// +/// The track's own artwork tag is reused: Jellyfin serves an item's primary +/// image by id and treats the tag as a cache key, and an embedded-art track +/// carries the album cover anyway. +/// +/// TRACES: UR-024, UR-034 | IR-024, JA-016 | UT-241 +fn album_from_track(track: &MediaItem, album_id: String) -> MediaItem { + MediaItem { + id: album_id, + name: track + .album_name + .clone() + .unwrap_or_else(|| "Unknown Album".to_string()), + item_type: "MusicAlbum".to_string(), + kind: crate::domain::MediaKind::Album, + is_folder: true, + server_id: track.server_id.clone(), + parent_id: None, + library_id: track.library_id.clone(), + overview: None, + genres: track.genres.clone(), + production_year: track.production_year, + premiere_date: track.premiere_date.clone(), + community_rating: None, + official_rating: None, + // A track's duration says nothing about the album's, and its track + // number, album link and streams belong to the leaf alone. + runtime_ticks: None, + duration_ms: None, + primary_image_tag: track.primary_image_tag.clone(), + image_id: track.image_id.clone(), + backdrop_image_tags: track.backdrop_image_tags.clone(), + parent_backdrop_image_tags: track.parent_backdrop_image_tags.clone(), + album_id: None, + album_name: None, + album_artist: track.album_artist.clone(), + artists: track.artists.clone(), + artist_items: track.artist_items.clone(), + index_number: None, + parent_index_number: None, + series_id: None, + series_name: None, + season_id: None, + season_name: None, + user_data: None, + media_streams: None, + media_sources: None, + people: None, + } +} + /// Build the Jellyfin endpoint for a Next Up listing. /// /// `EnableResumable=false` is the point of this query: the server default is @@ -1755,18 +1866,34 @@ impl MediaRepository for OnlineRepository { Ok(media_item) } + /// Recently Added, one card per thing that was added. + /// + /// The server is asked to group (`GroupItems=true`) *and* the answer is + /// grouped again here — see `collapse_tracks_into_albums` for why trusting + /// the server alone left the row full of one album's songs. + /// + /// TRACES: UR-024, UR-034 | IR-024, JA-016 | UT-241, UT-244 async fn get_latest_items( &self, parent_id: &str, limit: Option, ) -> Result, RepoError> { - let endpoint = build_latest_items_endpoint(&self.user_id, parent_id, limit); + let limit_val = limit.unwrap_or(16); + let endpoint = build_latest_items_endpoint( + &self.user_id, + parent_id, + Some(latest_items_fetch_limit(limit_val)), + ); let items: Vec = self.get_json(&endpoint).await?; - Ok(items + let items = items .into_iter() .map(|item| item.into_media_item(self.user_id.clone())) - .collect()) + .collect(); + + let mut collapsed = collapse_tracks_into_albums(items); + collapsed.truncate(limit_val); + Ok(collapsed) } /// Continue Watching: the items this user has started and not finished. @@ -4122,6 +4249,135 @@ mod tests { assert!(endpoint.contains("Limit=16")); } + /// Build a `MediaItem` the way a real listing does — through the Jellyfin + /// payload — so the fixtures cannot drift from the parsed shape. + fn item_from_json(json: &str) -> MediaItem { + let parsed: JellyfinItem = serde_json::from_str(json).expect("fixture must parse"); + parsed.into_media_item("srv".to_string()) + } + + fn track(id: &str, name: &str, album_id: Option<&str>) -> MediaItem { + let album = match album_id { + Some(a) => format!(r#""AlbumId": "{a}", "Album": "Kind of Blue","#), + None => String::new(), + }; + item_from_json(&format!( + r#"{{ + "Id": "{id}", + "Name": "{name}", + "Type": "Audio", + {album} + "ImageTags": {{"Primary": "art-{id}"}}, + "AlbumArtist": "Miles Davis", + "Artists": ["Miles Davis"], + "IndexNumber": 1, + "RunTimeTicks": 1000 + }}"# + )) + } + + /// A newly-imported album must read as *one* new album, not one new song + /// per track — even when the server hands back the raw leaves despite + /// `GroupItems=true` (older servers, and libraries whose tracks resolve no + /// album parent, ignore it). + /// + /// TRACES: UR-024, UR-034 | IR-024, JA-016 | UT-241 + #[test] + fn test_collapse_tracks_into_albums_shows_one_card_per_album() { + let movie = item_from_json( + r#"{"Id": "mov-1", "Name": "Heat", "Type": "Movie", "ImageTags": {"Primary": "art-mov"}}"#, + ); + let items = vec![ + track("trk-1", "So What", Some("alb-1")), + track("trk-2", "Blue in Green", Some("alb-1")), + movie, + track("trk-3", "Flamenco Sketches", Some("alb-1")), + ]; + + let collapsed = collapse_tracks_into_albums(items); + + assert_eq!( + collapsed.len(), + 2, + "three tracks of one album plus a movie must read as two cards, got: {:?}", + collapsed.iter().map(|i| &i.name).collect::>() + ); + + let album = &collapsed[0]; + assert_eq!(album.id, "alb-1", "the card must open the album"); + assert_eq!(album.name, "Kind of Blue"); + assert_eq!(album.item_type, "MusicAlbum"); + assert_eq!(album.kind, crate::domain::MediaKind::Album); + assert!(album.is_folder); + assert_eq!(album.album_artist.as_deref(), Some("Miles Davis")); + assert!(album.image_id.is_some(), "album card needs artwork"); + // Track-only detail must not ride along on a container. + assert!(album.index_number.is_none()); + assert!(album.album_id.is_none()); + assert!(album.runtime_ticks.is_none()); + + // The movie keeps its place after the album its tracks stood in front of. + assert_eq!(collapsed[1].id, "mov-1"); + } + + /// When the server *did* group, its own album row wins — the tracks it also + /// returned must not add a second card for the same album. + /// + /// TRACES: UR-024, UR-034 | IR-024, JA-016 | UT-242 + #[test] + fn test_collapse_prefers_the_album_row_the_server_returned() { + let album = item_from_json( + r#"{"Id": "alb-1", "Name": "Kind of Blue", "Type": "MusicAlbum", "IsFolder": true, + "Overview": "1959", "ImageTags": {"Primary": "art-alb"}}"#, + ); + let items = vec![ + album, + track("trk-1", "So What", Some("alb-1")), + track("trk-2", "Blue in Green", Some("alb-1")), + ]; + + let collapsed = collapse_tracks_into_albums(items); + + assert_eq!(collapsed.len(), 1, "one album, one card"); + assert_eq!(collapsed[0].id, "alb-1"); + assert_eq!( + collapsed[0].overview.as_deref(), + Some("1959"), + "the server's own album row must survive, not a track-built stand-in" + ); + } + + /// A track with no album has no container to collapse into, so it stays — + /// same reasoning that leaves movies alone. + /// + /// TRACES: UR-024, UR-034 | IR-024, JA-016 | UT-243 + #[test] + fn test_collapse_leaves_a_standalone_track_alone() { + let items = vec![track("trk-1", "Field Recording", None)]; + + let collapsed = collapse_tracks_into_albums(items); + + assert_eq!(collapsed.len(), 1); + assert_eq!(collapsed[0].id, "trk-1"); + assert_eq!(collapsed[0].item_type, "Audio"); + } + + /// Collapsing shrinks the listing, so the request has to over-fetch: asking + /// for exactly 16 rows and then folding one 14-track album into them leaves + /// an almost empty "Recently Added". + /// + /// TRACES: UR-024, UR-034 | IR-024, JA-016 | UT-244 + #[test] + fn test_latest_items_over_fetches_before_collapsing() { + assert!( + latest_items_fetch_limit(16) > 16, + "must ask for more rows than the row shows" + ); + let endpoint = + build_latest_items_endpoint("u1", "lib-1", Some(latest_items_fetch_limit(16))); + assert!(endpoint.contains(&format!("Limit={}", latest_items_fetch_limit(16)))); + } + /// UT-190 — Next Up asks the server to leave resumable episodes out. /// /// Jellyfin's `/Shows/NextUp` defaults `EnableResumable=true`, which returns