Remove parent_id argument from BaseStore.unparent

- remove the parent_id argument and adjust the code accordingly
- adjust the test cases
- (rename some test classes to keep things consistent)

GTG/core/base_store.py

@@ -192,23 +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 parent_id not in self.lookup:
-            raise KeyError("parent_id is not in store: "+str(parent_id))
-        if self.lookup[item_id].parent != self.lookup[parent_id]:
-            raise ValueError("non-existing parent-child relationship")
+        if self.lookup[item_id].parent is None:
+            return
 
         child = self.lookup[item_id]
-        parent = self.lookup[parent_id]
+        parent = child.parent
+        assert parent is not None
 
         parent.children.remove(child)
         self.data.append(child)
         child.parent = None
 
-        self.emit('parent-removed',self.lookup[item_id],self.lookup[parent_id])
+        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):
@@ -136,7 +136,7 @@ def test_invalid_parent_id_does_not_update_root_elements(self):
 
 
 
-class BaseStoreUnparent(TestCase):
+class TestBaseStoreUnparent(TestCase):
 
 
     def setUp(self):
@@ -153,30 +153,25 @@ def setUp(self):
 
 
     def test_parent_is_unset(self):
-        self.store.unparent(self.child1.id,self.root.id)
+        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.root.id)
+        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.root.id)
+        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,self.root.id)
+            self.store.unparent(self.invalid_id)
 
 
-    def test_invalid_parent_id_raises_exception(self):
-        with self.assertRaises(KeyError):
-            self.store.unparent(self.child1.id,self.invalid_id)
-
-
-    def test_invalid_parent_child_combo_raises_exception(self):
-        with self.assertRaises(ValueError):
-            self.store.unparent(self.child1.id,self.child2.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)