Sub-task of #1763640: Introduce config context to make original config and different overrides accessible

Problem/Motivation

Currently there is no direct way to activate language specific configuration overrides. One workaround is to create an user with specific language and end up in a user context with this new user, something like below:

    $account = entity_create('user', array(
      'name' => 'French user',
      'mail' => 'test@example.com',
      'created' => REQUEST_TIME,
      'status' => 1,
      'preferred_langcode' => 'fr',
    ));

    $user_config_context = config_context_enter('Drupal\user\UserConfigContext');
    $user_config_context->setAccount($account);

The problem with this approach is we can't create new user for every time we need to access config of another language, and this also makes actual user context responding modules believe that a user is present in the context, while we just used it to work around limitations. A good example where this is needed is config_translation module which needs to display admin pages in different specific languages for various site configuration pages.

Proposed solution

Introduce LanguageConfigContext so that the above code changes to:

    $language = language_load('fr');
    $user_config_context = config_context_enter('Drupal\language\LanguageConfigContext');
    $user_config_context->setLanguage($language);

References

Example of where this should be used: #2020347: Temporary fix for config_translation_enter_context until language context becomes an option

CommentFileSizeAuthor
#15 2020361-language-config-context-15.patch996 bytesvijaycs85
#11 2020361-language-config-context-11.patch6.05 KBvijaycs85
#11 2020361-diff-9-11.txt1.84 KBvijaycs85
#9 2020361-language-config-context-9.patch6.05 KBvijaycs85
#9 2020361-diff-6-9.txt3.97 KBvijaycs85
#6 2020361-language-config-context-6.patch5.77 KBvijaycs85
#6 2020361-diff-1-6.txt4.45 KBvijaycs85
#2 2020361-language-config-context-1.patch1.32 KBvijaycs85
#1 2020361-language-config-context-1.patch1.32 KBvijaycs85

Comments

vijaycs85’s picture

vijaycs85
Location London, UK
commented
StatusFileSize
new 2020361-language-config-context-1.patch1.32 KB

Initial patch... not sure is it a valid approach. If it is valid, will update test cases.

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Active » Needs review
StatusFileSize
new 2020361-language-config-context-1.patch1.32 KB

Initial patch... not sure is it a valid approach. If it is valid, will update test cases.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

I think this in itself looks good, however I'm not sure the locale event listener will obey this language. How it looks to me is it will attempt to take language from the user, then if that is not there, it will take from the negotiation. Not from a specific language set in the context. Tests here would prove that.

Also found a minor code style issue.

+++ b/core/modules/language/lib/Drupal/language/LanguageConfigContext.phpundefined
@@ -0,0 +1,44 @@
+ *

Extra whitespace.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Category: feature » task
Issue tags: +D8MI, +sprint, +language-config
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Add new LanguageConfigContext to override language specific configuration » LanguageConfigContext needed so a fake user is not required to activate language overrides
vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs work » Needs review
StatusFileSize
new 2020361-diff-1-6.txt4.45 KB
new 2020361-language-config-context-6.patch5.77 KB

Thanks for your review @Gábor Hojtsy. Updated the LocaleEventSubscriber to take language set by LanguageConfigContext on priority. Not sure whether it is OK to override in context iteself (i.e. instead of setting set('language') and assigning to 'locale.language', can we directly set('locale.language') in language contexet set).so far updated test cases are working locally.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

I think its a mistake to modify the user context tests, they are supposed to test user contexts. Parallel tests for language contexts should be added :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

BTW I like how you modified the locale override though; it disconnects the locale computed language from an explicitly set one, which is good. :)

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 2020361-diff-6-9.txt3.97 KB
new 2020361-language-config-context-9.patch6.05 KB

Thanks for the quick review @Gábor Hojtsy. Added new test case for language.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

Looks good! Except: :)

+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigLocaleOverride.phpundefined
@@ -150,6 +150,75 @@ function testConfigLocaleUserOverride() {
+    // Ensure that we get the expected value when we leave the user context. The
+    // locale overrides contain an English override too, so although we are not
+    // in a user based language override context, the English language override
+    // applies due to the negotiated language for the page.
...
+    // Ensure that we get the expected value when we leave the english user
+    // context.
...
+    // Ensure that we get the expected value when we leave the german user
+    // context.

These all mention "user context", not appropriate here :) just "context" or "language context" sounds good instead.

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs work » Needs review
StatusFileSize
new 2020361-diff-9-11.txt1.84 KB
new 2020361-language-config-context-11.patch6.05 KB

Fixed #10

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

This patch looks great. I also made some minor edits on the issue summary to make it read better.

dries’s picture

dries commented 
+++ b/core/modules/locale/lib/Drupal/locale/LocaleConfigSubscriber.phpundefined
@@ -62,10 +62,14 @@ public function __construct(LanguageManager $language_manager, ContextInterface
+    // If there is a language set explicitly in current context, use it.
+    // otherwise check if there is a user set in the current context,
+    // to set the language based on the preferred language of the user.
+    // Otherwise set it based on the negotiated interface language.

- 'in current context' should be 'in the current context'.

- 'otherwise' should be 'Otherwise'.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: LanguageConfigContext needed so a fake user is not required to activate language overrides » Create LanguageConfigContext to activate configuration overrides based on language
Status: Reviewed & tested by the community » Fixed

Committed beffe8a and pushed to 8.x. Thanks!

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Fixed » Needs review
StatusFileSize
new 2020361-language-config-context-15.patch996 bytes

Thanks @alexpott and @Dries. Here is the fix for #13.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Looks good :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

BTW added documentation on language contexts in https://drupal.org/node/1928898/revisions/view/2659996/2735165 and https://drupal.org/node/1928898/revisions/view/2735165/2735167.

Also added this issue to the existing change notice at https://drupal.org/node/1928922

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Create LanguageConfigContext to activate configuration overrides based on language » Followup for: Create LanguageConfigContext to activate configuration overrides based on language
Assigned: Unassigned » jhodgdon
dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Followup for: Create LanguageConfigContext to activate configuration overrides based on language » Create LanguageConfigContext to activate configuration overrides based on language
Assigned: jhodgdon » Unassigned
Issue tags: -sprint

Thanks!

Status: Fixed » Closed (fixed)
Issue tags: +sprint

Automatically closed -- issue fixed for 2 weeks with no activity.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: -sprint

Superb, removing from sprint.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes

Updated issue summary.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
Issue tags: +Configuration system, +Configuration context