fix(catalog): group Recently Added tracks into albums in the client
🏗️ Build and Test JellyTau / Run Tests (push) Waiting to run
🏗️ Build and Test JellyTau / Android Compile Check (push) Blocked by required conditions
🏗️ Build and Test JellyTau / Supply Chain (push) Waiting to run
Traceability Validation / Check Requirement Traces (push) Waiting to run
Publish Documentation / Build & publish docs to gitea-pages (push) Canceled after 0s
🏗️ Build and Test JellyTau / Run Tests (push) Waiting to run
🏗️ Build and Test JellyTau / Android Compile Check (push) Blocked by required conditions
🏗️ Build and Test JellyTau / Supply Chain (push) Waiting to run
Traceability Validation / Check Requirement Traces (push) Waiting to run
Publish Documentation / Build & publish docs to gitea-pages (push) Canceled after 0s
Recently Added still listed a newly-imported album one song at a time. GroupItems=true asks Jellyfin to collapse leaves into their container, but the server only groups a track whose parent chain actually resolves a MusicAlbum, and older servers ignore the parameter outright — so the raw leaves kept arriving. The home row passes no library, so the offline branch (which collapses in SQL) contributes nothing there and the server's answer *is* the row. Group again in the online repository, so the shape of the row is a property of this app rather than of the server it happens to be talking to: - A track naming an album_id collapses into one MusicAlbum card, placed where the first of its tracks stood so recency order survives. The card keeps the artwork, album name and artists; track number, duration, album link, streams and per-track user data stay with the leaf. - If the server did return the album row, that row wins and its tracks are dropped — it carries detail a track-built stand-in cannot. - Tracks with no album, movies, episodes and folders pass through untouched. - Collapsing only shrinks a listing, so the request over-fetches 3x and truncates afterwards; otherwise one 14-track import left the row nearly empty. Episodes are still grouped only by the server, so a freshly added season can flood the row the same way — same fix applies if it shows up.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -1306,6 +1306,117 @@ fn build_latest_items_endpoint(user_id: &str, parent_id: &str, limit: Option<usi
|
||||
)
|
||||
}
|
||||
|
||||
/// How many rows to ask the server for, given how many the row will show.
|
||||
///
|
||||
/// Collapsing only ever shrinks a listing, so a request for exactly the number
|
||||
/// of cards the row shows can come back as a handful after one freshly-ripped
|
||||
/// album folds its tracks together. Over-fetch and truncate after collapsing.
|
||||
///
|
||||
/// TRACES: UR-024, UR-034 | IR-024, JA-016 | UT-244
|
||||
fn latest_items_fetch_limit(limit: usize) -> 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<MediaItem>) -> Vec<MediaItem> {
|
||||
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<String> = items
|
||||
.iter()
|
||||
.filter(|i| i.kind == crate::domain::MediaKind::Album)
|
||||
.map(|i| i.id.clone())
|
||||
.collect();
|
||||
|
||||
let mut seen_albums: HashSet<String> = 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<usize>,
|
||||
) -> Result<Vec<MediaItem>, 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<JellyfinItem> = 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::<Vec<_>>()
|
||||
);
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user