-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
Fixes language switching to default language #2689
Fixes language switching to default language #2689
Conversation
d47b425
to
1cd021c
Compare
@robinboening thanks. The spec failures seem to be related. Mind taking a look? |
1cd021c
to
1c2b53e
Compare
I overlooked another session check in a spec and might have forgotten to push again last night. All specs but 1 are fixed. I am failing to see how this one failing admin spec could possibly be related. I could not find any reference to a language dependency in the session in the Admin Controller stack (ControllerActions -> BaseController -> Admin::BaseController -> Admin::PagesController). Any idea how this could be related? |
@robinboening the CurrentLanguage module is mixed into the Admin Pages controller and this expects that the |
@robinboening any chance to look into failing specs, so we can add this to 7.1 milestone? |
4ea5c7d
to
19e4094
Compare
1c2b53e
to
f68f5f4
Compare
We need to still store the language in the session for admin requests, otherwise the language switch in the admin would not be persisted. This is why the spec stilled failed. I amended the fix to your commit. |
f68f5f4
to
6c87e35
Compare
Closes AlchemyCMS#2632 From this commit on the language id won't be stored in the session any more for frontend requests. Upon every page visit the language id got stored in the session. When switching from a secondary language to the default language no language param is passed to the controller. The order how the language is found prioritized the session and the default was just a fallback. Coming from a secondary language and therefore it's id in the session, it would never load the default language, but the secondary. Storing the language in the session is still done in the admin section.
6c87e35
to
f63157e
Compare
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
[7.0-stable] Merge pull request #2689 from robinboening/fix_switching_to_default_language
[7.1-stable] Merge pull request #2689 from robinboening/fix_switching_to_default_language
Thanks for taking over and fixing it! |
Closes #2632
From this commit on the language id won't be stored in the session any more.
Upon every page visit the language id got stored in the session. When switching from a secondary language to the default language no language param is passed to the controller. The order how the language is found prioritized the session and the default was just a fallback. Coming from a secondary language and therefore it's id in the session, it would never load the default language, but the secondary.
Storing the language in the session was most likely a performance optimization. Removing this should not impact current behaviour.