Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement undo #520

Closed
wants to merge 9 commits into from
50 changes: 50 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import click
import hypothesis.strategies as st
import pytest
import pytz
from dateutil.tz import tzlocal
from freezegun import freeze_time
from hypothesis import given
Expand Down Expand Up @@ -749,6 +750,55 @@ def test_done_recurring(runner, todo_factory, todos):
assert todo.rrule == rrule


def test_undo(runner, todo_factory, todos):
completed_at = datetime.datetime.now(tz=pytz.UTC)
percentage = 100
todo = todo_factory(completed_at=completed_at, percent_complete=percentage)

result = runner.invoke(cli, ["undo", "1"])
assert not result.exception

todo = next(todos(status="ANY"))
assert todo.percent_complete == 0
assert todo.is_completed is False
assert not todo.rrule


@pytest.mark.xfail(reason="Not implemented")
r4ulill0 marked this conversation as resolved.
Show resolved Hide resolved
def test_undo_recurring(runner, default_database, todo_factory, todos):
rrule = "FREQ=DAILY;UNTIL=20990315T020000Z"
recurred = todo_factory(id=2, rrule=rrule)

completed_at = datetime.datetime.now(tz=pytz.UTC)
percentage = 100
related = [recurred]
original = todo_factory(
id=1,
rrule=None,
completed_at=completed_at,
percent_complete=percentage,
related=related,
)
assert original.related

default_database.update_cache()
result = runner.invoke(cli, ["undo", "1"])
assert not result.exception

default_database.update_cache()

todos = todos(status="ANY")
todo = next(todos)

assert todo.percent_complete == 0
assert todo.is_completed is False
assert not todo.related
assert todo.rrule == rrule

todo = next(todos)
assert not todo


def test_cancel(runner, todo_factory, todos):
todo = todo_factory()

Expand Down
24 changes: 24 additions & 0 deletions todoman/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,30 @@ def done(ctx, todos):
click.echo(ctx.formatter.detailed(todo))


@cli.command()
@pass_ctx
@click.argument(
"todos",
nargs=-1,
required=True,
type=click.IntRange(0),
callback=_validate_todos,
)
@catch_errors
def undo(ctx, todos):
"""Undo one or more tasks."""
toremove = []
for todo in todos:
removable = todo.undo()
if removable:
toremove.append(removable)
ctx.db.save(todo)
click.echo(ctx.formatter.detailed(todo))

for todo in toremove:
ctx.db.delete(todo)


@cli.command()
@pass_ctx
@click.argument(
Expand Down
21 changes: 21 additions & 0 deletions todoman/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,27 @@ def complete(self) -> None:
self.percent_complete = 100
self.status = "COMPLETED"

def undo(self) -> Todo | None:
"""Immediately restores this todo.

Marks it as needs action, resets the percentage to 0 and deletes the
completed_at datetime.
If this todo belongs to a series, the one created at completion will
be deleted.

Returns the todo that should be deleted.
"""
recurred = None
if self.is_recurring and self.related:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran:

todo new -l test test
todo new 1

The completed todo looks like this:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:io.barrera.todoman
BEGIN:VTODO
COMPLETED:20231118T090119Z
CREATED:20231118T090107Z
DTSTAMP:20231118T090107Z
DUE:20231119T090107Z
LAST-MODIFIED:20231118T090119Z
PERCENT-COMPLETE:100
SEQUENCE:2
STATUS:COMPLETED
SUMMARY:test
UID:[email protected]
END:VTODO
END:VCALENDAR

It doesn't have a recurrence rule any more (that is moved to the next one in the series), so this condition does not match.

If I run todo undo 1, both todos are kept around.

This is not ideal, but I don't think that it is terrible either. The RFC doesn't even cover how a recurring TODO is completed, so there's no correct behavior to any of this either way.

However, this if will never run, so this code is misleading. I think it should be removed (unless I'm missing something here).

recurred = self.related.pop()
self.rrule = recurred.rrule

self.completed_at = None
self.percent_complete = 0
self.status = "NEEDS-ACTION"

return recurred

@cached_property
def path(self) -> str:
if not self.list:
Expand Down