From 8b5c64cae29825abe984ce4969896b8f35a46cfe Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Thu, 12 Nov 2026 06:03:18 +0100 Subject: [PATCH 1/3] test: revive the body of the functional test Since 0c36d0f the whole body of _run_plann_tests sat one level too deep, after the `return ret` of the nested dag() helper, so test_plann only started and stopped xandikos and asserted nothing. Dedented; it passes again. Prompt: So yesterday and today I've had a calendar with tasks "pinned" to the calendar through events. Did I make any `interactive`-command for going through those events and answering y/n on whether the tasks were done or not? Followup-Prompt: Yes, I need that. [an interactive command to go through past pinning events and ask whether each task was done] Followup-Prompt: go for it, including fixing the dead functional test. Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox --- tests/test_functional.py | 512 +++++++++++++++++++-------------------- 1 file changed, 256 insertions(+), 256 deletions(-) diff --git a/tests/test_functional.py b/tests/test_functional.py index 9dfd9df..e22d5a8 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -195,262 +195,262 @@ def dag(obj, reltype, observed=None): ret[x] = dag(x, reltype, observed) return ret - ## We create two tasks todo1 and todo2, todo2 being a child of todo1 - todo1 = _add_todo(ctx, summary=['make plann good'], set_due='2012-12-20 23:15:00', set_dtstart='2012-12-20 22:15:00', set_uid='todo1', set_categories='plann,keyboard') - uid1 = str(todo1.icalendar_component['uid']) - todo2 = _add_todo(ctx, summary=['fix some bugs in plann'], set_parent=[uid1], set_due='2012-12-21 23:15:00', set_dtstart='2012-12-21 22:15:00', set_uid='todo2', set_categories=('plann', 'keyboard')) - uid2 = str(todo2.icalendar_component['uid']) - - ## Selecting the tasks should yield ... 2 (but only one if skip_children or skip_parents is used) - _select(ctx, todo=True) - assert len(ctx.obj['objs'])==2 - _select(ctx, todo=True, skip_children=True) - assert len(ctx.obj['objs'])==1 - _select(ctx, todo=True, skip_parents=True) - assert len(ctx.obj['objs'])==1 - _select(ctx, summary='make plann good') - ## this breaks with xandikos - #assert len(ctx.obj['objs'])==1 - _select(ctx, summary='make plann good', todo=True) - assert len(ctx.obj['objs'])==1 - _select(ctx, event=True) - assert len(ctx.obj['objs'])==0 - - ## Journal tests - _add_journal(ctx, summary=['bought new keyboard'], set_dtstart='2012-12-20') - _select(ctx, journal=True) - assert len(ctx.obj['objs'])==1 - _select(ctx, todo=True) - assert len(ctx.obj['objs'])==2 - _select(ctx, event=True) - assert len(ctx.obj['objs'])==0 - - ## assert that relations are as expected - todo1.load() - todo2.load() - assert(dag(todo1, 'CHILD') == {uid2: {}}) - assert(dag(todo2, 'PARENT') == {uid1: {}}) - assert(not _adjust_ical_relations(todo1, {'CHILD': {uid2}, 'PARENT': set()})) - assert(not _adjust_ical_relations(todo2, {'PARENT': {uid1}, 'CHILD': set()})) - - _select(ctx, todo=True) - ## This should return a list of human-readable strings, with an indented child. - list_td = _list(ctx.obj['objs'], top_down=True, echo=False) - assert(list_td[0][0] != ' ') - assert(list_td[1][0] == ' ') - assert('good' in list_td[0]) - - ## This should return a list of human-readable strings, with an indented parent. - list_bu = _list(ctx.obj['objs'], bottom_up=True, echo=False) - assert(list_bu[0][0] != ' ') - assert(list_bu[1][0] == ' ') - assert('good' in list_bu[1]) - - ## Test that the calendar_name template attribute works - list_cal = _list(ctx.obj['objs'], template="{calendar_name}", echo=False) - assert list_cal[0] == todo1.parent.get_display_name() - assert list_cal[1] == todo2.parent.get_display_name() - - ## panic planning, timeline_suggestion. - ## We have two tasks in the calendar, each with one hour duration - timeline = timeline_suggestion(ctx, hours_per_day=24) - assert(timeline.count() == 2) - foo = timeline.get(datetime_(year=2012, month=12, day=20, hour=22, minute=35)) - assert foo['begin'] == datetime_(year=2012, month=12, day=20, hour=22, minute=15) - assert foo['end'] == datetime_(year=2012, month=12, day=20, hour=23, minute=15) - - ## panic planning, timeline_suggestion with timeline_end - ## Those two tasks should be neatly stacked up - assert len(ctx.obj['objs'])==2 - timeline = timeline_suggestion(ctx, timeline_end=datetime_(year=2011, month=11, day=11, hour=11, minute=11), hours_per_day=24) - assert(timeline.count() == 2) - foo = timeline.get(datetime_(year=2011, month=11, day=11, hour=11, minute=10)) - assert foo['begin'] == datetime_(year=2011, month=11, day=11, hour=10, minute=11) - assert foo['end'] == datetime_(year=2011, month=11, day=11, hour=11, minute=11) - foo = timeline.get(datetime_(year=2011, month=11, day=11, hour=10, minute=10)) - assert foo['begin'] == datetime_(year=2011, month=11, day=11, hour=9, minute=11) - assert foo['end'] == datetime_(year=2011, month=11, day=11, hour=10, minute=11) - - ## fix_timeline feature - ## This will create events that are children of the tasks. - ## (Those events contains a suggested timeslot for doing the task - ## Task contains due date (may be a hard limit) and duration - with patch('plann.panic_planning._now', new=lambda: datetime_(2010,10,9,23,33,33)) as _now: - _check_for_panic(ctx, timeline_start='2010-10-10', timeline_end=datetime_(year=2010, month=11, day=11, hour=11, minute=11), hours_per_day=24, fix_timeline=True, include_all_events=True) - - ## Two events should be created by now - _select(ctx, event=True) - assert len(ctx.obj['objs'])==2 - - ## Re-running it should be a noop - _select(ctx, todo=True) - with patch('plann.panic_planning._now', new=lambda: datetime_(2010,10,9,23,33,33)) as _now: - _check_for_panic(ctx, timeline_start='2010-10-10', timeline_end=datetime_(year=2010, month=11, day=11, hour=11, minute=11), hours_per_day=24, fix_timeline=True, include_all_events=True) - _select(ctx, event=True) - assert len(ctx.obj['objs'])==2 - - ## We now have one task with one subtask, and both tasks have two children - ## events. Let's find the uids - e1 = [x for x in ctx.obj['objs'] if x.icalendar_component['summary'] == 'make plann good'][0] - e2 = [x for x in ctx.obj['objs'] if x.icalendar_component['summary'] == 'fix some bugs in plann'][0] - uide1 = str(e1.icalendar_component['uid']) - str(e1.icalendar_component['uid']) - - ## Testing the _adjust_relations - todo1.load() - ## todo1 has children todo2 and e1. _adjust_relations over those should be a noop - with patch.object(todo1, 'save') as _t1save: - with patch.object(todo2, 'save') as _t2save: - with patch.object(e1, 'save') as _e1save: - with patch.object(e2, 'save') as _e2save: - _adjust_relations(parent=todo1, children=[todo2, e1]) - assert not any(x.call_count for x in (_t1save, _t2save, _e1save, _e2save)) - - ## Testing to remove todo2 as child of todo1 - _adjust_relations(parent=todo1, children=[e1]) - todo1.load() - todo2.load() - e1.load() - ## _adjust_ical_relations returns a dict if the object was changed - assert(not _adjust_ical_relations(todo1, {'CHILD': {uide1}})) - assert(not _adjust_ical_relations(todo2, {'PARENT': set()})) - - ## Testing interactive fix timeline, and checking that past events are ignored in the - ## panic planning. - ## We were busy with other things at the scheduled time. Let's try to reschedule ... - with patch('plann.commands._editor', new=passthrough) as _editor: - with patch('plann.panic_planning._now', new=lambda: datetime_(2010,12,9,23,33,33)) as _now: - _select(ctx, todo=True) - _check_for_panic(ctx, timeline_start='2010-10-10', timeline_end=datetime_(year=2011, month=11, day=11, hour=11, minute=11), hours_per_day=24, print_timeline=False, interactive_fix_timeline=True, include_all_events=True) - _select(ctx, event=True) - assert len(ctx.obj['objs'])==4 - - ## Reset ... let's connect todo1 and todo2 again, and remove events - ## Reload to get fresh ETags — _check_for_panic modified these objects on the server - todo1.load() - todo2.load() - _adjust_relations(todo1, {todo2}) - _adjust_relations(todo2, set()) - todo1.load() - todo2.load() - assert(not _adjust_ical_relations(todo1, {'CHILD': {uid2}, 'PARENT': set()})) - assert(not _adjust_ical_relations(todo2, {'PARENT': {uid1}, 'CHILD': set()})) - - ## Testing the interactive relation edit - if doing nothing in the - ## editor, the algorithm should change nothing. - _select(ctx, todo=True) - cal_pre_interactive_relation_edit = "\n".join([x.data for x in ctx.obj['objs']]) - with patch('plann.interactive._editor', new=passthrough) as _editor: - _interactive_relation_edit(ctx.obj['objs']) - - for obj in ctx.obj['objs']: - obj.load() - cal_post_interactive_relation_edit = "\n".join([x.data for x in ctx.obj['objs']]) - assert cal_post_interactive_relation_edit == cal_pre_interactive_relation_edit - - ## Let's add some more tasks - todo3 = _add_todo(ctx, summary=['fix some more features in plann'], set_parent=[uid1], set_due='2012-12-21 23:15:00', set_dtstart='2012-12-21 22:15:00', set_uid='todo3', set_category='plann') - todo4 = _add_todo(ctx, summary=['make plann even better'], set_parent=[uid1], set_due='2012-12-21 23:15:00', set_dtstart='2012-12-21 22:15:00', set_uid='todo4') - todo5 = _add_todo(ctx, summary=['use plann on a daily basis to find more bugs and missing features'], set_parent=[uid1], set_due='2012-12-21 23:15:00', set_dtstart='2012-12-21 22:15:00', set_uid='todo5') - - ## TODO: testing of set_category / set_categories should be moved to a separate test in test_lib / test_command / test_cli - assert(set([str(x) for x in todo1.icalendar_component['CATEGORIES'].cats]) == {'plann', 'keyboard'}) - assert(set([str(x) for x in todo2.icalendar_component['CATEGORIES'].cats]) == {'plann', 'keyboard'}) - assert(set([str(x) for x in todo3.icalendar_component['CATEGORIES'].cats]) == {'plann'}) - - uid3 = str(todo3.icalendar_component['uid']) - uid4 = str(todo4.icalendar_component['uid']) - uid5 = str(todo5.icalendar_component['uid']) - - todo1.load() - assert(dag(todo1, 'CHILD') == {uid2: {}, uid3: {}, uid4: {}, uid5: {}}) - assert(dag(todo4, 'PARENT') == {uid1: {}}) - - ## Let's add some incremental indentation to the last lines. - ## This one will leave todo1 with two children, then the next lines are indented more. - def add_indent(text): - return f"{uid1}: todo1\n {uid2}: todo2\n {uid3}: todo3\n {uid4}: todo4\n {uid5}: todo5" - with patch('plann.interactive._editor', new=add_indent) as _editor: - _interactive_relation_edit([todo1]) - - ## Reload the object list - for obj in (todo1, todo2, todo3, todo4, todo5): - obj.load() - - assert(dag(todo1, 'CHILD') == {uid2: {}, uid3: {uid4: {uid5: {}}}}) - assert(dag(todo2, 'PARENT') == {uid1: {}}) - assert(dag(todo5, 'PARENT') == {uid4: {uid3: {uid1: {}}}}) - - ## This should not throw one into the debugger - list_td = _list(ctx.obj['objs'], top_down=True, echo=False) - - def remove_parent(input): - return f"{uid2}: todo2\n{uid3}: todo3\n {uid4}: todo4\n {uid5}: todo5" - - with patch('plann.interactive._editor', new=remove_parent) as _editor: - _interactive_relation_edit([todo1]) - - assert(dag(todo1, 'CHILD') == {}) - assert(dag(todo1, 'PARENT') == {}) - assert(dag(todo2, 'PARENT') == {}) - assert(dag(todo2, 'CHILD') == {}) - assert(dag(todo3, 'CHILD') == {uid4: {uid5: {}}}) - assert(dag(todo5, 'PARENT') == {uid4: {uid3: {}}}) - - ## This should not throw one into the debugger - #list_td = _list(ctx.obj['objs'], top_down=True, echo=False) - - def gen_prompt(ret): - def prompt(*largs, **kwargs): - return ret - return prompt - - ## testing _interactive_edit - with patch('click.echo') as _echo: - ## ignore should do nothing - with patch('click.prompt', new=gen_prompt('ignore')): - _interactive_edit(todo1) - ## no asserts needed - ## complete should complete it - with patch('click.prompt', new=gen_prompt('complete')): - _interactive_edit(todo1) - todo1.load() - assert(todo1.icalendar_component['STATUS']=='COMPLETED') - todo1.uncomplete() - with patch('click.prompt', new=gen_prompt('postpone 1d')): - _interactive_edit(todo1) - todo1.load() - assert(todo1.icalendar_component['DUE'].dt > datetime_(year=2023, month=9, day=19, hour=15)) - with patch('click.prompt', new=gen_prompt('set categories=foo')): - _interactive_edit(todo1) - todo1.load() - assert(set([str(x) for x in todo1.icalendar_component['CATEGORIES'].cats]) == {'foo'}) - with patch('click.prompt', new=gen_prompt('set category=bar')): - _interactive_edit(todo1) - todo1.load() - assert(set([str(x) for x in todo1.icalendar_component['CATEGORIES'].cats]) == {'foo', 'bar'}) - ## TODO: part, split, family - ## TODO: cancel, - - ## testing mass interactive edit and mass interactive reprioritize - ## with 0 objects, it would earlier raise an error. - with patch('plann.interactive._editor', new=passthrough) as _editor: - _mass_interactive_edit([], default='complete') - for todo in (todo1, todo2, todo3, todo4, todo5): - todo.load() - assert todo.icalendar_component['STATUS'] == 'NEEDS-ACTION' - with patch('plann.interactive._editor', new=passthrough) as _editor: - _mass_reprioritize([todo1, todo2, todo3]) - with patch('plann.interactive._editor', new=passthrough) as _editor: - _mass_interactive_edit([todo1, todo2, todo3], default='complete') - for todo in (todo1, todo2, todo3, todo4, todo5): - todo.load() - assert todo.icalendar_component['STATUS'] == 'COMPLETED' - - ## check that duration won't change if due is moved - ## TODO: run this for dtend and dtstart on an event as well - command_edit(todo1, 'set due=2012-12-19 19:15:00', interactive=False) - assert todo1.icalendar_component['DUE'].dt > todo1.icalendar_component['DTSTART'].dt + ## We create two tasks todo1 and todo2, todo2 being a child of todo1 + todo1 = _add_todo(ctx, summary=['make plann good'], set_due='2012-12-20 23:15:00', set_dtstart='2012-12-20 22:15:00', set_uid='todo1', set_categories='plann,keyboard') + uid1 = str(todo1.icalendar_component['uid']) + todo2 = _add_todo(ctx, summary=['fix some bugs in plann'], set_parent=[uid1], set_due='2012-12-21 23:15:00', set_dtstart='2012-12-21 22:15:00', set_uid='todo2', set_categories=('plann', 'keyboard')) + uid2 = str(todo2.icalendar_component['uid']) + + ## Selecting the tasks should yield ... 2 (but only one if skip_children or skip_parents is used) + _select(ctx, todo=True) + assert len(ctx.obj['objs'])==2 + _select(ctx, todo=True, skip_children=True) + assert len(ctx.obj['objs'])==1 + _select(ctx, todo=True, skip_parents=True) + assert len(ctx.obj['objs'])==1 + _select(ctx, summary='make plann good') + ## this breaks with xandikos + #assert len(ctx.obj['objs'])==1 + _select(ctx, summary='make plann good', todo=True) + assert len(ctx.obj['objs'])==1 + _select(ctx, event=True) + assert len(ctx.obj['objs'])==0 + + ## Journal tests + _add_journal(ctx, summary=['bought new keyboard'], set_dtstart='2012-12-20') + _select(ctx, journal=True) + assert len(ctx.obj['objs'])==1 + _select(ctx, todo=True) + assert len(ctx.obj['objs'])==2 + _select(ctx, event=True) + assert len(ctx.obj['objs'])==0 + + ## assert that relations are as expected + todo1.load() + todo2.load() + assert(dag(todo1, 'CHILD') == {uid2: {}}) + assert(dag(todo2, 'PARENT') == {uid1: {}}) + assert(not _adjust_ical_relations(todo1, {'CHILD': {uid2}, 'PARENT': set()})) + assert(not _adjust_ical_relations(todo2, {'PARENT': {uid1}, 'CHILD': set()})) + + _select(ctx, todo=True) + ## This should return a list of human-readable strings, with an indented child. + list_td = _list(ctx.obj['objs'], top_down=True, echo=False) + assert(list_td[0][0] != ' ') + assert(list_td[1][0] == ' ') + assert('good' in list_td[0]) + + ## This should return a list of human-readable strings, with an indented parent. + list_bu = _list(ctx.obj['objs'], bottom_up=True, echo=False) + assert(list_bu[0][0] != ' ') + assert(list_bu[1][0] == ' ') + assert('good' in list_bu[1]) + + ## Test that the calendar_name template attribute works + list_cal = _list(ctx.obj['objs'], template="{calendar_name}", echo=False) + assert list_cal[0] == todo1.parent.get_display_name() + assert list_cal[1] == todo2.parent.get_display_name() + + ## panic planning, timeline_suggestion. + ## We have two tasks in the calendar, each with one hour duration + timeline = timeline_suggestion(ctx, hours_per_day=24) + assert(timeline.count() == 2) + foo = timeline.get(datetime_(year=2012, month=12, day=20, hour=22, minute=35)) + assert foo['begin'] == datetime_(year=2012, month=12, day=20, hour=22, minute=15) + assert foo['end'] == datetime_(year=2012, month=12, day=20, hour=23, minute=15) + + ## panic planning, timeline_suggestion with timeline_end + ## Those two tasks should be neatly stacked up + assert len(ctx.obj['objs'])==2 + timeline = timeline_suggestion(ctx, timeline_end=datetime_(year=2011, month=11, day=11, hour=11, minute=11), hours_per_day=24) + assert(timeline.count() == 2) + foo = timeline.get(datetime_(year=2011, month=11, day=11, hour=11, minute=10)) + assert foo['begin'] == datetime_(year=2011, month=11, day=11, hour=10, minute=11) + assert foo['end'] == datetime_(year=2011, month=11, day=11, hour=11, minute=11) + foo = timeline.get(datetime_(year=2011, month=11, day=11, hour=10, minute=10)) + assert foo['begin'] == datetime_(year=2011, month=11, day=11, hour=9, minute=11) + assert foo['end'] == datetime_(year=2011, month=11, day=11, hour=10, minute=11) + + ## fix_timeline feature + ## This will create events that are children of the tasks. + ## (Those events contains a suggested timeslot for doing the task + ## Task contains due date (may be a hard limit) and duration + with patch('plann.panic_planning._now', new=lambda: datetime_(2010,10,9,23,33,33)) as _now: + _check_for_panic(ctx, timeline_start='2010-10-10', timeline_end=datetime_(year=2010, month=11, day=11, hour=11, minute=11), hours_per_day=24, fix_timeline=True, include_all_events=True) + + ## Two events should be created by now + _select(ctx, event=True) + assert len(ctx.obj['objs'])==2 + + ## Re-running it should be a noop + _select(ctx, todo=True) + with patch('plann.panic_planning._now', new=lambda: datetime_(2010,10,9,23,33,33)) as _now: + _check_for_panic(ctx, timeline_start='2010-10-10', timeline_end=datetime_(year=2010, month=11, day=11, hour=11, minute=11), hours_per_day=24, fix_timeline=True, include_all_events=True) + _select(ctx, event=True) + assert len(ctx.obj['objs'])==2 + + ## We now have one task with one subtask, and both tasks have two children + ## events. Let's find the uids + e1 = [x for x in ctx.obj['objs'] if x.icalendar_component['summary'] == 'make plann good'][0] + e2 = [x for x in ctx.obj['objs'] if x.icalendar_component['summary'] == 'fix some bugs in plann'][0] + uide1 = str(e1.icalendar_component['uid']) + str(e1.icalendar_component['uid']) + + ## Testing the _adjust_relations + todo1.load() + ## todo1 has children todo2 and e1. _adjust_relations over those should be a noop + with patch.object(todo1, 'save') as _t1save: + with patch.object(todo2, 'save') as _t2save: + with patch.object(e1, 'save') as _e1save: + with patch.object(e2, 'save') as _e2save: + _adjust_relations(parent=todo1, children=[todo2, e1]) + assert not any(x.call_count for x in (_t1save, _t2save, _e1save, _e2save)) + + ## Testing to remove todo2 as child of todo1 + _adjust_relations(parent=todo1, children=[e1]) + todo1.load() + todo2.load() + e1.load() + ## _adjust_ical_relations returns a dict if the object was changed + assert(not _adjust_ical_relations(todo1, {'CHILD': {uide1}})) + assert(not _adjust_ical_relations(todo2, {'PARENT': set()})) + + ## Testing interactive fix timeline, and checking that past events are ignored in the + ## panic planning. + ## We were busy with other things at the scheduled time. Let's try to reschedule ... + with patch('plann.commands._editor', new=passthrough) as _editor: + with patch('plann.panic_planning._now', new=lambda: datetime_(2010,12,9,23,33,33)) as _now: + _select(ctx, todo=True) + _check_for_panic(ctx, timeline_start='2010-10-10', timeline_end=datetime_(year=2011, month=11, day=11, hour=11, minute=11), hours_per_day=24, print_timeline=False, interactive_fix_timeline=True, include_all_events=True) + _select(ctx, event=True) + assert len(ctx.obj['objs'])==4 + + ## Reset ... let's connect todo1 and todo2 again, and remove events + ## Reload to get fresh ETags — _check_for_panic modified these objects on the server + todo1.load() + todo2.load() + _adjust_relations(todo1, {todo2}) + _adjust_relations(todo2, set()) + todo1.load() + todo2.load() + assert(not _adjust_ical_relations(todo1, {'CHILD': {uid2}, 'PARENT': set()})) + assert(not _adjust_ical_relations(todo2, {'PARENT': {uid1}, 'CHILD': set()})) + + ## Testing the interactive relation edit - if doing nothing in the + ## editor, the algorithm should change nothing. + _select(ctx, todo=True) + cal_pre_interactive_relation_edit = "\n".join([x.data for x in ctx.obj['objs']]) + with patch('plann.interactive._editor', new=passthrough) as _editor: + _interactive_relation_edit(ctx.obj['objs']) + + for obj in ctx.obj['objs']: + obj.load() + cal_post_interactive_relation_edit = "\n".join([x.data for x in ctx.obj['objs']]) + assert cal_post_interactive_relation_edit == cal_pre_interactive_relation_edit + + ## Let's add some more tasks + todo3 = _add_todo(ctx, summary=['fix some more features in plann'], set_parent=[uid1], set_due='2012-12-21 23:15:00', set_dtstart='2012-12-21 22:15:00', set_uid='todo3', set_category='plann') + todo4 = _add_todo(ctx, summary=['make plann even better'], set_parent=[uid1], set_due='2012-12-21 23:15:00', set_dtstart='2012-12-21 22:15:00', set_uid='todo4') + todo5 = _add_todo(ctx, summary=['use plann on a daily basis to find more bugs and missing features'], set_parent=[uid1], set_due='2012-12-21 23:15:00', set_dtstart='2012-12-21 22:15:00', set_uid='todo5') + + ## TODO: testing of set_category / set_categories should be moved to a separate test in test_lib / test_command / test_cli + assert(set([str(x) for x in todo1.icalendar_component['CATEGORIES'].cats]) == {'plann', 'keyboard'}) + assert(set([str(x) for x in todo2.icalendar_component['CATEGORIES'].cats]) == {'plann', 'keyboard'}) + assert(set([str(x) for x in todo3.icalendar_component['CATEGORIES'].cats]) == {'plann'}) + + uid3 = str(todo3.icalendar_component['uid']) + uid4 = str(todo4.icalendar_component['uid']) + uid5 = str(todo5.icalendar_component['uid']) + + todo1.load() + assert(dag(todo1, 'CHILD') == {uid2: {}, uid3: {}, uid4: {}, uid5: {}}) + assert(dag(todo4, 'PARENT') == {uid1: {}}) + + ## Let's add some incremental indentation to the last lines. + ## This one will leave todo1 with two children, then the next lines are indented more. + def add_indent(text): + return f"{uid1}: todo1\n {uid2}: todo2\n {uid3}: todo3\n {uid4}: todo4\n {uid5}: todo5" + with patch('plann.interactive._editor', new=add_indent) as _editor: + _interactive_relation_edit([todo1]) + + ## Reload the object list + for obj in (todo1, todo2, todo3, todo4, todo5): + obj.load() + + assert(dag(todo1, 'CHILD') == {uid2: {}, uid3: {uid4: {uid5: {}}}}) + assert(dag(todo2, 'PARENT') == {uid1: {}}) + assert(dag(todo5, 'PARENT') == {uid4: {uid3: {uid1: {}}}}) + + ## This should not throw one into the debugger + list_td = _list(ctx.obj['objs'], top_down=True, echo=False) + + def remove_parent(input): + return f"{uid2}: todo2\n{uid3}: todo3\n {uid4}: todo4\n {uid5}: todo5" + + with patch('plann.interactive._editor', new=remove_parent) as _editor: + _interactive_relation_edit([todo1]) + + assert(dag(todo1, 'CHILD') == {}) + assert(dag(todo1, 'PARENT') == {}) + assert(dag(todo2, 'PARENT') == {}) + assert(dag(todo2, 'CHILD') == {}) + assert(dag(todo3, 'CHILD') == {uid4: {uid5: {}}}) + assert(dag(todo5, 'PARENT') == {uid4: {uid3: {}}}) + + ## This should not throw one into the debugger + #list_td = _list(ctx.obj['objs'], top_down=True, echo=False) + + def gen_prompt(ret): + def prompt(*largs, **kwargs): + return ret + return prompt + + ## testing _interactive_edit + with patch('click.echo') as _echo: + ## ignore should do nothing + with patch('click.prompt', new=gen_prompt('ignore')): + _interactive_edit(todo1) + ## no asserts needed + ## complete should complete it + with patch('click.prompt', new=gen_prompt('complete')): + _interactive_edit(todo1) + todo1.load() + assert(todo1.icalendar_component['STATUS']=='COMPLETED') + todo1.uncomplete() + with patch('click.prompt', new=gen_prompt('postpone 1d')): + _interactive_edit(todo1) + todo1.load() + assert(todo1.icalendar_component['DUE'].dt > datetime_(year=2023, month=9, day=19, hour=15)) + with patch('click.prompt', new=gen_prompt('set categories=foo')): + _interactive_edit(todo1) + todo1.load() + assert(set([str(x) for x in todo1.icalendar_component['CATEGORIES'].cats]) == {'foo'}) + with patch('click.prompt', new=gen_prompt('set category=bar')): + _interactive_edit(todo1) + todo1.load() + assert(set([str(x) for x in todo1.icalendar_component['CATEGORIES'].cats]) == {'foo', 'bar'}) + ## TODO: part, split, family + ## TODO: cancel, + + ## testing mass interactive edit and mass interactive reprioritize + ## with 0 objects, it would earlier raise an error. + with patch('plann.interactive._editor', new=passthrough) as _editor: + _mass_interactive_edit([], default='complete') + for todo in (todo1, todo2, todo3, todo4, todo5): + todo.load() + assert todo.icalendar_component['STATUS'] == 'NEEDS-ACTION' + with patch('plann.interactive._editor', new=passthrough) as _editor: + _mass_reprioritize([todo1, todo2, todo3]) + with patch('plann.interactive._editor', new=passthrough) as _editor: + _mass_interactive_edit([todo1, todo2, todo3], default='complete') + for todo in (todo1, todo2, todo3, todo4, todo5): + todo.load() + assert todo.icalendar_component['STATUS'] == 'COMPLETED' + + ## check that duration won't change if due is moved + ## TODO: run this for dtend and dtstart on an event as well + command_edit(todo1, 'set due=2012-12-19 19:15:00', interactive=False) + assert todo1.icalendar_component['DUE'].dt > todo1.icalendar_component['DTSTART'].dt ## TODO: ## Things to be tested: lib._procrastinate, cli._select, cli._cats, cli._list, cli._interactive_edit, cli._set_something, cli._interactive_ical_edit, cli._edit, cli._check_for_panic, _add_todo, _agenda, _check_due, From 1e949eaa31cc354816034269800bb45d13ab68f2 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Thu, 12 Nov 2026 06:05:24 +0100 Subject: [PATCH 2/3] feat: interactive check-pinned, review past pins Goes through last week's events pinning a task to the calendar and asks if the task was done. Yes completes the task (with subtasks), confirms the event and offers to cancel its other pins; no cancels the event; partially runs the split flow. Confirmed events are not asked about again, which matters for recurring tasks. It runs first in manage-tasks, replacing the TODO there. The test server's event loop is now closed on stop, so two functional tests can run in one session. Prompt: So yesterday and today I've had a calendar with tasks "pinned" to the calendar through events. Did I make any `interactive`-command for going through those events and answering y/n on whether the tasks were done or not? Followup-Prompt: Yes, I need that. [an interactive command to go through past pinning events and ask whether each task was done] Followup-Prompt: go for it, including fixing the dead functional test. Followup-Prompt: Fix the commit, I'll push manually [add the PR link to the CHANGELOG entry, and fix the typo in the recorded prompt as in the first commit] Assisted-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++ TASK_MANAGEMENT.md | 8 ++- plann/cli.py | 32 ++++++--- plann/commands.py | 86 ++++++++++++++++++++++- plann/interactive.py | 3 + tests/test_functional.py | 148 ++++++++++++++++++++++++++++++++++++++- 6 files changed, 264 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e461a46..083b4ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ and I do try to adhere to [Semantic Versioning](https://semver.org/spec/v2.0.0.h ## Unreleased +### Added + +* `plann interactive check-pinned` goes through the past week's events pinning tasks to the calendar (as made by the panic planning in `interactive manage-tasks`) and asks if each task was done. A done task is completed (with its subtasks), and its event is marked as confirmed; its other pins may be cancelled. If not done, the event is cancelled. It is also run first in `interactive manage-tasks`. https://github.com/pycalendar/plann/pull/89 + ### Fixed * `interactive manage-tasks`: a parent task postponed together with its child could be postponed once more in its own turn, and with caldav 3.6 this crashed with a "412 Precondition Failed" error. It is now skipped, with a note. A task changed on the server in the meantime is reloaded before it is postponed. diff --git a/TASK_MANAGEMENT.md b/TASK_MANAGEMENT.md index 9df5ceb..c340a21 100644 --- a/TASK_MANAGEMENT.md +++ b/TASK_MANAGEMENT.md @@ -371,13 +371,15 @@ There is another interactive edit function, `plann select --summary 'clean' edit `plann interactive manage-tasks` will do the following: +* `plann interactive check-pinned` - asks if the tasks pinned to the calendar through events the last week were done * `plann interactive split-huge-tasks` -* `plan interactive split-high-pri-tasks` +* `plann interactive split-high-pri-tasks` * `plann interactive set-task-attribs` -* `plann agenda` * `plann interactive dismiss-panic --hours-per-day=24` +* `plann interactive dismiss-panic --hours-per-day=14` * `plann interactive check-due` -* `plann interactive dismiss-panic --hours-per-day=8` +* pins tasks to the calendar for the next five days, through the panic planning +* `plann agenda` ### Completing tasks diff --git a/plann/cli.py b/plann/cli.py index 3c9d693..67a20ac 100755 --- a/plann/cli.py +++ b/plann/cli.py @@ -32,6 +32,7 @@ _cats, _check_due, _check_for_panic, + _check_pinned, _dismiss_panic, _edit, _select, @@ -666,8 +667,8 @@ def interactive(ctx): experimenting with his own task list and testing out daily procedures, hence it's also quite optimized towards whatever work-flows that seems to work out for the maintainer of the - plann. Things are changed rapidly without warnings and the - interactive stuff is not covered by any test code whatsoever. + plann. Things are changed rapidly without warnings and most of + the interactive stuff is not covered by test code. """ @@ -676,18 +677,17 @@ def interactive(ctx): def manage_tasks(ctx): """ This goes through the daily task management procedures: - * set-task-attribs - * agenda - * check-due + * check-pinned * split-huge-tasks * split-high-pri-tasks + * set-task-attribs * dismiss-panic - * interactive-fix-panic + * check-due + * pin tasks to the calendar (check-for-panic, interactive fix of the timeline) + * agenda """ - ## TODO: pinned tasks for the previous week. - ## * Go through the events ... for each of them, ask if it was done or not - ## * If the event was done, mark the task as completed - ## * If the event wasn't done, mark the event as cancelled + click.echo("Checking if the tasks pinned to your calendar the last week were done") + _check_pinned(ctx) click.echo("Checking if there are any huge tasks that may need to be split") _split_huge_tasks(ctx) click.echo("Checking if there are any high-priority tasks ... it's adviceable to split them up into subtasks with a shorter due-date") @@ -715,6 +715,18 @@ def check_due(ctx, limit, lookahead): """ return _check_due(ctx, limit, lookahead) +@interactive.command() +@click.option('--lookback', help='how far back to look for events pinning tasks to the calendar', default='7d') +@click.pass_context +def check_pinned(ctx, lookback): + """ + Go through past events pinning tasks to the calendar, and ask if the tasks were done + + Done tasks are completed, and the event is confirmed. If a task + was not done, the event is cancelled. + """ + return _check_pinned(ctx, lookback) + @interactive.command() @click.option('--hours-per-day', help='how many hours per day you expect to be able to dedicate to those tasks/events', default=4) @click.option('--lookahead', help='timespan to investigate', default='60d') diff --git a/plann/commands.py b/plann/commands.py index 68157f7..cad8e0d 100644 --- a/plann/commands.py +++ b/plann/commands.py @@ -7,6 +7,7 @@ ## TODO: can we remove the click-dependency? import click +from caldav.lib.error import ETagMismatchError, PutError, ScheduleTagMismatchError from icalendar_searcher import Searcher from plann.interactive import ( @@ -192,11 +193,11 @@ def __select(ctx, extend_objects=False, all=None, uid=[], abort_on_missing_uid=N for obj in ctx.obj['objs']: if isinstance(obj, caldav.Event): if obj.icalendar_component.get('STATUS', '') != 'CANCELLED': - parents = _relships_by_type(obj, 'PARENT').get('PARENT',[]) - if any(x for x in parents if isinstance(x, caldav.Todo) and x.icalendar_component.get('STATUS', 'NEEDS-ACTION') == 'NEEDS-ACTION') == pinned_tasks: + open_parents = _pinned_task_parents(obj) + if bool(open_parents) == pinned_tasks: if kwargs_.get('todo'): ## TODO: special handling for recurring tasks - ret_objs.extend([x for x in parents if isinstance(x, caldav.Todo) and x.icalendar_component.get('STATUS', 'NEEDS-ACTION') == 'NEEDS-ACTION']) + ret_objs.extend(open_parents) else: ret_objs.append(obj) if isinstance(obj, caldav.Todo) and not pinned_tasks: @@ -477,6 +478,7 @@ def _add_event(ctx, timespec, **kwargs): (dtstart, dtend) = parse_timespec(timespec, for_storage=True) event = cal.save_event(dtstart=dtstart, dtend=dtend, **ctx.obj['set_args'], no_overwrite=True) click.echo(f"uid={event.id}") + return event def _agenda(ctx): start = datetime.datetime.now() @@ -501,6 +503,84 @@ def _check_due(ctx, limit=16, lookahead='24h'): continue _interactive_edit(obj) +def _pinned_task_parents(event): + """the open tasks an event pins to the calendar""" + return [x for x in _relships_by_type(event, 'PARENT').get('PARENT', []) + if isinstance(x, caldav.Todo) and x.icalendar_component.get('STATUS', 'NEEDS-ACTION') == 'NEEDS-ACTION'] + +def _set_event_status(event, status): + event.icalendar_component['STATUS'] = status + event.save() + +def _cancel_other_pins(task, event): + """offer to cancel the other events pinning a completed task to the calendar""" + if task.icalendar_component.get('RRULE'): + ## the next recurrence is still to be done + return + pins = [x for x in _relships_by_type(task, 'CHILD').get('CHILD', []) + if isinstance(x, caldav.Event) + and x.icalendar_component.get('STATUS', '') not in ('CANCELLED', 'CONFIRMED') + and x.icalendar_component['UID'] != event.icalendar_component['UID']] + if pins and click.confirm(f"Cancel {len(pins)} other pinned event(s) for '{_summary(task)}'?"): + for pin in pins: + _set_event_status(pin, 'CANCELLED') + +def _check_pinned(ctx, lookback='7d'): + """Go through past events pinning a task to the calendar, and ask + if the task was done. A done task is completed, the event is + marked as CONFIRMED. If the task wasn't done, the event is + cancelled. + """ + now = _now() + _select(ctx=ctx, event=True, start=now-parse_add_dur(None, lookback), end=now, pinned_tasks=True) + ## sorted on the timestamps, events may be stored in different timezones + for event in sorted(ctx.obj['objs'], key=lambda x: _ensure_ts(x.icalendar_component.get('DTSTART'))): + comp = event.icalendar_component + if comp.get('STATUS', '') == 'CONFIRMED': + ## already dealt with + continue + dtstart = _ensure_ts(comp.get('DTSTART')).astimezone(tz.implicit_timezone) + dtend = _ensure_ts(comp.get('DTEND')).astimezone(tz.implicit_timezone) if comp.get('DTEND') else dtstart + event.get_duration() + if dtend > now: + ## still ongoing + continue + tasks = _pinned_task_parents(event) + if not tasks: + ## e.g. completed when answering for an earlier pin + continue + end_format = '%H:%M' if dtend.date() == dtstart.date() else '%F %H:%M' + click.echo(f"{dtstart:%F %H:%M} - {dtend:{end_format}}: {_summary(event)}") + while True: + answer = click.prompt("Was it done? y(es) / n(o) / p(artially) / e(dit task) / s(kip)", default='s')[:1].lower() + if answer == 'e': + for task in tasks: + _interactive_edit(task) + tasks = _pinned_task_parents(event) + if not tasks: + break + continue + break + try: + if answer == 'y': + ## one cache for all, so a subtask pinned along with its parent is completed once + cache = _RelativeCache() + cache.add_objects(tasks) + completed = set() + for task in tasks: + _complete(task, err_callback=click.echo, cache=cache, _seen=completed) + _set_event_status(event, 'CONFIRMED') + for task in tasks: + _cancel_other_pins(task, event) + elif answer == 'n': + _set_event_status(event, 'CANCELLED') + elif answer == 'p': + split = [interactive_split_task(task, partially_complete=True, too_big=False) for task in tasks] + if any(split): + _set_event_status(event, 'CONFIRMED') + except (ETagMismatchError, ScheduleTagMismatchError, PutError) as e: + ## i.e. 423 Locked from a busy server - don't abort the rest of the list + click.echo(f"Could not save the changes for '{_summary(event)}': {e}") + def _dismiss_panic(ctx, hours_per_day, lookahead='60d'): ## TODO: fetch both events and tasks lookahead=f"+{lookahead}" diff --git a/plann/interactive.py b/plann/interactive.py index 16c86ca..6bbbe49 100644 --- a/plann/interactive.py +++ b/plann/interactive.py @@ -311,6 +311,7 @@ def _mass_interactive_edit(objs, default='ignore'): _command_line_edit(line, interactive=True, calendar=objs[0].parent, complete_cache=complete_cache, completed=completed) def interactive_split_task(obj, partially_complete=False, too_big=True): + """returns True if any subtask was split out""" comp = obj.icalendar_component summary = _summary(obj) estimate = obj.get_duration() @@ -350,6 +351,8 @@ def interactive_split_task(obj, partially_complete=False, too_big=True): if postpone not in ('0h', '0'): ## TODO: regexp? _procrastinate([obj], postpone, check_dependent='interactive', err_callback=click.echo, confirm_callback=click.confirm) obj.save() + return True + return False def _editor(sometext): with tempfile.NamedTemporaryFile(mode='w', encoding='UTF-8', delete=False) as tmpfile: diff --git a/tests/test_functional.py b/tests/test_functional.py index e22d5a8..d9d0091 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -5,19 +5,24 @@ ## TODO: tests with multiple source calendars. Some of the interactive edit-through-editor functions will probably break. import asyncio +import datetime import os +import re import tempfile import threading import time from unittest.mock import MagicMock, patch +from zoneinfo import ZoneInfo import aiohttp import aiohttp.web import caldav import niquests as requests +from caldav.lib.error import PutError from xandikos.web import SingleUserFilesystemBackend, XandikosApp from plann.cli import _add_journal, _add_todo, _check_for_panic, _list, _select +from plann.commands import _add_event, _check_pinned from plann.interactive import ( _interactive_edit, _interactive_relation_edit, @@ -25,8 +30,9 @@ _mass_reprioritize, command_edit, ) -from plann.lib import _adjust_ical_relations, _adjust_relations +from plann.lib import _adjust_ical_relations, _adjust_relations, _summary from plann.panic_planning import timeline_suggestion +from plann.timespec import _now, tz from tests.test_panic import datetime_ @@ -148,6 +154,8 @@ def silly_request(): time.sleep(0.05) i += 1 assert i < 100 + server_params['xandikos_app_loop'].close() + asyncio.set_event_loop(None) server_params['serverdir'].__exit__(None, None, None) @@ -452,5 +460,143 @@ def prompt(*largs, **kwargs): command_edit(todo1, 'set due=2012-12-19 19:15:00', interactive=False) assert todo1.icalendar_component['DUE'].dt > todo1.icalendar_component['DTSTART'].dt +def test_check_pinned(): + conn_details = start_xandikos_server() + try: + with caldav.get_calendars( + url=conn_details['caldav_url'], + username=conn_details['caldav_user'], + password=conn_details['caldav_password'], + raise_errors=True, + ) as calendars: + _run_check_pinned_tests(calendars) + finally: + stop_xandikos_server(conn_details) + + +def _run_check_pinned_tests(calendars): + ctx = MagicMock() + ctx.obj = dict() + ctx.obj['calendars'] = list(calendars) + hour = datetime.timedelta(hours=1) + now = _now() + ## midnight two days ago - all pins are well in the past, whatever the time of day + base = (now - 48*hour).replace(hour=0, minute=0, second=0) + + def task(summary, parent=None): + kwargs = {'set_parent': [parent]} if parent else {} + return _add_todo(ctx, summary=[summary], set_due=f"{now + 48*hour:%F %H:%M:%S}", set_duration='1h', set_uid=summary.replace(' ', '-'), **kwargs) + + def uid(obj): + return str(obj.icalendar_component['UID']) + + def pin(todo, begin, status='TENTATIVE', parents=None): + return _add_event(ctx, summary=_summary(todo), timespec=(begin, begin+hour), set_status=status, set_parent=parents or [uid(todo)]) + + def status(obj): + obj.load() + return obj.icalendar_component['STATUS'] + + done = task('write report') + not_done = task('fix bike') + skipped = task('call mom') + reviewed = task('water plants') + parent = task('paint house') + child = task('buy paint', parent=uid(parent)) + partial = task('tidy garage') + early = task('early') + late = task('late') + + ## two past pins of the same task - the first one is asked about + ev_done_first = pin(done, base) + ev_done = pin(done, base + 2*hour) + ev_not_done = pin(not_done, base + 4*hour) + ev_skipped = pin(skipped, base + 6*hour) + ## already answered on an earlier run + ev_reviewed = pin(reviewed, base + 8*hour, status='CONFIRMED') + ## one event pinning both a task and its subtask + pin(parent, base + 10*hour, parents=[uid(parent), uid(child)]) + ev_partial = pin(partial, base + 12*hour) + ## the day before, stored in different timezones. "early" is 09:00 + ## UTC, but its wall clock time 18:00 sorts after the 10:00 of "late" + day = (base - 24*hour).date() + pin(early, datetime.datetime(day.year, day.month, day.day, 18, tzinfo=ZoneInfo('Asia/Tokyo'))) + pin(late, datetime.datetime(day.year, day.month, day.day, 10, tzinfo=ZoneInfo('UTC'))) + ## pinned for later, should not be asked about + ev_future = pin(done, now + 24*hour) + ev_not_done_future = pin(not_done, now + 24*hour) + ## ongoing, should not be asked about + ev_ongoing = pin(not_done, now - hour/2) + ## pinned too long ago + pin(skipped, now - 30*24*hour) + + answers = {'write report': 'y', 'fix bike': 'n', 'paint house': 'y', 'tidy garage': 'p'} + asked = [] + echoed = [] + def prompt(*largs, **kwargs): + question = [x for x in echoed if re.match(r'\d{4}-\d\d-\d\d \d\d:\d\d - ', x)][-1] + asked.append(question) + return answers.get(question.split(': ', 1)[1], 's') + + completed = [] + orig_complete = caldav.Todo.complete + def complete(self, *largs, **kwargs): + completed.append(uid(self)) + return orig_complete(self, *largs, **kwargs) + + with patch('plann.commands.click.echo', new=lambda x='', **kw: echoed.append(str(x))): + with patch('plann.commands.click.prompt', new=prompt): + ## the user declines to split out the partially done work + with patch('click.confirm', side_effect=lambda text, **kw: 'working on this' not in text) as confirm: + with patch.object(caldav.Todo, 'complete', new=complete): + _check_pinned(ctx, lookback='7d') + summaries = [x.split(': ', 1)[1] for x in asked] + ## chronologically, whatever timezone the events are stored in + assert summaries == ['early', 'late', 'write report', 'fix bike', 'call mom', 'paint house', 'tidy garage'] + ## shown in local time + tokyo_local = datetime.datetime(day.year, day.month, day.day, 18, tzinfo=ZoneInfo('Asia/Tokyo')).astimezone(tz.implicit_timezone) + assert asked[0].startswith(f"{tokyo_local:%F %H:%M} - ") + ## asked about the other pins of the completed task, and about the split + assert [x.args[0].startswith('Cancel 2 other pinned') for x in confirm.call_args_list] == [True, False] + + assert status(done) == 'COMPLETED' + assert status(ev_done_first) == 'CONFIRMED' + assert status(ev_done) == 'CANCELLED' + assert status(ev_future) == 'CANCELLED' + assert status(not_done) == 'NEEDS-ACTION' + assert status(ev_not_done) == 'CANCELLED' + assert status(ev_not_done_future) == 'TENTATIVE' + assert status(ev_ongoing) == 'TENTATIVE' + assert status(skipped) == 'NEEDS-ACTION' + assert status(ev_skipped) == 'TENTATIVE' + assert status(ev_reviewed) == 'CONFIRMED' + ## the subtask is completed along with its parent, and only once + assert status(parent) == 'COMPLETED' + assert status(child) == 'COMPLETED' + assert completed.count(uid(child)) == 1 + ## nothing was split out, so it is asked about again + assert status(partial) == 'NEEDS-ACTION' + assert status(ev_partial) == 'TENTATIVE' + + ## a second run only asks about the skipped ones + answers.clear() + asked.clear() + with patch('plann.commands.click.echo', new=lambda x='', **kw: echoed.append(str(x))): + with patch('plann.commands.click.prompt', new=prompt): + with patch('click.confirm', return_value=False) as confirm: + _check_pinned(ctx, lookback='7d') + assert [x.split(': ', 1)[1] for x in asked] == ['early', 'late', 'call mom', 'tidy garage'] + assert not confirm.called + + ## a server refusing to save does not stop the rest of the questions + answers.update({'early': 'n', 'late': 'n'}) + asked.clear() + with patch('plann.commands.click.echo', new=lambda x='', **kw: echoed.append(str(x))): + with patch('plann.commands.click.prompt', new=prompt): + with patch('plann.commands._set_event_status', side_effect=PutError('423 Locked')): + _check_pinned(ctx, lookback='7d') + assert [x.split(': ', 1)[1] for x in asked] == ['early', 'late', 'call mom', 'tidy garage'] + assert any('423 Locked' in x for x in echoed) + ## TODO: ## Things to be tested: lib._procrastinate, cli._select, cli._cats, cli._list, cli._interactive_edit, cli._set_something, cli._interactive_ical_edit, cli._edit, cli._check_for_panic, _add_todo, _agenda, _check_due, From a4575e72959f081948d3043ff82dc83211dd53cd Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Thu, 12 Nov 2026 14:35:22 +0100 Subject: [PATCH 3/3] fix: check-pinned edge cases from review Other pins of a completed task are only offered for cancelling when they pin no other open task, and before the event is confirmed, so a failed save no longer leaves them behind. A task closed through the edit prompt now confirms (or cancels) its event. A task or event deleted on the server meanwhile no longer stops the run. Tests cover recurring tasks and a successful partial split. Prompt: [fix issues found during the review process] Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox --- plann/cli.py | 2 +- plann/commands.py | 30 ++++++++--- tests/test_functional.py | 110 ++++++++++++++++++++++++++++++--------- 3 files changed, 108 insertions(+), 34 deletions(-) diff --git a/plann/cli.py b/plann/cli.py index 67a20ac..8f5773c 100755 --- a/plann/cli.py +++ b/plann/cli.py @@ -681,7 +681,7 @@ def manage_tasks(ctx): * split-huge-tasks * split-high-pri-tasks * set-task-attribs - * dismiss-panic + * dismiss-panic, with 24 and then 14 hours per day * check-due * pin tasks to the calendar (check-for-panic, interactive fix of the timeline) * agenda diff --git a/plann/commands.py b/plann/commands.py index cad8e0d..386d1e2 100644 --- a/plann/commands.py +++ b/plann/commands.py @@ -7,7 +7,7 @@ ## TODO: can we remove the click-dependency? import click -from caldav.lib.error import ETagMismatchError, PutError, ScheduleTagMismatchError +from caldav.lib.error import ETagMismatchError, NotFoundError, PutError, ScheduleTagMismatchError from icalendar_searcher import Searcher from plann.interactive import ( @@ -508,22 +508,30 @@ def _pinned_task_parents(event): return [x for x in _relships_by_type(event, 'PARENT').get('PARENT', []) if isinstance(x, caldav.Todo) and x.icalendar_component.get('STATUS', 'NEEDS-ACTION') == 'NEEDS-ACTION'] +## errors that should not stop the questions about the remaining events +_save_errors = (ETagMismatchError, ScheduleTagMismatchError, PutError, NotFoundError) + def _set_event_status(event, status): event.icalendar_component['STATUS'] = status event.save() def _cancel_other_pins(task, event): - """offer to cancel the other events pinning a completed task to the calendar""" + """offer to cancel the other events pinning a completed task to the + calendar - unless they also pin some task that is still open""" if task.icalendar_component.get('RRULE'): ## the next recurrence is still to be done return pins = [x for x in _relships_by_type(task, 'CHILD').get('CHILD', []) if isinstance(x, caldav.Event) and x.icalendar_component.get('STATUS', '') not in ('CANCELLED', 'CONFIRMED') - and x.icalendar_component['UID'] != event.icalendar_component['UID']] + and x.icalendar_component['UID'] != event.icalendar_component['UID'] + and not _pinned_task_parents(x)] if pins and click.confirm(f"Cancel {len(pins)} other pinned event(s) for '{_summary(task)}'?"): for pin in pins: - _set_event_status(pin, 'CANCELLED') + try: + _set_event_status(pin, 'CANCELLED') + except _save_errors as e: + click.echo(f"Could not cancel '{_summary(pin)}': {e}") def _check_pinned(ctx, lookback='7d'): """Go through past events pinning a task to the calendar, and ask @@ -550,34 +558,42 @@ def _check_pinned(ctx, lookback='7d'): continue end_format = '%H:%M' if dtend.date() == dtstart.date() else '%F %H:%M' click.echo(f"{dtstart:%F %H:%M} - {dtend:{end_format}}: {_summary(event)}") + edited = [] while True: answer = click.prompt("Was it done? y(es) / n(o) / p(artially) / e(dit task) / s(kip)", default='s')[:1].lower() if answer == 'e': for task in tasks: _interactive_edit(task) + edited.extend(tasks) tasks = _pinned_task_parents(event) if not tasks: break continue break try: - if answer == 'y': + if answer == 'e': + ## every task was closed in the edit + done = any(x.icalendar_component.get('STATUS') == 'COMPLETED' for x in edited) + _set_event_status(event, 'CONFIRMED' if done else 'CANCELLED') + elif answer == 'y': ## one cache for all, so a subtask pinned along with its parent is completed once cache = _RelativeCache() cache.add_objects(tasks) completed = set() for task in tasks: _complete(task, err_callback=click.echo, cache=cache, _seen=completed) - _set_event_status(event, 'CONFIRMED') + ## before confirming the event: if that fails, the event + ## is not asked about again, as the task is done for task in tasks: _cancel_other_pins(task, event) + _set_event_status(event, 'CONFIRMED') elif answer == 'n': _set_event_status(event, 'CANCELLED') elif answer == 'p': split = [interactive_split_task(task, partially_complete=True, too_big=False) for task in tasks] if any(split): _set_event_status(event, 'CONFIRMED') - except (ETagMismatchError, ScheduleTagMismatchError, PutError) as e: + except _save_errors as e: ## i.e. 423 Locked from a busy server - don't abort the rest of the list click.echo(f"Could not save the changes for '{_summary(event)}': {e}") diff --git a/tests/test_functional.py b/tests/test_functional.py index d9d0091..340f0ea 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -22,7 +22,7 @@ from xandikos.web import SingleUserFilesystemBackend, XandikosApp from plann.cli import _add_journal, _add_todo, _check_for_panic, _list, _select -from plann.commands import _add_event, _check_pinned +from plann.commands import _add_event, _check_pinned, _set_event_status from plann.interactive import ( _interactive_edit, _interactive_relation_edit, @@ -483,15 +483,17 @@ def _run_check_pinned_tests(calendars): ## midnight two days ago - all pins are well in the past, whatever the time of day base = (now - 48*hour).replace(hour=0, minute=0, second=0) - def task(summary, parent=None): + def task(summary, parent=None, rrule=None): kwargs = {'set_parent': [parent]} if parent else {} + if rrule: + kwargs['set_rrule'] = rrule return _add_todo(ctx, summary=[summary], set_due=f"{now + 48*hour:%F %H:%M:%S}", set_duration='1h', set_uid=summary.replace(' ', '-'), **kwargs) def uid(obj): return str(obj.icalendar_component['UID']) - def pin(todo, begin, status='TENTATIVE', parents=None): - return _add_event(ctx, summary=_summary(todo), timespec=(begin, begin+hour), set_status=status, set_parent=parents or [uid(todo)]) + def pin(todo, begin, status='TENTATIVE', parents=None, summary=None): + return _add_event(ctx, summary=summary or _summary(todo), timespec=(begin, begin+hour), set_status=status, set_parent=parents or [uid(todo)]) def status(obj): obj.load() @@ -506,6 +508,11 @@ def status(obj): partial = task('tidy garage') early = task('early') late = task('late') + pack = task('pack bag') + food = task('buy food') + edited = task('clean deck') + weekly = task('weekly backup', rrule='FREQ=WEEKLY') + screws = task('sort screws') ## two past pins of the same task - the first one is asked about ev_done_first = pin(done, base) @@ -517,6 +524,15 @@ def status(obj): ## one event pinning both a task and its subtask pin(parent, base + 10*hour, parents=[uid(parent), uid(child)]) ev_partial = pin(partial, base + 12*hour) + ev_pack = pin(pack, base + 14*hour) + ## shared with a task that is not done, so not offered for cancelling + ev_shared = pin(pack, base + 16*hour, parents=[uid(pack), uid(food)], summary='shared pin') + ## completed through the edit prompt + ev_edited = pin(edited, base + 18*hour) + ev_weekly = pin(weekly, base + 20*hour) + ev_weekly_future = pin(weekly, now + 24*hour) + ## partially done, a subtask is split out + ev_screws = pin(screws, base + 22*hour) ## the day before, stored in different timezones. "early" is 09:00 ## UTC, but its wall clock time 18:00 sorts after the 10:00 of "late" day = (base - 24*hour).date() @@ -530,13 +546,25 @@ def status(obj): ## pinned too long ago pin(skipped, now - 30*24*hour) - answers = {'write report': 'y', 'fix bike': 'n', 'paint house': 'y', 'tidy garage': 'p'} + answers = {'write report': 'y', 'fix bike': 'n', 'paint house': 'y', 'tidy garage': 'p', + 'pack bag': 'y', 'shared pin': 's', 'clean deck': 'e', 'weekly backup': 'y', 'sort screws': 'p'} asked = [] echoed = [] - def prompt(*largs, **kwargs): - question = [x for x in echoed if re.match(r'\d{4}-\d\d-\d\d \d\d:\d\d - ', x)][-1] - asked.append(question) - return answers.get(question.split(': ', 1)[1], 's') + def prompt(text, default=None, **kwargs): + if text.startswith('Was it done?'): + question = [x for x in echoed if re.match(r'\d{4}-\d\d-\d\d \d\d:\d\d - ', x)][-1] + asked.append(question) + return answers.get(question.split(': ', 1)[1], 's') + ## the edit prompt, the questions when splitting out a subtask + replies = {'postpone': 'complete', 'Name for the subtask': default, 'what is the remaining': '1h', + 'Summary of the parent': default, 'Should we postpone': '0h'} + return next(v for k, v in replies.items() if text.startswith(k)) + + def confirm_(text, **kwargs): + if 'working on this' in text: + ## the user declines to split out the work on the garage + return not [x for x in echoed if ': estimate is ' in x][-1].startswith('tidy garage') + return True completed = [] orig_complete = caldav.Todo.complete @@ -544,20 +572,21 @@ def complete(self, *largs, **kwargs): completed.append(uid(self)) return orig_complete(self, *largs, **kwargs) - with patch('plann.commands.click.echo', new=lambda x='', **kw: echoed.append(str(x))): - with patch('plann.commands.click.prompt', new=prompt): - ## the user declines to split out the partially done work - with patch('click.confirm', side_effect=lambda text, **kw: 'working on this' not in text) as confirm: + with patch('click.echo', new=lambda x='', **kw: echoed.append(str(x))): + with patch('click.prompt', new=prompt): + with patch('click.confirm', side_effect=confirm_) as confirm: with patch.object(caldav.Todo, 'complete', new=complete): _check_pinned(ctx, lookback='7d') summaries = [x.split(': ', 1)[1] for x in asked] ## chronologically, whatever timezone the events are stored in - assert summaries == ['early', 'late', 'write report', 'fix bike', 'call mom', 'paint house', 'tidy garage'] + assert summaries == ['early', 'late', 'write report', 'fix bike', 'call mom', 'paint house', 'tidy garage', + 'pack bag', 'shared pin', 'clean deck', 'weekly backup', 'sort screws'] ## shown in local time tokyo_local = datetime.datetime(day.year, day.month, day.day, 18, tzinfo=ZoneInfo('Asia/Tokyo')).astimezone(tz.implicit_timezone) assert asked[0].startswith(f"{tokyo_local:%F %H:%M} - ") - ## asked about the other pins of the completed task, and about the split - assert [x.args[0].startswith('Cancel 2 other pinned') for x in confirm.call_args_list] == [True, False] + ## asked about the other pins of the completed task, and about the two splits - + ## not about the pin shared with an open task, nor the pins of a recurring task + assert [x.args[0][:22] for x in confirm.call_args_list] == ['Cancel 2 other pinned ', "So you've been working", "So you've been working"] assert status(done) == 'COMPLETED' assert status(ev_done_first) == 'CONFIRMED' @@ -577,26 +606,55 @@ def complete(self, *largs, **kwargs): ## nothing was split out, so it is asked about again assert status(partial) == 'NEEDS-ACTION' assert status(ev_partial) == 'TENTATIVE' + assert status(pack) == 'COMPLETED' + assert status(ev_pack) == 'CONFIRMED' + assert status(food) == 'NEEDS-ACTION' + assert status(ev_shared) == 'TENTATIVE' + assert status(edited) == 'COMPLETED' + assert status(ev_edited) == 'CONFIRMED' + ## the recurring task moves on to its next recurrence + weekly_due = weekly.get_due() + assert status(weekly) == 'NEEDS-ACTION' + assert weekly.get_due() > weekly_due + assert status(ev_weekly) == 'CONFIRMED' + assert status(ev_weekly_future) == 'TENTATIVE' + assert status(screws) == 'NEEDS-ACTION' + assert status(ev_screws) == 'CONFIRMED' + subtasks = [x for x in screws.get_relatives({'CHILD'})['CHILD'] if isinstance(x, caldav.Todo)] + assert [_summary(x) for x in subtasks] == ['Work on sort screws'] + assert status(subtasks[0]) == 'COMPLETED' ## a second run only asks about the skipped ones answers.clear() asked.clear() - with patch('plann.commands.click.echo', new=lambda x='', **kw: echoed.append(str(x))): - with patch('plann.commands.click.prompt', new=prompt): + with patch('click.echo', new=lambda x='', **kw: echoed.append(str(x))): + with patch('click.prompt', new=prompt): with patch('click.confirm', return_value=False) as confirm: _check_pinned(ctx, lookback='7d') - assert [x.split(': ', 1)[1] for x in asked] == ['early', 'late', 'call mom', 'tidy garage'] + assert [x.split(': ', 1)[1] for x in asked] == ['early', 'late', 'call mom', 'tidy garage', 'shared pin'] assert not confirm.called - ## a server refusing to save does not stop the rest of the questions - answers.update({'early': 'n', 'late': 'n'}) + ## a server refusing to save does not stop the rest of the questions, + ## and the other pins of a completed task are still offered for cancelling + mow = task('mow lawn') + ev_mow = pin(mow, base + 23*hour) + ev_mow_future = pin(mow, now + 24*hour) + answers.update({'early': 'n', 'late': 'n', 'mow lawn': 'y'}) asked.clear() - with patch('plann.commands.click.echo', new=lambda x='', **kw: echoed.append(str(x))): - with patch('plann.commands.click.prompt', new=prompt): - with patch('plann.commands._set_event_status', side_effect=PutError('423 Locked')): - _check_pinned(ctx, lookback='7d') - assert [x.split(': ', 1)[1] for x in asked] == ['early', 'late', 'call mom', 'tidy garage'] + def set_event_status(event, status): + if status == 'CONFIRMED' or _summary(event) in ('early', 'late'): + raise PutError('423 Locked') + return _set_event_status(event, status) + with patch('click.echo', new=lambda x='', **kw: echoed.append(str(x))): + with patch('click.prompt', new=prompt): + with patch('click.confirm', return_value=True) as confirm: + with patch('plann.commands._set_event_status', new=set_event_status): + _check_pinned(ctx, lookback='7d') + assert [x.split(': ', 1)[1] for x in asked] == ['early', 'late', 'call mom', 'tidy garage', 'shared pin', 'mow lawn'] assert any('423 Locked' in x for x in echoed) + assert status(mow) == 'COMPLETED' + assert status(ev_mow) == 'TENTATIVE' + assert status(ev_mow_future) == 'CANCELLED' ## TODO: ## Things to be tested: lib._procrastinate, cli._select, cli._cats, cli._list, cli._interactive_edit, cli._set_something, cli._interactive_ical_edit, cli._edit, cli._check_for_panic, _add_todo, _agenda, _check_due,