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

Files: 
CommentFileSizeAuthor
#15 2020361-language-config-context-15.patch996 bytesvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 57,985 pass(es). View
#11 2020361-language-config-context-11.patch6.05 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 56,342 pass(es). View
#11 2020361-diff-9-11.txt1.84 KBvijaycs85
#9 2020361-language-config-context-9.patch6.05 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 57,769 pass(es). View
#9 2020361-diff-6-9.txt3.97 KBvijaycs85
#6 2020361-language-config-context-6.patch5.77 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 57,823 pass(es). View
#6 2020361-diff-1-6.txt4.45 KBvijaycs85
#2 2020361-language-config-context-1.patch1.32 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 57,573 pass(es). View
#1 2020361-language-config-context-1.patch1.32 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 56,216 pass(es). View

Comments

vijaycs85’s picture

FileSize
1.32 KB
PASSED: [[SimpleTest]]: [MySQL] 56,216 pass(es). View

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

vijaycs85’s picture

Status: Active » Needs review
FileSize
1.32 KB
PASSED: [[SimpleTest]]: [MySQL] 57,573 pass(es). View

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

Gábor Hojtsy’s picture

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

Category: feature » task
Issue tags: +D8MI, +sprint, +language-config
Gábor Hojtsy’s picture

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

Status: Needs work » Needs review
FileSize
4.45 KB
5.77 KB
PASSED: [[SimpleTest]]: [MySQL] 57,823 pass(es). View

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

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

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

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
3.97 KB
6.05 KB
PASSED: [[SimpleTest]]: [MySQL] 57,769 pass(es). View

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

Gábor Hojtsy’s picture

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

Status: Needs work » Needs review
FileSize
1.84 KB
6.05 KB
PASSED: [[SimpleTest]]: [MySQL] 56,342 pass(es). View

Fixed #10

Gábor Hojtsy’s picture

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

+++ 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

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

Status: Fixed » Needs review
FileSize
996 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,985 pass(es). View

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

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good :)

Gábor Hojtsy’s picture

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

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

Status: Reviewed & tested by the community » Fixed

Committed to 8.x.

Gábor Hojtsy’s picture

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

Issue tags: -sprint

Superb, removing from sprint.

Gábor Hojtsy’s picture

Issue summary: View changes

Updated issue summary.

Gábor Hojtsy’s picture