Permalink
Browse files

Simplify type casting code for timezone aware attributes

With the addition of String#in_time_zone and Date#in_time_zone
we can simplify the type casting code by checking if the value
has an `in_time_zone` method.
  • Loading branch information...
1 parent bc982cb commit 41ff6a10216f48f43605a1f9cd6094765cab750f @pixeltrix pixeltrix committed Jan 26, 2013
Showing with 5 additions and 10 deletions.
  1. +5 −10 activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
@@ -33,17 +33,12 @@ module ClassMethods
def define_method_attribute=(attr_name)
if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name])
method_body, line = <<-EOV, __LINE__ + 1
- def #{attr_name}=(original_time)
- original_time = nil if original_time.blank?
- time = original_time
- unless time.acts_like?(:time)
- time = time.is_a?(String) ? Time.zone.parse(time) : time.to_time rescue time
- end
- time = time.in_time_zone rescue nil if time
+ def #{attr_name}=(time)
+ time_with_zone = time.respond_to?(:in_time_zone) ? time.in_time_zone : nil
previous_time = attribute_changed?("#{attr_name}") ? changed_attributes["#{attr_name}"] : read_attribute(:#{attr_name})
- write_attribute(:#{attr_name}, original_time)
- #{attr_name}_will_change! if previous_time != time
- @attributes_cache["#{attr_name}"] = time
+ write_attribute(:#{attr_name}, time)
+ #{attr_name}_will_change! if previous_time != time_with_zone
+ @attributes_cache["#{attr_name}"] = time_with_zone
end
EOV
generated_attribute_methods.module_eval(method_body, __FILE__, line)

6 comments on commit 41ff6a1

Owner

jeremy replied Feb 13, 2013

Subtle behavior change here. When you assign a Date, it used to be converted to a time then converted to the current tz. Now it skips the to_time.

Assigning Date.today in Rails 3:

>> Date.today.to_time.in_time_zone
=> Wed, 13 Feb 2013 07:00:00 UTC +00:00

Assigning Date.today in Rails 4:

>> Date.today.in_time_zone
=> Wed, 13 Feb 2013 00:00:00 UTC +00:00

This is what I'd expect, but heads up that it may break some date/time conversion hacks people are doing to work around the local/utc snafu.

Contributor

trevorturk replied Feb 14, 2013

I wonder if this is worth mentioning in the CHANGELOG, actually. I think the change is smart, but it might surprise some people.

rails3> Post.new(created_at: Date.today).created_at
=> Wed, 13 Feb 2013 06:00:00 UTC +00:00

rails4> Post.new(created_at: Date.today).created_at
=> Wed, 13 Feb 2013 00:00:00 UTC +00:00
Owner

pixeltrix replied Feb 14, 2013

@jeremy I'll add a CHANGELOG entry. If you're supplying a Date to created_at most people would be using a date column type hopefully, in which case this conversion won't be called.

Owner

pixeltrix replied Feb 14, 2013

Also it's more consistent mixing strings and dates.

Rails 3.2:

>> Time.zone = 'EST'
=> "EST"
>> Date.civil(2000, 1, 1).to_time.in_time_zone
=> Fri, 31 Dec 1999 19:00:00 EST -05:00
>> Time.zone.parse("2000-01-01")
=> Sat, 01 Jan 2000 00:00:00 EST -05:00

Rails 4.0

>> Time.zone = 'EST'
=> "EST"
>> Date.civil(2000, 1, 1).in_time_zone
=> Sat, 01 Jan 2000 00:00:00 EST -05:00
>> "2000-01-01".in_time_zone
=> Sat, 01 Jan 2000 00:00:00 EST -05:00
Contributor

trevorturk replied Feb 14, 2013

I definitely think this is a good change -- just seems worth mentioning since it might take people by surprise (as it did us). Thanks, @pixeltrix!

Owner

pixeltrix replied Feb 17, 2013

Added a CHANGELOG entry in 41ff6a1

Please sign in to comment.