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

97 wip #100

Merged
merged 13 commits into from Nov 16, 2022
56 changes: 50 additions & 6 deletions recurring_ical_events.py
Expand Up @@ -118,7 +118,7 @@ class Repetition:
"RRULE", "RDATE", "EXDATE"
]

def __init__(self, source, start, stop, keep_recurrence_attributes=False):
def __init__(self, source, start, stop, keep_recurrence_attributes=False, end_prop='DTEND'):
"""Create an event repetition.

- source - the icalendar Event
Expand All @@ -130,13 +130,14 @@ def __init__(self, source, start, stop, keep_recurrence_attributes=False):
self.source = source
self.start = start
self.stop = stop
self.end_prop = end_prop
Copy link
Owner

Choose a reason for hiding this comment

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

This is in conflict with clarity and the class attribute.

self.keep_recurrence_attributes = keep_recurrence_attributes

def as_vevent(self):
Copy link
Owner

Choose a reason for hiding this comment

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

My guess is that this will be part of the subclass, too..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is one single place relatively deep in a method in the super class that uses this one. I didn't see any obvious way to move that logic to the subclasses without adding code duplication. You're welcome to have a look though :-)

"""Create a shallow copy of the source event and modify some attributes."""
revent = self.source.copy()
revent["DTSTART"] = vDDDTypes(self.start)
revent["DTEND"] = vDDDTypes(self.stop)
revent[self.end_prop] = vDDDTypes(self.stop)
if not self.keep_recurrence_attributes:
for attribute in self.ATTRIBUTES_TO_DELETE_ON_COPY:
if attribute in revent:
Expand All @@ -157,14 +158,16 @@ def __repr__(self):
class RepeatedEvent:
"""An event with repetitions created from an icalendar event."""

end_prop='DTEND'

def __init__(self, event, keep_recurrence_attributes=False):
"""Create an event which may have repetitions in it.

