Problem/Motivation

CCK fields and options are translated using i18n_cck submodule of the suite i18n, we need to migrate those respecting the language settings. It may be that options are done in a another patch.

Proposed resolution

Patch
Review
Commit

Remaining tasks

Add translations to test fixture
Write a test
Write a patch to migrate field labels and options.

CommentFileSizeAuthor
#33 interdiff-30-33.patch656 bytesquietone
#33 2959410-33.patch16.02 KBquietone
#30 interdiff.txt750 bytesquietone
#30 2959410-30.patch16 KBquietone
#28 2959410-28.patch15.24 KBjofitz
#18 interdiff.txt22.49 KBquietone
#18 2959410-18.patch15.65 KBquietone
#10 interdiff.txt2.36 KBquietone
#10 2959410-10.patch14.64 KBquietone
#7 interdiff.txt2.85 KBquietone
#7 2959410-7.patch14.48 KBquietone
#3 2959410-3.patch14.73 KBquietone
#23 2959410-23.patch15.64 KBquietone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Assigned: Unassigned » quietone

making progress but not ready to post anything

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Active » Needs review
FileSize
2959410-3.patch14.73 KB

This migrates field label and description translation. The i18n_strings table includes rows for options as well. I've not checked to see if they can be done in the same migration.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs review » Needs work

Need to look at the option translations and determine if they must be done in a separate migration or included here.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Title: Migrate D6 i18n CCK fields » Migrate D6 i18n CCK fields label and description

As I suspected the options need to be migrated separately. Retitling and found a few things to change.

  1. +++ b/core/modules/config_translation/src/Plugin/migrate/source/d6/FieldLabelDescriptionTranslation.php
@@ -0,0 +1,60 @@
+ * Gets i18n strings profile field source from database.

    copy paste error 

  2. +++ b/core/modules/config_translation/tests/src/Kernel/Plugin/migrate/source/d6/FieldInstanceLabelDescriptionTranslationTest.php
@@ -0,0 +1,79 @@
+ * Tests the i18nProfileField source plugin.

    Another copy paste error 

  3. +++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php
@@ -47070,8 +47055,8 @@
+  'access' => '1523699733',
+  'login' => '1523699552',

    This can be reverted

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Assigned: quietone » Unassigned
quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
2959410-7.patch14.48 KB
interdiff.txt2.85 KB

Fixed the items in #5

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Assigned: Unassigned » mikeryan
mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Assigned: mikeryan » Unassigned
Status: Needs review » Needs work 
  1. +++ b/core/modules/config_translation/migrations/d6_field_instance_label_description_translation.yml
@@ -0,0 +1,56 @@
+label: Field translation

    The label should be more specific, like "Field label and description translation". 

  2. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateFieldInstanceLabelDescriptionTest.php
@@ -0,0 +1,132 @@
+    // Delete a field in the database that a label and description translation.

    s/that/with/ ? 

  3. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateFieldInstanceLabelDescriptionTest.php
@@ -0,0 +1,132 @@
+    $this->assertSame("fr - An example text field.", $config_translation->get('description'));
...
+    $this->assertSame("Un exemple plusieurs valeurs champ décimal.", $config_translation->get('description'));

    Probably out of scope here, but why are most (but not all) "French" translations in English? 

  4. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateFieldInstanceLabelDescriptionTest.php
@@ -0,0 +1,132 @@
+    // Tests fields on 'test_plant' node type.

    test_planet

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
2959410-10.patch14.64 KB
interdiff.txt2.36 KB

1. Fixed.
2. To test the migration continues with somewhat broken data. Added comment.
3. Because I am mono-lingual and I created the translations at different times for different issues. One on occasion I had a friend visiting who spoke French and he was able to translate a couple of items. The translations are either in the non English language and it is obvious or it is in English with a prefix of the two character language code for the language it should be written in.
4. Fixed

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Status: Needs review » Reviewed & tested by the community

Looks good, thanks!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2959410-10.patch, failed testing. View results

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Status: Needs work » Reviewed & tested by the community

