From: Christian Heller Date: Thu, 30 May 2024 10:00:58 +0000 (+0200) Subject: Fix some ProcessStepping bugs. X-Git-Url: https://plomlompom.com/repos/te"st.html?a=commitdiff_plain;h=d7b039106e74a250d343ab036fdf90ad1a27eb3f;p=plomtask Fix some ProcessStepping bugs. --- diff --git a/plomtask/http.py b/plomtask/http.py index 944f79a..fd20603 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -379,9 +379,6 @@ class TaskHandler(BaseHTTPRequestHandler): if step_id not in self.form_data.get_all_int('steps'): raise BadFormatException('trying to keep unknown step') for step_id in self.form_data.get_all_int('steps'): - for step_process_id in self.form_data.get_all_int( - f'new_step_to_{step_id}'): - steps += [(None, step_process_id, step_id)] if step_id not in self.form_data.get_all_int('keep_step'): continue step_process_id = self.form_data.get_int( @@ -389,8 +386,13 @@ class TaskHandler(BaseHTTPRequestHandler): parent_id = self.form_data.get_int_or_none( f'step_{step_id}_parent_id') steps += [(step_id, step_process_id, parent_id)] + for step_id in self.form_data.get_all_int('steps'): + for step_process_id in self.form_data.get_all_int( + f'new_step_to_{step_id}'): + steps += [(None, step_process_id, step_id)] for step_process_id in self.form_data.get_all_int('new_top_step'): steps += [(None, step_process_id, None)] + process.uncache() process.set_steps(self.conn, steps) process.save(self.conn) return f'/process?id={process.id_}' diff --git a/plomtask/processes.py b/plomtask/processes.py index 684dec8..027e975 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -124,7 +124,6 @@ class Process(BaseModel[int], ConditionsRelations): just deleted under its feet), or if the parent step would not be owned by the current Process. """ - def walk_steps(node: ProcessStep) -> None: if node.step_process_id == self.id_: raise BadFormatException('bad step selection causes recursion') diff --git a/tests/processes.py b/tests/processes.py index 127a3f6..7d1d0f1 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -95,32 +95,49 @@ class TestsWithDB(TestCaseWithDB): assert isinstance(p2.id_, int) assert isinstance(p3.id_, int) steps_p1: list[tuple[int | None, int, int | None]] = [] + # add step of process p2 as first (top-level) step to p1 add_step(p1, steps_p1, (None, p2.id_, None), 1) p1_dict: dict[int, ProcessStepsNode] = {} p1_dict[1] = ProcessStepsNode(p2, None, True, {}, False) self.assertEqual(p1.get_steps(self.db_conn, None), p1_dict) + # add step of process p3 as second (top-level) step to p1 add_step(p1, steps_p1, (None, p3.id_, None), 2) step_2 = p1.explicit_steps[-1] p1_dict[2] = ProcessStepsNode(p3, None, True, {}, False) self.assertEqual(p1.get_steps(self.db_conn, None), p1_dict) + # add step of process p3 as first (top-level) step to p2, + # expect it as implicit sub-step of p1's second (p3) step steps_p2: list[tuple[int | None, int, int | None]] = [] add_step(p2, steps_p2, (None, p3.id_, None), 3) p1_dict[1].steps[3] = ProcessStepsNode(p3, None, False, {}, False) self.assertEqual(p1.get_steps(self.db_conn, None), p1_dict) + # add step of process p2 as explicit sub-step to p1's first sub-step add_step(p1, steps_p1, (None, p2.id_, step_2.id_), 4) step_3 = ProcessStepsNode(p3, None, False, {}, True) p1_dict[2].steps[4] = ProcessStepsNode(p2, step_2.id_, True, {3: step_3}, False) self.assertEqual(p1.get_steps(self.db_conn, None), p1_dict) + # add step of process p3 as explicit sub-step to non-existing p1 + # sub-step (of id=999), expect it to become another p1 top-level step add_step(p1, steps_p1, (None, p3.id_, 999), 5) p1_dict[5] = ProcessStepsNode(p3, None, True, {}, False) self.assertEqual(p1.get_steps(self.db_conn, None), p1_dict) + # add step of process p3 as explicit sub-step to p1's implicit p3 + # sub-step, expect it to become another p1 top-level step add_step(p1, steps_p1, (None, p3.id_, 3), 6) p1_dict[6] = ProcessStepsNode(p3, None, True, {}, False) self.assertEqual(p1.get_steps(self.db_conn, None), p1_dict) self.assertEqual(p1.used_as_step_by(self.db_conn), []) self.assertEqual(p2.used_as_step_by(self.db_conn), [p1]) self.assertEqual(p3.used_as_step_by(self.db_conn), [p1, p2]) + # add step of process p2 as explicit sub-step to p1's second (p3) + # top-level step + add_step(p1, steps_p1, (None, p3.id_, 2), 7) + p1_dict[2].steps[7] = ProcessStepsNode(p3, 2, True, {}, False) + # import pprint + # pprint.pp(p1.get_steps(self.db_conn, None)) + # pprint.pp(p1_dict) + self.assertEqual(p1.get_steps(self.db_conn, None), p1_dict) def test_Process_conditions(self) -> None: """Test setting Process.conditions/enables/disables.""" @@ -245,9 +262,11 @@ class TestsWithServer(TestCaseWithServer): def test_do_POST_process_steps(self) -> None: """Test behavior of ProcessStep posting.""" + # pylint: disable=too-many-statements form_data_1 = self.post_process(1) self.post_process(2) self.post_process(3) + # post first (top-level) step of process 2 to process 1 form_data_1['new_top_step'] = [2] self.post_process(1, form_data_1) retrieved_process = Process.by_id(self.db_conn, 1) @@ -256,12 +275,15 @@ class TestsWithServer(TestCaseWithServer): self.assertEqual(retrieved_step.step_process_id, 2) self.assertEqual(retrieved_step.owner_id, 1) self.assertEqual(retrieved_step.parent_step_id, None) + # post empty steps list to process, expect clean slate, and old step to + # completely disappear form_data_1['new_top_step'] = [] self.post_process(1, form_data_1) retrieved_process = Process.by_id(self.db_conn, 1) self.assertEqual(retrieved_process.explicit_steps, []) with self.assertRaises(NotFoundException): ProcessStep.by_id(self.db_conn, retrieved_step.id_) + # post new first (top_level) step of process 3 to process 1 form_data_1['new_top_step'] = [3] self.post_process(1, form_data_1) retrieved_process = Process.by_id(self.db_conn, 1) @@ -269,17 +291,21 @@ class TestsWithServer(TestCaseWithServer): self.assertEqual(retrieved_step.step_process_id, 3) self.assertEqual(retrieved_step.owner_id, 1) self.assertEqual(retrieved_step.parent_step_id, None) + # post to process steps list without keeps, expect clean slate form_data_1['new_top_step'] = [] form_data_1['steps'] = [retrieved_step.id_] self.post_process(1, form_data_1) retrieved_process = Process.by_id(self.db_conn, 1) self.assertEqual(retrieved_process.explicit_steps, []) + # post to process empty steps list but keep, expect 400 form_data_1['steps'] = [] form_data_1['keep_step'] = [retrieved_step.id_] self.check_post(form_data_1, '/process?id=1', 400, '/process?id=1') + # post to process steps list with keep on non-created step, expect 400 form_data_1['steps'] = [retrieved_step.id_] form_data_1['keep_step'] = [retrieved_step.id_] self.check_post(form_data_1, '/process?id=1', 400, '/process?id=1') + # post to process steps list with keep and process ID, expect 200 form_data_1[f'step_{retrieved_step.id_}_process_id'] = [2] self.post_process(1, form_data_1) retrieved_process = Process.by_id(self.db_conn, 1) @@ -288,12 +314,69 @@ class TestsWithServer(TestCaseWithServer): self.assertEqual(retrieved_step.step_process_id, 2) self.assertEqual(retrieved_step.owner_id, 1) self.assertEqual(retrieved_step.parent_step_id, None) + # post nonsensical new_top_step id and otherwise zero'd steps, expect + # 400 and preservation of previous state form_data_1['new_top_step'] = ['foo'] form_data_1['steps'] = [] form_data_1['keep_step'] = [] self.check_post(form_data_1, '/process?id=1', 400, '/process?id=1') retrieved_process = Process.by_id(self.db_conn, 1) self.assertEqual(len(retrieved_process.explicit_steps), 1) + retrieved_step = retrieved_process.explicit_steps[0] + self.assertEqual(retrieved_step.step_process_id, 2) + self.assertEqual(retrieved_step.owner_id, 1) + self.assertEqual(retrieved_step.parent_step_id, None) + # post to process steps list with keep and process ID, expect 200 + form_data_1['new_top_step'] = [3] + form_data_1['steps'] = [retrieved_step.id_] + form_data_1['keep_step'] = [retrieved_step.id_] + self.post_process(1, form_data_1) + retrieved_process = Process.by_id(self.db_conn, 1) + self.assertEqual(len(retrieved_process.explicit_steps), 2) + retrieved_step_0 = retrieved_process.explicit_steps[0] + self.assertEqual(retrieved_step_0.step_process_id, 2) + self.assertEqual(retrieved_step_0.owner_id, 1) + self.assertEqual(retrieved_step_0.parent_step_id, None) + retrieved_step_1 = retrieved_process.explicit_steps[1] + self.assertEqual(retrieved_step_1.step_process_id, 3) + self.assertEqual(retrieved_step_1.owner_id, 1) + self.assertEqual(retrieved_step_1.parent_step_id, None) + # post to process steps list with keeps etc., but trigger recursion + form_data_1['new_top_step'] = [] + form_data_1['steps'] = [retrieved_step_0.id_, retrieved_step_1.id_] + form_data_1['keep_step'] = [retrieved_step_0.id_, retrieved_step_1.id_] + form_data_1[f'step_{retrieved_step_0.id_}_process_id'] = [2] + form_data_1[f'step_{retrieved_step_1.id_}_process_id'] = [1] + self.check_post(form_data_1, '/process?id=1', 400, '/process?id=1') + # check previous status preserved despite failed steps setting + retrieved_process = Process.by_id(self.db_conn, 1) + self.assertEqual(len(retrieved_process.explicit_steps), 2) + retrieved_step_0 = retrieved_process.explicit_steps[0] + self.assertEqual(retrieved_step_0.step_process_id, 2) + self.assertEqual(retrieved_step_0.owner_id, 1) + self.assertEqual(retrieved_step_0.parent_step_id, None) + retrieved_step_1 = retrieved_process.explicit_steps[1] + self.assertEqual(retrieved_step_1.step_process_id, 3) + self.assertEqual(retrieved_step_1.owner_id, 1) + self.assertEqual(retrieved_step_1.parent_step_id, None) + form_data_1[f'step_{retrieved_step_1.id_}_process_id'] = [3] + # post sub-step to step + form_data_1[f'new_step_to_{retrieved_step_1.id_}'] = [3] + self.post_process(1, form_data_1) + retrieved_process = Process.by_id(self.db_conn, 1) + self.assertEqual(len(retrieved_process.explicit_steps), 3) + retrieved_step_0 = retrieved_process.explicit_steps[0] + self.assertEqual(retrieved_step_0.step_process_id, 2) + self.assertEqual(retrieved_step_0.owner_id, 1) + self.assertEqual(retrieved_step_0.parent_step_id, None) + retrieved_step_1 = retrieved_process.explicit_steps[1] + self.assertEqual(retrieved_step_1.step_process_id, 3) + self.assertEqual(retrieved_step_1.owner_id, 1) + self.assertEqual(retrieved_step_1.parent_step_id, None) + retrieved_step_2 = retrieved_process.explicit_steps[2] + self.assertEqual(retrieved_step_2.step_process_id, 3) + self.assertEqual(retrieved_step_2.owner_id, 1) + self.assertEqual(retrieved_step_2.parent_step_id, retrieved_step_1.id_) def test_do_GET(self) -> None: """Test /process and /processes response codes."""