Test and refactor BaseStore.unparent

GTG/core/base_store.py

@@ -192,21 +192,22 @@ def parent(self, item_id: UUID, parent_id: UUID) -> None:
         self.emit('parent-change', item, self.lookup[parent_id])
 
 
-    def unparent(self, item_id: UUID, parent_id: UUID) -> None:
-        """Remove child item from a parent."""
+    def unparent(self, item_id: UUID) -> None:
+        """Remove child item from its parent (if exists)."""
+        if item_id not in self.lookup:
+            raise KeyError("item_id is not in store: "+str(item_id))
+        if self.lookup[item_id].parent is None:
+            return
 
-        for child in self.lookup[parent_id].children:
-            if child.id == item_id:
-                self.data.append(child)
-                self.lookup[parent_id].children.remove(child)
-                child.parent = None
+        child = self.lookup[item_id]
+        parent = child.parent
+        assert parent is not None
 
-                self.emit('parent-removed',
-                          self.lookup[item_id],
-                          self.lookup[parent_id])
-                return
+        parent.children.remove(child)
+        self.data.append(child)
+        child.parent = None
 
-        raise KeyError
+        self.emit('parent-removed',child,parent)
 
 
     # --------------------------------------------------------------------------

GTG/core/tags.py

@@ -437,17 +437,19 @@ def parent(self, item_id: UUID, parent_id: UUID) -> None:
         self.lookup[parent_id].notify('children_count')
 
 
-    def unparent(self, item_id: UUID, parent_id: UUID) -> None:
+    def unparent(self, item_id: UUID) -> None:
 
         item = self.lookup[item_id]
+        parent = item.parent
+        if parent is None:
+            return
 
         # Remove from UI
         self._remove_from_parent_model(item_id)
 
-        super().unparent(item_id, parent_id)
+        super().unparent(item_id)
 
         # Add back to UI
-        item = self.lookup[item_id]
         self.model.append(item)
 
-        self.lookup[parent_id].notify('children_count')
+        parent.notify('children_count')

GTG/core/tasks.py

@@ -1009,17 +1009,18 @@ def parent(self, item_id: UUID, parent_id: UUID) -> None:
         item.parent.notify('has_children')
 
 
-    def unparent(self, item_id: UUID, parent_id: UUID) -> None:
+    def unparent(self, item_id: UUID) -> None:
 
         item = self.lookup[item_id]
-        parent = self.lookup[parent_id]
+        parent = item.parent
+        if parent is None:
+            return
 
         # Remove from UI
         self._remove_from_parent_model(item_id)
-        assert item.parent is not None
-        item.parent.notify('has_children')
+        parent.notify('has_children')
 
-        super().unparent(item_id, parent_id)
+        super().unparent(item_id)
 
         # remove inline references to the former subtask
         parent.content = re.sub(r'\{\!\s*'+str(item_id)+r'\s*\!\}','',parent.content)

GTG/gtk/browser/main_window.py

@@ -1106,7 +1106,7 @@ def on_add_parent(self, widget=None):
 
                 for task in selection:
                     self.app.ds.tasks.refresh_lookup_cache()
-                    self.app.ds.tasks.unparent(task.id, parent.id)
+                    self.app.ds.tasks.unparent(task.id)
                     self.app.ds.tasks.parent(task.id, new_parent.id)
         else:
             new_parent = self.app.ds.tasks.new()

GTG/gtk/browser/sidebar.py

@@ -598,7 +598,7 @@ def drag_drop(self, target, value, x, y):
             return
 
         if value.parent:
-            self.ds.tags.unparent(value.id, value.parent.id)
+            self.ds.tags.unparent(value.id)
 
         self.ds.tags.parent(value.id, dropped.id)
         self.ds.refresh_tag_stats()
@@ -653,7 +653,7 @@ def notify_task(self, task: Task) -> None:
 
     def on_toplevel_tag_drop(self, drop_target, tag, x, y):
         if tag.parent:
-            self.ds.tags.unparent(tag.id, tag.parent.id)
+            self.ds.tags.unparent(tag.id)
             self.ds.refresh_tag_stats()
             self.refresh_tags()
             try:

