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

RRULE: Fix floating UNTIL with dateutil > 2.6.1 #115

Merged
merged 4 commits into from
Jul 8, 2018
Merged

Conversation

Unrud
Copy link
Contributor

@Unrud Unrud commented Apr 25, 2018

Fixes #113, #112

dateutil > 2.6.1 raises an exception when UNTIL is floating and DTSTART is not. This PR fixes the issue by parsing the RRULE first without DTSTART to extract the UNTIL parameter and then without the UNTIL parameter but with DTSTART. The RRULE without UNTIL and the normalized UNTIL are combined at the end.

This PR makes #111 obsolete.

Unrud added 2 commits April 25, 2018 04:48
dateutil raises an exception when UNTIL is UTC and ``dtstart`` is not set.
This restores the old  behaviour, because ``isinstance(dtstart, datetime.date)`` is always true.
@Unrud
Copy link
Contributor Author

Unrud commented Apr 25, 2018

I now realize that the original behaviour is wrong. ignoretz=isinstance(dtstart, datetime.date) is always True, this causes vobject to always drop the time zone from UNTIL. If DTSTART has a time zone and UNTIL is UTC, the time zone of UNTIL gets replaced by the time zone of DTSTART without adjustment.

Unrud added 2 commits April 25, 2018 15:26
This reverts commit 8e28129.

The original behaviour was wrong. ``ignoretz=isinstance(dtstart, datetime.date)`` is always True, this causes vobject to always drop the time zone from UNTIL. If DTSTART has a time zone and UNTIL is UTC, the time zone of UNTIL gets replaced by the time zone of DTSTART without adjustment.
@wpercy
Copy link
Contributor

wpercy commented May 19, 2018

@Unrud this looks good, but can you write a test for this behavior (ie floating UNTIL and DTSTART with defined timezone)?

@dotlambda
Copy link

Any update on this?

@wpercy wpercy merged commit da73536 into skarim:master Jul 8, 2018
@wpercy wpercy added this to the 0.9.6 milestone Jul 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_recurrence fails
3 participants