Commit 2a9be13f authored by Bram Schoenmakers's avatar Bram Schoenmakers

Eliminate possibility for multiple todos having the same id: value.

When there is a todo in the list with an id: tag but no child todos,
that value could be reused when creating a new dependency between two
(unrelated) items.

This is because the value of the dangling parent todo is not present in
the dependency graph, but the function generating a new ID depended on
that.

Instead, simply iterate over all available todo items to check whether
the proposed value already exists somewhere.
parent e664ac52
...@@ -238,6 +238,7 @@ class TodoListDependencyTester(TopydoTest): ...@@ -238,6 +238,7 @@ class TodoListDependencyTester(TopydoTest):
self.todolist.add("Something with +Project") self.todolist.add("Something with +Project")
self.todolist.add("Another one with +Project") self.todolist.add("Another one with +Project")
self.todolist.add("Todo with +AnotherProject") self.todolist.add("Todo with +AnotherProject")
self.todolist.add("Todo without children id:3")
def test_check_dep(self): def test_check_dep(self):
children = self.todolist.children(self.todolist.todo(1)) children = self.todolist.children(self.todolist.todo(1))
...@@ -268,8 +269,8 @@ class TodoListDependencyTester(TopydoTest): ...@@ -268,8 +269,8 @@ class TodoListDependencyTester(TopydoTest):
todo5 = self.todolist.todo(5) todo5 = self.todolist.todo(5)
self.todolist.add_dependency(todo5, todo4) self.todolist.add_dependency(todo5, todo4)
self.assertTrue(todo5.has_tag('id', '3')) self.assertTrue(todo5.has_tag('id', '4'))
self.assertTrue(todo4.has_tag('p', '3')) self.assertTrue(todo4.has_tag('p', '4'))
def test_add_dep2(self): def test_add_dep2(self):
""" """
...@@ -284,8 +285,8 @@ class TodoListDependencyTester(TopydoTest): ...@@ -284,8 +285,8 @@ class TodoListDependencyTester(TopydoTest):
self.todolist.add_dependency(todo5, todo4) self.todolist.add_dependency(todo5, todo4)
self.todolist.add_dependency(todo4, todo1) self.todolist.add_dependency(todo4, todo1)
self.assertTrue(todo4.has_tag('id', '4')) self.assertTrue(todo4.has_tag('id', '5'))
self.assertTrue(todo1.has_tag('p', '4')) self.assertTrue(todo1.has_tag('p', '5'))
def test_add_dep3(self): def test_add_dep3(self):
""" """
......
...@@ -115,9 +115,19 @@ class TodoList(TodoListBase): ...@@ -115,9 +115,19 @@ class TodoList(TodoListBase):
Unused means that no task has it as an 'id' value or as a 'p' Unused means that no task has it as an 'id' value or as a 'p'
value. value.
""" """
def id_exists(p_id):
"""
Returns True if there exists a todo with the given parent
ID.
"""
for todo in self._todos:
if todo.has_tag('id', str(p_id)):
return True
return False
new_id = 1 new_id = 1
while self._depgraph.has_edge_id(str(new_id)): while id_exists(new_id):
new_id += 1 new_id += 1
return str(new_id) return str(new_id)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment