Closed (fixed)
Project:
Drupal core
Version:
8.9.x-dev
Component:
migration system
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
6 Aug 2019 at 19:36 UTC
Updated:
6 Jul 2020 at 11:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
gábor hojtsy#2979966: Migrate D7 i18n taxonomy term language landed, so this should be possible to do.
Comment #3
quietone commentedThis patch adds some fields to a couple of terms and the fixture hasn't been reduced yet so the patch is big. I discovered that there wasn't a test of the TaxonomyTermDeriver so that is added here since it needs changes to the fixture. The MigrateTaxonomyTermTest is changed to test the migration of the new fields and their values. And MigrateTaxonomyTermTranslationTest tests the migration of the translation of those fields. This is not complete.
To check is the migration of field_training and the changes to the source plugin, specifically in selecting the langcode to use. Looking at #2980996: Migrate Drupal 7 taxonomy term entity translations data to Drupal 8 may help.
Comment #5
quietone commentedMost of the errors where because of changes to the fixture that has now been removed. I made an interdiff between the two review only patches, it if helps.
Comment #6
quietone commentedComment #8
quietone commentedMake sure language is defined in the source plugin and update entity counts.
I am still not convinced the language is set correctly in the source plugin.
Comment #9
quietone commentedBefore the changes in the patch the language was set to the default language of the D8 site, which is 'en' for testing, if it was not changed by an entity translation or i18n. This means that if the language in the source was 'und' it became 'en'. Is that what we want? Should it be changed to 'en'? Ah, I just remembered this issue, #3072784: Language specific term(i18n_taxonomy) should not rely on entity translation in d7 taxonomy term migration. The proposed solution there probably needs to be done here.
Comment #10
quietone commentedJust a small improvement.
Comment #11
gábor hojtsy@quietone: I see the actual patch changes is just about enabling translation support and fixing default language fallback behaviour. Then there are lot of tests that look good.
For default language fallback, the 'und' items from the original site should be treated in the language of the original site. Are we assuming that will be same default language on the target site because in a complete migration that would be the case?
Comment #12
quietone commented@Gábor Hojtsy, yes, but not because of using the results of a previous migration, which would be default_language.yml. In the prepareRow method of the source plugin being used here, Term.php the default language is retrieved from the 'language_default' variable in the source database,
$default_language = (array) $this->variableGet('language_default', ['language' => 'en']);Comment #14
quietone commentedAdd test for 8.9.x
Comment #15
gábor hojtsyAh thanks, in that case this looks good. That was the only "interesting" thing I found :) Sorry for not coming back to confirm.
Comment #18
catchCommitted/pushed to 9.0.x and cherry-picked to 8.8.x
I think this could probably be backported to a patch release too, but moving to 'to be ported' for now.
Comment #20
benjifisherI think you meant to say "cherry-picked to 8.9.x".
Since this issue has already been fixed for 8.9.x and it is now too late to back-port it to 8.8.x, I am marking it Fixed.
Comment #21
catch@benjifisher, whoops that's correct, thank you!