Problem/Motivation

The installer used to not be able to assign proper default languages to entities and was therefore unable to create translatable entities (which need a configured language). This was patched with workarounds in #2148071: Cannot install in foreign language (Undefined index: und in ContentEntityBase->language()). The reason for this not working was that the language list was not properly populated / returned in the installer.

Proposed resolution

The underlying problem was fixed in #1862202: Objectify the language system, so we can remove the workarounds added. The site default language should be applied to entities created in the installer as well. #2148071: Cannot install in foreign language (Undefined index: und in ContentEntityBase->language()) already added test coverage for this (along with the workarounds we now propose to remove).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Test coverage for this was added a short time ago in #2148071: Cannot install in foreign language (Undefined index: und in ContentEntityBase->language()), that's why HEAD is currently failing.

The fix there was to set the language explicitly.

xjm’s picture

Edit: berdir was faster, again. :)

amateescu’s picture

Category: Bug report » Task
Priority: Critical » Normal

So.. we should document this somewhere or is there anything else we can do?

webchick’s picture

I think the code in #0 should be made to work. I'd love to see EntityContent base class (or whatever) automatically setting the language property if it's not specified.

Status: Needs review » Needs work

The last submitted patch, broken_multilingual_installer.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Closed (duplicate)
Issue tags: +language-content

Having a default language for entities when not specified is covered in #1966436: Default *content* entity languages are not set for entities created with the API, and that is not that far if someone can take it over from @Schnitzel who apparently does not have time for it anymore :/ I think that fully covers what people propose here?

I assume the "HEAD fail" comment was temporary and a patch was rolled back to fix it?!

Berdir’s picture

Assigned: Unassigned » Berdir
Status: Closed (duplicate) » Active

Keeping this open for now.

I don't think the referenced issue fixes this. That would only change und to $default_language, but we don't haven have und, apparently only during the installer, otherwise the default behavior works.

The defaults are applied in FieldableDatabaseStorageController::create() makes no sense to me at all how we'd be able to circument that, will try to investigate tonight.

Berdir’s picture

Ok, I think I know what's going on.

The problem isn't that the langcode isn't set to the default, that works just fine.

The problem is that language_list(Language::STATE_ALL); during then installer when do select a different language than english does *not* return LANGUAGE_NOT_SPECIFIED/und. That seems wrong to me?

So what happens is that the following runs into the else, defaultLanguage is und, that is not in the list, boom.

    if ($this->activeLangcode != Language::LANGCODE_DEFAULT) {
      if (!isset($this->languages[$this->activeLangcode])) {
        $this->languages += language_list(Language::STATE_ALL);
      }
      $language = $this->languages[$this->activeLangcode];
    }
    else {
      $language = $this->languages[$this->defaultLangcode];
    }

So, in my opinion, this makes this a bug in the installer, not the entity system and #2148071: Cannot install in foreign language (Undefined index: und in ContentEntityBase->language()) therefore unfortunately only a bandaid fix (with useful test coverage! ;)).

And #1966436: Default *content* entity languages are not set for entities created with the API would in a way solve this, by resulting in the same bandaid, namely that the langcode of the newly created user or other entity would match the single existing language, but that doesn't seem like the proper fix to me.

Gábor Hojtsy’s picture

It is not a good idea to create entities without specific languages if they contain textual information in some way. Then the best practice is to have a concrete language on it, so if/when they become translatable, the source language is properly known. Or even if they don't become translatable ever, views with language arguments would work well. So #1966436: Default *content* entity languages are not set for entities created with the API applies that best practice.

Anyway, while in most cases not a good idea, it is still a feature to create entities with unknown language, and it should not break in any way. I looked at https://api.drupal.org/api/drupal/core%21includes%21bootstrap.inc/functi... and theoretically it ensures that before language module is installed, the language list would contain the locked languages while once language module is installed, it would take the list from the language entities.

There may be a short window, when the modulehandler believes it is installed but the language entities are not in place(?) and/or when we preload the local cached list but don't clear it after the language module is fully installed, so in the same request it still is an empty language list. Just thinking out loud here.

Gábor Hojtsy’s picture

Issue tags: +sprint
plach’s picture

Component: content_translation.module » entity system

On my todo list :)

Gábor Hojtsy’s picture

amateescu’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, broken_multilingual_installer.patch, failed testing.

The last submitted patch, broken_multilingual_installer.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
YesCT’s picture

Title: Creating a translatable content entity during installation doesn't work » Allow creating translatable content without specifying a language
Issue summary: View changes
Status: Needs review » Active

So, head passes with that, so I think this is active?
We need a discussion on what the real problem is, and how to fix it.

I'll try updating the issue summary. But I dont really understand the discussion, so will need some help there.

Maybe we need to define the use cases that core currently doesn't allow. (like creating entities with an unknown language? does that mean with langcode = und, or without langcode set? or?)

saving the summary to get some feedback on it.

Berdir’s picture

Right, so the assumption that the objectify language issue fixed this has been proven as TRUE. This essentially means that the bug here as been resolved.

So we can either close this as duplicate/fixed, or we can check if there's some cleanup that we can do, e.g. for the shortcut quickfix that went in to make sure it has a language.

amateescu’s picture

Well, if that patch passed, is there a problem that we still need to talk about?

Edit: crossposted with Berdir.

amateescu’s picture

Status: Active » Needs review
FileSize
1.72 KB

This is what we should be able to clean up.

amateescu’s picture

FileSize
1.12 KB

Err, no, user_install() doesn't use the APIs so that will probably have to stay.

The last submitted patch, 20: 2156945.patch, failed testing.

Gábor Hojtsy’s picture

The proposed cleanups look good to me :)

Gábor Hojtsy’s picture

Title: Allow creating translatable content without specifying a language » Clean up installer entity defaults following bugfix
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs issue summary update

Retitled for current scope. Looks good as said :)

@YesCT can you help with updating the issue summary? :)

Gábor Hojtsy’s picture

Issue summary: View changes
Issue tags: -Needs issue summary update

Updated the issue summary to be short and sweet :)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 98ea163 and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Issue tags: -sprint

Yay, thanks! Removing from sprint.

Status: Fixed » Closed (fixed)

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