pbogut / recurring_events

Elixir library for dealing with recurring events
MIT License
26 stars 8 forks source link

Feature/timezone support #15

Open pbogut opened 6 years ago

pbogut commented 6 years ago

Attempt to resolve #14

The idea is to make Timex optional dependency and if available use different date helper to manipulate dates with timezone awareness.

At this point it should respect timezone for dates.

Todo:

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 75


Files with Coverage Reduction New Missed Lines %
lib/recurring_events/date.ex 15 83.15%
<!-- Total: 15 -->
Totals Coverage Status
Change from base Build 62: -5.08%
Covered Lines: 280
Relevant Lines: 295

💛 - Coveralls
pbogut commented 6 years ago

So if the rule is :hourly, :minutely etc. module is using Timex.shift, so it should handle timezone correctly.

Now, I'm not sure what to do with by_hourif it goes into timezone change. Two examples:

Let say we are working with Europe/Warsaw timezone. When we have by_hour: 2 rule and time is changed form summer time (GTM+2) to winter time (GTM+1). We can have 2 AM GTM+1 and 2 AM GTM+2 as we are moving clock back from 3am to 2am. How to decide which one should be used?

In reverse example we would change clock from 1:59am to 3:00am, so there is no 2am at all. Should 3am be set instead?

bgentry commented 6 years ago

Let say we are working with Europe/Warsaw timezone. When we have by_hour: 2 rule and time is changed form summer time (GTM+2) to winter time (GTM+1). We can have 2 AM GTM+1 and 2 AM GTM+2 as we are moving clock back from 3am to 2am. How to decide which one should be used?

In reverse example we would change clock from 1:59am to 3:00am, so there is no 2am at all. Should 3am be set instead?

I submitted #16 with broken tests for both of these cases. The expected behavior is outlined by RFC5545:

If, based on the definition of the referenced time zone, the local time described occurs more than once (when changing from daylight to standard time), the DATE-TIME value refers to the first occurrence of the referenced time. Thus, TZID=America/ New_York:20071104T013000 indicates November 4, 2007 at 1:30 A.M. EDT (UTC-04:00). If the local time described does not occur (when changing from standard to daylight time), the DATE-TIME value is interpreted using the UTC offset before the gap in local times. Thus, TZID=America/New_York:20070311T023000 indicates March 11, 2007 at 3:30 A.M. EDT (UTC-04:00), one hour after 1:30 A.M. EST (UTC-05:00).

bgentry commented 6 years ago

@pbogut I just want to make sure you saw the following note from #16 about one of those broken tests needing some changes:

Note that the expected value here is also wrong because it has an ambiguous timezone result (since 1:30AM occurs twice). We likely need to explicitly choose the expected timezone for this result to make sure it is the UTC-7 / PDT value, not the UTC-8 PST value (or ambiguous).

pbogut commented 6 years ago

@bgentry yes, I've read the comment, thank you for that. I will take it into account and make sure tests are checking correct timezone as well.

voughtdq commented 5 years ago

So, I think we can go off this:

When using Timex, Timex.to_datetime({{2007, 11, 4}, {1, 30, 0}}, "America/New_York") will return an AmbiguousDateTime struct. Because RFC5545 says to refer to the first occurrence, we can safely select the :before field in the struct. When the time is impossible (like 2007-3-11 2:30AM America/New_York) we get an {:error, _} tuple. In all other cases, we should get a %DateTime{} with the correct tz ref.

Here's an example of what I'm thinking about:

dt = {{2017, 11, 4}, {1, 30, 0}}
tz = "America/New_York"
 case Timex.to_datetime(dt, tz) do
   %DateTime{} = datetime ->
     datetime
   %Timex.AmbiguousDateTime{before: before, after: _after} ->
     # ambiguous, use the first occurrence per the spec
     before
   {:error, {:could_not_resolve_timezone, ^tz, _gregorian_seconds, :wall}} ->
     # non-existent time, add 1 hour
     {date, {h, m, s}} = dt
     Timex.to_datetime({date, {h + 1, m, s}}, tz)
 end

Now the last case with the {:error, _} tuple may need to be reworked a little bit because this assumes the only resolution error would be due to daylight savings and that all DST offsets are always 1 hour. (and the possibility that h+1 ends up being something like 24)

pbogut commented 5 years ago

@voughtdq that's very helpful. I did implementation for dates but couldn't figure out good way to do time. I think your idea should work all right. Will test it and see how its doing. Thanks.

tlvenn commented 5 years ago

@pbogut have you been able to make some progress on this ?

pbogut commented 5 years ago

I didn't spent too much time on this I'm afraid, I started it but its still in progress. I'm happy to accept PR if someone is able to do that, I may not have time to look at it for another few months unfortunately.