Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time zone trouble with time_ago_in_words #63

Open
redbar0n opened this issue Sep 29, 2015 · 17 comments
Open

Time zone trouble with time_ago_in_words #63

redbar0n opened this issue Sep 29, 2015 · 17 comments
Labels

Comments

@redbar0n
Copy link
Contributor

There seems to be a problem with how dotiw overwrites the time_ago_in_words method with respect to handling of time zones.

Maybe because it is using Time.now instead of Time.zone.now ?

With dotiw:

irb(main):010:0> Time.now
=> 2015-09-29 11:57:55 +0000
irb(main):011:0> Time.zone.now
=> Tue, 29 Sep 2015 13:57:59 CEST +02:00
irb(main):016:0> helper.time_ago_in_words(3.minutes.from_now)
=> "1 hour and 2 minutes"

irb(main):018:0> helper.time_ago_in_words(Time.now+3.minutes)
=> "2 minutes"
irb(main):019:0> helper.time_ago_in_words(Time.zone.now+3.minutes)
=> "1 hour and 2 minutes"

As you can see, there comes an extra hour there. I don't know why.

In another app, without dotiw:

2.1.1 :002 > helper.time_ago_in_words(3.minutes.from_now)
 => "3 minutes" 
2.1.1 :003 > helper.time_ago_in_words(Time.now+3.minutes)
 => "3 minutes" 
2.1.1 :004 > helper.time_ago_in_words(Time.zone.now+3.minutes)
 => "3 minutes" 
@redbar0n
Copy link
Contributor Author

The 1 hour difference seems to come from:

irb(main):007:0> helper.distance_of_time_in_words(Time.now, Time.zone.now)
=> "1 hour"

Which is weird in itself, because the time zone is actually 2 hours in front of GMT:

irb(main):008:0> Time.now
=> 2015-09-29 12:23:23 +0000
irb(main):009:0> Time.zone.now
=> Tue, 29 Sep 2015 14:23:26 CEST +02:00

Does dotiw do something especially, when accounting for summer time (like CEST)?

@radar
Copy link
Owner

radar commented Sep 30, 2015

No, dotiw does nothing special when accounting for any particular timezone.

@radar
Copy link
Owner

radar commented Sep 30, 2015

It's possible this bug is caused by using Time.now. Please submit a PR that uses Time.zone.now instead.

@arjun810
Copy link

The Time.zone.now fix helped, but there's still a bug here if someone passes both times in:

[28] » from_time = Time.zone.now
=> Thu, 17 Mar 2016 02:09:31 PDT -07:00
[29] » p a.h.distance_of_time_in_words(from_time+1.hour, Time.now, {})
"1 hour and 59 minutes"
=> "1 hour and 59 minutes"

I think the cause is here:
https://github.com/radar/dotiw/blob/master/lib/dotiw/time_hash.rb#L21
(lines 21 and 22)

Are these necessary? It seems that subtracting times already handles dst properly, and these are actually causing bugs.

@redbar0n
Copy link
Contributor Author

I agree with @arjun810's analysis.

I think the problem lies in that 3.minutes.from_now actually uses Time.zone.now, which already handles DST (like CEST), and so those lines in dotiw's TimeHash class that tries to handle DST too will cause those bugs.

I think it is better to leave it to Ruby/Rails to handle time zones and DST.

http://apidock.com/rails/ActiveSupport/Duration/from_now
is alias for http://apidock.com/rails/ActiveSupport/Duration/since
which uses http://apidock.com/rails/Time/current/class
which "Returns Time.zone.now when Time.zone or config.time_zone are set, otherwise just returns Time.now."

@lauranjansen
Copy link
Collaborator

I will submit a PR to remove the weird time zone adjustment lines, which shouldn't be needed anymore.

@alexmarles
Copy link

I recently ran into this problem using the last version of the gem and I see the last response to this thread is from over a month ago. Any ideas on how to solve it or any workaround/patch to apply?

@theoreichel
Copy link

I can easily reproduce the issue with the following example:

> include ActionView::Helpers::DateHelper
> Time.zone = "CET"
> distance_of_time_in_words(Time.now.utc, 1.hour.ago)
=> "less than 1 second"

and of course this (almost) works:

> distance_of_time_in_words(Time.now.utc, 1.hour.ago.utc)
=> "59 minutes"

So forcing time to UTC seams to solve the issue, just like if comparison were done without handling timezone.

@lauranjansen
Copy link
Collaborator

I tried just removing the time zone adjustments to see if using Time.zone.now, but for me that didn't solve the issue. I will try to add the provided cases to the tests and will play around with it some more when I have time.

@ivanovv
Copy link

ivanovv commented May 7, 2020

Just my 2 cents here:

we have an issue with this gem and clockwork_web gem, all timestamps are off by an hour

irb(main):085:0> helper.distance_of_time_in_words_hash(Time.now, ClockworkWeb.last_heartbeat)
=> {:seconds=>9}
irb(main):086:0> helper.distance_of_time_in_words_hash(Time.current, ClockworkWeb.last_heartbeat)
=> {:hours=>1, :seconds=>9}

I think when we are speaking about time difference, we really don't care about th timezone, because essentially things happen at one specific point in time, timezones just define the way we show those points (timestamps), but for two specific moments in time the distance between them is always the same and doesn't depend on timezone

So I do think that converting to to_i and using the diff between two numbers is the way to go here.

@shaicoleman
Copy link

shaicoleman commented Jan 10, 2021

I think it's because it calcluates differences in times and not in dates.

The specs are failing when run in a different timezone, despite the timezone set to UTC, e.g.

$ TZ='Europe/Dublin' bundle exec rspec
rspec './spec/lib/dotiw_spec.rb[1:1:9]' # A better distance_of_time_in_words #distance_of_time 14515200 == 5 months, 2 weeks, and 1 day
rspec './spec/lib/dotiw_spec.rb[1:4:10]' # A better distance_of_time_in_words #distance_of_time_in_words should be 11 months and 2 days
rspec './spec/lib/dotiw_spec.rb[1:4:11]' # A better distance_of_time_in_words #distance_of_time_in_words should be 11 months, 2 days, and 1 minute
rspec './spec/lib/dotiw_spec.rb[1:4:12]' # A better distance_of_time_in_words #distance_of_time_in_words should be 1 year, 4 weeks, and 1 day
rspec './spec/lib/dotiw_spec.rb[1:4:15]' # A better distance_of_time_in_words #distance_of_time_in_words should be 10 months, 4 weeks, and 2 days
rspec './spec/lib/dotiw_spec.rb[1:4:34:6]' # A better distance_of_time_in_words #distance_of_time_in_words without finish time should be 5 months, 2 weeks, and 1 day

Also to_time is parsed differently, even when setting the timezone, e.g.

$ TZ='Europe/Dublin' bundle exec irb
require './lib/dotiw'
Time.zone = 'UTC'
'2001-05-01'.to_time.utc 
# => 2001-04-30 23:00:00 UTC
Time.parse('2001-05-01').utc
# => 2001-04-30 23:00:00 UTC
'2001-05-01'.to_time(:utc)
# => 2001-05-01 00:00:00 UTC

When working with dates, the calculation should do the calculation as dates, instead of converting them to times.
Alternatively, another set of functions should be added, e.g. distance_of_date_in_words

If your app doesn't care at all about timezones, a workaround is to set the TZ environment variable, e.g. ENV['TZ'] = 'UTC'

@dblock
Copy link
Collaborator

dblock commented Jan 11, 2021

I would like this to be documented/improved. Please contribute!

@ivanovv
Copy link

ivanovv commented May 5, 2022

I believe this issue has been fixed by #128

@shaicoleman
Copy link

shaicoleman commented May 5, 2022

@ivanovv , still not fixed, e.g. the specs are still failing when running TZ='Europe/Dublin' bundle exec rspec

@ivanovv
Copy link

ivanovv commented May 5, 2022

interesting!

it is def fixed for my use case

5.3.2

irb(main):041:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"].in_time_zone
=> "less than 1 second"
irb(main):042:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"]
=> "12 seconds"
irb(main):043:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"]
=> "13 seconds"
irb(main):044:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"]
=> "14 seconds"
irb(main):045:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"].in_time_zone
=> "less than 1 second"
irb(main):046:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"].in_time_zone
=> "less than 1 second"
irb(main):047:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"]
=> "20 seconds"

it says less than 1 second because the result is negative

5.3.3

irb(main):004:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"]
=> "32 seconds"
irb(main):005:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"].in_time_zone
=> "42 seconds"
irb(main):006:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"].in_time_zone
=> "44 seconds"
irb(main):007:0> helper.time_ago_in_words  ClockworkWeb.last_runs["a_recurring_job"]
=> "46 seconds"

