From 24ba8263349bff3ba7ca89016c3928d2d4751d1c Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 5 Aug 2024 07:57:34 +0200 Subject: [PATCH 01/16] Minor fixes of freshly introduced bugs. --- plomtask/processes.py | 8 +++++--- plomtask/todos.py | 16 +++++++--------- tests/processes.py | 16 ++++++++++------ 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/plomtask/processes.py b/plomtask/processes.py index b68ffd8..56dd282 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -14,11 +14,13 @@ class ProcessStepsNode(DictableNode): """Collects what's useful to know for ProcessSteps tree display.""" # pylint: disable=too-few-public-methods step: ProcessStep + process: Process is_explicit: bool steps: list[ProcessStepsNode] seen: bool = False is_suppressed: bool = False - _to_dict = ['step', 'is_explicit', 'steps', 'seen', 'is_suppressed'] + _to_dict = ['step', 'process', 'is_explicit', 'steps', 'seen', + 'is_suppressed'] class Process(BaseModel[int], ConditionsRelations): @@ -98,8 +100,8 @@ class Process(BaseModel[int], ConditionsRelations): step_steps = [] if not suppressed: step_steps = process.get_steps(db_conn, external_owner) - return ProcessStepsNode(step, is_explicit, step_steps, False, - suppressed) + return ProcessStepsNode(step, process, is_explicit, step_steps, + False, suppressed) def walk_steps(node: ProcessStepsNode) -> None: node.seen = node.step.id_ in seen_step_ids diff --git a/plomtask/todos.py b/plomtask/todos.py index 3291107..9f9fdb4 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -94,26 +94,24 @@ class Todo(BaseModel[int], ConditionsRelations): adoptables = [t for t in Todo.by_date(db_conn, parent.date) if (t not in parent.children) and (t != parent) - and step_node.step.step_process_id == t.process.id_] + and step_node.process.id_ == t.process_id] satisfier = None for adoptable in adoptables: satisfier = adoptable break if not satisfier: - proc = Process.by_id(db_conn, step_node.step.step_process_id) - satisfier = Todo(None, proc, False, parent.date) + satisfier = Todo(None, step_node.process, False, parent.date) satisfier.save(db_conn) - sub_step_nodes = sorted(step_node.steps, - key=lambda s: s.step.step_process_id) + sub_step_nodes = sorted( + step_node.steps, + key=lambda s: s.process.id_ if s.process.id_ else 0) for sub_node in sub_step_nodes: if sub_node.is_suppressed: continue n_slots = len([n for n in sub_step_nodes - if n.step.step_process_id - == sub_node.step.step_process_id]) + if n.process == sub_node.process]) filled_slots = len([t for t in satisfier.children - if t.process.id_ - == sub_node.step.step_process_id_]) + if t.process.id_ == sub_node.process.id_]) # if we did not newly create satisfier, it may already fill # some step dependencies, so only fill what remains open if n_slots - filled_slots > 0: diff --git a/tests/processes.py b/tests/processes.py index 9affa84..6f2dc42 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -252,12 +252,15 @@ class ExpectedGetProcess(Expected): @staticmethod def stepnode_as_dict(step_id: int, + proc_id: int, seen: bool = False, steps: None | list[dict[str, object]] = None, is_explicit: bool = True, is_suppressed: bool = False) -> dict[str, object]: + # pylint: disable=too-many-arguments """Return JSON of ProcessStepNode to expect.""" return {'step': step_id, + 'process': proc_id, 'seen': seen, 'steps': steps if steps else [], 'is_explicit': is_explicit, @@ -352,7 +355,7 @@ class TestsWithServer(TestCaseWithServer): # post first (top-level) step of proc 2 to proc 1 by 'step_of' in 2 self.post_exp_process([exp], {'step_of': 1}, 2) exp.lib_set('ProcessStep', [exp.procstep_as_dict(1, 1, 2)]) - exp.set('steps', [exp.stepnode_as_dict(1)]) + exp.set('steps', [exp.stepnode_as_dict(1, 2)]) self.check_json_get(url, exp) # post empty/absent steps list to process, expect clean slate, and old # step to completely disappear @@ -364,7 +367,7 @@ class TestsWithServer(TestCaseWithServer): self.post_exp_process([exp], {'new_top_step': 2}, 1) exp.lib_set('ProcessStep', [exp.procstep_as_dict(1, 1, 2)]) self.post_exp_process([exp], {'kept_steps': [1]}, 1) - exp.set('steps', [exp.stepnode_as_dict(1)]) + exp.set('steps', [exp.stepnode_as_dict(1, 2)]) self.check_json_get(url, exp) # fail on single- and multi-step recursion p_min = {'title': '', 'description': '', 'effort': 0} @@ -381,15 +384,16 @@ class TestsWithServer(TestCaseWithServer): self.post_exp_process([exp], {'kept_steps': [1], 'new_top_step': 4}, 1) exp.lib_set('ProcessStep', [exp.procstep_as_dict(1, 1, 4), exp.procstep_as_dict(2, 1, 4)]) - exp.set('steps', [exp.stepnode_as_dict(1), exp.stepnode_as_dict(2)]) + exp.set('steps', [exp.stepnode_as_dict(1, 4), + exp.stepnode_as_dict(2, 4)]) self.check_json_get(url, exp) # post sub-step chain p = {'kept_steps': [1, 2], 'new_step_to_2': 4} self.post_exp_process([exp], p, 1) exp.lib_set('ProcessStep', [exp.procstep_as_dict(3, 1, 4, 2)]) - exp.set('steps', [exp.stepnode_as_dict(1), - exp.stepnode_as_dict(2, steps=[ - exp.stepnode_as_dict(3)])]) + exp.set('steps', [exp.stepnode_as_dict(1, 4), + exp.stepnode_as_dict(2, 4, steps=[ + exp.stepnode_as_dict(3, 4)])]) self.check_json_get(url, exp) # fail posting sub-step that would cause recursion self.post_exp_process([exp], {}, 6) -- 2.30.2 From e7256be2e49b9d784d920e711f584b3e4d658358 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 5 Aug 2024 09:07:33 +0200 Subject: [PATCH 02/16] Improve readability of do_POST_process handler code. --- plomtask/http.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index fa04579..b10ce3a 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -722,10 +722,10 @@ class TaskHandler(BaseHTTPRequestHandler): calendarize = self._form_data.get_bool('calendarize') step_of = self._form_data.get_all_str('step_of') suppresses = self._form_data.get_all_int('suppresses') - step_ids = self._form_data.get_all_int('kept_steps') + kept_steps = self._form_data.get_all_int('kept_steps') new_top_steps = self._form_data.get_all_str('new_top_step') new_steps_to = {} - for step_id in step_ids: + for step_id in kept_steps: name = f'new_step_to_{step_id}' new_steps_to[step_id] = self._form_data.get_all_int(name) for k, v in versioned.items(): @@ -735,6 +735,11 @@ class TaskHandler(BaseHTTPRequestHandler): process.save(self.conn) assert isinstance(process.id_, int) # set relations to, and if non-existant yet: create, other Processes + # pylint: disable=fixme + # TODO: in what order to set owners, owneds, and possibly step + # suppressions can make the difference between recursion checks + # failing; should probably be handled class-internally to Process + # rather than here! # 1. owners (upwards) owners_to_set = [] new_owner_title = None @@ -747,18 +752,19 @@ class TaskHandler(BaseHTTPRequestHandler): # 2. owneds (downwards) new_step_title = None steps: list[ProcessStep] = [ProcessStep.by_id(self.conn, step_id) - for step_id in step_ids] - for step_id in step_ids: - new = [ProcessStep(None, process.id_, step_process_id, step_id) - for step_process_id in new_steps_to[step_id]] - steps += new - for step_identifier in new_top_steps: + for step_id in kept_steps] + for step_id in kept_steps: + new_sub_steps = [ + ProcessStep(None, process.id_, step_process_id, step_id) + for step_process_id in new_steps_to[step_id]] + steps += new_sub_steps + for step_id_or_new_title in new_top_steps: try: - step_process_id = int(step_identifier) + step_process_id = int(step_id_or_new_title) step = ProcessStep(None, process.id_, step_process_id, None) steps += [step] except ValueError: - new_step_title = step_identifier + new_step_title = step_id_or_new_title process.set_steps(self.conn, steps) process.set_step_suppressions(self.conn, suppresses) # encode titles for potentially newly created Processes up or down -- 2.30.2 From 82e0437e1310b1adea92ccde4a965ba78ed8bd03 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 5 Aug 2024 09:29:05 +0200 Subject: [PATCH 03/16] Minor improvements of Processes tests. --- tests/processes.py | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/processes.py b/tests/processes.py index 6f2dc42..649aee5 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -4,8 +4,7 @@ from tests.utils import (TestCaseSansDB, TestCaseWithDB, TestCaseWithServer, Expected) from plomtask.processes import Process, ProcessStep from plomtask.conditions import Condition -from plomtask.exceptions import HandledException, NotFoundException -from plomtask.todos import Todo +from plomtask.exceptions import NotFoundException class TestsSansDB(TestCaseSansDB): @@ -193,26 +192,21 @@ class TestsWithDB(TestCaseWithDB): step = ProcessStep(None, p2.id_, p1.id_, None) p2.set_steps(self.db_conn, [step]) step_id = step.id_ - with self.assertRaises(HandledException): - p1.remove(self.db_conn) p2.set_steps(self.db_conn, []) with self.assertRaises(NotFoundException): + # check unset ProcessSteps actually cannot be found anymore assert step_id is not None ProcessStep.by_id(self.db_conn, step_id) p1.remove(self.db_conn) step = ProcessStep(None, p2.id_, p3.id_, None) p2.set_steps(self.db_conn, [step]) step_id = step.id_ + # check _can_ remove Process pointed to by ProcessStep.owner_id, and … p2.remove(self.db_conn) with self.assertRaises(NotFoundException): + # … being dis-owned eliminates ProcessStep assert step_id is not None ProcessStep.by_id(self.db_conn, step_id) - todo = Todo(None, p3, False, '2024-01-01') - todo.save(self.db_conn) - with self.assertRaises(HandledException): - p3.remove(self.db_conn) - todo.remove(self.db_conn) - p3.remove(self.db_conn) class TestsWithDBForProcessStep(TestCaseWithDB): @@ -338,13 +332,20 @@ class TestsWithServer(TestCaseWithServer): self.check_json_get('/process?id=1', exp) # check cannot delete if Todos to Process self.check_post({'delete': ''}, '/process?id=1', 500) + # check cannot delete if some ProcessStep's .step_process_id + self.post_exp_process([exp], valid_post, 2) + self.post_exp_process([exp], valid_post | {'new_top_step': 2}, 3) + self.check_post({'delete': ''}, '/process?id=2', 500) # check successful deletion - self.post_exp_process([], valid_post, 2) - self.check_post({'delete': ''}, '/process?id=2', 302, '/processes') - exp = ExpectedGetProcess(2) + self.post_exp_process([exp], valid_post, 4) + self.check_post({'delete': ''}, '/process?id=4', 302, '/processes') + exp = ExpectedGetProcess(4) exp.set_proc_from_post(1, valid_post) - exp.force('process_candidates', [1]) - self.check_json_get('/process?id=2', exp) + exp.set_proc_from_post(2, valid_post) + exp.set_proc_from_post(3, valid_post) + exp.lib_set('ProcessStep', [exp.procstep_as_dict(1, 3, 2)]) + exp.force('process_candidates', [1, 2, 3]) + self.check_json_get('/process?id=4', exp) def test_POST_process_steps(self) -> None: """Test behavior of ProcessStep posting.""" -- 2.30.2 From ff1c4c071834cbf39bab5cc7bac6a9ac8d33df2b Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Mon, 5 Aug 2024 10:01:13 +0200 Subject: [PATCH 04/16] Slightly reduce the do_POST_todo code. --- plomtask/http.py | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index b10ce3a..1aa2e49 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -636,19 +636,20 @@ class TaskHandler(BaseHTTPRequestHandler): # pylint: disable=too-many-locals # pylint: disable=too-many-branches 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') + to_make = {'full': self._form_data.get_all_int('make_full'), + 'empty': self._form_data.get_all_int('make_empty')} step_fillers = self._form_data.get_all_str('step_filler') to_update = { 'is_done': self._form_data.get_bool('done'), 'calendarize': self._form_data.get_bool('calendarize'), 'comment': self._form_data.get_str('comment', ignore_strict=True)} + cond_rels = [self._form_data.get_all_int(name) for name in + ['conditions', 'blockers', 'enables', 'disables']] try: to_update['effort'] = self._form_data.get_float_or_none('effort') except NotFoundException: pass - cond_rel_id_lists = [self._form_data.get_all_int(name) for name in - ['conditions', 'blockers', 'enables', 'disables']] + todo.set_condition_relations(self.conn, *cond_rels) for filler in [f for f in step_fillers if f != 'ignore']: target_id: int to_int = filler @@ -661,15 +662,14 @@ class TaskHandler(BaseHTTPRequestHandler): msg = 'bad fill_for target: {filler}' raise BadFormatException(msg) from e if filler.startswith('make_empty_'): - processes_to_make_empty += [target_id] + to_make['empty'] += [target_id] elif filler.startswith('make_full_'): - processes_to_make_full += [target_id] + to_make['full'] += [target_id] else: adopted_child_ids += [target_id] to_remove = [] for child in todo.children: - assert isinstance(child.id_, int) - if child.id_ not in adopted_child_ids: + if child.id_ and (child.id_ not in adopted_child_ids): to_remove += [child.id_] for id_ in to_remove: child = Todo.by_id(self.conn, id_) @@ -677,19 +677,15 @@ class TaskHandler(BaseHTTPRequestHandler): for child_id in adopted_child_ids: if child_id not in [c.id_ for c in todo.children]: todo.add_child(Todo.by_id(self.conn, child_id)) - for process_id in processes_to_make_empty: - process = Process.by_id(self.conn, process_id) - made = Todo(None, process, False, todo.date) - made.save(self.conn) - todo.add_child(made) - for process_id in processes_to_make_full: - process = Process.by_id(self.conn, process_id) - made = Todo(None, process, False, todo.date) - made.save(self.conn) - made.ensure_children(self.conn) - todo.add_child(made) - todo.set_condition_relations(self.conn, *cond_rel_id_lists) todo.update_attrs(**to_update) + for approach, proc_ids in to_make.items(): + for process_id in proc_ids: + process = Process.by_id(self.conn, process_id) + made = Todo(None, process, False, todo.date) + made.save(self.conn) + if 'full' == approach: + made.ensure_children(self.conn) + todo.add_child(made) # todo.save() may destroy Todo if .effort < 0, so retrieve .id_ early url = f'/todo?id={todo.id_}' todo.save(self.conn) -- 2.30.2 From 156802cfa6fd264d5aa972f983fb089b6d95dc68 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Wed, 7 Aug 2024 14:57:06 +0200 Subject: [PATCH 05/16] Fix broken POST /condition parsing of "is active" setting. --- plomtask/http.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 1aa2e49..2c8eecb 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -786,12 +786,11 @@ class TaskHandler(BaseHTTPRequestHandler): def do_POST_condition(self, condition: Condition) -> str: """Update/insert Condition of ?id= and fields defined in postvars.""" try: - is_active = self._form_data.get_str('is_active') == 'True' title = self._form_data.get_str('title') description = self._form_data.get_str('description') except NotFoundException as e: raise BadFormatException(e) from e - condition.is_active = is_active + condition.is_active = self._form_data.get_bool('is_active') condition.title.set(title) condition.description.set(description) condition.save(self.conn) -- 2.30.2 From 08264accb5a5958c0eb97987b209621548bd618f Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Thu, 8 Aug 2024 05:40:51 +0200 Subject: [PATCH 06/16] Simplify InputParser code/usage. --- plomtask/http.py | 225 +++++++++++++++---------------- tests/conditions.py | 1 - tests/misc.py | 318 +++++++++++++++++++------------------------- 3 files changed, 240 insertions(+), 304 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 2c8eecb..25e377d 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -36,82 +36,69 @@ class TaskServer(HTTPServer): class InputsParser: """Wrapper for validating and retrieving dict-like HTTP inputs.""" - def __init__(self, dict_: dict[str, list[str]], - strictness: bool = True) -> None: + def __init__(self, dict_: dict[str, list[str]]) -> None: self.inputs = dict_ - self.strict = strictness # return None on absence of key, or fail? - def get_str(self, key: str, default: str = '', - ignore_strict: bool = False) -> str: - """Retrieve single/first string value of key, or default.""" - if key not in self.inputs.keys() or 0 == len(self.inputs[key]): - if self.strict and not ignore_strict: - raise NotFoundException(f'no value found for key {key}') - return default - return self.inputs[key][0] + def get_all_str(self, key: str) -> list[str]: + """Retrieve list of string values at key (empty if no key).""" + if key not in self.inputs.keys(): + return [] + return self.inputs[key] - def get_first_strings_starting(self, prefix: str) -> dict[str, str]: - """Retrieve dict of (first) strings at key starting with prefix.""" - ret = {} - for key in [k for k in self.inputs.keys() if k.startswith(prefix)]: - ret[key] = self.inputs[key][0] - return ret + def get_all_int(self, key: str) -> list[int]: + """Retrieve list of int values at key.""" + all_str = self.get_all_str(key) + try: + return [int(s) for s in all_str if len(s) > 0] + except ValueError as e: + msg = f'cannot int a form field value for key {key} in: {all_str}' + raise BadFormatException(msg) from e - def get_int(self, key: str) -> int: - """Retrieve single/first value of key as int, error if empty.""" - val = self.get_int_or_none(key) - if val is None: - raise BadFormatException(f'unexpected empty value for: {key}') - return val + def get_str(self, key: str, default: str | None = None) -> str | None: + """Retrieve single/first string value of key, or default.""" + vals = self.get_all_str(key) + if vals: + return vals[0] + return default + + def get_str_or_fail(self, key: str, default: str | None = None) -> str: + """Retrieve first string value of key, if none: fail or default.""" + vals = self.get_all_str(key) + if not vals: + if default is not None: + return default + raise BadFormatException(f'no value found for key: {key}') + return vals[0] def get_int_or_none(self, key: str) -> int | None: """Retrieve single/first value of key as int, return None if empty.""" - val = self.get_str(key, ignore_strict=True) + val = self.get_str_or_fail(key, '') if val == '': return None try: return int(val) - except ValueError as e: + except (ValueError, TypeError) as e: msg = f'cannot int form field value for key {key}: {val}' raise BadFormatException(msg) from e - def get_float(self, key: str) -> float: - """Retrieve float value of key from self.postvars.""" - val = self.get_str(key) - try: - return float(val) - except ValueError as e: - msg = f'cannot float form field value for key {key}: {val}' - raise BadFormatException(msg) from e - - def get_float_or_none(self, key: str) -> float | None: - """Retrieve float value of key from self.postvars, None if empty.""" - val = self.get_str(key) - if '' == val: - return None - try: - return float(val) - except ValueError as e: - msg = f'cannot float form field value for key {key}: {val}' - raise BadFormatException(msg) from e - def get_bool(self, key: str) -> bool: - """Return if key occurs and maps to a boolean Truth.""" - return bool(self.get_all_str(key)) + """Return if key occurs _and_ its value maps to a boolean Truth.""" + return self.get_str(key) not in {None, 'False'} - def get_all_str(self, key: str) -> list[str]: - """Retrieve list of string values at key.""" - if key not in self.inputs.keys(): - return [] - return self.inputs[key] + def get_firsts_of_key_prefixed(self, prefix: str) -> dict[str, str]: + """Retrieve dict of (first) strings at key starting with prefix.""" + ret = {} + for key in [k for k in self.inputs.keys() if k.startswith(prefix)]: + ret[key] = self.inputs[key][0] + return ret - def get_all_int(self, key: str) -> list[int]: - """Retrieve list of int values at key.""" - all_str = self.get_all_str(key) + def get_float_or_fail(self, key: str) -> float: + """Retrieve float value of key from self.postvars, fail if none.""" + val = self.get_str_or_fail(key) try: - return [int(s) for s in all_str if len(s) > 0] + return float(val) except ValueError as e: - msg = f'cannot int a form field value for key {key} in: {all_str}' + msg = f'cannot float form field value for key {key}: {val}' raise BadFormatException(msg) from e def get_all_floats_or_nones(self, key: str) -> list[float | None]: @@ -135,7 +122,7 @@ class TaskHandler(BaseHTTPRequestHandler): server: TaskServer conn: DatabaseConnection _site: str - _form_data: InputsParser + _form: InputsParser _params: InputsParser def _send_page( @@ -253,8 +240,10 @@ class TaskHandler(BaseHTTPRequestHandler): self.conn = DatabaseConnection(self.server.db) parsed_url = urlparse(self.path) self._site = path_split(parsed_url.path)[1] - params = parse_qs(parsed_url.query, strict_parsing=True) - self._params = InputsParser(params, False) + params = parse_qs(parsed_url.query, + keep_blank_values=True, + strict_parsing=True) + self._params = InputsParser(params) handler_name = f'do_{http_method}_{self._site}' if hasattr(self, handler_name): handler = getattr(self, handler_name) @@ -294,8 +283,8 @@ class TaskHandler(BaseHTTPRequestHandler): """Handle POST with handler, prepare redirection to result.""" length = int(self.headers['content-length']) postvars = parse_qs(self.rfile.read(length).decode(), - keep_blank_values=True, strict_parsing=True) - self._form_data = InputsParser(postvars) + keep_blank_values=True) + self._form = InputsParser(postvars) redir_target = handler() self.conn.commit() return redir_target @@ -332,8 +321,9 @@ class TaskHandler(BaseHTTPRequestHandler): same, the only difference being the HTML template they are rendered to, which .do_GET selects from their method name. """ - start, end = self._params.get_str('start'), self._params.get_str('end') - end = end if end else date_in_n_days(366) + start = self._params.get_str_or_fail('start', '') + end = self._params.get_str_or_fail('end', '') + end = end if end != '' else date_in_n_days(366) days, start, end = Day.by_date_range_with_limits(self.conn, (start, end), 'id') days = Day.with_filled_gaps(days, start, end) @@ -350,9 +340,9 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_day(self) -> dict[str, object]: """Show single Day of ?date=.""" - date = self._params.get_str('date', date_in_n_days(0)) + date = self._params.get_str_or_fail('date', date_in_n_days(0)) day = Day.by_id_or_create(self.conn, date) - make_type = self._params.get_str('make_type') + make_type = self._params.get_str_or_fail('make_type', '') conditions_present = [] enablers_for = {} disablers_for = {} @@ -451,11 +441,11 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_todos(self) -> dict[str, object]: """Show Todos from ?start= to ?end=, of ?process=, ?comment= pattern""" - sort_by = self._params.get_str('sort_by') - start = self._params.get_str('start') - end = self._params.get_str('end') + sort_by = self._params.get_str_or_fail('sort_by', '') + start = self._params.get_str_or_fail('start', '') + end = self._params.get_str_or_fail('end', '') process_id = self._params.get_int_or_none('process_id') - comment_pattern = self._params.get_str('comment_pattern') + comment_pattern = self._params.get_str_or_fail('comment_pattern', '') todos = [] ret = Todo.by_date_range_with_limits(self.conn, (start, end)) todos_by_date_range, start, end = ret @@ -469,8 +459,8 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_conditions(self) -> dict[str, object]: """Show all Conditions.""" - pattern = self._params.get_str('pattern') - sort_by = self._params.get_str('sort_by') + pattern = self._params.get_str_or_fail('pattern', '') + sort_by = self._params.get_str_or_fail('sort_by', '') conditions = Condition.matching(self.conn, pattern) sort_by = Condition.sort_by(conditions, sort_by) return {'conditions': conditions, @@ -542,8 +532,8 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_processes(self) -> dict[str, object]: """Show all Processes.""" - pattern = self._params.get_str('pattern') - sort_by = self._params.get_str('sort_by') + pattern = self._params.get_str_or_fail('pattern', '') + sort_by = self._params.get_str_or_fail('sort_by', '') processes = Process.matching(self.conn, pattern) sort_by = Process.sort_by(processes, sort_by) return {'processes': processes, 'sort_by': sort_by, 'pattern': pattern} @@ -560,7 +550,7 @@ class TaskHandler(BaseHTTPRequestHandler): # (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'): + for _ in self._form.get_all_str('delete'): if id_ is None: msg = 'trying to delete non-saved ' +\ f'{target_class.__name__}' @@ -581,7 +571,7 @@ class TaskHandler(BaseHTTPRequestHandler): id_ = self._params.get_int_or_none('id') item = cls.by_id(self.conn, id_) attr = getattr(item, attr_name) - for k, v in self._form_data.get_first_strings_starting('at:').items(): + for k, v in self._form.get_firsts_of_key_prefixed('at:').items(): old = k[3:] if old[19:] != v: attr.reset_timestamp(old, f'{v}.0') @@ -591,17 +581,14 @@ class TaskHandler(BaseHTTPRequestHandler): def do_POST_day(self) -> str: """Update or insert Day of date and Todos mapped to it.""" # pylint: disable=too-many-locals - try: - date = self._params.get_str('date') - day_comment = self._form_data.get_str('day_comment') - make_type = self._form_data.get_str('make_type') - except NotFoundException as e: - raise BadFormatException from e - old_todos = self._form_data.get_all_int('todo_id') - new_todos_by_process = self._form_data.get_all_int('new_todo') - comments = self._form_data.get_all_str('comment') - efforts = self._form_data.get_all_floats_or_nones('effort') - done_todos = self._form_data.get_all_int('done') + date = self._params.get_str_or_fail('date') + day_comment = self._form.get_str_or_fail('day_comment') + make_type = self._form.get_str_or_fail('make_type') + old_todos = self._form.get_all_int('todo_id') + new_todos_by_process = self._form.get_all_int('new_todo') + comments = self._form.get_all_str('comment') + efforts = self._form.get_all_floats_or_nones('effort') + done_todos = self._form.get_all_int('done') for _ in [id_ for id_ in done_todos if id_ not in old_todos]: raise BadFormatException('"done" field refers to unknown Todo') is_done = [t_id in done_todos for t_id in old_todos] @@ -635,20 +622,26 @@ class TaskHandler(BaseHTTPRequestHandler): """Update Todo and its children.""" # pylint: disable=too-many-locals # pylint: disable=too-many-branches - adopted_child_ids = self._form_data.get_all_int('adopt') - to_make = {'full': self._form_data.get_all_int('make_full'), - 'empty': self._form_data.get_all_int('make_empty')} - step_fillers = self._form_data.get_all_str('step_filler') - to_update = { - 'is_done': self._form_data.get_bool('done'), - 'calendarize': self._form_data.get_bool('calendarize'), - 'comment': self._form_data.get_str('comment', ignore_strict=True)} - cond_rels = [self._form_data.get_all_int(name) for name in + adopted_child_ids = self._form.get_all_int('adopt') + to_make = {'full': self._form.get_all_int('make_full'), + 'empty': self._form.get_all_int('make_empty')} + step_fillers = self._form.get_all_str('step_filler') + to_update: dict[str, Any] = { + 'is_done': self._form.get_bool('done'), + 'calendarize': self._form.get_bool('calendarize'), + 'comment': self._form.get_str_or_fail('comment', '')} + cond_rels = [self._form.get_all_int(name) for name in ['conditions', 'blockers', 'enables', 'disables']] - try: - to_update['effort'] = self._form_data.get_float_or_none('effort') - except NotFoundException: - pass + effort_or_not = self._form.get_str('effort') + if effort_or_not is not None: + if effort_or_not == '': + to_update['effort'] = None + else: + try: + to_update['effort'] = float(effort_or_not) + except ValueError as e: + msg = 'cannot float form field value for key: effort' + raise BadFormatException(msg) from e todo.set_condition_relations(self.conn, *cond_rels) for filler in [f for f in step_fillers if f != 'ignore']: target_id: int @@ -707,23 +700,20 @@ class TaskHandler(BaseHTTPRequestHandler): def do_POST_process(self, process: Process) -> str: """Update or insert Process of ?id= and fields defined in postvars.""" # pylint: disable=too-many-locals - try: - versioned = {'title': self._form_data.get_str('title'), - 'description': self._form_data.get_str('description'), - 'effort': self._form_data.get_float('effort')} - except NotFoundException as e: - raise BadFormatException from e - cond_rels = [self._form_data.get_all_int(s) for s + versioned = {'title': self._form.get_str_or_fail('title'), + 'description': self._form.get_str_or_fail('description'), + 'effort': self._form.get_float_or_fail('effort')} + cond_rels = [self._form.get_all_int(s) for s in ['conditions', 'blockers', 'enables', 'disables']] - calendarize = self._form_data.get_bool('calendarize') - step_of = self._form_data.get_all_str('step_of') - suppresses = self._form_data.get_all_int('suppresses') - kept_steps = self._form_data.get_all_int('kept_steps') - new_top_steps = self._form_data.get_all_str('new_top_step') + calendarize = self._form.get_bool('calendarize') + step_of = self._form.get_all_str('step_of') + suppresses = self._form.get_all_int('suppresses') + kept_steps = self._form.get_all_int('kept_steps') + new_top_steps = self._form.get_all_str('new_top_step') new_steps_to = {} for step_id in kept_steps: name = f'new_step_to_{step_id}' - new_steps_to[step_id] = self._form_data.get_all_int(name) + new_steps_to[step_id] = self._form.get_all_int(name) for k, v in versioned.items(): getattr(process, k).set(v) process.set_condition_relations(self.conn, *cond_rels) @@ -785,12 +775,9 @@ class TaskHandler(BaseHTTPRequestHandler): @_delete_or_post(Condition, '/conditions') def do_POST_condition(self, condition: Condition) -> str: """Update/insert Condition of ?id= and fields defined in postvars.""" - try: - title = self._form_data.get_str('title') - description = self._form_data.get_str('description') - except NotFoundException as e: - raise BadFormatException(e) from e - condition.is_active = self._form_data.get_bool('is_active') + title = self._form.get_str_or_fail('title') + description = self._form.get_str_or_fail('description') + condition.is_active = self._form.get_bool('is_active') condition.title.set(title) condition.description.set(description) condition.save(self.conn) diff --git a/tests/conditions.py b/tests/conditions.py index 333267f..cc208fb 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -79,7 +79,6 @@ class TestsWithServer(TestCaseWithServer): url = '/condition' self.check_post({}, url, 400) self.check_post({'title': ''}, url, 400) - self.check_post({'title': '', 'description': ''}, url, 400) self.check_post({'title': '', 'is_active': False}, url, 400) self.check_post({'description': '', 'is_active': False}, url, 400) # check valid POST payload on bad paths diff --git a/tests/misc.py b/tests/misc.py index ce5f87a..8ac82f5 100644 --- a/tests/misc.py +++ b/tests/misc.py @@ -2,208 +2,158 @@ from unittest import TestCase from tests.utils import TestCaseWithServer from plomtask.http import InputsParser -from plomtask.exceptions import BadFormatException, NotFoundException +from plomtask.exceptions import BadFormatException class TestsSansServer(TestCase): """Tests that do not require DB setup or a server.""" + def test_InputsParser_get_str_or_fail(self) -> None: + """Test InputsParser.get_str.""" + parser = InputsParser({}) + with self.assertRaises(BadFormatException): + parser.get_str_or_fail('foo') + self.assertEqual('bar', parser.get_str_or_fail('foo', 'bar')) + parser = InputsParser({'foo': []}) + with self.assertRaises(BadFormatException): + parser.get_str_or_fail('foo') + self.assertEqual('bar', parser.get_str_or_fail('foo', 'bar')) + parser = InputsParser({'foo': ['baz']}) + self.assertEqual('baz', parser.get_str_or_fail('foo', 'bar')) + parser = InputsParser({'foo': ['baz', 'quux']}) + self.assertEqual('baz', parser.get_str_or_fail('foo', 'bar')) + def test_InputsParser_get_str(self) -> None: - """Test InputsParser.get_str on strict and non-strictk.""" - parser = InputsParser({}, False) - self.assertEqual('', parser.get_str('foo')) + """Test InputsParser.get_str.""" + parser = InputsParser({}) + self.assertEqual(None, parser.get_str('foo')) self.assertEqual('bar', parser.get_str('foo', 'bar')) - parser.strict = True - with self.assertRaises(NotFoundException): - parser.get_str('foo') - with self.assertRaises(NotFoundException): - parser.get_str('foo', 'bar') - parser = InputsParser({'foo': []}, False) + parser = InputsParser({'foo': []}) + self.assertEqual(None, parser.get_str('foo')) self.assertEqual('bar', parser.get_str('foo', 'bar')) - with self.assertRaises(NotFoundException): - InputsParser({'foo': []}, True).get_str('foo', 'bar') - for strictness in (False, True): - parser = InputsParser({'foo': ['baz']}, strictness) - self.assertEqual('baz', parser.get_str('foo', 'bar')) - parser = InputsParser({'foo': ['baz', 'quux']}, strictness) - self.assertEqual('baz', parser.get_str('foo', 'bar')) - - def test_InputsParser_get_first_strings_starting(self) -> None: - """Test InputsParser.get_first_strings_starting [non-]strict.""" - for strictness in (False, True): - parser = InputsParser({}, strictness) - self.assertEqual({}, - parser.get_first_strings_starting('')) - parser = InputsParser({}, strictness) - self.assertEqual({}, - parser.get_first_strings_starting('foo')) - parser = InputsParser({'foo': ['bar']}, strictness) - self.assertEqual({'foo': 'bar'}, - parser.get_first_strings_starting('')) - parser = InputsParser({'x': ['y']}, strictness) - self.assertEqual({'x': 'y'}, - parser.get_first_strings_starting('x')) - parser = InputsParser({'xx': ['y']}, strictness) - self.assertEqual({'xx': 'y'}, - parser.get_first_strings_starting('x')) - parser = InputsParser({'xx': ['y']}, strictness) - self.assertEqual({}, - parser.get_first_strings_starting('xxx')) - d = {'xxx': ['x'], 'xxy': ['y'], 'xyy': ['z']} - parser = InputsParser(d, strictness) - self.assertEqual({'xxx': 'x', 'xxy': 'y'}, - parser.get_first_strings_starting('xx')) - d = {'xxx': ['x', 'y', 'z'], 'xxy': ['y', 'z']} - parser = InputsParser(d, strictness) - self.assertEqual({'xxx': 'x', 'xxy': 'y'}, - parser.get_first_strings_starting('xx')) + parser = InputsParser({'foo': ['baz']}) + self.assertEqual('baz', parser.get_str('foo', 'bar')) + parser = InputsParser({'foo': ['baz', 'quux']}) + self.assertEqual('baz', parser.get_str('foo', 'bar')) - def test_InputsParser_get_int(self) -> None: - """Test InputsParser.get_int on strict and non-strict.""" - for strictness in (False, True): - with self.assertRaises(BadFormatException): - InputsParser({}, strictness).get_int('foo') - with self.assertRaises(BadFormatException): - InputsParser({'foo': []}, strictness).get_int('foo') - with self.assertRaises(BadFormatException): - InputsParser({'foo': ['']}, strictness).get_int('foo') - with self.assertRaises(BadFormatException): - InputsParser({'foo': ['bar']}, strictness).get_int('foo') - with self.assertRaises(BadFormatException): - InputsParser({'foo': ['0.1']}).get_int('foo') - parser = InputsParser({'foo': ['0']}, strictness) - self.assertEqual(0, parser.get_int('foo')) - parser = InputsParser({'foo': ['17', '23']}, strictness) - self.assertEqual(17, parser.get_int('foo')) + def test_InputsParser_get_firsts_of_key_prefixed(self) -> None: + """Test InputsParser.get_firsts_of_key_prefixed.""" + parser = InputsParser({}) + self.assertEqual({}, + parser.get_firsts_of_key_prefixed('')) + self.assertEqual({}, + parser.get_firsts_of_key_prefixed('foo')) + parser = InputsParser({'foo': ['bar']}) + self.assertEqual({'foo': 'bar'}, + parser.get_firsts_of_key_prefixed('')) + parser = InputsParser({'x': ['y']}) + self.assertEqual({'x': 'y'}, + parser.get_firsts_of_key_prefixed('x')) + parser = InputsParser({'xx': ['y']}) + self.assertEqual({'xx': 'y'}, + parser.get_firsts_of_key_prefixed('x')) + parser = InputsParser({'xx': ['y']}) + self.assertEqual({}, + parser.get_firsts_of_key_prefixed('xxx')) + parser = InputsParser({'xxx': ['x'], 'xxy': ['y'], 'xyy': ['z']}) + self.assertEqual({'xxx': 'x', 'xxy': 'y'}, + parser.get_firsts_of_key_prefixed('xx')) + parser = InputsParser({'xxx': ['x', 'y', 'z'], 'xxy': ['y', 'z']}) + self.assertEqual({'xxx': 'x', 'xxy': 'y'}, + parser.get_firsts_of_key_prefixed('xx')) def test_InputsParser_get_int_or_none(self) -> None: - """Test InputsParser.get_int_or_none on strict and non-strict.""" - for strictness in (False, True): - parser = InputsParser({}, strictness) - self.assertEqual(None, parser.get_int_or_none('foo')) - parser = InputsParser({'foo': []}, strictness) - self.assertEqual(None, parser.get_int_or_none('foo')) - parser = InputsParser({'foo': ['']}, strictness) - self.assertEqual(None, parser.get_int_or_none('foo')) - parser = InputsParser({'foo': ['0']}, strictness) - self.assertEqual(0, parser.get_int_or_none('foo')) - with self.assertRaises(BadFormatException): - InputsParser({'foo': ['None']}, - strictness).get_int_or_none('foo') - with self.assertRaises(BadFormatException): - InputsParser({'foo': ['0.1']}, - strictness).get_int_or_none('foo') - parser = InputsParser({'foo': ['23']}, strictness) - self.assertEqual(23, parser.get_int_or_none('foo')) - - def test_InputsParser_get_float(self) -> None: - """Test InputsParser.get_float on strict and non-strict.""" - for strictness in (False, True): - with self.assertRaises(BadFormatException): - InputsParser({'foo': ['']}, strictness).get_float('foo') - with self.assertRaises(BadFormatException): - InputsParser({'foo': ['bar']}, strictness).get_float('foo') - parser = InputsParser({'foo': ['0']}, strictness) - self.assertEqual(0, parser.get_float('foo')) - parser = InputsParser({'foo': ['0.1']}, strictness) - self.assertEqual(0.1, parser.get_float('foo')) - parser = InputsParser({'foo': ['1.23', '456']}, strictness) - self.assertEqual(1.23, parser.get_float('foo')) - if strictness: - with self.assertRaises(NotFoundException): - InputsParser({}, strictness).get_float('foo') - with self.assertRaises(NotFoundException): - InputsParser({'foo': []}, strictness).get_float('foo') - else: - with self.assertRaises(BadFormatException): - InputsParser({}, strictness).get_float('foo') - with self.assertRaises(BadFormatException): - InputsParser({'foo': []}, strictness).get_float('foo') + """Test InputsParser.get_int_or_none.""" + parser = InputsParser({}) + self.assertEqual(None, parser.get_int_or_none('foo')) + parser = InputsParser({'foo': []}) + self.assertEqual(None, parser.get_int_or_none('foo')) + parser = InputsParser({'foo': ['']}) + self.assertEqual(None, parser.get_int_or_none('foo')) + parser = InputsParser({'foo': ['0']}) + self.assertEqual(0, parser.get_int_or_none('foo')) + with self.assertRaises(BadFormatException): + InputsParser({'foo': ['None']}).get_int_or_none('foo') + with self.assertRaises(BadFormatException): + InputsParser({'foo': ['0.1']}).get_int_or_none('foo') + parser = InputsParser({'foo': ['23']}) + self.assertEqual(23, parser.get_int_or_none('foo')) - def test_InputsParser_get_float_or_none(self) -> None: - """Test InputsParser.get_float_or_none on strict and non-strict.""" - for strictness in (False, True): - with self.assertRaises(BadFormatException): - InputsParser({'foo': ['bar']}, strictness).\ - get_float_or_none('foo') - parser = InputsParser({'foo': ['']}, strictness) - self.assertEqual(None, parser.get_float_or_none('foo')) - parser = InputsParser({'foo': ['0']}, strictness) - self.assertEqual(0, parser.get_float_or_none('foo')) - parser = InputsParser({'foo': ['0.1']}, strictness) - self.assertEqual(0.1, parser.get_float_or_none('foo')) - parser = InputsParser({'foo': ['1.23', '456']}, strictness) - self.assertEqual(1.23, parser.get_float_or_none('foo')) - if strictness: - with self.assertRaises(NotFoundException): - InputsParser({}, strictness).get_float_or_none('foo') - with self.assertRaises(NotFoundException): - InputsParser({'foo': []}, strictness).get_float_or_none('foo') - else: - parser = InputsParser({}, strictness) - self.assertEqual(None, parser.get_float_or_none('foo')) - parser = InputsParser({'foo': []}, strictness) - self.assertEqual(None, parser.get_float_or_none('foo')) + def test_InputsParser_get_float_or_fail(self) -> None: + """Test InputsParser.get_float_or_fail.""" + with self.assertRaises(BadFormatException): + InputsParser({}).get_float_or_fail('foo') + with self.assertRaises(BadFormatException): + InputsParser({'foo': ['']}).get_float_or_fail('foo') + with self.assertRaises(BadFormatException): + InputsParser({'foo': ['bar']}).get_float_or_fail('foo') + parser = InputsParser({'foo': ['0']}) + self.assertEqual(0, parser.get_float_or_fail('foo')) + parser = InputsParser({'foo': ['0.1']}) + self.assertEqual(0.1, parser.get_float_or_fail('foo')) + parser = InputsParser({'foo': ['1.23', '456']}) + self.assertEqual(1.23, parser.get_float_or_fail('foo')) + with self.assertRaises(BadFormatException): + InputsParser({}).get_float_or_fail('foo') + with self.assertRaises(BadFormatException): + InputsParser({'foo': []}).get_float_or_fail('foo') def test_InputsParser_get_bool(self) -> None: - """Test InputsParser.get_all_str on strict and non-strict.""" - for strictness in (False, True): - parser = InputsParser({}, strictness) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'val': ['true']}, strictness) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'val': ['True']}, strictness) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'val': ['1']}, strictness) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'val': ['foo']}, strictness) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'foo': []}, strictness) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'foo': ['None']}, strictness) - self.assertEqual(True, parser.get_bool('foo')) - parser = InputsParser({'foo': ['0']}, strictness) - self.assertEqual(True, parser.get_bool('foo')) - parser = InputsParser({'foo': ['']}, strictness) - self.assertEqual(True, parser.get_bool('foo')) - parser = InputsParser({'foo': ['bar']}, strictness) - self.assertEqual(True, parser.get_bool('foo')) - parser = InputsParser({'foo': ['bar', 'baz']}, strictness) - self.assertEqual(True, parser.get_bool('foo')) - parser = InputsParser({'foo': ['False']}, strictness) - self.assertEqual(True, parser.get_bool('foo')) + """Test InputsParser.get_all_str.""" + parser = InputsParser({}) + self.assertEqual(False, parser.get_bool('foo')) + parser = InputsParser({'val': ['true']}) + self.assertEqual(False, parser.get_bool('foo')) + parser = InputsParser({'val': ['True']}) + self.assertEqual(False, parser.get_bool('foo')) + parser = InputsParser({'val': ['1']}) + self.assertEqual(False, parser.get_bool('foo')) + parser = InputsParser({'val': ['foo']}) + self.assertEqual(False, parser.get_bool('foo')) + parser = InputsParser({'foo': []}) + self.assertEqual(False, parser.get_bool('foo')) + parser = InputsParser({'foo': ['None']}) + self.assertEqual(True, parser.get_bool('foo')) + parser = InputsParser({'foo': ['0']}) + self.assertEqual(True, parser.get_bool('foo')) + parser = InputsParser({'foo': ['']}) + self.assertEqual(True, parser.get_bool('foo')) + parser = InputsParser({'foo': ['bar']}) + self.assertEqual(True, parser.get_bool('foo')) + parser = InputsParser({'foo': ['bar', 'baz']}) + self.assertEqual(True, parser.get_bool('foo')) + parser = InputsParser({'foo': ['False']}) + self.assertEqual(False, parser.get_bool('foo')) def test_InputsParser_get_all_str(self) -> None: - """Test InputsParser.get_all_str on strict and non-strict.""" - for strictness in (False, True): - parser = InputsParser({}, strictness) - self.assertEqual([], parser.get_all_str('foo')) - parser = InputsParser({'foo': []}, strictness) - self.assertEqual([], parser.get_all_str('foo')) - parser = InputsParser({'foo': ['bar']}, strictness) - self.assertEqual(['bar'], parser.get_all_str('foo')) - parser = InputsParser({'foo': ['bar', 'baz']}, strictness) - self.assertEqual(['bar', 'baz'], parser.get_all_str('foo')) + """Test InputsParser.get_all_str.""" + parser = InputsParser({}) + self.assertEqual([], parser.get_all_str('foo')) + parser = InputsParser({'foo': []}) + self.assertEqual([], parser.get_all_str('foo')) + parser = InputsParser({'foo': ['bar']}) + self.assertEqual(['bar'], parser.get_all_str('foo')) + parser = InputsParser({'foo': ['bar', 'baz']}) + self.assertEqual(['bar', 'baz'], parser.get_all_str('foo')) def test_InputsParser_strict_get_all_int(self) -> None: - """Test InputsParser.get_all_int on strict and non-strict.""" - for strictness in (False, True): - parser = InputsParser({}, strictness) - self.assertEqual([], parser.get_all_int('foo')) - parser = InputsParser({'foo': []}, strictness) - self.assertEqual([], parser.get_all_int('foo')) - parser = InputsParser({'foo': ['']}, strictness) - self.assertEqual([], parser.get_all_int('foo')) - parser = InputsParser({'foo': ['0']}, strictness) - self.assertEqual([0], parser.get_all_int('foo')) - parser = InputsParser({'foo': ['0', '17']}, strictness) - self.assertEqual([0, 17], parser.get_all_int('foo')) - parser = InputsParser({'foo': ['0.1', '17']}, strictness) - with self.assertRaises(BadFormatException): - parser.get_all_int('foo') - parser = InputsParser({'foo': ['None', '17']}, strictness) - with self.assertRaises(BadFormatException): - parser.get_all_int('foo') + """Test InputsParser.get_all_int.""" + parser = InputsParser({}) + self.assertEqual([], parser.get_all_int('foo')) + parser = InputsParser({'foo': []}) + self.assertEqual([], parser.get_all_int('foo')) + parser = InputsParser({'foo': ['']}) + self.assertEqual([], parser.get_all_int('foo')) + parser = InputsParser({'foo': ['0']}) + self.assertEqual([0], parser.get_all_int('foo')) + parser = InputsParser({'foo': ['0', '17']}) + self.assertEqual([0, 17], parser.get_all_int('foo')) + parser = InputsParser({'foo': ['0.1', '17']}) + with self.assertRaises(BadFormatException): + parser.get_all_int('foo') + parser = InputsParser({'foo': ['None', '17']}) + with self.assertRaises(BadFormatException): + parser.get_all_int('foo') class TestsWithServer(TestCaseWithServer): -- 2.30.2 From 7e7288310fe7490a4bf976f46b1b22b8ff06e290 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Thu, 8 Aug 2024 09:01:49 +0200 Subject: [PATCH 07/16] Treat non-sending of boolean form fields as _not_ setting them, rather than setting them negative. --- plomtask/http.py | 24 ++++++++++++------ tests/conditions.py | 20 +++++++-------- tests/days.py | 20 +++++++-------- tests/misc.py | 36 +++++++++++++++------------ tests/todos.py | 59 ++++++++++++++++++++++++--------------------- tests/utils.py | 16 +++++++----- 6 files changed, 97 insertions(+), 78 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 25e377d..77438dc 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -81,9 +81,12 @@ class InputsParser: msg = f'cannot int form field value for key {key}: {val}' raise BadFormatException(msg) from e - def get_bool(self, key: str) -> bool: - """Return if key occurs _and_ its value maps to a boolean Truth.""" - return self.get_str(key) not in {None, 'False'} + def get_bool_or_none(self, key: str) -> bool | None: + """Return value to key if truish; if no value to key, None.""" + val = self.get_str(key) + if val is None: + return None + return val in {'True', 'true', '1', 'on'} def get_firsts_of_key_prefixed(self, prefix: str) -> dict[str, str]: """Retrieve dict of (first) strings at key starting with prefix.""" @@ -627,9 +630,11 @@ class TaskHandler(BaseHTTPRequestHandler): 'empty': self._form.get_all_int('make_empty')} step_fillers = self._form.get_all_str('step_filler') to_update: dict[str, Any] = { - 'is_done': self._form.get_bool('done'), - 'calendarize': self._form.get_bool('calendarize'), 'comment': self._form.get_str_or_fail('comment', '')} + for k1, k2 in [('is_done', 'done'), ('calendarize', 'calendarize')]: + v = self._form.get_bool_or_none(k2) + if v is not None: + to_update[k1] = v cond_rels = [self._form.get_all_int(name) for name in ['conditions', 'blockers', 'enables', 'disables']] effort_or_not = self._form.get_str('effort') @@ -705,7 +710,7 @@ class TaskHandler(BaseHTTPRequestHandler): 'effort': self._form.get_float_or_fail('effort')} cond_rels = [self._form.get_all_int(s) for s in ['conditions', 'blockers', 'enables', 'disables']] - calendarize = self._form.get_bool('calendarize') + calendarize = self._form.get_bool_or_none('calendarize') step_of = self._form.get_all_str('step_of') suppresses = self._form.get_all_int('suppresses') kept_steps = self._form.get_all_int('kept_steps') @@ -717,7 +722,8 @@ class TaskHandler(BaseHTTPRequestHandler): for k, v in versioned.items(): getattr(process, k).set(v) process.set_condition_relations(self.conn, *cond_rels) - process.calendarize = calendarize + if calendarize is not None: + process.calendarize = calendarize process.save(self.conn) assert isinstance(process.id_, int) # set relations to, and if non-existant yet: create, other Processes @@ -777,7 +783,9 @@ class TaskHandler(BaseHTTPRequestHandler): """Update/insert Condition of ?id= and fields defined in postvars.""" title = self._form.get_str_or_fail('title') description = self._form.get_str_or_fail('description') - condition.is_active = self._form.get_bool('is_active') + is_active = self._form.get_bool_or_none('is_active') + if is_active is not None: + condition.is_active = is_active condition.title.set(title) condition.description.set(description) condition.save(self.conn) diff --git a/tests/conditions.py b/tests/conditions.py index cc208fb..f6de4f8 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -16,7 +16,7 @@ class TestsSansDB(TestCaseSansDB): class TestsWithDB(TestCaseWithDB): """Tests requiring DB, but not server setup.""" checked_class = Condition - default_init_kwargs = {'is_active': False} + default_init_kwargs = {'is_active': 0} def test_remove(self) -> None: """Test .remove() effects on DB and cache.""" @@ -79,10 +79,10 @@ class TestsWithServer(TestCaseWithServer): url = '/condition' self.check_post({}, url, 400) self.check_post({'title': ''}, url, 400) - self.check_post({'title': '', 'is_active': False}, url, 400) - self.check_post({'description': '', 'is_active': False}, url, 400) + self.check_post({'title': '', 'is_active': 0}, url, 400) + self.check_post({'description': '', 'is_active': 0}, url, 400) # check valid POST payload on bad paths - valid_payload = {'title': '', 'description': '', 'is_active': False} + valid_payload = {'title': '', 'description': '', 'is_active': 0} self.check_post(valid_payload, '/condition?id=foo', 400) def test_POST_condition(self) -> None: @@ -91,7 +91,7 @@ class TestsWithServer(TestCaseWithServer): exp_all = ExpectedGetConditions() all_exps = [exp_single, exp_all] # test valid POST's effect on single /condition and full /conditions - post = {'title': 'foo', 'description': 'oof', 'is_active': False} + post = {'title': 'foo', 'description': 'oof', 'is_active': 0} self.post_exp_cond(all_exps, 1, post, '', '?id=1') self.check_json_get('/condition?id=1', exp_single) self.check_json_get('/conditions', exp_all) @@ -99,7 +99,7 @@ class TestsWithServer(TestCaseWithServer): self.check_post({}, '/condition?id=1', 400) self.check_json_get('/condition?id=1', exp_single) # test effect of POST changing title and activeness - post = {'title': 'bar', 'description': 'oof', 'is_active': True} + post = {'title': 'bar', 'description': 'oof', 'is_active': 1} self.post_exp_cond(all_exps, 1, post, '?id=1', '?id=1') self.check_json_get('/condition?id=1', exp_single) self.check_json_get('/conditions', exp_all) @@ -116,7 +116,7 @@ class TestsWithServer(TestCaseWithServer): # make Condition and two Processes that among them establish all # possible ConditionsRelations to it, check /condition displays all exp = ExpectedGetCondition(1) - cond_post = {'title': 'foo', 'description': 'oof', 'is_active': False} + cond_post = {'title': 'foo', 'description': 'oof', 'is_active': 0} self.post_exp_cond([exp], 1, cond_post, '', '?id=1') proc1_post = {'title': 'A', 'description': '', 'effort': 1.1, 'conditions': [1], 'disables': [1]} @@ -138,9 +138,9 @@ class TestsWithServer(TestCaseWithServer): exp.set('sort_by', 'title') # for clarity (already default) self.check_json_get('/conditions?sort_by=foo&pattern=bar&foo=x', exp) # test non-empty result, automatic (positive) sorting by title - post_cond1 = {'is_active': False, 'title': 'foo', 'description': 'oof'} - post_cond2 = {'is_active': False, 'title': 'bar', 'description': 'rab'} - post_cond3 = {'is_active': True, 'title': 'baz', 'description': 'zab'} + post_cond1 = {'is_active': 0, 'title': 'foo', 'description': 'oof'} + post_cond2 = {'is_active': 0, 'title': 'bar', 'description': 'rab'} + post_cond3 = {'is_active': 1, 'title': 'baz', 'description': 'zab'} for i, post in enumerate([post_cond1, post_cond2, post_cond3]): self.post_exp_cond([exp], i+1, post, '', f'?id={i+1}') exp.set('pattern', '') diff --git a/tests/days.py b/tests/days.py index 78c5552..0c6ee72 100644 --- a/tests/days.py +++ b/tests/days.py @@ -136,7 +136,7 @@ class ExpectedGetDay(Expected): if t['date'] == self._fields['day']] self.lib_get('Day', self._fields['day'])['todos'] = self.as_ids(todos) self._fields['top_nodes'] = [ - {'children': [], 'seen': False, 'todo': todo['id']} + {'children': [], 'seen': 0, 'todo': todo['id']} for todo in todos] for todo in todos: proc = self.lib_get('Process', todo['process_id']) @@ -308,9 +308,9 @@ class TestsWithServer(TestCaseWithServer): exp.lib_get('Todo', 1)['children'] = [2] exp.lib_set('Todo', [exp.todo_as_dict(2, 2)]) top_nodes = [{'todo': 1, - 'seen': False, + 'seen': 0, 'children': [{'todo': 2, - 'seen': False, + 'seen': 0, 'children': []}]}] exp.force('top_nodes', top_nodes) self.check_json_get(f'/day?date={date}', exp) @@ -318,9 +318,9 @@ class TestsWithServer(TestCaseWithServer): self.post_exp_day([exp], {'make_type': 'full', 'new_todo': [1]}) exp.lib_set('Todo', [exp.todo_as_dict(3, 1, children=[2])]) top_nodes += [{'todo': 3, - 'seen': False, + 'seen': 0, 'children': [{'todo': 2, - 'seen': True, + 'seen': 1, 'children': []}]}] exp.force('top_nodes', top_nodes) self.check_json_get(f'/day?date={date}', exp) @@ -328,7 +328,7 @@ class TestsWithServer(TestCaseWithServer): self.post_exp_day([exp], {'make_type': 'empty', 'new_todo': [1]}) exp.lib_set('Todo', [exp.todo_as_dict(4, 1)]) top_nodes += [{'todo': 4, - 'seen': False, + 'seen': 0, 'children': []}] exp.force('top_nodes', top_nodes) self.check_json_get(f'/day?date={date}', exp) @@ -343,9 +343,9 @@ class TestsWithServer(TestCaseWithServer): # make-full-day-post batch of Todos of both Processes in one order …, self.post_exp_day([exp], {'make_type': 'full', 'new_todo': [1, 2]}) top_nodes: list[dict[str, Any]] = [{'todo': 1, - 'seen': False, + 'seen': 0, 'children': [{'todo': 2, - 'seen': False, + 'seen': 0, 'children': []}]}] exp.force('top_nodes', top_nodes) exp.lib_get('Todo', 1)['children'] = [2] @@ -368,8 +368,8 @@ class TestsWithServer(TestCaseWithServer): date = '2024-01-01' exp = ExpectedGetDay(date) # check non-referenced Conditions not shown - cond_posts = [{'is_active': False, 'title': 'A', 'description': 'a'}, - {'is_active': True, 'title': 'B', 'description': 'b'}] + cond_posts = [{'is_active': 0, 'title': 'A', 'description': 'a'}, + {'is_active': 1, 'title': 'B', 'description': 'b'}] for i, cond_post in enumerate(cond_posts): self.check_post(cond_post, f'/condition?id={i+1}') self.check_json_get(f'/day?date={date}', exp) diff --git a/tests/misc.py b/tests/misc.py index 8ac82f5..1efa335 100644 --- a/tests/misc.py +++ b/tests/misc.py @@ -98,32 +98,36 @@ class TestsSansServer(TestCase): with self.assertRaises(BadFormatException): InputsParser({'foo': []}).get_float_or_fail('foo') - def test_InputsParser_get_bool(self) -> None: + def test_InputsParser_get_bool_or_none(self) -> None: """Test InputsParser.get_all_str.""" parser = InputsParser({}) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'val': ['true']}) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'val': ['True']}) - self.assertEqual(False, parser.get_bool('foo')) - parser = InputsParser({'val': ['1']}) - self.assertEqual(False, parser.get_bool('foo')) + self.assertEqual(None, parser.get_bool_or_none('foo')) parser = InputsParser({'val': ['foo']}) - self.assertEqual(False, parser.get_bool('foo')) + self.assertEqual(None, parser.get_bool_or_none('foo')) + parser = InputsParser({'val': ['True']}) + self.assertEqual(None, parser.get_bool_or_none('foo')) parser = InputsParser({'foo': []}) - self.assertEqual(False, parser.get_bool('foo')) + self.assertEqual(None, parser.get_bool_or_none('foo')) parser = InputsParser({'foo': ['None']}) - self.assertEqual(True, parser.get_bool('foo')) + self.assertEqual(False, parser.get_bool_or_none('foo')) parser = InputsParser({'foo': ['0']}) - self.assertEqual(True, parser.get_bool('foo')) + self.assertEqual(False, parser.get_bool_or_none('foo')) parser = InputsParser({'foo': ['']}) - self.assertEqual(True, parser.get_bool('foo')) + self.assertEqual(False, parser.get_bool_or_none('foo')) parser = InputsParser({'foo': ['bar']}) - self.assertEqual(True, parser.get_bool('foo')) + self.assertEqual(False, parser.get_bool_or_none('foo')) parser = InputsParser({'foo': ['bar', 'baz']}) - self.assertEqual(True, parser.get_bool('foo')) + self.assertEqual(False, parser.get_bool_or_none('foo')) parser = InputsParser({'foo': ['False']}) - self.assertEqual(False, parser.get_bool('foo')) + self.assertEqual(False, parser.get_bool_or_none('foo')) + parser = InputsParser({'foo': ['true']}) + self.assertEqual(True, parser.get_bool_or_none('foo')) + parser = InputsParser({'foo': ['True']}) + self.assertEqual(True, parser.get_bool_or_none('foo')) + parser = InputsParser({'foo': ['1']}) + self.assertEqual(True, parser.get_bool_or_none('foo')) + parser = InputsParser({'foo': ['on']}) + self.assertEqual(True, parser.get_bool_or_none('foo')) def test_InputsParser_get_all_str(self) -> None: """Test InputsParser.get_all_str.""" diff --git a/tests/todos.py b/tests/todos.py index 752e289..07597ed 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -294,30 +294,31 @@ class TestsWithServer(TestCaseWithServer): def test_basic_POST_todo(self) -> None: """Test basic POST /todo manipulations.""" exp = ExpectedGetTodo(1) - self.post_exp_process([exp], {}, 1) + self.post_exp_process([exp], {'calendarize': 0}, 1) self.post_exp_day([exp], {'new_todo': [1]}) # test posting naked entity at first changes nothing self.check_json_get('/todo?id=1', exp) self.check_post({}, '/todo?id=1') self.check_json_get('/todo?id=1', exp) # test posting doneness, comment, calendarization, effort - todo_post = {'done': '', 'calendarize': '', + todo_post = {'done': 1, 'calendarize': 1, 'comment': 'foo', 'effort': 2.3} self._post_exp_todo(1, todo_post, exp) self.check_json_get('/todo?id=1', exp) - # test implicitly un-setting all of those except effort by empty post + # test implicitly un-setting (only) comment by empty post self.check_post({}, '/todo?id=1') - exp.lib_set('Todo', [exp.todo_as_dict(effort=2.3)]) + exp.lib_get('Todo', 1)['comment'] = '' self.check_json_get('/todo?id=1', exp) # test effort post can be explicitly unset by "effort":"" post self.check_post({'effort': ''}, '/todo?id=1') - exp.lib_set('Todo', [exp.todo_as_dict(effort=None)]) + exp.lib_get('Todo', 1)['effort'] = None self.check_json_get('/todo?id=1', exp) # test Condition posts - c1_post = {'title': 'foo', 'description': 'oof', 'is_active': False} - c2_post = {'title': 'bar', 'description': 'rab', 'is_active': True} + c1_post = {'title': 'foo', 'description': 'oof', 'is_active': 0} + c2_post = {'title': 'bar', 'description': 'rab', 'is_active': 1} self.post_exp_cond([exp], 1, c1_post, '?id=1', '?id=1') self.post_exp_cond([exp], 2, c2_post, '?id=2', '?id=2') + self.check_json_get('/todo?id=1', exp) todo_post = {'conditions': [1], 'disables': [1], 'blockers': [2], 'enables': [2]} self._post_exp_todo(1, todo_post, exp) @@ -536,33 +537,35 @@ class TestsWithServer(TestCaseWithServer): # test Todo with adoptee can only be set done if adoptee is done too self.post_exp_day([], {'new_todo': [1]}) self.post_exp_day([], {'new_todo': [1]}) - self.check_post({'adopt': 2, 'done': ''}, '/todo?id=1', 400) - self.check_post({'done': ''}, '/todo?id=2') - self.check_post({'adopt': 2, 'done': ''}, '/todo?id=1', 302) + self.check_post({'adopt': 2, 'done': 1}, '/todo?id=1', 400) + self.check_post({'done': 1}, '/todo?id=2') + self.check_post({'adopt': 2, 'done': 1}, '/todo?id=1', 302) # test Todo cannot be set undone with adopted Todo not done yet - self.check_post({}, '/todo?id=2') - self.check_post({'adopt': 2}, '/todo?id=1', 400) + self.check_post({'done': 0}, '/todo?id=2') + self.check_post({'adopt': 2, 'done': 0}, '/todo?id=1', 400) # test unadoption relieves block - self.check_post({}, '/todo?id=1', 302) + self.check_post({'done': 0}, '/todo?id=1', 302) # test Condition being set or unset can block doneness setting - c1_post = {'title': '', 'description': '', 'is_active': False} - c2_post = {'title': '', 'description': '', 'is_active': True} + c1_post = {'title': '', 'description': '', 'is_active': 0} + c2_post = {'title': '', 'description': '', 'is_active': 1} self.check_post(c1_post, '/condition', redir='/condition?id=1') self.check_post(c2_post, '/condition', redir='/condition?id=2') - self.check_post({'conditions': [1], 'done': ''}, '/todo?id=1', 400) - self.check_post({'done': ''}, '/todo?id=1', 302) - self.check_post({'blockers': [2]}, '/todo?id=1', 400) - self.check_post({'done': ''}, '/todo?id=1', 302) + self.check_post({'conditions': [1], 'done': 1}, '/todo?id=1', 400) + self.check_post({'done': 1}, '/todo?id=1', 302) + self.check_post({'done': 0}, '/todo?id=1', 302) + self.check_post({'blockers': [2], 'done': 1}, '/todo?id=1', 400) + self.check_post({'done': 1}, '/todo?id=1', 302) # test setting Todo doneness can set/un-set Conditions, but only on # doneness change, not by mere passive state - self.check_post({'enables': [1], 'done': ''}, '/todo?id=1') - self.check_post({'conditions': [1], 'done': ''}, '/todo?id=2', 400) - self.check_post({'enables': [1]}, '/todo?id=1') - self.check_post({'enables': [1], 'done': ''}, '/todo?id=1') - self.check_post({'conditions': [1], 'done': ''}, '/todo?id=2') - self.check_post({'blockers': [1]}, '/todo?id=2', 400) - self.check_post({'disables': [1], 'done': ''}, '/todo?id=1') - self.check_post({'blockers': [1]}, '/todo?id=2', 400) + self.check_post({'done': 0}, '/todo?id=2', 302) + self.check_post({'enables': [1], 'done': 1}, '/todo?id=1') + self.check_post({'conditions': [1], 'done': 1}, '/todo?id=2', 400) + self.check_post({'enables': [1], 'done': 0}, '/todo?id=1') + self.check_post({'enables': [1], 'done': 1}, '/todo?id=1') + self.check_post({'conditions': [1], 'done': 1}, '/todo?id=2') + self.check_post({'blockers': [1], 'done': 0}, '/todo?id=2', 400) + self.check_post({'disables': [1], 'done': 1}, '/todo?id=1') + self.check_post({'blockers': [1], 'done': 0}, '/todo?id=2', 400) self.check_post({'disables': [1]}, '/todo?id=1') - self.check_post({'disables': [1], 'done': ''}, '/todo?id=1') + self.check_post({'disables': [1], 'done': 1}, '/todo?id=1') self.check_post({'blockers': [1]}, '/todo?id=2') diff --git a/tests/utils.py b/tests/utils.py index df3feea..acfabe7 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -26,6 +26,7 @@ from plomtask.exceptions import NotFoundException, HandledException VERSIONED_VALS: dict[str, list[str] | list[float]] = {'str': ['A', 'B'], 'float': [0.3, 1.1]} +VALID_TRUES = {True, 'True', 'true', '1', 'on'} class TestCaseAugmented(TestCase): @@ -750,20 +751,23 @@ class Expected: def set_todo_from_post(self, id_: int, d: dict[str, Any]) -> None: """Set Todo of id_ in library based on POST dict d.""" - corrected_kwargs: dict[str, Any] = {} + corrected_kwargs: dict[str, Any] = {'children': []} for k, v in d.items(): if k in {'adopt', 'step_filler'}: - if 'children' not in corrected_kwargs: - corrected_kwargs['children'] = [] new_children = v if isinstance(v, list) else [v] corrected_kwargs['children'] += new_children continue if 'done' == k: k = 'is_done' if k in {'is_done', 'calendarize'}: - v = True + v = v in VALID_TRUES corrected_kwargs[k] = v - todo = self.todo_as_dict(id_, **corrected_kwargs) + todo = self.lib_get('Todo', id_) + if todo: + for k, v in corrected_kwargs.items(): + todo[k] = v + else: + todo = self.todo_as_dict(id_, **corrected_kwargs) self.lib_set('Todo', [todo]) @staticmethod @@ -832,7 +836,7 @@ class Expected: or k.startswith('step_') or k.startswith('new_step_to'): continue if k in {'calendarize'}: - v = True + v = v in VALID_TRUES elif k in {'suppressed_steps', 'explicit_steps', 'conditions', 'disables', 'enables', 'blockers'}: if not isinstance(v, list): -- 2.30.2 From c532d3b553e3881b241aa0885b14525224332c41 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Thu, 8 Aug 2024 09:52:53 +0200 Subject: [PATCH 08/16] Rename POST form key to field manipulated. --- plomtask/http.py | 6 +++--- templates/todo.html | 4 ++-- tests/conditions.py | 2 +- tests/todos.py | 44 ++++++++++++++++++++++---------------------- tests/utils.py | 2 -- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 77438dc..c7897e8 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -631,10 +631,10 @@ class TaskHandler(BaseHTTPRequestHandler): step_fillers = self._form.get_all_str('step_filler') to_update: dict[str, Any] = { 'comment': self._form.get_str_or_fail('comment', '')} - for k1, k2 in [('is_done', 'done'), ('calendarize', 'calendarize')]: - v = self._form.get_bool_or_none(k2) + for k in ('is_done', 'calendarize'): + v = self._form.get_bool_or_none(k) if v is not None: - to_update[k1] = v + to_update[k] = v cond_rels = [self._form.get_all_int(name) for name in ['conditions', 'blockers', 'enables', 'disables']] effort_or_not = self._form.get_str('effort') diff --git a/templates/todo.html b/templates/todo.html index 732f788..20279bb 100644 --- a/templates/todo.html +++ b/templates/todo.html @@ -57,8 +57,8 @@ select{ font-size: 0.5em; margin: 0; padding: 0; } done - -{% if not todo.is_doable and todo.is_done %}{% endif %} + +{% if not todo.is_doable and todo.is_done %}{% endif %} diff --git a/tests/conditions.py b/tests/conditions.py index f6de4f8..3edafd6 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -134,7 +134,7 @@ class TestsWithServer(TestCaseWithServer): # test ignorance of meaningless non-empty params (incl. unknown key), # that 'sort_by' default to 'title' (even if set to something else, as # long as without handler) and 'pattern' get preserved - exp.set('pattern', 'bar') # preserved despite zero effect! + exp.set('pattern', 'bar') exp.set('sort_by', 'title') # for clarity (already default) self.check_json_get('/conditions?sort_by=foo&pattern=bar&foo=x', exp) # test non-empty result, automatic (positive) sorting by title diff --git a/tests/todos.py b/tests/todos.py index 07597ed..d84bb70 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -301,7 +301,7 @@ class TestsWithServer(TestCaseWithServer): self.check_post({}, '/todo?id=1') self.check_json_get('/todo?id=1', exp) # test posting doneness, comment, calendarization, effort - todo_post = {'done': 1, 'calendarize': 1, + todo_post = {'is_done': 1, 'calendarize': 1, 'comment': 'foo', 'effort': 2.3} self._post_exp_todo(1, todo_post, exp) self.check_json_get('/todo?id=1', exp) @@ -537,35 +537,35 @@ class TestsWithServer(TestCaseWithServer): # test Todo with adoptee can only be set done if adoptee is done too self.post_exp_day([], {'new_todo': [1]}) self.post_exp_day([], {'new_todo': [1]}) - self.check_post({'adopt': 2, 'done': 1}, '/todo?id=1', 400) - self.check_post({'done': 1}, '/todo?id=2') - self.check_post({'adopt': 2, 'done': 1}, '/todo?id=1', 302) + self.check_post({'adopt': 2, 'is_done': 1}, '/todo?id=1', 400) + self.check_post({'is_done': 1}, '/todo?id=2') + self.check_post({'adopt': 2, 'is_done': 1}, '/todo?id=1', 302) # test Todo cannot be set undone with adopted Todo not done yet - self.check_post({'done': 0}, '/todo?id=2') - self.check_post({'adopt': 2, 'done': 0}, '/todo?id=1', 400) + self.check_post({'is_done': 0}, '/todo?id=2') + self.check_post({'adopt': 2, 'is_done': 0}, '/todo?id=1', 400) # test unadoption relieves block - self.check_post({'done': 0}, '/todo?id=1', 302) + self.check_post({'is_done': 0}, '/todo?id=1', 302) # test Condition being set or unset can block doneness setting c1_post = {'title': '', 'description': '', 'is_active': 0} c2_post = {'title': '', 'description': '', 'is_active': 1} self.check_post(c1_post, '/condition', redir='/condition?id=1') self.check_post(c2_post, '/condition', redir='/condition?id=2') - self.check_post({'conditions': [1], 'done': 1}, '/todo?id=1', 400) - self.check_post({'done': 1}, '/todo?id=1', 302) - self.check_post({'done': 0}, '/todo?id=1', 302) - self.check_post({'blockers': [2], 'done': 1}, '/todo?id=1', 400) - self.check_post({'done': 1}, '/todo?id=1', 302) + self.check_post({'conditions': [1], 'is_done': 1}, '/todo?id=1', 400) + self.check_post({'is_done': 1}, '/todo?id=1', 302) + self.check_post({'is_done': 0}, '/todo?id=1', 302) + self.check_post({'blockers': [2], 'is_done': 1}, '/todo?id=1', 400) + self.check_post({'is_done': 1}, '/todo?id=1', 302) # test setting Todo doneness can set/un-set Conditions, but only on # doneness change, not by mere passive state - self.check_post({'done': 0}, '/todo?id=2', 302) - self.check_post({'enables': [1], 'done': 1}, '/todo?id=1') - self.check_post({'conditions': [1], 'done': 1}, '/todo?id=2', 400) - self.check_post({'enables': [1], 'done': 0}, '/todo?id=1') - self.check_post({'enables': [1], 'done': 1}, '/todo?id=1') - self.check_post({'conditions': [1], 'done': 1}, '/todo?id=2') - self.check_post({'blockers': [1], 'done': 0}, '/todo?id=2', 400) - self.check_post({'disables': [1], 'done': 1}, '/todo?id=1') - self.check_post({'blockers': [1], 'done': 0}, '/todo?id=2', 400) + self.check_post({'is_done': 0}, '/todo?id=2', 302) + self.check_post({'enables': [1], 'is_done': 1}, '/todo?id=1') + self.check_post({'conditions': [1], 'is_done': 1}, '/todo?id=2', 400) + self.check_post({'enables': [1], 'is_done': 0}, '/todo?id=1') + self.check_post({'enables': [1], 'is_done': 1}, '/todo?id=1') + self.check_post({'conditions': [1], 'is_done': 1}, '/todo?id=2') + self.check_post({'blockers': [1], 'is_done': 0}, '/todo?id=2', 400) + self.check_post({'disables': [1], 'is_done': 1}, '/todo?id=1') + self.check_post({'blockers': [1], 'is_done': 0}, '/todo?id=2', 400) self.check_post({'disables': [1]}, '/todo?id=1') - self.check_post({'disables': [1], 'done': 1}, '/todo?id=1') + self.check_post({'disables': [1], 'is_done': 1}, '/todo?id=1') self.check_post({'blockers': [1]}, '/todo?id=2') diff --git a/tests/utils.py b/tests/utils.py index acfabe7..1a306d1 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -757,8 +757,6 @@ class Expected: new_children = v if isinstance(v, list) else [v] corrected_kwargs['children'] += new_children continue - if 'done' == k: - k = 'is_done' if k in {'is_done', 'calendarize'}: v = v in VALID_TRUES corrected_kwargs[k] = v -- 2.30.2 From 051c62713adb93a233713589222efde640fd994d Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Thu, 8 Aug 2024 10:13:42 +0200 Subject: [PATCH 09/16] Minor tests refactoring. --- tests/conditions.py | 26 ++++++++------------------ tests/processes.py | 42 +++++++++++------------------------------- tests/utils.py | 8 ++++++++ 3 files changed, 27 insertions(+), 49 deletions(-) diff --git a/tests/conditions.py b/tests/conditions.py index 3edafd6..6feda94 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -138,33 +138,23 @@ class TestsWithServer(TestCaseWithServer): exp.set('sort_by', 'title') # for clarity (already default) self.check_json_get('/conditions?sort_by=foo&pattern=bar&foo=x', exp) # test non-empty result, automatic (positive) sorting by title + exp.set('pattern', '') post_cond1 = {'is_active': 0, 'title': 'foo', 'description': 'oof'} post_cond2 = {'is_active': 0, 'title': 'bar', 'description': 'rab'} post_cond3 = {'is_active': 1, 'title': 'baz', 'description': 'zab'} for i, post in enumerate([post_cond1, post_cond2, post_cond3]): self.post_exp_cond([exp], i+1, post, '', f'?id={i+1}') - exp.set('pattern', '') - exp.force('conditions', [2, 3, 1]) - self.check_json_get('/conditions', exp) + self.check_filter(exp, 'conditions', 'sort_by', 'title', [2, 3, 1]) # test other sortings - exp.set('sort_by', '-title') - exp.force('conditions', [1, 3, 2]) - self.check_json_get('/conditions?sort_by=-title', exp) - exp.set('sort_by', 'is_active') - exp.force('conditions', [1, 2, 3]) - self.check_json_get('/conditions?sort_by=is_active', exp) - exp.set('sort_by', '-is_active') - exp.force('conditions', [3, 2, 1]) - self.check_json_get('/conditions?sort_by=-is_active', exp) + self.check_filter(exp, 'conditions', 'sort_by', '-title', [1, 3, 2]) + self.check_filter(exp, 'conditions', 'sort_by', 'is_active', [1, 2, 3]) + self.check_filter(exp, 'conditions', 'sort_by', '-is_active', + [3, 2, 1]) # test pattern matching on title exp.set('sort_by', 'title') - exp.set('pattern', 'ba') - exp.force('conditions', [2, 3]) exp.lib_del('Condition', 1) - self.check_json_get('/conditions?pattern=ba', exp) + self.check_filter(exp, 'conditions', 'pattern', 'ba', [2, 3]) # test pattern matching on description - exp.set('pattern', 'of') exp.lib_wipe('Condition') exp.set_cond_from_post(1, post_cond1) - exp.force('conditions', [1]) - self.check_json_get('/conditions?pattern=of', exp) + self.check_filter(exp, 'conditions', 'pattern', 'of', [1]) diff --git a/tests/processes.py b/tests/processes.py index 649aee5..422c283 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -434,7 +434,7 @@ class TestsWithServer(TestCaseWithServer): # test on meaningless non-empty params (incl. entirely un-used key), # that 'sort_by' default to 'title' (even if set to something else, as # long as without handler) and 'pattern' get preserved - exp.set('pattern', 'bar') # preserved despite zero effect! + exp.set('pattern', 'bar') url = '/processes?sort_by=foo&pattern=bar&foo=x' self.check_json_get(url, exp) # test non-empty result, automatic (positive) sorting by title @@ -449,42 +449,22 @@ class TestsWithServer(TestCaseWithServer): exp.lib_set('ProcessStep', [exp.procstep_as_dict(1, 2, 1), exp.procstep_as_dict(2, 3, 1), exp.procstep_as_dict(3, 3, 2)]) - exp.lib_get('Process', '') exp.set('pattern', '') - exp.force('processes', [2, 3, 1]) - self.check_json_get('/processes', exp) + self.check_filter(exp, 'processes', 'sort_by', 'title', [2, 3, 1]) # test other sortings - exp.set('sort_by', '-title') - exp.force('processes', [1, 3, 2]) - self.check_json_get('/processes?sort_by=-title', exp) - exp.set('sort_by', 'effort') - exp.force('processes', [3, 1, 2]) - self.check_json_get('/processes?sort_by=effort', exp) - exp.set('sort_by', '-effort') - exp.force('processes', [2, 1, 3]) - self.check_json_get('/processes?sort_by=-effort', exp) - exp.set('sort_by', 'steps') - exp.force('processes', [1, 2, 3]) - self.check_json_get('/processes?sort_by=steps', exp) - exp.set('sort_by', '-steps') - exp.force('processes', [3, 2, 1]) - self.check_json_get('/processes?sort_by=-steps', exp) - exp.set('sort_by', 'owners') - exp.force('processes', [3, 2, 1]) - self.check_json_get('/processes?sort_by=owners', exp) - exp.set('sort_by', '-owners') - exp.force('processes', [1, 2, 3]) - self.check_json_get('/processes?sort_by=-owners', exp) + self.check_filter(exp, 'processes', 'sort_by', '-title', [1, 3, 2]) + self.check_filter(exp, 'processes', 'sort_by', 'effort', [3, 1, 2]) + self.check_filter(exp, 'processes', 'sort_by', '-effort', [2, 1, 3]) + self.check_filter(exp, 'processes', 'sort_by', 'steps', [1, 2, 3]) + self.check_filter(exp, 'processes', 'sort_by', '-steps', [3, 2, 1]) + self.check_filter(exp, 'processes', 'sort_by', 'owners', [3, 2, 1]) + self.check_filter(exp, 'processes', 'sort_by', '-owners', [1, 2, 3]) # test pattern matching on title - exp.set('pattern', 'ba') exp.set('sort_by', 'title') exp.lib_del('Process', '1') - exp.force('processes', [2, 3]) - self.check_json_get('/processes?pattern=ba', exp) + self.check_filter(exp, 'processes', 'pattern', 'ba', [2, 3]) # test pattern matching on description - exp.set('pattern', 'of') exp.lib_wipe('Process') exp.lib_wipe('ProcessStep') self.post_exp_process([exp], {'description': 'oof', 'effort': 1.0}, 1) - exp.force('processes', [1]) - self.check_json_get('/processes?pattern=of', exp) + self.check_filter(exp, 'processes', 'pattern', 'of', [1]) diff --git a/tests/utils.py b/tests/utils.py index 1a306d1..e04fba6 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -911,6 +911,14 @@ class TestCaseWithServer(TestCaseWithDB): exp.set_proc_from_post(id_, payload) return payload + def check_filter(self, exp: Expected, category: str, key: str, + val: str, list_ids: list[int]) -> None: + """Check GET /{category}?{key}={val} sorts to list_ids.""" + # pylint: disable=too-many-arguments + exp.set(key, val) + exp.force(category, list_ids) + self.check_json_get(f'/{category}?{key}={val}', exp) + def check_redirect(self, target: str) -> None: """Check that self.conn answers with a 302 redirect to target.""" response = self.conn.getresponse() -- 2.30.2 From 1af7e2710a2af8c8e03e5e679921399825a4407a Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 9 Aug 2024 16:14:55 +0200 Subject: [PATCH 10/16] Private TaskHandler.conn to ._conn. --- plomtask/http.py | 114 +++++++++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index c7897e8..ef9438a 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -123,7 +123,7 @@ class TaskHandler(BaseHTTPRequestHandler): """Handles single HTTP request.""" # pylint: disable=too-many-public-methods server: TaskServer - conn: DatabaseConnection + _conn: DatabaseConnection _site: str _form: InputsParser _params: InputsParser @@ -240,7 +240,7 @@ class TaskHandler(BaseHTTPRequestHandler): # (because pylint here fails to detect the use of wrapper as a # method to self with respective access privileges) try: - self.conn = DatabaseConnection(self.server.db) + self._conn = DatabaseConnection(self.server.db) parsed_url = urlparse(self.path) self._site = path_split(parsed_url.path)[1] params = parse_qs(parsed_url.query, @@ -266,7 +266,7 @@ class TaskHandler(BaseHTTPRequestHandler): ctx = {'msg': error} self._send_page(ctx, 'msg', error.http_code) finally: - self.conn.close() + self._conn.close() return wrapper return decorator @@ -289,7 +289,7 @@ class TaskHandler(BaseHTTPRequestHandler): keep_blank_values=True) self._form = InputsParser(postvars) redir_target = handler() - self.conn.commit() + self._conn.commit() return redir_target # GET handlers @@ -306,9 +306,9 @@ class TaskHandler(BaseHTTPRequestHandler): # method to self with respective access privileges) id_ = self._params.get_int_or_none('id') if target_class.can_create_by_id: - item = target_class.by_id_or_create(self.conn, id_) + item = target_class.by_id_or_create(self._conn, id_) else: - item = target_class.by_id(self.conn, id_) + item = target_class.by_id(self._conn, id_) return f(self, item) return wrapper return decorator @@ -327,7 +327,7 @@ class TaskHandler(BaseHTTPRequestHandler): start = self._params.get_str_or_fail('start', '') end = self._params.get_str_or_fail('end', '') end = end if end != '' else date_in_n_days(366) - days, start, end = Day.by_date_range_with_limits(self.conn, + days, start, end = Day.by_date_range_with_limits(self._conn, (start, end), 'id') days = Day.with_filled_gaps(days, start, end) today = date_in_n_days(0) @@ -344,7 +344,7 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_day(self) -> dict[str, object]: """Show single Day of ?date=.""" date = self._params.get_str_or_fail('date', date_in_n_days(0)) - day = Day.by_id_or_create(self.conn, date) + day = Day.by_id_or_create(self._conn, date) make_type = self._params.get_str_or_fail('make_type', '') conditions_present = [] enablers_for = {} @@ -354,10 +354,10 @@ class TaskHandler(BaseHTTPRequestHandler): if condition not in conditions_present: conditions_present += [condition] enablers_for[condition.id_] = [p for p in - Process.all(self.conn) + Process.all(self._conn) if condition in p.enables] disablers_for[condition.id_] = [p for p in - Process.all(self.conn) + Process.all(self._conn) if condition in p.disables] seen_todos: set[int] = set() top_nodes = [t.get_step_tree(seen_todos) @@ -368,7 +368,7 @@ class TaskHandler(BaseHTTPRequestHandler): 'enablers_for': enablers_for, 'disablers_for': disablers_for, 'conditions_present': conditions_present, - 'processes': Process.all(self.conn)} + 'processes': Process.all(self._conn)} @_get_item(Todo) def do_GET_todo(self, todo: Todo) -> dict[str, object]: @@ -379,7 +379,7 @@ class TaskHandler(BaseHTTPRequestHandler): steps_nodes: list[TodoOrProcStepNode]) -> int: for process_step_node in process_step_nodes: node_id += 1 - proc = Process.by_id(self.conn, + proc = Process.by_id(self._conn, process_step_node.step.step_process_id) node = TodoOrProcStepNode(node_id, None, proc, []) steps_nodes += [node] @@ -420,7 +420,7 @@ class TaskHandler(BaseHTTPRequestHandler): return ids todo_steps = [step.todo for step in todo.get_step_tree(set()).children] - process_tree = todo.process.get_steps(self.conn, None) + process_tree = todo.process.get_steps(self._conn, None) steps_todo_to_process: list[TodoOrProcStepNode] = [] last_node_id = walk_process_steps(0, process_tree, steps_todo_to_process) @@ -428,8 +428,8 @@ class TaskHandler(BaseHTTPRequestHandler): steps_node.fillable = True walk_todo_steps(last_node_id, todo_steps, steps_todo_to_process) adoptables: dict[int, list[Todo]] = {} - any_adoptables = [Todo.by_id(self.conn, t.id_) - for t in Todo.by_date(self.conn, todo.date) + any_adoptables = [Todo.by_id(self._conn, t.id_) + for t in Todo.by_date(self._conn, todo.date) if t.id_ is not None and t != todo] for id_ in collect_adoptables_keys(steps_todo_to_process): @@ -438,9 +438,9 @@ class TaskHandler(BaseHTTPRequestHandler): return {'todo': todo, 'steps_todo_to_process': steps_todo_to_process, 'adoption_candidates_for': adoptables, - 'process_candidates': sorted(Process.all(self.conn)), + 'process_candidates': sorted(Process.all(self._conn)), 'todo_candidates': any_adoptables, - 'condition_candidates': Condition.all(self.conn)} + 'condition_candidates': Condition.all(self._conn)} def do_GET_todos(self) -> dict[str, object]: """Show Todos from ?start= to ?end=, of ?process=, ?comment= pattern""" @@ -450,7 +450,7 @@ class TaskHandler(BaseHTTPRequestHandler): process_id = self._params.get_int_or_none('process_id') comment_pattern = self._params.get_str_or_fail('comment_pattern', '') todos = [] - ret = Todo.by_date_range_with_limits(self.conn, (start, end)) + ret = Todo.by_date_range_with_limits(self._conn, (start, end)) todos_by_date_range, start, end = ret todos = [t for t in todos_by_date_range if comment_pattern in t.comment @@ -458,13 +458,13 @@ class TaskHandler(BaseHTTPRequestHandler): sort_by = Todo.sort_by(todos, sort_by) return {'start': start, 'end': end, 'process_id': process_id, 'comment_pattern': comment_pattern, 'todos': todos, - 'all_processes': Process.all(self.conn), 'sort_by': sort_by} + 'all_processes': Process.all(self._conn), 'sort_by': sort_by} def do_GET_conditions(self) -> dict[str, object]: """Show all Conditions.""" pattern = self._params.get_str_or_fail('pattern', '') sort_by = self._params.get_str_or_fail('sort_by', '') - conditions = Condition.matching(self.conn, pattern) + conditions = Condition.matching(self._conn, pattern) sort_by = Condition.sort_by(conditions, sort_by) return {'conditions': conditions, 'sort_by': sort_by, @@ -473,7 +473,7 @@ class TaskHandler(BaseHTTPRequestHandler): @_get_item(Condition) def do_GET_condition(self, c: Condition) -> dict[str, object]: """Show Condition of ?id=.""" - ps = Process.all(self.conn) + ps = Process.all(self._conn) return {'condition': c, 'is_new': c.id_ is None, 'enabled_processes': [p for p in ps if c in p.conditions], 'disabled_processes': [p for p in ps if c in p.blockers], @@ -504,19 +504,19 @@ class TaskHandler(BaseHTTPRequestHandler): raise BadFormatException(msg) from exc process.title.set(title) preset_top_step = None - owners = process.used_as_step_by(self.conn) + owners = process.used_as_step_by(self._conn) for step_id in owner_ids: - owners += [Process.by_id(self.conn, step_id)] + owners += [Process.by_id(self._conn, step_id)] for process_id in owned_ids: - Process.by_id(self.conn, process_id) # to ensure ID exists + Process.by_id(self._conn, process_id) # to ensure ID exists preset_top_step = process_id return {'process': process, 'is_new': process.id_ is None, 'preset_top_step': preset_top_step, - 'steps': process.get_steps(self.conn), + 'steps': process.get_steps(self._conn), 'owners': owners, - 'n_todos': len(Todo.by_process_id(self.conn, process.id_)), - 'process_candidates': Process.all(self.conn), - 'condition_candidates': Condition.all(self.conn)} + 'n_todos': len(Todo.by_process_id(self._conn, process.id_)), + 'process_candidates': Process.all(self._conn), + 'condition_candidates': Condition.all(self._conn)} @_get_item(Process) def do_GET_process_titles(self, p: Process) -> dict[str, object]: @@ -537,7 +537,7 @@ class TaskHandler(BaseHTTPRequestHandler): """Show all Processes.""" pattern = self._params.get_str_or_fail('pattern', '') sort_by = self._params.get_str_or_fail('sort_by', '') - processes = Process.matching(self.conn, pattern) + processes = Process.matching(self._conn, pattern) sort_by = Process.sort_by(processes, sort_by) return {'processes': processes, 'sort_by': sort_by, 'pattern': pattern} @@ -558,13 +558,13 @@ class TaskHandler(BaseHTTPRequestHandler): 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) + 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_) + item = target_class.by_id_or_create(self._conn, id_) else: - item = target_class.by_id(self.conn, id_) + item = target_class.by_id(self._conn, id_) return f(self, item) return wrapper return decorator @@ -572,13 +572,13 @@ class TaskHandler(BaseHTTPRequestHandler): def _change_versioned_timestamps(self, cls: Any, attr_name: str) -> str: """Update history timestamps for VersionedAttribute.""" id_ = self._params.get_int_or_none('id') - item = cls.by_id(self.conn, id_) + item = cls.by_id(self._conn, id_) attr = getattr(item, attr_name) for k, v in self._form.get_firsts_of_key_prefixed('at:').items(): old = k[3:] if old[19:] != v: attr.reset_timestamp(old, f'{v}.0') - attr.save(self.conn) + attr.save(self._conn) return f'/{cls.name_lowercase()}_{attr_name}s?id={item.id_}' def do_POST_day(self) -> str: @@ -600,24 +600,24 @@ class TaskHandler(BaseHTTPRequestHandler): msg = 'not equal number each of number of todo_id, comments, ' +\ 'and efforts inputs' raise BadFormatException(msg) - day = Day.by_id_or_create(self.conn, date) + day = Day.by_id_or_create(self._conn, date) day.comment = day_comment - day.save(self.conn) + day.save(self._conn) new_todos = [] for process_id in sorted(new_todos_by_process): - process = Process.by_id(self.conn, process_id) + process = Process.by_id(self._conn, process_id) todo = Todo(None, process, False, date) - todo.save(self.conn) + todo.save(self._conn) new_todos += [todo] if 'full' == make_type: for todo in new_todos: - todo.ensure_children(self.conn) + todo.ensure_children(self._conn) for i, todo_id in enumerate(old_todos): - todo = Todo.by_id(self.conn, todo_id) + todo = Todo.by_id(self._conn, todo_id) todo.is_done = is_done[i] todo.comment = comments[i] todo.effort = efforts[i] - todo.save(self.conn) + todo.save(self._conn) return f'/day?date={date}&make_type={make_type}' @_delete_or_post(Todo, '/') @@ -647,7 +647,7 @@ class TaskHandler(BaseHTTPRequestHandler): except ValueError as e: msg = 'cannot float form field value for key: effort' raise BadFormatException(msg) from e - todo.set_condition_relations(self.conn, *cond_rels) + todo.set_condition_relations(self._conn, *cond_rels) for filler in [f for f in step_fillers if f != 'ignore']: target_id: int to_int = filler @@ -670,23 +670,23 @@ class TaskHandler(BaseHTTPRequestHandler): if child.id_ and (child.id_ not in adopted_child_ids): to_remove += [child.id_] for id_ in to_remove: - child = Todo.by_id(self.conn, id_) + child = Todo.by_id(self._conn, id_) todo.remove_child(child) for child_id in adopted_child_ids: if child_id not in [c.id_ for c in todo.children]: - todo.add_child(Todo.by_id(self.conn, child_id)) + todo.add_child(Todo.by_id(self._conn, child_id)) todo.update_attrs(**to_update) for approach, proc_ids in to_make.items(): for process_id in proc_ids: - process = Process.by_id(self.conn, process_id) + process = Process.by_id(self._conn, process_id) made = Todo(None, process, False, todo.date) - made.save(self.conn) + made.save(self._conn) if 'full' == approach: - made.ensure_children(self.conn) + made.ensure_children(self._conn) todo.add_child(made) # todo.save() may destroy Todo if .effort < 0, so retrieve .id_ early url = f'/todo?id={todo.id_}' - todo.save(self.conn) + todo.save(self._conn) return url def do_POST_process_descriptions(self) -> str: @@ -721,10 +721,10 @@ class TaskHandler(BaseHTTPRequestHandler): new_steps_to[step_id] = self._form.get_all_int(name) for k, v in versioned.items(): getattr(process, k).set(v) - process.set_condition_relations(self.conn, *cond_rels) + process.set_condition_relations(self._conn, *cond_rels) if calendarize is not None: process.calendarize = calendarize - process.save(self.conn) + process.save(self._conn) assert isinstance(process.id_, int) # set relations to, and if non-existant yet: create, other Processes # pylint: disable=fixme @@ -740,10 +740,10 @@ class TaskHandler(BaseHTTPRequestHandler): owners_to_set += [int(owner_identifier)] except ValueError: new_owner_title = owner_identifier - process.set_owners(self.conn, owners_to_set) + process.set_owners(self._conn, owners_to_set) # 2. owneds (downwards) new_step_title = None - steps: list[ProcessStep] = [ProcessStep.by_id(self.conn, step_id) + steps: list[ProcessStep] = [ProcessStep.by_id(self._conn, step_id) for step_id in kept_steps] for step_id in kept_steps: new_sub_steps = [ @@ -757,8 +757,8 @@ class TaskHandler(BaseHTTPRequestHandler): steps += [step] except ValueError: new_step_title = step_id_or_new_title - process.set_steps(self.conn, steps) - process.set_step_suppressions(self.conn, suppresses) + process.set_steps(self._conn, steps) + process.set_step_suppressions(self._conn, suppresses) # encode titles for potentially newly created Processes up or down params = f'id={process.id_}' if new_step_title: @@ -767,7 +767,7 @@ class TaskHandler(BaseHTTPRequestHandler): elif new_owner_title: title_b64_encoded = b64encode(new_owner_title.encode()).decode() params = f'has_step={process.id_}&title_b64={title_b64_encoded}' - process.save(self.conn) + process.save(self._conn) return f'/process?{params}' def do_POST_condition_descriptions(self) -> str: @@ -788,5 +788,5 @@ class TaskHandler(BaseHTTPRequestHandler): condition.is_active = is_active condition.title.set(title) condition.description.set(description) - condition.save(self.conn) + condition.save(self._conn) return f'/condition?id={condition.id_}' -- 2.30.2 From fa927e56c36522f1148635c28b1133fd370cff80 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 9 Aug 2024 16:51:35 +0200 Subject: [PATCH 11/16] Minor improvements to query parameter handling/defaulting. --- plomtask/http.py | 18 ++++++++++++++---- tests/days.py | 3 ++- tests/utils.py | 10 +++++----- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index ef9438a..c2e1f5d 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -327,6 +327,7 @@ class TaskHandler(BaseHTTPRequestHandler): start = self._params.get_str_or_fail('start', '') end = self._params.get_str_or_fail('end', '') end = end if end != '' else date_in_n_days(366) + # days, start, end = Day.by_date_range_with_limits(self._conn, (start, end), 'id') days = Day.with_filled_gaps(days, start, end) @@ -344,8 +345,9 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_day(self) -> dict[str, object]: """Show single Day of ?date=.""" date = self._params.get_str_or_fail('date', date_in_n_days(0)) + make_type = self._params.get_str_or_fail('make_type', 'full') + # day = Day.by_id_or_create(self._conn, date) - make_type = self._params.get_str_or_fail('make_type', '') conditions_present = [] enablers_for = {} disablers_for = {} @@ -444,11 +446,12 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_todos(self) -> dict[str, object]: """Show Todos from ?start= to ?end=, of ?process=, ?comment= pattern""" - sort_by = self._params.get_str_or_fail('sort_by', '') + sort_by = self._params.get_str_or_fail('sort_by', 'title') start = self._params.get_str_or_fail('start', '') end = self._params.get_str_or_fail('end', '') process_id = self._params.get_int_or_none('process_id') comment_pattern = self._params.get_str_or_fail('comment_pattern', '') + # todos = [] ret = Todo.by_date_range_with_limits(self._conn, (start, end)) todos_by_date_range, start, end = ret @@ -463,7 +466,8 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_conditions(self) -> dict[str, object]: """Show all Conditions.""" pattern = self._params.get_str_or_fail('pattern', '') - sort_by = self._params.get_str_or_fail('sort_by', '') + sort_by = self._params.get_str_or_fail('sort_by', 'title') + # conditions = Condition.matching(self._conn, pattern) sort_by = Condition.sort_by(conditions, sort_by) return {'conditions': conditions, @@ -496,6 +500,7 @@ class TaskHandler(BaseHTTPRequestHandler): owner_ids = self._params.get_all_int('step_to') owned_ids = self._params.get_all_int('has_step') title_64 = self._params.get_str('title_b64') + # if title_64: try: title = b64decode(title_64.encode()).decode() @@ -536,7 +541,8 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_processes(self) -> dict[str, object]: """Show all Processes.""" pattern = self._params.get_str_or_fail('pattern', '') - sort_by = self._params.get_str_or_fail('sort_by', '') + sort_by = self._params.get_str_or_fail('sort_by', 'title') + # processes = Process.matching(self._conn, pattern) sort_by = Process.sort_by(processes, sort_by) return {'processes': processes, 'sort_by': sort_by, 'pattern': pattern} @@ -592,6 +598,7 @@ class TaskHandler(BaseHTTPRequestHandler): comments = self._form.get_all_str('comment') efforts = self._form.get_all_floats_or_nones('effort') done_todos = self._form.get_all_int('done') + # for _ in [id_ for id_ in done_todos if id_ not in old_todos]: raise BadFormatException('"done" field refers to unknown Todo') is_done = [t_id in done_todos for t_id in old_todos] @@ -638,6 +645,7 @@ class TaskHandler(BaseHTTPRequestHandler): cond_rels = [self._form.get_all_int(name) for name in ['conditions', 'blockers', 'enables', 'disables']] effort_or_not = self._form.get_str('effort') + # if effort_or_not is not None: if effort_or_not == '': to_update['effort'] = None @@ -719,6 +727,7 @@ class TaskHandler(BaseHTTPRequestHandler): for step_id in kept_steps: name = f'new_step_to_{step_id}' new_steps_to[step_id] = self._form.get_all_int(name) + # for k, v in versioned.items(): getattr(process, k).set(v) process.set_condition_relations(self._conn, *cond_rels) @@ -784,6 +793,7 @@ class TaskHandler(BaseHTTPRequestHandler): title = self._form.get_str_or_fail('title') description = self._form.get_str_or_fail('description') is_active = self._form.get_bool_or_none('is_active') + # if is_active is not None: condition.is_active = is_active condition.title.set(title) diff --git a/tests/days.py b/tests/days.py index 0c6ee72..aac150b 100644 --- a/tests/days.py +++ b/tests/days.py @@ -123,7 +123,7 @@ class ExpectedGetCalendar(Expected): class ExpectedGetDay(Expected): """Builder of expectations for GET /day.""" - _default_dict = {'make_type': ''} + _default_dict = {'make_type': 'full'} _on_empty_make_temp = ('Day', 'day_as_dict') def __init__(self, date: str, *args: Any, **kwargs: Any) -> None: @@ -163,6 +163,7 @@ class TestsWithServer(TestCaseWithServer): def test_basic_GET_day(self) -> None: """Test basic (no Processes/Conditions/Todos) GET /day basics.""" # check illegal date parameters + self.check_get('/day?date=', 400) self.check_get('/day?date=foo', 400) self.check_get('/day?date=2024-02-30', 400) # check undefined day diff --git a/tests/utils.py b/tests/utils.py index e04fba6..71da9fb 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1015,9 +1015,9 @@ class TestCaseWithServer(TestCaseWithDB): try: self.assertEqual(cmp, retrieved) except AssertionError as e: - print('EXPECTED:') - pprint(cmp) - print('RETRIEVED:') - pprint(retrieved) - walk_diffs('', cmp, retrieved) + # print('EXPECTED:') + # pprint(cmp) + # print('RETRIEVED:') + # pprint(retrieved) + # walk_diffs('', cmp, retrieved) raise e -- 2.30.2 From 5a1ad2652cd2984a73eb9c1dc3f1c23f361a2365 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 9 Aug 2024 18:15:04 +0200 Subject: [PATCH 12/16] Clean up do_POST_process code. --- plomtask/http.py | 65 ++++++++++++++++++----------------------- plomtask/processes.py | 67 +++++++++++++++++++++++++++---------------- 2 files changed, 70 insertions(+), 62 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index c2e1f5d..51b35cb 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -645,7 +645,6 @@ class TaskHandler(BaseHTTPRequestHandler): cond_rels = [self._form.get_all_int(name) for name in ['conditions', 'blockers', 'enables', 'disables']] effort_or_not = self._form.get_str('effort') - # if effort_or_not is not None: if effort_or_not == '': to_update['effort'] = None @@ -655,7 +654,6 @@ class TaskHandler(BaseHTTPRequestHandler): except ValueError as e: msg = 'cannot float form field value for key: effort' raise BadFormatException(msg) from e - todo.set_condition_relations(self._conn, *cond_rels) for filler in [f for f in step_fillers if f != 'ignore']: target_id: int to_int = filler @@ -673,6 +671,8 @@ class TaskHandler(BaseHTTPRequestHandler): to_make['full'] += [target_id] else: adopted_child_ids += [target_id] + # + todo.set_condition_relations(self._conn, *cond_rels) to_remove = [] for child in todo.children: if child.id_ and (child.id_ not in adopted_child_ids): @@ -713,6 +713,16 @@ class TaskHandler(BaseHTTPRequestHandler): def do_POST_process(self, process: Process) -> str: """Update or insert Process of ?id= and fields defined in postvars.""" # pylint: disable=too-many-locals + + def id_or_title(l_id_or_title: list[str]) -> tuple[str, list[int]]: + l_ids, title = [], '' + for id_or_title in l_id_or_title: + try: + l_ids += [int(id_or_title)] + except ValueError: + title = id_or_title + return title, l_ids + versioned = {'title': self._form.get_str_or_fail('title'), 'description': self._form.get_str_or_fail('description'), 'effort': self._form.get_float_or_fail('effort')} @@ -720,55 +730,36 @@ class TaskHandler(BaseHTTPRequestHandler): in ['conditions', 'blockers', 'enables', 'disables']] calendarize = self._form.get_bool_or_none('calendarize') step_of = self._form.get_all_str('step_of') - suppresses = self._form.get_all_int('suppresses') + suppressions = self._form.get_all_int('suppresses') kept_steps = self._form.get_all_int('kept_steps') - new_top_steps = self._form.get_all_str('new_top_step') + new_top_step_procs = self._form.get_all_str('new_top_step') new_steps_to = {} for step_id in kept_steps: name = f'new_step_to_{step_id}' new_steps_to[step_id] = self._form.get_all_int(name) + new_owner_title, owners_to_set = id_or_title(step_of) + new_step_title, new_top_step_proc_ids = id_or_title(new_top_step_procs) # for k, v in versioned.items(): getattr(process, k).set(v) - process.set_condition_relations(self._conn, *cond_rels) if calendarize is not None: process.calendarize = calendarize process.save(self._conn) assert isinstance(process.id_, int) - # set relations to, and if non-existant yet: create, other Processes - # pylint: disable=fixme - # TODO: in what order to set owners, owneds, and possibly step - # suppressions can make the difference between recursion checks - # failing; should probably be handled class-internally to Process - # rather than here! - # 1. owners (upwards) - owners_to_set = [] - new_owner_title = None - for owner_identifier in step_of: - try: - owners_to_set += [int(owner_identifier)] - except ValueError: - new_owner_title = owner_identifier - process.set_owners(self._conn, owners_to_set) - # 2. owneds (downwards) - new_step_title = None - steps: list[ProcessStep] = [ProcessStep.by_id(self._conn, step_id) - for step_id in kept_steps] - for step_id in kept_steps: - new_sub_steps = [ + # set relations to Conditions and ProcessSteps / other Processes + process.set_condition_relations(self._conn, *cond_rels) + owned_steps = [] + for step_id in kept_steps: # collecting sub-steps + owned_steps += [ProcessStep.by_id(self._conn, step_id)] + owned_steps += [ # new sub-steps ProcessStep(None, process.id_, step_process_id, step_id) for step_process_id in new_steps_to[step_id]] - steps += new_sub_steps - for step_id_or_new_title in new_top_steps: - try: - step_process_id = int(step_id_or_new_title) - step = ProcessStep(None, process.id_, step_process_id, None) - steps += [step] - except ValueError: - new_step_title = step_id_or_new_title - process.set_steps(self._conn, steps) - process.set_step_suppressions(self._conn, suppresses) - # encode titles for potentially newly created Processes up or down + for step_process_id in new_top_step_proc_ids: + owned_steps += [ProcessStep(None, process.id_, step_process_id, + None)] + process.set_step_relations(self._conn, owners_to_set, suppressions, + owned_steps) + # encode titles for potential newly-to-create Processes up or down params = f'id={process.id_}' if new_step_title: title_b64_encoded = b64encode(new_step_title.encode()).decode() diff --git a/plomtask/processes.py b/plomtask/processes.py index 56dd282..23eb624 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -129,16 +129,54 @@ class Process(BaseModel[int], ConditionsRelations): walk_steps(step_node) return step_nodes - def set_step_suppressions(self, db_conn: DatabaseConnection, - step_ids: list[int]) -> None: + def set_step_relations(self, + db_conn: DatabaseConnection, + owners: list[int], + suppressions: list[int], + owned_steps: list[ProcessStep] + ) -> None: + """Set step owners, suppressions, and owned steps.""" + self._set_owners(db_conn, owners) + self._set_step_suppressions(db_conn, suppressions) + self.set_steps(db_conn, owned_steps) + + def _set_step_suppressions(self, + db_conn: DatabaseConnection, + step_ids: list[int] + ) -> None: """Set self.suppressed_steps from step_ids.""" assert isinstance(self.id_, int) db_conn.delete_where('process_step_suppressions', 'process', self.id_) self.suppressed_steps = [ProcessStep.by_id(db_conn, s) for s in step_ids] - def set_steps(self, db_conn: DatabaseConnection, - steps: list[ProcessStep]) -> None: + def _set_owners(self, + db_conn: DatabaseConnection, + owner_ids: list[int] + ) -> None: + """Re-set owners to those identified in owner_ids.""" + owners_old = self.used_as_step_by(db_conn) + losers = [o for o in owners_old if o.id_ not in owner_ids] + owners_old_ids = [o.id_ for o in owners_old] + winners = [Process.by_id(db_conn, id_) for id_ in owner_ids + if id_ not in owners_old_ids] + steps_to_remove = [] + for loser in losers: + steps_to_remove += [s for s in loser.explicit_steps + if s.step_process_id == self.id_] + for step in steps_to_remove: + step.remove(db_conn) + for winner in winners: + assert isinstance(winner.id_, int) + assert isinstance(self.id_, int) + new_step = ProcessStep(None, winner.id_, self.id_, None) + new_explicit_steps = winner.explicit_steps + [new_step] + winner.set_steps(db_conn, new_explicit_steps) + + def set_steps(self, + db_conn: DatabaseConnection, + steps: list[ProcessStep] + ) -> None: """Set self.explicit_steps in bulk. Checks against recursion, and turns into top-level steps any of @@ -166,27 +204,6 @@ class Process(BaseModel[int], ConditionsRelations): walk_steps(step) step.save(db_conn) - def set_owners(self, db_conn: DatabaseConnection, - owner_ids: list[int]) -> None: - """Re-set owners to those identified in owner_ids.""" - owners_old = self.used_as_step_by(db_conn) - losers = [o for o in owners_old if o.id_ not in owner_ids] - owners_old_ids = [o.id_ for o in owners_old] - winners = [Process.by_id(db_conn, id_) for id_ in owner_ids - if id_ not in owners_old_ids] - steps_to_remove = [] - for loser in losers: - steps_to_remove += [s for s in loser.explicit_steps - if s.step_process_id == self.id_] - for step in steps_to_remove: - step.remove(db_conn) - for winner in winners: - assert isinstance(winner.id_, int) - assert isinstance(self.id_, int) - new_step = ProcessStep(None, winner.id_, self.id_, None) - new_explicit_steps = winner.explicit_steps + [new_step] - winner.set_steps(db_conn, new_explicit_steps) - def save(self, db_conn: DatabaseConnection) -> None: """Add (or re-write) self and connected items to DB.""" super().save(db_conn) -- 2.30.2 From 9c615407aa166e6ce39aedde0ed45890aa87e58b Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 9 Aug 2024 20:04:28 +0200 Subject: [PATCH 13/16] Simplify do_POST_todo code. --- plomtask/http.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 51b35cb..704ae06 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -632,7 +632,7 @@ class TaskHandler(BaseHTTPRequestHandler): """Update Todo and its children.""" # pylint: disable=too-many-locals # pylint: disable=too-many-branches - adopted_child_ids = self._form.get_all_int('adopt') + adoptees = self._form.get_all_int('adopt') to_make = {'full': self._form.get_all_int('make_full'), 'empty': self._form.get_all_int('make_empty')} step_fillers = self._form.get_all_str('step_filler') @@ -670,19 +670,14 @@ class TaskHandler(BaseHTTPRequestHandler): elif filler.startswith('make_full_'): to_make['full'] += [target_id] else: - adopted_child_ids += [target_id] + adoptees += [target_id] # todo.set_condition_relations(self._conn, *cond_rels) - to_remove = [] - for child in todo.children: - if child.id_ and (child.id_ not in adopted_child_ids): - to_remove += [child.id_] - for id_ in to_remove: - child = Todo.by_id(self._conn, id_) - todo.remove_child(child) - for child_id in adopted_child_ids: - if child_id not in [c.id_ for c in todo.children]: - todo.add_child(Todo.by_id(self._conn, child_id)) + for child in [c for c in todo.children if c.id_ not in adoptees]: + todo.remove_child(child) + for child_id in [id_ for id_ in adoptees + if id_ not in [c.id_ for c in todo.children]]: + todo.add_child(Todo.by_id(self._conn, child_id)) todo.update_attrs(**to_update) for approach, proc_ids in to_make.items(): for process_id in proc_ids: @@ -749,7 +744,7 @@ class TaskHandler(BaseHTTPRequestHandler): # set relations to Conditions and ProcessSteps / other Processes process.set_condition_relations(self._conn, *cond_rels) owned_steps = [] - for step_id in kept_steps: # collecting sub-steps + for step_id in kept_steps: owned_steps += [ProcessStep.by_id(self._conn, step_id)] owned_steps += [ # new sub-steps ProcessStep(None, process.id_, step_process_id, step_id) -- 2.30.2 From 46e509c1467535e2281922718dc60ee89a0a019e Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 10 Aug 2024 03:12:21 +0200 Subject: [PATCH 14/16] Minor improvements of TaskHandler code. --- plomtask/http.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 704ae06..cba55d6 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -452,7 +452,6 @@ class TaskHandler(BaseHTTPRequestHandler): process_id = self._params.get_int_or_none('process_id') comment_pattern = self._params.get_str_or_fail('comment_pattern', '') # - todos = [] ret = Todo.by_date_range_with_limits(self._conn, (start, end)) todos_by_date_range, start, end = ret todos = [t for t in todos_by_date_range @@ -500,14 +499,16 @@ class TaskHandler(BaseHTTPRequestHandler): owner_ids = self._params.get_all_int('step_to') owned_ids = self._params.get_all_int('has_step') title_64 = self._params.get_str('title_b64') - # + title_new = None if title_64: try: - title = b64decode(title_64.encode()).decode() + title_new = b64decode(title_64.encode()).decode() except binascii_Exception as exc: msg = 'invalid base64 for ?title_b64=' raise BadFormatException(msg) from exc - process.title.set(title) + # + if title_new: + process.title.set(title_new) preset_top_step = None owners = process.used_as_step_by(self._conn) for step_id in owner_ids: @@ -598,15 +599,15 @@ class TaskHandler(BaseHTTPRequestHandler): comments = self._form.get_all_str('comment') efforts = self._form.get_all_floats_or_nones('effort') done_todos = self._form.get_all_int('done') - # - for _ in [id_ for id_ in done_todos if id_ not in old_todos]: - raise BadFormatException('"done" field refers to unknown Todo') is_done = [t_id in done_todos for t_id in old_todos] if not (len(old_todos) == len(is_done) == len(comments) == len(efforts)): msg = 'not equal number each of number of todo_id, comments, ' +\ 'and efforts inputs' raise BadFormatException(msg) + for _ in [id_ for id_ in done_todos if id_ not in old_todos]: + raise BadFormatException('"done" field refers to unknown Todo') + # day = Day.by_id_or_create(self._conn, date) day.comment = day_comment day.save(self._conn) @@ -674,10 +675,10 @@ class TaskHandler(BaseHTTPRequestHandler): # todo.set_condition_relations(self._conn, *cond_rels) for child in [c for c in todo.children if c.id_ not in adoptees]: - todo.remove_child(child) + todo.remove_child(child) for child_id in [id_ for id_ in adoptees if id_ not in [c.id_ for c in todo.children]]: - todo.add_child(Todo.by_id(self._conn, child_id)) + todo.add_child(Todo.by_id(self._conn, child_id)) todo.update_attrs(**to_update) for approach, proc_ids in to_make.items(): for process_id in proc_ids: -- 2.30.2 From 76af5f86ab547bcad3c1c944a369bf8b216ee5da Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 10 Aug 2024 03:18:20 +0200 Subject: [PATCH 15/16] In Todo view, allow filling of steps below sub-steps. --- templates/todo.html | 2 -- 1 file changed, 2 deletions(-) diff --git a/templates/todo.html b/templates/todo.html index 20279bb..61d4675 100644 --- a/templates/todo.html +++ b/templates/todo.html @@ -22,7 +22,6 @@ select{ font-size: 0.5em; margin: 0; padding: 0; } {{item.todo.title_then|e}} {% else %} {{item.process.title.newest|e}} -{% if indent == 0 %} · fill: {% endif %} -{% endif %} {% for child in item.children %} -- 2.30.2 From 4c6908e51c5eeaeef66dd66325d367dc42b29b75 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 10 Aug 2024 05:32:55 +0200 Subject: [PATCH 16/16] Add TaskHandler code to actually make previous commit work. --- plomtask/http.py | 82 ++++++++++++++++++++++++++------------------- templates/todo.html | 10 +++--- tests/misc.py | 36 ++++++++++---------- tests/todos.py | 61 +++++++-------------------------- tests/utils.py | 16 +++++---- 5 files changed, 94 insertions(+), 111 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index cba55d6..e307f14 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -88,11 +88,11 @@ class InputsParser: return None return val in {'True', 'true', '1', 'on'} - def get_firsts_of_key_prefixed(self, prefix: str) -> dict[str, str]: - """Retrieve dict of (first) strings at key starting with prefix.""" + def get_all_of_key_prefixed(self, key_prefix: str) -> dict[str, list[str]]: + """Retrieve dict of strings at keys starting with key_prefix.""" ret = {} - for key in [k for k in self.inputs.keys() if k.startswith(prefix)]: - ret[key] = self.inputs[key][0] + for key in [k for k in self.inputs.keys() if k.startswith(key_prefix)]: + ret[key[len(key_prefix):]] = self.inputs[key] return ret def get_float_or_fail(self, key: str) -> float: @@ -581,10 +581,9 @@ class TaskHandler(BaseHTTPRequestHandler): id_ = self._params.get_int_or_none('id') item = cls.by_id(self._conn, id_) attr = getattr(item, attr_name) - for k, v in self._form.get_firsts_of_key_prefixed('at:').items(): - old = k[3:] - if old[19:] != v: - attr.reset_timestamp(old, f'{v}.0') + for k, vals in self._form.get_all_of_key_prefixed('at:').items(): + if k[19:] != vals[0]: + attr.reset_timestamp(k, f'{vals[0]}.0') attr.save(self._conn) return f'/{cls.name_lowercase()}_{attr_name}s?id={item.id_}' @@ -633,10 +632,14 @@ class TaskHandler(BaseHTTPRequestHandler): """Update Todo and its children.""" # pylint: disable=too-many-locals # pylint: disable=too-many-branches - adoptees = self._form.get_all_int('adopt') - to_make = {'full': self._form.get_all_int('make_full'), - 'empty': self._form.get_all_int('make_empty')} - step_fillers = self._form.get_all_str('step_filler') + # pylint: disable=too-many-statements + assert todo.id_ is not None + adoptees = [(id_, todo.id_) for id_ in self._form.get_all_int('adopt')] + to_make = {'full': [(id_, todo.id_) + for id_ in self._form.get_all_int('make_full')], + 'empty': [(id_, todo.id_) + for id_ in self._form.get_all_int('make_empty')]} + step_fillers_to = self._form.get_all_of_key_prefixed('step_filler_to_') to_update: dict[str, Any] = { 'comment': self._form.get_str_or_fail('comment', '')} for k in ('is_done', 'calendarize'): @@ -655,39 +658,50 @@ class TaskHandler(BaseHTTPRequestHandler): except ValueError as e: msg = 'cannot float form field value for key: effort' raise BadFormatException(msg) from e - for filler in [f for f in step_fillers if f != 'ignore']: - target_id: int - to_int = filler - for prefix in [p for p in ['make_empty_', 'make_full_'] - if filler.startswith(p)]: - to_int = filler[len(prefix):] + for k, fillers in step_fillers_to.items(): try: - target_id = int(to_int) + parent_id = int(k) except ValueError as e: - msg = 'bad fill_for target: {filler}' + msg = f'bad step_filler_to_ key: {k}' raise BadFormatException(msg) from e - if filler.startswith('make_empty_'): - to_make['empty'] += [target_id] - elif filler.startswith('make_full_'): - to_make['full'] += [target_id] - else: - adoptees += [target_id] + for filler in [f for f in fillers if f != 'ignore']: + target_id: int + prefix = 'make_' + to_int = filler[5:] if filler.startswith(prefix) else filler + try: + target_id = int(to_int) + except ValueError as e: + msg = f'bad fill_for target: {filler}' + raise BadFormatException(msg) from e + if filler.startswith(prefix): + to_make['empty'] += [(target_id, parent_id)] + else: + adoptees += [(target_id, parent_id)] # todo.set_condition_relations(self._conn, *cond_rels) - for child in [c for c in todo.children if c.id_ not in adoptees]: - todo.remove_child(child) - for child_id in [id_ for id_ in adoptees - if id_ not in [c.id_ for c in todo.children]]: - todo.add_child(Todo.by_id(self._conn, child_id)) + for parent in [Todo.by_id(self._conn, a[1]) + for a in adoptees] + [todo]: + for child in parent.children: + if child not in [t[0] for t in adoptees + if t[0] == child.id_ and t[1] == parent.id_]: + parent.remove_child(child) + parent.save(self._conn) + for child_id, parent_id in adoptees: + parent = Todo.by_id(self._conn, parent_id) + if child_id not in [c.id_ for c in parent.children]: + parent.add_child(Todo.by_id(self._conn, child_id)) + parent.save(self._conn) todo.update_attrs(**to_update) - for approach, proc_ids in to_make.items(): - for process_id in proc_ids: + for approach, make_data in to_make.items(): + for process_id, parent_id in make_data: + parent = Todo.by_id(self._conn, parent_id) process = Process.by_id(self._conn, process_id) made = Todo(None, process, False, todo.date) made.save(self._conn) if 'full' == approach: made.ensure_children(self._conn) - todo.add_child(made) + parent.add_child(made) + parent.save(self._conn) # todo.save() may destroy Todo if .effort < 0, so retrieve .id_ early url = f'/todo?id={todo.id_}' todo.save(self._conn) diff --git a/templates/todo.html b/templates/todo.html index 61d4675..de5dbd2 100644 --- a/templates/todo.html +++ b/templates/todo.html @@ -22,19 +22,21 @@ select{ font-size: 0.5em; margin: 0; padding: 0; } {{item.todo.title_then|e}} {% else %} {{item.process.title.newest|e}} -· fill: - - + {% for adoptable in adoption_candidates_for[item.process.id_] %} {% endfor %} +{% endif %} + {% endif %} {% for child in item.children %} -{{ draw_tree_row(child, item, indent+1) }} +{{ draw_tree_row(child, item.todo, indent+1) }} {% endfor %} {% endmacro %} diff --git a/tests/misc.py b/tests/misc.py index 1efa335..86474c7 100644 --- a/tests/misc.py +++ b/tests/misc.py @@ -36,31 +36,31 @@ class TestsSansServer(TestCase): parser = InputsParser({'foo': ['baz', 'quux']}) self.assertEqual('baz', parser.get_str('foo', 'bar')) - def test_InputsParser_get_firsts_of_key_prefixed(self) -> None: - """Test InputsParser.get_firsts_of_key_prefixed.""" + def test_InputsParser_get_all_of_key_prefixed(self) -> None: + """Test InputsParser.get_all_of_key_prefixed.""" parser = InputsParser({}) self.assertEqual({}, - parser.get_firsts_of_key_prefixed('')) + parser.get_all_of_key_prefixed('')) self.assertEqual({}, - parser.get_firsts_of_key_prefixed('foo')) + parser.get_all_of_key_prefixed('foo')) parser = InputsParser({'foo': ['bar']}) - self.assertEqual({'foo': 'bar'}, - parser.get_firsts_of_key_prefixed('')) - parser = InputsParser({'x': ['y']}) - self.assertEqual({'x': 'y'}, - parser.get_firsts_of_key_prefixed('x')) - parser = InputsParser({'xx': ['y']}) - self.assertEqual({'xx': 'y'}, - parser.get_firsts_of_key_prefixed('x')) + self.assertEqual({'foo': ['bar']}, + parser.get_all_of_key_prefixed('')) + parser = InputsParser({'x': ['y', 'z']}) + self.assertEqual({'': ['y', 'z']}, + parser.get_all_of_key_prefixed('x')) + parser = InputsParser({'xx': ['y', 'Z']}) + self.assertEqual({'x': ['y', 'Z']}, + parser.get_all_of_key_prefixed('x')) parser = InputsParser({'xx': ['y']}) self.assertEqual({}, - parser.get_firsts_of_key_prefixed('xxx')) + parser.get_all_of_key_prefixed('xxx')) parser = InputsParser({'xxx': ['x'], 'xxy': ['y'], 'xyy': ['z']}) - self.assertEqual({'xxx': 'x', 'xxy': 'y'}, - parser.get_firsts_of_key_prefixed('xx')) - parser = InputsParser({'xxx': ['x', 'y', 'z'], 'xxy': ['y', 'z']}) - self.assertEqual({'xxx': 'x', 'xxy': 'y'}, - parser.get_firsts_of_key_prefixed('xx')) + self.assertEqual({'x': ['x'], 'y': ['y']}, + parser.get_all_of_key_prefixed('xx')) + parser = InputsParser({'xxx': ['x', 'y'], 'xxy': ['y', 'z']}) + self.assertEqual({'x': ['x', 'y'], 'y': ['y', 'z']}, + parser.get_all_of_key_prefixed('xx')) def test_InputsParser_get_int_or_none(self) -> None: """Test InputsParser.get_int_or_none.""" diff --git a/tests/todos.py b/tests/todos.py index d84bb70..2ecf3b8 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -282,14 +282,16 @@ class TestsWithServer(TestCaseWithServer): self.check_post({}, '/todo?id=1', 404) # test malformed values on existing Todo self.post_exp_day([], {'new_todo': [1]}) - for name in [ - 'adopt', 'effort', 'make_full', 'make_empty', 'step_filler', - 'conditions', 'disables', 'blockers', 'enables']: + for name in ['adopt', 'effort', 'make_full', 'make_empty', + 'conditions', 'disables', 'blockers', 'enables']: self.check_post({name: 'x'}, '/todo?id=1', 400, '/todo') - for prefix in ['make_empty_', 'make_full_']: + for prefix in ['make_', '']: for suffix in ['', 'x', '1.1']: - self.check_post({'step_filler': f'{prefix}{suffix}'}, - '/todo?id=1', 400, '/todo') + self.check_post({'step_filler_to_1': [f'{prefix}{suffix}']}, + '/todo?id=1', 400, '/todo') + for suffix in ['', 'x', '1.1']: + self.check_post({'step_filler_to_{suffix}': ['1']}, + '/todo?id=1', 400, '/todo') def test_basic_POST_todo(self) -> None: """Test basic POST /todo manipulations.""" @@ -398,14 +400,15 @@ class TestsWithServer(TestCaseWithServer): self.post_exp_day([exp], {'new_todo': [2]}) self.post_exp_day([exp], {'new_todo': [3]}) self.check_json_get('/todo?id=1', exp) - self._post_exp_todo(1, {'step_filler': 5, 'adopt': [4]}, exp) + self._post_exp_todo(1, {'step_filler_to_1': 5, 'adopt': [4]}, exp) + exp.lib_get('Todo', 1)['children'] += [5] step1_proc2 = exp.step_as_dict(1, [], 2, 4, True) step2_proc3 = exp.step_as_dict(2, [], 3, 5, True) exp.set('steps_todo_to_process', [step1_proc2, step2_proc3]) self.check_json_get('/todo?id=1', exp) # test 'ignore' values for 'step_filler' are ignored, and intable # 'step_filler' values are interchangeable with those of 'adopt' - todo_post = {'adopt': 5, 'step_filler': ['ignore', 4]} + todo_post = {'adopt': 5, 'step_filler_to_1': ['ignore', 4]} self.check_post(todo_post, '/todo?id=1') self.check_json_get('/todo?id=1', exp) # test cannot adopt into non-top-level elements of chain, instead @@ -423,46 +426,8 @@ class TestsWithServer(TestCaseWithServer): step4_todo6]) self.check_json_get('/todo?id=1', exp) - def test_POST_todo_make_full(self) -> None: - """Test creation and adoption via POST /todo with "make_full".""" - # create chain of Processes - exp = ExpectedGetTodo(1) - self.post_exp_process([exp], {}, 1) - for i in range(1, 4): - self.post_exp_process([exp], {'new_top_step': i}, i+1) - exp.lib_set('ProcessStep', [exp.procstep_as_dict(1, 2, 1), - exp.procstep_as_dict(2, 3, 2), - exp.procstep_as_dict(3, 4, 3)]) - step3_proc1 = exp.step_as_dict(3, [], 1, None, False) - step2_proc2 = exp.step_as_dict(2, [step3_proc1], 2, None, False) - step1_proc3 = exp.step_as_dict(1, [step2_proc2], 3, None, True) - exp.set('steps_todo_to_process', [step1_proc3]) - # post (childless) Todo of chain end, then make_full on next in line - self.post_exp_day([exp], {'new_todo': [4]}) - self.check_post({'step_filler': 'make_full_3'}, '/todo?id=1') - exp.set_todo_from_post(4, {'process_id': 1}) - exp.set_todo_from_post(3, {'process_id': 2, 'children': [4]}) - exp.set_todo_from_post(2, {'process_id': 3, 'children': [3]}) - exp.set_todo_from_post(1, {'process_id': 4, 'children': [2]}) - step3_proc1 = exp.step_as_dict(3, [], 1, 4, True) - step2_proc2 = exp.step_as_dict(2, [step3_proc1], 2, 3, True) - step1_proc3 = exp.step_as_dict(1, [step2_proc2], 3, 2, True) - exp.set('steps_todo_to_process', [step1_proc3]) - self.check_json_get('/todo?id=1', exp) - # make new chain next to expected, find steps_todo_to_process extended, - # expect existing Todo demanded by new chain be adopted into new chain - self.check_post({'make_full': 2, 'adopt': [2]}, '/todo?id=1') - exp.set_todo_from_post(5, {'process_id': 2, 'children': [4]}) - exp.set_todo_from_post(1, {'process_id': 4, 'children': [2, 5]}) - step5_todo4 = exp.step_as_dict(5, [], None, 4) - step4_todo5 = exp.step_as_dict(4, [step5_todo4], None, 5) - exp.set('steps_todo_to_process', [step1_proc3, step4_todo5]) - self.check_json_get('/todo?id=1', exp) - # fail on trying to call make_full on non-existing Process - self.check_post({'make_full': 5}, '/todo?id=1', 404) - def test_POST_todo_make_empty(self) -> None: - """Test creation and adoption via POST /todo with "make_empty".""" + """Test creation via POST /todo "step_filler_to"/"make".""" # create chain of Processes exp = ExpectedGetTodo(1) self.post_exp_process([exp], {}, 1) @@ -478,7 +443,7 @@ class TestsWithServer(TestCaseWithServer): step1_proc3 = exp.step_as_dict(1, [step2_proc2], 3, None, True) exp.set('steps_todo_to_process', [step1_proc3]) self.check_json_get('/todo?id=1', exp) - self.check_post({'step_filler': 'make_empty_3'}, '/todo?id=1') + self.check_post({'step_filler_to_1': 'make_3'}, '/todo?id=1') exp.set_todo_from_post(2, {'process_id': 3}) exp.set_todo_from_post(1, {'process_id': 4, 'children': [2]}) step2_proc2 = exp.step_as_dict(2, [step3_proc1], 2, None, True) diff --git a/tests/utils.py b/tests/utils.py index 71da9fb..d1b6eac 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -753,11 +753,13 @@ class Expected: """Set Todo of id_ in library based on POST dict d.""" corrected_kwargs: dict[str, Any] = {'children': []} for k, v in d.items(): - if k in {'adopt', 'step_filler'}: + if k.startswith('step_filler_to_'): + continue + elif 'adopt' == k: new_children = v if isinstance(v, list) else [v] corrected_kwargs['children'] += new_children continue - if k in {'is_done', 'calendarize'}: + elif k in {'is_done', 'calendarize'}: v = v in VALID_TRUES corrected_kwargs[k] = v todo = self.lib_get('Todo', id_) @@ -1015,9 +1017,9 @@ class TestCaseWithServer(TestCaseWithDB): try: self.assertEqual(cmp, retrieved) except AssertionError as e: - # print('EXPECTED:') - # pprint(cmp) - # print('RETRIEVED:') - # pprint(retrieved) - # walk_diffs('', cmp, retrieved) + print('EXPECTED:') + pprint(cmp) + print('RETRIEVED:') + pprint(retrieved) + walk_diffs('', cmp, retrieved) raise e -- 2.30.2