Make the drop decision a rule that can be tested

The gesture is Slint's and cannot be driven from a test — synthetic drags
do not reach a DragArea at all — but the decision it leads to is where
this can actually go wrong, and it was buried in a callback.

`decide_drop` names the three outcomes and the order that separates them:
an image drag always fills the payload, so photographs pressed after a
row was clicked are still filed rather than read as a rearrangement. A row
dropped on itself is a no-op here rather than a cycle error from the
catalog, and an empty drag with nothing remembered — a file from another
application — leaves the tree alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ui/dr-ui/src/collections_ui.rs

@@ -187,6 +187,40 @@ pub struct CollectionsController {
     activity: Rc<crate::activity::ActivityLog>,
 }
 
+/// What a release over a collection row means.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum Drop {
+    /// File these photographs in the target.
+    FileImages(Vec<ImageId>),
+    /// Move this collection under the target.
+    Reparent(CollectionId),
+    /// Nothing to do — an empty drag, or a row dropped on itself.
+    Nothing,
+}
+
+/// Decide what a drop on `target` should do.
+///
+/// Separated from the callback because the gesture cannot be driven from a
+/// test — Slint owns it — while this decision is where it can actually go
+/// wrong. The order matters: an image drag always fills `carried`, so
+/// photographs can never be mistaken for a rearrangement, and only an empty
+/// payload consults the row remembered from the press.
+pub fn decide_drop(
+    carried: &[ImageId],
+    pressed_row: Option<CollectionId>,
+    target: CollectionId,
+) -> Drop {
+    if !carried.is_empty() {
+        return Drop::FileImages(carried.to_vec());
+    }
+    match pressed_row {
+        // A collection cannot go inside itself. Caught here as well as in the
+        // catalog so the common case is a no-op rather than an error message.
+        Some(source) if source != target => Drop::Reparent(source),
+        _ => Drop::Nothing,
+    }
+}
+
 impl CollectionsController {
     pub fn new(activity: Rc<crate::activity::ActivityLog>) -> Rc<Self> {
         Rc::new(Self {
@@ -1601,17 +1635,14 @@ pub fn wire<S, R, C>(
             let Some(w) = weak.upgrade() else { return };
             let id = CollectionId(id as u64);
             let carried = ctl.dragging.borrow().clone();
+            // Taken, not read: a remembered press must be spent by the drop it
+            // belongs to, or a later empty drop — a file dragged in from
+            // outside, say — would move a collection nobody touched.
+            let pressed = ctl.dragging_collection.borrow_mut().take();
 
-            // An empty image payload with a remembered source row means the
-            // tree is being rearranged, not filed into. Checked in this order
-            // because an image drag always fills `dragging`, so photographs can
-            // never be mistaken for a reparent.
-            if carried.is_empty() {
-                // Taken, not read: a remembered press must be spent by the drop
-                // it belongs to, or a later empty drop — a file dragged in from
-                // outside, say — would move a collection nobody touched.
-                let source = ctl.dragging_collection.borrow_mut().take();
-                if let Some(source) = source.filter(|s| *s != id) {
+            let carried = match decide_drop(&carried, pressed, id) {
+                Drop::Nothing => return,
+                Drop::Reparent(source) => {
                     let result = {
                         let borrow = catalog.borrow();
                         let Some(cat) = borrow.as_ref() else { return };
@@ -1631,9 +1662,10 @@ pub fn wire<S, R, C>(
                         // silently does nothing.
                         Err(e) => w.set_collection_error(format!("moving collection: {e}").into()),
                     }
+                    return;
                 }
-                return;
-            }
+                Drop::FileImages(images) => images,
+            };
 
             // Scoped so the borrow is released before the spring cleanup below,
             // which needs the catalog itself.
@@ -2288,6 +2320,44 @@ mod tests {
         (1..=n).map(ImageId).collect()
     }
 
+    #[test]
+    fn dragging_a_collection_onto_another_moves_it() {
+        // The gesture the tree rearrangement exists for: no images carried, a
+        // row remembered from the press, dropped somewhere else.
+        assert_eq!(
+            decide_drop(&[], Some(CollectionId(7)), CollectionId(9)),
+            Drop::Reparent(CollectionId(7))
+        );
+    }
+
+    #[test]
+    fn photographs_are_filed_even_when_a_row_was_pressed_first() {
+        // Clicking a collection and then dragging photographs into another must
+        // file them, not move the collection that happens to be remembered.
+        // This is the ordering the whole discrimination rests on.
+        assert_eq!(
+            decide_drop(&ids(3), Some(CollectionId(7)), CollectionId(9)),
+            Drop::FileImages(ids(3))
+        );
+    }
+
+    #[test]
+    fn a_collection_dropped_on_itself_does_nothing() {
+        // The catalog would refuse it as a cycle; catching it here makes the
+        // commonest slip a no-op rather than an error the user has to read.
+        assert_eq!(
+            decide_drop(&[], Some(CollectionId(7)), CollectionId(7)),
+            Drop::Nothing
+        );
+    }
+
+    #[test]
+    fn an_empty_drag_with_nothing_remembered_moves_nothing() {
+        // A file dragged in from another application lands here with no images
+        // and no pressed row. It must not disturb the tree.
+        assert_eq!(decide_drop(&[], None, CollectionId(9)), Drop::Nothing);
+    }
+
     #[test]
     fn a_plain_press_replaces_the_selection() {
         let all = ids(5);