GTG/gtk/browser/task_pane.py

@@ -621,7 +621,7 @@ def drag_drop(self, target, task, x, y):
             return
 
         if task.parent:
-            self.ds.tasks.unparent(task.id, task.parent.id)
+            self.ds.tasks.unparent(task.id)
 
         self.ds.tasks.parent(task.id, dropped.id)
         self.refresh()
@@ -654,7 +654,7 @@ def on_task_RMB_click(self, gesture, sequence) -> None:
 
     def on_toplevel_tag_drop(self, drop_target, task, x, y):
         if task.parent:
-            self.ds.tasks.unparent(task.id, task.parent.id)
+            self.ds.tasks.unparent(task.id)
             self.ds.tasks.tree_model.emit('items-changed', 0, 0, 0)
             self.refresh()
 

GTG/gtk/editor/editor.py

@@ -698,7 +698,7 @@ def new_subtask(self, title=None, tid=None):
     def remove_subtask(self, tid):
         """Remove a subtask of this task."""
 
-        self.app.ds.tasks.unparent(tid, self.task.id)
+        self.app.ds.tasks.unparent(tid)
 
     def rename_subtask(self, tid, new_title):
         """Rename a subtask of this task."""

tests/core/test_base_store.py

@@ -22,7 +22,7 @@
 from GTG.core.base_store import StoreItem, BaseStore
 
 
-class BaseStoreRemove(TestCase):
+class TestBaseStoreRemove(TestCase):
 
 
     def setUp(self):
@@ -70,7 +70,7 @@ def on_remove(obj,s):
 
 
 
-class BaseStoreParent(TestCase):
+class TestBaseStoreParent(TestCase):
 
 
     def setUp(self):
@@ -133,3 +133,45 @@ def test_invalid_parent_id_does_not_update_root_elements(self):
         except KeyError:
             pass
         self.assertEqual(self.store.count(root_only=True),2)
+
+
+
+class TestBaseStoreUnparent(TestCase):
+
+
+    def setUp(self):
+        self.store = BaseStore()
+
+        self.root = StoreItem(uuid4())
+        self.child1 = StoreItem(uuid4())
+        self.child2 = StoreItem(uuid4())
+        self.invalid_id = uuid4()
+
+        self.store.add(self.root)
+        self.store.add(self.child1,parent_id=self.root.id)
+        self.store.add(self.child2,parent_id=self.root.id)
+
+
+    def test_parent_is_unset(self):
+        self.store.unparent(self.child1.id)
+        self.assertIsNone(self.child1.parent)
+
+
+    def test_children_list_is_updated(self):
+        self.store.unparent(self.child1.id)
+        self.assertEqual(self.root.children,[self.child2])
+
+
+    def test_list_of_roots_is_updated(self):
+        self.store.unparent(self.child2.id)
+        self.assertIn(self.child2,self.store.data)
+
+
+    def test_invalid_item_id_raises_exception(self):
+        with self.assertRaises(KeyError):
+            self.store.unparent(self.invalid_id)
+
+
+    def test_unparenting_root_element_has_no_effect(self):
+        self.store.unparent(self.root.id)
+        self.assertEqual(self.store.data,[self.root])

tests/core/test_task.py

@@ -298,7 +298,7 @@ def test_parenting(self):
         self.assertEqual(child_task.parent, root_task)
         self.assertEqual(root_task.children[0], child_task)
 
-        store.unparent(child_task.id, root_task.id)
+        store.unparent(child_task.id)
 
         self.assertEqual(store.count(), 2)
         self.assertEqual(store.count(root_only=True), 2)
@@ -317,7 +317,7 @@ def test_parenting(self):
         self.assertEqual(child_task.parent, root_task)
         self.assertEqual(inner_child_task.parent, child_task)
 
-        store.unparent(inner_child_task.id, inner_child_task.parent.id)
+        store.unparent(inner_child_task.id)
         self.assertEqual(inner_child_task.parent, None)
         self.assertEqual(len(child_task.children), 0)