From d7f1ca1841eb4168cd9edf3eafe15ed9feb8cbe4 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sat, 3 Feb 2024 13:07:37 -0800 Subject: [PATCH] Add API for expanding recurring Todo items (#281) * Add view for repeated todo items * Revert mypy changes and add gitignore * Address lint errors * Simplify handling of combining todo items with the same uid * Update test coverage for start validation and improve types * Update test coverage for updating the entire series * Add Todo test coverage * Update computed duration checking * Simplify todo list view to improve test coverage --- .gitignore | 2 + ical/calendar.py | 9 +++ ical/list.py | 113 ++++++++++++++++++++++++++++++++++++++ ical/todo.py | 74 +++++++++++++++++++++---- tests/test_list.py | 107 ++++++++++++++++++++++++++++++++++++ tests/test_store.py | 88 +++++++++++++++++++++++++---- tests/test_todo.py | 50 +++++++++++++++++ tests/types/test_recur.py | 27 ++++++++- 8 files changed, 447 insertions(+), 23 deletions(-) create mode 100644 ical/list.py create mode 100644 tests/test_list.py diff --git a/.gitignore b/.gitignore index b6e4761..7f6b817 100644 --- a/.gitignore +++ b/.gitignore @@ -127,3 +127,5 @@ dmypy.json # Pyre type checker .pyre/ + +.DS_Store diff --git a/ical/calendar.py b/ical/calendar.py index 3b1860b..05124b9 100644 --- a/ical/calendar.py +++ b/ical/calendar.py @@ -2,6 +2,7 @@ from __future__ import annotations +from collections.abc import Iterable import datetime import itertools import logging @@ -21,6 +22,7 @@ from .parsing.property import ParsedProperty from .timeline import Timeline, calendar_timeline from .timezone import Timezone, TimezoneModel, IcsTimezoneInfo +from .list import todo_list_view from .todo import Todo from .util import local_timezone, prodid_factory @@ -80,6 +82,13 @@ def timeline_tz(self, tzinfo: datetime.tzinfo | None = None) -> Timeline: """ return calendar_timeline(self.events, tzinfo=tzinfo or local_timezone()) + def todo_list(self, tzinfo: datetime.tzinfo | None = None) -> Iterable[Todo]: + """Return a list of all todos on the calendar. + + This view accounts for recurring todos. + """ + return todo_list_view(self.todos, tzinfo=tzinfo or local_timezone()) + @root_validator(pre=True) def _propagate_timezones(cls, values: dict[str, Any]) -> dict[str, Any]: """Propagate timezone information down to date-time objects. diff --git a/ical/list.py b/ical/list.py new file mode 100644 index 0000000..c1a0c91 --- /dev/null +++ b/ical/list.py @@ -0,0 +1,113 @@ +"""A List is a set of objects on a calendar. + +A List is used to iterate over all objects, including expanded recurring +objects. A List is similar to a Timeline, except it does not repeat recurring +objects on the list and they are only shown once. A list does not repeat +forever. +""" + +import datetime +from collections.abc import Generator, Iterable +import logging + +from .todo import Todo +from .iter import ( + LazySortableItem, + MergedIterable, + RecurIterable, + SortableItem, + SortableItemValue, +) +from .types.recur import RecurrenceId + + +_LOGGER = logging.getLogger(__name__) +_SortableTodoItem = SortableItem[datetime.datetime | datetime.date | None, Todo] + + +class RecurAdapter: + """An adapter that expands an Todo instance for a recurrence rule. + + This adapter is given an todo, then invoked with a specific date/time instance + that the todo is due from a recurrence rule. The todo is copied with + necessary updated fields to act as a flattened instance of the todo item. + """ + + def __init__(self, todo: Todo, tzinfo: datetime.tzinfo | None = None): + """Initialize the RecurAdapter.""" + self._todo = todo + self._duration = todo.computed_duration + self._tzinfo = tzinfo + + def get(self, dtstart: datetime.datetime | datetime.date) -> _SortableTodoItem: + """Return a lazy sortable item.""" + + recur_id_dt = dtstart + # Make recurrence_id floating time to avoid dealing with serializing + # TZID. This value will still be unique within the series and is in + # the context of dtstart which may have a timezone. + if isinstance(recur_id_dt, datetime.datetime) and recur_id_dt.tzinfo: + recur_id_dt = recur_id_dt.replace(tzinfo=None) + recurrence_id = RecurrenceId.__parse_property_value__(recur_id_dt) + + def build() -> Todo: + updates = { + "dtstart": dtstart, + "recurrence_id": recurrence_id, + } + if self._todo.due and self._duration: + updates["due"] = dtstart + self._duration + return self._todo.copy(update=updates) + + return LazySortableItem(dtstart, build) + + +def _todos_by_uid(todos: list[Todo]) -> dict[str, list[Todo]]: + todos_by_uid: dict[str, list[Todo]] = {} + for todo in todos: + if todo.uid is None: + raise ValueError("Todo must have a UID") + if todo.uid not in todos_by_uid: + todos_by_uid[todo.uid] = [] + todos_by_uid[todo.uid].append(todo) + return todos_by_uid + + +def _pick_todo(todos: list[Todo], tzinfo: datetime.tzinfo) -> Todo: + """Pick a todo to return in a list from a list of recurring todos. + + The items passed in must all be for the same original todo (either a + single todo or instance of a recurring todo including any edits). An + edited instance of a recurring todo has a recurrence-id that is + different from the original todo. This function will return the + next todo that is incomplete and has the latest due date. + """ + # For a recurring todo, the dtstart is after the last due date. Therefore + # we can stort items by dtstart and pick the last one that hasn't happened + iters: list[Iterable[_SortableTodoItem]] = [] + for todo in todos: + if not (recur := todo.as_rrule()): + iters.append([SortableItemValue(todo.dtstart, todo)]) + continue + iters.append(RecurIterable(RecurAdapter(todo, tzinfo=tzinfo).get, recur)) + + root_iter = MergedIterable(iters) + + # Pick the first todo that hasn't started yet based on its dtstart + now = datetime.datetime.now(tzinfo) + it = iter(root_iter) + last = next(it) + while cur := next(it, None): + if cur.item.start_datetime is None or cur.item.start_datetime > now: + break + last = cur + return last.item + + +def todo_list_view( + todos: list[Todo], tzinfo: datetime.tzinfo +) -> Generator[Todo, None, None]: + """Create a list view for todos on a calendar, including recurrence.""" + todos_by_uid = _todos_by_uid(todos) + for todos in todos_by_uid.values(): + yield _pick_todo(todos, tzinfo=tzinfo) diff --git a/ical/todo.py b/ical/todo.py index f3c163d..1cb5b2b 100644 --- a/ical/todo.py +++ b/ical/todo.py @@ -2,6 +2,7 @@ from __future__ import annotations +from collections.abc import Iterable import datetime import enum from typing import Any, Optional, Union @@ -13,6 +14,8 @@ from .alarm import Alarm from .component import ComponentModel, validate_until_dtstart, validate_recurrence_dates +from .exceptions import CalendarParseError +from .iter import RulesetIterable from .parsing.property import ParsedProperty from .types import ( CalAddress, @@ -87,15 +90,6 @@ class Todo(ComponentModel): duration: Optional[datetime.timedelta] = None """The duration of the item as an alternative to an explicit end date/time.""" - exdate: list[Union[datetime.datetime, datetime.date]] = Field(default_factory=list) - """Defines the list of exceptions for recurring todo item. - - The exception dates are used in computing the recurrence set. The recurrence set is - the complete set of recurrence instances for a calendar component (based on rrule, rdate, - exdate). The recurrence set is generated by gathering the rrule and rdate properties - then excluding any times specified by exdate. - """ - geo: Optional[Geo] = None """Specifies a latitude and longitude global position for the activity.""" @@ -143,6 +137,24 @@ class Todo(ComponentModel): sure all instances have the same start time regardless of time zone changing. """ + rdate: list[Union[datetime.datetime, datetime.date]] = Field(default_factory=list) + """Defines the list of date/time values for recurring events. + + Can appear along with the rrule property to define a set of repeating occurrences of the + event. The recurrence set is the complete set of recurrence instances for a calendar component + (based on rrule, rdate, exdate). The recurrence set is generated by gathering the rrule + and rdate properties then excluding any times specified by exdate. + """ + + exdate: list[Union[datetime.datetime, datetime.date]] = Field(default_factory=list) + """Defines the list of exceptions for recurring events. + + The exception dates are used in computing the recurrence set. The recurrence set is + the complete set of recurrence instances for a calendar component (based on rrule, rdate, + exdate). The recurrence set is generated by gathering the rrule and rdate properties + then excluding any times specified by exdate. + """ + sequence: Optional[int] = None """The revision sequence number in the calendar component. @@ -189,6 +201,48 @@ def start_datetime(self) -> datetime.datetime | None: return None return normalize_datetime(self.dtstart).astimezone(tz=datetime.timezone.utc) + @property + def computed_duration(self) -> datetime.timedelta | None: + """Return the event duration.""" + if self.due is None or self.dtstart is None: + return None + return self.due - self.dtstart + + @property + def recurring(self) -> bool: + """Return true if this Todo is recurring. + + A recurring event is typically evaluated specially on the list. The + data model has a single todo, but the timeline evaluates the recurrence + to expand and copy the the event to multiple places on the timeline + using `as_rrule`. + """ + if self.rrule or self.rdate: + return True + return False + + def as_rrule(self) -> Iterable[datetime.datetime | datetime.date] | None: + """Return an iterable containing the occurrences of a recurring todo. + + A recurring todo is typically evaluated specially on the todo list. The + data model has a single todo item, but the timeline evaluates the recurrence + to expand and copy the the item to multiple places on the timeline. + + This is only valid for events where `recurring` is True. + """ + if not self.rrule and not self.rdate: + return None + if not self.start: + raise CalendarParseError("Event must have a start date to be recurring") + if not self.due: + raise CalendarParseError("Event must have a due date to be recurring") + return RulesetIterable( + self.start, + [self.rrule.as_rrule(self.start)] if self.rrule else [], + self.rdate, + self.exdate, + ) + @root_validator def validate_one_due_or_duration(cls, values: dict[str, Any]) -> dict[str, Any]: """Validate that only one of duration or end date may be set.""" @@ -198,7 +252,7 @@ def validate_one_due_or_duration(cls, values: dict[str, Any]) -> dict[str, Any]: @root_validator def validate_duration_requires_start(cls, values: dict[str, Any]) -> dict[str, Any]: - """Validate that only one of duration or end date may be set.""" + """Validate that a duration requires the dtstart.""" if values.get("duration") and not values.get("dtstart"): raise ValueError("Duration requires that dtstart is specified") return values diff --git a/tests/test_list.py b/tests/test_list.py new file mode 100644 index 0000000..03e82fc --- /dev/null +++ b/tests/test_list.py @@ -0,0 +1,107 @@ +"""Tests for list view of todo items.""" + +import datetime + +import freezegun +import pytest + +from ical.list import todo_list_view +from ical.todo import Todo +from ical.types.recur import Recur + + +def test_empty_list() -> None: + """Test an empty list.""" + view = todo_list_view([], tzinfo=datetime.timezone.utc) + assert list(view) == [] + + +@pytest.mark.parametrize( + ("status"), + [ + ("NEEDS-ACTION"), + ("IN-PROCESS"), + ], +) +def test_daily_recurring_item_due_today_incomplete(status: str) -> None: + """Test a daily recurring item that is due today .""" + with freezegun.freeze_time("2024-01-10T10:05:00-05:00"): + todo = Todo( + dtstart=datetime.date.today() - datetime.timedelta(days=1), + summary="Daily incomplete", + due=datetime.date.today(), + rrule=Recur.from_rrule("FREQ=DAILY"), + status=status, + ) + view = list(todo_list_view([todo], tzinfo=datetime.timezone.utc)) + + assert len(view) == 1 + assert view[0].summary == todo.summary + assert view[0].dtstart == datetime.date(2024, 1, 10) + assert view[0].due == datetime.date(2024, 1, 11) + assert view[0].recurrence_id == "20240110" + + +@pytest.mark.parametrize( + ("status"), + [ + ("NEEDS-ACTION"), + ("IN-PROCESS"), + ], +) +def test_daily_recurring_item_due_tomorrow(status: str) -> None: + """Test a daily recurring item that is due tomorrow.""" + with freezegun.freeze_time("2024-01-10T10:05:00-05:00"): + todo = Todo( + dtstart=datetime.date.today(), + summary="Daily incomplete", + due=datetime.date.today() + datetime.timedelta(days=1), + rrule=Recur.from_rrule("FREQ=DAILY"), + status=status, + ) + view = list(todo_list_view([todo], tzinfo=datetime.timezone.utc)) + + assert len(view) == 1 + assert view[0].summary == todo.summary + assert view[0].dtstart == datetime.date(2024, 1, 10) + assert view[0].due == datetime.date(2024, 1, 11) + assert view[0].recurrence_id == "20240110" + + +@pytest.mark.parametrize( + ("status"), + [ + ("NEEDS-ACTION"), + ("IN-PROCESS"), + ], +) +def test_daily_recurring_item_due_yesterday(status: str) -> None: + """Test a daily recurring item that is due yesterday .""" + + with freezegun.freeze_time("2024-01-10T10:05:00-05:00"): + todo = Todo( + dtstart=datetime.date.today() - datetime.timedelta(days=1), + summary="Daily incomplete", + due=datetime.date.today(), + rrule=Recur.from_rrule("FREQ=DAILY"), + status=status, + ) + view = list(todo_list_view([todo], tzinfo=datetime.timezone.utc)) + + # The item should be returned with a recurrence_id of today + assert len(view) == 1 + assert view[0].summary == todo.summary + assert view[0].dtstart == datetime.date(2024, 1, 10) + assert view[0].due == datetime.date(2024, 1, 11) + assert view[0].recurrence_id == "20240110" + assert view[0].status == status + + with freezegun.freeze_time("2024-01-11T08:05:00-05:00"): + view = list(todo_list_view([todo], tzinfo=datetime.timezone.utc)) + + assert len(view) == 1 + assert view[0].summary == todo.summary + assert view[0].dtstart == datetime.date(2024, 1, 11) + assert view[0].due == datetime.date(2024, 1, 12) + assert view[0].recurrence_id == "20240111" + assert view[0].status == status diff --git a/tests/test_store.py b/tests/test_store.py index 87aef53..20fccd4 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -16,7 +16,7 @@ from ical.calendar import Calendar from ical.event import Event -from ical.todo import Todo +from ical.todo import Todo, TodoStatus from ical.store import EventStore, TodoStore, StoreError from ical.types.recur import Range, Recur from ical.types import RelationshipType, RelatedTo @@ -89,7 +89,7 @@ def mock_fetch_todos( """Fixture to return todos on the calendar.""" def _func(keys: set[str] | None = None) -> list[dict[str, Any]]: - return [compact_dict(todo.dict(), keys) for todo in calendar.todos] + return [compact_dict(todo.dict(), keys) for todo in calendar.todo_list()] return _func @@ -1204,11 +1204,16 @@ def test_delete_event_parent_cascade_to_children( duration=datetime.timedelta(minutes=30), ) ) - assert [ item['uid'] for item in fetch_events() ] == [ event1.uid, event2.uid, event3.uid, event4.uid ] + assert [item["uid"] for item in fetch_events()] == [ + event1.uid, + event2.uid, + event3.uid, + event4.uid, + ] # Delete parent and cascade to children store.delete("mock-uid-1") - assert [ item['uid'] for item in fetch_events() ] == [ event4.uid ] + assert [item["uid"] for item in fetch_events()] == [event4.uid] @pytest.mark.parametrize( @@ -1216,7 +1221,7 @@ def test_delete_event_parent_cascade_to_children( [ (RelationshipType.SIBBLING), (RelationshipType.CHILD), - ] + ], ) def test_unsupported_event_reltype( store: EventStore, @@ -1339,7 +1344,9 @@ def test_edit_todo( def test_todo_store_invalid_uid(todo_store: TodoStore) -> None: """Edit a todo that does not exist.""" with pytest.raises(StoreError, match="No existing"): - todo_store.edit("mock-uid-1", Todo(due="2022-08-29T09:05:00", summary="Delayed")) + todo_store.edit( + "mock-uid-1", Todo(due="2022-08-29T09:05:00", summary="Delayed") + ) with pytest.raises(StoreError, match="No existing"): todo_store.delete("mock-uid-1") @@ -1385,7 +1392,6 @@ def test_todo_timezone_for_datetime( assert calendar.timezones[1].tz_id == "America/New_York" - def test_todo_timezone_offset_not_supported( calendar: Calendar, todo_store: TodoStore, @@ -1436,11 +1442,16 @@ def test_delete_parent_todo_cascade_to_children( summary="Milk", ) ) - assert [ item['uid'] for item in fetch_todos() ] == [ todo1.uid, todo2.uid, todo3.uid, todo4.uid ] + assert [item["uid"] for item in fetch_todos()] == [ + todo1.uid, + todo2.uid, + todo3.uid, + todo4.uid, + ] # Delete parent and cascade to children todo_store.delete("mock-uid-1") - assert [ item['uid'] for item in fetch_todos() ] == [ todo4.uid ] + assert [item["uid"] for item in fetch_todos()] == [todo4.uid] @pytest.mark.parametrize( @@ -1448,7 +1459,7 @@ def test_delete_parent_todo_cascade_to_children( [ (RelationshipType.SIBBLING), (RelationshipType.CHILD), - ] + ], ) def test_unsupported_todo_reltype( todo_store: TodoStore, @@ -1476,4 +1487,59 @@ def test_unsupported_todo_reltype( ) todo2.related_to = [RelatedTo(uid=todo1.uid, reltype=reltype)] with pytest.raises(StoreError, match=r"Unsupported relationship type"): - todo_store.edit(todo2.uid, todo2) \ No newline at end of file + todo_store.edit(todo2.uid, todo2) + + +def test_recurring_item( + todo_store: TodoStore, + fetch_todos: Callable[..., list[dict[str, Any]]], + frozen_time: FrozenDateTimeFactory, +) -> None: + """Test a basic recurring item.""" + + frozen_time.move_to("2024-01-09T10:00:05") + + # Create a recurring to-do item + todo_store.add( + Todo( + summary="Walk dog", + dtstart="2024-01-09", + due="2024-01-10", + status="NEEDS-ACTION", + rrule=Recur.from_rrule("FREQ=DAILY;COUNT=10"), + ) + ) + assert fetch_todos(["uid", "recurrence_id", "due", "summary", "status"]) == [ + { + "uid": "mock-uid-1", + "recurrence_id": "20240109", + "due": "2024-01-10", + "summary": "Walk dog", + "status": TodoStatus.NEEDS_ACTION, + }, + ] + # Mark the entire series as completed + todo_store.edit("mock-uid-1", Todo(status="COMPLETED")) + assert fetch_todos(["uid", "recurrence_id", "due", "summary", "status"]) == [ + { + "uid": "mock-uid-1", + "recurrence_id": "20240109", + "due": "2024-01-10", + "summary": "Walk dog", + "status": TodoStatus.COMPLETED, + }, + ] + + # Advance to the next day. + frozen_time.move_to("2024-01-10T10:00:00") + + # All instances are completed + assert fetch_todos(["uid", "recurrence_id", "due", "summary", "status"]) == [ + { + "uid": "mock-uid-1", + "recurrence_id": "20240110", + "due": "2024-01-11", + "summary": "Walk dog", + "status": TodoStatus.COMPLETED, + }, + ] diff --git a/tests/test_todo.py b/tests/test_todo.py index 3b5198e..5a95d8f 100644 --- a/tests/test_todo.py +++ b/tests/test_todo.py @@ -4,12 +4,14 @@ import datetime import zoneinfo +from typing import Any from unittest.mock import patch import pytest from ical.exceptions import CalendarParseError from ical.todo import Todo +from ical.types.recur import Recur def test_empty() -> None: @@ -53,3 +55,51 @@ def test_duration() -> None: "ical.util.local_timezone", return_value=zoneinfo.ZoneInfo("America/Regina") ): assert todo.start_datetime.isoformat() == "2022-08-07T06:00:00+00:00" + + +@pytest.mark.parametrize( + ("params"), + [ + ({}), + ( + { + "start": datetime.datetime(2022, 9, 6, 6, 0, 0), + } + ), + ( + { + "due": datetime.datetime(2022, 9, 6, 6, 0, 0), + } + ), + ( + { + "duration": datetime.timedelta(hours=1), + } + ), + ], +) +def test_validate_rrule_required_fields(params: dict[str, Any]) -> None: + """Test that a Todo with an rrule requires a dtstart.""" + with pytest.raises(CalendarParseError): + todo = Todo( + summary="Todo 1", + rrule=Recur.from_rrule("FREQ=WEEKLY;BYDAY=WE,MO,TU,TH,FR;COUNT=3"), + **params, + ) + todo.as_rrule() + +def test_is_recurring() -> None: + """Test that a Todo with an rrule requires a dtstart.""" + todo = Todo( + summary="Todo 1", + rrule=Recur.from_rrule("FREQ=DAILY;COUNT=3"), + dtstart="2024-02-02", + due="2024-02-03", + ) + assert todo.recurring + assert todo.computed_duration == datetime.timedelta(days=1) + assert list(todo.as_rrule()) == [ + datetime.date(2024, 2, 2), + datetime.date(2024, 2, 3), + datetime.date(2024, 2, 4), + ] diff --git a/tests/types/test_recur.py b/tests/types/test_recur.py index 8623598..adec741 100644 --- a/tests/types/test_recur.py +++ b/tests/types/test_recur.py @@ -12,6 +12,7 @@ from ical.event import Event from ical.parsing.property import ParsedProperty, ParsedPropertyParameter from ical.timeline import Timeline +from ical.todo import Todo from ical.types.recur import Frequency, Recur, RecurrenceId, Weekday, WeekdayValue @@ -705,6 +706,25 @@ def test_recur_as_string(recur: Recur) -> None: assert event.rrule.as_rrule_str() == "FREQ=DAILY;INTERVAL=2" +@pytest.mark.parametrize( + "recur", + [ + Recur(freq=Frequency.DAILY, interval=2), + Recur.from_rrule("FREQ=DAILY;INTERVAL=2"), + ], +) +def test_todo_recur_as_string(recur: Recur) -> None: + """Test converting a recurrence rule back to a string.""" + + event = Todo( + summary="summary", + due=datetime.date(2022, 8, 1), + rrule=recur, + ) + assert event.rrule + assert event.rrule.as_rrule_str() == "FREQ=DAILY;INTERVAL=2" + + @pytest.mark.parametrize( "recur", [ @@ -846,6 +866,7 @@ def test_rrule_exdate_mismatch() -> None: # BYSETPOS + def test_bysetpos() -> None: """Test how all day events are handled with RDATE.""" @@ -857,7 +878,9 @@ def test_bysetpos() -> None: start=datetime.date(2023, 1, 31), end=datetime.date(2023, 2, 1), # Last work day of the month - rrule=Recur.from_rrule("FREQ=MONTHLY;BYSETPOS=-1;BYDAY=MO,TU,WE,TH,FR;COUNT=5"), + rrule=Recur.from_rrule( + "FREQ=MONTHLY;BYSETPOS=-1;BYDAY=MO,TU,WE,TH,FR;COUNT=5" + ), ), ] ) @@ -868,4 +891,4 @@ def test_bysetpos() -> None: (datetime.date(2023, 3, 31), "Monthly Event"), (datetime.date(2023, 4, 28), "Monthly Event"), (datetime.date(2023, 5, 31), "Monthly Event"), - ] \ No newline at end of file + ]