close
Skip to content

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

Merged
kenjis merged 3 commits into
codeigniter4:4.4from
kenjis:feat-IncomingRequest-setValidLocales
Mar 4, 2023
Merged

fix: can't change and override valid locales#7309
kenjis merged 3 commits into
codeigniter4:4.4from
kenjis:feat-IncomingRequest-setValidLocales

Conversation

@kenjis

@kenjis kenjis commented Feb 27, 2023

Copy link
Copy Markdown
Member

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

@MGatner MGatner left a comment

Copy link
Copy Markdown
Member

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

kenjis commented Mar 2, 2023

Copy link
Copy Markdown
Member Author

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

iRedds commented Mar 4, 2023

Copy link
Copy Markdown
Collaborator

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

kenjis commented Mar 4, 2023

Copy link
Copy Markdown
Member Author

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

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