Follow-up to #1966436-250: Default *content* entity languages are not set for entities created with the API

Problem/Motivation

Issue was closed but there's @todo reference in code

/**
* Implements hook_install().
*/
function user_install() {
  $storage = \Drupal::entityManager()->getStorage('user');
  // @todo Rely on the default value for langcode in
  //   https://drupal.org/node/1966436
  $langcode = \Drupal::languageManager()->getDefaultLanguage()->getId();

Proposed resolution

Remove @todo and fix language assign in install for user

Remaining tasks

Fix

User interface changes

No

API changes

No

CommentFileSizeAuthor
#7 interdiff.txt1.13 KBgábor hojtsy
#7 2368081-user-install-langcode-7.patch1.98 KBgábor hojtsy
#1 2368081-user-install-langcode.patch871 bytesgábor hojtsy

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Active » Needs review
StatusFileSize
new 2368081-user-install-langcode.patch871 bytes

I don't think the default language will be applied so far as we use the storage directly instead of the entity API? Let's see what happens :D

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

It works!

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Needs tests

Do we need test for that?
Manually tested install with ru language - it works

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Yep we could add an assertion to InstallerTranslationTest

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

So there is a KernelTestBase test for this - see UserInstallTest. But I don't trust it to properly test the integration of the installer, entity system and language so early during an install. So lets add an assertion to InstallerTranslationTest for this too.

gábor hojtsy’s picture

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

Based on git blame, #2148071: Cannot install in foreign language (Undefined index: und in ContentEntityBase->language()) added the langcode to user_install(). It did add a test to ensure the warning seen before did not appear anymore but no specific test as to what langcode the user was created in. We can add asserts to existing tests to ensure the user got created with a good langcode. InstallerTranslationTest looks good for this.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs work » Needs review
StatusFileSize
new 2368081-user-install-langcode-7.patch1.98 KB
new interdiff.txt1.13 KB

Note also that UserInstallTest tests this with English and ensures both users get created with the site default language (English in that test). Here is a little update to InstallerTranslationTest to test that those get created in German.

gábor hojtsy’s picture

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

Looks good? :)

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

exactly!

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: +sprint
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed bee3e48 on 8.0.x 
    Issue #2368081 by Gábor Hojtsy | andypost: Fixed Remove outdated @todo...
gábor hojtsy’s picture

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

Superb, thanks!

Status: Fixed » Closed (fixed)

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