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

Fix reversion of RRULE UNTIL behavior #88

Merged
merged 2 commits into from
Nov 7, 2021
Merged

Fix reversion of RRULE UNTIL behavior #88

merged 2 commits into from
Nov 7, 2021

Conversation

tresni
Copy link
Contributor

@tresni tresni commented Oct 29, 2021

Prior to 0.1.25, ics files containing events like the following worked normally. A change in 11dfa8c caused these to break. This is an ics I'm getting from Google for my scheduled lunches :)

This restores the pre-0.1.25 behavior and continues to pass all tests.

BEGIN:VEVENT
SUMMARY:LUNCH
DTSTART;TZID=America/Boise:20210929T130000
DTEND;TZID=America/Boise:20210929T135000
DTSTAMP:20211029T005456Z
UID:not_sure_if_this_uid_is_secret@google.com
SEQUENCE:3
RRULE:FREQ=WEEKLY;UNTIL=20211020;BYDAY=MO,TH,WE;WKST=SU
EXDATE;TZID=America/Boise:20211013T130000
CREATED:20210811T153934Z
DESCRIPTION:
LAST-MODIFIED:20210927T184338Z
LOCATION:
STATUS:CONFIRMED
TRANSP:OPAQUE
X-APPLE-TRAVEL-ADVISORY-BEHAVIOR:AUTOMATIC
BEGIN:VALARM
ACKNOWLEDGED:20210830T185450Z
ACTION:NONE
TRIGGER;VALUE=DATE-TIME:19760401T005545Z
UID:343EB08E-9C4C-4151-92D0-80B351A080B2
X-WR-ALARMUID:343EB08E-9C4C-4151-92D0-80B351A080B2
END:VALARM
END:VEVENT

This event fails to parse with the following error:

ValueError: RRULE UNTIL values must be specified in UTC when DTSTART is timezone-aware

@eigenmannmartin eigenmannmartin self-requested a review October 29, 2021 19:13
@codecov
Copy link

codecov bot commented Oct 29, 2021

Codecov Report

Merging #88 (d7c153b) into master (c151e7c) will decrease coverage by 0.59%.
The diff coverage is 100.00%.

❗ Current head d7c153b differs from pull request most recent head 16573fc. Consider uploading reports for the commit 16573fc to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
- Coverage   87.60%   87.00%   -0.60%     
==========================================
  Files           4        4              
  Lines         355      354       -1     
  Branches       92       91       -1     
==========================================
- Hits          311      308       -3     
- Misses         21       22       +1     
- Partials       23       24       +1     
Impacted Files Coverage Δ
icalevents/icalparser.py 87.78% <100.00%> (-0.05%) ⬇️
icalevents/icaldownload.py 78.57% <0.00%> (-4.77%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c151e7c...16573fc. Read the comment docs.

Copy link
Member

@eigenmannmartin eigenmannmartin left a comment

Choose a reason for hiding this comment

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

great catch!

Thank you for your pr :-D

@@ -156,6 +156,21 @@ def test_events_rrule_until_all_day_google(self):
self.assertEqual(ev_2.all_day, True, "All day event")
self.assertEqual(ev_2.summary, "Busy")

def test_events_rrule_until_only_date(self):
Copy link
Member

Choose a reason for hiding this comment

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

awesome!

Prior to 0.1.25, ics files containing events like the following worked normally.  [A change](11dfa8c#diff-6659079db1c5e26c4bd075f64b432522644ff3ee3bd4264f2863bd6f69d63e72L287-R405) in the 11dfa8c caused these to break.  This is an ics I'm getting from Google for my scheduled lunches :)

This restores the pre-0.1.25 behavior and continues to pass all tests.

```
BEGIN:VEVENT
SUMMARY:LUNCH
DTSTART;TZID=America/Boise:20210929T130000
DTEND;TZID=America/Boise:20210929T135000
DTSTAMP:20211029T005456Z
UID:not_sure_if_this_uid_is_secret@google.com
SEQUENCE:3
RRULE:FREQ=WEEKLY;UNTIL=20211020;BYDAY=MO,TH,WE;WKST=SU
EXDATE;TZID=America/Boise:20211013T130000
CREATED:20210811T153934Z
DESCRIPTION:
LAST-MODIFIED:20210927T184338Z
LOCATION:
STATUS:CONFIRMED
TRANSP:OPAQUE
X-APPLE-TRAVEL-ADVISORY-BEHAVIOR:AUTOMATIC
BEGIN:VALARM
ACKNOWLEDGED:20210830T185450Z
ACTION:NONE
TRIGGER;VALUE=DATE-TIME:19760401T005545Z
UID:343EB08E-9C4C-4151-92D0-80B351A080B2
X-WR-ALARMUID:343EB08E-9C4C-4151-92D0-80B351A080B2
END:VALARM
END:VEVENT
```
@tresni
Copy link
Contributor Author

tresni commented Oct 30, 2021

Should be ready to merge now @eigenmannmartin . Thanks for the heads up :)

@eigenmannmartin eigenmannmartin self-requested a review November 7, 2021 17:00
Copy link
Member

@eigenmannmartin eigenmannmartin left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@eigenmannmartin eigenmannmartin merged commit a4919ad into jazzband:master Nov 7, 2021
capuanob pushed a commit to ennamarie19/icalevents that referenced this pull request Mar 6, 2023
Fix reversion of RRULE UNTIL behavior
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.

2 participants