From 56b8aa166d51d139edb41f0ef250b1854a5e93e8 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Thu, 18 Apr 2024 22:28:51 +0200 Subject: [PATCH 01/16] Refactor Process/ProcessStep setting and saving. --- plomtask/db.py | 2 +- plomtask/http.py | 16 ++++++++------- plomtask/processes.py | 48 +++++++++++++++++++++++-------------------- tests/processes.py | 41 ++++++++++++++++++++++-------------- tests/todos.py | 4 ++-- 5 files changed, 64 insertions(+), 47 deletions(-) diff --git a/plomtask/db.py b/plomtask/db.py index 4a82132..f53a94c 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -23,7 +23,7 @@ class DatabaseFile: # pylint: disable=too-few-public-methods self._check() def _check(self) -> None: - """Check file exists and is of proper schema.""" + """Check file exists, and is of proper schema.""" self.exists = isfile(self.path) if self.exists: self._validate_schema() diff --git a/plomtask/http.py b/plomtask/http.py index cb00825..25be899 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -225,23 +225,25 @@ class TaskHandler(BaseHTTPRequestHandler): self.form_data.get_all_int('condition')) process.set_fulfills(self.conn, self.form_data.get_all_int('fulfills')) process.set_undoes(self.conn, self.form_data.get_all_int('undoes')) - process.save_without_steps(self.conn) + process.save_id(self.conn) assert process.id_ is not None # for mypy process.explicit_steps = [] + steps: list[tuple[int | None, int, int | None]] = [] for step_id in self.form_data.get_all_int('steps'): - for step_process_id in\ - self.form_data.get_all_int(f'new_step_to_{step_id}'): - process.add_step(self.conn, None, step_process_id, step_id) + for step_process_id in self.form_data.get_all_int( + f'new_step_to_{step_id}'): + steps += [(None, step_process_id, step_id)] if step_id not in self.form_data.get_all_int('keep_step'): continue step_process_id = self.form_data.get_int( f'step_{step_id}_process_id') parent_id = self.form_data.get_int_or_none( f'step_{step_id}_parent_id') - process.add_step(self.conn, step_id, step_process_id, parent_id) + steps += [(step_id, step_process_id, parent_id)] for step_process_id in self.form_data.get_all_int('new_top_step'): - process.add_step(self.conn, None, step_process_id, None) - process.fix_steps(self.conn) + steps += [(None, step_process_id, None)] + process.set_steps(self.conn, steps) + process.save(self.conn) def do_POST_condition(self) -> None: """Update/insert Condition of ?id= and fields defined in postvars.""" diff --git a/plomtask/processes.py b/plomtask/processes.py index dc0613f..76ed30d 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -149,12 +149,15 @@ class Process: """Set self.undoes to Conditions identified by ids.""" self.set_conditions(db_conn, ids, 'undoes') - def add_step(self, db_conn: DatabaseConnection, id_: int | None, - step_process_id: int, - parent_step_id: int | None) -> ProcessStep: + def _add_step(self, + db_conn: DatabaseConnection, + id_: int | None, + step_process_id: int, + parent_step_id: int | None) -> ProcessStep: """Create new ProcessStep, save and add it to self.explicit_steps. Also checks against step recursion. + The new step's parent_step_id will fall back to None either if no matching ProcessStep is found (which can be assumed in case it was just deleted under its feet), or if the parent step would not be @@ -180,10 +183,27 @@ class Process: step.save(db_conn) # NB: This ensures a non-None step.id_. return step - def save_without_steps(self, db_conn: DatabaseConnection) -> None: - """Add (or re-write) self and connected VersionedAttributes to DB.""" + def set_steps(self, db_conn: DatabaseConnection, + steps: list[tuple[int | None, int, int | None]]) -> None: + """Set self.explicit_steps in bulk.""" + for step in self.explicit_steps: + assert step.id_ is not None + del db_conn.cached_process_steps[step.id_] + self.explicit_steps = [] + db_conn.exec('DELETE FROM process_steps WHERE owner_id = ?', + (self.id_,)) + for step_tuple in steps: + self._add_step(db_conn, step_tuple[0], + step_tuple[1], step_tuple[2]) + + def save_id(self, db_conn: DatabaseConnection) -> None: + """Write bare-bones self (sans connected items), ensuring self.id_.""" cursor = db_conn.exec('REPLACE INTO processes VALUES (?)', (self.id_,)) self.id_ = cursor.lastrowid + + def save(self, db_conn: DatabaseConnection) -> None: + """Add (or re-write) self and connected items to DB.""" + self.save_id(db_conn) self.title.save(db_conn) self.description.save(db_conn) self.effort.save(db_conn) @@ -203,27 +223,11 @@ class Process: db_conn.exec('INSERT INTO process_undoes VALUES (?,?)', (self.id_, condition.id_)) assert self.id_ is not None - db_conn.cached_processes[self.id_] = self - - def fix_steps(self, db_conn: DatabaseConnection) -> None: - """Rewrite ProcessSteps from self.explicit_steps. - - This also fixes illegal Step.parent_step_id values, i.e. those pointing - to steps now absent, or owned by a different Process, fall back into - .parent_step_id=None - """ db_conn.exec('DELETE FROM process_steps WHERE owner_id = ?', (self.id_,)) for step in self.explicit_steps: - if step.parent_step_id is not None: - try: - parent_step = ProcessStep.by_id(db_conn, - step.parent_step_id) - if parent_step.owner_id != self.id_: - step.parent_step_id = None - except NotFoundException: - step.parent_step_id = None step.save(db_conn) + db_conn.cached_processes[self.id_] = self class ProcessStep: diff --git a/tests/processes.py b/tests/processes.py index 390b130..960fd70 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -28,61 +28,71 @@ class TestsWithDB(TestCaseWithDB): def setUp(self) -> None: super().setUp() self.proc1 = Process(None) - self.proc1.save_without_steps(self.db_conn) + self.proc1.save(self.db_conn) self.proc2 = Process(None) - self.proc2.save_without_steps(self.db_conn) + self.proc2.save(self.db_conn) self.proc3 = Process(None) - self.proc3.save_without_steps(self.db_conn) + self.proc3.save(self.db_conn) def test_Process_ids(self) -> None: - """Test Process.save_without_steps() re Process.id_.""" + """Test Process.save() re Process.id_.""" self.assertEqual(self.proc1.id_, Process.by_id(self.db_conn, 1, create=False).id_) self.assertEqual(self.proc2.id_, Process.by_id(self.db_conn, 2, create=False).id_) proc5 = Process(5) - proc5.save_without_steps(self.db_conn) + proc5.save(self.db_conn) self.assertEqual(proc5.id_, Process.by_id(self.db_conn, 5, create=False).id_) def test_Process_steps(self) -> None: """Test addition, nesting, and non-recursion of ProcessSteps""" + def add_step(proc: Process, + steps_proc: list[tuple[int | None, int, int | None]], + step_tuple: tuple[int | None, int, int | None], + expected_id: int) -> None: + steps_proc += [step_tuple] + proc.set_steps(self.db_conn, steps_proc) + steps_proc[-1] = (expected_id, step_tuple[1], step_tuple[2]) assert self.proc2.id_ is not None assert self.proc3.id_ is not None - self.proc1.add_step(self.db_conn, None, self.proc2.id_, None) + steps_proc1: list[tuple[int | None, int, int | None]] = [] + add_step(self.proc1, steps_proc1, (None, self.proc2.id_, None), 1) p_1_dict: dict[int, dict[str, Any]] = {1: { 'process': self.proc2, 'parent_id': None, 'is_explicit': True, 'steps': {}, 'seen': False }} self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) - s_b = self.proc1.add_step(self.db_conn, None, self.proc3.id_, None) + add_step(self.proc1, steps_proc1, (None, self.proc3.id_, None), 2) + step_2 = self.proc1.explicit_steps[-1] p_1_dict[2] = { 'process': self.proc3, 'parent_id': None, 'is_explicit': True, 'steps': {}, 'seen': False } self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) - s_c = self.proc2.add_step(self.db_conn, None, self.proc3.id_, None) - assert s_c.id_ is not None + steps_proc2: list[tuple[int | None, int, int | None]] = [] + add_step(self.proc2, steps_proc2, (None, self.proc3.id_, None), 3) p_1_dict[1]['steps'] = {3: { 'process': self.proc3, 'parent_id': None, 'is_explicit': False, 'steps': {}, 'seen': False }} self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) - self.proc1.add_step(self.db_conn, None, self.proc2.id_, s_b.id_) + add_step(self.proc1, steps_proc1, (None, self.proc2.id_, step_2.id_), + 4) p_1_dict[2]['steps'][4] = { - 'process': self.proc2, 'parent_id': s_b.id_, 'seen': False, + 'process': self.proc2, 'parent_id': step_2.id_, 'seen': False, 'is_explicit': True, 'steps': {3: { 'process': self.proc3, 'parent_id': None, 'is_explicit': False, 'steps': {}, 'seen': True }}} self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) - self.proc1.add_step(self.db_conn, None, self.proc3.id_, 999) + add_step(self.proc1, steps_proc1, (None, self.proc3.id_, 999), 5) p_1_dict[5] = { 'process': self.proc3, 'parent_id': None, 'is_explicit': True, 'steps': {}, 'seen': False } self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) - self.proc1.add_step(self.db_conn, None, self.proc3.id_, 3) + add_step(self.proc1, steps_proc1, (None, self.proc3.id_, 3), 6) p_1_dict[6] = { 'process': self.proc3, 'parent_id': None, 'is_explicit': True, 'steps': {}, 'seen': False @@ -133,7 +143,8 @@ class TestsWithDB(TestCaseWithDB): def test_ProcessStep_singularity(self) -> None: """Test pointers made for single object keep pointing to it.""" assert self.proc2.id_ is not None - step = self.proc1.add_step(self.db_conn, None, self.proc2.id_, None) + self.proc1.set_steps(self.db_conn, [(None, self.proc2.id_, None)]) + step = self.proc1.explicit_steps[-1] assert step.id_ is not None step_retrieved = ProcessStep.by_id(self.db_conn, step.id_) step.parent_step_id = 99 @@ -142,7 +153,7 @@ class TestsWithDB(TestCaseWithDB): def test_Process_singularity(self) -> None: """Test pointers made for single object keep pointing to it.""" assert self.proc2.id_ is not None - self.proc1.add_step(self.db_conn, None, self.proc2.id_, None) + self.proc1.set_steps(self.db_conn, [(None, self.proc2.id_, None)]) p_retrieved = Process.by_id(self.db_conn, self.proc1.id_) self.assertEqual(self.proc1.explicit_steps, p_retrieved.explicit_steps) diff --git a/tests/todos.py b/tests/todos.py index f33534d..6bc5a05 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -18,7 +18,7 @@ class TestsWithDB(TestCaseWithDB): self.day2 = Day('2024-01-02') self.day2.save(self.db_conn) self.proc = Process(None) - self.proc.save_without_steps(self.db_conn) + self.proc.save(self.db_conn) self.cond1 = Condition(None) self.cond1.save(self.db_conn) self.cond2 = Condition(None) @@ -30,7 +30,7 @@ class TestsWithDB(TestCaseWithDB): todo = Todo(None, process_unsaved, False, self.day1) with self.assertRaises(NotFoundException): todo.save(self.db_conn) - process_unsaved.save_without_steps(self.db_conn) + process_unsaved.save(self.db_conn) todo.save(self.db_conn) self.assertEqual(Todo.by_id(self.db_conn, 1), todo) with self.assertRaises(NotFoundException): -- 2.30.2 From c5fab0b28785bb8f3a8e2b8e455fd679cfe83d25 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 00:50:06 +0200 Subject: [PATCH 02/16] Base core models on BaseModel providing sensible defaults. --- plomtask/conditions.py | 19 ++++++++----------- plomtask/days.py | 17 +++++++++++------ plomtask/db.py | 28 +++++++++++++++++++++++++++ plomtask/http.py | 2 +- plomtask/processes.py | 43 +++++++++++++++++------------------------- plomtask/todos.py | 29 +++++++++++++++++++--------- tests/conditions.py | 2 +- tests/processes.py | 23 ++++++++++++---------- 8 files changed, 99 insertions(+), 64 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index 5c57d85..27ab62c 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -1,19 +1,18 @@ """Non-doable elements of ProcessStep/Todo chains.""" from __future__ import annotations from sqlite3 import Row -from plomtask.db import DatabaseConnection +from plomtask.db import DatabaseConnection, BaseModel from plomtask.misc import VersionedAttribute -from plomtask.exceptions import BadFormatException, NotFoundException +from plomtask.exceptions import NotFoundException -class Condition: +class Condition(BaseModel): """Non Process-dependency for ProcessSteps and Todos.""" + table_name = 'conditions' + to_save = ['is_active'] def __init__(self, id_: int | None, is_active: bool = False) -> None: - if (id_ is not None) and id_ < 1: - msg = f'illegal Condition ID, must be >=1: {id_}' - raise BadFormatException(msg) - self.id_ = id_ + self.set_int_id(id_) self.is_active = is_active self.title = VersionedAttribute(self, 'condition_titles', 'UNNAMED') self.description = VersionedAttribute(self, 'condition_descriptions', @@ -65,10 +64,8 @@ class Condition: def save(self, db_conn: DatabaseConnection) -> None: """Save self and its VersionedAttributes to DB and cache.""" - cursor = db_conn.exec('REPLACE INTO conditions VALUES (?, ?)', - (self.id_, self.is_active)) - self.id_ = cursor.lastrowid + self.save_core(db_conn) self.title.save(db_conn) self.description.save(db_conn) - assert self.id_ is not None + assert isinstance(self.id_, int) db_conn.cached_conditions[self.id_] = self diff --git a/plomtask/days.py b/plomtask/days.py index abfce06..553579e 100644 --- a/plomtask/days.py +++ b/plomtask/days.py @@ -3,7 +3,7 @@ from __future__ import annotations from datetime import datetime, timedelta from sqlite3 import Row from plomtask.exceptions import BadFormatException, NotFoundException -from plomtask.db import DatabaseConnection +from plomtask.db import DatabaseConnection, BaseModel DATE_FORMAT = '%Y-%m-%d' @@ -25,11 +25,13 @@ def todays_date() -> str: return datetime.now().strftime(DATE_FORMAT) -class Day: +class Day(BaseModel): """Individual days defined by their dates.""" + table_name = 'days' + to_save = ['comment'] def __init__(self, date: str, comment: str = '') -> None: - self.date = valid_date(date) + self.id_: str = valid_date(date) self.datetime = datetime.strptime(self.date, DATE_FORMAT) self.comment = comment @@ -88,6 +90,11 @@ class Day: assert isinstance(day, Day) return day + @property + def date(self) -> str: + """Return self.id_ under the assumption it's a date string.""" + return self.id_ + @property def weekday(self) -> str: """Return what weekday matches self.date.""" @@ -107,6 +114,4 @@ class Day: def save(self, db_conn: DatabaseConnection) -> None: """Add (or re-write) self to DB and cache.""" - db_conn.exec('REPLACE INTO days VALUES (?, ?)', - (self.date, self.comment)) - db_conn.cached_days[self.date] = self + self.save_core(db_conn, update_with_lastrowid=False) diff --git a/plomtask/db.py b/plomtask/db.py index f53a94c..dfb388b 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -66,3 +66,31 @@ class DatabaseConnection: def close(self) -> None: """Close DB connection.""" self.conn.close() + + +class BaseModel: + """Template for most of the models we use/derive from the DB.""" + table_name = '' + to_save: list[str] = [] + id_: None | int | str + + def set_int_id(self, id_: int | None) -> None: + """Set id_ if >= 1 or None, else fail.""" + if (id_ is not None) and id_ < 1: + msg = f'illegal {self.__class__.__name__} ID, must be >=1: {id_}' + raise HandledException(msg) + self.id_ = id_ + + def save_core(self, db_conn: DatabaseConnection, + update_with_lastrowid: bool = True) -> None: + """Write bare-bones self (sans connected items), ensuring self.id_.""" + q_marks = ','.join(['?'] * (len(self.to_save) + 1)) + values = tuple([self.id_] + [getattr(self, key) + for key in self.to_save]) + table_name = self.table_name + cursor = db_conn.exec(f'REPLACE INTO {table_name} VALUES ({q_marks})', + values) + if update_with_lastrowid: + self.id_ = cursor.lastrowid + cache = getattr(db_conn, f'cached_{table_name}') + cache[self.id_] = self diff --git a/plomtask/http.py b/plomtask/http.py index 25be899..a14233a 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -225,7 +225,7 @@ class TaskHandler(BaseHTTPRequestHandler): self.form_data.get_all_int('condition')) process.set_fulfills(self.conn, self.form_data.get_all_int('fulfills')) process.set_undoes(self.conn, self.form_data.get_all_int('undoes')) - process.save_id(self.conn) + process.save_core(self.conn) assert process.id_ is not None # for mypy process.explicit_steps = [] steps: list[tuple[int | None, int, int | None]] = [] diff --git a/plomtask/processes.py b/plomtask/processes.py index 76ed30d..45de9db 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -2,21 +2,20 @@ from __future__ import annotations from sqlite3 import Row from typing import Any, Set -from plomtask.db import DatabaseConnection +from plomtask.db import DatabaseConnection, BaseModel from plomtask.misc import VersionedAttribute from plomtask.conditions import Condition from plomtask.exceptions import NotFoundException, BadFormatException -class Process: +class Process(BaseModel): """Template for, and metadata for, Todos, and their arrangements.""" + table_name = 'processes' # pylint: disable=too-many-instance-attributes def __init__(self, id_: int | None) -> None: - if (id_ is not None) and id_ < 1: - raise BadFormatException(f'illegal Process ID, must be >=1: {id_}') - self.id_ = id_ + self.set_int_id(id_) self.title = VersionedAttribute(self, 'process_titles', 'UNNAMED') self.description = VersionedAttribute(self, 'process_descriptions', '') self.effort = VersionedAttribute(self, 'process_efforts', 1.0) @@ -29,7 +28,7 @@ class Process: def from_table_row(cls, db_conn: DatabaseConnection, row: Row) -> Process: """Make Process from database row, with empty VersionedAttributes.""" process = cls(row[0]) - assert process.id_ is not None + assert isinstance(process.id_, int) db_conn.cached_processes[process.id_] = process return process @@ -125,7 +124,7 @@ class Process: external_owner = self for step in [s for s in self.explicit_steps if s.parent_step_id is None]: - assert step.id_ is not None # for mypy + assert isinstance(step.id_, int) steps[step.id_] = make_node(step) for step_id, step_node in steps.items(): walk_steps(step_id, step_node) @@ -176,7 +175,7 @@ class Process: parent_step_id = None except NotFoundException: parent_step_id = None - assert self.id_ is not None + assert isinstance(self.id_, int) step = ProcessStep(id_, self.id_, step_process_id, parent_step_id) walk_steps(step) self.explicit_steps += [step] @@ -187,7 +186,7 @@ class Process: steps: list[tuple[int | None, int, int | None]]) -> None: """Set self.explicit_steps in bulk.""" for step in self.explicit_steps: - assert step.id_ is not None + assert isinstance(step.id_, int) del db_conn.cached_process_steps[step.id_] self.explicit_steps = [] db_conn.exec('DELETE FROM process_steps WHERE owner_id = ?', @@ -196,14 +195,9 @@ class Process: self._add_step(db_conn, step_tuple[0], step_tuple[1], step_tuple[2]) - def save_id(self, db_conn: DatabaseConnection) -> None: - """Write bare-bones self (sans connected items), ensuring self.id_.""" - cursor = db_conn.exec('REPLACE INTO processes VALUES (?)', (self.id_,)) - self.id_ = cursor.lastrowid - def save(self, db_conn: DatabaseConnection) -> None: """Add (or re-write) self and connected items to DB.""" - self.save_id(db_conn) + self.save_core(db_conn) self.title.save(db_conn) self.description.save(db_conn) self.effort.save(db_conn) @@ -222,7 +216,7 @@ class Process: for condition in self.undoes: db_conn.exec('INSERT INTO process_undoes VALUES (?,?)', (self.id_, condition.id_)) - assert self.id_ is not None + assert isinstance(self.id_, int) db_conn.exec('DELETE FROM process_steps WHERE owner_id = ?', (self.id_,)) for step in self.explicit_steps: @@ -230,12 +224,14 @@ class Process: db_conn.cached_processes[self.id_] = self -class ProcessStep: +class ProcessStep(BaseModel): """Sub-unit of Processes.""" + table_name = 'process_steps' + to_save = ['owner_id', 'step_process_id', 'parent_step_id'] def __init__(self, id_: int | None, owner_id: int, step_process_id: int, parent_step_id: int | None) -> None: - self.id_ = id_ + self.set_int_id(id_) self.owner_id = owner_id self.step_process_id = step_process_id self.parent_step_id = parent_step_id @@ -245,7 +241,7 @@ class ProcessStep: row: Row) -> ProcessStep: """Make ProcessStep from database row, store in DB cache.""" step = cls(row[0], row[1], row[2], row[3]) - assert step.id_ is not None + assert isinstance(step.id_, int) db_conn.cached_process_steps[step.id_] = step return step @@ -262,10 +258,5 @@ class ProcessStep: raise NotFoundException(f'found no ProcessStep of ID {id_}') def save(self, db_conn: DatabaseConnection) -> None: - """Save to database and cache.""" - cursor = db_conn.exec('REPLACE INTO process_steps VALUES (?, ?, ?, ?)', - (self.id_, self.owner_id, self.step_process_id, - self.parent_step_id)) - self.id_ = cursor.lastrowid - assert self.id_ is not None - db_conn.cached_process_steps[self.id_] = self + """Default to simply calling self.save_core for simple cases.""" + self.save_core(db_conn) diff --git a/plomtask/todos.py b/plomtask/todos.py index ce83fad..5d46b30 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -1,7 +1,7 @@ """Actionables.""" from __future__ import annotations from sqlite3 import Row -from plomtask.db import DatabaseConnection +from plomtask.db import DatabaseConnection, BaseModel from plomtask.days import Day from plomtask.processes import Process from plomtask.conditions import Condition @@ -9,14 +9,18 @@ from plomtask.exceptions import (NotFoundException, BadFormatException, HandledException) -class Todo: +class Todo(BaseModel): """Individual actionable.""" # pylint: disable=too-many-instance-attributes + name = 'Todo' + table_name = 'todos' + to_save = ['process_id', 'is_done', 'date'] + def __init__(self, id_: int | None, process: Process, is_done: bool, day: Day) -> None: - self.id_ = id_ + self.set_int_id(id_) self.process = process self._is_done = is_done self.day = day @@ -37,7 +41,7 @@ class Todo: process=Process.by_id(db_conn, row[1]), is_done=bool(row[2]), day=Day.by_date(db_conn, row[3])) - assert todo.id_ is not None + assert isinstance(todo.id_, int) db_conn.cached_todos[todo.id_] = todo return todo @@ -115,6 +119,16 @@ class Todo: return False return True + @property + def process_id(self) -> int | str | None: + """Return ID of tasked Process.""" + return self.process.id_ + + @property + def date(self) -> str: + """Return date of used Day.""" + return self.day.date + @property def is_done(self) -> bool: """Wrapper around self._is_done so we can control its setter.""" @@ -171,11 +185,8 @@ class Todo: """Write self and children to DB and its cache.""" if self.process.id_ is None: raise NotFoundException('Process of Todo without ID (not saved?)') - cursor = db_conn.exec('REPLACE INTO todos VALUES (?,?,?,?)', - (self.id_, self.process.id_, - self.is_done, self.day.date)) - self.id_ = cursor.lastrowid - assert self.id_ is not None + self.save_core(db_conn) + assert isinstance(self.id_, int) db_conn.cached_todos[self.id_] = self db_conn.exec('DELETE FROM todo_children WHERE parent = ?', (self.id_,)) diff --git a/tests/conditions.py b/tests/conditions.py index 4781246..b6510a1 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -50,6 +50,6 @@ class TestsWithServer(TestCaseWithServer): """Test /condition and /conditions response codes.""" self.check_get('/condition', 200) self.check_get('/condition?id=', 200) - self.check_get('/condition?id=0', 400) + self.check_get('/condition?id=0', 500) self.check_get('/condition?id=FOO', 400) self.check_get('/conditions', 200) diff --git a/tests/processes.py b/tests/processes.py index 960fd70..491a48f 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -4,7 +4,7 @@ from typing import Any from tests.utils import TestCaseWithDB, TestCaseWithServer from plomtask.processes import Process, ProcessStep from plomtask.conditions import Condition -from plomtask.exceptions import NotFoundException, BadFormatException +from plomtask.exceptions import NotFoundException, HandledException class TestsSansDB(TestCase): @@ -18,7 +18,7 @@ class TestsSansDB(TestCase): def test_Process_legal_ID(self) -> None: """Test Process cannot be instantiated with id_=0.""" - with self.assertRaises(BadFormatException): + with self.assertRaises(HandledException): Process(0) @@ -54,8 +54,8 @@ class TestsWithDB(TestCaseWithDB): steps_proc += [step_tuple] proc.set_steps(self.db_conn, steps_proc) steps_proc[-1] = (expected_id, step_tuple[1], step_tuple[2]) - assert self.proc2.id_ is not None - assert self.proc3.id_ is not None + assert isinstance(self.proc2.id_, int) + assert isinstance(self.proc3.id_, int) steps_proc1: list[tuple[int | None, int, int | None]] = [] add_step(self.proc1, steps_proc1, (None, self.proc2.id_, None), 1) p_1_dict: dict[int, dict[str, Any]] = {1: { @@ -65,6 +65,7 @@ class TestsWithDB(TestCaseWithDB): self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) add_step(self.proc1, steps_proc1, (None, self.proc3.id_, None), 2) step_2 = self.proc1.explicit_steps[-1] + assert isinstance(step_2.id_, int) p_1_dict[2] = { 'process': self.proc3, 'parent_id': None, 'is_explicit': True, 'steps': {}, 'seen': False @@ -111,10 +112,10 @@ class TestsWithDB(TestCaseWithDB): for target in ('conditions', 'fulfills', 'undoes'): c1 = Condition(None, False) c1.save(self.db_conn) - assert c1.id_ is not None + assert isinstance(c1.id_, int) c2 = Condition(None, False) c2.save(self.db_conn) - assert c2.id_ is not None + assert isinstance(c2.id_, int) self.proc1.set_conditions(self.db_conn, [], target) self.assertEqual(getattr(self.proc1, target), []) self.proc1.set_conditions(self.db_conn, [c1.id_], target) @@ -142,23 +143,25 @@ class TestsWithDB(TestCaseWithDB): def test_ProcessStep_singularity(self) -> None: """Test pointers made for single object keep pointing to it.""" - assert self.proc2.id_ is not None + assert isinstance(self.proc2.id_, int) self.proc1.set_steps(self.db_conn, [(None, self.proc2.id_, None)]) step = self.proc1.explicit_steps[-1] - assert step.id_ is not None + assert isinstance(step.id_, int) step_retrieved = ProcessStep.by_id(self.db_conn, step.id_) step.parent_step_id = 99 self.assertEqual(step.parent_step_id, step_retrieved.parent_step_id) def test_Process_singularity(self) -> None: """Test pointers made for single object keep pointing to it.""" - assert self.proc2.id_ is not None + assert isinstance(self.proc1.id_, int) + assert isinstance(self.proc2.id_, int) self.proc1.set_steps(self.db_conn, [(None, self.proc2.id_, None)]) p_retrieved = Process.by_id(self.db_conn, self.proc1.id_) self.assertEqual(self.proc1.explicit_steps, p_retrieved.explicit_steps) def test_Process_versioned_attributes_singularity(self) -> None: """Test behavior of VersionedAttributes on saving (with .title).""" + assert isinstance(self.proc1.id_, int) self.proc1.title.set('named') p_loaded = Process.by_id(self.db_conn, self.proc1.id_) self.assertEqual(self.proc1.title.history, p_loaded.title.history) @@ -201,6 +204,6 @@ class TestsWithServer(TestCaseWithServer): """Test /process and /processes response codes.""" self.check_get('/process', 200) self.check_get('/process?id=', 200) - self.check_get('/process?id=0', 400) + self.check_get('/process?id=0', 500) self.check_get('/process?id=FOO', 400) self.check_get('/processes', 200) -- 2.30.2 From 5195f3f36960b76d1b6530ef1822d0806db221d8 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 02:20:33 +0200 Subject: [PATCH 03/16] Refactor from_table_row methods of core DB models. --- plomtask/conditions.py | 9 +++-- plomtask/days.py | 13 +++----- plomtask/db.py | 12 ++++++- plomtask/http.py | 4 +-- plomtask/processes.py | 21 ++---------- plomtask/todos.py | 32 +++++------------- templates/todo.html | 2 +- tests/todos.py | 76 ++++++++++++++++++++---------------------- 8 files changed, 71 insertions(+), 98 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index 27ab62c..9fab77f 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -22,13 +22,16 @@ class Condition(BaseModel): def from_table_row(cls, db_conn: DatabaseConnection, row: Row) -> Condition: """Build condition from row, including VersionedAttributes.""" - condition = cls(row[0], row[1]) + condition = super().from_table_row(db_conn, row) + assert isinstance(condition, Condition) for title_row in db_conn.exec('SELECT * FROM condition_titles ' 'WHERE parent_id = ?', (row[0],)): - condition.title.history[title_row[1]] = title_row[2] + condition.title.history[title_row[1]]\ + = title_row[2] # pylint: disable=no-member for desc_row in db_conn.exec('SELECT * FROM condition_descriptions ' 'WHERE parent_id = ?', (row[0],)): - condition.description.history[desc_row[1]] = desc_row[2] + condition.description.history[desc_row[1]]\ + = desc_row[2] # pylint: disable=no-member return condition @classmethod diff --git a/plomtask/days.py b/plomtask/days.py index 553579e..a21b4ef 100644 --- a/plomtask/days.py +++ b/plomtask/days.py @@ -1,7 +1,6 @@ """Collecting Day and date-related items.""" from __future__ import annotations from datetime import datetime, timedelta -from sqlite3 import Row from plomtask.exceptions import BadFormatException, NotFoundException from plomtask.db import DatabaseConnection, BaseModel @@ -29,6 +28,7 @@ class Day(BaseModel): """Individual days defined by their dates.""" table_name = 'days' to_save = ['comment'] + id_type = str def __init__(self, date: str, comment: str = '') -> None: self.id_: str = valid_date(date) @@ -41,13 +41,6 @@ class Day(BaseModel): def __lt__(self, other: Day) -> bool: return self.date < other.date - @classmethod - def from_table_row(cls, db_conn: DatabaseConnection, row: Row) -> Day: - """Make Day from database row, write to cache.""" - day = cls(row[0], row[1]) - db_conn.cached_days[day.date] = day - return day - @classmethod def all(cls, db_conn: DatabaseConnection, date_range: tuple[str, str] = ('', ''), @@ -82,7 +75,9 @@ class Day(BaseModel): assert isinstance(day, Day) return day for row in db_conn.exec('SELECT * FROM days WHERE date = ?', (date,)): - return cls.from_table_row(db_conn, row) + day = cls.from_table_row(db_conn, row) + assert isinstance(day, Day) + return day if not create: raise NotFoundException(f'Day not found for date: {date}') day = cls(date) diff --git a/plomtask/db.py b/plomtask/db.py index dfb388b..2cc1d64 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -1,7 +1,7 @@ """Database management.""" from os.path import isfile from difflib import Differ -from sqlite3 import connect as sql_connect, Cursor +from sqlite3 import connect as sql_connect, Cursor, Row from typing import Any, Dict from plomtask.exceptions import HandledException @@ -73,6 +73,16 @@ class BaseModel: table_name = '' to_save: list[str] = [] id_: None | int | str + id_type: type[Any] = int + + @classmethod + def from_table_row(cls, db_conn: DatabaseConnection, row: Row) -> Any: + """Make from DB row, write to DB cache.""" + obj = cls(*row) + assert isinstance(obj.id_, cls.id_type) + cache = getattr(db_conn, f'cached_{cls.table_name}') + cache[obj.id_] = obj + return obj def set_int_id(self, id_: int | None) -> None: """Set id_ if >= 1 or None, else fail.""" diff --git a/plomtask/http.py b/plomtask/http.py index a14233a..55120ff 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -137,7 +137,7 @@ class TaskHandler(BaseHTTPRequestHandler): id_ = self.params.get_int_or_none('id') todo = Todo.by_id(self.conn, id_) return {'todo': todo, - 'todo_candidates': Todo.by_date(self.conn, todo.day.date), + 'todo_candidates': Todo.by_date(self.conn, todo.date), 'condition_candidates': Condition.all(self.conn)} def do_GET_conditions(self) -> dict[str, object]: @@ -193,7 +193,7 @@ class TaskHandler(BaseHTTPRequestHandler): process_id = self.form_data.get_int_or_none('new_todo') if process_id is not None: process = Process.by_id(self.conn, process_id) - todo = Todo(None, process, False, day) + todo = Todo(None, process, False, day.date) todo.save(self.conn) def do_POST_todo(self) -> None: diff --git a/plomtask/processes.py b/plomtask/processes.py index 45de9db..2f8c2d5 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -1,6 +1,5 @@ """Collecting Processes and Process-related items.""" from __future__ import annotations -from sqlite3 import Row from typing import Any, Set from plomtask.db import DatabaseConnection, BaseModel from plomtask.misc import VersionedAttribute @@ -24,14 +23,6 @@ class Process(BaseModel): self.fulfills: list[Condition] = [] self.undoes: list[Condition] = [] - @classmethod - def from_table_row(cls, db_conn: DatabaseConnection, row: Row) -> Process: - """Make Process from database row, with empty VersionedAttributes.""" - process = cls(row[0]) - assert isinstance(process.id_, int) - db_conn.cached_processes[process.id_] = process - return process - @classmethod def all(cls, db_conn: DatabaseConnection) -> list[Process]: """Collect all Processes and their connected VersionedAttributes.""" @@ -236,15 +227,6 @@ class ProcessStep(BaseModel): self.step_process_id = step_process_id self.parent_step_id = parent_step_id - @classmethod - def from_table_row(cls, db_conn: DatabaseConnection, - row: Row) -> ProcessStep: - """Make ProcessStep from database row, store in DB cache.""" - step = cls(row[0], row[1], row[2], row[3]) - assert isinstance(step.id_, int) - db_conn.cached_process_steps[step.id_] = step - return step - @classmethod def by_id(cls, db_conn: DatabaseConnection, id_: int) -> ProcessStep: """Retrieve ProcessStep by id_, or throw NotFoundException.""" @@ -254,7 +236,8 @@ class ProcessStep(BaseModel): return step for row in db_conn.exec('SELECT * FROM process_steps ' 'WHERE step_id = ?', (id_,)): - return cls.from_table_row(db_conn, row) + step = cls.from_table_row(db_conn, row) + assert isinstance(step, ProcessStep) raise NotFoundException(f'found no ProcessStep of ID {id_}') def save(self, db_conn: DatabaseConnection) -> None: diff --git a/plomtask/todos.py b/plomtask/todos.py index 5d46b30..cfac5b5 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -1,8 +1,6 @@ """Actionables.""" from __future__ import annotations -from sqlite3 import Row from plomtask.db import DatabaseConnection, BaseModel -from plomtask.days import Day from plomtask.processes import Process from plomtask.conditions import Condition from plomtask.exceptions import (NotFoundException, BadFormatException, @@ -14,16 +12,15 @@ class Todo(BaseModel): # pylint: disable=too-many-instance-attributes - name = 'Todo' table_name = 'todos' to_save = ['process_id', 'is_done', 'date'] def __init__(self, id_: int | None, process: Process, - is_done: bool, day: Day) -> None: + is_done: bool, date: str) -> None: self.set_int_id(id_) self.process = process self._is_done = is_done - self.day = day + self.date = date self.children: list[Todo] = [] self.parents: list[Todo] = [] self.conditions: list[Condition] = [] @@ -34,17 +31,6 @@ class Todo(BaseModel): self.fulfills = process.fulfills[:] self.undoes = process.undoes[:] - @classmethod - def from_table_row(cls, db_conn: DatabaseConnection, row: Row) -> Todo: - """Make Todo from database row, write to DB cache.""" - todo = cls(id_=row[0], - process=Process.by_id(db_conn, row[1]), - is_done=bool(row[2]), - day=Day.by_date(db_conn, row[3])) - assert isinstance(todo.id_, int) - db_conn.cached_todos[todo.id_] = todo - return todo - @classmethod def by_id(cls, db_conn: DatabaseConnection, id_: int | None) -> Todo: """Get Todo of .id_=id_ and children (from DB cache if possible).""" @@ -54,6 +40,11 @@ class Todo(BaseModel): todo = None for row in db_conn.exec('SELECT * FROM todos WHERE id = ?', (id_,)): + row = list(row) + if row[1] == 0: + raise NotFoundException('calling Todo of ' + 'unsaved Process') + row[1] = Process.by_id(db_conn, row[1]) todo = cls.from_table_row(db_conn, row) break if todo is None: @@ -92,7 +83,7 @@ class Todo(BaseModel): for row in db_conn.exec('SELECT todo FROM todo_fulfills ' 'WHERE condition = ?', (condition.id_,)): todo = cls.by_id(db_conn, row[0]) - if todo.day.date == date: + if todo.date == date: enablers += [todo] return enablers @@ -104,7 +95,7 @@ class Todo(BaseModel): for row in db_conn.exec('SELECT todo FROM todo_undoes ' 'WHERE condition = ?', (condition.id_,)): todo = cls.by_id(db_conn, row[0]) - if todo.day.date == date: + if todo.date == date: disablers += [todo] return disablers @@ -124,11 +115,6 @@ class Todo(BaseModel): """Return ID of tasked Process.""" return self.process.id_ - @property - def date(self) -> str: - """Return date of used Day.""" - return self.day.date - @property def is_done(self) -> bool: """Wrapper around self._is_done so we can control its setter.""" diff --git a/templates/todo.html b/templates/todo.html index 9be13a8..1fa2922 100644 --- a/templates/todo.html +++ b/templates/todo.html @@ -5,7 +5,7 @@

