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

Time zone parsing support from iCal #555

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marcus-deans
Copy link

This is a direct copy of #526 by @epologee, to try and get this code in, having been based on the current master.

The base implementation of IceCube::IcalParser.schedule_from_ical ignores time zone identifiers, resulting in all times being parsed in the current time zone of the host.

This PR exposes the issue with a series of specs, and fixes it with a (somewhat lengthy) time zone lookup. This last part was added because the TZID strings of the iCal format don't map cleanly onto Rails time zones.
Input schedule

DTSTART;TZID=MDT:20130731T143000
RRULE:FREQ=WEEKLY;UNTIL=20140730T203000Z;BYDAY=MO,WE,FR
RDATE;TZID=MDT:20150812T143000
RDATE;TZID=MDT:20150807T143000
EXDATE;TZID=MDT:20130823T143000
EXDATE;TZID=MDT:20130812T143000
EXDATE;TZID=MDT:20130807T143000
DTEND;TZID=MDT:20130731T153000

Output schedule on master

DTSTART;TZID=CEST:20130731T143000
RRULE:FREQ=WEEKLY;UNTIL=20140730T203000Z;BYDAY=MO,WE,FR
RDATE;TZID=CEST:20150812T143000
RDATE;TZID=CEST:20150807T143000
EXDATE;TZID=CEST:20130823T143000
EXDATE;TZID=CEST:20130812T143000
EXDATE;TZID=CEST:20130807T143000
DTEND;TZID=CEST:20130731T153000

Output schedule on this branch

DTSTART;TZID=MDT:20130731T143000
RRULE:FREQ=WEEKLY;UNTIL=20140730T203000Z;BYDAY=MO,WE,FR
RDATE;TZID=MDT:20150812T143000
RDATE;TZID=MDT:20150807T143000
EXDATE;TZID=MDT:20130823T143000
EXDATE;TZID=MDT:20130812T143000
EXDATE;TZID=MDT:20130807T143000
DTEND;TZID=MDT:20130731T153000

P.S. Note that time zones will not always match the exact same string, because something like "America/Denver" will get converted to "MDT" along the way. The times will still match semantically.

@marcus-deans marcus-deans marked this pull request as ready for review June 22, 2024 18:29
@marcus-deans marcus-deans marked this pull request as draft June 22, 2024 18:30
@marcus-deans marcus-deans marked this pull request as ready for review June 22, 2024 18:35
@marcus-deans
Copy link
Author

@pacso this is an up-to-date version of a PR you previously reviewed, with rspec all passing locally. Would you be able to run the workflow to illuminate any potential errors? Would be great to have this merged in to avoid relying on a fork

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.

1 participant