From cf57678ccf03cdad259016ec7c05765e89727489 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Fri, 16 Oct 2026 16:11:27 +0200 Subject: [PATCH 1/4] feat: complete pending children with their parent A child has to be done before its parent can be completed, so completing a parent now completes its pending children recursively, with a warning for each. This applies to edit --complete, the interactive complete command and completed! in set-task-attribs. When the selection also holds a child, it resolves to the selected object and is completed once, so a recurring child does not advance twice, a later save does not undo the completion, and it is not prompted for again. Prompts (comments made inline in the document while reviewing it): > (on "nothing stops you completing a parent with pending children") Completing a parent means auto-completing all the children. This should be implemented and warned properly about in plann. Please fix or file. Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- CHANGELOG.md | 1 + USER_GUIDE.md | 2 ++ plann/commands.py | 27 +++++++++++----- plann/interactive.py | 11 ++++--- plann/lib.py | 34 ++++++++++++++++++++ tests/test_lib.py | 69 +++++++++++++++++++++++++++++++++++++++++ tests/test_plann_cli.py | 61 ++++++++++++++++++++++++++++++++++++ 7 files changed, 193 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c14556..05af937 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and I do try to adhere to [Semantic Versioning](https://semver.org/spec/v2.0.0.h ### Changed +* Completing a task (`edit --complete`, `complete` in the interactive edit, `completed!` in `set-task-attribs`) now also completes its pending child tasks, recursively, and prints a warning for each. A child has to be done before the parent can be completed. * Shell tab completion scripts (bash, zsh, fish) now ship with the package. They work without shell configuration for system-wide installs (and for bash user/venv installs with bash-completion 2.12+); see the README for other setups. The `make install-completion` targets are gone. * icalendar 6.2.0 or newer is required. diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 927d6e2..6ccc6f1 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -157,6 +157,8 @@ plann select --todo --category computer-work --start=2022-04-04 --end=2022-05-05 plann select --todo --category computer-work --overdue edit --postpone=5d ``` +A child task has to be done before its parent can be completed, so completing a parent also completes its pending children and grandchildren, with a warning for each. A recurring child moves on to its next recurrence, the same as a recurring task completed on its own. Children are found through the parent's own `RELATED-TO;RELTYPE=CHILD` lines; a child that is linked only from its own side (`RELTYPE=PARENT`, as some other clients write it) is not found. + ## Sharing busy time between calendars `freebusy-sync` copies every busy period in the selected calendars (`--config-section`, plus any given with `--caldav-url`/`--calendar-url`) into the calendar of the config section given with `--to-section`, which must resolve to exactly one calendar (set `calendar_url` or `calendar_name` in it). The copies hold only the time, in UTC, and a fixed summary - no description, location, attendees, timezone or UID from the original. Run it regularly, e.g. from a systemd timer: copies are moved or deleted when the original changes, and other events in the target calendar are left alone. Copies are never copied onward, so syncing both ways is safe: diff --git a/plann/commands.py b/plann/commands.py index 5bb9df1..247904c 100644 --- a/plann/commands.py +++ b/plann/commands.py @@ -26,11 +26,13 @@ _add_comma_list, _comma_list_canonical, _comma_list_tokens, + _complete, _component_type, _is_comma_list_attr, _list, _process_set_arg, _procrastinate, + _RelativeCache, _relships_by_type, _set_something, _summary, @@ -298,9 +300,13 @@ def _edit(ctx, cancel=None, interactive_ical=False, interactive_relations=False, ## TODO: should be possible to combine this with other opitions return + complete_cache = _RelativeCache() + complete_cache.add_objects(ctx.obj['objs']) + completed = set() for obj in ctx.obj['objs']: - if interactive: - _interactive_edit(obj) + if interactive and str(obj.icalendar_component['UID']) not in completed: + ## (a task completed along with its parent is not asked about again) + _interactive_edit(obj, complete_cache, completed) comp = obj.icalendar_component if kwargs.get('pdb'): _pdb_edit(obj) @@ -309,7 +315,7 @@ def _edit(ctx, cancel=None, interactive_ical=False, interactive_relations=False, for canonical, tokens in add_args: _add_comma_list(obj, canonical, tokens) if complete: - obj.complete(handle_rrule=complete_recurrence_mode, rrule_mode=complete_recurrence_mode) + _complete(obj, handle_rrule=complete_recurrence_mode, rrule_mode=complete_recurrence_mode, err_callback=lambda msg: click.echo(msg, err=True), cache=complete_cache, _seen=completed) elif complete is False: obj.uncomplete() if cancel: @@ -589,6 +595,11 @@ def _set_task_attribs(ctx): ## rather than issuing a fresh server select for each attribute (E4). _select(ctx=ctx, todo=True, sort_key=['{DTSTART:?{DUE:?(0000)?}?%F %H:%M:%S}', '{PRIORITY:?0?}']) todos = list(ctx.obj['objs']) + ## completing a task completes its children - make them resolve to the + ## objects in todos, so a stale copy is neither prompted for nor saved + complete_cache = _RelativeCache() + complete_cache.add_objects(todos) + completed = set() def _set_something_(something, help_text, help_url=None, default=None, objs=None): something_ = _comma_list_canonical(something) if _is_comma_list_attr(something) else something @@ -620,15 +631,17 @@ def _set_something_(something, help_text, help_url=None, default=None, objs=None click.echo(f"For each task, {help_text}") click.echo('(or enter "completed!" with bang but without quotes if the task is already done)') for obj in objs: + if str(obj.icalendar_component['UID']) in completed: + continue comp = obj.icalendar_component summary = _summary(comp) value = click.prompt(summary, default=default) if value == 'completed!': - obj.complete() + _complete(obj, err_callback=lambda msg: click.echo(msg, err=True), cache=complete_cache, _seen=completed) obj.save() - ## todos is fetched once - drop the task so later - ## attributes don't prompt for it again - todos.remove(obj) + ## todos is fetched once - drop the task and the children + ## completed with it, so later attributes don't prompt for them + todos[:] = [x for x in todos if str(x.icalendar_component['UID']) not in completed] continue if _is_comma_list_attr(something): comp.add(something_, _comma_list_tokens(something, value)) diff --git a/plann/interactive.py b/plann/interactive.py index 7c9dbd3..69ff50d 100644 --- a/plann/interactive.py +++ b/plann/interactive.py @@ -25,6 +25,7 @@ from plann.lib import ( _adjust_relations, + _complete, _component_type, _list, _now, @@ -49,7 +50,7 @@ def _pdb_edit(obj, interactive=True): click.echo("happy hacking") breakpoint() -def command_edit(obj, command, interactive=True): +def command_edit(obj, command, interactive=True, complete_cache=None, completed=None): if command == 'ignore': return elif command in ('part', 'partially-complete'): @@ -76,7 +77,7 @@ def command_edit(obj, command, interactive=True): ## TODO: we probably shouldn't be doing this interactively here? _procrastinate([obj], command.split(' ')[1], **with_params) elif command == 'complete': - obj.complete(handle_rrule=True) + _complete(obj, handle_rrule=True, err_callback=click.echo, cache=complete_cache, _seen=completed) elif command == 'cancel': obj.icalendar_component['STATUS'] = 'CANCELLED' elif command.lower().startswith('set rrule='): @@ -213,7 +214,7 @@ def _abort(message): click.echo(message) raise click.Abort(message) -def _interactive_edit(obj): +def _interactive_edit(obj, complete_cache=None, completed=None): if _component_type(obj) != 'VTODO': raise NotImplementedError("interactive editing only implemented for tasks") comp = obj.icalendar_component @@ -228,7 +229,7 @@ def _interactive_edit(obj): click.echo(f"pri={pri} {dtstart:%F %H:%M:%S %Z} - {due:%F %H:%M:%S %Z}: {summary}") input = click.prompt("postpone d / ignore / part(ially-complete) / complete / split / cancel / set foo=bar / edit / family / start / pdb?", default='ignore') try: - command_edit(obj, input, interactive=True) + command_edit(obj, input, interactive=True, complete_cache=complete_cache, completed=completed) except NotImplementedError as e: ## e.g. `start` when extra_config.time_tracking is not configured - ## the prompt advertises the command, so report why it did not work @@ -237,7 +238,7 @@ def _interactive_edit(obj): return if input == 'start': ## time tracking has been started - re-prompt so a follow-up command can be given for the same task - _interactive_edit(obj) + _interactive_edit(obj, complete_cache, completed) def _mass_reprioritize(objs): text = """\ diff --git a/plann/lib.py b/plann/lib.py index 01b9aaa..6013c90 100644 --- a/plann/lib.py +++ b/plann/lib.py @@ -459,6 +459,12 @@ def __init__(self): self._objects = {} self._relships = {} + def add_objects(self, objs): + """Seed the cache with objects already in memory, so relatives + resolve to those very objects rather than to fresh copies""" + for obj in objs: + self._objects[(getattr(obj.parent, 'url', None), str(obj.icalendar_component['UID']))] = obj + def get_object(self, calendar, uid): key = (getattr(calendar, 'url', None), str(uid)) if key not in self._objects: @@ -516,6 +522,34 @@ def _relships_by_type(obj, reltype_wanted=None, cache=None): cache.store_relships(obj, reltype_wanted, ret) return ret +def _complete(obj, handle_rrule=True, rrule_mode='safe', err_callback=print, cache=None, _seen=None): + """Mark obj as completed. + + A child has to be done before the parent can be completed, so + completing a parent means the pending children (and grandchildren) + are done as well - they get auto-completed first, with a warning for + each. Recurring children are handled the same way as obj. + + When completing several objects, pass the same cache (seeded with + the objects) and the same _seen set to every call, so that an object + that is also a child of another one is completed exactly once. + """ + if cache is None: + cache = _RelativeCache() + if _seen is None: + _seen = set() + uid = str(obj.icalendar_component['UID']) + if uid in _seen: + return + _seen.add(uid) + for child in _relships_by_type(obj, 'CHILD', cache=cache).get('CHILD', []): + if str(child.icalendar_component['UID']) in _seen or not child.is_pending(): + continue + err_callback(f"Completing child task '{_summary(child)}' of '{_summary(obj)}'") + _complete(child, handle_rrule, rrule_mode, err_callback, cache, _seen) + if obj.is_pending(): + obj.complete(handle_rrule=handle_rrule, rrule_mode=rrule_mode) + def _relationship_text(obj, reltype_wanted=None): rels = obj.get_relatives(reltypes=reltype_wanted) if not rels: diff --git a/tests/test_lib.py b/tests/test_lib.py index 9a99db2..4662ce0 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -9,9 +9,11 @@ _add_comma_list, _adjust_ical_relations, _caldav_objclass, + _complete, _component_type, _process_set_arg, _procrastinate, + _RelativeCache, _set_something, _split_vcal, _summary, @@ -547,3 +549,70 @@ def test_add_time_tracking_timew_multiple_category_lines(mock_run): add_time_tracking_timew(obj, datetime(2020, 2, 20, 20, 2), datetime(2020, 2, 20, 20, 20)) cmd_arr = mock_run.mock_calls[0].args[0] assert [x for x in cmd_arr if x.startswith('category:')] == ['category:foo', 'category:bar', 'category:baz'] + +def _todo_with_uid(uid, status='NEEDS-ACTION'): + t = Todo() + t.data = todo.replace('19970901T130000Z-123404@host.com', uid).replace('STATUS:NEEDS-ACTION', f'STATUS:{status}') + return t + +def test_complete_also_completes_pending_children(): + """Completing a parent means the children are done too - they are + auto-completed (grandchildren included) before the parent, with a + warning for each""" + parent = _todo_with_uid('parent') + child = _todo_with_uid('child') + grandchild = _todo_with_uid('grandchild') + done_child = _todo_with_uid('done_child', status='COMPLETED') + children = {'parent': [child, done_child], 'child': [grandchild]} + def relships(obj, reltype_wanted=None, cache=None): + assert reltype_wanted == 'CHILD' + return {'CHILD': children.get(str(obj.icalendar_component['UID']), [])} + order = [] + def complete(self, **kwargs): + order.append(str(self.icalendar_component['UID'])) + self._complete_ical(completion_timestamp=datetime.now(utc)) + warnings = [] + with patch('plann.lib._relships_by_type', side_effect=relships), \ + patch.object(Todo, 'complete', complete): + _complete(parent, err_callback=warnings.append) + assert order == ['grandchild', 'child', 'parent'] + assert len(warnings) == 2 + assert all('Fix a party' in w for w in warnings) + +def test_complete_survives_relation_cycles(): + a = _todo_with_uid('a') + b = _todo_with_uid('b') + children = {'a': [b], 'b': [a]} + def relships(obj, reltype_wanted=None, cache=None): + return {'CHILD': children[str(obj.icalendar_component['UID'])]} + with patch('plann.lib._relships_by_type', side_effect=relships), \ + patch.object(Todo, 'save'), patch.object(Todo, 'load'): + _complete(a, err_callback=lambda x: None) + assert a.icalendar_component['STATUS'] == 'COMPLETED' + assert b.icalendar_component['STATUS'] == 'COMPLETED' + +def test_complete_selection_with_parent_and_child_completes_child_once(): + """`select ... edit --complete` may hold both a parent and its child. + The child must be completed once - a recurring child is pending again + after completion, and would otherwise advance twice - and on the + selected object itself, so that saving the selection afterwards does + not write a stale copy back""" + parent = _todo_with_uid('parent') + child = _todo_with_uid('child') + cache = _RelativeCache() + cache.add_objects([parent, child]) + def relships(obj, reltype_wanted=None, cache=None): + if str(obj.icalendar_component['UID']) != 'parent': + return {'CHILD': []} + return {'CHILD': [cache.get_object(obj.parent, 'child')]} + completed = [] + def complete(self, **kwargs): + ## like a recurring task moved on to its next occurrence: still pending + completed.append(self) + seen = set() + with patch('plann.lib._relships_by_type', side_effect=relships), \ + patch.object(Todo, 'complete', complete): + for obj in (parent, child): + _complete(obj, err_callback=lambda x: None, cache=cache, _seen=seen) + assert completed == [child, parent] + assert completed[0] is child diff --git a/tests/test_plann_cli.py b/tests/test_plann_cli.py index 3eda16b..a0f3f82 100644 --- a/tests/test_plann_cli.py +++ b/tests/test_plann_cli.py @@ -446,3 +446,64 @@ def test_freebusyhack_warns(caplog): ctx = _FakeCtx() commands_mod._select(ctx, freebusyhack='busy') assert 'deprecated' in caplog.text + + +def _family(**kwargs): + """A parent 'p' with a recurring child 'c', related both ways""" + parent = _make_todo('p', **kwargs) + parent.icalendar_component.add('RELATED-TO', 'c', parameters={'RELTYPE': 'CHILD'}) + child = _make_todo('c', **kwargs) + child.icalendar_component.add('RELATED-TO', 'p', parameters={'RELTYPE': 'PARENT'}) + child.icalendar_component.add('RRULE', {'FREQ': 'YEARLY'}) + return parent, child + + +def _record_completions(calls): + """Stands in for Todo.complete: records (uid, handle_rrule) and, like a + recurring task that advanced, leaves the child pending""" + def complete(self, completion_timestamp=None, handle_rrule=False, rrule_mode='safe'): + calls.append((str(self.icalendar_component['UID']), bool(handle_rrule))) + if 'RRULE' not in self.icalendar_component: + self._complete_ical(completion_timestamp=datetime.now().astimezone()) + return complete + + +def test_set_task_attribs_completed_parent_completes_selected_child(): + """completed! on a parent completes the child in the selection - once, + honouring its RRULE - and the child is not prompted for afterwards""" + parent, child = _family() + + def fake_select(ctx, **kwargs): + ctx.obj['objs'] = [parent, child] + + calls = [] + ctx = _FakeCtx() + with patch.object(commands_mod, '_select', fake_select), \ + patch.object(Todo, 'save'), \ + patch.object(Todo, 'complete', _record_completions(calls)), \ + patch('plann.commands.click.echo'), \ + patch('plann.commands.click.prompt', return_value='completed!') as prompt: + _set_task_attribs(ctx) + + assert prompt.call_count == 1 + assert calls == [('c', True), ('p', True)] + + +def test_interactive_edit_complete_parent_completes_selected_child(): + """`edit --interactive`: completing the parent completes the selected + child object itself (so the save at the end of the loop does not write + a stale copy back), once, and the child is not prompted for""" + parent, child = _family(due=True, dtstart=True) + ctx = _FakeCtx() + ctx.obj['objs'] = [parent, child] + ctx.obj['set_args'] = {} + calls = [] + with patch.object(Todo, 'save'), \ + patch.object(Todo, 'complete', _record_completions(calls)), \ + patch('plann.interactive.click.echo'), \ + patch('plann.commands.click.echo'), \ + patch('plann.interactive.click.prompt', side_effect=['complete']) as prompt: + commands_mod._edit(ctx, interactive=True) + + assert prompt.call_count == 1 + assert calls == [('c', True), ('p', True)] From a3dfbc46adf596cb6bb05e4653834accc39e578f Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Fri, 16 Oct 2026 16:23:49 +0200 Subject: [PATCH 2/4] docs: plan recurring task trees An RRULE on every node of a task tree breaks the rule that a parent is due after its children once a child is completed. Describe the planned model, RRULE on the root only with the subtree regenerated on completion, and add it to the roadmap. Prompts (comments made inline in the document while reviewing it): > (on recurring child tasks breaking the rule that a parent is due after its children) Any suggestions? Are there any prior-art on this? (Some research was done on prior-art already - see the [calendaring-client] directory. Few systems supports (sic) both dependencies and recurring tasks). > (on the proposal: RRULE on the root only, regenerating the subtree) Then this should be included in the documentation for plann and calendaring-client, together with a /calendar-todo Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- TASK_MANAGEMENT.md | 6 ++++++ docs/ROADMAP.md | 1 + 2 files changed, 7 insertions(+) diff --git a/TASK_MANAGEMENT.md b/TASK_MANAGEMENT.md index db284b3..795d628 100644 --- a/TASK_MANAGEMENT.md +++ b/TASK_MANAGEMENT.md @@ -187,6 +187,12 @@ The current default logic is to duplicate/split the completed task into a comple There is not so much support for recurrences outside the task completion code, as for now the rrule has to be manually added when creating or editing the task, or the recurrence has to be set through some another caldav client tool. I believe recurring tasks is an important functionality, so I will implement better support for this at some point. +#### Recurring task trees + +A recurring job is often a tree - "varnish the boat every spring and autumn" breaks down into areas, and each area into layers that have to be done in order. Putting an `RRULE` on every node of the tree works with the completion logic above, but it breaks the rule that a parent is due after its children: once a child is completed it advances to its next occurrence, while the parent is still pending. `RELATED-TO` points to a UID, not to a single occurrence, so the `RECURRENCE-ID` model cannot link the right occurrences to each other either. + +The prior art (OmniFocus repeating projects and action groups, Todoist resetting the sub-tasks of a recurring parent; see the task model survey in calendaring-client) puts the recurrence on the root only. The plan for plann is the same: an `RRULE` only on the root. Completing the root completes its pending children (plann does that already), and then regenerates the subtree for the next occurrence: every descendant is copied with a new UID, its `DTSTART`/`DUE` is shifted by the same delta as the root, and the copy is linked to the root. The completed children stay linked to the completed copy of the root. This is not implemented yet. + ### Timestamps **TLDR:** Timestamps are important. DUE should indicate when we need or want to be done with the task, DURATION should be the estimated time for doing the task. Since DURATION and DUE cannot be combined, let rather DTSTART indicate the last possible time one can start working with the task and still have a hope to get done before the DUE timestamp. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index bed650c..9d9f452 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -275,6 +275,7 @@ the first priority will always be to get food on the table. | Item | Rough estimate | Notes | Issue | |---|---|---|---| | Creating and editing recurring tasks from the CLI | 16-24 h | `TASK_MANAGEMENT.md` calls recurring tasks "important functionality"; today the RRULE must be written by hand or set from another client. The completion logic already handles them | | +| Recurring task trees | 12-20 h | `RRULE` on the root only; completing the root regenerates the subtree for the next occurrence, shifted by the same delta. See "Recurring task trees" in `TASK_MANAGEMENT.md` | | | `pin` - sticking tasks to the calendar | 12-20 h | Fully specified in `DESIGN.md`, never implemented. The conceptual core of the VTODO/VEVENT pairing idea | | | Loadable module support in `select` | 12-16 h | Arbitrary Python for filtering, editing and ical transformation | [#6](https://github.com/pycalendar/plann/issues/6) | | Mergecal support - privacy-preserving calendar merging | 12-16 h | Copy busy-time between work and private calendars without leaking detail | [#24](https://github.com/pycalendar/plann/issues/24) | From 3992358ae2adf40bfc70f5175f13be21389dea91 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Fri, 16 Oct 2026 19:52:43 +0200 Subject: [PATCH 3/4] docs: specify links when regenerating task trees The recurring-task-tree plan linked every regenerated copy to the root, which flattens deeper trees, and assumed the completed children stay linked to the completed copy of the root, which gets a new UID in safe completion. Link each copy to the copy of its own parent, remap every RELATED-TO on both ends, drop the completed copy's link to a parent of the root, and note that caldav has to expose the completed copy first. Todoist re-adds sub-tasks one level deep only. Prompt: [fix issues found during the review process] Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- TASK_MANAGEMENT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TASK_MANAGEMENT.md b/TASK_MANAGEMENT.md index 795d628..9df5ceb 100644 --- a/TASK_MANAGEMENT.md +++ b/TASK_MANAGEMENT.md @@ -191,7 +191,7 @@ There is not so much support for recurrences outside the task completion code, a A recurring job is often a tree - "varnish the boat every spring and autumn" breaks down into areas, and each area into layers that have to be done in order. Putting an `RRULE` on every node of the tree works with the completion logic above, but it breaks the rule that a parent is due after its children: once a child is completed it advances to its next occurrence, while the parent is still pending. `RELATED-TO` points to a UID, not to a single occurrence, so the `RECURRENCE-ID` model cannot link the right occurrences to each other either. -The prior art (OmniFocus repeating projects and action groups, Todoist resetting the sub-tasks of a recurring parent; see the task model survey in calendaring-client) puts the recurrence on the root only. The plan for plann is the same: an `RRULE` only on the root. Completing the root completes its pending children (plann does that already), and then regenerates the subtree for the next occurrence: every descendant is copied with a new UID, its `DTSTART`/`DUE` is shifted by the same delta as the root, and the copy is linked to the root. The completed children stay linked to the completed copy of the root. This is not implemented yet. +The prior art (OmniFocus repeating projects and action groups; Todoist re-adding the sub-tasks of a recurring parent, one level deep; see the task model survey in calendaring-client) puts the recurrence on the root only. The plan for plann is the same: an `RRULE` only on the root. Completing the root completes its pending descendants (plann does that already) and splits the completed occurrence off as a copy with a new UID, as the "safe" completion mode does. Then the subtree is regenerated for the next occurrence: every descendant is copied with a new UID, its `DTSTART`/`DUE` is shifted by the same delta as the root, and each copy is linked to the copy of its own parent, so deeper trees keep their shape. Copying carries every `RELATED-TO` along, and plann stores the parent/child links on both ends (`PARENT` on the child, `CHILD` on the parent), so every link - ordering links such as RFC 9253 `DEPENDS-ON` between descendants included - is rewritten to the corresponding copy, or dropped when there is none. After copying, the completed copy of the root already lists the old children as `CHILD`, and the regenerated first-level copies already say `PARENT` to the root; the old children's `PARENT` must be pointed at the completed copy, the root's `CHILD` set replaced by the new copies, and deeper copies remapped within their generation. If the root has a parent of its own, the completed copy drops that `PARENT` link, rather than piling up one child per occurrence on the grandparent. The caldav library builds the completed copy in the private `_build_recurring_safe_completed()`, but `complete()` returns nothing, so it has to expose that copy first. This is not implemented yet. ### Timestamps **TLDR:** Timestamps are important. DUE should indicate when we need or want to be done with the task, DURATION should be the estimated time for doing the task. Since DURATION and DUE cannot be combined, let rather DTSTART indicate the last possible time one can start working with the task and still have a hope to get done before the DUE timestamp. From c3d604bb0f7a079743a4e4200bc91e3c225f5065 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Mon, 19 Oct 2026 22:16:33 +0200 Subject: [PATCH 4/4] fix: complete a child once in multi-edit, too edit --multi-edit completed each line's task without the shared cache and seen set, so a recurring child marked complete along with its parent advanced twice. Lines now share one cache, still fetching each task after the editor closes. A task that cannot be completed, being not pending or having a STATUS outside the RFC, is reported before its children are touched, and such a child is skipped with a warning. Document that completed! now honours RRULE and that a recurring parent's children are not regenerated for its next occurrence. Prompt: [fix issues found during the review process] Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- CHANGELOG.md | 3 +- USER_GUIDE.md | 2 +- plann/interactive.py | 18 ++++++++---- plann/lib.py | 22 +++++++++++++-- tests/test_lib.py | 47 +++++++++++++++++++++++++++++-- tests/test_plann_cli.py | 62 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 141 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05af937..290b0cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ and I do try to adhere to [Semantic Versioning](https://semver.org/spec/v2.0.0.h ### Changed -* Completing a task (`edit --complete`, `complete` in the interactive edit, `completed!` in `set-task-attribs`) now also completes its pending child tasks, recursively, and prints a warning for each. A child has to be done before the parent can be completed. +* Completing a task (`edit --complete`, `complete` in the interactive and multi-edit modes, `completed!` in `set-task-attribs`) now also completes its pending child tasks, recursively, and prints a warning for each. A child has to be done before the parent can be completed. +* `completed!` in `set-task-attribs` moves a recurring task on to its next recurrence, like `edit --complete` does, instead of completing the whole series. * Shell tab completion scripts (bash, zsh, fish) now ship with the package. They work without shell configuration for system-wide installs (and for bash user/venv installs with bash-completion 2.12+); see the README for other setups. The `make install-completion` targets are gone. * icalendar 6.2.0 or newer is required. diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 6ccc6f1..ad73e82 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -157,7 +157,7 @@ plann select --todo --category computer-work --start=2022-04-04 --end=2022-05-05 plann select --todo --category computer-work --overdue edit --postpone=5d ``` -A child task has to be done before its parent can be completed, so completing a parent also completes its pending children and grandchildren, with a warning for each. A recurring child moves on to its next recurrence, the same as a recurring task completed on its own. Children are found through the parent's own `RELATED-TO;RELTYPE=CHILD` lines; a child that is linked only from its own side (`RELTYPE=PARENT`, as some other clients write it) is not found. +A child task has to be done before its parent can be completed, so completing a parent also completes its pending children and grandchildren, with a warning for each. A recurring child moves on to its next recurrence, the same as a recurring task completed on its own. Children are found through the parent's own `RELATED-TO;RELTYPE=CHILD` lines; a child that is linked only from its own side (`RELTYPE=PARENT`, as some other clients write it) is not found. When a recurring parent moves on to its next recurrence, its non-recurring children stay completed and linked to it, and no fresh ones are made for the next recurrence (regenerating them is planned, see [TASK_MANAGEMENT.md](TASK_MANAGEMENT.md)). ## Sharing busy time between calendars diff --git a/plann/interactive.py b/plann/interactive.py index 69ff50d..5717eaa 100644 --- a/plann/interactive.py +++ b/plann/interactive.py @@ -32,6 +32,7 @@ _process_set_arg, _procrastinate, _relationship_text, + _RelativeCache, _set_something, _split_vcal, _summary, @@ -299,10 +300,15 @@ def _mass_interactive_edit(objs, default='ignore'): ## TODO: this is bad design template=default + " {UID}: due={DUE} Pri={PRIORITY:?0?} {SUMMARY:?{DESCRIPTION:?(no summary given)?}?} (STATUS={STATUS:-})", filter=lambda obj: obj.icalendar_component.get('STATUS', 'NEEDS-ACTION')=='NEEDS-ACTION')) edited = _editor(text) + ## tasks are fetched once the editor is closed, and shared between + ## lines, so a task completed along with its parent is not completed + ## again on its own line + complete_cache = _RelativeCache() + completed = set() for line in edited.split('\n'): ## TODO: BUG: does not work if the source data comes from multiple calendars! ## (possible fix: make a dict from uid to calendar(s)) - _command_line_edit(line, interactive=True, calendar=objs[0].parent) + _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): comp = obj.icalendar_component @@ -367,7 +373,7 @@ def _strip_line(line): line = line.strip() return line -def _get_obj_from_line(line, calendar): +def _get_obj_from_line(line, calendar, cache=None): ## TODO: the calendar we get here may be invalid. ## We need to map uids to objects when fetching events ## this should be done globally to reduce code duplication @@ -378,10 +384,12 @@ def _get_obj_from_line(line, calendar): found = uid_re.match(line) assert found uid = found.group(1) + if cache is not None: + return cache.get_object(calendar, uid) obj = calendar.object_by_uid(uid) return obj -def _command_line_edit(line, calendar, interactive=True): +def _command_line_edit(line, calendar, interactive=True, complete_cache=None, completed=None): regexp = re.compile(rf"((?:set [^ ]*=[^ ]*)|(?:postpone (?:[0-9]+[{DURATION_UNITS}]|20[0-9][0-9]-[0-9][0-9]-[0-9][0-9]))|[^ ]*) (.*)$") line = _strip_line(line) if not line: @@ -389,7 +397,7 @@ def _command_line_edit(line, calendar, interactive=True): splitted = regexp.match(line) assert splitted command = splitted.group(1) - obj = _get_obj_from_line(splitted.group(2), calendar) + obj = _get_obj_from_line(splitted.group(2), calendar, complete_cache) assert obj - command_edit(obj, command, interactive) + command_edit(obj, command, interactive, complete_cache, completed) diff --git a/plann/lib.py b/plann/lib.py index 6013c90..4815edf 100644 --- a/plann/lib.py +++ b/plann/lib.py @@ -542,13 +542,29 @@ def _complete(obj, handle_rrule=True, rrule_mode='safe', err_callback=print, cac if uid in _seen: return _seen.add(uid) + try: + pending = obj.is_pending() + except AssertionError: + ## caldav refuses a STATUS outside the RFC + pending = False + if not pending: + ## checked before the children, so a task that cannot be + ## completed does not leave its tree half done + err_callback(f"Not completing '{_summary(obj)}': STATUS is {obj.icalendar_component.get('STATUS')}") + return for child in _relships_by_type(obj, 'CHILD', cache=cache).get('CHILD', []): - if str(child.icalendar_component['UID']) in _seen or not child.is_pending(): + if str(child.icalendar_component['UID']) in _seen: + continue + try: + if not child.is_pending(): + continue + except AssertionError: + ## caldav refuses a STATUS outside the RFC + err_callback(f"Not completing child task '{_summary(child)}' of '{_summary(obj)}': unknown STATUS {child.icalendar_component.get('STATUS')}") continue err_callback(f"Completing child task '{_summary(child)}' of '{_summary(obj)}'") _complete(child, handle_rrule, rrule_mode, err_callback, cache, _seen) - if obj.is_pending(): - obj.complete(handle_rrule=handle_rrule, rrule_mode=rrule_mode) + obj.complete(handle_rrule=handle_rrule, rrule_mode=rrule_mode) def _relationship_text(obj, reltype_wanted=None): rels = obj.get_relatives(reltypes=reltype_wanted) diff --git a/tests/test_lib.py b/tests/test_lib.py index 4662ce0..e027777 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -552,7 +552,7 @@ def test_add_time_tracking_timew_multiple_category_lines(mock_run): def _todo_with_uid(uid, status='NEEDS-ACTION'): t = Todo() - t.data = todo.replace('19970901T130000Z-123404@host.com', uid).replace('STATUS:NEEDS-ACTION', f'STATUS:{status}') + t.data = todo.replace('19970901T130000Z-123404@host.com', uid).replace('STATUS:NEEDS-ACTION', f'STATUS:{status}').replace('SUMMARY:Fix a party', f'SUMMARY:task {uid}') return t def test_complete_also_completes_pending_children(): @@ -576,8 +576,8 @@ def complete(self, **kwargs): patch.object(Todo, 'complete', complete): _complete(parent, err_callback=warnings.append) assert order == ['grandchild', 'child', 'parent'] - assert len(warnings) == 2 - assert all('Fix a party' in w for w in warnings) + assert warnings == ["Completing child task 'task child' of 'task parent'", + "Completing child task 'task grandchild' of 'task child'"] def test_complete_survives_relation_cycles(): a = _todo_with_uid('a') @@ -616,3 +616,44 @@ def complete(self, **kwargs): _complete(obj, err_callback=lambda x: None, cache=cache, _seen=seen) assert completed == [child, parent] assert completed[0] is child + +def test_complete_explicit_non_pending_task_warns(): + """caldav cannot complete a task that is not pending, e.g. a + cancelled one; it is left alone, but not silently""" + cancelled = _todo_with_uid('cancelled', status='CANCELLED') + warnings = [] + with patch('plann.lib._relships_by_type', return_value={}), \ + patch.object(Todo, 'save'), patch.object(Todo, 'load'): + _complete(cancelled, err_callback=warnings.append) + assert cancelled.icalendar_component['STATUS'] == 'CANCELLED' + assert warnings == ["Not completing 'task cancelled': STATUS is CANCELLED"] + +def test_complete_skips_child_with_non_standard_status(): + """A child with a STATUS outside the RFC is left alone, with a + warning, rather than crashing halfway through the tree""" + parent = _todo_with_uid('parent') + odd = _todo_with_uid('odd', status='WAITING') + def relships(obj, reltype_wanted=None, cache=None): + return {'CHILD': [odd] if obj is parent else []} + warnings = [] + with patch('plann.lib._relships_by_type', side_effect=relships), \ + patch.object(Todo, 'save'), patch.object(Todo, 'load'): + _complete(parent, err_callback=warnings.append) + assert parent.icalendar_component['STATUS'] == 'COMPLETED' + assert odd.icalendar_component['STATUS'] == 'WAITING' + assert len(warnings) == 1 and 'task odd' in warnings[0] + +def test_complete_task_with_non_standard_status_warns(): + """A selected task with a STATUS outside the RFC gets a warning, not + a crash after its children were completed""" + odd = _todo_with_uid('odd', status='WAITING') + child = _todo_with_uid('child') + def relships(obj, reltype_wanted=None, cache=None): + return {'CHILD': [child] if obj is odd else []} + warnings = [] + with patch('plann.lib._relships_by_type', side_effect=relships), \ + patch.object(Todo, 'save'), patch.object(Todo, 'load'): + _complete(odd, err_callback=warnings.append) + assert odd.icalendar_component['STATUS'] == 'WAITING' + assert child.icalendar_component['STATUS'] == 'NEEDS-ACTION' + assert warnings == ["Not completing 'task odd': STATUS is WAITING"] diff --git a/tests/test_plann_cli.py b/tests/test_plann_cli.py index a0f3f82..981724b 100644 --- a/tests/test_plann_cli.py +++ b/tests/test_plann_cli.py @@ -507,3 +507,65 @@ def test_interactive_edit_complete_parent_completes_selected_child(): assert prompt.call_count == 1 assert calls == [('c', True), ('p', True)] + + +class _FamilyCalendar: + """Hands out a fresh copy of the task on every lookup, like a server""" + url = 'https://calendar.example.com/tasks/' + + def __init__(self, objs): + self.data = {str(o.icalendar_component['UID']): o.data for o in objs} + + def object_by_uid(self, uid): + obj = Todo() + obj.data = self.data[uid] + obj.parent = self + return obj + get_object_by_uid = object_by_uid + + +def test_mass_interactive_edit_complete_parent_and_child_completes_child_once(): + """`edit --multi-edit` with both parent and recurring child marked + complete: the child advances once, not once with the parent and once + on its own line""" + parent, child = _family(due=True) + cal = _FamilyCalendar([parent, child]) + parent.parent = child.parent = cal + calls = [] + with patch.object(Todo, 'save'), \ + patch.object(Todo, 'complete', _record_completions(calls)), \ + patch('plann.interactive.click.echo'), \ + patch('plann.interactive._editor', return_value="complete p: task p\ncomplete c: task c\n"): + commands_mod._mass_interactive_edit([parent, child]) + assert calls == [('c', True), ('p', True)] + + +def test_edit_complete_selection_with_parent_and_child_completes_child_once(): + """`select ... edit --complete` holding parent and recurring child, in + either order: the child is completed once""" + for order in ((0, 1), (1, 0)): + family = _family(due=True) + ctx = _FakeCtx() + ctx.obj['objs'] = [family[i] for i in order] + ctx.obj['set_args'] = {} + calls = [] + with patch.object(Todo, 'save'), \ + patch.object(Todo, 'complete', _record_completions(calls)), \ + patch('plann.commands.click.echo'): + commands_mod._edit(ctx, complete=True) + assert calls == [('c', True), ('p', True)] + + +def test_mass_interactive_edit_works_on_tasks_fetched_after_editing(): + """The editor may stay open for a while; a line acts on the task as it + is on the server when the editor closes, not on the listed copy""" + task = _make_todo('t', due=True) + cal = _FamilyCalendar([task]) + task.parent = cal + saved = [] + with patch.object(Todo, 'save', lambda self: saved.append(self)), \ + patch('plann.interactive._editor', return_value="set priority=3 t: task t\n"): + commands_mod._mass_interactive_edit([task]) + assert len(saved) == 1 + assert saved[0] is not task + assert saved[0].icalendar_component['PRIORITY'] == 3