Problem/Motivation

This is a followup to #2979966: Migrate D7 i18n taxonomy term language to make sure that the translations on taxonomy fields are migrated.

Proposed resolution

Add fields to vocabularies.

  • vocabfixed: field_training
  • vocablocalized: field_sector
  • vocabtranslate: field_chancellor

Build and trim the fixture
Add a test of the taxonomy term deriver.
Add assertions of the non translated fields to MigrateTaxonomyTermTest
Make a new test for any translated term fields, MigrateTaxonomyTermTranslationTest

Remaining tasks

Review and testing.

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 3073050-10.patch38.23 KBquietone
#10 interdiff-8-10.txt1.44 KBquietone
#8 3073050-8.patch38.46 KBquietone
#8 interdiff-5.8.txt2.29 KBquietone
#5 interdiff-3-5-review-only.txt8.82 KBquietone
#5 3073050-5.patch37.33 KBquietone
#5 3073050-5-review-only.txt8.27 KBquietone
#3 3073050-3.patch1.03 MBquietone
#3 3073050-3-review-only.txt12.89 KBquietone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

#2979966: Migrate D7 i18n taxonomy term language landed, so this should be possible to do.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Active » Needs review
FileSize
3073050-3-review-only.txt12.89 KB
3073050-3.patch1.03 MB

This 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.

Status: Needs review » Needs work

The last submitted patch, 3: 3073050-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Needs work » Needs review
FileSize
3073050-5-review-only.txt8.27 KB
3073050-5.patch37.33 KB
interdiff-3-5-review-only.txt8.82 KB

Most 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.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 5: 3073050-5.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Needs work » Needs review
FileSize
interdiff-5.8.txt2.29 KB
3073050-8.patch38.46 KB

Make 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.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Related issues: +#3072784: Language specific term(i18n_taxonomy) should not rely on entity translation in d7 taxonomy term migration

Before 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.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
FileSize
interdiff-8-10.txt1.44 KB
3073050-10.patch38.23 KB

Just a small improvement.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

@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?

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce 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']);

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented

Add test for 8.9.x

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs review » Reviewed & tested by the community

Ah thanks, in that case this looks good. That was the only "interesting" thing I found :) Sorry for not coming back to confirm.

  • catch committed 6c9ef9e on 9.0.x 
    Issue #3073050 by quietone, Gábor Hojtsy: Migrate D7 i18n taxonomy term...

  • catch committed 56f77cb on 8.9.x 
    Issue #3073050 by quietone, Gábor Hojtsy: Migrate D7 i18n taxonomy term...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/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.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
CreditAttribution: benjifisher as a volunteer commented
Status: Patch (to be ported) » Fixed

Committed/pushed to 9.0.x and cherry-picked to 8.8.x

I 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.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

@benjifisher, whoops that's correct, thank you!

Status: Fixed » Closed (fixed)

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