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

LocaleSelectorFilter shouldn't set default locale #7208

Closed
pperejon opened this issue Aug 10, 2015 · 4 comments · Fixed by #7213
Closed

LocaleSelectorFilter shouldn't set default locale #7208

pperejon opened this issue Aug 10, 2015 · 4 comments · Fixed by #7213
Milestone

Comments

@pperejon
Copy link
Contributor

Hello!
Recently I had some problems with the Translate behavior in a new app I'm developing using CakePHP 3.0. The prob was that translations queries were never executed because of this two reasons:

So, in this case, current locale and default locale will never differ. If you don't call explicitly I18n:locale('whatever'), or $model->local('whatever') in your code, i18n tables will never be accessed.

My suggestion is to change this line to be I18n:locale($locale). Actually, when you make a request with an accept language header, it shouldn't affect the default locale that (AFAIK) should be a static configuration parameter.

Thanks in advance and congrats for your good work!

@ADmad ADmad added this to the 3.0.12 milestone Aug 10, 2015
@ADmad ADmad added the defect label Aug 10, 2015
@lorenzo
Copy link
Member

lorenzo commented Aug 10, 2015

Would you like sending us a pull request with your proposed fix?

@markstory markstory added the i18n label Aug 11, 2015
@pperejon
Copy link
Contributor Author

Sure! This is going to be my first collaboration to open source using git, github, unit tests, and so on. The pull request will be ready in a few minutes. Though is an ultra-simple change I hope I make no mistakes :)

@Marlinc
Copy link
Contributor

Marlinc commented Aug 11, 2015

@pperejon don't worry. Its easy to do and very rewarding 😄

@ADmad
Copy link
Member

ADmad commented Aug 11, 2015

Closing are related PR is open.

@ADmad ADmad closed this as completed Aug 11, 2015
markstory added a commit that referenced this issue Aug 11, 2015
LocaleSelectorFilter now sets the current locale (issue #7208)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants