Make frontend's LocaleController compatible with solidus_i18n #2626
Conversation
| @@ -0,0 +1,55 @@ | |||
| require 'spec_helper' | |||
There was a problem hiding this comment.
Missing magic comment # frozen_string_literal: true.
jhawthorn
force-pushed
the
frontend_locale_set
branch
from
4ad5acc to
27f410a
Compare
There was a problem hiding this comment.
Works awesome. Really like this change 👍
Note: Only works currently with solidusio/solidus_i18n#114
This no longer has any effect (we now use spree_user_return_to) and we should not be using it.
Previously we accepted only the :locale param in LocaleController#set. solidus_i18n, however, only accepts the :switch_to_locale param. This is because solidus_i18n uses the :locale param as part of it's path (ex. /en/products). For better compatiblity with solidus_i18n, and so that we can merge most of it's functionality, we should accept either param here (and probably should prefer :switch_to_locale).
Previously, we attempted to redirect back to the previous URL, and failing that, the store's root URL. Redirecting back sounds like a nice feature, but if we are using the :locale filter from the routing-filter gem (as solidus_i18n does), this isn't what we want, because the previous URL won't be prefixed by the current locale. The existing behaviour of solidus_i18n is already to always redirect to the root, so we should adopt the same behaviour in solidus itself so that that override can be removed from solidus_i18n.
Our existing route to locale#set was a GET (which it shouldn't be), this adds it again as a POST. The existing get route is left for compatibility. The route is named "select_locale" instead of "set_locale" to avoid a conflict with the existing solidus_i18n route.
jhawthorn
force-pushed
the
frontend_locale_set
branch
from
27f410a to
7d5e383
Compare
It won't cause any problems without that merged (it just won't do anything). This change will allow us to remove the decorator entirely from solidus_i18n 🎉 |
solidus_18n also takes the preferred_available_locales is the current store into account. What this PR is not. Do you plan to support this feature as well in core? |
#2627 😉 My implementation is a bit different and has different attribute names. |
Written on top of #2559
This adjusts the behaviour of frontend's
LocaleController#setso that it can be used bysolidus_i18nwithout needing to make any changes to the controller.Specifically, this plays nicely with
filter :localein routes from therouting-filtergem, which is included by solidus_i18n.