From: Christian Heller Date: Sun, 28 Apr 2024 21:17:58 +0000 (+0200) Subject: Perform sensible redirects on POSTs. X-Git-Url: https://plomlompom.com/repos/%7B%7Bdb.prefix%7D%7D/%7B%7B%20web_path%20%7D%7D/%7B%7Bprefix%7D%7D/error?a=commitdiff_plain;h=e14580b4ee47363cad317e4ec1de91affe03d53a;p=plomtask Perform sensible redirects on POSTs. --- diff --git a/plomtask/http.py b/plomtask/http.py index b4c2b08..800193c 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -182,18 +182,18 @@ class TaskHandler(BaseHTTPRequestHandler): keep_blank_values=True, strict_parsing=True) self.form_data = InputsParser(postvars) if self.site in ('day', 'process', 'todo', 'condition'): - getattr(self, f'do_POST_{self.site}')() + redir_target = getattr(self, f'do_POST_{self.site}')() self.conn.commit() else: msg = f'Page not known as POST target: /{self.site}' raise NotFoundException(msg) - self._redirect('/') + self._redirect(redir_target) except HandledException as error: self._send_msg(error, code=error.http_code) finally: self.conn.close() - def do_POST_day(self) -> None: + def do_POST_day(self) -> str: """Update or insert Day of date and Todos mapped to it.""" date = self.params.get_str('date') day = Day.by_id(self.conn, date, create=True) @@ -207,8 +207,9 @@ class TaskHandler(BaseHTTPRequestHandler): todo.adopt_from(existing_todos) todo.make_missing_children(self.conn) todo.save(self.conn) + return f'/day?date={date}' - def do_POST_todo(self) -> None: + def do_POST_todo(self) -> str: """Update Todo and its children.""" id_ = self.params.get_int('id') todo = Todo.by_id(self.conn, id_) @@ -232,14 +233,15 @@ class TaskHandler(BaseHTTPRequestHandler): condition.save(self.conn) for condition in todo.disables: condition.save(self.conn) + return f'/todo?id={todo.id_}' - def do_POST_process(self) -> None: + def do_POST_process(self) -> str: """Update or insert Process of ?id= and fields defined in postvars.""" id_ = self.params.get_int_or_none('id') for _ in self.form_data.get_all_str('delete'): process = Process.by_id(self.conn, id_) process.remove(self.conn) - return + return '/processes' process = Process.by_id(self.conn, id_, create=True) process.title.set(self.form_data.get_str('title')) process.description.set(self.form_data.get_str('description')) @@ -266,14 +268,16 @@ class TaskHandler(BaseHTTPRequestHandler): steps += [(None, step_process_id, None)] process.set_steps(self.conn, steps) process.save(self.conn) + return f'/process?id={process.id_}' - def do_POST_condition(self) -> None: + def do_POST_condition(self) -> str: """Update/insert Condition of ?id= and fields defined in postvars.""" id_ = self.params.get_int_or_none('id') condition = Condition.by_id(self.conn, id_, create=True) condition.title.set(self.form_data.get_str('title')) condition.description.set(self.form_data.get_str('description')) condition.save(self.conn) + return f'/condition?id={condition.id_}' def _init_handling(self) -> None: # pylint: disable=attribute-defined-outside-init diff --git a/tests/conditions.py b/tests/conditions.py index b6510a1..3b95de1 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -43,7 +43,7 @@ class TestsWithServer(TestCaseWithServer): def test_do_POST_condition(self) -> None: """Test POST /condition and its effect on the database.""" form_data = {'title': 'foo', 'description': 'foo'} - self.check_post(form_data, '/condition', 302, '/') + self.check_post(form_data, '/condition', 302, '/condition?id=1') self.assertEqual(1, len(Condition.all(self.db_conn))) def test_do_GET(self) -> None: diff --git a/tests/days.py b/tests/days.py index 895f59d..17c0ba9 100644 --- a/tests/days.py +++ b/tests/days.py @@ -118,5 +118,5 @@ class TestsWithServer(TestCaseWithServer): form_data = {'comment': ''} self.check_post(form_data, '/day', 400) self.check_post(form_data, '/day?date=foo', 400) - self.check_post(form_data, '/day?date=2024-01-01', 302, '/') + self.check_post(form_data, '/day?date=2024-01-01', 302) self.check_post({'foo': ''}, '/day?date=2024-01-01', 400) diff --git a/tests/processes.py b/tests/processes.py index 6695f78..2ac8cf4 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -181,7 +181,7 @@ class TestsWithServer(TestCaseWithServer): """Test POST /process and its effect on the database.""" self.assertEqual(0, len(Process.all(self.db_conn))) form_data = {'title': 'foo', 'description': 'foo', 'effort': 1.1} - self.check_post(form_data, '/process?id=', 302, '/') + self.check_post(form_data, '/process?id=', 302, '/process?id=1') self.assertEqual(1, len(Process.all(self.db_conn))) self.check_post(form_data, '/process?id=FOO', 400) form_data['effort'] = 'foo' @@ -196,16 +196,20 @@ class TestsWithServer(TestCaseWithServer): self.assertEqual(1, len(Process.all(self.db_conn))) form_data = {'title': 'foo', 'description': 'foo', 'effort': 1.0, 'condition': []} - self.check_post(form_data, '/process?id=', 302, '/') + self.check_post(form_data, '/process?id=', 302, '/process?id=2') form_data['condition'] = [1] self.check_post(form_data, '/process?id=', 404) form_data_cond = {'title': 'foo', 'description': 'foo'} - self.check_post(form_data_cond, '/condition', 302, '/') - self.check_post(form_data, '/process?id=', 302, '/') + self.check_post(form_data_cond, '/condition', 302, '/condition?id=1') + self.check_post(form_data, '/process?id=', 302, '/process?id=3') form_data['disables'] = [1] - self.check_post(form_data, '/process?id=', 302, '/') + self.check_post(form_data, '/process?id=', 302, '/process?id=4') form_data['enables'] = [1] - self.check_post(form_data, '/process?id=', 302, '/') + self.check_post(form_data, '/process?id=', 302, '/process?id=5') + form_data['delete'] = '' + self.check_post(form_data, '/process?id=', 404) + self.check_post(form_data, '/process?id=6', 404) + self.check_post(form_data, '/process?id=5', 302, '/processes') def test_do_GET(self) -> None: """Test /process and /processes response codes.""" diff --git a/tests/todos.py b/tests/todos.py index b47834c..182bd55 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -273,15 +273,15 @@ class TestsWithServer(TestCaseWithServer): def test_do_POST_day(self) -> None: """Test Todo posting of POST /day.""" form_data = {'title': '', 'description': '', 'effort': 1} - self.check_post(form_data, '/process?id=', 302, '/') - self.check_post(form_data, '/process?id=', 302, '/') + self.check_post(form_data, '/process?id=', 302, '/process?id=1') + self.check_post(form_data, '/process?id=', 302, '/process?id=2') proc = Process.by_id(self.db_conn, 1) proc2 = Process.by_id(self.db_conn, 2) form_data = {'comment': ''} - self.check_post(form_data, '/day?date=2024-01-01', 302, '/') + self.check_post(form_data, '/day?date=2024-01-01', 302) self.assertEqual(Todo.by_date(self.db_conn, '2024-01-01'), []) form_data['new_todo'] = str(proc.id_) - self.check_post(form_data, '/day?date=2024-01-01', 302, '/') + self.check_post(form_data, '/day?date=2024-01-01', 302) todos = Todo.by_date(self.db_conn, '2024-01-01') self.assertEqual(1, len(todos)) todo1 = todos[0] @@ -289,7 +289,7 @@ class TestsWithServer(TestCaseWithServer): self.assertEqual(todo1.process.id_, proc.id_) self.assertEqual(todo1.is_done, False) form_data['new_todo'] = str(proc2.id_) - self.check_post(form_data, '/day?date=2024-01-01', 302, '/') + self.check_post(form_data, '/day?date=2024-01-01', 302) todos = Todo.by_date(self.db_conn, '2024-01-01') todo1 = todos[1] self.assertEqual(todo1.id_, 2) @@ -300,11 +300,11 @@ class TestsWithServer(TestCaseWithServer): """Test POST /todo.""" def post_and_reload(form_data: dict[str, object], status: int = 302) -> Todo: - self.check_post(form_data, '/todo?id=1', status, '/') + self.check_post(form_data, '/todo?id=1', status) 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, '/process?id=1') form_data = {'comment': '', 'new_todo': 1} self.check_post(form_data, '/day?date=2024-01-01', 302) # test posting to bad URLs @@ -360,9 +360,9 @@ class TestsWithServer(TestCaseWithServer): 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, '/') + self.check_post(form_data, '/process', 302, '/process?id=1') form_data['new_top_step'] = 1 - self.check_post(form_data, '/process', 302, '/') + self.check_post(form_data, '/process', 302, '/process?id=2') form_data = {'comment': '', 'new_todo': 1} self.check_post(form_data, '/day?date=2024-01-01', 302) form_data = {'comment': '', 'new_todo': 2} @@ -377,9 +377,9 @@ class TestsWithServer(TestCaseWithServer): def test_do_GET_todo(self) -> None: """Test GET /todo response codes.""" form_data = {'title': '', 'description': '', 'effort': 1} - self.check_post(form_data, '/process?id=', 302, '/') + self.check_post(form_data, '/process?id=', 302, '/process?id=1') 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) self.check_get('/todo', 400) self.check_get('/todo?id=', 400) self.check_get('/todo?id=foo', 400) diff --git a/tests/utils.py b/tests/utils.py index 63b07e9..2a919a3 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -63,13 +63,15 @@ class TestCaseWithServer(TestCaseWithDB): self.assertEqual(self.conn.getresponse().status, expected_code) def check_post(self, data: Mapping[str, object], target: str, - expected_code: int, redirect_location: str = '/') -> None: + expected_code: int, redirect_location: str = '') -> None: """Check that POST of data to target yields expected_code.""" encoded_form_data = urlencode(data, doseq=True).encode('utf-8') headers = {'Content-Type': 'application/x-www-form-urlencoded', 'Content-Length': str(len(encoded_form_data))} self.conn.request('POST', target, body=encoded_form_data, headers=headers) + if redirect_location == '': + redirect_location = target if 302 == expected_code: self.check_redirect(redirect_location) else: