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

fix: can't change and override valid locales #7309

Merged

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Feb 27, 2023

Description
Fixes #4297

  • add IncomingRequest::setValidLocales()

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added bug Verified issues on the current code behavior or pull requests that will fix them 4.4 labels Feb 27, 2023
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An elegant solution.

@kenjis kenjis force-pushed the feat-IncomingRequest-setValidLocales branch from a86bf1a to 561e249 Compare March 2, 2023 02:43
@kenjis
Copy link
Member Author

kenjis commented Mar 2, 2023

Added docs.

@kenjis kenjis merged commit dddf6b8 into codeigniter4:4.4 Mar 4, 2023
@kenjis kenjis deleted the feat-IncomingRequest-setValidLocales branch March 4, 2023 00:45
@iRedds
Copy link
Collaborator

iRedds commented Mar 4, 2023

Of course this is a bit late, but what does request state have to do with locales?
The incoming request either contains the locale or does not.
I think the request class should not be concerned with locale validation.

@kenjis
Copy link
Member Author

kenjis commented Mar 4, 2023

Now that we have added setValidLocales(), we can change the locale to any value later.
However, there is still room for improvement in where the locale of the request is determined.

I found that we still cannot change valid locales for content negotiation:

$this->setLocale($this->negotiate('language', $config->supportedLocales));

in the constructor:
$this->detectLocale($config);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.4 bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants