move drag/drop deck logic to backend

pylib/anki/decks.py

@@ -39,6 +39,8 @@ DeckConfig = Union[FilteredDeck, Config]
 """ New/lrn/rev conf, from deck config"""
 QueueConfig = Dict[str, Any]
 
+DeckID = int
+
 
 class DecksDictProxy:
     def __init__(self, col: anki.collection.Collection):
@@ -260,43 +262,22 @@ class DeckManager:
     # Drag/drop
     #############################################################
 
+    def drag_drop_decks(self, source_decks: List[DeckID], target_deck: DeckID) -> None:
+        """Rename one or more source decks that were dropped on `target_deck`.
+        If target_deck is 0, decks will be placed at the top level."""
+        self.col.backend.drag_drop_decks(
+            source_deck_ids=source_decks, target_deck_id=target_deck
+        )
+
+    # legacy
     def renameForDragAndDrop(
-        self, draggedDeckDid: int, ontoDeckDid: Optional[Union[int, str]]
+        self, draggedDeckDid: Union[int, str], ontoDeckDid: Optional[Union[int, str]]
     ) -> None:
-        draggedDeck = self.get(draggedDeckDid)
+        if not ontoDeckDid:
-        draggedDeckName = draggedDeck["name"]
+            onto = 0
-        ontoDeckName = self.get(ontoDeckDid)["name"]
-
-        if ontoDeckDid is None or ontoDeckDid == "":
-            if len(self.path(draggedDeckName)) > 1:
-                self.rename(draggedDeck, self.basename(draggedDeckName))
-        elif self._canDragAndDrop(draggedDeckName, ontoDeckName):
-            draggedDeck = self.get(draggedDeckDid)
-            draggedDeckName = draggedDeck["name"]
-            ontoDeckName = self.get(ontoDeckDid)["name"]
-            assert ontoDeckName.strip()
-            self.rename(
-                draggedDeck, ontoDeckName + "::" + self.basename(draggedDeckName)
-            )
-
-    def _canDragAndDrop(self, draggedDeckName: str, ontoDeckName: str) -> bool:
-        if (
-            draggedDeckName == ontoDeckName
-            or self._isParent(ontoDeckName, draggedDeckName)
-            or self._isAncestor(draggedDeckName, ontoDeckName)
-        ):
-            return False
         else:
-            return True
+            onto = int(ontoDeckDid)
-
+        self.drag_drop_decks([int(draggedDeckDid)], onto)
-    def _isParent(self, parentDeckName: str, childDeckName: str) -> bool:
-        return self.path(childDeckName) == self.path(parentDeckName) + [
-            self.basename(childDeckName)
-        ]
-
-    def _isAncestor(self, ancestorDeckName: str, descendantDeckName: str) -> bool:
-        ancestorPath = self.path(ancestorDeckName)
-        return ancestorPath == self.path(descendantDeckName)[0 : len(ancestorPath)]
 
     # Deck configurations
     #############################################################

pylib/tests/test_decks.py

@@ -106,15 +106,15 @@ def test_renameForDragAndDrop():
     col.decks.renameForDragAndDrop(chinese_did, languages_did)
     assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"]
 
-    # Dragging a col onto itself is a no-op
+    # Dragging a deck onto itself is a no-op
     col.decks.renameForDragAndDrop(languages_did, languages_did)
     assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"]
 
-    # Dragging a col onto its parent is a no-op
+    # Dragging a deck onto its parent is a no-op
     col.decks.renameForDragAndDrop(hsk_did, chinese_did)
     assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"]
 
-    # Dragging a col onto a descendant is a no-op
+    # Dragging a deck onto a descendant is a no-op
     col.decks.renameForDragAndDrop(languages_did, hsk_did)
     assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"]
 
@@ -122,11 +122,11 @@ def test_renameForDragAndDrop():
     col.decks.renameForDragAndDrop(hsk_did, languages_did)
     assert deckNames() == ["Languages", "Languages::Chinese", "Languages::HSK"]
 
