Migrate translations for D7 i18n taxonomy 'localized' terms

Triaging the issue queue.

I've been looking at this and it is very different than Drupal 6. Here, the taxonomy_term_data table has an additional field for the language meaning the translated terms for all vocabulary multilingual types are stored here and not in i18n string table. I don't think a separate migration will be needed for localized terms and this can be closed.

However, leaving this open while work progresses on the migration of translated term in #2979966: Migrate D7 i18n taxonomy term language.

I don't think a separate migration will be needed for localized terms and this can be closed.

@quietone I'm a little confused on this because I have taxonomies with localized terms and the translations aren't migrating over. If a separate migration isn't needed, then can you point to how it should be done?

At quick glance, it looks like the terms are referenced in the i18n_string, locales_source, and locales_target D7 tables and I don't see them referenced in d7_taxonomy_term or the patch at https://www.drupal.org/files/issues/2019-04-25/2979966-21.patch.

#9 Actually for 'localized' terms, the data structure is pretty much identical to D6:

D6 (copied from #2886609: Migrate translations for D6 i18n taxonomy 'localized' terms):

SELECT td.tid, td.name, lt.language, lt.translation FROM term_data td LEFT JOIN i18n_strings ist ON td.tid = ist.objectid LEFT JOIN locales_target lt ON ist.lid = lt.lid WHERE ist.type = 'term' 

D7:

SELECT td.tid, td.name, lt.language, lt.translation FROM taxonomy_term_data td LEFT JOIN i18n_string ist ON td.tid = ist.objectid LEFT JOIN locales_target lt ON ist.lid = lt.lid WHERE ist.type = 'term'

I agree with comment #10 in thinking this ticket is still needed. And curious about the other approach if it isn't needed.

#2979966: Migrate D7 i18n taxonomy term language only adds language information for migration the source language version of a taxonomy term.

We still need something to migrate the translations.

Created a patch based on the hint from #11, but without test coverage.

Changed:

• '6' to '7' in strings in files, filenames and folder name
• 'i18ntaxonomy' to 'i18n_taxonomy'
• 'i18n_strings' to 'i18n_string'

Created patch from above the Drupal root. Corrected that in patch. See attached.

Yea, this is needed. I just got mixed up.

This needs a test so I started one. I have not added an interdiff since it is the same size of the patch, which is now large since a fixture change was required.

Now, reduce the size of the fixture which will also fix some of the tests. The interdiff excludes changes to the fixture.

1. +++ b/core/modules/content_translation/migrations/d7_taxonomy_term_localized_translation.yml
   @@ -0,0 +1,44 @@
   +  langcode: ltlanguage
   

   The query should be changed so that this is language. At least I think that was done in another issue.

2. +++ b/core/modules/taxonomy/src/Plugin/migrate/source/d7/TermLocalizedTranslation.php
   @@ -0,0 +1,102 @@
   +  public function prepareRow(Row $row) {
   

   Need to find out if this can use I18nQueryTrait

1. No change. The language field is set in the parent query so let's keep this simple. I've added a comment to the migration to help explain the unusual name.
2. Can't do this either. The trait was made assuming the objectid in the i18n_string or i18n_strings table is test. However, for taxonomy terms it is the tid, an integer, for the term. So that won't work. However, I reworked prepareRow to use as much of the same code as possible for consistency.

This is based on the D6 version and thus is fairly straight forward. I haven't done a self review, I just wanted to get all the tests passing.

Can anyone do some manual testing?

Reviews welcome.

This looks good to me, thanks for the added docs on the unusual field name.

This needs a reroll.

There is a change in that the instead of the migration of i18n_taxonomy being set to 'finished' I have kept it at 'not_finished' pending on the completion of #3035392: Migrate vocabulary translations and taxonomy term references for Drupal 7 node translations. That seems reasonable to me. Agree?

And test for i18n_taxonomy and i18n_translation in the will not be upgraded list.

Thanks for the reroll. I agree keeping this marked unfinished is the right way to go.

1. +++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php
   @@ -16199,183 +16199,183 @@
   -  'context' => 'comment_body:comment_node_article:label',
   -  'objectid' => 'comment_node_article',
   ...
   -  'context' => 'comment_body:comment_node_blog:label',
   -  'objectid' => 'comment_node_blog',
   

   what happened to these?

2. +++ b/core/modules/taxonomy/src/Plugin/migrate/source/d7/TermLocalizedTranslation.php
   @@ -0,0 +1,109 @@
   +    $query = $this->select('i18n_string', 'i18n')
   

   is there a performance cost of doing this for each row? any way it can be done in a single query for all rows?

Thanks larowlan!

#29.1 I've restored the two rows that were accidentally removed from i18n_string, now using with an previously unused 'lid'. There are no translations for those fields so nothing else to change.

#29.2. The first issue (whatever it was) where I needed to do this I did try to get both translations in the query() method but then the query wasn't joinable. This same technique is uses in d7/BlockCustomTranslation and d6/MenuLinkTranslation source plugins. I'm pretty sure this getting the 'other' translation in prepareRow() was discussed at a migrate meeting when this way of getting the translations was first implemented.

+++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php
@@ -18421,6 +18441,150 @@
+->values(array(
+  'lid' => '92',
+  'location' => 'taxonomy:vocabulary:6:description',
+  'textgroup' => 'taxonomy',
+  'source' => 'Vocabulary translate option',
+  'context' => 'vocabulary:6:description',
+  'version' => '1',
+))
+->values(array(
+  'lid' => '93',
+  'location' => 'taxonomy:vocabulary:4:name',
+  'textgroup' => 'taxonomy',
+  'source' => 'vocabulary name clearly different than machine name and much longer than thirty two characters',
+  'context' => 'vocabulary:4:name',
+  'version' => '1',
+))
+->values(array(
+  'lid' => '94',
+  'location' => 'taxonomy:vocabulary:4:description',
+  'textgroup' => 'taxonomy',
+  'source' => 'description of vocabulary name much longer than thirty two characters',
+  'context' => 'vocabulary:4:description',
+  'version' => '1',
+))

Lot of these (and more above) don't actually seem to be appearing in tests or are their translations there?

Those changes are to the locales_source table and do not have any effect on the migration or the tests. They are not necessarily for items that have a translation, that is the locales_target table may not have an entry with the same lid.

However, those changes are needed for a D7 site using the test fixture to work as expected. I confirmed this by removing those changes, imported the test fixture and navigating to admin/structure/taxonomy/vocabulary_name_much_longer_than_thirty_two_characters/translate which should not have a translation but now does and it is incorrect, pointing to a translation for something else (didn't bother to find out what it was). That makes the fixture unusable for creating a new dump.

Ok, right, well, they are merely the sources for possible translations, ok.

I did not find any other parts that were concerning.

Committed 07c4124 and pushed to 8.8.x. Thanks!

@larowlan, thanks for the commit.
Changing back to fixed since this has been committed

Oh, dear this is missing a test of the source plugin! Follow up added #3073442: Add test of d7 term localized source plugin

I think we should highlight a list of all the significant improvements to multilingual migrations in 8.8 (whether or not the module stabilizes in this release). Tagging accordingly!

Shouldn't the localized term description formats be migrated as well as part of this migration?