Add ActiveSupport::TimeZone.country_zones helper #20625

Envek · 2015-06-18T20:25:55Z

I always wondered why there is a method to retrieve all time zones for US, but no such methods for another countries until I discovered that tzdata is already knows about all countries and their timezones and tzinfo gem even have an API for that.

Live example:

2.3.0 :001 > puts ActiveSupport::TimeZone.country_zones('us').map(&:to_s)
(GMT-10:00) Hawaii
(GMT-09:00) Alaska
(GMT-08:00) Pacific Time (US & Canada)
(GMT-07:00) Arizona
(GMT-07:00) Mountain Time (US & Canada)
(GMT-06:00) Central Time (US & Canada)
(GMT-05:00) Eastern Time (US & Canada)
(GMT-05:00) Indiana (East)
2.3.0 :002 > puts ActiveSupport::TimeZone.country_zones('ru').map(&:to_s)
(GMT+02:00) Kaliningrad
(GMT+03:00) Moscow
(GMT+03:00) Volgograd
(GMT+04:00) Samara
(GMT+05:00) Ekaterinburg
(GMT+06:00) Novosibirsk
(GMT+07:00) Krasnoyarsk
(GMT+08:00) Irkutsk
(GMT+09:00) Yakutsk
(GMT+10:00) Magadan
(GMT+10:00) Vladivostok
(GMT+11:00) Srednekolymsk
(GMT+12:00) Kamchatka
2.3.0 :003 > puts ActiveSupport::TimeZone.country_zones('fr').map(&:to_s)
(GMT+01:00) Paris

UPDATE: Method ActiveSupport::TimeZone.us_zones reimplemented with ActiveSupport::TimeZone.country_zones('us')

P.S> This PR is a good addition to my talk about working with timezones in Rails apps at DevConf

senny · 2015-06-19T08:10:31Z

@Envek can us_zones be implemented in terms of this new method?

Envek · 2015-06-19T08:25:22Z

@senny us_zones can be implemented, but the order of returned time zone objects in that case is different.

Envek · 2015-06-19T11:10:59Z

I added soritng to zones array, so order of zones from country_zones(:us) is matching these from us_zones and implemented us_zones with this new method.

senny · 2015-06-19T11:14:19Z

activesupport/lib/active_support/values/time_zone.rb

@@ -248,7 +249,19 @@ def [](arg)
      # A convenience method for returning a collection of TimeZone objects
      # for time zones in the USA.
      def us_zones
-        @us_zones ||= all.find_all { |z| z.name =~ /US|Arizona|Indiana|Hawaii|Alaska/ }
+        country_zones('US')


tests are showing the use of a symbol. Can we use :us?

Sure we can. Anyway it will be converted to string and upcased. Will push fix in a minute.

senny · 2015-06-19T11:26:03Z

👍 /cc @rafaelfranca

Envek · 2015-07-28T19:59:04Z

Can this PR be reviewed further? It's got stuck and I do not know why.

senny · 2015-09-07T08:52:42Z

Looks good to me. I assigned @pixeltrix to have another look.

jonatack · 2015-09-07T11:27:25Z

Nice idea @Envek.

pixeltrix · 2015-09-07T15:07:25Z

activesupport/lib/active_support/values/time_zone.rb

+            MAPPING.key(tz_id)
+          end.map do |tz_id|
+            self[MAPPING.key(tz_id)]
+          end.sort


This block is a little unwieldy - try breaking it out into a couple of private methods. Also try to do it in one map if possible so that we're not generating too many arrays. If you can't don't worry as they are cached so they're not regenerated on every request.

pixeltrix · 2015-09-07T15:08:02Z

I'm 👍 on the idea - just need to fix the merge conflicts and tidy up the core loop

Envek · 2015-09-07T15:44:46Z

I've simplified block that searches for ActiveSupport's TimeZone objects: converted select and map to map and flatten and replaced sort with sort! (unfortunately compact! not always returns self and I can not modify output of TZInfo::Country.get(code).zone_identifiers in place as it is frozen array).

Now its task should be clear.

Also rebased on top of current master.

arthurnn · 2015-10-12T19:45:09Z

activesupport/lib/active_support/values/time_zone.rb

+      def country_zones(country_code)
+        code = country_code.to_s.upcase
+        @country_zones[code] ||=
+          TZInfo::Country.get(code).zone_identifiers.map do |tz_id|


will this return a ThreadSafe::Cache ? If not, we need to use a each here instead of a map, and push things to our ThreadSafe::Cache instead.

Why should it return a ThreadSafe::Cache? I've initialized cache instance earlier and only adding key/value pairs to it, never overwriting. Key is the country code, value is the array of timezone objects.
The another cache (@lazy_zones_map) in this file used similarly — created beforehand and added timezone objects by timezone names as keys afterwards when needed.

@arthurnn: I think @Envek is using @country_zones (ThreadSafe::Cache) correctly here.

PS:

I've initialized cache instance earlier and only adding key/value pairs to it, never overwriting.

@Envek just wanted to clarify, it is thread-safe to do something like this:

cache = ThreadSafe::Cache.new cache[:foo] = [1,2] cache[:foo] = [1,2,3] # rewriting/updating cache is ok cache[:foo] += [4] # this is just syntactic sugar for cache[:foo] = cache[:foo] + [4]

however this wouldn't be thread-safe:

cache = ThreadSafe::Cache.new cache[:foo] = [1,2] cache[:foo] << 3 # updating ruby Arrays is not thread-safe

PS n. 2: ThreadSafe::Cache has been merged into https://github.com/ruby-concurrency/concurrent-ruby, so on rebase you'll need to replace ThreadSafe::Cache.new with Concurrent::Map.new.

@thedarkone thanks for the clarification! I'm only adding new key/value pairs to the cache, so no place for such a mistake, but I will remember for future use. Sadly, I can't see good docs with explanations for this neither in thread_safe nor in concurrent-ruby.
Sure, will change to Concurrent::Map as soon as core team will be satisfied and rebase time will come.

Envek · 2015-10-15T13:21:00Z

@arthurnn @pixeltrix is there any issues left? If not I will be glad to rebase.

Envek · 2016-01-05T15:46:06Z

Rebased on top of current master, changed cache storage from ThreadSafe::Cache to Concurrent::Map.

Envek · 2016-03-19T14:07:49Z

@pixeltrix could you please review this PR?

Envek · 2016-04-15T07:31:30Z

Is there anything prevents this PR from being merged?

P.S> Now with signed commit!

That helper will return time zones for any country that tzdata knows about. So it will be much simpler for non-US people to list own country time zones in HTML selects or anywhere.

Add ActiveSupport::TimeZone.country_zones helper

senny added the activesupport label Jun 19, 2015

senny reviewed Jun 19, 2015
View reviewed changes

senny assigned pixeltrix Sep 7, 2015

pixeltrix reviewed Sep 7, 2015
View reviewed changes

arthurnn reviewed Oct 12, 2015
View reviewed changes

Add ActiveSupport::TimeZone.country_zones helper

318ee54

That helper will return time zones for any country that tzdata knows about. So it will be much simpler for non-US people to list own country time zones in HTML selects or anywhere.

Envek mentioned this pull request Apr 19, 2016

Methods for ActiveSupport::Duration parsing from ISO 8601 and output to it #16917

Closed

jeremy merged commit 318ee54 into rails:master Apr 20, 2016

jeremy added a commit that referenced this pull request Apr 20, 2016

Merge pull request #20625 from Envek/add_country_zones_method

a0dcc95

Add ActiveSupport::TimeZone.country_zones helper

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ActiveSupport::TimeZone.country_zones helper #20625

Add ActiveSupport::TimeZone.country_zones helper #20625

Envek commented Jun 18, 2015 •

edited

senny commented Jun 19, 2015

Envek commented Jun 19, 2015

Envek commented Jun 19, 2015

senny Jun 19, 2015

Envek Jun 19, 2015

senny commented Jun 19, 2015

Envek commented Jul 28, 2015

senny commented Sep 7, 2015

jonatack commented Sep 7, 2015

pixeltrix Sep 7, 2015

pixeltrix commented Sep 7, 2015

Envek commented Sep 7, 2015

arthurnn Oct 12, 2015

Envek Oct 13, 2015

thedarkone Oct 14, 2015

Envek Oct 14, 2015

Envek commented Oct 15, 2015

Envek commented Jan 5, 2016

Envek commented Mar 19, 2016

Envek commented Apr 15, 2016

Add ActiveSupport::TimeZone.country_zones helper #20625

Add ActiveSupport::TimeZone.country_zones helper #20625

Conversation

Envek commented Jun 18, 2015 • edited

senny commented Jun 19, 2015

Envek commented Jun 19, 2015

Envek commented Jun 19, 2015

senny Jun 19, 2015

Choose a reason for hiding this comment

Envek Jun 19, 2015

Choose a reason for hiding this comment

senny commented Jun 19, 2015

Envek commented Jul 28, 2015

senny commented Sep 7, 2015

jonatack commented Sep 7, 2015

pixeltrix Sep 7, 2015

Choose a reason for hiding this comment

pixeltrix commented Sep 7, 2015

Envek commented Sep 7, 2015

arthurnn Oct 12, 2015

Choose a reason for hiding this comment

Envek Oct 13, 2015

Choose a reason for hiding this comment

thedarkone Oct 14, 2015

Choose a reason for hiding this comment

Envek Oct 14, 2015

Choose a reason for hiding this comment

Envek commented Oct 15, 2015

Envek commented Jan 5, 2016

Envek commented Mar 19, 2016

Envek commented Apr 15, 2016

Envek commented Jun 18, 2015 •

edited