Problem/Motivation

In order to be able to identify a given migration as multilingual, we need to migrate the content type translation settings for the different content types.

Proposed resolution

Content type translation setting variables:

  • language_content_type_$type (one for each content type). Has 4 possible states:
    • No value: Means no translatable.
    • 0: Means no translatable.
    • 1: Means has language.
    • 2: Means has language, and it is translatable.
  • i18n_newnode_current_$type (one for each content type). A checkbox that indicates if the current language is the default for new content.
  • i18n_required_node_$type (one for each content type). A checkbox that makes language required (neutral not allowed).
  • i18n_lock_node_$type (one for each content type). A checkbox that makes the language locked, so it cannot be changed on edit.
  • i18n_node_$type (one for each content type). Has 4 possible states:
    • No value: not applicable, no translatable.
    • 1: All enabled languages allowed.
    • 2: All defined languages allowed.
    • 3: All defined languages allowed, but not displayed in links.
  • language_content_type_negotiation ?
  • language_content_type_default ?

Target: language.settings.yml

entities:
  node:
    article:
      language:
        default_configuration:
          langcode: current_interface
          language_show: true

Original report by Ryan Weal

Content type translation settings (D6/D7 core content translation and entity_translation contrib -> D8 content type config). This will bring what language settings are allowed for content authors of a particular content type. It will determine what the translation method was (content translation == 1 node per language, entity_translation = 1 und node with translatable fields). Config here specifies if it is possible to set langauge_none/undefined/language neutral/UND or if setting a language is mandatory.

Comments

pcambra’s picture

rvilar’s picture

Assigned: Unassigned » rvilar
penyaskito’s picture

Issue summary: View changes
penyaskito’s picture

Issue summary: View changes
rvilar’s picture

Assigned: rvilar » Unassigned

I'm unassigning me from this issu beacuse I can't work on this issue at the moment.

penyaskito’s picture

Issue summary: View changes
penyaskito’s picture

Issue tags: +Amsterdam2014
penyaskito’s picture

Work in progress...

penyaskito’s picture

FileSize
12.1 KB

New patch. Includes custom destination and the migration itself. Migration tests looks good, but needs to be expanded.

Most of the variables we had are not longer necessary because those features are not there (they don't make sense anymore, or they would be at contrib). Not sure if those should be documented on the source (as they are now) or moved to the migration yml.

I just found source test coverage is insufficient too, I thought I had this already covered.

Sorry, no interdiff.

benjy’s picture

FileSize
15.57 KB

Patch attached has a POC for the use of a Load plugin instead. It's just a fork of LoadEntity and would need some clean up. Posting here for discussion.

penyaskito’s picture

Status: Active » Postponed
penyaskito’s picture

Assigned: Unassigned » penyaskito
Status: Postponed » Active

I should come back to this one.

phenaproxima’s picture

Project: IMP » Drupal core
Version: » 8.0.x-dev
Component: Code » migration system
Assigned: penyaskito » Unassigned
phenaproxima’s picture

Status: Active » Postponed
Issue tags: -Amsterdam2014
phenaproxima’s picture

Status: Postponed » Needs work
Issue tags: +Needs reroll

OK, this is unblocked :) But it's a year old so it positively aches for a re-roll...

phenaproxima’s picture

Status: Needs work » Postponed
svendecabooter’s picture

Status: Postponed » Active

Unblocked

svendecabooter’s picture

Status: Active » Needs review
Issue tags: -Needs reroll +Needs tests
FileSize
2.92 KB

This functionality has changed quite a bit since the last patch.
Attached is a new patch based on the work by penyaskito & benjy.

It migrates the D6 content type language settings that are appropriate in D8.
Still needs test, and a check to see if this can be generalised for D7 as well.

svendecabooter’s picture

Title: Migrate content type language settings from Drupal 6 » Migrate content type language settings from Drupal 6 & 7
Issue tags: -drupal6
FileSize
4.48 KB

Patch attached that migrates from D6 and D7.
Still needs tests.

svendecabooter’s picture

Issue tags: +D8MI
catch’s picture

Priority: Normal » Major
Issue tags: +Migrate critical
quietone’s picture

A test for D7 only, with a schema error. How to fix that?

Also, penyaskito, in #9, talked about where to put comments about the variables that are no longer necessary. I've tried to put that in the yml template. It seems the best place to look to get an overview of what the migration will do.

quietone’s picture

Don't know what happened but I needed to upload the patch and interdiff again.

Status: Needs review » Needs work

The last submitted patch, 23: 2225271-22.patch, failed testing.

quietone’s picture

