Ensure POST /todo autoremoval via .effort < -1 causes 404 rather than 500.

diff --git a/plomtask/http.py b/plomtask/http.py
index 98242e5c90e392d72679b42e12cdbb75ee951af5..2aa521566bbbf8ebcf5297dadfa25a661e7d1658 100644 (file)
--- a/plomtask/http.py
+++ b/plomtask/http.py
@@ -690,8 +690,10 @@ class TaskHandler(BaseHTTPRequestHandler):
         todo.is_done = is_done
         todo.calendarize = calendarize
         todo.comment = comment
+        # todo.save() may destroy Todo if .effort < 0, so retrieve .id_ early
+        url = f'/todo?id={todo.id_}'
         todo.save(self.conn)
-        return f'/todo?id={todo.id_}'
+        return url
 
     def do_POST_process_descriptions(self) -> str:
         """Update history timestamps for Process.description."""

diff --git a/tests/todos.py b/tests/todos.py
index 42bff442fc61fcd69f86e3817736736930c5d1f7..1fa3a9dcda705e1d905b0f5ea036c29643d99583 100644 (file)
--- a/tests/todos.py
+++ b/tests/todos.py
@@ -169,7 +169,7 @@ class TestsWithDB(TestCaseWithDB, TestCaseSansDB):
         self.assertEqual(cmp_0_dict, cmp_1_dict)
 
     def test_Todo_create_with_children(self) -> None:
-        """Test parenthood guaranteeds of Todo.create_with_children."""
+        """Test parenthood guarantees of Todo.create_with_children."""
         assert isinstance(self.proc.id_, int)
         proc2 = Process(None)
         proc2.save(self.db_conn)
@@ -207,46 +207,6 @@ class TestsWithDB(TestCaseWithDB, TestCaseSansDB):
         self.assertEqual(len(todo_3.children), 1)
         self.assertEqual(todo_3.children[0].process, proc4)
 
-    def test_Todo_remove(self) -> None:
-        """Test removal."""
-        todo_1 = Todo(None, self.proc, False, self.date1)
-        todo_1.save(self.db_conn)
-        assert todo_1.id_ is not None
-        todo_0 = Todo(None, self.proc, False, self.date1)
-        todo_0.save(self.db_conn)
-        todo_0.add_child(todo_1)
-        todo_2 = Todo(None, self.proc, False, self.date1)
-        todo_2.save(self.db_conn)
-        todo_1.add_child(todo_2)
-        todo_1_id = todo_1.id_
-        todo_1.remove(self.db_conn)
-        with self.assertRaises(NotFoundException):
-            Todo.by_id(self.db_conn, todo_1_id)
-        self.assertEqual(todo_0.children, [])
-        self.assertEqual(todo_2.parents, [])
-        todo_2.comment = 'foo'
-        with self.assertRaises(HandledException):
-            todo_2.remove(self.db_conn)
-        todo_2.comment = ''
-        todo_2.effort = 5
-        with self.assertRaises(HandledException):
-            todo_2.remove(self.db_conn)
-
-    def test_Todo_autoremoval(self) -> None:
-        """"Test automatic removal for Todo.effort < 0."""
-        todo_1 = Todo(None, self.proc, False, self.date1)
-        todo_1.save(self.db_conn)
-        todo_1.comment = 'foo'
-        todo_1.effort = -0.1
-        todo_1.save(self.db_conn)
-        assert todo_1.id_ is not None
-        Todo.by_id(self.db_conn, todo_1.id_)
-        todo_1.comment = ''
-        todo_1_id = todo_1.id_
-        todo_1.save(self.db_conn)
-        with self.assertRaises(NotFoundException):
-            Todo.by_id(self.db_conn, todo_1_id)
-
 
 class TestsWithServer(TestCaseWithServer):
     """Tests against our HTTP server/handler (and database)."""
@@ -364,6 +324,18 @@ class TestsWithServer(TestCaseWithServer):
         todo_dict['id'] = 2
         expected = self.GET_todo_dict(2, [todo_dict], [proc_dict])
         self.check_json_get('/todo?id=2', expected)
+        # test cannot delete Todo with comment or effort
+        self.check_post({'comment': 'foo'}, '/todo?id=2')
+        self.check_post({'delete': ''}, '/todo?id=2', 500, '/')
+        self.check_post({'effort': 5}, '/todo?id=2')
+        self.check_post({'delete': ''}, '/todo?id=2', 500, '/')
+        # test deletion via effort < 0, but only once deletable
+        self.check_post({'effort': -1, 'comment': 'foo'}, '/todo?id=2')
+        todo_dict['comment'] = 'foo'
+        todo_dict['effort'] = -1
+        self.check_json_get('/todo?id=2', expected)
+        self.check_post({}, '/todo?id=2')
+        self.check_get('/todo?id=2', 404)
 
     def test_POST_todo_adoption(self) -> None:
         """Test adoption via POST /todo with "adopt"."""
@@ -538,39 +510,6 @@ class TestsWithServer(TestCaseWithServer):
         expected['adoption_candidates_for'] = {'2': [], '3': [], '4': []}
         self.check_json_get('/todo?id=1', expected)
 
-    def test_do_POST_day(self) -> None:
-        """Test Todo posting of POST /day."""
-        self.post_process(2)
-        proc = Process.by_id(self.db_conn, 1)
-        proc2 = Process.by_id(self.db_conn, 2)
-        # check posting no Todos to Day makes Todo.by_date return empty list
-        form_data = {'day_comment': '', 'make_type': 'full'}
-        self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302)
-        self.assertEqual(Todo.by_date(self.db_conn, '2024-01-01'), [])
-        proc = Process.by_id(self.db_conn, 1)
-        # post Todo to Day and check its display
-        form_data['new_todo'] = str(proc.id_)
-        self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302)
-        todos = Todo.by_date(self.db_conn, '2024-01-01')
-        self.assertEqual(1, len(todos))
-        todo1 = todos[0]
-        self.assertEqual(todo1.id_, 1)
-        proc = Process.by_id(self.db_conn, 1)
-        self.assertEqual(todo1.process.id_, proc.id_)
-        self.assertEqual(todo1.is_done, False)
-        # post second Todo, check its appearance
-        proc2 = Process.by_id(self.db_conn, 2)
-        form_data['new_todo'] = str(proc2.id_)
-        self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302)
-        todos = Todo.by_date(self.db_conn, '2024-01-01')
-        todo1 = todos[1]
-        self.assertEqual(todo1.id_, 2)
-        proc2 = Process.by_id(self.db_conn, 1)
-        todo1 = Todo.by_date(self.db_conn, '2024-01-01')[0]
-        self.assertEqual(todo1.id_, 1)
-        self.assertEqual(todo1.process.id_, proc2.id_)
-        self.assertEqual(todo1.is_done, False)
-
     def test_do_POST_day_todo_adoption(self) -> None:
         """Test Todos posted to Day view may adopt existing Todos."""
         form_data = self.post_process(
@@ -586,18 +525,6 @@ class TestsWithServer(TestCaseWithServer):
         self.assertEqual(todo2.children, [todo1])
         self.assertEqual(todo2.parents, [])
 
-    def test_do_POST_day_todo_multiple(self) -> None:
-        """Test multiple Todos can be posted to Day view."""
-        # form_data = self.post_process()
-        form_data = self.post_process(2)
-        form_data = {'day_comment': '', 'new_todo': [1, 2],
-                     'make_type': 'full'}
-        self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 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.process.id_, 1)
-        self.assertEqual(todo2.process.id_, 2)
-
     def test_do_POST_day_todo_multiple_inner_adoption(self) -> None:
         """Test multiple Todos can be posted to Day view w. inner adoption."""