From 244270eed71df45faf9554d0666b816be9876f77 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 06:18:54 +0200 Subject: [PATCH] Refactor request handler delete or retrieving items on POST. --- plomtask/db.py | 5 ++++ plomtask/http.py | 67 +++++++++++++++++++++++---------------------- scripts/pre-commit | 1 + tests/conditions.py | 2 +- tests/days.py | 13 ++++----- tests/todos.py | 2 +- 6 files changed, 49 insertions(+), 41 deletions(-) diff --git a/plomtask/db.py b/plomtask/db.py index e95c27d..71c59d5 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -330,6 +330,11 @@ class BaseModel(Generic[BaseModelId]): assert isinstance(as_dict['id'], (int, str)) return as_dict['id'] + @classmethod + def name_lowercase(cls) -> str: + """Convenience method to return cls' name in lowercase.""" + return cls.__name__.lower() + # cache management # (we primarily use the cache to ensure we work on the same object in # memory no matter where and how we retrieve it, e.g. we don't want diff --git a/plomtask/http.py b/plomtask/http.py index f317366..18b77a6 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -11,8 +11,8 @@ from os.path import split as path_split from jinja2 import Environment as JinjaEnv, FileSystemLoader as JinjaFSLoader from plomtask.dating import date_in_n_days from plomtask.days import Day -from plomtask.exceptions import HandledException, BadFormatException, \ - NotFoundException +from plomtask.exceptions import (HandledException, BadFormatException, + NotFoundException) from plomtask.db import DatabaseConnection, DatabaseFile from plomtask.processes import Process, ProcessStep, ProcessStepsNode from plomtask.conditions import Condition @@ -495,6 +495,32 @@ class TaskHandler(BaseHTTPRequestHandler): # POST handlers + @staticmethod + def _delete_or_post(target_class: Any, redir_target: str = '/' + ) -> Callable[..., Callable[[TaskHandler], str]]: + def decorator(f: Callable[..., str] + ) -> Callable[[TaskHandler], str]: + def wrapper(self: TaskHandler) -> str: + # pylint: disable=protected-access + # (because pylint here fails to detect the use of wrapper as a + # method to self with respective access privileges) + id_ = self._params.get_int_or_none('id') + for _ in self._form_data.get_all_str('delete'): + if id_ is None: + msg = 'trying to delete non-saved ' +\ + f'{target_class.__name__}' + raise NotFoundException(msg) + item = target_class.by_id(self.conn, id_) + item.remove(self.conn) + return redir_target + if target_class.can_create_by_id: + item = target_class.by_id_or_create(self.conn, id_) + else: + item = target_class.by_id(self.conn, id_) + return f(self, item) + return wrapper + return decorator + def _change_versioned_timestamps(self, cls: Any, attr_name: str) -> str: """Update history timestamps for VersionedAttribute.""" id_ = self._params.get_int_or_none('id') @@ -505,8 +531,7 @@ class TaskHandler(BaseHTTPRequestHandler): if old[19:] != v: attr.reset_timestamp(old, f'{v}.0') attr.save(self.conn) - cls_name = cls.__name__.lower() - return f'/{cls_name}_{attr_name}s?id={item.id_}' + return f'/{cls.name_lowercase()}_{attr_name}s?id={item.id_}' def do_POST_day(self) -> str: """Update or insert Day of date and Todos mapped to it.""" @@ -535,16 +560,9 @@ class TaskHandler(BaseHTTPRequestHandler): todo.save(self.conn) return f'/day?date={date}&make_type={make_type}' - def do_POST_todo(self) -> str: + @_delete_or_post(Todo, '/') + def do_POST_todo(self, todo: Todo) -> str: """Update Todo and its children.""" - # pylint: disable=too-many-locals - # pylint: disable=too-many-branches - id_ = self._params.get_int('id') - for _ in self._form_data.get_all_str('delete'): - todo = Todo .by_id(self.conn, id_) - todo.remove(self.conn) - return '/' - todo = Todo.by_id(self.conn, id_) adopted_child_ids = self._form_data.get_all_int('adopt') processes_to_make_full = self._form_data.get_all_int('make_full') processes_to_make_empty = self._form_data.get_all_int('make_empty') @@ -602,17 +620,9 @@ class TaskHandler(BaseHTTPRequestHandler): """Update history timestamps for Process.title.""" return self._change_versioned_timestamps(Process, 'title') - def do_POST_process(self) -> str: + @_delete_or_post(Process, '/processes') + def do_POST_process(self, process: Process) -> str: """Update or insert Process of ?id= and fields defined in postvars.""" - # pylint: disable=too-many-branches - id_ = self._params.get_int_or_none('id') - for _ in self._form_data.get_all_str('delete'): - if id_ is None: - raise NotFoundException('trying to delete non-saved Process') - process = Process.by_id(self.conn, id_) - process.remove(self.conn) - return '/processes' - process = Process.by_id_or_create(self.conn, id_) process.title.set(self._form_data.get_str('title')) process.description.set(self._form_data.get_str('description')) process.effort.set(self._form_data.get_float('effort')) @@ -682,16 +692,9 @@ class TaskHandler(BaseHTTPRequestHandler): """Update history timestamps for Condition.title.""" return self._change_versioned_timestamps(Condition, 'title') - def do_POST_condition(self) -> str: + @_delete_or_post(Condition, '/conditions') + def do_POST_condition(self, condition: Condition) -> str: """Update/insert Condition of ?id= and fields defined in postvars.""" - id_ = self._params.get_int_or_none('id') - for _ in self._form_data.get_all_str('delete'): - if id_ is None: - raise NotFoundException('trying to delete non-saved Condition') - condition = Condition.by_id_or_create(self.conn, id_) - condition.remove(self.conn) - return '/conditions' - condition = Condition.by_id_or_create(self.conn, id_) condition.is_active = self._form_data.get_str('is_active') == 'True' condition.title.set(self._form_data.get_str('title')) condition.description.set(self._form_data.get_str('description')) diff --git a/scripts/pre-commit b/scripts/pre-commit index 7abafb9..e448035 100755 --- a/scripts/pre-commit +++ b/scripts/pre-commit @@ -1,5 +1,6 @@ #!/bin/sh set -e +# for dir in $(echo 'tests'); do for dir in $(echo '.' 'plomtask' 'tests'); do echo "Running mypy on ${dir}/ …." python3 -m mypy --strict ${dir}/*.py diff --git a/tests/conditions.py b/tests/conditions.py index e488f34..25a044f 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -51,6 +51,7 @@ class TestsWithServer(TestCaseWithServer): self.check_post(post, '/condition', 302, '/condition?id=1') # … single /condition cond = self.cond_as_dict(titles=['foo'], descriptions=['oof']) + assert isinstance(cond['_versioned'], dict) expected_single: dict[str, object] expected_single = {'is_new': False, 'enabled_processes': [], @@ -72,7 +73,6 @@ class TestsWithServer(TestCaseWithServer): # test effect of POST changing title and activeness post = {'title': 'bar', 'description': 'oof', 'is_active': True} self.check_post(post, '/condition?id=1', 302) - assert isinstance(cond['_versioned'], dict) cond['_versioned']['title'][1] = 'bar' cond['is_active'] = True self.check_json_get('/condition?id=1', expected_single) diff --git a/tests/days.py b/tests/days.py index bfec21a..008fc1e 100644 --- a/tests/days.py +++ b/tests/days.py @@ -156,13 +156,12 @@ class TestsWithServer(TestCaseWithServer): # check defined day date = '2024-01-01' expected = self.get_day_dict(date) + assert isinstance(expected['_library'], dict) self.check_json_get(f'/day?date={date}', expected) # check saved day post: dict[str, object] = {'day_comment': 'foo', 'make_type': ''} self.post_day(f'date={date}', post) - assert isinstance(expected['_library'], dict) - day = expected['_library']['Day'][date] - day['comment'] = post['day_comment'] + expected['_library']['Day'][date]['comment'] = post['day_comment'] self.check_json_get(f'/day?date={date}', expected) # check GET parameter to GET requests affects immediate reply, but … expected['make_type'] = 'bar' @@ -208,21 +207,21 @@ class TestsWithServer(TestCaseWithServer): procs_expected = self.post_batch(procs_data, [], ['title', 'description', 'effort'], self.proc_as_dict, self.post_process) - self.post_day(f'date={date}') expected = self.get_day_dict(date) assert isinstance(expected['_library'], dict) expected['processes'] = self.as_id_list(procs_expected) expected['_library']['Process'] = self.as_refs(procs_expected) + self.post_day(f'date={date}') self.check_json_get(f'/day?date={date}', expected) # post Todos of either process and check their display post_day: dict[str, object] post_day = {'day_comment': '', 'make_type': '', 'new_todo': [1, 2]} - self.post_day(f'date={date}', post_day) todos = [self.todo_as_dict(1, 1, date), self.todo_as_dict(2, 2, date)] expected['_library']['Todo'] = self.as_refs(todos) expected['_library']['Day'][date]['todos'] = self.as_id_list(todos) nodes = [self.todo_node_as_dict(1), self.todo_node_as_dict(2)] expected['top_nodes'] = nodes + self.post_day(f'date={date}', post_day) self.check_json_get(f'/day?date={date}', expected) # add a comment to one Todo and set the other's doneness and effort post_day['new_todo'] = [] @@ -230,10 +229,10 @@ class TestsWithServer(TestCaseWithServer): post_day['done'] = [2] post_day['comment'] = ['FOO', ''] post_day['effort'] = ['2.3', ''] - self.post_day(f'date={date}', post_day) expected['_library']['Todo']['1']['comment'] = 'FOO' expected['_library']['Todo']['1']['effort'] = 2.3 expected['_library']['Todo']['2']['is_done'] = True + self.post_day(f'date={date}', post_day) self.check_json_get(f'/day?date={date}', expected) def test_do_GET_day_with_conditions(self) -> None: @@ -260,7 +259,6 @@ class TestsWithServer(TestCaseWithServer): # add Todos in relation to Conditions, check consequences post_day: dict[str, object] post_day = {'day_comment': '', 'make_type': '', 'new_todo': [1, 2]} - self.post_day(f'date={date}', post_day) todos = [self.todo_as_dict(1, 1, date, [1], [1], [2], [2]), self.todo_as_dict(2, 2, date, [2], [2], [1], [1])] expected['_library']['Todo'] = self.as_refs(todos) @@ -270,6 +268,7 @@ class TestsWithServer(TestCaseWithServer): expected['disablers_for'] = {'1': [1], '2': [2]} expected['enablers_for'] = {'1': [2], '2': [1]} expected['conditions_present'] = self.as_id_list(conds_expected) + self.post_day(f'date={date}', post_day) self.check_json_get(f'/day?date={date}', expected) def test_do_GET(self) -> None: diff --git a/tests/todos.py b/tests/todos.py index 8b8099f..7f106bc 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -274,7 +274,7 @@ class TestsWithServer(TestCaseWithServer): '/day?date=2024-01-01&make_type=full', 302) # test posting to bad URLs self.check_post({}, '/todo=', 404) - self.check_post({}, '/todo?id=', 400) + self.check_post({}, '/todo?id=', 404) self.check_post({}, '/todo?id=FOO', 400) self.check_post({}, '/todo?id=0', 404) # test posting naked entity -- 2.30.2