Migrate taxonomy term references for D6 Node translations

I think there are entity reference migration problems overall that may be more related to this, given taxonomy terms are a special case of entity reference now. I suspect the field is set translated but the translations entity references never get the taxonomy term value set.

Hm, so as per @catch the main reference issue outstanding is #2447727: Add base class for migrating reference fields which is not Drupal 6 related on a cursory look. So may not get you a solution there...

Add tag.

#2886609: Migrate translations for D6 i18n taxonomy 'localized' terms landed.

We need to do the same thing for D7-D8. What do others think, should we open a separate issue for that or increase the scope of this issue to cover both?

Add tag.

Updated issue summary as part of clarifying the scope for each of the multilingual vocabulary / term migration issues.

Postponed on #2886609: Migrate translations for D6 i18n taxonomy 'localized' terms. The other related issue is a soft blocker as the multilingual settings of the vocabulary can be easily modified manually in D8 #2975509: Migrate D6 vocabulary language settings.

Further updates to the issue summary.

Postponed on #2886609: Migrate translations for D6 i18n taxonomy 'localized' terms. The other related issue is a soft blocker as the multilingual settings of the vocabulary can be easily modified on Drupal 8 site, see #2975509: Migrate D6 vocabulary language settings.

Unpostponed as this landed: #2886609: Migrate translations for D6 i18n taxonomy 'localized' terms. The D7 part might need a separate follow-up.

Triaging the issue queue.

For test case 1:
Add a localized taxonomy term to the employee content type and add data. Then add a test for the migration of localized term on a translated node. But the test fails in the migration with

1) Drupal\Tests\taxonomy\Kernel\Migrate\d6\MigrateTermNodeTranslationTest::testTermNode
A translation already exists for the specified language (fr). (/opt/sites/d8/core/lib/Drupal/Core/Entity/ContentEntityBase.php:810)
Failed asserting that false is true.

Next is to find out why it doesn't just update the entity.

Ah, I forgot to add 'translations: true' to the migration.

Remove some test code and comments left in the previous patch by mistake.

This adds 'translations: true' to the destination in d6_term_node_translation.yml allowing the translation to be saved. But the vid wasn't being updated and found that d6_term_node_translation needed to be added to the list of plugins altered in migrate_drupal_migration_plugins_alter in migrate_drupal. Then found that the vocabulary Id being saved for the translated node was the same as for the untranslated node. Changing the vocabulary field to be translatable fixed that. That was done in d6_vocabulary_field_instance.yml and I'm sure other tests will fail because of that. So, stopping now to see what testbot finds.

The interdiff for the above patch

Add source data and test for test case 2, 'per language'. Also, some cleanup and comments.

Reroll because #2754493: D6 synchronized field settings aren't migrated properly was committed.

Trimming the changes to the fixture.

Adjust the field entity count

Fix for Upgrade6Test.

Change the migrate_drupal plugin_later to only add d6_term_node_translation if the module config_translation is installed.

Need to enable config_translation too!

Just want to double check that the source data matches the recommendation in the IS.

Added the implementation of the Case scenarios to the IS. It looks like the cases are covered. Therefor this is ready for review.

@jhodgdon, yes! Much appreciated.

Awesome, 2 'Woot!!'s. It was nice to see that this morning.

Now the error message in 46.a is interesting. I can't find where it is created in my local Drupal 6. So, I went to http://cgit.drupalcode.org/i18n/tree/i18nstrings/i18nstrings.install?h=6... and found it is created in this:

/**
 * Implementation of hook_schema_alter().
 */
function i18nstrings_schema_alter(&$schema) {
  // Add index for textgroup and location to {locales_source}.
  $schema['locales_source']['indexes']['textgroup_location'] = array(array('textgroup', 30), 'location');
  // Add field for tracking whether translations need updating.
  $schema['locales_target']['fields']['i18n_status'] = array(
    'description' => 'A boolean indicating whether this translation needs to be updated.',
    'type' => 'int',
    'not null' => TRUE,
    'default' => 0,
  );
}

but my local has

/**
 * Implementation of hook_schema_alter().
 */
function i18nstrings_schema_alter(&$schema) {
  // Add index for textgroup and location to {locales_source}.
  $schema['locales_source']['indexes']['textgroup_location'] = array(array('textgroup', 30), 'location');
  // Add field for tracking whether translations need updating.
  $schema['locales_target']['fields']['status'] = array(
    'description' => 'A boolean indicating whether this translation needs to be updated.',
    'type' => 'int',
    'not null' => TRUE,
    'default' => 0,
  );
}

The difference is that one has a column of 'i18n_status' and the other is 'status'. The change is in the last commit before The Great Git Migration. So, do we handle both situations or not? I don't recall if there ever was a decision for migrate to handle the latest dev version or the latest release of a module and I don't think we have ever run into a similar situation. But, to answer my question, I think common sense rules and both table schemas should be handled. Someone correct me if I am wrong.

The error in #46.a is not caused by this patch so it is moved to a new issue #3035259: Unknown column 'lt.i18n_status' in 'field list'.

I reviewed patch #40 and repeated the manual test. So we have now two independent manual tests, first by @jhodgdon and now from me.

Test results from my manual tests:

D6 test setup

• I have two language in D6: English and Finnish
• Story content type has two different taxonomy terms. One uses 'localized' and the second 'per language' setting.

Test 1 against the acceptance criteria of the issue summary:

• I created one pair of translated nodes which are associated with each other as translations.
• Both language versions of this node have a 'localized' term associated with it. The 'localized' term is translated in D6 using string translation.

Test 2 against the acceptance criteria of the issue summary:

• I created two terms to the 'per language' taxonomy. One in English and one in Finnish.
• I created a second pair of translated nodes which are associated with each other as translations.
• The English node has the English 'per language' term and the Finnish node has the Finnish 'per language' term.

Test results
Test 1:

• The node pair is migrated to D8 so that we have one shared nid for both node language versions.
• The 'localized' term is migrated to D8 so that we have shared tid for both term language versions.
• The English version of the node is associated with the term. TEST OK.
• The Finnish version of the node is associated with the term. TEST OK.
• TEST 1 PASSED.

Test 2:

• The node pair is migrated to D8 so that we have one shared nid for both node language versions.
• There are two different terms migrated in the 'per language' taxonomy. One of them is in English and another in Finnish. These terms have different tids.
• The English version of the node is associated with the English term. TEST OK.
• The Finnish version of the node is associated with the Finnish term. TEST OK.
• TEST 2 PASSED.

Patch review results
I reviewed the patch. It looks good to me. The D6 fixture has test data for both tests described above and there is test coverage for both tests.

Conclusion:

• I did not encounter the issue reported in #46.a. @quietone already explained the background for that + created a follow-up
• The patch #40 has been reviewed by me and it looks good.
• The patch #40 has been manually tested by both @jhodgon and by me and both were reporting positive test results.
• RTBC. Let's land this. Thank you so much @quietone for your excellent work with this. These multilingual taxonomy migrations are ridiculously complex but you mastered this perfectly. Woop, woop!

Cheers,
Markus

Queued the latest patch also for PostgreSQL and SQLite to be on the safe side.

Removed the TODO item from the issue summary on the D7 evaluation. I opened #3035392: Migrate vocabulary translations and taxonomy term references for Drupal 7 node translations as a D7 follow-up for this.

Retesting PostgreSQL, some CI error happened.

+++ b/core/modules/content_translation/migrations/d6_term_node_translation.yml
@@ -0,0 +1,44 @@
+migration_tags:
+  - Drupal 6
+  - Content

This should be tagged multilingual, no?

Yes, it should! Thanks. All fixed but it looks like there is a PostgreSQL error.

The MySQL errors are unrelated, Media.Drupal\Tests\media\FunctionalJavascript\MediaStandardProfileTest
Sadly the PostgreSQL one is

Resting

The problem here is that the source plugin id is not including the language and that the query can return rows with a NULL language. That has been fixed and the test updated to test for both the zu and fr translation of a vocabulary which should prevent this in the future.

All feedback seems addressed.

#2981393: Migrate D6 comment type language settings landed, this will need a reroll.

Rerolling, which is just a change to the field_config entity count in Upgrade6Test.

Queued for PostgreSQL and & SQLite once more

Ah, good thinking masipila.

Can we have an interdiff between 59-62?

Edit: 59-62, not 60-62

Because of the reroll, the interdiff fails. However, fortunately it was only a one line change and I remember what it was, it was this line.

+++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d6/Upgrade6Test.php
@@ -68,7 +68,7 @@ protected function getEntityCounts() {
+      'field_config' => 95,

Ok, thanks, fair enough.

I tested this thoroughly (manually) earlier, see #49. Since then, @quietone had to make some changes in #59 to resolve a test failure. @heddn reviewed this in #60.

The test cases for both localized and per language vocavularied are defined in the IS and they as well as their implementation.

Since the automated tests are green on all three databases and all feedback has been addressed, this is ready to land.

Thanks Vicki!

Markus

Thanks a lot! Committed to 8.8. This cannot be merged to 8.7 at this point due to commit freeze though. Keeping RTBC for 8.7.x.

Finally was able to merge in, sorry for the delay.