Merge pull request 'fix(memories): sort Month span by time within a day; test the real sorts' (#156) from fix/memories-month-sort into master

Reviewed-on: #156

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 {
+    sort_memories(&mut memories_with_dates, 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,
-            });
-        }
-    }
 
     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::*;
@@ -1094,105 +1093,127 @@ mod tests {
         assert!(!excluder.is_excluded(&keep));
     }
 
+    fn memory_item(path: &str, created: Option<i64>, library_id: i32) -> MemoryItem {
+        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,
+        }
+    }
+
+    fn utc_ts(y: i32, m: u32, d: u32, h: u32) -> i64 {
+        NaiveDate::from_ymd_opt(y, m, d)
+            .unwrap()
+            .and_hms_opt(h, 0, 0)
+            .unwrap()
+            .and_utc()
+            .timestamp()
+    }
+
     #[test]
-    fn test_week_span_sorting_chronological_by_day() {
+    fn week_span_sorts_by_full_date_then_time() {
-        // Test that Week span sorts by:
+        // Week is chronological (7d2a314): full date first, so years group
-        // 1. Day of month (ascending)
+        // together, then time within a day.
-        // 2. Full timestamp oldest to newest (year + time combined)
+        let jan = |y, d| NaiveDate::from_ymd_opt(y, 1, d).unwrap();
-
+        let mut memories = vec![
-        // Create test data:
-        // - Jan 15, 2024 at 9:00 AM
-        // - Jan 15, 2020 at 10:00 AM
-        // - Jan 16, 2021 at 8:00 AM
-
-        let jan_15_2024_9am = NaiveDate::from_ymd_opt(2024, 1, 15)
-            .unwrap()
-            .and_hms_opt(9, 0, 0)
-            .unwrap()
-            .and_utc()
-            .timestamp();
-
-        let jan_15_2020_10am = NaiveDate::from_ymd_opt(2020, 1, 15)
-            .unwrap()
-            .and_hms_opt(10, 0, 0)
-            .unwrap()
-            .and_utc()
-            .timestamp();
-
-        let jan_16_2021_8am = NaiveDate::from_ymd_opt(2021, 1, 16)
-            .unwrap()
-            .and_hms_opt(8, 0, 0)
-            .unwrap()
-            .and_utc()
-            .timestamp();
-
-        let mut memories_with_dates = [
             (
-                MemoryItem {
+                memory_item("2024-01-15.jpg", Some(utc_ts(2024, 1, 15, 9)), 1),
-                    path: "photo1.jpg".to_string(),
+                jan(2024, 15),
-                    created: Some(jan_15_2024_9am),
-                    modified: Some(jan_15_2024_9am),
-                    library_id: 1,
-                    date_utc: None,
-                    camera_offset_minutes: None,
-                    date_taken_tz_source: None,
-                    wall_clock: None,
-                },
-                NaiveDate::from_ymd_opt(2024, 1, 15).unwrap(),
             ),
             (
-                MemoryItem {
+                memory_item("2020-01-15-10h.jpg", Some(utc_ts(2020, 1, 15, 10)), 1),
-                    path: "photo2.jpg".to_string(),
+                jan(2020, 15),
-                    created: Some(jan_15_2020_10am),
-                    modified: Some(jan_15_2020_10am),
-                    library_id: 1,
-                    date_utc: None,
-                    camera_offset_minutes: None,
-                    date_taken_tz_source: None,
-                    wall_clock: None,
-                },
-                NaiveDate::from_ymd_opt(2020, 1, 15).unwrap(),
             ),
             (
-                MemoryItem {
+                memory_item("2021-01-16.jpg", Some(utc_ts(2021, 1, 16, 8)), 2),
-                    path: "photo3.jpg".to_string(),
+                jan(2021, 16),
-                    created: Some(jan_16_2021_8am),
+            ),
-                    modified: Some(jan_16_2021_8am),
+            (
-                    library_id: 1,
+                memory_item("2020-01-15-07h.jpg", Some(utc_ts(2020, 1, 15, 7)), 2),
-                    date_utc: None,
+                jan(2020, 15),
-                    camera_offset_minutes: None,
-                    date_taken_tz_source: None,
-                    wall_clock: None,
-                },
-                NaiveDate::from_ymd_opt(2021, 1, 16).unwrap(),
             ),
         ];
 
-        // Sort using Week span logic
+        sort_memories(&mut memories, MemoriesSpan::Week);
-        memories_with_dates.sort_by(|a, b| {
-            // First, sort by day of month
-            let day_cmp = a.1.day().cmp(&b.1.day());
-            if day_cmp != std::cmp::Ordering::Equal {
-                return day_cmp;
-            }
 
-            // Then sort by full created timestamp (oldest to newest)
+        let paths: Vec<&str> = memories.iter().map(|(m, _)| m.path.as_str()).collect();
-            match (a.0.created, b.0.created) {
+        assert_eq!(
-                (Some(a_time), Some(b_time)) => a_time.cmp(&b_time),
+            paths,
-                (Some(_), None) => std::cmp::Ordering::Less,
+            vec![
-                (None, Some(_)) => std::cmp::Ordering::Greater,
+                "2020-01-15-07h.jpg",
-                (None, None) => std::cmp::Ordering::Equal,
+                "2020-01-15-10h.jpg",
-            }
+                "2021-01-16.jpg",
-        });
+                "2024-01-15.jpg"
+            ]
+        );
+    }
 
-        // Expected order:
+    #[test]
-        // 1. Jan 15, 2020 at 10:00 AM (oldest Jan 15 photo)
+    fn day_span_sorts_by_timestamp_across_years() {
-        // 2. Jan 15, 2024 at 9:00 AM (newer Jan 15 photo)
+        // Day: the same calendar day across years, oldest first; undated last.
-        // 3. Jan 16, 2021 at 8:00 AM (all Jan 16 photos after Jan 15)
+        let d = |y| NaiveDate::from_ymd_opt(y, 9, 29).unwrap();
+        let mut memories = vec![
+            (
+                memory_item("2022.jpg", Some(utc_ts(2022, 9, 29, 8)), 1),
+                d(2022),
+            ),
+            (memory_item("undated.jpg", None, 1), d(2022)),
+            (
+                memory_item("2018-18h.jpg", Some(utc_ts(2018, 9, 29, 18)), 2),
+                d(2018),
+            ),
+            (
+                memory_item("2018-06h.jpg", Some(utc_ts(2018, 9, 29, 6)), 1),
+                d(2018),
+            ),
+        ];
 
-        assert_eq!(memories_with_dates[0].0.created.unwrap(), jan_15_2020_10am);
+        sort_memories(&mut memories, MemoriesSpan::Day);
-        assert_eq!(memories_with_dates[1].0.created.unwrap(), jan_15_2024_9am);
+
-        assert_eq!(memories_with_dates[2].0.created.unwrap(), jan_16_2021_8am);
+        let paths: Vec<&str> = memories.iter().map(|(m, _)| m.path.as_str()).collect();
+        assert_eq!(
+            paths,
+            vec!["2018-06h.jpg", "2018-18h.jpg", "2022.jpg", "undated.jpg"]
+        );
+    }
+
+    #[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| utc_ts(2019, 9, 14, h);
+        let item = memory_item;
+        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) ----