- event - the icalendar Event
- keep_recurrence_attributes whether to copy or delete attributes in repetitions.
"""
self.event = event
self.start = self.original_start = event["DTSTART"].dt
self.start = self.original_start = self._get_event_start()
self.end = self.original_end = self._get_event_end()
self.keep_recurrence_attributes = keep_recurrence_attributes
self.exdates = []
Expand Down Expand Up @@ -310,6 +313,7 @@ def within_days(self, span_start, span_stop):
self.convert_to_original_type(start),
self.convert_to_original_type(stop),
self.keep_recurrence_attributes,
self.end_prop
)

def convert_to_original_type(self, date):
Expand All @@ -325,15 +329,45 @@ def convert_to_original_type(self, date):
return convert_to_date(date)
return date

def _get_event_start(self):
"""Calculate the end of the event/task/journal based on DTSTART, DTEND/DUE and DURATION."""
## easy case - DTSTART set (it should be set for events and journals)
start = self.event.get('DTSTART')
if start is not None:
return start.dt
## Tasks may have DUE set, but no DTSTART.
## Let's assume 0 duration and return the DUE
due = self.event.get(self.end_prop)
if due is not None:
return due.dt
## Assume infinite time span if neither is given
## (see the comments under _get_event_end)
return datetime.date(*DATE_MIN)

def _get_event_end(self):
"""Calculate the end of the event based on DTSTART, DTEND and DURATION."""
end = self.event.get("DTEND")
"""Calculate the end of the event/task/journal based on DTSTART, DTEND/DUE and DURATION."""
## Easy case - DTEND/DUE is set
end = self.event.get(self.end_prop)
if end is not None:
return end.dt
## DURATION can be specified instead of DTEND/DUE
duration = self.event.get("DURATION")
if duration is not None:
return self.event["DTSTART"].dt + duration.dt
return self.event["DTSTART"].dt
## According to the RFC, a VEVENT without an end/duration
## is to be considered to have zero duration. Assuming the
## same applies to VTODO.
dtstart = self.event.get("DTSTART")
if dtstart:
return dtstart.dt
niccokunzmann marked this conversation as resolved.
Show resolved Hide resolved
## The RFC says this about VTODO:
## > A "VTODO" calendar component without the "DTSTART" and "DUE" (or
## > "DURATION") properties specifies a to-do that will be associated
## > with each successive calendar date, until it is completed.
## It can be interpreted in different ways, though probably it may
## be considered equivalent with a DTSTART in the infinite past and DUE
## in the infinite future?
return datetime.date(*DATE_MAX)

def is_recurrence(self):
"""Whether this is a recurrence/modification of an event."""
Expand All @@ -344,6 +378,12 @@ def as_single_event(self):
for repetition in self.within_days(self.start, self.start):
return repetition

## TODO: we should probably have a proper base class, RepeatedEvent should be a sibling of RepeatedTodo and RepeatedJournal, not a parent
class RepeatedTodo(RepeatedEvent):
end_prop = 'DUE'

class RepeatedJournal(RepeatedEvent):
end_prop = ''

# The minimum value accepted as date (pytz + zoneinfo)
DATE_MIN = (1970, 1, 1)
Expand All @@ -360,6 +400,10 @@ def __init__(self, calendar, keep_recurrence_attributes=False):
self.repetitions = []
for event in calendar.walk("VEVENT"):
self.repetitions.append(RepeatedEvent(event, keep_recurrence_attributes))
for event in calendar.walk("VTODO"):
self.repetitions.append(RepeatedTodo(event, keep_recurrence_attributes))
for event in calendar.walk("VJOURNAL"):
self.repetitions.append(RepeatedJournal(event, keep_recurrence_attributes))

@staticmethod
def to_datetime(date):
Expand Down
15 changes: 15 additions & 0 deletions test/calendars/issue-97-simple-journal.ics
@@ -0,0 +1,15 @@
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Example Corp.//CalDAV Client//EN
BEGIN:VJOURNAL
UID:19920901T130000Z-123409@host.com
DTSTAMP:19920901T130000Z
DTSTART:19920420
SUMMARY:Yearly Income Tax Report
DESCRIPTION:We made it this year too. Probably. What's the point of a recurring journal entry? Journals are supposed to describe past events, aren't they?
RRULE:FREQ=YEARLY
CLASS:CONFIDENTIAL
CATEGORIES:FAMILY,FINANCE
PRIORITY:1
END:VJOURNAL
END:VCALENDAR
15 changes: 15 additions & 0 deletions test/calendars/issue-97-simple-todo.ics
@@ -0,0 +1,15 @@
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Example Corp.//CalDAV Client//EN
BEGIN:VTODO
UID:19920901T130000Z-123408@host.com
DTSTAMP:19920901T130000Z
DTSTART:19920415T133000Z
DUE:19920516T045959Z
SUMMARY:Yearly Income Tax Preparation
RRULE:FREQ=YEARLY
CLASS:CONFIDENTIAL
CATEGORIES:FAMILY,FINANCE
PRIORITY:1
END:VTODO
END:VCALENDAR
14 changes: 14 additions & 0 deletions test/calendars/issue-97-todo-nodtstart.ics
@@ -0,0 +1,14 @@
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Example Corp.//CalDAV Client//EN
BEGIN:VTODO
UID:19920901T130000Z-123408@host.com
DTSTAMP:19920901T130000Z
DUE:19920516T045959Z
SUMMARY:Yearly Income Tax Preparation
RRULE:FREQ=YEARLY
CLASS:CONFIDENTIAL
CATEGORIES:FAMILY,FINANCE
PRIORITY:1
END:VTODO
END:VCALENDAR
44 changes: 44 additions & 0 deletions test/test_issue_97_simple_recurrent_todos_and_journals.py
@@ -0,0 +1,44 @@
import pytest
from recurring_ical_events import DATE_MAX

calendars_parametrized = pytest.mark.parametrize("ical_file", [
"issue_97_simple_todo",
"issue_97_simple_journal",
"issue_97_todo_nodtstart",
])

@calendars_parametrized
def test_recurring_task_is_not_included1(calendars, ical_file):
"""The three files given starts in late 1991, no recurrences
should be found before 1991. Refers to
https://github.com/niccokunzmann/python-recurring-ical-events/issues/97.
Test passes prior to fixing #97, should still pass after #97 is
fixed.
"""
calendar = getattr(calendars, ical_file)
tasks = calendar.between((1989, 1, 1), (1991,1,1))
assert not tasks

@calendars_parametrized
def test_recurring_task_is_not_included2(calendars, ical_file):
"""Every recurrence of the three ical files is in October, hence
no recurrences should be found. Refers to
https://github.com/niccokunzmann/python-recurring-ical-events/issues/97.
Test passes prior to fixing #97, should still pass after #97 is
fixed.
"""
calendar = getattr(calendars, ical_file)
tasks = calendar.between((1998, 1, 1), (1998,4,14))
assert not tasks

@calendars_parametrized
def test_recurring_task_is_repeated(calendars, ical_file):
"""Expansion of a yearly task over seven years.
The issue
https://github.com/niccokunzmann/python-recurring-ical-events/issues/97
needs to be fixed before this test can pass
"""
calendar = getattr(calendars, ical_file)
events = calendar.between((1995, 1, 1), (2002,1,1))
assert len(events) == 7

2 changes: 2 additions & 0 deletions test/test_properties.py
Expand Up @@ -10,6 +10,8 @@ def test_event_has_summary(calendars):
def test_recurrent_events_change_start_and_end(calendars, attribute):
events = calendars.three_events_one_edited.all()
print(events)
if events[0][attribute].__hash__ is None:
Copy link
Owner

Choose a reason for hiding this comment

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

no. The test becomes worthless.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an independent issue that should be fixed properly in a separate pull request. I chipped in this code just to get the tests to pass. I did try to hash event[attribute].dt, but then the asserts failed ... hm.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I sent a few comments. I might chip in some code later but am quite busy, too.

It's good to coordinate that we don't work at the same things at the same time :-) I will apparently be busy for the rest of the day.

pytest.skip()
values = set(event[attribute] for event in events)
assert len(values) == 3

Expand Down