I know it is late but the D6 migration looks wrong to me. The D6 migration template is essentially the same as the D7 one. But the D6 i18n variables are different and they are using the same source plugin, which is specific to D7.

  1. +++ b/core/modules/language/src/Plugin/migrate/source/LanguageContentSettings.php
    @@ -0,0 +1,73 @@
    +    if ($this->getModuleSchemaVersion('system') >= 7000) {
    

    Seems to be for D7 not D6.

  2. +++ b/core/modules/language/src/Plugin/migrate/source/LanguageContentSettings.php
    @@ -0,0 +1,73 @@
    +      $i18n_node_options = $this->variableGet('i18n_node_options_' . $type, NULL);
    

    D6 doesn't have a variable i18n_node_options_

  3. +++ b/core/modules/language/src/Plugin/migrate/source/LanguageContentSettings.php
    @@ -0,0 +1,73 @@
    +        $row->setSourceProperty('i18n_lock_node', 1);
    

    The lock variable for D6 is i18n_lock_node_$type

quietone’s picture

Status: Needs work » Needs review
FileSize
10.62 KB
10.76 KB

For a bit of clarity the D6 and D7 versions are separated in this patch.

quietone’s picture

Issue tags: -Needs tests

So there are now tests for D6 and D7 that are passing.

quietone’s picture

Issue tags: +migrate-d6-d8, +migrate-d7-d8

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

penyaskito’s picture

Rerolled #26. 3-way merge worked automatically, let's see the results.

Status: Needs review » Needs work

The last submitted patch, 30: 2225271-30.patch, failed testing.

penyaskito’s picture

Status: Needs work » Needs review
FileSize
1.26 KB
10.78 KB

MigrateDrupalNTestBase were moved, so need to update the use statement.

These are green locally.

penyaskito’s picture

Issue tags: +neworleans2016
quietone’s picture

@penyaskito, thanks.

Made two changes:

  • Changed the two executeMigration to a single executeMigrations d6/MigrateLanguageContentSettingsTest.php.
  • Moved the two migration tests to the Kernel namespace, Drupal\Tests\language\Kernel\Migrate;
vasi’s picture

Status: Needs review » Needs work

Some preliminary notes on review:

For the properties that we're setting, we need to verify that they do the right things
* default_langcode: current_interface?
* language_alterable meaning?
* Is content_translation/enabled really tri-valued? NULL/TRUE/FALSE
* What about the fields themselves, are they marked translatable?

Tests need to check that non-translatable types have correct settings, too
We do a test for system schema >= 7000 in the d7 source, shouldn't that be always true?
Should we separate or coalesce d6/d7?

vasi’s picture

I did a brief review of the fields we're setting on the language_content_settings config entity:

  • default_langcode: We're setting this to 'current_interface' for translated content. Is there any reason for it to be 'current_interface', instead of the default 'site_default'?
  • language_alterable: This being false has slightly different semantics than i18n_lock_node_TYPE. The former means you can't choose the language when you create a node; the latter means you can choose it, but not edit it later and change your mind. I think it's close enough.
  • third_party_settings/content_translation/enabled: This looks good. NULL is not a real valid value, but in that case (language_content_type_TYPE == 0) the row will be skipped anyhow. I'd like there to be a comment that the value isn't used, so it's clear that we're not really setting that to NULL.

Also, I actually tried to do a migration with this patch, and it worked well! The fields are indeed marked as translatable, since that's the default. So the remaining issues for me are:

  • Justify the setting for default_langcode.
  • Add a comment for third_party_settings/content_translation/enabled.
  • Remove the D6-or-D7 check from the source d7_language_content_settings.
  • Add an assertion that language_alterable is TRUE when there's no i18n_lock_node_TYPE.
  • Add an assertion that a content type that is not translatable generates no language_content_settings config entity.
penyaskito’s picture

@vasi:

default_langcode: We're setting this to 'current_interface' for translated content. Is there any reason for it to be 'current_interface', instead of the default 'site_default'?

We want that content to be in the language negotiated by the page. Using 'site_default' would mean using the default language of the site.

Add a comment for third_party_settings/content_translation/enabled.

Added

Remove the D6-or-D7 check from the source d7_language_content_settings.

Done

Add an assertion that a content type that is not translatable generates no language_content_settings config entity.

We are using all the types as a source, so we would need to modify the patch for that. I'm not sure if it's worth it. I added assertions for ensuring that they have the default configuration instead.

TBD:

Add an assertion that language_alterable is TRUE when there's no i18n_lock_node_TYPE.

NR just for the tests, should be back to NW afterwards.

Status: Needs review » Needs work

The last submitted patch, 37: 2225271-37.patch, failed testing.

penyaskito’s picture

Tests are green, next steps:

Add an assertion that language_alterable is TRUE when there's no i18n_lock_node_TYPE.

Are there any types already that meets those conditions in d6/d7 dumps?

vasi’s picture

We are using all the types as a source, so we would need to modify the patch for that. I'm not sure if it's worth it.

But we are using skip_on_empty to skip the ones that are not actually translatable. No patch modifications should be needed.

Are there any types already that meets those conditions in d6/d7 dumps?

I think there's none, you're right. It would be nice if language_alterable = TRUE was tested, but it's probably not worth changing the dumps.

penyaskito’s picture

Status: Needs work » Needs review
FileSize
2.6 KB
12.11 KB

Added the dump modification so we can test that, and added the new test. For that I had to fix a bug in the source, so thanks for asking for that test ;-)

penyaskito’s picture

Issue tags: +DrupalNorth2016

Tagging with the DrupalNorth sprint, sorry for the noise.

penyaskito’s picture

FileSize
1.85 KB
11.8 KB

phpcs complained about a couple of things, fixed those.

vasi’s picture

Status: Needs review » Reviewed & tested by the community

Looks good!

  • catch committed f773bf9 on 8.2.x
    Issue #2225271 by penyaskito, quietone, svendecabooter, benjy, vasi:...

  • catch committed 8f20e36 on 8.1.x
    Issue #2225271 by penyaskito, quietone, svendecabooter, benjy, vasi:...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!

Status: Fixed » Closed (fixed)

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