diff --git a/GTG/core/base_store.py b/GTG/core/base_store.py index 2aa12d85f..412184f7d 100644 --- a/GTG/core/base_store.py +++ b/GTG/core/base_store.py @@ -25,17 +25,52 @@ import logging from lxml.etree import _Element -from typing import Dict, List, Optional, TypeVar, Generic, Protocol +from typing import Dict, List, Optional, TypeVar, Generic +from typing_extensions import Self log = logging.getLogger(__name__) -S = TypeVar('S',bound='Storeable') +S = TypeVar('S',bound='StoreItem') + +class StoreItem(GObject.Object): + """Base class for items in BaseStore.""" + + __gtype_name__ = 'gtg_StoreItem' + + + def __init__(self,id: UUID): + self.id: UUID = id + self.parent: Optional[Self] = None + self.children: list[Self] = [] + super().__init__() + + + @GObject.Property(type=int) + def children_count(self) -> int: + """Read only property.""" + return len(self.children) + + + @GObject.Property(type=bool, default=False) + def has_children(self) -> bool: + return len(self.children) > 0 + + + def get_ancestors(self) -> List[Self]: + """Return all ancestors of this tag""" + ancestors: List[Self] = [] + here = self + while here.parent: + here = here.parent + ancestors.append(here) + return ancestors + + + def is_parentable_to(self, target) -> bool: + """Check for parenting an item to its own descendant or to itself.""" + return self != target and self not in target.get_ancestors() -class Storeable(Protocol[S]): - id: UUID - parent: Optional[S] - children: List[S] class BaseStore(GObject.Object,Generic[S]): diff --git a/GTG/core/tags.py b/GTG/core/tags.py index 8eabf1e91..7d2fa773d 100644 --- a/GTG/core/tags.py +++ b/GTG/core/tags.py @@ -29,7 +29,7 @@ from lxml.etree import Element, SubElement, _Element from typing import Dict, List, Set, Optional -from GTG.core.base_store import BaseStore +from GTG.core.base_store import BaseStore, StoreItem log = logging.getLogger(__name__) @@ -40,26 +40,23 @@ def extract_tags_from_text(text): return re.findall(r'(?:^|[\s])(@[\w\/\.\-\:\&]*\w)', text) -class Tag(GObject.Object): +class Tag(StoreItem): """A tag that can be applied to a Task.""" __gtype_name__ = 'gtg_Tag' def __init__(self, id: UUID, name: str) -> None: - self.id = id self._name = name self._icon: Optional[str] = None self._color: Optional[str] = None self.actionable = True - self.children: List[Tag] = [] - self.parent = None self._task_count_open = 0 self._task_count_actionable = 0 self._task_count_closed = 0 - super(Tag, self).__init__() + super().__init__(id) def __str__(self) -> str: @@ -80,11 +77,6 @@ def __eq__(self, other) -> bool: return self.id == other.id - @GObject.Property(type=int) - def children_count(self) -> int: - """Read only property.""" - - return len(self.children) @GObject.Property(type=str) @@ -170,16 +162,6 @@ def set_task_count_closed(self, value: int) -> None: self._task_count_closed = value - def get_ancestors(self) -> List['Tag']: - """Return all ancestors of this tag""" - ancestors: List[Tag] = [] - here = self - while here.parent: - here = here.parent - ancestors.append(here) - return ancestors - - def get_matching_tags(self) -> List['Tag']: """Return the tag with its descendants.""" matching = [self] diff --git a/GTG/core/tasks.py b/GTG/core/tasks.py index 9150ae6a1..4dc6370ef 100644 --- a/GTG/core/tasks.py +++ b/GTG/core/tasks.py @@ -32,7 +32,7 @@ from lxml.etree import Element, _Element, SubElement, CDATA -from GTG.core.base_store import BaseStore +from GTG.core.base_store import BaseStore, StoreItem from GTG.core.tags import Tag, TagStore from GTG.core.dates import Date @@ -78,19 +78,16 @@ class Filter(Enum): DEFAULT_TITLE = _('New Task') -class Task(GObject.Object): +class Task(StoreItem): """A single task.""" __gtype_name__ = 'gtg_Task' def __init__(self, id: UUID, title: str) -> None: - self.id = id self.raw_title = title.strip('\t\n') self.content = '' self.tags: Set[Tag] = set() - self.children: List[Task] = [] self.status = Status.ACTIVE - self.parent: Optional[Task] = None self._date_added = Date.no_date() self._date_due = Date.no_date() @@ -115,7 +112,7 @@ def default_duplicate_cb(t: Task): raise NotImplementedError self.duplicate_cb: Callable[[Task],Task] = default_duplicate_cb - super(Task, self).__init__() + super().__init__(id) @GObject.Property(type=bool, default=True) @@ -617,11 +614,6 @@ def set_is_active(self, value) -> None: self._is_active = value - @GObject.Property(type=bool, default=False) - def has_children(self) -> bool: - return bool(len(self.children)) - - @GObject.Property(type=str) def icons(self) -> str: icons_text = '' @@ -702,7 +694,7 @@ def __hash__(self) -> int: # STORE # ------------------------------------------------------------------------------ -class TaskStore(BaseStore): +class TaskStore(BaseStore[Task]): """A tree of tasks.""" __gtype_name__ = 'gtg_TaskStore' @@ -1013,6 +1005,7 @@ def parent(self, item_id: UUID, parent_id: UUID) -> None: # Add back to UI self._append_to_parent_model(item_id) + assert item.parent is not None item.parent.notify('has_children') @@ -1023,6 +1016,7 @@ def unparent(self, item_id: UUID, parent_id: UUID) -> None: # Remove from UI self._remove_from_parent_model(item_id) + assert item.parent is not None item.parent.notify('has_children') super().unparent(item_id, parent_id) diff --git a/GTG/gtk/browser/sidebar.py b/GTG/gtk/browser/sidebar.py index c0118a3f7..07bd3feb1 100644 --- a/GTG/gtk/browser/sidebar.py +++ b/GTG/gtk/browser/sidebar.py @@ -590,27 +590,11 @@ def drag_end(self, source, drag, unused): source.get_widget().set_opacity(1) - def check_parent(self, value, target) -> bool: - """Check for parenting a tag to its own descendant or to itself.""" - - if value == target: - return False - - item = target - while item.parent: - if item.parent == value: - return False - - item = item.parent - - return True - - def drag_drop(self, target, value, x, y): """Callback when dropping onto a target""" dropped = target.get_widget().props.tag - if not self.check_parent(value, dropped): + if not value.is_parentable_to(dropped): return if value.parent: diff --git a/GTG/gtk/browser/task_pane.py b/GTG/gtk/browser/task_pane.py index a58308b05..2be0fa05e 100644 --- a/GTG/gtk/browser/task_pane.py +++ b/GTG/gtk/browser/task_pane.py @@ -612,28 +612,12 @@ def drop_enter(self, target, x, y, user_data=None): return Gdk.DragAction.COPY - def check_parent(self, value, target) -> bool: - """Check for parenting a task to its own descendant or to itself.""" - - if value == target: - return False - - item = target - while item.parent: - if item.parent == value: - return False - - item = item.parent - - return True - - def drag_drop(self, target, task, x, y): """Callback when dropping onto a target""" dropped = target.get_widget().props.task - if not self.check_parent(task, dropped): + if not task.is_parentable_to(dropped): return if task.parent: diff --git a/docs/contributors/coding best practices.md b/docs/contributors/coding best practices.md index 251266b37..572aede70 100644 --- a/docs/contributors/coding best practices.md +++ b/docs/contributors/coding best practices.md @@ -171,7 +171,7 @@ Modules should begin with the following header (updated to the current year): ``` # ----------------------------------------------------------------------------- -# Gettings Things GNOME! - a personal organizer for the GNOME desktop +# Getting Things GNOME! - a personal organizer for the GNOME desktop # Copyright (c) 2008-2022 - the GTG contributors # # This program is free software: you can redistribute it and/or modify it under diff --git a/tests/core/test_store_item.py b/tests/core/test_store_item.py new file mode 100644 index 000000000..4d88c6224 --- /dev/null +++ b/tests/core/test_store_item.py @@ -0,0 +1,144 @@ +# ----------------------------------------------------------------------------- +# Getting Things GNOME! - a personal organizer for the GNOME desktop +# Copyright (c) 2008-2024 - the GTG contributors +# +# This program is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free Software +# Foundation, either version 3 of the License, or (at your option) any later +# version. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program. If not, see . +# ----------------------------------------------------------------------------- + +from unittest import TestCase +from uuid import uuid4 + +from GTG.core.base_store import StoreItem + + +class TestStoreItemProperties(TestCase): + + + def test_init(self): + uuid = uuid4() + item = StoreItem(uuid) + self.assertEqual(item.id, uuid) + + + def test_children_count_default_value(self): + item = StoreItem(uuid4()) + self.assertEqual(item.children_count, 0) + self.assertFalse(item.has_children) + + + def test_children_count_single_child(self): + item = StoreItem(uuid4()) + item.children.append(StoreItem(uuid4)) + self.assertEqual(item.children_count, 1) + self.assertTrue(item.has_children) + + + def test_children_count_multiple_children(self): + item = StoreItem(uuid4()) + item.children = [ StoreItem(uuid4) for _ in range(15) ] + self.assertEqual(item.children_count, 15) + self.assertTrue(item.has_children) + + + +class TestStoreItemGetAncestors(TestCase): + + + def setUp(self): + self.root = StoreItem(uuid4()) + + self.children = [ StoreItem(uuid4()) for _ in range(5) ] + self.root.children = self.children + for c in self.children: + c.parent = self.root + + self.grandchildren = [ StoreItem(uuid4()) for _ in range(7) ] + self.root.children[-1].children = self.grandchildren + for c in self.grandchildren: + c.parent = self.root.children[-1] + + self.greatgrandchildren = [ StoreItem(uuid4()) for _ in range(2) ] + self.root.children[-1].children[0].children = self.greatgrandchildren + for c in self.greatgrandchildren: + c.parent = self.root.children[-1].children[0] + + + def test_default_value(self): + item = StoreItem(uuid4()) + self.assertEqual(item.get_ancestors(),[]) + + + def test_root_element(self): + self.assertEqual(self.root.get_ancestors(),[]) + + + def test_single_ancestor(self): + self.assertEqual(self.children[3].get_ancestors(),[self.root]) + + + def test_multiple_ancestors(self): + expected = [self.root.children[-1].children[0],self.root.children[-1],self.root] + self.assertEqual(self.greatgrandchildren[0].get_ancestors(),expected) + + + +class TestStoreItemIsParentableTo(TestCase): + + + def setUp(self): + self.root = StoreItem(uuid4()) + self.strangers = [ StoreItem(uuid4()) for _ in range(3) ] + + self.children = [ StoreItem(uuid4()) for _ in range(5) ] + self.root.children = self.children + for c in self.children: + c.parent = self.root + + self.grandchildren = [ StoreItem(uuid4()) for _ in range(7) ] + self.root.children[-1].children = self.grandchildren + for c in self.grandchildren: + c.parent = self.root.children[-1] + + self.greatgrandchildren = [ StoreItem(uuid4()) for _ in range(2) ] + self.root.children[-1].children[0].children = self.greatgrandchildren + for c in self.greatgrandchildren: + c.parent = self.root.children[-1].children[0] + + + def test_forbid_selfparenting(self): + self.assertFalse(self.root.is_parentable_to(self.root)) + + + def test_forbid_children(self): + self.assertFalse(self.root.is_parentable_to(self.children[0])) + + + def test_forbid_distant_descendant(self): + self.assertFalse(self.root.is_parentable_to(self.greatgrandchildren[1])) + + + def test_allow_parent(self): + self.assertTrue(self.children[1].is_parentable_to(self.root)) + + + def test_allow_distant_ancestor(self): + self.assertTrue(self.greatgrandchildren[0].is_parentable_to(self.root)) + + + def test_allow_sibling(self): + self.assertTrue(self.children[1].is_parentable_to(self.children[2])) + + + def test_allow_other_trees(self): + self.assertTrue(self.greatgrandchildren[0].is_parentable_to(self.strangers[2]))