fix(library): podcasts list newest episode first
A Jellypod podcast listed its episodes alphabetically. The store pinned SortBy=SortName onto every drill-down, which overrode the order the channel plugin returns — and since Jellypod prefixes played episodes with "[Played]", the name sort also clumped every heard episode at the top. Which order a container's children take is domain knowledge, so it moves to Rust: the caller names the container (GetItemsOptions.parentKind) and default_listing_sort answers with the sort. A channel folder is PremiereDate descending, every other container keeps SortName ascending, and a caller naming no container still gets no SortBy, so the paths that rely on the server's own order keep it. An explicit sort always wins. ChannelFolderItem with is_folder now maps to MediaKind::ChannelFolder instead of collapsing into Folder — while both were Folder there was nothing to key the rule on. The offline leg of the cache/server race applies the same order, so the cached list no longer flashes in name order before the server's arrives. TRACES: UR-007 | DR-257 | UT-229, UT-230, UT-231
This commit is contained in:
@@ -1239,10 +1239,29 @@ impl MediaRepository for OfflineRepository {
|
||||
let start_index = opts.start_index.unwrap_or(0);
|
||||
|
||||
// SortBy=Random is the only sort the landing pages rely on offline (the
|
||||
// hero "surprise" pool); everything else keeps the stable name order.
|
||||
let order_by = match opts.sort_by.as_deref() {
|
||||
Some("Random") => "RANDOM()",
|
||||
_ => "i.sort_name ASC, i.name ASC",
|
||||
// hero "surprise" pool); PremiereDate is what a channel folder's
|
||||
// children are listed by (DR-257), so the cached leg of the race agrees
|
||||
// with the server's order instead of flashing a name-sorted list first.
|
||||
// Everything else keeps the stable name order.
|
||||
//
|
||||
// Rows with no premiere date sort last rather than leading the list.
|
||||
let default_sort = default_listing_sort(opts.parent_kind);
|
||||
let sort_field = opts
|
||||
.sort_by
|
||||
.as_deref()
|
||||
.or(default_sort.map(|(field, _)| field));
|
||||
let descending = opts
|
||||
.sort_order
|
||||
.as_deref()
|
||||
.or(default_sort.map(|(_, order)| order))
|
||||
== Some("Descending");
|
||||
let order_by = match sort_field {
|
||||
Some("Random") => "RANDOM()".to_string(),
|
||||
Some("PremiereDate") => format!(
|
||||
"i.premiere_date IS NULL, i.premiere_date {}, i.sort_name ASC",
|
||||
if descending { "DESC" } else { "ASC" }
|
||||
),
|
||||
_ => "i.sort_name ASC, i.name ASC".to_string(),
|
||||
};
|
||||
|
||||
// Bind the type filter rather than interpolating it: `include_item_types`
|
||||
|
||||
@@ -1230,7 +1230,22 @@ fn build_get_items_endpoint(
|
||||
.collect();
|
||||
endpoint.push_str(&format!("&IncludeItemTypes={}", encoded.join(",")));
|
||||
}
|
||||
if let Some(sort_by) = &opts.sort_by {
|
||||
// An explicit sort always wins; the container's default only fills the
|
||||
// gap when the caller named none. A caller that names neither gets no
|
||||
// SortBy at all, leaving the server's own order intact.
|
||||
//
|
||||
// TRACES: UR-007 | DR-257 | UT-229
|
||||
let default_sort = default_listing_sort(opts.parent_kind);
|
||||
let sort_by = opts
|
||||
.sort_by
|
||||
.as_deref()
|
||||
.or(default_sort.map(|(field, _)| field));
|
||||
let sort_order = opts
|
||||
.sort_order
|
||||
.as_deref()
|
||||
.or(default_sort.map(|(_, order)| order));
|
||||
|
||||
if let Some(sort_by) = sort_by {
|
||||
// SortBy is likewise a comma-delimited list (`hybrid.rs` sends
|
||||
// "ParentIndexNumber,IndexNumber,SortName"), so encode per field.
|
||||
let encoded: Vec<String> = sort_by
|
||||
@@ -1239,7 +1254,7 @@ fn build_get_items_endpoint(
|
||||
.collect();
|
||||
endpoint.push_str(&format!("&SortBy={}", encoded.join(",")));
|
||||
}
|
||||
if let Some(sort_order) = &opts.sort_order {
|
||||
if let Some(sort_order) = sort_order {
|
||||
endpoint.push_str(&format!("&SortOrder={}", urlencoding::encode(sort_order)));
|
||||
}
|
||||
if let Some(recursive) = opts.recursive {
|
||||
@@ -2988,6 +3003,7 @@ impl MediaRepository for OnlineRepository {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::domain::MediaKind;
|
||||
use crate::utils::lock::MutexSafe;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -3987,6 +4003,70 @@ mod tests {
|
||||
assert!(endpoint.contains("ParentId=lib-1"), "{endpoint}");
|
||||
}
|
||||
|
||||
/// The reported bug: a Jellypod podcast listed its episodes alphabetically,
|
||||
/// so "[Played] …" titles clumped at the top and a new episode landed
|
||||
/// wherever its name happened to fall.
|
||||
///
|
||||
/// The cause was the frontend asking for `SortBy=SortName` on *every*
|
||||
/// drill-down, which overrides the order the channel plugin itself would
|
||||
/// have returned. Which order a container's children take is domain
|
||||
/// knowledge, so the caller now names the container and the repository
|
||||
/// answers with the sort: a channel folder is release-date-newest-first,
|
||||
/// everything else keeps the name order it had.
|
||||
///
|
||||
/// TRACES: UR-007 | DR-257 | UT-229
|
||||
#[test]
|
||||
fn test_get_items_endpoint_orders_channel_folders_by_release_date() {
|
||||
let podcast = build_get_items_endpoint(
|
||||
"u1",
|
||||
"podcast-1",
|
||||
Some(&GetItemsOptions {
|
||||
parent_kind: Some(MediaKind::ChannelFolder),
|
||||
..Default::default()
|
||||
}),
|
||||
);
|
||||
assert!(
|
||||
podcast.contains("&SortBy=PremiereDate&SortOrder=Descending"),
|
||||
"{podcast}"
|
||||
);
|
||||
|
||||
// Every other container keeps the name order the app has always used.
|
||||
let season = build_get_items_endpoint(
|
||||
"u1",
|
||||
"season-1",
|
||||
Some(&GetItemsOptions {
|
||||
parent_kind: Some(MediaKind::Season),
|
||||
..Default::default()
|
||||
}),
|
||||
);
|
||||
assert!(
|
||||
season.contains("&SortBy=SortName&SortOrder=Ascending"),
|
||||
"{season}"
|
||||
);
|
||||
|
||||
// An explicit sort still wins — the default only fills a gap.
|
||||
let explicit = build_get_items_endpoint(
|
||||
"u1",
|
||||
"podcast-1",
|
||||
Some(&GetItemsOptions {
|
||||
parent_kind: Some(MediaKind::ChannelFolder),
|
||||
sort_by: Some("SortName".to_string()),
|
||||
sort_order: Some("Ascending".to_string()),
|
||||
..Default::default()
|
||||
}),
|
||||
);
|
||||
assert!(
|
||||
explicit.contains("&SortBy=SortName&SortOrder=Ascending"),
|
||||
"{explicit}"
|
||||
);
|
||||
assert!(!explicit.contains("SortBy=PremiereDate"), "{explicit}");
|
||||
|
||||
// A caller that names no container is left alone, so the paths that
|
||||
// rely on the server's own order (a playlist's stored order) keep it.
|
||||
let unspecified = build_get_items_endpoint("u1", "lib-1", None);
|
||||
assert!(!unspecified.contains("SortBy="), "{unspecified}");
|
||||
}
|
||||
|
||||
/// A newly-added album must arrive as one entry, not one per track.
|
||||
///
|
||||
/// Jellyfin's `/Items/Latest` defaults to `GroupItems=false`, which returns
|
||||
|
||||
@@ -342,6 +342,36 @@ pub struct GetItemsOptions {
|
||||
/// TRACES: UR-067 | DR-116 | UT-104
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub favorites_only: Option<bool>,
|
||||
/// What the container being listed *is*, so the repository can pick the
|
||||
/// order its children belong in when the caller names none. The frontend
|
||||
/// sends the neutral kind it already holds; what that kind implies about
|
||||
/// ordering is decided here, the same division as `SearchScope`.
|
||||
///
|
||||
/// TRACES: UR-007 | DR-257
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub parent_kind: Option<crate::domain::MediaKind>,
|
||||
}
|
||||
|
||||
/// The order a container's children take when the caller asked for none.
|
||||
///
|
||||
/// Ordering by *name* is right for a library, a series or an album, and wrong
|
||||
/// for a channel folder: plugin channels — a podcast feed, say — carry a
|
||||
/// release date and are read newest-first, and Jellypod additionally prefixes
|
||||
/// played episodes with "[Played]", so a name sort clumped every heard episode
|
||||
/// at the top of the list. Returns `None` when no container kind was given, so
|
||||
/// callers that deliberately rely on the server's own order keep it.
|
||||
///
|
||||
/// This mapping is domain vocabulary and lives here rather than in the
|
||||
/// frontend, for the reason in docs/specs/scoped-search-boundary.md.
|
||||
///
|
||||
/// TRACES: UR-007 | DR-257 | UT-229
|
||||
pub fn default_listing_sort(
|
||||
parent_kind: Option<crate::domain::MediaKind>,
|
||||
) -> Option<(&'static str, &'static str)> {
|
||||
match parent_kind? {
|
||||
crate::domain::MediaKind::ChannelFolder => Some(("PremiereDate", "Descending")),
|
||||
_ => Some(("SortName", "Ascending")),
|
||||
}
|
||||
}
|
||||
|
||||
/// An opaque search scope the frontend selects; Rust owns what it *means*.
|
||||
|
||||
Reference in New Issue
Block a user