Removed override of encode_with in TimeWithZone #10481

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

pftg commented May 5, 2013

Added for ActiveSupport::TimeWithZone to serialize to yaml
with TimeZone information like Time#to_yaml does,
instead saving only UTC value.

Closes #9183

Contributor

pftg commented May 5, 2013

Owner

pixeltrix commented May 6, 2013

@pftg waiting on comments on the original ticket - the UTC is an intentional change and I need to understand why it was made before I can apply this.

@pftg pftg Removed override of encode_with in TimeWithZone
Added for ActiveSupport::TimeWithZone to serialize to yaml
with TimeZone information like Time#to_yaml does,
instead saving only UTC value.

Closes #9183
8c9da2e

xinuc commented Oct 8, 2014

wow, it's been a while.

Having the same problem with rails 4.1.6.
I think this should be merged.

Owner

pixeltrix commented Oct 8, 2014

Not the right fix because it's still not a round trip:

>> Time.current.class
=> ActiveSupport::TimeWithZone
>> YAML.load(Time.current.to_yaml).class
=> Time

consequently the time will be converted to a local time and not the original timezone:

>> Time.current.in_time_zone('Hawaii')
=> Tue, 07 Oct 2014 23:43:02 HST -10:00
>> YAML.load(Time.current.in_time_zone('Hawaii').to_yaml)
=> 2014-10-08 10:43:05 +0100

xinuc commented Oct 8, 2014

any idea what is the right fix?

Contributor

pftg commented Oct 8, 2014

@pixeltrix thanks for your review, I'll try to figure is it possible to fix this

Owner

pixeltrix commented Oct 8, 2014

It's probably going to take a custom init_with and encode_with method to get it to work properly. The problem is that ISO 8601 doesn't have a way of representing the actual time zone and not just the offset so you're always going to run into problems by encoding it as a scalar value.

The only alternative is to encode it as a Ruby object with a hash that's passed to init_with. The best way to do this is by a encoding the time zone using the tzinfo identifier and the time as a UTC instance. You could try to tidy it up with a custom init_with/encode_with implementation on ActiveSupport::TimeZone as well.

Contributor

repinel commented Jun 21, 2015

@pixeltrix Could this be closed? It was fixed by 3aa26cf

pftg closed this Jan 12, 2016

pftg deleted the jetthoughts:9183_store_timezone_for_time_with_zone_on_to_yaml branch Jan 12, 2016

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