Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  Dan Barry May 5th, 2008 @ 04:53 PM

  These tests only pass if your computer's clock is set to the same time zone as Time.zone is set to in the test. Change your computer's clock to any other time zone and the tests fail.

• You flagged this item as spam.
  

  Scott Fleckenstein May 5th, 2008 @ 05:02 PM

  LOL. My bad. I'll work on fixes to them :)

• You flagged this item as spam.
  

  Dan Barry May 6th, 2008 @ 12:00 AM

  I've attached a new patch that works when the machine's clock is set to a different time zone than Time.zone.

• You flagged this item as spam.
  

  Scott Fleckenstein May 6th, 2008 @ 12:20 AM

  Thanks Dan!

  I was going to work on fixing the patch tonight but I see that you up and solved the problems without me. :)

• You flagged this item as spam.
  

  Dan Barry May 6th, 2008 @ 02:57 AM

  I noticed a problem with DST due to TimeZone not knowing which period it is in. This has been fixed in the new patch (as well as the tests using time zones from both hemispheres so that there is always something that must be adjusted).

• You flagged this item as spam.
  

  Kyle Hargraves May 6th, 2008 @ 03:21 AM

  The tests are now consistently passing for me no matter my system date / time zone / DST value. +1, this is a handy feature.

• You flagged this item as spam.
  

  Kevin Glowacz May 6th, 2008 @ 03:42 PM

  I've been having trouble with time zones in my project. This helps. +1

• You flagged this item as spam.
  

  Dan Barry May 6th, 2008 @ 04:30 PM

  I just realized that strings without specified time zones are interpreted as being in UTC. While one could theoretically just use Time.zone.parse, it defeats the purpose of having all of this happen automatically.

  String#to_time should always interpret 'May 5th 2:00pm' as being 2pm in the user's time zone, not UTC. Unless someone can come up with a good reason that this shouldn't be the case, I'll add this as well.

• You flagged this item as spam.
  

  Dan Barry May 6th, 2008 @ 05:50 PM

  The newly attached patch interprets strings without time zone information as being in the current time zone.

  • 0003-String-to_time-works-for-string-with-timezone-inform.patch 6.2 KB
• 

  Geoff Buesing May 7th, 2008 @ 07:52 AM

  • Assigned user set to “Geoff Buesing”

  The time zone enhancement to String#to_time does make sense, but it might break apps that rely on the existing behavior (i.e., ignore any time zone info in the string), so I'm hesitant to change it.

  Fortunately, the stated goal of this patch -- to make ActiveRecord time zone aware attributes work with time zone information in a supplied string -- can be achieved without changing String#to_time. We'd just need to make Time.zone.parse respect time zone info in the string (as has been fixed in the existing patch) and change the AR attribute writer method to call Time.zone.parse whenever a string is passed in -- I'd be happy to pull those changes in.

  Let me know if you want to update this patch, or if you want me to take it from here. Thanks!

• You flagged this item as spam.
  

  Scott Fleckenstein May 7th, 2008 @ 03:02 PM

  time_zone_parse.diff is attached, a new patch that works as Geoff advertised, replacing #to_time with Time.zone.parse.

  Thanks for the advice.

  • time_zone_parse.diff 4.2 KB

• You flagged this item as spam.
  

  Dan Barry May 7th, 2008 @ 08:42 PM

  Geoff,

  While I'm not sure whether someone would expect a string without an explicit time zone to be interpreted as UTC or the time zone set in Time.zone, I think that you'd expect a string with a time zone specified to be interpreted as being in that time zone.

  I feel that not parsing the time zone given in a string leads to an unnecessary violation of the principle of least surprise: time.to_s.to_time will be different from the original time. It's not even the same time in a different time zone; it's the wrong time.

  Is backwards compatibility worth arguably broken behavior?

• 

  Geoff Buesing May 8th, 2008 @ 05:19 AM

  @Scott: thanks for the updated patch; I'll try to pull this in in the next couple of days.

  @Dan: String#to_time has ignored the time zone since it was first added in Rails 0.14, over three years ago, and there have been no attempts to fix this behavior since, which is why I'm hesitant to change it now without a compelling use case.

  If you like, please open a separate ticket about this particular issue -- if there's community enthusiasm for this, and no significant downsides that anyone can think of, then we're probably ok to go ahead with this change.

• 

  Geoff Buesing May 9th, 2008 @ 03:49 AM

  • State changed from “new” to “resolved”

  patch applied in http://github.com/rails/rails/co...