spree-contrib / spree_i18n

I18n translation files for Spree Commerce.
http://guides.spreecommerce.org
BSD 3-Clause "New" or "Revised" License
348 stars 762 forks source link

Login/sign-up/password recover not localized #593

Open mbajur opened 9 years ago

mbajur commented 9 years ago

Hi, i'm not sure if that's a bug on my side but all my login/sign-up/pasword recovery sections are not being translated. Also, the language picker doesn't know about current locale neither because it always shows EN no matter what's the current route locale.

Also, the action of sign-in form does not include a locale prefix so user gets redirected to the default locale home page url after successfull sign-in.

I'm using spree_auth_devise. I would love to make a PR for that but i have no idea how to fix that. Thanks

JDutil commented 9 years ago

@mbajur perhaps the translations need to be added to spree_auth_devise for your locale. Unless it's just simply that your application is thinking your current_locale is EN, which would be why the locale picker would think EN.

mbajur commented 9 years ago

It seems that my application is thinking that current_locale is EN but only for devise controllers. Because it works everywhere but on the devise pages

JDutil commented 9 years ago

I've seen that before, but I forget off the top of my head what caused it. I believe it has something to do with https://github.com/spree/spree/blob/master/core/lib/spree/core/controller_helpers/common.rb#L52-L58 being overridden by https://github.com/spree-contrib/spree_i18n/blob/master/lib/spree_i18n/controller_locale_helper.rb#L9-L25 but not used by spree_auth_devise when it should.

mbajur commented 9 years ago

But have you fixed that or just heard about it? My client is really stressed about that and i'm not sure what to tell him so even a really ugly solution will work for me ATM :)

JDutil commented 9 years ago

I thought I fixed it, but I don't have time to look into it. I'd recommend just making sure your application uses the spree_i18n version of the method I pointed out above within their devise controllers. Either override devise or do it in your application controller or something.

alepore commented 9 years ago

@mbajur it's not clear which version are you using and what are the steps to reproduce the problem. i have many spree sites where devise is properly translated

mbajur commented 9 years ago

Oh, right, i forgot to write that. Here are all the gems related to spree i'm using

gem 'spree', github: 'spree/spree', branch: 'master'
gem 'spree_gateway', github: 'spree/spree_gateway', branch: 'master'
gem 'spree_auth_devise', github: 'spree/spree_auth_devise', branch: 'master'
gem 'spree_i18n', github: 'spree-contrib/spree_i18n'

gem 'spree_mail_settings', github: 'joseramonc/spree_mail_settings', branch: 'patch-1'
gem "spree_multi_currency", github: 'spree-contrib/spree_multi_currency'
# gem 'spree_static_content', github: 'spree-contrib/spree_static_content', branch: 'master'
gem 'spree_payu_integration', github: 'RaVbaker/spree_payu_integration', branch: 'spree_version_bump'
gem 'spree_paypal_express', github: 'mbajur/better_spree_paypal_express', branch: 'master'
gem 'spree_dotpay_pl', github: 'mbajur/spree_dotpay_pl'

rails is 4.2.0 ruby 2.2.1

mbajur commented 9 years ago

Ok guys, i have made a spree_auth_devise controllers decorators and now it works as it should. However, i'm not sure if we should consider it as a bug and which side (spree_auth_devise, spree_i18n or host-app?) should fix it...

JDutil commented 9 years ago

As @alepore pointed out this should just be working... We can't assume users of spree_i18n will be using spree_auth_devise, but I think the fix should be in spree_i18n if it's not already. So I would have a check for the Devise being defined before trying to decorate it at all.

tvdeyen commented 9 years ago

We are not using spree_auth_devise, but using Devise and don't have this problems, so please make this dependent from spree_auth_devise, not Devise alone.

There is also the devise_i18n gem. Or put the translations into spree_auth_devise.

BTW: I never get the benefit in not shipping translations with the gem at all. Separating the translations from core is always problematic. There are always some hacks necessary and this is a burden. Putting GUI translations into the core of a largely used project like spree would be mandatory for me. We do this with AlchemyCMS and it doesn't hurt all, it's the opposite, it helps. You ship the documentation with the core. Why not the translations?

But hey, this is another story.

cshapeshifter commented 8 years ago

I have encountered the same problem. Gems:

gem 'spree', '3.0.4'
gem 'spree_gateway', github: 'spree/spree_gateway', branch: '3-0-stable'
gem 'braintree'
gem 'spree_auth_devise', github: 'spree/spree_auth_devise', branch: '3-0-stable'

gem 'spree_wombat', github: 'spree/spree_wombat', branch: '3-0-stable'
gem 'spree_i18n', github: 'spree-contrib/spree_i18n', branch: '3-0-stable'
gem 'spree_multi_domain', github: 'spree/spree-multi-domain', branch: '3-0-stable'
gem 'spree_mail_settings', github: 'spree-contrib/spree_mail_settings', branch: '3-0-stable'
gem "spree_product_zoom", :git => "git://github.com/spree/spree_product_zoom.git", branch: '3-0-stable'
gem 'spree_product_assembly', github: 'spree-contrib/spree-product-assembly', branch: '3-0-stable'
gem 'spree_static_content', github: 'spree-contrib/spree_static_content', branch: '3-0-stable'
gem 'spree_print_invoice', github: 'spree-contrib/spree_print_invoice', branch: '3-0-stable'

Locale settings in config/initializers/spree.rb which, in words, means: backend in English, Frontend only in German, but it should be possible to add translations to products already:

Spree::Backend::Config[:locale] = :en
Spree::Frontend::Config[:locale] = :de
SpreeI18n::Config.available_locales = [:de, :en]
SpreeI18n::Config.supported_locales = [:de]

With this, most of the frontend is German indeed, but all devise-related pages are in English.

But then I noticed something: I'm able to fix and then re-introduce the problem via the following steps:

  1. Add another language to supported_locales, i.e.:

    Spree::Backend::Config[:locale] = :en
    Spree::Frontend::Config[:locale] = :de
    SpreeI18n::Config.available_locales = [:de, :en]
    SpreeI18n::Config.supported_locales = [:de, :en]

    and restart spree.

  2. Visit a devise page, e.g. /login and observe that the language selector is turned to "EN" instead of "DE". Change it to the desired language (e.g. "DE"). Devise shows the translated page.
  3. Remove the additional supported language, i.e.:

    Spree::Backend::Config[:locale] = :en
    Spree::Frontend::Config[:locale] = :de
    SpreeI18n::Config.available_locales = [:de, :en]
    SpreeI18n::Config.supported_locales = [:de]

    Restart spree. Now, the frontend is German, including the devise pages, even though the settings are identical to those at the beginning.

  4. To re-introduce the problem, just re-enable the ":en" locale, restart, switch to "EN" on a devise-related page, then disable the ":en" locale, restart -> Now, once again, the shop is German but the devise pages remain English.

So it seems like the devise compontent stores this value somewhere and ignores the site-wide settings if only 1 language is available.

PS: Just noticed that this only works until a logout occurs. Then the devise pages are reset to english no matter which language was set and displayed earlier.

cshapeshifter commented 8 years ago

I though I had half-heartedly worked around the issue by setting config.i18n.default_locale = :de as this fixes the devise pages to the DE locale, but that doesn't work out well for me. Since I use the :en backend, it so happens that inputting something in the English back-end dialogs enters the data into the database for the DE locale and only in the translations is it possible to set the english contents. It's not a bug, but it's confusing, so I'm resetting default_locale = :en, but now the devise pages are English again.

@mbajur Would you be so kind as to post the decorators you implemented to fix the locale of the devise pages? Thanks!

mbajur commented 8 years ago

I'm not sure if that's it (i'm not able to test it, i just have a source code) but i have a decorator in app/controllers/spree/ for each devise controller with the following code (obviously just the decorated controller name is different):

Spree::UserConfirmationsController.class_eval do
  include SpreeI18n::ControllerLocaleHelper
end
cshapeshifter commented 8 years ago

Thanks for that. Oddly enough, this has no effect for me. I know the decorators are working because I'm modifying other stuff, but the devise pages are still ignoring the Spree::Frontend::Config[:locale] = :de

alepore commented 8 years ago

forgot about this issue, but i'm also including ControllerLocaleHelper on all User* controllers on my apps. This is required and i think spree_i18n should do this

cshapeshifter commented 8 years ago

I'm really confused why this works for you guys. As I understand the method set_user_language in ControllerLocaleHelper, it looks for a param based config, then for the return value of config_locale, which is what I'm interested in, and then falls back on the default configured by rails.

It seems like config_locale (which comes from Spree::StoreController) isn't defined for the user* controllers. See this rails console output:

irb(main):038:0> Spree::StoreController.new.send(:config_locale)
=> "de"
irb(main):039:0> Spree::UserSessionsController.new.send(:config_locale)
NoMethodError: undefined method `config_locale' for #<Spree::UserSessionsController:0x0000000564dd40>

This is with the following decorator in app/controllers/spree/user_sessions_controller_decorator.rb:

Spree::UserSessionsController.class_eval do
  include SpreeI18n::ControllerLocaleHelper
end

Does this just mean that the decorator workaround will only work for param based localization, but not for the default config from Spree::Frontend::Config[:locale]?

PS: In this regard, re-adding a config_locale method to my decorators fixes the problem for me as well. But it's not pretty I guess.

jhovad commented 7 years ago

Just saying that issue is still there (verision 3.3) and that the advice about creating the decorator described above really works (I have absolutely no idea how it's possible :D )