LocalizedHelper not respecting the language

rubas commented 2 years ago

Version: 0.3-dev

Somehow we can't get the LocalizedHelper to work. It's not respecting the current locale and takes always the first defined route.

The problem exists with all verbs (live, get, delete ..).
The routes are working.
We tried different combination of setting the locale with and without backend Gettext, directly in Cldr - with the same result.
Your tests in ex_cldr_routes (including the LocalizedHelpers) are passing, but we can't find any meaningful difference between your test and our setup.

What are we missing? 😵‍💫

iex> MarkoWeb.Router.LocalizedHelpers.user_session_path(%Plug.Conn{}, :create)
"/de/anmeldung"
iex(2)> Gettext.get_locale Marko.Gettext
"en"
iex(2)> Gettext.put_locale(Marko.Gettext, "fr")
nil
iex> MarkoWeb.Router.LocalizedHelpers.user_session_path(%Plug.Conn{}, :create)       
"/de/anmeldung"

Routes

user_session_path  POST    /de/anmeldung   MarkoWeb.UserSessionController :create
user_session_path  POST    /en/login              MarkoWeb.UserSessionController :create
user_session_path  POST    /fr/connexion       MarkoWeb.UserSessionController :create

Config

Router

defmodule MarkoWeb.Router do
    use Phoenix.Router
    use Marko.Cldr.Routes

  live_session :non_auth_user do
    scope "/", MarkoWeb do
      pipe_through([:browser, :redirect_if_user_is_authenticated])

      localize do
        post("/login", UserSessionController, :create)
      end
    end
  end
end

Cldr

defmodule Marko.Cldr do
  @moduledoc false

  use Cldr,
    gettext: Marko.Gettext,
    force_locale_download: true,
    providers: [
      Cldr.Territory,
      Cldr.Route
    ]
end

Gettext

defmodule Marko.Gettext do
  use Gettext, otp_app: :marko

  @spec put_locale(Gettext.locale) :: Gettext.locale | nil
  def put_locale(locale), do: Gettext.put_locale(__MODULE__, locale)

  @spec get_locale :: Gettext.locale
  def get_locale(), do: Gettext.get_locale(__MODULE__)
end

config.exs

config :marko, Marko.Gettext,
  default_locale: "en",
  allowed_locales: ~w(en de fr)

config :ex_cldr,
  default_backend: Marko.Cldr,
  json_library: Jason

kipcole9 commented 2 years ago

@rubas Thank you for the report and my apologies for the inconvenience. I'll take a look at this on Sunday (I'm in transit all of tomorrow). Hope thats ok. Will revert ASAP. The Helpers are borrowed from Phoenix and despite some tests, clearly not good enough. I'll get this sorted out.

kipcole9 commented 2 years ago

Apologies for not meeting my own deadline, the day job is definitely impacting this week. I will get to this as quickly as I can.

kipcole9 commented 2 years ago

This is the worst time-to-resolution in my 6 years of building ex_cldr, my sincere apologies.

As of commit f98f6c5 I have rewritten the LocalizedHelpers module and I'm much happier with the implementation since it no longer forks the Phoenix Helpers module.

There is one open issue I need to fix which is that if you specify an :as option to a route it will be overwritten. This is because I am using the :as option to generate one route helper per locale and using the :as option to differentiate.

I will be able to honour the :as option (still appending a locale name) and thats the last step before release I believe.

Nevertheless I wanted to share this with you as quickly as possible to get your feedback. The changelog entry currently reads:

Enhancements

Add support for live routes. Thanks to @ringofhealth for the report. Closes #1.
Uses the macros from Gettext rather than the functions when generating routes. This means that the mix tasks gettext.extract and gettext.merge now work as expected.
Rewrite the LocalizedHelpers module that now proxies into the standard Phoenix Helpers module rather than maintaining a separate forked module. As a result:
- The standard Helpers module now generates helper names that have locale suffixes. That is user_path becomes user_en_path, user_fr_path and so on.
- The LocalizedHelpers module hosts the standard helper names (like user_path) which will then call the appropriate standard helper depending on the result of Cldr.get_locale/1.

Thanks to @rubas and @ringofhealth for their extreme patience while I worked this through. Closes #1, and #4.

kipcole9 commented 2 years ago

Additional testing found a bug in helper generation so I recommend holding off reviewing for now. Will have this resolved today.

kipcole9 commented 2 years ago

OK, I think its ready for a workout now. As of commit 415b9fb any :as parameter is honoured. In addition, error messages are aligned to the standard Phoenix routing error messages/exceptions.

At the moment any errors associated with calling a valid helper, but a helper that is not localised for the current locale, will raise a Phoenix standard error but not mention an issue related to locales. Thats the next step and, I hope, the last one before the next release.

I will tackle the issue of route prefixes after that.

kipcole9 commented 2 years ago

BTW, I neglected to note that the locale that is used to drive localised routes is the Cldr locale (ie, derived from calling Cldr.get_locale/1. Its the associated :gettext_locale_name that is used to drive translations. But the correct approach is either:

iex> MyApp.Cldr.put_locale "de"                              
{:ok, #Cldr.LanguageTag<de [validated]>}
iex> MyApp.Router.LocalizedHelpers.user_path %Plug.Conn{}, :index  
"/users_de"

Or

iex> Cldr.with_locale("de", MyApp.Cldr, fn ->
  MyApp.Router.LocalizedHelpers.user_path %Plug.Conn{}, :index  
end)
"/users_de"

elixir-cldr / cldr_routes