@dblock
Copy link
Collaborator

dblock commented May 5, 2022

@shaicoleman What's your output? Maybe it matters what time of day these specs run? :) Passes on my machine rn (EST 1:120PM).

@shaicoleman
Copy link

shaicoleman commented May 5, 2022

TZ='Europe/Dublin' bundle exec rspec
rspec './spec/lib/dotiw_spec.rb[1:1:9]' # A better distance_of_time_in_words #distance_of_time 14515200 == 5 months, 2 weeks, and 1 day # got: "5 months and 2 weeks"
rspec './spec/lib/dotiw_spec.rb[1:4:10]' # A better distance_of_time_in_words #distance_of_time_in_words should be 11 months and 2 days # got: "11 months, 2 days, and 2 hours"
rspec './spec/lib/dotiw_spec.rb[1:4:11]' # A better distance_of_time_in_words #distance_of_time_in_words should be 11 months, 2 days, and 1 minute # got: "11 months, 2 days, 2 hours, and 1 minute"
rspec './spec/lib/dotiw_spec.rb[1:4:12]' # A better distance_of_time_in_words #distance_of_time_in_words should be 1 year, 4 weeks, and 1 day # got: "1 year, 4 weeks, 1 day, and 22 hours"
rspec './spec/lib/dotiw_spec.rb[1:4:15]' # A better distance_of_time_in_words #distance_of_time_in_words should be 10 months, 4 weeks, and 2 days # got: "10 months, 4 weeks, 2 days, and 2 hours"
rspec './spec/lib/dotiw_spec.rb[1:4:34:6]' # A better distance_of_time_in_words #distance_of_time_in_words without finish time should be 5 months, 2 weeks, and 1 day # got: "5 months and 2 weeks"
rspec './spec/lib/dotiw_spec.rb[1:4:35:3]' # A better distance_of_time_in_words #distance_of_time_in_words with mixed inputs should be 5 months, 2 weeks, and 1 day # got: "5 months and 2 weeks"

The following timezones have failing specs (Ubuntu 20.04, Ruby 2.7). Different timezones have different failing specs

Africa/Casablanca
Africa/El_Aaiun
Africa/Khartoum
Africa/Windhoek
America/Argentina/San_Luis
America/Bahia_Banderas
America/Cancun
America/Caracas
America/Eirunepe
America/Fort_Nelson
America/Grand_Turk
America/Havana
America/Metlakatla
America/North_Dakota/Beulah
America/Punta_Arenas
America/Rio_Branco
America/Santarem
Antarctica/Davis
Antarctica/Macquarie
Antarctica/Mawson
Antarctica/Palmer
Antarctica/Troll
Asia/Anadyr
Asia/Barnaul
Asia/Chita
Asia/Choibalsan
Asia/Dhaka
Asia/Gaza
Asia/Hebron
Asia/Irkutsk
Asia/Jerusalem
Asia/Kamchatka
Asia/Karachi
Asia/Khandyga
Asia/Krasnoyarsk
Asia/Magadan
Asia/Novokuznetsk
Asia/Novosibirsk
Asia/Omsk
Asia/Pyongyang
Asia/Qyzylorda
Asia/Sakhalin
Asia/Srednekolymsk
Asia/Tehran
Asia/Tomsk
Asia/Ust-Nera
Asia/Vladivostok
Asia/Yakutsk
Asia/Yekaterinburg
Atlantic/Stanley
Australia/Lord_Howe
Europe/Astrakhan
Europe/Dublin
Europe/Istanbul
Europe/Kaliningrad
Europe/Kirov
Europe/Minsk
Europe/Moscow
Europe/Samara
Europe/Saratov
Europe/Simferopol
Europe/Ulyanovsk
Europe/Volgograd
Pacific/Bougainville
Pacific/Norfolk

With other timezones, the specs pass, e.g.

TZ='America/New_York' bundle exec rspec

Sample script to test over different timezones

#!/usr/bin/env ruby
timezones = `timedatectl list-timezones`.lines
results = {}
timezones.each do |tz|
  tz.chomp!
  next if File.symlink?("/usr/share/zoneinfo/#{tz}")
  ENV['TZ'] = tz
  result = `bundle exec rspec`
  puts "#{tz} - #{$?.success?}"
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants