diff --git a/core/dr-catalog/src/collections.rs b/core/dr-catalog/src/collections.rs index bdcbe69..b34105a 100644 --- a/core/dr-catalog/src/collections.rs +++ b/core/dr-catalog/src/collections.rs @@ -376,6 +376,64 @@ pub fn set_order( touch(conn, id) } +/// Whether this collection has a manual order worth reading. +/// +/// The grid asks before choosing an `ORDER BY`, and the answer is narrower +/// than "is it manual". Two things have to be true. +/// +/// It must be **manual**: a saved filter's contents are whatever its selector +/// matches now, in whatever order the query returns them, and there are no +/// member rows to carry a position. +/// +/// And it must have **no children**. A parent's contents are the union of its +/// own members and its descendants' ([`descendants`]), and positions are only +/// ever assigned within one collection — so two children's positions are +/// unrelated integers, and interleaving them by value would order the grid by +/// a coincidence. Capture time is the honest answer for a set. +/// +/// Kept here rather than assembled at the call site, so the rule has one name +/// and one test, and the grid does not have to know that "no children" is part +/// of it. +pub fn orders_manually(conn: &Connection, id: CollectionId) -> Result { + if kind_of(conn, id)? != CollectionKind::Manual { + return Ok(false); + } + + let has_child: Option = conn + .query_row( + "SELECT 1 FROM collections + WHERE parent_id = ?1 AND deleted = 0 LIMIT 1", + [id.0 as i64], + |r| r.get(0), + ) + .optional()?; + + Ok(has_child.is_none()) +} + +/// Every image in a manual collection, in manual order. +/// +/// The whole membership, not the filtered view the grid happens to be showing. +/// [`set_order`] renumbers exactly the images it is handed, so a caller that +/// reordered a filtered list would renumber those and leave every hidden image +/// on a stale position — two images sharing a position, and a grid that +/// rearranges itself when the filter comes off. +/// +/// `image_id` breaks ties. Positions are dense in practice, but a merge can +/// deliver two devices' rows at the same position, and an order that is not +/// total is one the grid draws differently each time it reads it. +pub fn members_in_order(conn: &Connection, id: CollectionId) -> Result, CatalogError> { + let mut stmt = conn.prepare( + "SELECT image_id FROM collection_members + WHERE collection_id = ?1 + ORDER BY position, image_id", + )?; + let rows = stmt + .query_map([id.0 as i64], |r| Ok(ImageId(r.get::<_, i64>(0)? as u64)))? + .collect::, _>>()?; + Ok(rows) +} + /// Save a smart collection's selector. /// /// Refuses a selector that references this collection, directly or through @@ -789,6 +847,106 @@ mod tests { ImageId(i) } + #[test] + fn a_manual_leaf_orders_manually() { + let cat = seeded(); + let c = cat.connection(); + let id = create(c, "Trip", None, CollectionKind::Manual).unwrap(); + assert!(orders_manually(c, id).unwrap()); + } + + #[test] + fn a_saved_filter_has_no_manual_order() { + let cat = seeded(); + let c = cat.connection(); + let id = create(c, "Picks", None, CollectionKind::Smart).unwrap(); + assert!( + !orders_manually(c, id).unwrap(), + "a smart collection has no member rows to carry a position" + ); + } + + #[test] + fn a_parent_has_no_manual_order_of_its_own() { + let cat = seeded(); + let c = cat.connection(); + let parent = create(c, "2026", None, CollectionKind::Manual).unwrap(); + let child = create(c, "Iceland", Some(parent), CollectionKind::Manual).unwrap(); + + assert!( + !orders_manually(c, parent).unwrap(), + "a set shows its descendants' images, whose positions are unrelated" + ); + assert!( + orders_manually(c, child).unwrap(), + "the child is still a leaf and still orders manually" + ); + } + + #[test] + fn deleting_the_last_child_gives_the_parent_its_order_back() { + let cat = seeded(); + let c = cat.connection(); + let parent = create(c, "2026", None, CollectionKind::Manual).unwrap(); + let child = create(c, "Iceland", Some(parent), CollectionKind::Manual).unwrap(); + assert!(!orders_manually(c, parent).unwrap()); + + delete(c, child).unwrap(); + assert!( + orders_manually(c, parent).unwrap(), + "a tombstoned child must not keep counting as a child" + ); + } + + #[test] + fn members_come_back_in_the_order_they_were_put_in() { + let cat = seeded(); + let c = cat.connection(); + let id = create(c, "Trip", None, CollectionKind::Manual).unwrap(); + add_images(c, id, &[img(3), img(1), img(2)]).unwrap(); + + assert_eq!( + members_in_order(c, id).unwrap(), + vec![img(3), img(1), img(2)], + "adding assigns position in the order given, and reading returns it" + ); + } + + #[test] + fn members_come_back_in_the_order_set_order_wrote() { + let cat = seeded(); + let c = cat.connection(); + let id = create(c, "Trip", None, CollectionKind::Manual).unwrap(); + add_images(c, id, &[img(1), img(2), img(3)]).unwrap(); + + set_order(c, id, &[img(3), img(2), img(1)]).unwrap(); + assert_eq!( + members_in_order(c, id).unwrap(), + vec![img(3), img(2), img(1)] + ); + } + + #[test] + fn members_in_order_is_total_even_where_positions_collide() { + let cat = seeded(); + let c = cat.connection(); + let id = create(c, "Trip", None, CollectionKind::Manual).unwrap(); + add_images(c, id, &[img(1), img(2), img(3)]).unwrap(); + + // What a merge can deliver: two devices' rows landing on one position. + c.execute( + "UPDATE collection_members SET position = 0 WHERE collection_id = ?1", + [id.0 as i64], + ) + .unwrap(); + + assert_eq!( + members_in_order(c, id).unwrap(), + vec![img(1), img(2), img(3)], + "image_id breaks the tie, so two reads cannot disagree" + ); + } + #[test] fn a_created_collection_appears_in_the_tree() { let cat = seeded(); diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index 70a2b3a..dbc77c8 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -2245,6 +2245,23 @@ fn load_window(window: &AppWindow, ctl: &Rc) { refresh_timeline(window, catalog, ctl); } + // TRACES: FR-CAT-7 + // Whether this scope has an order a drag can change, which is what lets the + // grid draw an insertion caret and accept a drop. + // + // Answered here because this is the one place that runs on every re-read — + // a scope change, a reorder, a collection gaining a child that turns it + // into a set — so the caret cannot outlive the scope that justified it. + // + // The trash is never reorderable. It is a view of what was deleted, ordered + // by when, and it is not a collection at all. + window.set_library_manual_order( + !trash + && scope.is_some_and(|c| { + dr_catalog::collections::orders_manually(catalog.connection(), c).unwrap_or(false) + }), + ); + let cells = if trash { library::read_trashed_cells(catalog, offset, window_size) } else {