Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

respect I18n.available_locales if present, otherwise load all translations #1470

Closed
wants to merge 2 commits into from
@jpmckinney

rails-i18n has a very sophisticated way of loading translation files, which ActiveAdmin should use. If you read this short paragraph of the documentation, you'll see that the rails-i18n method respects I18n.available_locales if present. Otherwise, it loads all locale files. Currently, users have no way to ensure that AA respects I18n.available_locales.

@travisbot

This pull request passes (merged 1c4bca1 into b5d23bb).

@pcreux
Collaborator

This is great!

Could you spec this?

@pcreux pcreux closed this
@pcreux pcreux reopened this
@travisbot

This pull request passes (merged 1c4bca1 into b5d23bb).

@jpmckinney

specs added (mostly copied from Rails-i18n)

@travisbot

This pull request fails (merged 7543090 into b5d23bb).

@jpmckinney

Specs pass when run individually, but fail when run all together due to Exception encountered: #<RuntimeError: You cannot have more than one Rails::Application>. We would have to rewrite all the require 'spec_helper' to be something like require 'spec_helper_with_rails' to get around this constraint. The specs for i18n integration need to create different Rails apps, so we can't just re-use the Rails app that AA already had for tests.

Let me know if you want me to make this change.

@sepastian

I ran into the same issues that led jpmckinney to provide this pull request.

What is the status of it? Can it be expected in AA any time soon?

@gregbell
Owner

@jpmckinney Maybe we could put them into their own spec/i18n top level directory then run them individually like the on https://github.com/gregbell/active_admin/blob/master/tasks/test.rake#L10

Would this solve the problem?

@gregbell
Owner

If something like that isn't going to work, then probably we need to write some tests that mock out Rails and just ensure that we are using it correctly, rather than actually depend on Rails in tests.

@jpmckinney

Hadn't thought of that - I'll give it a try!

@keysen

I'm encountering the problem on the new 0.5 version and with the git master branch, the only thing I can do it's to use the ugly way of @hron84 (thanks to him) to fix this bug. Anyone has an idea ?

@johnnyshields

+1 to this

Using @jpmckinney's "i18n_fixes_take_two" branch fixed an issue with locale detection in my routes.rb file, specifically this works now:

scope "(:locale)", :locale => /en|ja/ do
    ActiveAdmin.routes(self)
end

I have a use case to have the admin console be both in English and Japanese, i.e.:

/en/admin/dashboard
/ja/admin/dashboard
/admin/dashboard --> defaults to English

I've user-tested fairly thoroughly and have not encountered any issues yet.

@hron84

I impatiently wait for this PR be merged. I'd like drop out my hack from my app because it slows down the bootstrap process.

@johnnyshields

@jpmckinney or @gregbell, is there any progress on this pull request?

@pcreux
Collaborator

@jpmckinney I'd love to merge this one. Do you have some time to update the specs?

@jpmckinney

Not in the short term.

@pcreux
Collaborator

:confused:

@atipugin

Tried out this fix, works fine! Can't wait to see it in the master. :+1:

@badosu

This should be on master. Ran into problems that this Pull Request could solve.

:+1:

@seanlinsley
Owner

@jpmckinney, want some help on this?

@Dagnan

As the author of #434 I'm looking forward to have this :) Great!

@seanlinsley
Owner

@gregbell by the way, I'm going to work on updating this PR so it works with the latest code.

@netmute netmute referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@yas4891

I still have issue #434 with active 0.6.0 - is that to be expected?

@mbixby

Two years after #434 was opened, what's the status on this PR?

@seanlinsley
Owner

As you can see, nothing has happened in those two years. I'll eventually get to it, but you're more than willing to update this PR.

@johnnyshields

Still :+1: for this. I18n handling remains a major annoyance in AA.

@seanlinsley
Owner

@johnnyshields would you mind taking this on?

@johnnyshields

@seanlinsley it looks like @netmute actually merged @jpmckinney's code in commit ab1c858. Maybe this is now fixed?

@seanlinsley
Owner

That's on what I assume to be a fork of AA that @netmute later deleted.

@johnnyshields

