IceCube has TimeZone issues #115

Closed
bitboxer opened this Issue Nov 11, 2012 · 13 comments

Comments

Projects
None yet
5 participants

Somehow the TimeZone information is lost during scheduling of events:

First I created a new Time in the GMT Timezone

time = Time.new(2012, 10, 10, 20, 15, 0, 0)
p time
#2012-10-10 20:15:00 +0000
p time.getlocal
#2012-10-10 22:15:00 +0200

Currently I am not im GMT but in CET and now I create the IceCube Schedule

s = IceCub::Schedule.new(time)
s.add_recurrence_rule IceCube::Rule.daily
p s.next_occurrence
#2012-11-11 20:15:00 +0100

Now the next occurrence is not created with the correct time zone information. It should be 20:15:00 +0000.

forrest commented Nov 11, 2012

Is it possible that you're going over a daylight savings time boundary?

The 20:15 i put in there is gmt, i get back a 20:15 in gmt+1. the problem can't be the dailight saving time

Owner

seejohnrun commented Nov 13, 2012

Time.new should create a local by default - can you try using Time.gm?

No, the Time.new does not use local time because I add the time offset as last parameter. See in the first output of p time that the time is in +0000 = GMT.

Owner

seejohnrun commented Nov 13, 2012

I see the issue - it's with next_occurrence, next_occurrences, and remaining_occurrences. I'll have a fix in today, you in the meantime can use next_occurrence(Time.now.utc)

Owner

seejohnrun commented Nov 13, 2012

All set
Just a note: Time.new(2012, 2, 1, 0, 0, 0, 0).utc? # false even if the #utc_offset == 0

I don't understand the fix 100%. What happens If I enter the occurrence time in USA East coast time and my local time is European Time? As I read this it will still give the wrong time, doesn't it?

Owner

seejohnrun commented Nov 13, 2012

The problem is -
With ActiveSupport the fix is easy, but with Ruby we'll need a way of generating Time.now accurately in any time zone. I'll give it some more thought. If the goal is UTC this fix will work, but I'll reopen the ticket and think about a better way to get you the desired behavior

@seejohnrun seejohnrun reopened this Nov 13, 2012

Collaborator

avit commented Nov 29, 2012

Poking my head into this too since I'm having related problems...

https://github.com/seejohnrun/ice_cube/blob/master/lib/ice_cube/schedule.rb#L367 causes an inconsistent bug:

Time.zone = "America/Vancouver"
local_midnight = 1.day.from_now.beginning_of_day # future
# => Thu, 29 Nov 2012 00:00:00 PST -08:00

s = IceCube::Schedule.new(local_midnight)
s.add_recurrence_rule IceCube::Rule.daily

s.next_occurrences(2)
# => [Thu, 29 Nov 2012 00:00:00 PST -08:00, Fri, 30 Nov 2012 00:00:00 PST -08:00]

The above works as expected: find_occurrences uses the future start_time (with correct time zone). Now see what happens when we just change the start time to the past:

s.start_time = Time.zone.now.beginning_of_day # past

s.next_occurrences(2)
# => [2012-11-30 00:00:00 +0000, 2012-12-01 00:00:00 +0000]

Note the occurrences have shifted by the utc_offset and the times are plain Time objects (without a time zone).

UPDATE: In the process of writing a failing example against your latest code, it looks like you fixed the UTC offset, but the ActiveSupport time zone still shouldn't get dropped whether Time.now is before or after schedule.start_time.

Maybe TimeUtil.now should take an optional parameter for the time zone in order to set it correctly and return the right class?

Collaborator

avit commented Nov 29, 2012

With ActiveSupport the fix is easy, but with Ruby we'll need a way of generating Time.now accurately in any time zone

I believe I fixed the ActiveSupport conditional, and I was just looking into this for plain Time. It should respect:

  • the start time's local zone if present (for DST)
  • the start time's utc_offset when not in utc

Currently these are respected when start_time is in the future, but not after the schedule has started. I'm adding some example specs to cover this.

Note, DST won't be respected when the time isn't local to the current system zone, so shifting to other zones can't be done without ActiveSupport:

start_time = Time.new(2012, 7, 1) # => 2012-07-01 00:00:00 -0700
start_time.zone # => "PDT"
start_time.dst? # => true

start_time.localtime("-05:00") # => 2012-07-01 02:00:00 -0500
start_time.zone # => nil
start_time.dst? # => false

start_time.localtime # => 2012-07-01 00:00:00 -0700
start_time.zone # => "PDT"
start_time.dst? # => true

Woah!! This looks like just what we need right now. Any change this can get merged in and released in a new version?

Collaborator

avit commented Nov 30, 2012

@seejohnrun Can we get your review on this?

I see 2 failing specs in your master@76a7266 for occurs_on? which looks related to Date-Time conversion over midnight boundary (aka more time zone stuff). Should those be fixed before closing this ticket?

Collaborator

avit commented Dec 2, 2012

Fixed the 2 failing specs: now it correctly respects the same time zone as the schedule's start time in all cases.

@avit avit referenced this issue Dec 2, 2012

Closed

bug in occurs_on? #95

avit added a commit that referenced this issue Dec 21, 2012

@avit avit closed this in f35b2a5 Dec 21, 2012

avit added a commit that referenced this issue Dec 21, 2012

Merge pull request #125 from avit/issues/115-time-zone-issues
Apply correct time zone and offset to occurrences based on start_time.

Fixes #115.

rlivsey added a commit to rlivsey/ice_cube that referenced this issue Jun 18, 2013

rlivsey added a commit to rlivsey/ice_cube that referenced this issue Jun 18, 2013

rlivsey added a commit to rlivsey/ice_cube that referenced this issue Jun 18, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment