Migrate D6 vocabulary language settings

Assigning to myself, I'm about halfway through making a patch.

@jhodgdon, thanks for the research, most helpful. I didn't see it until after I had done another look at the d6 code and I came to the same conclusion as you.

I manually tested this patch and it worked just fine but I haven't reviewed the code yet, I just got something to work.

Let's try that again. That was a bad patch.

This adds a source plugin test

D6 has four options in vocabulary language settings. Let's cover all of them in this issue but limit the scope of this issue strictly to D8 Vocabulary settings as it was originally.

I updated the issue summary and proposed the mappings to all four scenarios.

Remaining tasks:
1. Review and agree the configuration settings of the proposed resolution
2. Update the patch accordingly with tests.
3. Test manually.

Cheers,
Markus

Formatting and fix to broken img URL.

Looks all fine, except these ones which are the same in all cases:

D8 vocabulary language: Migrate from D6 vocabulary language. If empty on D6, migrate to 'und' (Not specified).

This may be fine, but I was wondering it we should migrate it to the site default language or just English when empty. I think D6 makes assumptions about this, and while I am not sure it will probably assume the site default language(?)

Re: #13.

I was thinking the same myself. I don't think we should use English as a fallback. I can easily imagine a D6 site where the primary language is something else but the vocabulary language is empty in D6. Let's fallback to the site default language.

I'll update the issue summary later today (I'm just about to hop off from the bus).

Markus

Re #13-14: IS updated.

D7 i18n_taxonomy module has the same 4 vocabulary options that we have in D6. The data is stored in taxonomy_vocabulary database table, we have columns 'language' and 'i18n_mode'. In D6 the data is stored in variables as described in the issue summary.

Do we want to increase the scope of this issue and cover both D6 and D7 in this same issue? Conceptually they are the same which would justify handling them in this same issue even though the scope of the issue creeps. I'm ok to handle these also as separate issues if that's what others are preferring.

Cheers,
Markus

Re #16: Answering myself, let's handle that as a separate issue as the vocabulary itself can be translated as well. For D6-D8 that part has been already covered in #2225781: Migrate D6 i18n taxonomy vocabularies and for D7 we need to cover a) the vocabulary settings as described in this issue + b) handle the translation of the vocabulary itself. I'll create a separate issue for that.

Re: #17: Created #2979970: Migrate D7 vocabulary language settings

I was evaluating more about this. D8 vocabluary does not have 'site default' language so the proposed resolution I wrote earlier is quite tricky. I'll continue evaluating and will provide a new patch this evening my time and will update the issue summary with an updated proposed resolution.

Hope you don't mind @quietone that I jump in with patching!

Cheers,
Markus

@masipila, not at all.

Ok, I figured out the rest of this.

The D6 vocabulary 'language' can be defined if (and only if) the multilanguage mode is set as 'Set language to vocabulary'. D6 UI forces an empty value to this drop down list if any other multilanguage mode is selected. The language is saved in D6 database in vocabulary.language column. This field controls the language of the new terms to be created i.e. it is an equivalent to D8 'Term language / Default language'.

The D8 vocabulary 'language' field is a different thing and is an attribute of the 'Taxonomy vocabulary' configuration entity. The D6 configuration attributes that that we are migrating here are attributes of the D8 'Content language settings' configuration entity.

Issue summary updated accordingly. Patch will come in the next comment.

Cheers,
Markus

Typo / formatting in IS.

Here's the patch that should be migrating all three fields according to the issue summary.

I did not make any changes to the tests yet, tagging for tests.

Markus

+source:
+  plugin: d6_language_content_settings_taxonomy_vocabulary
+  constants:
+    target_type: 'taxonomy_term'
+    default_langcode: 'site_default'

This works, but I noticed that @maxocub used a language code 'xx-et-current' in another migration, see #2073467: Migrate Drupal 7 Entity Translation settings to Drupal 8.

What is this 'xx-et-default' and should we use that one instead of 'site_default'?

Markus

The xx-et-* langcodes are specific to the entity_translation module ('et' = 'entity_translation'), so I think 'site_default' is good here.

I'll look at the tests this week.

Thanks masipila! Updated the test to check the default langcode and the third party settings according to the new migration values.

• Added language column to the source plugin test.
• Added comments in the source plugin (taken from i18ntaxonomy.module) explaining the i18nvocabulary setting values.
• Updated the entity count in MigrateUpgrade6Test
• Added content_translation_update_definition to the destination plugin configuration.

I'm checking the tests at the moment.

A couple of copy-paste error nits.

1.

+++ b/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateLanguageContentTaxonomyVocabularySettingsTest.php
@@ -0,0 +1,76 @@
+ * Tests migration of the ability to translate menu content.

Should be something like 'Tests migration of i18ntaxonomy vocabulary settings.'

2.

+++ b/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateLanguageContentTaxonomyVocabularySettingsTest.php
@@ -0,0 +1,76 @@
+   * Asserts an content language settings configuration.

s/an/the.

3.

+++ b/core/modules/language/tests/src/Kernel/Plugin/migrate/source/d6/LanguageContentTaxonomyVocabularySettingsTest.php
@@ -0,0 +1,81 @@
+ * Tests menu source plugin.

Should be something like 'Tests i18ntaxonomy vocabulary setting source plugin.'

@masipila, thanks for the review!

Yes, I still leave copy/paste errors behind.

All items in #32 fixed.

Removed the needs tests tags because the tests are here.

I realized that the vocabulary language settings is not the only thing we need to cover.

• The D8 content type / field settings has the term reference field.
• This term reference field has a setting 'Users may translate this field'
• In the 'per language' concept we need to enable this setting so that the English and Finnish language versions of the node can point to different terms. In all other scenarios, we need to leave this as FALSE.

Issue summary updated with expected result for each of the scenarios.

This is not finished but I think it is working properly. At minimum, tests are needed for the changes in d6_vocabulary_field_instance and the changes to the dump needs checking.

I've added the names of the test fixture vocabularies to the proposed resolution to help with reviews.

The patch is #36 was a work in progress and it was made from HEAD. Oops. This patch is and the interdiff is against the patch in #33.

The four modes of translation are now enabled in the test fixture. Only one vocabulary required a change, that was to set vocabulary vocabulary_3_i_0_ to translate 'localized term'. And they are tested according to the excellent descriptions by masipila in the proposed resolution. I've added the taxonomy names there to help reviewers.

Edit: I really must remember to name interdiff correctly.

Not sure how the hierarchy for the vocabulary was changed from 2 to 0. It is back to 2 now. And fixed the entity count as well/

This is ready for review. It should now implement all of the proposed resolution, that is the vocabulary settings and the language content settings for the 4 modes of taxonomy translation available in Drupal6.

Change the IS to show that the patch is made and it has tests.

I tested this manually and the patch #40 works exactly as the issue summary describes. I also reviewed the patch and it looks good. There is also sufficient test coverage.

I'd RTBC this otherwise but I contributed to the patch in #24 so I'm not sure I can do that. But as said, the patch #40 is doing exactly what the issue summary defines for the different scenarios.

Thanks @quietone!

Cheers,
Markus

Just found a few things, otherwise this looks good, great work everyone!

1. +++ b/core/modules/language/migrations/d6_language_content_taxonomy_vocabulary_settings.yml
   @@ -0,0 +1,56 @@
   +  default_langcode:
   +    -
   +      plugin: get
   +      source: language
   +    -
   +      plugin: default_value
   +      default_value: site_default
   

   I think this could be written as

   default_langcode:
     plugin: default_value
     default_value: site_default
     source: language
   

2. +++ b/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateLanguageContentTaxonomyVocabularySettingsTest.php
   @@ -0,0 +1,80 @@
   +    // Create some languages.
   +    ConfigurableLanguage::createFromLangcode('en')->save();
   +    ConfigurableLanguage::createFromLangcode('fr')->save();
   

   Instead of manually creating the languages, we can just run the 'language' migration.

3. +++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php
   @@ -8618,6 +8618,16 @@
   +  'nid' => '1',
   +  'timestamp' => '1531475240',
   +))
   +->values(array(
   +  'uid' => '1',
   +  'nid' => '2',
   +  'timestamp' => '1531475241',
   +))
   +->values(array(
   +  'uid' => '1',
   
   @@ -44015,6 +44025,12 @@
   +  'nid' => '9',
   +  'totalcount' => '1',
   +  'daycount' => '1',
   +  'timestamp' => '1531475237',
   +))
   +->values(array(
   

   I don't see the relevance of those changes. I think they can be remove without affecting the tests in this patch.

4. +++ b/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateVocabularyFieldInstanceTest.php
   @@ -39,19 +39,21 @@ protected function setUp() {
   +    // Test that the field exists.Tags has a multilingual option of 'None'.
   ...
   +    // Test the page bundle as well.Tags has a multilingual option of 'None'.
   

   Missing spaces after the periods.

Thanks maxocub.

This should fix #45.1-4.

Great, after reading a few times through the IS, the comments and the patch, I think this is ready.

Only one thing though, there's still one unused use statement:

+++ b/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateLanguageContentTaxonomyVocabularySettingsTest.php
@@ -0,0 +1,80 @@
+use Drupal\language\Entity\ConfigurableLanguage;

But that can be fixed on commit, right?

Queued the last patch to PostgreSQL and SQLite tests just to be sure.

Removed the unused use statement spotted in #47.

Thanks @quietone for the last fix!

I haven't touched the patch since it was last RTBC so I should be allowed to do it now.

Markus

+++ b/core/modules/language/migrations/d6_language_content_taxonomy_vocabulary_settings.yml
@@ -0,0 +1,53 @@
+  'third_party_settings/content_translation/enabled':

+++ b/core/modules/language/src/Plugin/migrate/source/d6/LanguageContentSettingsTaxonomyVocabulary.php
@@ -0,0 +1,65 @@
+ * @MigrateSource(
+ *   id = "d6_language_content_settings_taxonomy_vocabulary",
+ *   source_module = "taxonomy"
+ * )

One thing I can't see (and it might be just because I don't know) is how we are sure that content_translation is enabled on the target site?

Changing the status back to NR to address @alexpott's question in #52. Ping @quietone / @maxocub.

That pattern in the migration yml already exists in core in language/d6_language_content_setttings,yml and language/d7_language_content_settings.yml. That doesn't make it the 'correct' solution but it is out there.

This is further proof we need to discuss how to handle migrations that require multiple destination modules. Perhaps a new issue?

Edit: Made a new issue #3004472: Handle migrations that require multiple destination modules

Thanks @quietone for your insight!

I propose that we will not block this issue because of this. Instead, I will emphasize in the docs that when performing multilingual upgrades, the sitebuilder is expected to enable Config translation, Content translation, UI translation and Migrate Drupal multilingual. It is IMO quite safe assumption that more or less any multilingual site will anyway need these modules.

I'll add this to these two pages:
https://www.drupal.org/docs/8/upgrade/upgrading-multilingual-drupal-6-to...

https://www.drupal.org/docs/8/upgrade/upgrading-multilingual-drupal-7-to...

I'll change this to NW for the additional documentation and then back to RTBC once the docs have been updated.

Markus

The handbook documentation has now been updated so that we are emphasizing that Language, Interface Translation, Content Translation, Configuration Translation and Migrate Drupal Multilingual need to be enabled before running a multilingual upgrade.

This and @quietone's comment in #54 addresses @alexpott's question in #52.

Back to RTBC.

Cheers,
Markus

Patch #50 does not apply anymore, needs reroll

re-rolled

Using git bisect (first time) this error starts when #2994398: Not properly clearing EntityFieldManager's fieldMap leads to fatals, often after migration or bundle creation was committed. Skimming that patch I noticed that the installing the entity schema for node was added to the existing Migrate Language Content Settings tests. So, I added that to this test and it passed locally. Let's see if testbot agrees.

Arg!: My fingers have learned to use the wrong extension for interdiff.

I guess I ended up testing in the wrong branch.

Start over and try again.
The problem is it is trying to add a field to the taxonomy_term_field_data table, so let's just make sure it exists for the test!

There was a testbot hiccough so I reran the test and it is green now

Thanks @gaydabura for the reroll in #59!

I reviewed the patches from #59 onwards. As @quietone explained in #64, we need to have taxonomy_term schema in the test that failed in #59.

I queued the latest patch for PostgreSQL and SQLite. Once those are green, this can go back to RTBC as it was before.

Markus

Tests are passing on PostgreSQL and SQLite so setting to RTBC as recommended by masipila in #67.

Un-assigning myself

Committed and pushed d654e1038b to 8.7.x and 9e80041316 to 8.6.x. Thanks!