@seanlinsley did some more investigation. Seems like this commit has probably fixed the issue (apparently I hadn't bundled the latest AA):

8d4b780

Previously the problem was that AA had idiosyncratic code to load I18n which overrode the natural way that Rails I18n does it, and was loading more files than needed. In the process of switching AA to be a Rails::Engine, the idiosyncratic code has been removed, and I18n is done via to Rails I18n. So, unless if people are still seeing an issue on latest master (@mbixby, @luccamordente, @Pickachu?) we can probably close this.

@netmute

Sorry for the confusion that commit caused. I actually only merged it in a fork.

@johnnyshields

@netmute not at all, it was github that confused me, after you deleted your fork clicking ab1c858 in github makes it look like its part of the gregbell repo

@timoschilling

Since we switched from a Railtie to a Engine wich handles the locale loading, this can be cloesed. I even still see a problem in the Railtie locale loading, but that should be changed in Rails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 5, 2012
  1. @jpmckinney
Commits on Jul 6, 2012
  1. @jpmckinney
This page is out of date. Refresh to see the latest.
View
1  .gitignore
@@ -35,3 +35,4 @@ public
.rspec
.rails-version
.rbenv-version
+log
View
1  Gemfile
@@ -41,4 +41,5 @@ group :test do
gem 'guard-rspec'
gem "guard-coffeescript"
gem 'jasmine'
+ gem 'spork'
end
View
25 lib/active_admin.rb
@@ -49,11 +49,26 @@ module ActiveAdmin
autoload :ViewHelpers, 'active_admin/view_helpers'
autoload :Views, 'active_admin/views'
- class Railtie < ::Rails::Railtie
- config.after_initialize do
- # Add load paths straight to I18n, so engines and application can overwrite it.
- require 'active_support/i18n'
- I18n.load_path += Dir[File.expand_path('../active_admin/locales/*.yml', __FILE__)]
+ # @see https://github.com/svenfuchs/rails-i18n/blob/master/lib/rails_i18n/railtie.rb
+ class Railtie < ::Rails::Railtie #:nodoc:
+ initializer 'active_admin-i18n' do |app|
+ ActiveAdmin::Railtie.instance_eval do
+ pattern = pattern_from app.config.i18n.available_locales
+
+ add("active_admin/locales/#{pattern}.yml")
+ end
+ end
+
+ protected
+
+ def self.add(pattern)
+ files = Dir[File.join(File.dirname(__FILE__), pattern)]
+ I18n.load_path.concat(files)
+ end
+
+ def self.pattern_from(args)
+ array = Array(args || [])
+ array.blank? ? '*' : "{#{array.join ','}}"
end
end
View
26 spec/integration/backend_reload_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper_without_rails'
+require 'support/fake_app'
+
+# @see https://github.com/svenfuchs/rails-i18n/blob/master/spec/integration/backend_reload_spec.rb
+describe "I18n backend reloading" do
+ let(:app) do
+ ActiveAdmin::Spec::FakeApp
+ end
+
+ context "when called twice" do
+ let(:errors) do
+ app.run(lambda do
+ $stderr = StringIO.new
+ 2.times do
+ I18n.reload!
+ I18n.t :hello
+ end
+ $stderr.string
+ end)
+ end
+
+ it "doesn't produce warnings" do
+ errors.should == ''
+ end
+ end
+end
View
47 spec/integration/optional_locale_loading_spec.rb
@@ -0,0 +1,47 @@
+# encoding: utf-8
+require 'spec_helper_without_rails'
+require 'support/fake_app'
+
+# @see https://github.com/svenfuchs/rails-i18n/blob/master/spec/integration/optional_locale_loading_spec.rb
+describe "ActiveAdmin-i18n" do
+ let(:app) do
+ ActiveAdmin::Spec::FakeApp
+ end
+
+ context "when i18n.available_locales are specified in config" do
+ let(:i18n_results) do
+ de_translate = lambda { I18n.t "active_admin.view", :locale => :de }
+ ru_translate = lambda { I18n.t "active_admin.view", :locale => :ru }
+
+ app.run(de_translate, ru_translate) do |config|
+ config.i18n.available_locales = [:ru, :en]
+ end
+ end
+
+ it "loads only specified locales" do
+ de_translate, ru_translate = *i18n_results
+
+ de_translate.should == 'translation missing: de.active_admin.view'
+ ru_translate.should == 'Открыть'
+ end
+ end
+
+ context "when single locale is assigned to i18n.available_locales" do
+ let(:translations) do
+ fr = lambda { I18n.t "active_admin.view" }
+ it = lambda { I18n.t "active_admin.view", :locale => :it }
+
+ app.run(fr, it) do |config|
+ config.i18n.default_locale = 'fr'
+ config.i18n.available_locales = 'fr'
+ end
+ end
+
+ it "loads only this locale" do
+ fr_translation, it_translation = *translations
+
+ fr_translation.should == 'Voir'
+ it_translation.should == 'translation missing: it.active_admin.view'
+ end
+ end
+end
View
23 spec/integration/translation_spec.rb
@@ -0,0 +1,23 @@
+# encoding: utf-8
+require 'spec_helper_without_rails'
+require 'support/fake_app'
+
+# @see https://github.com/svenfuchs/rails-i18n/blob/master/spec/integration/translation_spec.rb
+describe "Translation" do
+
+ let(:app) do
+ ActiveAdmin::Spec::FakeApp
+ end
+
+ context "when default locale is not English" do
+ let(:translation) do
+ app.run lambda { I18n.t("active_admin.view") } do |config|
+ config.i18n.default_locale = :de
+ end
+ end
+
+ it "is available" do
+ translation.should == "Anzeigen"
+ end
+ end
+end
View
28 spec/support/fake_app.rb
@@ -0,0 +1,28 @@
+require 'spork'
+
+module ActiveAdmin
+ module Spec
+ module FakeApp
+ # Initialize Rails app in a clean environment.
+ # @param tests [Proc] which have to be run after app was initialized
+ # @return [Array, Object] single result if one test was passed given,
+ # otherwise returns an array of results
+ def self.run(*tests)
+ forker = Spork::Forker.new do
+ require 'active_admin'
+ require 'action_controller/railtie'
+
+ app = Class.new(Rails::Application)
+ app.config.active_support.deprecation = :log
+
+ yield(app.config) if block_given?
+ app.initialize!
+
+ results = tests.map &:call
+ results.size == 1 ? results.first : results
+ end
+ forker.result
+ end
+ end
+ end
+end
Something went wrong with that request. Please try again.