diff --git a/src/memories.rs b/src/memories.rs index becd537..9e437c5 100644 --- a/src/memories.rs +++ b/src/memories.rs @@ -660,35 +660,34 @@ pub fn gather_memory_items( // Sort once over the merged result set. The SQL filter handles the // matching; sort order is purely UI concern. - match span_mode { - // Month: chronological — gives an "overview" feel. - MemoriesSpan::Month => memories_with_dates.sort_by(|a, b| a.1.cmp(&b.1)), - // Week: full date then timestamp (oldest → newest). - MemoriesSpan::Week => { - memories_with_dates.sort_by(|a, b| { - a.1.cmp(&b.1) - .then_with(|| match (a.0.created, b.0.created) { - (Some(at), Some(bt)) => at.cmp(&bt), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - }) - }); - } - // Day: same calendar day across years, sub-sorted by timestamp. - MemoriesSpan::Day => { - memories_with_dates.sort_by(|a, b| match (a.0.created, b.0.created) { - (Some(at), Some(bt)) => at.cmp(&bt), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - }); - } - } + sort_memories(&mut memories_with_dates, span_mode); Ok(memories_with_dates.into_iter().map(|(m, _)| m).collect()) } +/// Order `(item, display_day)` pairs for the given span, oldest first. +/// +/// Rows arrive in DB order, merged across libraries, so every span needs a +/// timestamp key within a day — without one, a day's photos keep that +/// arbitrary order and their times jump back and forth. +fn sort_memories(memories: &mut [(MemoryItem, NaiveDate)], span: MemoriesSpan) { + // Timestamped items before ones without a date. + let by_created = |a: &MemoryItem, b: &MemoryItem| match (a.created, b.created) { + (Some(at), Some(bt)) => at.cmp(&bt), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + }; + match span { + // Month / Week: full date then timestamp (oldest → newest). + MemoriesSpan::Month | MemoriesSpan::Week => { + memories.sort_by(|a, b| a.1.cmp(&b.1).then_with(|| by_created(&a.0, &b.0))); + } + // Day: same calendar day across years, sorted by timestamp. + MemoriesSpan::Day => memories.sort_by(|a, b| by_created(&a.0, &b.0)), + } +} + #[cfg(test)] mod tests { use super::*; @@ -1195,6 +1194,49 @@ mod tests { assert_eq!(memories_with_dates[2].0.created.unwrap(), jan_16_2021_8am); } + #[test] + fn month_span_sorts_by_time_within_a_day() { + // Same display day, rows in DB order (merged across libraries) — + // the Month sort used to keep this order, so times jumped around. + let day = NaiveDate::from_ymd_opt(2019, 9, 14).unwrap(); + let at = |h: u32| day.and_hms_opt(h, 0, 0).unwrap().and_utc().timestamp(); + let item = |path: &str, created: Option, library_id: i32| MemoryItem { + path: path.to_string(), + created, + modified: created, + library_id, + date_utc: created, + camera_offset_minutes: None, + date_taken_tz_source: None, + wall_clock: None, + }; + let earlier_day = NaiveDate::from_ymd_opt(2019, 9, 2).unwrap(); + let mut memories = vec![ + (item("lib1-15h.jpg", Some(at(15)), 1), day), + (item("undated.jpg", None, 1), day), + (item("lib2-09h.jpg", Some(at(9)), 2), day), + (item("lib1-12h.jpg", Some(at(12)), 1), day), + ( + item("sept-2nd.jpg", Some(at(20) - 12 * 86_400), 2), + earlier_day, + ), + ]; + + sort_memories(&mut memories, MemoriesSpan::Month); + + let paths: Vec<&str> = memories.iter().map(|(m, _)| m.path.as_str()).collect(); + assert_eq!( + paths, + vec![ + "sept-2nd.jpg", + "lib2-09h.jpg", + "lib1-12h.jpg", + "lib1-15h.jpg", + "undated.jpg" + ] + ); + } + // ---- Day-matching (ticket #119) ---- #[test]