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

Host configuration messes up with locale parameter and routes #95

Closed
tagliala opened this issue Mar 15, 2015 · 7 comments
Closed

Host configuration messes up with locale parameter and routes #95

tagliala opened this issue Mar 15, 2015 · 7 comments
Labels
Milestone

Comments

@tagliala
Copy link
Collaborator

Hi,

I'm experiencing an issue with the host parameter set.

Repo: https://github.com/tagliala/test-route-translator-host

The issue is the following.

Given the following configuration:

RouteTranslator.config.host_locales = {
  'en.lvh.me' => :en,
  'it.lvh.me' => :it
}
# default locale = en

when I visit http://it.lvh.me:3000 I can't see the Italian translation, because something is messing up with the :locale param

Put a debugger here: https://github.com/enriclluelles/route_translator/blob/master/lib/route_translator/extensions/action_controller.rb#L14

   14:       debugger
=> 15:       tmp_locale = params[RouteTranslator.locale_param_key] || tmp_default_locale
   16:       if tmp_locale
   17:         current_locale = I18n.locale
   18:         I18n.locale    = tmp_locale
   19:       end
(byebug) params
{"controller"=>"welcome", "action"=>"index", "locale"=>"en"}
(byebug) params[RouteTranslator.locale_param_key]
"en"

By removing params[RouteTranslator.locale_param_key] I will have another major issue: the generated route includes /it

Erb template:

<%= link_to t('routes.pages'), pages_path %>

Generated html:

<a href="/it/pagine">pagine</a>

You probably know what is happening here. The solution doesn't seem trivial so I can't submit a PR at the moment

@danschultzer
Copy link
Contributor

I guess you have to use pages_native_it_path for that to work. I'm having the same issues since I expect views served on a host_locale domain would also use the native url's paths instead of /(:locale) paths.

To fix it you could add this to an initializer:

require 'route_translator/translator'
module RouteTranslator
  module Translator
    def self.route_name_for(args, old_name, suffix, kaller)
      args_hash          = args.detect{|arg| arg.is_a?(Hash)}
      args_locale = host_locales_option? && args_hash && args_hash[:locale]
      current_locale_name = I18n.locale.to_s.underscore

      locale = if args_locale
                 args_locale.to_s.underscore
               elsif kaller.respond_to?("#{old_name}_native_#{current_locale_name}_#{suffix}")
                "native_#{current_locale_name}"
               elsif kaller.respond_to?("#{old_name}_#{current_locale_name}_#{suffix}")
                 current_locale_name
               else
                 I18n.default_locale.to_s.underscore
               end

      "#{old_name}_#{locale}_#{suffix}"
    end
  end
end

AFAIK we can't know what domain the request was on, when building the route so this might be the best we can do. I would like a better solution though, since this could create routing issues.

Edit: Realized I was too far ahead in debugging my own stuff that I didn't go back to your actual issue with the locale param not being set correctly. The locale param is being received correctly here.

@xpepermint
Copy link

+1 :)

@tagliala
Copy link
Collaborator Author

tagliala commented May 2, 2015

@xpepermint I went for a custom solution to workaround this...

@danschultzer thanks and sorry for the late reply. PRs are very welcomed

@xpepermint
Copy link

@tagliala For now I created a before_filter where I set locale myself but it would be nice to have it by default (with host_locales).

@tagliala
Copy link
Collaborator Author

tagliala commented May 2, 2015

As readme states should turn off hide_locale but the behaviour is wrong

@tagliala
Copy link
Collaborator Author

tagliala commented May 2, 2015

@xpepermint I strongly advise for an around filter/action: #88 (comment)

@xpepermint
Copy link

@tagliala Ha... I already did exactly as you proposed, thx :). Looking forward for a fix...

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

Successfully merging a pull request may close this issue.

3 participants