Permalink
Browse files

Make `Time.zone.parse` to work with JavaScript date strings

Chrome, Safari and Firefox serialize Date objects to strings such
as 'Mon May 28 2012 00:00:00 GMT-0700 (PDT)'. When these strings
are parsed the zone is interpreted as 'GMT-0700' which doesn't
exist in the TzInfo list of timezones.

By taking advantage of the improved date/time handling in 1.9.3
we can use `Date._parse` and the `:offset` value which is parsed
correctly.

Three tests were amended to make them pass:

1.  test_parse_with_old_date

    This needed changing to a different value because the original
    value was before EST was adopted so was being changed to a
    LMT (Local Mean Time) value after the change. It didn't before
    because `DateTime` just has offsets from UTC not timezones.

2.  test_parse_should_not_black_out_system_timezone_dst_jump

    Changed the implementation of this test as the stubs were
    dependent on internal implementation details of the test.
    Confirmed that the modified test still failed when the
    implementation of `parse` was restored to pre-#5571.

3.  test_parse_should_black_out_app_timezone_dst_jump

    Ditto.

Closes #5770.
  • Loading branch information...
1 parent 0a1c611 commit 005d910624bbfa724b638426a000c8074d4201a2 @pixeltrix pixeltrix committed Dec 1, 2012
@@ -1,5 +1,7 @@
## Rails 4.0.0 (unreleased) ##
+* Make `Time.zone.parse` to work with JavaScript format date strings. *Andrew White*
+
* Add `DateTime#seconds_until_end_of_day` and `Time#seconds_until_end_of_day`
as a complement for `seconds_from_midnight`; useful when setting expiration
times for caches, e.g.:
@@ -278,18 +278,23 @@ def at(secs)
# Time.zone.now # => Fri, 31 Dec 1999 14:00:00 HST -10:00
# Time.zone.parse('22:30:00') # => Fri, 31 Dec 1999 22:30:00 HST -10:00
def parse(str, now=now)
- date_parts = Date._parse(str)
- return if date_parts.empty?
- time = Time.parse(str, now) rescue DateTime.parse(str)
-
- if date_parts[:offset].nil?
- if date_parts[:hour] && time.hour != date_parts[:hour]
- time = DateTime.parse(str)
- end
-
- ActiveSupport::TimeWithZone.new(nil, self, time)
+ parts = Date._parse(str, false)
+ return if parts.empty?
+
+ time = Time.new(
+ parts.fetch(:year, now.year),
+ parts.fetch(:mon, now.month),
+ parts.fetch(:mday, now.day),
+ parts.fetch(:hour, now.hour),
+ parts.fetch(:min, now.min),
+ parts.fetch(:sec, now.sec) + parts.fetch(:sec_fraction, 0),
+ parts.fetch(:offset, 0)
+ )
+
+ if parts[:offset]
+ TimeWithZone.new(time.utc, self)
else
- time.in_time_zone(self)
+ TimeWithZone.new(nil, self, time)
end
end
@@ -178,8 +178,8 @@ def test_parse_string_with_timezone
def test_parse_with_old_date
zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)']
- twz = zone.parse('1850-12-31 19:00:00')
- assert_equal [0,0,19,31,12,1850], twz.to_a[0,6]
+ twz = zone.parse('1883-12-31 19:00:00')
+ assert_equal [0,0,19,31,12,1883], twz.to_a[0,6]
assert_equal zone, twz.time_zone
end
@@ -204,21 +204,25 @@ def test_parse_with_incomplete_date
end
def test_parse_should_not_black_out_system_timezone_dst_jump
- zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)']
- zone.stubs(:now).returns(zone.now)
- Time.stubs(:parse).with('2012-03-25 03:29', zone.now).
- returns(Time.local(0,29,4,25,3,2012,nil,nil,true,"+03:00"))
- twz = zone.parse('2012-03-25 03:29')
- assert_equal [0, 29, 3, 25, 3, 2012], twz.to_a[0,6]
+ with_env_tz('EET') do
+ zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)']
+ twz = zone.parse('2012-03-25 03:29:00')
+ assert_equal [0, 29, 3, 25, 3, 2012], twz.to_a[0,6]
+ end
end
def test_parse_should_black_out_app_timezone_dst_jump
- zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)']
- zone.stubs(:now).returns(zone.now)
- Time.stubs(:parse).with('2012-03-11 02:29', zone.now).
- returns(Time.local(0,29,2,11,3,2012,nil,nil,false,"+02:00"))
- twz = zone.parse('2012-03-11 02:29')
- assert_equal [0, 29, 3, 11, 3, 2012], twz.to_a[0,6]
+ with_env_tz('EET') do
+ zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)']
+ twz = zone.parse('2012-03-11 02:29:00')
+ assert_equal [0, 29, 3, 11, 3, 2012], twz.to_a[0,6]
+ end
+ end
+
+ def test_parse_with_javascript_date
+ zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)']
+ twz = zone.parse("Mon May 28 2012 00:00:00 GMT-0700 (PDT)")
+ assert_equal Time.utc(2012, 5, 28, 7, 0, 0), twz.utc
end
def test_utc_offset_lazy_loaded_from_tzinfo_when_not_passed_in_to_initialize

10 comments on commit 005d910

Hey mate, this apparently broke an AR test according to Travis, mind taking a look?

Owner

pixeltrix replied Dec 1, 2012

Looking at it now

Owner

pixeltrix replied Dec 1, 2012

Should be fixed by c89b6c4 - AR tests pass locally for me afterwards.

BTW, why did you change this line to pass false as the second param? Was just wondering...
181: parts = Date._parse(str, false)

Owner

pixeltrix replied Jan 8, 2014

Because leaving it as true (which is the default) results in weird parsing - see: #12458 (comment)

I see, there's been a whole discussion :) To me it seems the best solution would be to let people pass that param along, depending on their needs, which may be different in different circumstances... We had to monkeypatch that in for now

Owner

pixeltrix replied Jan 8, 2014

@mitkouwcad did you see the bit about it not working like you think - it's parsed as YY/MM/DD' which is probably not what your users want - use chronic or timeliness if you want natural language date parsing.

Any reason it can't be passed in with the default value of false? That way the Rails abstraction still provides access to the underlying library it's relying on, fixes the "regression", and allows the developer to decide how to provide what the users want.

Owner

pixeltrix replied May 7, 2014

@saltracer we could do but then that's something else to document, support, test etc. so has an ongoing cost which I don't think is worth it for an option that isn't what you really want if you read my response on #12458 - the behaviour of the second argument changed from 1.8.7 to 1.9.3.

Please sign in to comment.