From 7e7288310fe7490a4bf976f46b1b22b8ff06e290 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Thu, 8 Aug 2024 09:01:49 +0200 Subject: [PATCH] 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