id: {{todo.id_}}
-day: {{todo.day.date}}
+day: {{todo.date}}
process: {{todo.process.title.newest|e}}
done:

diff --git a/tests/todos.py b/tests/todos.py index 6bc5a05..a90f466 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -1,7 +1,6 @@ """Test Todos module.""" from tests.utils import TestCaseWithDB, TestCaseWithServer from plomtask.todos import Todo -from plomtask.days import Day from plomtask.processes import Process from plomtask.conditions import Condition from plomtask.exceptions import (NotFoundException, BadFormatException, @@ -13,10 +12,8 @@ class TestsWithDB(TestCaseWithDB): def setUp(self) -> None: super().setUp() - self.day1 = Day('2024-01-01') - self.day1.save(self.db_conn) - self.day2 = Day('2024-01-02') - self.day2.save(self.db_conn) + self.date1 = '2024-01-01' + self.date2 = '2024-01-02' self.proc = Process(None) self.proc.save(self.db_conn) self.cond1 = Condition(None) @@ -27,7 +24,7 @@ class TestsWithDB(TestCaseWithDB): def test_Todo_by_id(self) -> None: """Test creation and findability of Todos.""" process_unsaved = Process(None) - todo = Todo(None, process_unsaved, False, self.day1) + todo = Todo(None, process_unsaved, False, self.date1) with self.assertRaises(NotFoundException): todo.save(self.db_conn) process_unsaved.save(self.db_conn) @@ -40,32 +37,32 @@ class TestsWithDB(TestCaseWithDB): def test_Todo_by_date(self) -> None: """Test findability of Todos by date.""" - t1 = Todo(None, self.proc, False, self.day1) + t1 = Todo(None, self.proc, False, self.date1) t1.save(self.db_conn) - t2 = Todo(None, self.proc, False, self.day1) + t2 = Todo(None, self.proc, False, self.date1) t2.save(self.db_conn) - self.assertEqual(Todo.by_date(self.db_conn, self.day1.date), [t1, t2]) - self.assertEqual(Todo.by_date(self.db_conn, self.day2.date), []) + self.assertEqual(Todo.by_date(self.db_conn, self.date1), [t1, t2]) + self.assertEqual(Todo.by_date(self.db_conn, self.date2), []) self.assertEqual(Todo.by_date(self.db_conn, 'foo'), []) def test_Todo_from_process(self) -> None: """Test spawning of Todo attributes from Process.""" - assert self.cond1.id_ is not None - assert self.cond2.id_ is not None + assert isinstance(self.cond1.id_, int) + assert isinstance(self.cond2.id_, int) self.proc.set_conditions(self.db_conn, [self.cond1.id_]) - todo = Todo(None, self.proc, False, self.day1) + todo = Todo(None, self.proc, False, self.date1) self.assertEqual(todo.conditions, [self.cond1]) todo.set_conditions(self.db_conn, [self.cond2.id_]) self.assertEqual(todo.conditions, [self.cond2]) self.assertEqual(self.proc.conditions, [self.cond1]) self.proc.set_fulfills(self.db_conn, [self.cond1.id_]) - todo = Todo(None, self.proc, False, self.day1) + todo = Todo(None, self.proc, False, self.date1) self.assertEqual(todo.fulfills, [self.cond1]) todo.set_fulfills(self.db_conn, [self.cond2.id_]) self.assertEqual(todo.fulfills, [self.cond2]) self.assertEqual(self.proc.fulfills, [self.cond1]) self.proc.set_undoes(self.db_conn, [self.cond1.id_]) - todo = Todo(None, self.proc, False, self.day1) + todo = Todo(None, self.proc, False, self.date1) self.assertEqual(todo.undoes, [self.cond1]) todo.set_undoes(self.db_conn, [self.cond2.id_]) self.assertEqual(todo.undoes, [self.cond2]) @@ -73,9 +70,9 @@ class TestsWithDB(TestCaseWithDB): def test_Todo_on_conditions(self) -> None: """Test effect of Todos on Conditions.""" - assert self.cond1.id_ is not None - assert self.cond2.id_ is not None - todo = Todo(None, self.proc, False, self.day1) + assert isinstance(self.cond1.id_, int) + assert isinstance(self.cond2.id_, int) + todo = Todo(None, self.proc, False, self.date1) todo.save(self.db_conn) todo.set_fulfills(self.db_conn, [self.cond1.id_]) todo.set_undoes(self.db_conn, [self.cond2.id_]) @@ -88,44 +85,42 @@ class TestsWithDB(TestCaseWithDB): def test_Todo_enablers_disablers(self) -> None: """Test Todo.enablers_for_at/disablers_for_at.""" - assert self.cond1.id_ is not None - assert self.cond2.id_ is not None - todo1 = Todo(None, self.proc, False, self.day1) + assert isinstance(self.cond1.id_, int) + assert isinstance(self.cond2.id_, int) + todo1 = Todo(None, self.proc, False, self.date1) todo1.save(self.db_conn) todo1.set_fulfills(self.db_conn, [self.cond1.id_]) todo1.set_undoes(self.db_conn, [self.cond2.id_]) todo1.save(self.db_conn) - todo2 = Todo(None, self.proc, False, self.day1) + todo2 = Todo(None, self.proc, False, self.date1) todo2.save(self.db_conn) todo2.set_fulfills(self.db_conn, [self.cond2.id_]) todo2.save(self.db_conn) - todo3 = Todo(None, self.proc, False, self.day2) + todo3 = Todo(None, self.proc, False, self.date2) todo3.save(self.db_conn) todo3.set_fulfills(self.db_conn, [self.cond2.id_]) todo3.save(self.db_conn) - date1 = self.day1.date - date2 = self.day2.date - enablers = Todo.enablers_for_at(self.db_conn, self.cond1, date1) + enablers = Todo.enablers_for_at(self.db_conn, self.cond1, self.date1) self.assertEqual(enablers, [todo1]) - enablers = Todo.enablers_for_at(self.db_conn, self.cond1, date2) + enablers = Todo.enablers_for_at(self.db_conn, self.cond1, self.date2) self.assertEqual(enablers, []) - disablers = Todo.disablers_for_at(self.db_conn, self.cond1, date1) + disablers = Todo.disablers_for_at(self.db_conn, self.cond1, self.date1) self.assertEqual(disablers, []) - disablers = Todo.disablers_for_at(self.db_conn, self.cond1, date2) + disablers = Todo.disablers_for_at(self.db_conn, self.cond1, self.date2) self.assertEqual(disablers, []) - enablers = Todo.enablers_for_at(self.db_conn, self.cond2, date1) + enablers = Todo.enablers_for_at(self.db_conn, self.cond2, self.date1) self.assertEqual(enablers, [todo2]) - enablers = Todo.enablers_for_at(self.db_conn, self.cond2, date2) + enablers = Todo.enablers_for_at(self.db_conn, self.cond2, self.date2) self.assertEqual(enablers, [todo3]) - disablers = Todo.disablers_for_at(self.db_conn, self.cond2, date1) + disablers = Todo.disablers_for_at(self.db_conn, self.cond2, self.date1) self.assertEqual(disablers, [todo1]) - disablers = Todo.disablers_for_at(self.db_conn, self.cond2, date2) + disablers = Todo.disablers_for_at(self.db_conn, self.cond2, self.date2) self.assertEqual(disablers, []) def test_Todo_children(self) -> None: """Test Todo.children relations.""" - todo_1 = Todo(None, self.proc, False, self.day1) - todo_2 = Todo(None, self.proc, False, self.day1) + todo_1 = Todo(None, self.proc, False, self.date1) + todo_2 = Todo(None, self.proc, False, self.date1) with self.assertRaises(HandledException): todo_1.add_child(todo_2) todo_1.save(self.db_conn) @@ -134,6 +129,7 @@ class TestsWithDB(TestCaseWithDB): todo_2.save(self.db_conn) todo_1.add_child(todo_2) todo_1.save(self.db_conn) + assert isinstance(todo_1.id_, int) todo_retrieved = Todo.by_id(self.db_conn, todo_1.id_) self.assertEqual(todo_retrieved.children, [todo_2]) with self.assertRaises(BadFormatException): @@ -141,10 +137,10 @@ class TestsWithDB(TestCaseWithDB): def test_Todo_conditioning(self) -> None: """Test Todo.doability conditions.""" - assert self.cond1.id_ is not None - todo_1 = Todo(None, self.proc, False, self.day1) + assert isinstance(self.cond1.id_, int) + todo_1 = Todo(None, self.proc, False, self.date1) todo_1.save(self.db_conn) - todo_2 = Todo(None, self.proc, False, self.day1) + todo_2 = Todo(None, self.proc, False, self.date1) todo_2.save(self.db_conn) todo_2.add_child(todo_1) with self.assertRaises(BadFormatException): @@ -160,12 +156,12 @@ class TestsWithDB(TestCaseWithDB): def test_Todo_singularity(self) -> None: """Test pointers made for single object keep pointing to it.""" - todo = Todo(None, self.proc, False, self.day1) + todo = Todo(None, self.proc, False, self.date1) todo.save(self.db_conn) retrieved_todo = Todo.by_id(self.db_conn, 1) todo.is_done = True self.assertEqual(retrieved_todo.is_done, True) - retrieved_todo = Todo.by_date(self.db_conn, self.day1.date)[0] + retrieved_todo = Todo.by_date(self.db_conn, self.date1)[0] retrieved_todo.is_done = False self.assertEqual(todo.is_done, False) -- 2.30.2 From 5a5d713ce0b223ab2f6ef34c15bb82b614bdda98 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 04:58:34 +0200 Subject: [PATCH 04/16] Refactor models' .by_id(). --- plomtask/conditions.py | 14 ++++++-------- plomtask/days.py | 16 ++++++---------- plomtask/db.py | 22 +++++++++++++++++++++- plomtask/http.py | 4 ++-- plomtask/processes.py | 18 ++++-------------- plomtask/todos.py | 35 +++++++++++++++++++++-------------- scripts/init.sql | 6 +++--- tests/days.py | 14 +++++++------- tests/todos.py | 12 +++++++----- 9 files changed, 77 insertions(+), 64 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index 9fab77f..80fc13e 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -1,5 +1,6 @@ """Non-doable elements of ProcessStep/Todo chains.""" from __future__ import annotations +from typing import Any from sqlite3 import Row from plomtask.db import DatabaseConnection, BaseModel from plomtask.misc import VersionedAttribute @@ -20,7 +21,7 @@ class Condition(BaseModel): @classmethod def from_table_row(cls, db_conn: DatabaseConnection, - row: Row) -> Condition: + row: Row | list[Any]) -> Condition: """Build condition from row, including VersionedAttributes.""" condition = super().from_table_row(db_conn, row) assert isinstance(condition, Condition) @@ -52,17 +53,14 @@ class Condition(BaseModel): create: bool = False) -> Condition: """Collect (or create) Condition and its VersionedAttributes.""" condition = None - if id_ in db_conn.cached_conditions.keys(): - condition = db_conn.cached_conditions[id_] - else: - for row in db_conn.exec('SELECT * FROM conditions WHERE id = ?', - (id_,)): - condition = cls.from_table_row(db_conn, row) - break + if id_: + condition, _ = super()._by_id(db_conn, id_) if not condition: if not create: raise NotFoundException(f'Condition not found of id: {id_}') condition = cls(id_, False) + condition.save(db_conn) + assert isinstance(condition, Condition) return condition def save(self, db_conn: DatabaseConnection) -> None: diff --git a/plomtask/days.py b/plomtask/days.py index a21b4ef..d838039 100644 --- a/plomtask/days.py +++ b/plomtask/days.py @@ -51,9 +51,9 @@ class Day(BaseModel): start_date = valid_date(date_range[0] if date_range[0] else min_date) end_date = valid_date(date_range[1] if date_range[1] else max_date) days = [] - sql = 'SELECT date FROM days WHERE date >= ? AND date <= ?' + sql = 'SELECT id FROM days WHERE id >= ? AND id <= ?' for row in db_conn.exec(sql, (start_date, end_date)): - days += [cls.by_date(db_conn, row[0])] + days += [cls.by_id(db_conn, row[0])] days.sort() if fill_gaps and len(days) > 1: gapless_days = [] @@ -67,15 +67,11 @@ class Day(BaseModel): return days @classmethod - def by_date(cls, db_conn: DatabaseConnection, - date: str, create: bool = False) -> Day: + def by_id(cls, db_conn: DatabaseConnection, + date: str, create: bool = False) -> Day: """Retrieve Day by date if in DB (prefer cache), else return None.""" - if date in db_conn.cached_days.keys(): - day = db_conn.cached_days[date] - assert isinstance(day, Day) - return day - for row in db_conn.exec('SELECT * FROM days WHERE date = ?', (date,)): - day = cls.from_table_row(db_conn, row) + day, _ = super()._by_id(db_conn, date) + if day: assert isinstance(day, Day) return day if not create: diff --git a/plomtask/db.py b/plomtask/db.py index 2cc1d64..abd8f61 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -76,7 +76,8 @@ class BaseModel: id_type: type[Any] = int @classmethod - def from_table_row(cls, db_conn: DatabaseConnection, row: Row) -> Any: + def from_table_row(cls, db_conn: DatabaseConnection, + row: Row | list[Any]) -> Any: """Make from DB row, write to DB cache.""" obj = cls(*row) assert isinstance(obj.id_, cls.id_type) @@ -84,6 +85,25 @@ class BaseModel: cache[obj.id_] = obj return obj + @classmethod + def _by_id(cls, + db_conn: DatabaseConnection, + id_: int | str) -> tuple[Any, bool]: + """Return instance found by ID, or None, and if from cache or not.""" + from_cache = False + obj = None + cache = getattr(db_conn, f'cached_{cls.table_name}') + if id_ in cache.keys(): + obj = cache[id_] + from_cache = True + else: + for row in db_conn.exec(f'SELECT * FROM {cls.table_name} ' + 'WHERE id = ?', (id_,)): + obj = cls.from_table_row(db_conn, row) + cache[id_] = obj + break + return obj, from_cache + def set_int_id(self, id_: int | None) -> None: """Set id_ if >= 1 or None, else fail.""" if (id_ is not None) and id_ < 1: diff --git a/plomtask/http.py b/plomtask/http.py index 55120ff..5f739b6 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -127,7 +127,7 @@ class TaskHandler(BaseHTTPRequestHandler): 'condition': condition, 'enablers': enablers, 'disablers': disablers}] - return {'day': Day.by_date(self.conn, date, create=True), + return {'day': Day.by_id(self.conn, date, create=True), 'todos': Todo.by_date(self.conn, date), 'processes': Process.all(self.conn), 'conditions_listing': conditions_listing} @@ -187,7 +187,7 @@ class TaskHandler(BaseHTTPRequestHandler): def do_POST_day(self) -> None: """Update or insert Day of date and Todos mapped to it.""" date = self.params.get_str('date') - day = Day.by_date(self.conn, date, create=True) + day = Day.by_id(self.conn, date, create=True) day.comment = self.form_data.get_str('comment') day.save(self.conn) process_id = self.form_data.get_int_or_none('new_todo') diff --git a/plomtask/processes.py b/plomtask/processes.py index 2f8c2d5..7872c33 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -40,15 +40,9 @@ class Process(BaseModel): def by_id(cls, db_conn: DatabaseConnection, id_: int | None, create: bool = False) -> Process: """Collect Process, its VersionedAttributes, and its child IDs.""" - if id_ in db_conn.cached_processes.keys(): - process = db_conn.cached_processes[id_] - assert isinstance(process, Process) - return process process = None - for row in db_conn.exec('SELECT * FROM processes ' - 'WHERE id = ?', (id_,)): - process = cls(row[0]) - break + if id_: + process, _ = super()._by_id(db_conn, id_) if not process: if not create: raise NotFoundException(f'Process not found of id: {id_}') @@ -230,14 +224,10 @@ class ProcessStep(BaseModel): @classmethod def by_id(cls, db_conn: DatabaseConnection, id_: int) -> ProcessStep: """Retrieve ProcessStep by id_, or throw NotFoundException.""" - if id_ in db_conn.cached_process_steps.keys(): - step = db_conn.cached_process_steps[id_] + step, _ = super()._by_id(db_conn, id_) + if step: assert isinstance(step, ProcessStep) return step - for row in db_conn.exec('SELECT * FROM process_steps ' - 'WHERE step_id = ?', (id_,)): - step = cls.from_table_row(db_conn, row) - assert isinstance(step, ProcessStep) raise NotFoundException(f'found no ProcessStep of ID {id_}') def save(self, db_conn: DatabaseConnection) -> None: diff --git a/plomtask/todos.py b/plomtask/todos.py index cfac5b5..840c298 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -1,5 +1,7 @@ """Actionables.""" from __future__ import annotations +from typing import Any +from sqlite3 import Row from plomtask.db import DatabaseConnection, BaseModel from plomtask.processes import Process from plomtask.conditions import Condition @@ -31,24 +33,29 @@ class Todo(BaseModel): self.fulfills = process.fulfills[:] self.undoes = process.undoes[:] + @classmethod + def from_table_row(cls, db_conn: DatabaseConnection, + row: Row | list[Any]) -> Todo: + """Make from DB row, write to DB cache.""" + if row[1] == 0: + raise NotFoundException('calling Todo of ' + 'unsaved Process') + row_as_list = list(row) + row_as_list[1] = Process.by_id(db_conn, row[1]) + todo = super().from_table_row(db_conn, row_as_list) + assert isinstance(todo, Todo) + return todo + @classmethod def by_id(cls, db_conn: DatabaseConnection, id_: int | None) -> Todo: """Get Todo of .id_=id_ and children (from DB cache if possible).""" - if id_ in db_conn.cached_todos.keys(): - todo = db_conn.cached_todos[id_] + if id_: + todo, from_cache = super()._by_id(db_conn, id_) else: - todo = None - for row in db_conn.exec('SELECT * FROM todos WHERE id = ?', - (id_,)): - row = list(row) - if row[1] == 0: - raise NotFoundException('calling Todo of ' - 'unsaved Process') - row[1] = Process.by_id(db_conn, row[1]) - todo = cls.from_table_row(db_conn, row) - break - if todo is None: - raise NotFoundException(f'Todo of ID not found: {id_}') + todo, from_cache = None, False + if todo is None: + raise NotFoundException(f'Todo of ID not found: {id_}') + if not from_cache: for row in db_conn.exec('SELECT child FROM todo_children ' 'WHERE parent = ?', (id_,)): todo.children += [cls.by_id(db_conn, row[0])] diff --git a/scripts/init.sql b/scripts/init.sql index 807e1e7..870e845 100644 --- a/scripts/init.sql +++ b/scripts/init.sql @@ -17,7 +17,7 @@ CREATE TABLE conditions ( is_active BOOLEAN NOT NULL ); CREATE TABLE days ( - date TEXT PRIMARY KEY, + id TEXT PRIMARY KEY, comment TEXT NOT NULL ); CREATE TABLE process_conditions ( @@ -49,7 +49,7 @@ CREATE TABLE process_fulfills ( FOREIGN KEY (condition) REFERENCES conditions(id) ); CREATE TABLE process_steps ( - step_id INTEGER PRIMARY KEY, + id INTEGER PRIMARY KEY, owner_id INTEGER NOT NULL, step_process_id INTEGER NOT NULL, parent_step_id INTEGER, @@ -108,5 +108,5 @@ CREATE TABLE todos ( is_done BOOLEAN NOT NULL, day TEXT NOT NULL, FOREIGN KEY (process_id) REFERENCES processes(id), - FOREIGN KEY (day) REFERENCES days(date) + FOREIGN KEY (day) REFERENCES days(id) ); diff --git a/tests/days.py b/tests/days.py index 3524a66..895f59d 100644 --- a/tests/days.py +++ b/tests/days.py @@ -35,17 +35,17 @@ class TestsSansDB(TestCase): class TestsWithDB(TestCaseWithDB): """Tests requiring DB, but not server setup.""" - def test_Day_by_date(self) -> None: - """Test Day.by_date().""" + def test_Day_by_id(self) -> None: + """Test Day.by_id().""" with self.assertRaises(NotFoundException): - Day.by_date(self.db_conn, '2024-01-01') + Day.by_id(self.db_conn, '2024-01-01') Day('2024-01-01').save(self.db_conn) self.assertEqual(Day('2024-01-01'), - Day.by_date(self.db_conn, '2024-01-01')) + Day.by_id(self.db_conn, '2024-01-01')) with self.assertRaises(NotFoundException): - Day.by_date(self.db_conn, '2024-01-02') + Day.by_id(self.db_conn, '2024-01-02') self.assertEqual(Day('2024-01-02'), - Day.by_date(self.db_conn, '2024-01-02', create=True)) + Day.by_id(self.db_conn, '2024-01-02', create=True)) def test_Day_all(self) -> None: """Test Day.all(), especially in regards to date range filtering.""" @@ -94,7 +94,7 @@ class TestsWithDB(TestCaseWithDB): """Test pointers made for single object keep pointing to it.""" day = Day('2024-01-01') day.save(self.db_conn) - retrieved_day = Day.by_date(self.db_conn, '2024-01-01') + retrieved_day = Day.by_id(self.db_conn, '2024-01-01') day.comment = 'foo' self.assertEqual(retrieved_day.comment, 'foo') diff --git a/tests/todos.py b/tests/todos.py index a90f466..17454c5 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -121,19 +121,21 @@ class TestsWithDB(TestCaseWithDB): """Test Todo.children relations.""" todo_1 = Todo(None, self.proc, False, self.date1) todo_2 = Todo(None, self.proc, False, self.date1) + todo_2.save(self.db_conn) with self.assertRaises(HandledException): todo_1.add_child(todo_2) todo_1.save(self.db_conn) + todo_3 = Todo(None, self.proc, False, self.date1) with self.assertRaises(HandledException): - todo_1.add_child(todo_2) - todo_2.save(self.db_conn) - todo_1.add_child(todo_2) + todo_1.add_child(todo_3) + todo_3.save(self.db_conn) + todo_1.add_child(todo_3) todo_1.save(self.db_conn) assert isinstance(todo_1.id_, int) todo_retrieved = Todo.by_id(self.db_conn, todo_1.id_) - self.assertEqual(todo_retrieved.children, [todo_2]) + self.assertEqual(todo_retrieved.children, [todo_3]) with self.assertRaises(BadFormatException): - todo_2.add_child(todo_1) + todo_3.add_child(todo_1) def test_Todo_conditioning(self) -> None: """Test Todo.doability conditions.""" -- 2.30.2 From 33cff8c5f6427c4e7e617c459ee024b5b6c2d32e Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 05:45:12 +0200 Subject: [PATCH 05/16] Improve consistency of DB column names. --- plomtask/conditions.py | 4 ++-- plomtask/misc.py | 2 +- plomtask/processes.py | 16 +++++++-------- scripts/init.sql | 46 +++++++++++++++++++++--------------------- 4 files changed, 34 insertions(+), 34 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index 80fc13e..98a0b99 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -26,11 +26,11 @@ class Condition(BaseModel): condition = super().from_table_row(db_conn, row) assert isinstance(condition, Condition) for title_row in db_conn.exec('SELECT * FROM condition_titles ' - 'WHERE parent_id = ?', (row[0],)): + 'WHERE parent = ?', (row[0],)): condition.title.history[title_row[1]]\ = title_row[2] # pylint: disable=no-member for desc_row in db_conn.exec('SELECT * FROM condition_descriptions ' - 'WHERE parent_id = ?', (row[0],)): + 'WHERE parent = ?', (row[0],)): condition.description.history[desc_row[1]]\ = desc_row[2] # pylint: disable=no-member return condition diff --git a/plomtask/misc.py b/plomtask/misc.py index bf07188..dcaad17 100644 --- a/plomtask/misc.py +++ b/plomtask/misc.py @@ -46,7 +46,7 @@ class VersionedAttribute: def save(self, db_conn: DatabaseConnection) -> None: """Save as self.history entries, but first wipe old ones.""" - db_conn.exec(f'DELETE FROM {self.table_name} WHERE parent_id = ?', + db_conn.exec(f'DELETE FROM {self.table_name} WHERE parent = ?', (self.parent.id_,)) for timestamp, value in self.history.items(): db_conn.exec(f'INSERT INTO {self.table_name} VALUES (?, ?, ?)', diff --git a/plomtask/processes.py b/plomtask/processes.py index 7872c33..9c5c824 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -48,16 +48,16 @@ class Process(BaseModel): raise NotFoundException(f'Process not found of id: {id_}') process = Process(id_) for row in db_conn.exec('SELECT * FROM process_titles ' - 'WHERE parent_id = ?', (process.id_,)): + 'WHERE parent = ?', (process.id_,)): process.title.history[row[1]] = row[2] for row in db_conn.exec('SELECT * FROM process_descriptions ' - 'WHERE parent_id = ?', (process.id_,)): + 'WHERE parent = ?', (process.id_,)): process.description.history[row[1]] = row[2] for row in db_conn.exec('SELECT * FROM process_efforts ' - 'WHERE parent_id = ?', (process.id_,)): + 'WHERE parent = ?', (process.id_,)): process.effort.history[row[1]] = row[2] for row in db_conn.exec('SELECT * FROM process_steps ' - 'WHERE owner_id = ?', (process.id_,)): + 'WHERE owner = ?', (process.id_,)): process.explicit_steps += [ProcessStep.from_table_row(db_conn, row)] for row in db_conn.exec('SELECT condition FROM process_conditions ' @@ -75,8 +75,8 @@ class Process(BaseModel): def used_as_step_by(self, db_conn: DatabaseConnection) -> list[Process]: """Return Processes using self for a ProcessStep.""" owner_ids = set() - for owner_id in db_conn.exec('SELECT owner_id FROM process_steps WHERE' - ' step_process_id = ?', (self.id_,)): + for owner_id in db_conn.exec('SELECT owner FROM process_steps WHERE' + ' step_process = ?', (self.id_,)): owner_ids.add(owner_id[0]) return [self.__class__.by_id(db_conn, id_) for id_ in owner_ids] @@ -174,7 +174,7 @@ class Process(BaseModel): assert isinstance(step.id_, int) del db_conn.cached_process_steps[step.id_] self.explicit_steps = [] - db_conn.exec('DELETE FROM process_steps WHERE owner_id = ?', + db_conn.exec('DELETE FROM process_steps WHERE owner = ?', (self.id_,)) for step_tuple in steps: self._add_step(db_conn, step_tuple[0], @@ -202,7 +202,7 @@ class Process(BaseModel): db_conn.exec('INSERT INTO process_undoes VALUES (?,?)', (self.id_, condition.id_)) assert isinstance(self.id_, int) - db_conn.exec('DELETE FROM process_steps WHERE owner_id = ?', + db_conn.exec('DELETE FROM process_steps WHERE owner = ?', (self.id_,)) for step in self.explicit_steps: step.save(db_conn) diff --git a/scripts/init.sql b/scripts/init.sql index 870e845..5fdd779 100644 --- a/scripts/init.sql +++ b/scripts/init.sql @@ -1,16 +1,16 @@ CREATE TABLE condition_descriptions ( - parent_id INTEGER NOT NULL, + parent INTEGER NOT NULL, timestamp TEXT NOT NULL, description TEXT NOT NULL, - PRIMARY KEY (parent_id, timestamp), - FOREIGN KEY (parent_id) REFERENCES conditions(id) + PRIMARY KEY (parent, timestamp), + FOREIGN KEY (parent) REFERENCES conditions(id) ); CREATE TABLE condition_titles ( - parent_id INTEGER NOT NULL, + parent INTEGER NOT NULL, timestamp TEXT NOT NULL, title TEXT NOT NULL, - PRIMARY KEY (parent_id, timestamp), - FOREIGN KEY (parent_id) REFERENCES conditions(id) + PRIMARY KEY (parent, timestamp), + FOREIGN KEY (parent) REFERENCES conditions(id) ); CREATE TABLE conditions ( id INTEGER PRIMARY KEY, @@ -28,18 +28,18 @@ CREATE TABLE process_conditions ( FOREIGN KEY (condition) REFERENCES conditions(id) ); CREATE TABLE process_descriptions ( - parent_id INTEGER NOT NULL, + parent INTEGER NOT NULL, timestamp TEXT NOT NULL, description TEXT NOT NULL, - PRIMARY KEY (parent_id, timestamp), - FOREIGN KEY (parent_id) REFERENCES processes(id) + PRIMARY KEY (parent, timestamp), + FOREIGN KEY (parent) REFERENCES processes(id) ); CREATE TABLE process_efforts ( - parent_id INTEGER NOT NULL, + parent INTEGER NOT NULL, timestamp TEXT NOT NULL, effort REAL NOT NULL, - PRIMARY KEY (parent_id, timestamp), - FOREIGN KEY (parent_id) REFERENCES processes(id) + PRIMARY KEY (parent, timestamp), + FOREIGN KEY (parent) REFERENCES processes(id) ); CREATE TABLE process_fulfills ( process INTEGER NOT NULL, @@ -50,19 +50,19 @@ CREATE TABLE process_fulfills ( ); CREATE TABLE process_steps ( id INTEGER PRIMARY KEY, - owner_id INTEGER NOT NULL, - step_process_id INTEGER NOT NULL, - parent_step_id INTEGER, - FOREIGN KEY (owner_id) REFERENCES processes(id), - FOREIGN KEY (step_process_id) REFERENCES processes(id), - FOREIGN KEY (parent_step_id) REFERENCES process_steps(step_id) + owner INTEGER NOT NULL, + step_process INTEGER NOT NULL, + parent_step INTEGER, + FOREIGN KEY (owner) REFERENCES processes(id), + FOREIGN KEY (step_process) REFERENCES processes(id), + FOREIGN KEY (parent_step) REFERENCES process_steps(step_id) ); CREATE TABLE process_titles ( - parent_id INTEGER NOT NULL, + parent INTEGER NOT NULL, timestamp TEXT NOT NULL, title TEXT NOT NULL, - PRIMARY KEY (parent_id, timestamp), - FOREIGN KEY (parent_id) REFERENCES processes(id) + PRIMARY KEY (parent, timestamp), + FOREIGN KEY (parent) REFERENCES processes(id) ); CREATE TABLE process_undoes ( process INTEGER NOT NULL, @@ -104,9 +104,9 @@ CREATE TABLE todo_undoes ( ); CREATE TABLE todos ( id INTEGER PRIMARY KEY, - process_id INTEGER NOT NULL, + process INTEGER NOT NULL, is_done BOOLEAN NOT NULL, day TEXT NOT NULL, - FOREIGN KEY (process_id) REFERENCES processes(id), + FOREIGN KEY (process) REFERENCES processes(id), FOREIGN KEY (day) REFERENCES days(id) ); -- 2.30.2 From 89624d5e05480c832a079008bbb9992f411be0dd Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 06:14:25 +0200 Subject: [PATCH 06/16] Refactor updates of relations tables. --- plomtask/db.py | 18 ++++++++++++++++-- plomtask/misc.py | 8 +++----- plomtask/processes.py | 23 +++++++---------------- plomtask/todos.py | 34 ++++++++-------------------------- 4 files changed, 34 insertions(+), 49 deletions(-) diff --git a/plomtask/db.py b/plomtask/db.py index abd8f61..bf633e2 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -67,6 +67,20 @@ class DatabaseConnection: """Close DB connection.""" self.conn.close() + def rewrite_relations(self, table_name: str, key: str, target: int, + rows: list[list[Any]]) -> None: + """Rewrite relations in table_name to target, with rows values.""" + self.exec(f'DELETE FROM {table_name} WHERE {key} = ?', (target,)) + for row in rows: + values = tuple([target] + row) + q_marks = self.__class__.q_marks_from_values(values) + self.exec(f'INSERT INTO {table_name} VALUES {q_marks}', values) + + @staticmethod + def q_marks_from_values(values: tuple[Any]) -> str: + """Return placeholder to insert values into SQL code.""" + return '(' + ','.join(['?'] * len(values)) + ')' + class BaseModel: """Template for most of the models we use/derive from the DB.""" @@ -114,11 +128,11 @@ class BaseModel: def save_core(self, db_conn: DatabaseConnection, update_with_lastrowid: bool = True) -> None: """Write bare-bones self (sans connected items), ensuring self.id_.""" - q_marks = ','.join(['?'] * (len(self.to_save) + 1)) values = tuple([self.id_] + [getattr(self, key) for key in self.to_save]) + q_marks = DatabaseConnection.q_marks_from_values(values) table_name = self.table_name - cursor = db_conn.exec(f'REPLACE INTO {table_name} VALUES ({q_marks})', + cursor = db_conn.exec(f'REPLACE INTO {table_name} VALUES {q_marks}', values) if update_with_lastrowid: self.id_ = cursor.lastrowid diff --git a/plomtask/misc.py b/plomtask/misc.py index dcaad17..efa8898 100644 --- a/plomtask/misc.py +++ b/plomtask/misc.py @@ -46,8 +46,6 @@ class VersionedAttribute: def save(self, db_conn: DatabaseConnection) -> None: """Save as self.history entries, but first wipe old ones.""" - db_conn.exec(f'DELETE FROM {self.table_name} WHERE parent = ?', - (self.parent.id_,)) - for timestamp, value in self.history.items(): - db_conn.exec(f'INSERT INTO {self.table_name} VALUES (?, ?, ?)', - (self.parent.id_, timestamp, value)) + db_conn.rewrite_relations(self.table_name, 'parent', self.parent.id_, + [[item[0], item[1]] + for item in self.history.items()]) diff --git a/plomtask/processes.py b/plomtask/processes.py index 9c5c824..a3682c5 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -183,25 +183,16 @@ class Process(BaseModel): def save(self, db_conn: DatabaseConnection) -> None: """Add (or re-write) self and connected items to DB.""" self.save_core(db_conn) + assert isinstance(self.id_, int) self.title.save(db_conn) self.description.save(db_conn) self.effort.save(db_conn) - db_conn.exec('DELETE FROM process_conditions WHERE process = ?', - (self.id_,)) - for condition in self.conditions: - db_conn.exec('INSERT INTO process_conditions VALUES (?,?)', - (self.id_, condition.id_)) - db_conn.exec('DELETE FROM process_fulfills WHERE process = ?', - (self.id_,)) - for condition in self.fulfills: - db_conn.exec('INSERT INTO process_fulfills VALUES (?,?)', - (self.id_, condition.id_)) - db_conn.exec('DELETE FROM process_undoes WHERE process = ?', - (self.id_,)) - for condition in self.undoes: - db_conn.exec('INSERT INTO process_undoes VALUES (?,?)', - (self.id_, condition.id_)) - assert isinstance(self.id_, int) + db_conn.rewrite_relations('process_conditions', 'process', self.id_, + [[c.id_] for c in self.conditions]) + db_conn.rewrite_relations('process_fulfills', 'process', self.id_, + [[c.id_] for c in self.fulfills]) + db_conn.rewrite_relations('process_undoes', 'process', self.id_, + [[c.id_] for c in self.undoes]) db_conn.exec('DELETE FROM process_steps WHERE owner = ?', (self.id_,)) for step in self.explicit_steps: diff --git a/plomtask/todos.py b/plomtask/todos.py index 840c298..348dbdd 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -181,29 +181,11 @@ class Todo(BaseModel): self.save_core(db_conn) assert isinstance(self.id_, int) db_conn.cached_todos[self.id_] = self - db_conn.exec('DELETE FROM todo_children WHERE parent = ?', - (self.id_,)) - for child in self.children: - db_conn.exec('INSERT INTO todo_children VALUES (?, ?)', - (self.id_, child.id_)) - db_conn.exec('DELETE FROM todo_fulfills WHERE todo = ?', (self.id_,)) - for condition in self.fulfills: - if condition.id_ is None: - raise NotFoundException('Fulfilled Condition of Todo ' - 'without ID (not saved?)') - db_conn.exec('INSERT INTO todo_fulfills VALUES (?, ?)', - (self.id_, condition.id_)) - db_conn.exec('DELETE FROM todo_undoes WHERE todo = ?', (self.id_,)) - for condition in self.undoes: - if condition.id_ is None: - raise NotFoundException('Undone Condition of Todo ' - 'without ID (not saved?)') - db_conn.exec('INSERT INTO todo_undoes VALUES (?, ?)', - (self.id_, condition.id_)) - db_conn.exec('DELETE FROM todo_conditions WHERE todo = ?', (self.id_,)) - for condition in self.conditions: - if condition.id_ is None: - raise NotFoundException('Condition of Todo ' - 'without ID (not saved?)') - db_conn.exec('INSERT INTO todo_conditions VALUES (?, ?)', - (self.id_, condition.id_)) + db_conn.rewrite_relations('todo_children', 'parent', self.id_, + [[c.id_] for c in self.children]) + db_conn.rewrite_relations('todo_conditions', 'todo', self.id_, + [[c.id_] for c in self.conditions]) + db_conn.rewrite_relations('todo_fulfills', 'todo', self.id_, + [[c.id_] for c in self.fulfills]) + db_conn.rewrite_relations('todo_undoes', 'todo', self.id_, + [[c.id_] for c in self.undoes]) -- 2.30.2 From 7adcf651f0053f0dc5d719457a016a0d5b12253b Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 06:46:11 +0200 Subject: [PATCH 07/16] Refactor VersionedAttributes, ProcessSteps, and Conditions retrieval. --- plomtask/conditions.py | 12 ++++-------- plomtask/db.py | 5 +++++ plomtask/misc.py | 5 +++++ plomtask/processes.py | 36 ++++++++++++++---------------------- 4 files changed, 28 insertions(+), 30 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index 98a0b99..b87e3ac 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -25,14 +25,10 @@ class Condition(BaseModel): """Build condition from row, including VersionedAttributes.""" condition = super().from_table_row(db_conn, row) assert isinstance(condition, Condition) - for title_row in db_conn.exec('SELECT * FROM condition_titles ' - 'WHERE parent = ?', (row[0],)): - condition.title.history[title_row[1]]\ - = title_row[2] # pylint: disable=no-member - for desc_row in db_conn.exec('SELECT * FROM condition_descriptions ' - 'WHERE parent = ?', (row[0],)): - condition.description.history[desc_row[1]]\ - = desc_row[2] # pylint: disable=no-member + for name in ('title', 'description'): + table_name = f'condition_{name}s' + for row_ in db_conn.all_where(table_name, 'parent', row[0]): + getattr(condition, name).history_from_row(row_) return condition @classmethod diff --git a/plomtask/db.py b/plomtask/db.py index bf633e2..f45d2b6 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -76,6 +76,11 @@ class DatabaseConnection: q_marks = self.__class__.q_marks_from_values(values) self.exec(f'INSERT INTO {table_name} VALUES {q_marks}', values) + def all_where(self, table_name: str, key: str, target: int) -> list[Row]: + """Return list of Rows at table where key == target.""" + return list(self.exec(f'SELECT * FROM {table_name} WHERE {key} = ?', + (target,))) + @staticmethod def q_marks_from_values(values: tuple[Any]) -> str: """Return placeholder to insert values into SQL code.""" diff --git a/plomtask/misc.py b/plomtask/misc.py index efa8898..5759c0d 100644 --- a/plomtask/misc.py +++ b/plomtask/misc.py @@ -1,6 +1,7 @@ """Attributes whose values are recorded as a timestamped history.""" from datetime import datetime from typing import Any +from sqlite3 import Row from plomtask.db import DatabaseConnection @@ -32,6 +33,10 @@ class VersionedAttribute: or value != self.history[self._newest_timestamp]: self.history[datetime.now().strftime('%Y-%m-%d %H:%M:%S')] = value + def history_from_row(self, row: Row) -> None: + """Extend self.history from expected table row format.""" + self.history[row[1]] = row[2] + def at(self, queried_time: str) -> str | float: """Retrieve value of timestamp nearest queried_time from the past.""" sorted_timestamps = sorted(self.history.keys()) diff --git a/plomtask/processes.py b/plomtask/processes.py index a3682c5..e5851d0 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -47,28 +47,20 @@ class Process(BaseModel): if not create: raise NotFoundException(f'Process not found of id: {id_}') process = Process(id_) - for row in db_conn.exec('SELECT * FROM process_titles ' - 'WHERE parent = ?', (process.id_,)): - process.title.history[row[1]] = row[2] - for row in db_conn.exec('SELECT * FROM process_descriptions ' - 'WHERE parent = ?', (process.id_,)): - process.description.history[row[1]] = row[2] - for row in db_conn.exec('SELECT * FROM process_efforts ' - 'WHERE parent = ?', (process.id_,)): - process.effort.history[row[1]] = row[2] - for row in db_conn.exec('SELECT * FROM process_steps ' - 'WHERE owner = ?', (process.id_,)): - process.explicit_steps += [ProcessStep.from_table_row(db_conn, - row)] - for row in db_conn.exec('SELECT condition FROM process_conditions ' - 'WHERE process = ?', (process.id_,)): - process.conditions += [Condition.by_id(db_conn, row[0])] - for row in db_conn.exec('SELECT condition FROM process_fulfills ' - 'WHERE process = ?', (process.id_,)): - process.fulfills += [Condition.by_id(db_conn, row[0])] - for row in db_conn.exec('SELECT condition FROM process_undoes ' - 'WHERE process = ?', (process.id_,)): - process.undoes += [Condition.by_id(db_conn, row[0])] + if isinstance(process.id_, int): + for name in ('title', 'description', 'effort'): + table = f'process_{name}s' + for row in db_conn.all_where(table, 'parent', process.id_): + getattr(process, name).history_from_row(row) + for row in db_conn.all_where('process_steps', 'owner', + process.id_): + step = ProcessStep.from_table_row(db_conn, row) + process.explicit_steps += [step] + for name in ('conditions', 'fulfills', 'undoes'): + table = f'process_{name}' + for row in db_conn.all_where(table, 'process', process.id_): + target = getattr(process, name) + target += [Condition.by_id(db_conn, row[1])] assert isinstance(process, Process) return process -- 2.30.2 From 2d0d3a138de69e5e09208936ac094b53b0785c0b Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 07:26:01 +0200 Subject: [PATCH 08/16] Hide (almost all) remaining SQL code in DB module. --- plomtask/conditions.py | 8 +++---- plomtask/db.py | 24 +++++++++++++++---- plomtask/http.py | 4 ++-- plomtask/processes.py | 30 ++++++++++++------------ plomtask/todos.py | 52 +++++++++++++++++++----------------------- tests/todos.py | 6 ++--- 6 files changed, 69 insertions(+), 55 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index b87e3ac..9a44200 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -27,7 +27,7 @@ class Condition(BaseModel): assert isinstance(condition, Condition) for name in ('title', 'description'): table_name = f'condition_{name}s' - for row_ in db_conn.all_where(table_name, 'parent', row[0]): + for row_ in db_conn.row_where(table_name, 'parent', row[0]): getattr(condition, name).history_from_row(row_) return condition @@ -38,9 +38,9 @@ class Condition(BaseModel): for id_, condition in db_conn.cached_conditions.items(): conditions[id_] = condition already_recorded = conditions.keys() - for row in db_conn.exec('SELECT id FROM conditions'): - if row[0] not in already_recorded: - condition = cls.by_id(db_conn, row[0]) + for id_ in db_conn.column_all('conditions', 'id'): + if id_ not in already_recorded: + condition = cls.by_id(db_conn, id_) conditions[condition.id_] = condition return list(conditions.values()) diff --git a/plomtask/db.py b/plomtask/db.py index f45d2b6..848e750 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -70,17 +70,34 @@ class DatabaseConnection: def rewrite_relations(self, table_name: str, key: str, target: int, rows: list[list[Any]]) -> None: """Rewrite relations in table_name to target, with rows values.""" - self.exec(f'DELETE FROM {table_name} WHERE {key} = ?', (target,)) + self.delete_where(table_name, key, target) for row in rows: values = tuple([target] + row) q_marks = self.__class__.q_marks_from_values(values) self.exec(f'INSERT INTO {table_name} VALUES {q_marks}', values) - def all_where(self, table_name: str, key: str, target: int) -> list[Row]: + def row_where(self, table_name: str, key: str, + target: int | str) -> list[Row]: """Return list of Rows at table where key == target.""" return list(self.exec(f'SELECT * FROM {table_name} WHERE {key} = ?', (target,))) + def column_where(self, table_name: str, column: str, key: str, + target: int | str) -> list[Any]: + """Return column of table where key == target.""" + return [row[0] for row in + self.exec(f'SELECT {column} FROM {table_name} ' + f'WHERE {key} = ?', (target,))] + + def column_all(self, table_name: str, column: str) -> list[Any]: + """Return complete column of table.""" + return [row[0] for row in + self.exec(f'SELECT {column} FROM {table_name}')] + + def delete_where(self, table_name: str, key: str, target: int) -> None: + """Delete from table where key == target.""" + self.exec(f'DELETE FROM {table_name} WHERE {key} = ?', (target,)) + @staticmethod def q_marks_from_values(values: tuple[Any]) -> str: """Return placeholder to insert values into SQL code.""" @@ -116,8 +133,7 @@ class BaseModel: obj = cache[id_] from_cache = True else: - for row in db_conn.exec(f'SELECT * FROM {cls.table_name} ' - 'WHERE id = ?', (id_,)): + for row in db_conn.row_where(cls.table_name, 'id', id_): obj = cls.from_table_row(db_conn, row) cache[id_] = obj break diff --git a/plomtask/http.py b/plomtask/http.py index 5f739b6..cc4358c 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -134,7 +134,7 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_todo(self) -> dict[str, object]: """Show single Todo of ?id=.""" - id_ = self.params.get_int_or_none('id') + id_ = self.params.get_int('id') todo = Todo.by_id(self.conn, id_) return {'todo': todo, 'todo_candidates': Todo.by_date(self.conn, todo.date), @@ -198,7 +198,7 @@ class TaskHandler(BaseHTTPRequestHandler): def do_POST_todo(self) -> None: """Update Todo and its children.""" - id_ = self.params.get_int_or_none('id') + id_ = self.params.get_int('id') todo = Todo.by_id(self.conn, id_) child_id = self.form_data.get_int_or_none('adopt') if child_id is not None: diff --git a/plomtask/processes.py b/plomtask/processes.py index e5851d0..490acc3 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -30,9 +30,9 @@ class Process(BaseModel): for id_, process in db_conn.cached_processes.items(): processes[id_] = process already_recorded = processes.keys() - for row in db_conn.exec('SELECT id FROM processes'): - if row[0] not in already_recorded: - process = cls.by_id(db_conn, row[0]) + for id_ in db_conn.column_all('processes', 'id'): + if id_ not in already_recorded: + process = cls.by_id(db_conn, id_) processes[process.id_] = process return list(processes.values()) @@ -50,26 +50,29 @@ class Process(BaseModel): if isinstance(process.id_, int): for name in ('title', 'description', 'effort'): table = f'process_{name}s' - for row in db_conn.all_where(table, 'parent', process.id_): + for row in db_conn.row_where(table, 'parent', process.id_): getattr(process, name).history_from_row(row) - for row in db_conn.all_where('process_steps', 'owner', + for row in db_conn.row_where('process_steps', 'owner', process.id_): step = ProcessStep.from_table_row(db_conn, row) process.explicit_steps += [step] for name in ('conditions', 'fulfills', 'undoes'): table = f'process_{name}' - for row in db_conn.all_where(table, 'process', process.id_): + for cond_id in db_conn.column_where(table, 'condition', + 'process', process.id_): target = getattr(process, name) - target += [Condition.by_id(db_conn, row[1])] + target += [Condition.by_id(db_conn, cond_id)] assert isinstance(process, Process) return process def used_as_step_by(self, db_conn: DatabaseConnection) -> list[Process]: """Return Processes using self for a ProcessStep.""" + if not self.id_: + return [] owner_ids = set() - for owner_id in db_conn.exec('SELECT owner FROM process_steps WHERE' - ' step_process = ?', (self.id_,)): - owner_ids.add(owner_id[0]) + for id_ in db_conn.column_where('process_steps', 'owner', + 'step_process', self.id_): + owner_ids.add(id_) return [self.__class__.by_id(db_conn, id_) for id_ in owner_ids] def get_steps(self, db_conn: DatabaseConnection, external_owner: @@ -162,12 +165,12 @@ class Process(BaseModel): def set_steps(self, db_conn: DatabaseConnection, steps: list[tuple[int | None, int, int | None]]) -> None: """Set self.explicit_steps in bulk.""" + assert isinstance(self.id_, int) for step in self.explicit_steps: assert isinstance(step.id_, int) del db_conn.cached_process_steps[step.id_] self.explicit_steps = [] - db_conn.exec('DELETE FROM process_steps WHERE owner = ?', - (self.id_,)) + db_conn.delete_where('process_steps', 'owner', self.id_) for step_tuple in steps: self._add_step(db_conn, step_tuple[0], step_tuple[1], step_tuple[2]) @@ -185,8 +188,7 @@ class Process(BaseModel): [[c.id_] for c in self.fulfills]) db_conn.rewrite_relations('process_undoes', 'process', self.id_, [[c.id_] for c in self.undoes]) - db_conn.exec('DELETE FROM process_steps WHERE owner = ?', - (self.id_,)) + db_conn.delete_where('process_steps', 'owner', self.id_) for step in self.explicit_steps: step.save(db_conn) db_conn.cached_processes[self.id_] = self diff --git a/plomtask/todos.py b/plomtask/todos.py index 348dbdd..fd72af6 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -47,30 +47,24 @@ class Todo(BaseModel): return todo @classmethod - def by_id(cls, db_conn: DatabaseConnection, id_: int | None) -> Todo: + def by_id(cls, db_conn: DatabaseConnection, id_: int) -> Todo: """Get Todo of .id_=id_ and children (from DB cache if possible).""" - if id_: - todo, from_cache = super()._by_id(db_conn, id_) - else: - todo, from_cache = None, False + todo, from_cache = super()._by_id(db_conn, id_) if todo is None: raise NotFoundException(f'Todo of ID not found: {id_}') if not from_cache: - for row in db_conn.exec('SELECT child FROM todo_children ' - 'WHERE parent = ?', (id_,)): - todo.children += [cls.by_id(db_conn, row[0])] - for row in db_conn.exec('SELECT parent FROM todo_children ' - 'WHERE child = ?', (id_,)): - todo.parents += [cls.by_id(db_conn, row[0])] - for row in db_conn.exec('SELECT condition FROM todo_conditions ' - 'WHERE todo = ?', (id_,)): - todo.conditions += [Condition.by_id(db_conn, row[0])] - for row in db_conn.exec('SELECT condition FROM todo_fulfills ' - 'WHERE todo = ?', (id_,)): - todo.fulfills += [Condition.by_id(db_conn, row[0])] - for row in db_conn.exec('SELECT condition FROM todo_undoes ' - 'WHERE todo = ?', (id_,)): - todo.undoes += [Condition.by_id(db_conn, row[0])] + for t_id in db_conn.column_where('todo_children', 'child', + 'parent', id_): + todo.children += [cls.by_id(db_conn, t_id)] + for t_id in db_conn.column_where('todo_children', 'parent', + 'child', id_): + todo.parents += [cls.by_id(db_conn, t_id)] + for name in ('conditions', 'fulfills', 'undoes'): + table = f'todo_{name}' + for cond_id in db_conn.column_where(table, 'condition', + 'todo', todo.id_): + target = getattr(todo, name) + target += [Condition.by_id(db_conn, cond_id)] assert isinstance(todo, Todo) return todo @@ -78,18 +72,19 @@ class Todo(BaseModel): def by_date(cls, db_conn: DatabaseConnection, date: str) -> list[Todo]: """Collect all Todos for Day of date.""" todos = [] - for row in db_conn.exec('SELECT id FROM todos WHERE day = ?', (date,)): - todos += [cls.by_id(db_conn, row[0])] + for id_ in db_conn.column_where('todos', 'id', 'day', date): + todos += [cls.by_id(db_conn, id_)] return todos @classmethod def enablers_for_at(cls, db_conn: DatabaseConnection, condition: Condition, date: str) -> list[Todo]: """Collect all Todos of day that enable condition.""" + assert isinstance(condition.id_, int) enablers = [] - for row in db_conn.exec('SELECT todo FROM todo_fulfills ' - 'WHERE condition = ?', (condition.id_,)): - todo = cls.by_id(db_conn, row[0]) + for id_ in db_conn.column_where('todo_fulfills', 'todo', 'condition', + condition.id_): + todo = cls.by_id(db_conn, id_) if todo.date == date: enablers += [todo] return enablers @@ -98,10 +93,11 @@ class Todo(BaseModel): def disablers_for_at(cls, db_conn: DatabaseConnection, condition: Condition, date: str) -> list[Todo]: """Collect all Todos of day that disable condition.""" + assert isinstance(condition.id_, int) disablers = [] - for row in db_conn.exec('SELECT todo FROM todo_undoes ' - 'WHERE condition = ?', (condition.id_,)): - todo = cls.by_id(db_conn, row[0]) + for id_ in db_conn.column_where('todo_undoes', 'todo', 'condition', + condition.id_): + todo = cls.by_id(db_conn, id_) if todo.date == date: disablers += [todo] return disablers diff --git a/tests/todos.py b/tests/todos.py index 17454c5..426bb91 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -210,7 +210,7 @@ class TestsWithServer(TestCaseWithServer): self.check_post(form_data, '/day?date=2024-01-01', 302, '/') form_data = {} self.check_post(form_data, '/todo=', 404) - self.check_post(form_data, '/todo?id=', 404) + self.check_post(form_data, '/todo?id=', 400) self.check_post(form_data, '/todo?id=FOO', 400) self.check_post(form_data, '/todo?id=0', 404) todo1 = post_and_reload(form_data) @@ -249,8 +249,8 @@ class TestsWithServer(TestCaseWithServer): self.check_post(form_data, '/process?id=', 302, '/') form_data = {'comment': '', 'new_todo': 1} self.check_post(form_data, '/day?date=2024-01-01', 302, '/') - self.check_get('/todo', 404) - self.check_get('/todo?id=', 404) + self.check_get('/todo', 400) + self.check_get('/todo?id=', 400) self.check_get('/todo?id=foo', 400) self.check_get('/todo?id=0', 404) self.check_get('/todo?id=1', 200) -- 2.30.2 From 8f6b4ec61b126d6edbfda4f20d62398d92390a95 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 07:55:22 +0200 Subject: [PATCH 09/16] Clean up enablers/disablers code and naming conventions. --- plomtask/http.py | 12 +++---- plomtask/processes.py | 29 +++++++++-------- plomtask/todos.py | 72 +++++++++++++++++++++--------------------- scripts/init.sql | 20 ++++++------ templates/process.html | 14 ++++---- templates/todo.html | 16 +++++----- tests/processes.py | 8 ++--- tests/todos.py | 32 +++++++++---------- 8 files changed, 102 insertions(+), 101 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index cc4358c..4bf58b4 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -205,13 +205,13 @@ class TaskHandler(BaseHTTPRequestHandler): child = Todo.by_id(self.conn, child_id) todo.add_child(child) todo.set_conditions(self.conn, self.form_data.get_all_int('condition')) - todo.set_fulfills(self.conn, self.form_data.get_all_int('fulfills')) - todo.set_undoes(self.conn, self.form_data.get_all_int('undoes')) + todo.set_enables(self.conn, self.form_data.get_all_int('enables')) + todo.set_disables(self.conn, self.form_data.get_all_int('disables')) todo.is_done = len(self.form_data.get_all_str('done')) > 0 todo.save(self.conn) - for condition in todo.fulfills: + for condition in todo.enables: condition.save(self.conn) - for condition in todo.undoes: + for condition in todo.disables: condition.save(self.conn) def do_POST_process(self) -> None: @@ -223,8 +223,8 @@ class TaskHandler(BaseHTTPRequestHandler): process.effort.set(self.form_data.get_float('effort')) process.set_conditions(self.conn, self.form_data.get_all_int('condition')) - process.set_fulfills(self.conn, self.form_data.get_all_int('fulfills')) - process.set_undoes(self.conn, self.form_data.get_all_int('undoes')) + process.set_enables(self.conn, self.form_data.get_all_int('enables')) + process.set_disables(self.conn, self.form_data.get_all_int('disables')) process.save_core(self.conn) assert process.id_ is not None # for mypy process.explicit_steps = [] diff --git a/plomtask/processes.py b/plomtask/processes.py index 490acc3..0e8846d 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -20,8 +20,8 @@ class Process(BaseModel): self.effort = VersionedAttribute(self, 'process_efforts', 1.0) self.explicit_steps: list[ProcessStep] = [] self.conditions: list[Condition] = [] - self.fulfills: list[Condition] = [] - self.undoes: list[Condition] = [] + self.enables: list[Condition] = [] + self.disables: list[Condition] = [] @classmethod def all(cls, db_conn: DatabaseConnection) -> list[Process]: @@ -56,7 +56,7 @@ class Process(BaseModel): process.id_): step = ProcessStep.from_table_row(db_conn, row) process.explicit_steps += [step] - for name in ('conditions', 'fulfills', 'undoes'): + for name in ('conditions', 'enables', 'disables'): table = f'process_{name}' for cond_id in db_conn.column_where(table, 'condition', 'process', process.id_): @@ -119,14 +119,15 @@ class Process(BaseModel): for id_ in ids: trgt_list += [Condition.by_id(db_conn, id_)] - def set_fulfills(self, db_conn: DatabaseConnection, - ids: list[int]) -> None: - """Set self.fulfills to Conditions identified by ids.""" - self.set_conditions(db_conn, ids, 'fulfills') + def set_enables(self, db_conn: DatabaseConnection, + ids: list[int]) -> None: + """Set self.enables to Conditions identified by ids.""" + self.set_conditions(db_conn, ids, 'enables') - def set_undoes(self, db_conn: DatabaseConnection, ids: list[int]) -> None: - """Set self.undoes to Conditions identified by ids.""" - self.set_conditions(db_conn, ids, 'undoes') + def set_disables(self, db_conn: DatabaseConnection, + ids: list[int]) -> None: + """Set self.disables to Conditions identified by ids.""" + self.set_conditions(db_conn, ids, 'disables') def _add_step(self, db_conn: DatabaseConnection, @@ -184,10 +185,10 @@ class Process(BaseModel): self.effort.save(db_conn) db_conn.rewrite_relations('process_conditions', 'process', self.id_, [[c.id_] for c in self.conditions]) - db_conn.rewrite_relations('process_fulfills', 'process', self.id_, - [[c.id_] for c in self.fulfills]) - db_conn.rewrite_relations('process_undoes', 'process', self.id_, - [[c.id_] for c in self.undoes]) + db_conn.rewrite_relations('process_enables', 'process', self.id_, + [[c.id_] for c in self.enables]) + db_conn.rewrite_relations('process_disables', 'process', self.id_, + [[c.id_] for c in self.disables]) db_conn.delete_where('process_steps', 'owner', self.id_) for step in self.explicit_steps: step.save(db_conn) diff --git a/plomtask/todos.py b/plomtask/todos.py index fd72af6..ebe35ac 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -26,12 +26,12 @@ class Todo(BaseModel): self.children: list[Todo] = [] self.parents: list[Todo] = [] self.conditions: list[Condition] = [] - self.fulfills: list[Condition] = [] - self.undoes: list[Condition] = [] + self.enables: list[Condition] = [] + self.disables: list[Condition] = [] if not self.id_: self.conditions = process.conditions[:] - self.fulfills = process.fulfills[:] - self.undoes = process.undoes[:] + self.enables = process.enables[:] + self.disables = process.disables[:] @classmethod def from_table_row(cls, db_conn: DatabaseConnection, @@ -59,7 +59,7 @@ class Todo(BaseModel): for t_id in db_conn.column_where('todo_children', 'parent', 'child', id_): todo.parents += [cls.by_id(db_conn, t_id)] - for name in ('conditions', 'fulfills', 'undoes'): + for name in ('conditions', 'enables', 'disables'): table = f'todo_{name}' for cond_id in db_conn.column_where(table, 'condition', 'todo', todo.id_): @@ -76,31 +76,30 @@ class Todo(BaseModel): todos += [cls.by_id(db_conn, id_)] return todos + @staticmethod + def _x_ablers_for_at(db_conn: DatabaseConnection, name: str, + cond: Condition, date: str) -> list[Todo]: + """Collect all Todos of day that [name] condition.""" + assert isinstance(cond.id_, int) + x_ablers = [] + table = f'todo_{name}' + for id_ in db_conn.column_where(table, 'todo', 'condition', cond.id_): + todo = Todo.by_id(db_conn, id_) + if todo.date == date: + x_ablers += [todo] + return x_ablers + @classmethod - def enablers_for_at(cls, db_conn: DatabaseConnection, condition: Condition, - date: str) -> list[Todo]: + def enablers_for_at(cls, db_conn: DatabaseConnection, + condition: Condition, date: str) -> list[Todo]: """Collect all Todos of day that enable condition.""" - assert isinstance(condition.id_, int) - enablers = [] - for id_ in db_conn.column_where('todo_fulfills', 'todo', 'condition', - condition.id_): - todo = cls.by_id(db_conn, id_) - if todo.date == date: - enablers += [todo] - return enablers + return cls._x_ablers_for_at(db_conn, 'enables', condition, date) @classmethod def disablers_for_at(cls, db_conn: DatabaseConnection, condition: Condition, date: str) -> list[Todo]: """Collect all Todos of day that disable condition.""" - assert isinstance(condition.id_, int) - disablers = [] - for id_ in db_conn.column_where('todo_undoes', 'todo', 'condition', - condition.id_): - todo = cls.by_id(db_conn, id_) - if todo.date == date: - disablers += [todo] - return disablers + return cls._x_ablers_for_at(db_conn, 'disables', condition, date) @property def is_doable(self) -> bool: @@ -130,19 +129,20 @@ class Todo(BaseModel): if self._is_done != value: self._is_done = value if value is True: - for condition in self.fulfills: + for condition in self.enables: condition.is_active = True - for condition in self.undoes: + for condition in self.disables: condition.is_active = False - def set_undoes(self, db_conn: DatabaseConnection, ids: list[int]) -> None: - """Set self.undoes to Conditions identified by ids.""" - self.set_conditions(db_conn, ids, 'undoes') - - def set_fulfills(self, db_conn: DatabaseConnection, + def set_disables(self, db_conn: DatabaseConnection, ids: list[int]) -> None: - """Set self.fulfills to Conditions identified by ids.""" - self.set_conditions(db_conn, ids, 'fulfills') + """Set self.disables to Conditions identified by ids.""" + self.set_conditions(db_conn, ids, 'disables') + + def set_enables(self, db_conn: DatabaseConnection, + ids: list[int]) -> None: + """Set self.enables to Conditions identified by ids.""" + self.set_conditions(db_conn, ids, 'enables') def set_conditions(self, db_conn: DatabaseConnection, ids: list[int], target: str = 'conditions') -> None: @@ -181,7 +181,7 @@ class Todo(BaseModel): [[c.id_] for c in self.children]) db_conn.rewrite_relations('todo_conditions', 'todo', self.id_, [[c.id_] for c in self.conditions]) - db_conn.rewrite_relations('todo_fulfills', 'todo', self.id_, - [[c.id_] for c in self.fulfills]) - db_conn.rewrite_relations('todo_undoes', 'todo', self.id_, - [[c.id_] for c in self.undoes]) + db_conn.rewrite_relations('todo_enables', 'todo', self.id_, + [[c.id_] for c in self.enables]) + db_conn.rewrite_relations('todo_disables', 'todo', self.id_, + [[c.id_] for c in self.disables]) diff --git a/scripts/init.sql b/scripts/init.sql index 5fdd779..b2979a5 100644 --- a/scripts/init.sql +++ b/scripts/init.sql @@ -34,6 +34,13 @@ CREATE TABLE process_descriptions ( PRIMARY KEY (parent, timestamp), FOREIGN KEY (parent) REFERENCES processes(id) ); +CREATE TABLE process_disables ( + process INTEGER NOT NULL, + condition INTEGER NOT NULL, + PRIMARY KEY(process, condition), + FOREIGN KEY (process) REFERENCES processes(id), + FOREIGN KEY (condition) REFERENCES conditions(id) +); CREATE TABLE process_efforts ( parent INTEGER NOT NULL, timestamp TEXT NOT NULL, @@ -41,7 +48,7 @@ CREATE TABLE process_efforts ( PRIMARY KEY (parent, timestamp), FOREIGN KEY (parent) REFERENCES processes(id) ); -CREATE TABLE process_fulfills ( +CREATE TABLE process_enables ( process INTEGER NOT NULL, condition INTEGER NOT NULL, PRIMARY KEY(process, condition), @@ -64,13 +71,6 @@ CREATE TABLE process_titles ( PRIMARY KEY (parent, timestamp), FOREIGN KEY (parent) REFERENCES processes(id) ); -CREATE TABLE process_undoes ( - process INTEGER NOT NULL, - condition INTEGER NOT NULL, - PRIMARY KEY(process, condition), - FOREIGN KEY (process) REFERENCES processes(id), - FOREIGN KEY (condition) REFERENCES conditions(id) -); CREATE TABLE processes ( id INTEGER PRIMARY KEY ); @@ -88,14 +88,14 @@ CREATE TABLE todo_conditions ( FOREIGN KEY (todo) REFERENCES todos(id), FOREIGN KEY (condition) REFERENCES conditions(id) ); -CREATE TABLE todo_fulfills ( +CREATE TABLE todo_disables ( todo INTEGER NOT NULL, condition INTEGER NOT NULL, PRIMARY KEY(todo, condition), FOREIGN KEY (todo) REFERENCES todos(id), FOREIGN KEY (condition) REFERENCES conditions(id) ); -CREATE TABLE todo_undoes ( +CREATE TABLE todo_enables ( todo INTEGER NOT NULL, condition INTEGER NOT NULL, PRIMARY KEY(todo, condition), diff --git a/templates/process.html b/templates/process.html index b55ee07..85651d7 100644 --- a/templates/process.html +++ b/templates/process.html @@ -56,12 +56,12 @@ add condition: {{condition_candidate.title.newest|e}} {% endfor %} -

fulfills

+

enables

-{% for condition in process.fulfills %} +{% for condition in process.enables %}
- + {{condition.title.newest|e}} @@ -69,13 +69,13 @@ add condition: {% endfor %}
-add fulfills: +add enables:

conditions

-{% for condition in process.undoes %} +{% for condition in process.disables %}
- + {{condition.title.newest|e}} @@ -83,7 +83,7 @@ add fulfills: +add disables:

steps

{% for step_id, step_node in steps.items() %} diff --git a/templates/todo.html b/templates/todo.html index 1fa2922..92b0657 100644 --- a/templates/todo.html +++ b/templates/todo.html @@ -28,12 +28,12 @@ add condition: {{condition_candidate.title.newest|e}} {% endfor %} -

fulfills

+

enables

-{% for condition in todo.fulfills %} +{% for condition in todo.enables %}
- + {{condition.title.newest|e}} @@ -41,13 +41,13 @@ add condition: {% endfor %}
-add fulfills: -

undoes

+add enables: +

disables

-{% for condition in todo.undoes%} +{% for condition in todo.disables%}
- + {{condition.title.newest|e}} @@ -55,7 +55,7 @@ add fulfills: +add disables:

parents

    {% for parent in todo.parents %} diff --git a/tests/processes.py b/tests/processes.py index 491a48f..084cfe0 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -108,8 +108,8 @@ class TestsWithDB(TestCaseWithDB): [self.proc1, self.proc2]) def test_Process_conditions(self) -> None: - """Test setting Process.conditions/fulfills/undoes.""" - for target in ('conditions', 'fulfills', 'undoes'): + """Test setting Process.conditions/enables/disables.""" + for target in ('conditions', 'enables', 'disables'): c1 = Condition(None, False) c1.save(self.db_conn) assert isinstance(c1.id_, int) @@ -195,9 +195,9 @@ class TestsWithServer(TestCaseWithServer): form_data_cond = {'title': 'foo', 'description': 'foo'} self.check_post(form_data_cond, '/condition', 302, '/') self.check_post(form_data, '/process?id=', 302, '/') - form_data['undoes'] = [1] + form_data['disables'] = [1] self.check_post(form_data, '/process?id=', 302, '/') - form_data['fulfills'] = [1] + form_data['enables'] = [1] self.check_post(form_data, '/process?id=', 302, '/') def test_do_GET(self) -> None: diff --git a/tests/todos.py b/tests/todos.py index 426bb91..c704c27 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -55,18 +55,18 @@ class TestsWithDB(TestCaseWithDB): todo.set_conditions(self.db_conn, [self.cond2.id_]) self.assertEqual(todo.conditions, [self.cond2]) self.assertEqual(self.proc.conditions, [self.cond1]) - self.proc.set_fulfills(self.db_conn, [self.cond1.id_]) + self.proc.set_enables(self.db_conn, [self.cond1.id_]) todo = Todo(None, self.proc, False, self.date1) - self.assertEqual(todo.fulfills, [self.cond1]) - todo.set_fulfills(self.db_conn, [self.cond2.id_]) - self.assertEqual(todo.fulfills, [self.cond2]) - self.assertEqual(self.proc.fulfills, [self.cond1]) - self.proc.set_undoes(self.db_conn, [self.cond1.id_]) + self.assertEqual(todo.enables, [self.cond1]) + todo.set_enables(self.db_conn, [self.cond2.id_]) + self.assertEqual(todo.enables, [self.cond2]) + self.assertEqual(self.proc.enables, [self.cond1]) + self.proc.set_disables(self.db_conn, [self.cond1.id_]) todo = Todo(None, self.proc, False, self.date1) - self.assertEqual(todo.undoes, [self.cond1]) - todo.set_undoes(self.db_conn, [self.cond2.id_]) - self.assertEqual(todo.undoes, [self.cond2]) - self.assertEqual(self.proc.undoes, [self.cond1]) + self.assertEqual(todo.disables, [self.cond1]) + todo.set_disables(self.db_conn, [self.cond2.id_]) + self.assertEqual(todo.disables, [self.cond2]) + self.assertEqual(self.proc.disables, [self.cond1]) def test_Todo_on_conditions(self) -> None: """Test effect of Todos on Conditions.""" @@ -74,8 +74,8 @@ class TestsWithDB(TestCaseWithDB): assert isinstance(self.cond2.id_, int) todo = Todo(None, self.proc, False, self.date1) todo.save(self.db_conn) - todo.set_fulfills(self.db_conn, [self.cond1.id_]) - todo.set_undoes(self.db_conn, [self.cond2.id_]) + todo.set_enables(self.db_conn, [self.cond1.id_]) + todo.set_disables(self.db_conn, [self.cond2.id_]) todo.is_done = True self.assertEqual(self.cond1.is_active, True) self.assertEqual(self.cond2.is_active, False) @@ -89,16 +89,16 @@ class TestsWithDB(TestCaseWithDB): assert isinstance(self.cond2.id_, int) todo1 = Todo(None, self.proc, False, self.date1) todo1.save(self.db_conn) - todo1.set_fulfills(self.db_conn, [self.cond1.id_]) - todo1.set_undoes(self.db_conn, [self.cond2.id_]) + todo1.set_enables(self.db_conn, [self.cond1.id_]) + todo1.set_disables(self.db_conn, [self.cond2.id_]) todo1.save(self.db_conn) todo2 = Todo(None, self.proc, False, self.date1) todo2.save(self.db_conn) - todo2.set_fulfills(self.db_conn, [self.cond2.id_]) + todo2.set_enables(self.db_conn, [self.cond2.id_]) todo2.save(self.db_conn) todo3 = Todo(None, self.proc, False, self.date2) todo3.save(self.db_conn) - todo3.set_fulfills(self.db_conn, [self.cond2.id_]) + todo3.set_enables(self.db_conn, [self.cond2.id_]) todo3.save(self.db_conn) enablers = Todo.enablers_for_at(self.db_conn, self.cond1, self.date1) self.assertEqual(enablers, [todo1]) -- 2.30.2 From c2004503dc42449f1fa129b8e56eeef0a6df4712 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 19 Apr 2024 08:08:06 +0200 Subject: [PATCH 10/16] Further refactor Conditions handling. --- plomtask/conditions.py | 23 +++++++++++++++++++++++ plomtask/processes.py | 23 ++--------------------- plomtask/todos.py | 23 ++--------------------- 3 files changed, 27 insertions(+), 42 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index 9a44200..cd147cb 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -66,3 +66,26 @@ class Condition(BaseModel): self.description.save(db_conn) assert isinstance(self.id_, int) db_conn.cached_conditions[self.id_] = self + + +class ConditionsRelations: + """Methods for handling relations to Conditions, for Todo and Process.""" + + def set_conditions(self, db_conn: DatabaseConnection, ids: list[int], + target: str = 'conditions') -> None: + """Set self.[target] to Conditions identified by ids.""" + target_list = getattr(self, target) + while len(target_list) > 0: + target_list.pop() + for id_ in ids: + target_list += [Condition.by_id(db_conn, id_)] + + def set_enables(self, db_conn: DatabaseConnection, + ids: list[int]) -> None: + """Set self.enables to Conditions identified by ids.""" + self.set_conditions(db_conn, ids, 'enables') + + def set_disables(self, db_conn: DatabaseConnection, + ids: list[int]) -> None: + """Set self.disables to Conditions identified by ids.""" + self.set_conditions(db_conn, ids, 'disables') diff --git a/plomtask/processes.py b/plomtask/processes.py index 0e8846d..eb43895 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -3,11 +3,11 @@ from __future__ import annotations from typing import Any, Set from plomtask.db import DatabaseConnection, BaseModel from plomtask.misc import VersionedAttribute -from plomtask.conditions import Condition +from plomtask.conditions import Condition, ConditionsRelations from plomtask.exceptions import NotFoundException, BadFormatException -class Process(BaseModel): +class Process(BaseModel, ConditionsRelations): """Template for, and metadata for, Todos, and their arrangements.""" table_name = 'processes' @@ -110,25 +110,6 @@ class Process(BaseModel): walk_steps(step_id, step_node) return steps - def set_conditions(self, db_conn: DatabaseConnection, ids: list[int], - trgt: str = 'conditions') -> None: - """Set self.[target] to Conditions identified by ids.""" - trgt_list = getattr(self, trgt) - while len(trgt_list) > 0: - trgt_list.pop() - for id_ in ids: - trgt_list += [Condition.by_id(db_conn, id_)] - - def set_enables(self, db_conn: DatabaseConnection, - ids: list[int]) -> None: - """Set self.enables to Conditions identified by ids.""" - self.set_conditions(db_conn, ids, 'enables') - - def set_disables(self, db_conn: DatabaseConnection, - ids: list[int]) -> None: - """Set self.disables to Conditions identified by ids.""" - self.set_conditions(db_conn, ids, 'disables') - def _add_step(self, db_conn: DatabaseConnection, id_: int | None, diff --git a/plomtask/todos.py b/plomtask/todos.py index ebe35ac..d060e23 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -4,12 +4,12 @@ from typing import Any from sqlite3 import Row from plomtask.db import DatabaseConnection, BaseModel from plomtask.processes import Process -from plomtask.conditions import Condition +from plomtask.conditions import Condition, ConditionsRelations from plomtask.exceptions import (NotFoundException, BadFormatException, HandledException) -class Todo(BaseModel): +class Todo(BaseModel, ConditionsRelations): """Individual actionable.""" # pylint: disable=too-many-instance-attributes @@ -134,25 +134,6 @@ class Todo(BaseModel): for condition in self.disables: condition.is_active = False - def set_disables(self, db_conn: DatabaseConnection, - ids: list[int]) -> None: - """Set self.disables to Conditions identified by ids.""" - self.set_conditions(db_conn, ids, 'disables') - - def set_enables(self, db_conn: DatabaseConnection, - ids: list[int]) -> None: - """Set self.enables to Conditions identified by ids.""" - self.set_conditions(db_conn, ids, 'enables') - - def set_conditions(self, db_conn: DatabaseConnection, ids: list[int], - target: str = 'conditions') -> None: - """Set self.[target] to Conditions identified by ids.""" - target_list = getattr(self, target) - while len(target_list) > 0: - target_list.pop() - for id_ in ids: - target_list += [Condition.by_id(db_conn, id_)] - def add_child(self, child: Todo) -> None: """Add child to self.children, guard against recursion""" def walk_steps(node: Todo) -> None: -- 2.30.2 From 54e6c8bccace28583cf9926aa00917a796628a00 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 22 Apr 2024 01:50:41 +0200 Subject: [PATCH 11/16] Improve placement of Todos and Conditions in Day view. --- plomtask/http.py | 7 ++++- plomtask/todos.py | 35 +++++++++++++++++++++ templates/base.html | 8 +++++ templates/day.html | 23 +++++++++++--- tests/todos.py | 76 ++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 142 insertions(+), 7 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 4bf58b4..e541057 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -120,6 +120,11 @@ class TaskHandler(BaseHTTPRequestHandler): """Show single Day of ?date=.""" date = self.params.get_str('date', todays_date()) conditions_listing = [] + top_todos = [t for t in Todo.by_date(self.conn, date) if not t.parents] + seen_todos: set[int] = set() + seen_conditions: set[int] = set() + todo_trees = [t.get_step_tree(seen_todos, seen_conditions) + for t in top_todos] for condition in Condition.all(self.conn): enablers = Todo.enablers_for_at(self.conn, condition, date) disablers = Todo.disablers_for_at(self.conn, condition, date) @@ -128,7 +133,7 @@ class TaskHandler(BaseHTTPRequestHandler): 'enablers': enablers, 'disablers': disablers}] return {'day': Day.by_id(self.conn, date, create=True), - 'todos': Todo.by_date(self.conn, date), + 'todo_trees': todo_trees, 'processes': Process.all(self.conn), 'conditions_listing': conditions_listing} diff --git a/plomtask/todos.py b/plomtask/todos.py index d060e23..336ec03 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -1,5 +1,6 @@ """Actionables.""" from __future__ import annotations +from collections import namedtuple from typing import Any from sqlite3 import Row from plomtask.db import DatabaseConnection, BaseModel @@ -9,6 +10,10 @@ from plomtask.exceptions import (NotFoundException, BadFormatException, HandledException) +TodoStepsNode = namedtuple('TodoStepsNode', + ('item', 'is_todo', 'children', 'seen')) + + class Todo(BaseModel, ConditionsRelations): """Individual actionable.""" @@ -134,6 +139,36 @@ class Todo(BaseModel, ConditionsRelations): for condition in self.disables: condition.is_active = False + def get_step_tree(self, seen_todos: set[int], + seen_conditions: set[int]) -> TodoStepsNode: + """Return tree of depended-on Todos and Conditions.""" + + def make_node(step: Todo | Condition) -> TodoStepsNode: + assert isinstance(step.id_, int) + is_todo = isinstance(step, Todo) + children = [] + if is_todo: + assert isinstance(step, Todo) + seen = step.id_ in seen_todos + seen_todos.add(step.id_) + potentially_enabled = set() + for child in step.children: + for condition in child.enables: + potentially_enabled.add(condition) + children += [make_node(child)] + for condition in [c for c in step.conditions + if (not c.is_active) + and (c not in potentially_enabled)]: + children += [make_node(condition)] + else: + assert isinstance(step, Condition) + seen = step.id_ in seen_conditions + seen_conditions.add(step.id_) + return TodoStepsNode(step, is_todo, children, seen) + + node = make_node(self) + return node + def add_child(self, child: Todo) -> None: """Add child to self.children, guard against recursion""" def walk_steps(node: Todo) -> None: diff --git a/templates/base.html b/templates/base.html index 399e1cc..3408d67 100644 --- a/templates/base.html +++ b/templates/base.html @@ -1,6 +1,14 @@ + processes conditions diff --git a/templates/day.html b/templates/day.html index 82eaa56..a089037 100644 --- a/templates/day.html +++ b/templates/day.html @@ -6,7 +6,20 @@ {{ todo_with_children(child, indent+1) }} {% endfor %} {% for condition in todo.conditions %} -
  • {% for i in range(indent+1) %}+{% endfor %} [{% if condition.is_active %}x{% else %} {% endif %}] {{condition.title.newest|e}} +
  • {% for i in range(indent) %} {% endfor %}  <[{% if condition.is_active %}x{% else %} {% endif %}] {{condition.title.newest|e}} +{% endfor %} +{% endmacro %} + +{% macro node_with_children(node, indent) %} +
  • {% for i in range(indent) %}+{% endfor %} +{% if node.is_todo %} +{% if not node.item.is_doable %}{% endif %}[{% if node.item.is_done %}x{% else %} {% endif %}]{% if not node.item.is_doable %}{% endif %} +{% if node.seen %}({% else %}{% endif %}{{node.item.process.title.newest|e}}{% if node.seen %}){% else %}{% endif %} +{% else %} +< {% if node.seen %}({% else %}{% endif %}{{node.item.title.newest|e}}{% if node.seen %}){% else %}{% endif %} +{% endif %} +{% for child in node.children %} +{{ node_with_children(child, indent+1) }} {% endfor %} {% endmacro %} @@ -30,18 +43,18 @@ add todo:
  • [{% if node['condition'].is_active %}x{% else %} {% endif %}] {{node['condition'].title.newest|e}}
      {% for enabler in node['enablers'] %} -
    • [+] {{enabler.process.title.newest|e}}
      • +
    • < {{enabler.process.title.newest|e}}
      • {% endfor %} {% for disabler in node['disablers'] %} -
    • [-] {{disabler.process.title.newest|e}}
      • +
    • ! {{disabler.process.title.newest|e}}
      • {% endfor %}
    • {% endfor %}

    todos

      -{% for todo in todos %} -{{ todo_with_children(todo, 0) }} +{% for node in todo_trees %} +{{ node_with_children(node, 0) }} {% endfor %}
    {% endblock %} diff --git a/tests/todos.py b/tests/todos.py index c704c27..52363c0 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -1,6 +1,6 @@ """Test Todos module.""" from tests.utils import TestCaseWithDB, TestCaseWithServer -from plomtask.todos import Todo +from plomtask.todos import Todo, TodoStepsNode from plomtask.processes import Process from plomtask.conditions import Condition from plomtask.exceptions import (NotFoundException, BadFormatException, @@ -156,6 +156,70 @@ class TestsWithDB(TestCaseWithDB): self.cond1.is_active = True todo_2.is_done = True + def test_Todo_step_tree(self) -> None: + """Test self-configuration of TodoStepsNode tree for Day view.""" + assert isinstance(self.cond1.id_, int) + assert isinstance(self.cond2.id_, int) + todo_1 = Todo(None, self.proc, False, self.date1) + todo_1.save(self.db_conn) + assert isinstance(todo_1.id_, int) + # test minimum + tree_expected = TodoStepsNode(todo_1, True, [], False) + self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + # test non_emtpy seen_todo does something + tree_expected = TodoStepsNode(todo_1, True, [], True) + self.assertEqual(todo_1.get_step_tree({todo_1.id_}, set()), + tree_expected) + # test child shows up + todo_2 = Todo(None, self.proc, False, self.date1) + todo_2.save(self.db_conn) + assert isinstance(todo_2.id_, int) + todo_1.add_child(todo_2) + node_todo_2 = TodoStepsNode(todo_2, True, [], False) + tree_expected = TodoStepsNode(todo_1, True, [node_todo_2], False) + self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + # test child shows up with child + todo_3 = Todo(None, self.proc, False, self.date1) + todo_3.save(self.db_conn) + assert isinstance(todo_3.id_, int) + todo_2.add_child(todo_3) + node_todo_3 = TodoStepsNode(todo_3, True, [], False) + node_todo_2 = TodoStepsNode(todo_2, True, [node_todo_3], False) + tree_expected = TodoStepsNode(todo_1, True, [node_todo_2], False) + self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + # test same todo can be child-ed multiple times at different locations + todo_1.add_child(todo_3) + node_todo_4 = TodoStepsNode(todo_3, True, [], True) + tree_expected = TodoStepsNode(todo_1, True, + [node_todo_2, node_todo_4], False) + self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + # test condition shows up + todo_1.set_conditions(self.db_conn, [self.cond1.id_]) + node_cond_1 = TodoStepsNode(self.cond1, False, [], False) + tree_expected = TodoStepsNode(todo_1, True, + [node_todo_2, node_todo_4, node_cond_1], + False) + self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + # test second condition shows up + todo_2.set_conditions(self.db_conn, [self.cond2.id_]) + node_cond_2 = TodoStepsNode(self.cond2, False, [], False) + node_todo_2 = TodoStepsNode(todo_2, True, + [node_todo_3, node_cond_2], False) + tree_expected = TodoStepsNode(todo_1, True, + [node_todo_2, node_todo_4, node_cond_1], + False) + self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + # test second condition is not hidden if fulfilled by non-sibling + todo_1.set_enables(self.db_conn, [self.cond2.id_]) + self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + # test second condition is hidden if fulfilled by sibling + todo_3.set_enables(self.db_conn, [self.cond2.id_]) + node_todo_2 = TodoStepsNode(todo_2, True, [node_todo_3], False) + tree_expected = TodoStepsNode(todo_1, True, + [node_todo_2, node_todo_4, node_cond_1], + False) + self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + def test_Todo_singularity(self) -> None: """Test pointers made for single object keep pointing to it.""" todo = Todo(None, self.proc, False, self.date1) @@ -204,33 +268,41 @@ class TestsWithServer(TestCaseWithServer): self.check_post(form_data, '/todo?id=1', status, '/') self.db_conn.cached_todos = {} return Todo.by_date(self.db_conn, '2024-01-01')[0] + # test minimum form_data = {'title': '', 'description': '', 'effort': 1} self.check_post(form_data, '/process', 302, '/') form_data = {'comment': '', 'new_todo': 1} self.check_post(form_data, '/day?date=2024-01-01', 302, '/') + # test posting to bad URLs form_data = {} self.check_post(form_data, '/todo=', 404) self.check_post(form_data, '/todo?id=', 400) self.check_post(form_data, '/todo?id=FOO', 400) self.check_post(form_data, '/todo?id=0', 404) + # test posting naked entity todo1 = post_and_reload(form_data) self.assertEqual(todo1.children, []) self.assertEqual(todo1.parents, []) self.assertEqual(todo1.is_done, False) + # test posting doneness form_data = {'done': ''} todo1 = post_and_reload(form_data) self.assertEqual(todo1.is_done, True) + # test implicitly posting non-doneness form_data = {} todo1 = post_and_reload(form_data) self.assertEqual(todo1.is_done, False) + # test malformed adoptions form_data = {'adopt': 'foo'} self.check_post(form_data, '/todo?id=1', 400) form_data = {'adopt': 1} self.check_post(form_data, '/todo?id=1', 400) form_data = {'adopt': 2} self.check_post(form_data, '/todo?id=1', 404) + # test posting second todo of same process form_data = {'comment': '', 'new_todo': 1} self.check_post(form_data, '/day?date=2024-01-01', 302, '/') + # test todo 1 adopting todo 2 form_data = {'adopt': 2} todo1 = post_and_reload(form_data) todo2 = Todo.by_date(self.db_conn, '2024-01-01')[1] @@ -238,7 +310,9 @@ class TestsWithServer(TestCaseWithServer): self.assertEqual(todo1.parents, []) self.assertEqual(todo2.children, []) self.assertEqual(todo2.parents, [todo1]) + # test failure of re-adopting same child self.check_post(form_data, '/todo?id=1', 400, '/') + # test todo1 cannot be set done with todo2 not done yet form_data = {'done': ''} todo1 = post_and_reload(form_data, 400) self.assertEqual(todo1.is_done, False) -- 2.30.2 From 010ef4bfea17be7436a937f28e7b54d3da17a1e1 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 22 Apr 2024 02:13:39 +0200 Subject: [PATCH 12/16] Allow Todo adoptions to be un-done in Todo view. --- plomtask/http.py | 11 +++++++++-- plomtask/todos.py | 9 ++++++++- templates/todo.html | 3 ++- tests/todos.py | 18 ++++++++++++------ 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index e541057..b5a6c16 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -205,8 +205,15 @@ class TaskHandler(BaseHTTPRequestHandler): """Update Todo and its children.""" id_ = self.params.get_int('id') todo = Todo.by_id(self.conn, id_) - child_id = self.form_data.get_int_or_none('adopt') - if child_id is not None: + adopted_child_ids = self.form_data.get_all_int('adopt') + for child in todo.children: + if child.id_ not in adopted_child_ids: + assert isinstance(child.id_, int) + child = Todo.by_id(self.conn, child.id_) + todo.remove_child(child) + for child_id in adopted_child_ids: + if child_id in [c.id_ for c in todo.children]: + continue child = Todo.by_id(self.conn, child_id) todo.add_child(child) todo.set_conditions(self.conn, self.form_data.get_all_int('condition')) diff --git a/plomtask/todos.py b/plomtask/todos.py index 336ec03..cf4c330 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -170,7 +170,7 @@ class Todo(BaseModel, ConditionsRelations): return node def add_child(self, child: Todo) -> None: - """Add child to self.children, guard against recursion""" + """Add child to self.children, avoid recursion, update parenthoods.""" def walk_steps(node: Todo) -> None: if node.id_ == self.id_: raise BadFormatException('bad child choice causes recursion') @@ -186,6 +186,13 @@ class Todo(BaseModel, ConditionsRelations): self.children += [child] child.parents += [self] + def remove_child(self, child: Todo) -> None: + """Remove child from self.children, update counter relations.""" + if child not in self.children: + raise HandledException('Cannot remove un-parented child.') + self.children.remove(child) + child.parents.remove(self) + def save(self, db_conn: DatabaseConnection) -> None: """Write self and children to DB and its cache.""" if self.process.id_ is None: diff --git a/templates/todo.html b/templates/todo.html index 92b0657..9debffc 100644 --- a/templates/todo.html +++ b/templates/todo.html @@ -65,7 +65,8 @@ add disables: {{child.process.title.newest|e}} +
  • +{{child.process.title.newest|e}} {% endfor %}
adopt: diff --git a/tests/todos.py b/tests/todos.py index 52363c0..4ab34ab 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -270,9 +270,9 @@ class TestsWithServer(TestCaseWithServer): return Todo.by_date(self.db_conn, '2024-01-01')[0] # test minimum form_data = {'title': '', 'description': '', 'effort': 1} - self.check_post(form_data, '/process', 302, '/') + self.check_post(form_data, '/process', 302) form_data = {'comment': '', 'new_todo': 1} - self.check_post(form_data, '/day?date=2024-01-01', 302, '/') + self.check_post(form_data, '/day?date=2024-01-01', 302) # test posting to bad URLs form_data = {} self.check_post(form_data, '/todo=', 404) @@ -301,7 +301,7 @@ class TestsWithServer(TestCaseWithServer): self.check_post(form_data, '/todo?id=1', 404) # test posting second todo of same process form_data = {'comment': '', 'new_todo': 1} - self.check_post(form_data, '/day?date=2024-01-01', 302, '/') + self.check_post(form_data, '/day?date=2024-01-01', 302) # test todo 1 adopting todo 2 form_data = {'adopt': 2} todo1 = post_and_reload(form_data) @@ -310,12 +310,18 @@ class TestsWithServer(TestCaseWithServer): self.assertEqual(todo1.parents, []) self.assertEqual(todo2.children, []) self.assertEqual(todo2.parents, [todo1]) - # test failure of re-adopting same child - self.check_post(form_data, '/todo?id=1', 400, '/') # test todo1 cannot be set done with todo2 not done yet - form_data = {'done': ''} + form_data = {'done': '', 'adopt': 2} todo1 = post_and_reload(form_data, 400) self.assertEqual(todo1.is_done, False) + # test todo1 un-adopting todo 2 by just not sending an adopt + form_data = {} + todo1 = post_and_reload(form_data, 302) + todo2 = Todo.by_date(self.db_conn, '2024-01-01')[1] + self.assertEqual(todo1.children, []) + self.assertEqual(todo1.parents, []) + self.assertEqual(todo2.children, []) + self.assertEqual(todo2.parents, []) def test_do_GET_todo(self) -> None: """Test GET /todo response codes.""" -- 2.30.2 From 951d8ad55c0d54286f9c986257a67dfa9710fcf2 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 22 Apr 2024 03:52:44 +0200 Subject: [PATCH 13/16] Simplify code with namedtuples and dataclasses. --- plomtask/http.py | 18 ++++++------- plomtask/processes.py | 34 +++++++++++++++++-------- plomtask/todos.py | 13 +++++++--- templates/day.html | 8 +++--- templates/process.html | 2 +- tests/processes.py | 39 ++++++++--------------------- tests/todos.py | 57 +++++++++++++++++------------------------- 7 files changed, 82 insertions(+), 89 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index b5a6c16..d8ca8df 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -1,5 +1,6 @@ """Web server stuff.""" from typing import Any +from collections import namedtuple from http.server import BaseHTTPRequestHandler from http.server import HTTPServer from urllib.parse import urlparse, parse_qs @@ -119,23 +120,22 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_day(self) -> dict[str, object]: """Show single Day of ?date=.""" date = self.params.get_str('date', todays_date()) - conditions_listing = [] top_todos = [t for t in Todo.by_date(self.conn, date) if not t.parents] seen_todos: set[int] = set() seen_conditions: set[int] = set() todo_trees = [t.get_step_tree(seen_todos, seen_conditions) for t in top_todos] - for condition in Condition.all(self.conn): - enablers = Todo.enablers_for_at(self.conn, condition, date) - disablers = Todo.disablers_for_at(self.conn, condition, date) - conditions_listing += [{ - 'condition': condition, - 'enablers': enablers, - 'disablers': disablers}] + ConditionsNode = namedtuple('ConditionsNode', + ('condition', 'enablers', 'disablers')) + conditions_nodes: list[ConditionsNode] = [] + for condi in Condition.all(self.conn): + enablers = Todo.enablers_for_at(self.conn, condi, date) + disablers = Todo.disablers_for_at(self.conn, condi, date) + conditions_nodes += [ConditionsNode(condi, enablers, disablers)] return {'day': Day.by_id(self.conn, date, create=True), 'todo_trees': todo_trees, 'processes': Process.all(self.conn), - 'conditions_listing': conditions_listing} + 'conditions_nodes': conditions_nodes} def do_GET_todo(self) -> dict[str, object]: """Show single Todo of ?id=.""" diff --git a/plomtask/processes.py b/plomtask/processes.py index eb43895..32eee4d 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -1,12 +1,23 @@ """Collecting Processes and Process-related items.""" from __future__ import annotations -from typing import Any, Set +from dataclasses import dataclass +from typing import Set from plomtask.db import DatabaseConnection, BaseModel from plomtask.misc import VersionedAttribute from plomtask.conditions import Condition, ConditionsRelations from plomtask.exceptions import NotFoundException, BadFormatException +@dataclass +class ProcessStepsNode: + """Collects what's useful to know for ProcessSteps tree display.""" + process: Process + parent_id: int | None + is_explicit: bool + steps: dict[int, ProcessStepsNode] + seen: bool + + class Process(BaseModel, ConditionsRelations): """Template for, and metadata for, Todos, and their arrangements.""" table_name = 'processes' @@ -76,29 +87,30 @@ class Process(BaseModel, ConditionsRelations): return [self.__class__.by_id(db_conn, id_) for id_ in owner_ids] def get_steps(self, db_conn: DatabaseConnection, external_owner: - Process | None = None) -> dict[int, dict[str, object]]: + Process | None = None) -> dict[int, ProcessStepsNode]: """Return tree of depended-on explicit and implicit ProcessSteps.""" - def make_node(step: ProcessStep) -> dict[str, object]: + def make_node(step: ProcessStep) -> ProcessStepsNode: is_explicit = False if external_owner is not None: is_explicit = step.owner_id == external_owner.id_ process = self.__class__.by_id(db_conn, step.step_process_id) step_steps = process.get_steps(db_conn, external_owner) - return {'process': process, 'parent_id': step.parent_step_id, - 'is_explicit': is_explicit, 'steps': step_steps} + return ProcessStepsNode(process, step.parent_step_id, + is_explicit, step_steps, False) - def walk_steps(node_id: int, node: dict[str, Any]) -> None: + def walk_steps(node_id: int, node: ProcessStepsNode) -> None: explicit_children = [s for s in self.explicit_steps if s.parent_step_id == node_id] for child in explicit_children: - node['steps'][child.id_] = make_node(child) - node['seen'] = node_id in seen_step_ids + assert isinstance(child.id_, int) + node.steps[child.id_] = make_node(child) + node.seen = node_id in seen_step_ids seen_step_ids.add(node_id) - for id_, step in node['steps'].items(): + for id_, step in node.steps.items(): walk_steps(id_, step) - steps: dict[int, dict[str, object]] = {} + steps: dict[int, ProcessStepsNode] = {} seen_step_ids: Set[int] = set() if external_owner is None: external_owner = self @@ -124,12 +136,14 @@ class Process(BaseModel, ConditionsRelations): just deleted under its feet), or if the parent step would not be owned by the current Process. """ + def walk_steps(node: ProcessStep) -> None: if node.step_process_id == self.id_: raise BadFormatException('bad step selection causes recursion') step_process = self.by_id(db_conn, node.step_process_id) for step in step_process.explicit_steps: walk_steps(step) + if parent_step_id is not None: try: parent_step = ProcessStep.by_id(db_conn, parent_step_id) diff --git a/plomtask/todos.py b/plomtask/todos.py index cf4c330..ff6cdcb 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -1,6 +1,6 @@ """Actionables.""" from __future__ import annotations -from collections import namedtuple +from dataclasses import dataclass from typing import Any from sqlite3 import Row from plomtask.db import DatabaseConnection, BaseModel @@ -10,8 +10,13 @@ from plomtask.exceptions import (NotFoundException, BadFormatException, HandledException) -TodoStepsNode = namedtuple('TodoStepsNode', - ('item', 'is_todo', 'children', 'seen')) +@dataclass +class TodoStepsNode: + """Collects what's useful to know for Todo/Condition tree display.""" + item: Todo | Condition + is_todo: bool + children: list[TodoStepsNode] + seen: bool class Todo(BaseModel, ConditionsRelations): @@ -171,11 +176,13 @@ class Todo(BaseModel, ConditionsRelations): def add_child(self, child: Todo) -> None: """Add child to self.children, avoid recursion, update parenthoods.""" + def walk_steps(node: Todo) -> None: if node.id_ == self.id_: raise BadFormatException('bad child choice causes recursion') for child in node.children: walk_steps(child) + if self.id_ is None: raise HandledException('Can only add children to saved Todos.') if child.id_ is None: diff --git a/templates/day.html b/templates/day.html index a089037..8887f9a 100644 --- a/templates/day.html +++ b/templates/day.html @@ -39,13 +39,13 @@ add todo:

conditions

-{% for node in conditions_listing %} -
  • [{% if node['condition'].is_active %}x{% else %} {% endif %}] {{node['condition'].title.newest|e}} +{% for node in conditions_nodes %} +
  • [{% if node.condition.is_active %}x{% else %} {% endif %}] {{node.condition.title.newest|e}}
      -{% for enabler in node['enablers'] %} +{% for enabler in node.enablers %}
    • < {{enabler.process.title.newest|e}}
      • {% endfor %} -{% for disabler in node['disablers'] %} +{% for disabler in node.disablers %}
    • ! {{disabler.process.title.newest|e}}
      • {% endfor %}
    diff --git a/templates/process.html b/templates/process.html index 85651d7..f8f7baf 100644 --- a/templates/process.html +++ b/templates/process.html @@ -86,7 +86,7 @@ add enables:

    steps

    -{% for step_id, step_node in steps.items() %} +{% for step_node in steps %} {{ step_with_steps(step_id, step_node, 0) }} {% endfor %}
    diff --git a/tests/processes.py b/tests/processes.py index 084cfe0..9413822 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -1,8 +1,7 @@ """Test Processes module.""" from unittest import TestCase -from typing import Any from tests.utils import TestCaseWithDB, TestCaseWithServer -from plomtask.processes import Process, ProcessStep +from plomtask.processes import Process, ProcessStep, ProcessStepsNode from plomtask.conditions import Condition from plomtask.exceptions import NotFoundException, HandledException @@ -58,46 +57,30 @@ class TestsWithDB(TestCaseWithDB): assert isinstance(self.proc3.id_, int) steps_proc1: list[tuple[int | None, int, int | None]] = [] add_step(self.proc1, steps_proc1, (None, self.proc2.id_, None), 1) - p_1_dict: dict[int, dict[str, Any]] = {1: { - 'process': self.proc2, 'parent_id': None, - 'is_explicit': True, 'steps': {}, 'seen': False - }} + p_1_dict: dict[int, ProcessStepsNode] = {} + p_1_dict[1] = ProcessStepsNode(self.proc2, None, True, {}, False) self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) add_step(self.proc1, steps_proc1, (None, self.proc3.id_, None), 2) step_2 = self.proc1.explicit_steps[-1] assert isinstance(step_2.id_, int) - p_1_dict[2] = { - 'process': self.proc3, 'parent_id': None, - 'is_explicit': True, 'steps': {}, 'seen': False - } + p_1_dict[2] = ProcessStepsNode(self.proc3, None, True, {}, False) self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) steps_proc2: list[tuple[int | None, int, int | None]] = [] add_step(self.proc2, steps_proc2, (None, self.proc3.id_, None), 3) - p_1_dict[1]['steps'] = {3: { - 'process': self.proc3, 'parent_id': None, - 'is_explicit': False, 'steps': {}, 'seen': False - }} + p_1_dict[1].steps[3] = ProcessStepsNode(self.proc3, None, + False, {}, False) self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) add_step(self.proc1, steps_proc1, (None, self.proc2.id_, step_2.id_), 4) - p_1_dict[2]['steps'][4] = { - 'process': self.proc2, 'parent_id': step_2.id_, 'seen': False, - 'is_explicit': True, 'steps': {3: { - 'process': self.proc3, 'parent_id': None, - 'is_explicit': False, 'steps': {}, 'seen': True - }}} + step_3 = ProcessStepsNode(self.proc3, None, False, {}, True) + p_1_dict[2].steps[4] = ProcessStepsNode(self.proc2, step_2.id_, True, + {3: step_3}, False) self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) add_step(self.proc1, steps_proc1, (None, self.proc3.id_, 999), 5) - p_1_dict[5] = { - 'process': self.proc3, 'parent_id': None, - 'is_explicit': True, 'steps': {}, 'seen': False - } + p_1_dict[5] = ProcessStepsNode(self.proc3, None, True, {}, False) self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) add_step(self.proc1, steps_proc1, (None, self.proc3.id_, 3), 6) - p_1_dict[6] = { - 'process': self.proc3, 'parent_id': None, - 'is_explicit': True, 'steps': {}, 'seen': False - } + p_1_dict[6] = ProcessStepsNode(self.proc3, None, True, {}, False) self.assertEqual(self.proc1.get_steps(self.db_conn, None), p_1_dict) self.assertEqual(self.proc1.used_as_step_by(self.db_conn), diff --git a/tests/todos.py b/tests/todos.py index 4ab34ab..6e88425 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -164,61 +164,50 @@ class TestsWithDB(TestCaseWithDB): todo_1.save(self.db_conn) assert isinstance(todo_1.id_, int) # test minimum - tree_expected = TodoStepsNode(todo_1, True, [], False) - self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + node_0 = TodoStepsNode(todo_1, True, [], False) + self.assertEqual(todo_1.get_step_tree(set(), set()), node_0) # test non_emtpy seen_todo does something - tree_expected = TodoStepsNode(todo_1, True, [], True) - self.assertEqual(todo_1.get_step_tree({todo_1.id_}, set()), - tree_expected) + node_0.seen = True + self.assertEqual(todo_1.get_step_tree({todo_1.id_}, set()), node_0) # test child shows up todo_2 = Todo(None, self.proc, False, self.date1) todo_2.save(self.db_conn) assert isinstance(todo_2.id_, int) todo_1.add_child(todo_2) - node_todo_2 = TodoStepsNode(todo_2, True, [], False) - tree_expected = TodoStepsNode(todo_1, True, [node_todo_2], False) - self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + node_2 = TodoStepsNode(todo_2, True, [], False) + node_0.children = [node_2] + node_0.seen = False + self.assertEqual(todo_1.get_step_tree(set(), set()), node_0) # test child shows up with child todo_3 = Todo(None, self.proc, False, self.date1) todo_3.save(self.db_conn) assert isinstance(todo_3.id_, int) todo_2.add_child(todo_3) - node_todo_3 = TodoStepsNode(todo_3, True, [], False) - node_todo_2 = TodoStepsNode(todo_2, True, [node_todo_3], False) - tree_expected = TodoStepsNode(todo_1, True, [node_todo_2], False) - self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + node_3 = TodoStepsNode(todo_3, True, [], False) + node_2.children = [node_3] + self.assertEqual(todo_1.get_step_tree(set(), set()), node_0) # test same todo can be child-ed multiple times at different locations todo_1.add_child(todo_3) - node_todo_4 = TodoStepsNode(todo_3, True, [], True) - tree_expected = TodoStepsNode(todo_1, True, - [node_todo_2, node_todo_4], False) - self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + node_4 = TodoStepsNode(todo_3, True, [], True) + node_0.children += [node_4] + self.assertEqual(todo_1.get_step_tree(set(), set()), node_0) # test condition shows up todo_1.set_conditions(self.db_conn, [self.cond1.id_]) - node_cond_1 = TodoStepsNode(self.cond1, False, [], False) - tree_expected = TodoStepsNode(todo_1, True, - [node_todo_2, node_todo_4, node_cond_1], - False) - self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + node_5 = TodoStepsNode(self.cond1, False, [], False) + node_0.children += [node_5] + self.assertEqual(todo_1.get_step_tree(set(), set()), node_0) # test second condition shows up todo_2.set_conditions(self.db_conn, [self.cond2.id_]) - node_cond_2 = TodoStepsNode(self.cond2, False, [], False) - node_todo_2 = TodoStepsNode(todo_2, True, - [node_todo_3, node_cond_2], False) - tree_expected = TodoStepsNode(todo_1, True, - [node_todo_2, node_todo_4, node_cond_1], - False) - self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + node_6 = TodoStepsNode(self.cond2, False, [], False) + node_2.children += [node_6] + self.assertEqual(todo_1.get_step_tree(set(), set()), node_0) # test second condition is not hidden if fulfilled by non-sibling todo_1.set_enables(self.db_conn, [self.cond2.id_]) - self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + self.assertEqual(todo_1.get_step_tree(set(), set()), node_0) # test second condition is hidden if fulfilled by sibling todo_3.set_enables(self.db_conn, [self.cond2.id_]) - node_todo_2 = TodoStepsNode(todo_2, True, [node_todo_3], False) - tree_expected = TodoStepsNode(todo_1, True, - [node_todo_2, node_todo_4, node_cond_1], - False) - self.assertEqual(todo_1.get_step_tree(set(), set()), tree_expected) + node_2.children.remove(node_6) + self.assertEqual(todo_1.get_step_tree(set(), set()), node_0) def test_Todo_singularity(self) -> None: """Test pointers made for single object keep pointing to it.""" -- 2.30.2 From 8e1a5416151dbcf506f2435823362e21d85aed2d Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 22 Apr 2024 04:38:49 +0200 Subject: [PATCH 14/16] Minor fixes. --- plomtask/http.py | 3 +-- templates/day.html | 10 ---------- templates/process.html | 2 +- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index d8ca8df..1009c1b 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -195,8 +195,7 @@ class TaskHandler(BaseHTTPRequestHandler): day = Day.by_id(self.conn, date, create=True) day.comment = self.form_data.get_str('comment') day.save(self.conn) - process_id = self.form_data.get_int_or_none('new_todo') - if process_id is not None: + for process_id in self.form_data.get_all_int('new_todo'): process = Process.by_id(self.conn, process_id) todo = Todo(None, process, False, day.date) todo.save(self.conn) diff --git a/templates/day.html b/templates/day.html index 8887f9a..b38d566 100644 --- a/templates/day.html +++ b/templates/day.html @@ -1,15 +1,5 @@ {% extends 'base.html' %} -{% macro todo_with_children(todo, indent) %} -
  • {% for i in range(indent) %}+{% endfor %} [{% if todo.is_done %}x{% else %} {% endif %}] {{todo.process.title.newest|e}} -{% for child in todo.children %} -{{ todo_with_children(child, indent+1) }} -{% endfor %} -{% for condition in todo.conditions %} -
  • {% for i in range(indent) %} {% endfor %}  <[{% if condition.is_active %}x{% else %} {% endif %}] {{condition.title.newest|e}} -{% endfor %} -{% endmacro %} - {% macro node_with_children(node, indent) %}
  • {% for i in range(indent) %}+{% endfor %} {% if node.is_todo %} diff --git a/templates/process.html b/templates/process.html index f8f7baf..85424fa 100644 --- a/templates/process.html +++ b/templates/process.html @@ -70,7 +70,7 @@ add condition: add enables: -

    conditions

    +

    disables

    {% for condition in process.disables %} -- 2.30.2 From 0ed3bc539d21d5536d0fc635760d88a2231587b9 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 22 Apr 2024 05:32:47 +0200 Subject: [PATCH 15/16] Fix Process retrieval/display/saving bugs. --- plomtask/http.py | 2 +- plomtask/processes.py | 42 ++++++++++++++++++++++-------------------- templates/process.html | 3 +-- tests/processes.py | 10 +++++++++- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 1009c1b..140f6bc 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -161,7 +161,7 @@ class TaskHandler(BaseHTTPRequestHandler): return {'process': process, 'steps': process.get_steps(self.conn), 'owners': process.used_as_step_by(self.conn), - 'process_candidates': Process.all(self.conn), + 'step_candidates': Process.all(self.conn), 'condition_candidates': Condition.all(self.conn)} def do_GET_processes(self) -> dict[str, object]: diff --git a/plomtask/processes.py b/plomtask/processes.py index 32eee4d..6249d48 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -52,27 +52,29 @@ class Process(BaseModel, ConditionsRelations): create: bool = False) -> Process: """Collect Process, its VersionedAttributes, and its child IDs.""" process = None + from_cache = False if id_: - process, _ = super()._by_id(db_conn, id_) - if not process: - if not create: - raise NotFoundException(f'Process not found of id: {id_}') - process = Process(id_) - if isinstance(process.id_, int): - for name in ('title', 'description', 'effort'): - table = f'process_{name}s' - for row in db_conn.row_where(table, 'parent', process.id_): - getattr(process, name).history_from_row(row) - for row in db_conn.row_where('process_steps', 'owner', - process.id_): - step = ProcessStep.from_table_row(db_conn, row) - process.explicit_steps += [step] - for name in ('conditions', 'enables', 'disables'): - table = f'process_{name}' - for cond_id in db_conn.column_where(table, 'condition', - 'process', process.id_): - target = getattr(process, name) - target += [Condition.by_id(db_conn, cond_id)] + process, from_cache = super()._by_id(db_conn, id_) + if not from_cache: + if not process: + if not create: + raise NotFoundException(f'Process not found of id: {id_}') + process = Process(id_) + if isinstance(process.id_, int): + for name in ('title', 'description', 'effort'): + table = f'process_{name}s' + for row in db_conn.row_where(table, 'parent', process.id_): + getattr(process, name).history_from_row(row) + for row in db_conn.row_where('process_steps', 'owner', + process.id_): + step = ProcessStep.from_table_row(db_conn, row) + process.explicit_steps += [step] + for name in ('conditions', 'enables', 'disables'): + table = f'process_{name}' + for c_id in db_conn.column_where(table, 'condition', + 'process', process.id_): + target = getattr(process, name) + target += [Condition.by_id(db_conn, c_id)] assert isinstance(process, Process) return process diff --git a/templates/process.html b/templates/process.html index 85424fa..ec06d3a 100644 --- a/templates/process.html +++ b/templates/process.html @@ -7,7 +7,6 @@ {% if step_node.is_explicit %} - {% endif %} @@ -86,7 +85,7 @@ add enables:

    steps

    -{% for step_node in steps %} +{% for step_id, step_node in steps.items() %} {{ step_with_steps(step_id, step_node, 0) }} {% endfor %}
    diff --git a/tests/processes.py b/tests/processes.py index 9413822..c718677 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -135,12 +135,20 @@ class TestsWithDB(TestCaseWithDB): self.assertEqual(step.parent_step_id, step_retrieved.parent_step_id) def test_Process_singularity(self) -> None: - """Test pointers made for single object keep pointing to it.""" + """Test pointers made for single object keep pointing to it, and + subsequent retrievals don't overload relations.""" assert isinstance(self.proc1.id_, int) assert isinstance(self.proc2.id_, int) + c1 = Condition(None, False) + c1.save(self.db_conn) + assert isinstance(c1.id_, int) + self.proc1.set_conditions(self.db_conn, [c1.id_]) self.proc1.set_steps(self.db_conn, [(None, self.proc2.id_, None)]) + self.proc1.save(self.db_conn) p_retrieved = Process.by_id(self.db_conn, self.proc1.id_) self.assertEqual(self.proc1.explicit_steps, p_retrieved.explicit_steps) + self.assertEqual(self.proc1.conditions, p_retrieved.conditions) + self.proc1.save(self.db_conn) def test_Process_versioned_attributes_singularity(self) -> None: """Test behavior of VersionedAttributes on saving (with .title).""" -- 2.30.2 From a7adce16f1969400cb988ff900f504157e454cce Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 22 Apr 2024 05:56:14 +0200 Subject: [PATCH 16/16] On posting a new Todo to a Day, auto-adopt existing ones per its Process' .explicit_steps. --- plomtask/http.py | 7 +++++++ tests/todos.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/plomtask/http.py b/plomtask/http.py index 140f6bc..64cc6f9 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -195,10 +195,17 @@ class TaskHandler(BaseHTTPRequestHandler): day = Day.by_id(self.conn, date, create=True) day.comment = self.form_data.get_str('comment') day.save(self.conn) + existing_todos = Todo.by_date(self.conn, date) for process_id in self.form_data.get_all_int('new_todo'): process = Process.by_id(self.conn, process_id) todo = Todo(None, process, False, day.date) todo.save(self.conn) + for step in todo.process.explicit_steps: + for t in [t for t in existing_todos + if t.process.id_ == step.step_process_id]: + todo.add_child(t) + break + todo.save(self.conn) def do_POST_todo(self) -> None: """Update Todo and its children.""" diff --git a/tests/todos.py b/tests/todos.py index 6e88425..b5953dc 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -312,6 +312,23 @@ class TestsWithServer(TestCaseWithServer): self.assertEqual(todo2.children, []) self.assertEqual(todo2.parents, []) + def test_do_POST_day_todo_adoption(self) -> None: + """Test Todos posted to Day view may adopt existing Todos.""" + form_data = {'title': '', 'description': '', 'effort': 1} + self.check_post(form_data, '/process', 302, '/') + form_data['new_top_step'] = 1 + self.check_post(form_data, '/process', 302, '/') + form_data = {'comment': '', 'new_todo': 1} + self.check_post(form_data, '/day?date=2024-01-01', 302) + form_data = {'comment': '', 'new_todo': 2} + self.check_post(form_data, '/day?date=2024-01-01', 302) + todo1 = Todo.by_date(self.db_conn, '2024-01-01')[0] + todo2 = Todo.by_date(self.db_conn, '2024-01-01')[1] + self.assertEqual(todo1.children, []) + self.assertEqual(todo1.parents, [todo2]) + self.assertEqual(todo2.children, [todo1]) + self.assertEqual(todo2.parents, []) + def test_do_GET_todo(self) -> None: """Test GET /todo response codes.""" form_data = {'title': '', 'description': '', 'effort': 1} -- 2.30.2