From 244270eed71df45faf9554d0666b816be9876f77 Mon Sep 17 00:00:00 2001
From: Christian Heller <c.heller@plomlompom.de>
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