-    # Can drag a col onto its sibling
+    # Can drag a deck onto its sibling
     col.decks.renameForDragAndDrop(hsk_did, chinese_did)
     assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"]
 
-    # Can drag a col back to the top level
+    # Can drag a deck back to the top level
     col.decks.renameForDragAndDrop(chinese_did, None)
     assert deckNames() == ["Chinese", "Chinese::HSK", "Languages"]
 

qt/aqt/deckbrowser.py

@@ -6,6 +6,7 @@ from __future__ import annotations
 from concurrent.futures import Future
 from copy import deepcopy
 from dataclasses import dataclass
+from typing import Any
 
 import aqt
 from anki.errors import DeckRenameError
@@ -63,7 +64,7 @@ class DeckBrowser:
     # Event handlers
     ##########################################################################
 
-    def _linkHandler(self, url):
+    def _linkHandler(self, url: str) -> Any:
         if ":" in url:
             (cmd, arg) = url.split(":", 1)
         else:
@@ -83,8 +84,8 @@ class DeckBrowser:
                 gui_hooks.sidebar_should_refresh_decks()
                 self.refresh()
         elif cmd == "drag":
-            draggedDeckDid, ontoDeckDid = arg.split(",")
+            source, target = arg.split(",")
-            self._dragDeckOnto(draggedDeckDid, ontoDeckDid)
+            self._handle_drag_and_drop(int(source), int(target or 0))
         elif cmd == "collapse":
             self._collapse(int(arg))
         return False
@@ -271,13 +272,9 @@ class DeckBrowser:
             node.collapsed = not node.collapsed
         self._renderPage(reuse=True)
 
-    def _dragDeckOnto(self, draggedDeckDid: int, ontoDeckDid: int):
+    def _handle_drag_and_drop(self, source: int, target: int) -> None:
-        try:
+        self.mw.col.decks.drag_drop_decks([source], target)
-            self.mw.col.decks.renameForDragAndDrop(draggedDeckDid, ontoDeckDid)
+        gui_hooks.sidebar_should_refresh_decks()
-            gui_hooks.sidebar_should_refresh_decks()
-        except DeckRenameError as e:
-            return showWarning(e.description)
-
         self.show()
 
     def ask_delete_deck(self, did: int) -> bool:

rslib/backend.proto

@@ -138,6 +138,7 @@ service BackendService {
   rpc GetDeckNames(GetDeckNamesIn) returns (DeckNames);
   rpc NewDeckLegacy(Bool) returns (Json);
   rpc RemoveDeck(DeckID) returns (Empty);
+  rpc DragDropDecks(DragDropDecksIn) returns (Empty);
 
   // deck config
 
@@ -991,6 +992,11 @@ message GetDeckNamesIn {
   bool include_filtered = 2;
 }
 
+message DragDropDecksIn {
+  repeated int64 source_deck_ids = 1;
+  int64 target_deck_id = 2;
+}
+
 message NoteIsDuplicateOrEmptyOut {
   enum State {
     NORMAL = 0;

rslib/src/backend/mod.rs

@@ -792,6 +792,17 @@ impl BackendService for Backend {
             .map(Into::into)
     }
 
+    fn drag_drop_decks(&self, input: pb::DragDropDecksIn) -> BackendResult<Empty> {
+        let source_dids: Vec<_> = input.source_deck_ids.into_iter().map(Into::into).collect();
+        let target_did = if input.target_deck_id == 0 {
+            None
+        } else {
+            Some(input.target_deck_id.into())
+        };
+        self.with_col(|col| col.drag_drop_decks(&source_dids, target_did))
+            .map(Into::into)
+    }
+
     // deck config
     //----------------------------------------------------
 

rslib/src/decks/mod.rs

@@ -175,6 +175,26 @@ pub(crate) fn immediate_parent_name(machine_name: &str) -> Option<&str> {
     machine_name.rsplitn(2, '\x1f').nth(1)
 }
 
+/// Determine name to rename a deck to, when `dragged` is dropped on `dropped`.
+/// `dropped` being unset represents a drop at the top or bottom of the deck list.
+/// The returned name should be used to rename `dragged`, and may be unchanged.
+/// Arguments are expected in 'machine' form with an \x1f separator.
+pub(crate) fn drag_drop_deck_name(dragged: &str, dropped: Option<&str>) -> String {
+    let dragged_base = dragged.rsplit('\x1f').next().unwrap();
+    if let Some(dropped) = dropped {
+        if dropped.starts_with(dragged) {
+            // foo onto foo::bar, or foo onto itself -> no-op
+            dragged.to_string()
+        } else {
+            // foo::bar onto baz -> baz::bar
+            format!("{}\x1f{}", dropped, dragged_base)
+        }
+    } else {
+        // foo::bar onto top level -> bar
+        dragged_base.into()
+    }
+}
+
 impl Collection {
     pub(crate) fn default_deck_is_empty(&self) -> Result<bool> {
         self.storage.deck_is_empty(DeckID(1))
@@ -522,11 +542,51 @@ impl Collection {
         deck.set_modified(usn);
         self.add_or_update_single_deck(deck, usn)
     }
+
+    pub fn drag_drop_decks(
+        &mut self,
+        source_decks: &[DeckID],
+        target: Option<DeckID>,
+    ) -> Result<()> {
+        self.state.deck_cache.clear();
+        let usn = self.usn()?;
+        self.transact(None, |col| {
+            let target_deck;
+            let mut target_name = None;
+            if let Some(target) = target {
+                if let Some(target) = col.storage.get_deck(target)? {
+                    target_deck = target;
+                    target_name = Some(target_deck.name.as_str());
+                }
+            }
+
+            for source in source_decks {
+                if let Some(mut source) = col.storage.get_deck(*source)? {
+                    let orig = source.clone();
+                    let new_name = drag_drop_deck_name(&source.name, target_name);
+                    if new_name == source.name {
+                        continue;
+                    }
+                    source.name = new_name;
+                    col.ensure_deck_name_unique(&mut source, usn)?;
+                    col.rename_child_decks(&orig, &source.name, usn)?;
+                    source.set_modified(usn);
+                    col.storage.update_deck(&source)?;
+                    // after updating, we need to ensure all grandparents exist, which may not be the case
+                    // in the parent->child case
+                    col.create_missing_parents(&source.name, usn)?;
+                }
+            }
+
+            Ok(())
+        })
+    }
 }
 
 #[cfg(test)]
 mod test {
     use super::{human_deck_name_to_native, immediate_parent_name, normalize_native_name};
+    use crate::decks::drag_drop_deck_name;
     use crate::{
         collection::{open_test_collection, Collection},
         err::Result,
@@ -675,4 +735,39 @@ mod test {
 
         Ok(())
     }
+
+    #[test]
+    fn drag_drop() {
+        // use custom separator to make the tests easier to read
+        fn n(s: &str) -> String {
+            s.replace(":", "\x1f")
+        }
+        assert_eq!(drag_drop_deck_name("drag", Some("drop")), n("drop:drag"));
+        assert_eq!(&drag_drop_deck_name("drag", None), "drag");
+        assert_eq!(&drag_drop_deck_name(&n("drag:child"), None), "child");
+        assert_eq!(
+            drag_drop_deck_name(&n("drag:child"), Some(&n("drop:deck"))),
+            n("drop:deck:child")
+        );
+        assert_eq!(
+            drag_drop_deck_name(&n("drag:child"), Some("drag")),
+            n("drag:child")
+        );
+        assert_eq!(
+            drag_drop_deck_name(&n("drag:child:grandchild"), Some("drag")),
+            n("drag:grandchild")
+        );
+        // while the renaming code should be able to cope with renaming a parent to a child,
+        // it's not often useful and can be difficult for the user to clean up if done accidentally,
+        // so it should be a no-op
+        assert_eq!(
+            drag_drop_deck_name(&n("drag"), Some(&n("drag:child:grandchild"))),
+            n("drag")
+        );
+        // name doesn't change when deck dropped on itself
+        assert_eq!(
+            drag_drop_deck_name(&n("foo:bar"), Some(&n("foo:bar"))),
+            n("foo:bar")
+        );
+    }
 }