Retesting.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2959410-10.patch, failed testing. View results

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Needs work » Reviewed & tested by the community

Testbot Snafu.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Reviewed & tested by the community » Needs review

This too need the files moves to different directories. It is the migration that should be in content_translation, the rest in fields.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs review » Needs work

That was supposed to be NW

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
2959410-18.patch15.65 KB
interdiff.txt22.49 KB

Moved the source plugin, the source plugin test and the migration test to the field module.

Modified MigrateUpgradeReviewPageTest to enable config_translation. Then put 'i18ncck' into the list of what will be upgraded but also had to add 'i18nprofile' which wasn't in that list before because config_translation was not enabled.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented

Testing with Postgres and sqlite just to be safe since #2886609: Migrate translations for D6 i18n taxonomy 'localized' terms failed on Postgres.

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented
Status: Needs review » Reviewed & tested by the community

This was a big interdiff so cross checked it against what @quietone explained in #18.

The changes in the interdiff are:

1. class FieldLabelDescriptionTranslation has moved from config_translation to field

2. class MigrateFieldInstanceLabelDescriptionTest has moved from config_translation to field

3. class FieldInstanceLabelDescriptionTranslationTest has moved from config_translation to field

-- These 3 things are exactly as @quietone wrote in #18 and this change makes sense to me.

4. Finally, at the very end of the interdiff, MigrateUpgrade6ReviewPageTest has the changes @quietone listed in #18 (config_translation enabled, 'i18ncck' and 'i18nprofile' are now in the list of what will be upgraded) which also is OK.

Conclusion: All changes since the last RTBC make sense and there are no open items since the last RTBC.

All tests with MySQL, SQLite and PostgreSQL are green. RTBC.

Cheers,
Markus

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 18: 2959410-18.patch, failed testing. View results

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented
Issue tags: +Needs reroll

Needs re-roll

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
2959410-23.patch15.64 KB

Reroll.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Issue tags: -Needs reroll

Reroll passes tests for all dbs so ready for review

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Reviewed & tested by the community

Diff stats the same here between #18 and #23. Back to RTBC.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 23: 2959410-23.patch, failed testing. View results

jofitz’s picture

jofitz CreditAttribution: jofitz at ComputerMinds commented
Status: Needs work » Needs review
FileSize
2959410-28.patch15.24 KB

Patch from #23 no longer applies. Re-rolled.

Status: Needs review » Needs work

The last submitted patch, 28: 2959410-28.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
2959410-30.patch16 KB
interdiff.txt750 bytes

i18ncck is now available for MigrateUpgrade6Test, so move it from MissingPaths to AvailablePaths.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented

Added tests for PostgreSQL and SQLite.

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented

Thanks for the re-roll Jo Fitzgerald!

@quietone, I think we need to add the 'Multilingual' migration tag to this, right?

Markus

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
FileSize
2959410-33.patch16.02 KB
interdiff-30-33.patch656 bytes

Oh! Yes, it does. Thx masipila!

Status: Needs review » Needs work

The last submitted patch, 33: interdiff-30-33.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review

I named the interdiff wrong (again) so the fail above is a fail on an interdiff. The tests are really passing.

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented

This was RTBC in #25. Changes since that are

1. i18ncck is now available for MigrateUpgrade6Test, so move it from MissingPaths to AvailablePaths

2. Multilingual tag was added.

All tests are green so back to RTBC.

Markus

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented
Status: Needs review » Reviewed & tested by the community

  • catch committed 1aee610 on 8.7.x 
    Issue #2959410 by quietone, Jo Fitzgerald, masipila, mikeryan: Migrate...

  • catch committed 3c0af37 on 8.6.x 
    Issue #2959410 by quietone, Jo Fitzgerald, masipila, mikeryan: Migrate...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.7.x-dev » 8.6.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.7.x and cherry-picked to 8.6.x. Thanks!

Status: Fixed » Closed (fixed)

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