From: Christian Heller Date: Wed, 17 Jul 2024 23:32:56 +0000 (+0200) Subject: Ensure POST /todo autoremoval via .effort < -1 causes 404 rather than 500. X-Git-Url: https://plomlompom.com/repos/%7B%7B%20web_path%20%7D%7D/static/%7B%7Bdb.prefix%7D%7D/%7B%7Bprefix%7D%7D/index.html?a=commitdiff_plain;h=0a7391fb2c75f19dc973bd70da94b40bf98b6e5b;p=plomtask Ensure POST /todo autoremoval via .effort < -1 causes 404 rather than 500. --- diff --git a/plomtask/http.py b/plomtask/http.py index 98242e5..2aa5215 100644 --- 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 42bff44..1fa3a9d 100644 --- 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."""