Migrate D6 i18n CCK field option translations

I was testing this manually and I stumble upon a couple exceptions. First, if you don't have the Profile module enabled on D6 you get this:

PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal.profile_fields' doesn't exist

Then if you enable the Profile module but don't create any profile you get this:

Drupal\Core\Database\InvalidQueryException: Query condition 'objectid NOT IN ()' cannot be empty. 

I don't see why this migration requires Profile, maybe there's a reason I don't know. In any case, I think we should avoid those exceptions.

Otherwise, I haven't completed my review yet, but I can say that after enabling the Profile module and adding a profile, the migration seems to be working well.

Discussed in the migrate maintainers meeting w/ @quietone and @phenaproxima and @heddn and none of think there is a BC break with the destination getIds because we don't have any existing field config destination migrations.

+++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityFieldStorageConfig.php
@@ -58,6 +58,9 @@ class EntityFieldStorageConfig extends EntityConfigBase {
+    if ($this->isTranslationDestination()) {
+      $ids['langcode']['type'] = 'string';
+    }

We should never use conditional logic in a getIds. Fix this in 9.0. Should add an @TODO and link to an issue.

Re-parenting so that all multilingual migrations can be found from the same meta. Moved original parent to related issues.

All properties.

This might just be a quick question and reply:

+++ b/core/modules/config_translation/migrations/d6_field_instance_option_translation.yml
@@ -0,0 +1,178 @@
+    map:
+      userreference:
+        userreference_select: entity_reference
+        userreference_buttons: entity_reference
+        userreference_autocomplete: entity_reference
+      nodereference:
+        nodereference_select: entity_reference

+++ b/core/modules/config_translation/migrations/d6_field_option_translation.yml
@@ -0,0 +1,144 @@
+    map:
+      userreference:
+        userreference_select: entity_reference
+        userreference_buttons: entity_reference
+        userreference_autocomplete: entity_reference
+      nodereference:
+        nodereference_select: entity_reference
+      number_integer:
+        number: integer

I assume this is a pretty standard list of mappings. And with our work on a base class for entity reference, node reference, user reference, instead of constantly copy/pasting this all over the place, would it make sense to move this in to a trait and do this in code? Or should this be a follow-up to clean these things up?

I feel woefully unqualified to RTBC this thing, but I cannot find anything else to comment about. Let's get that follow-up opened and on to RTBC then.

No more blockers.

Thanks, great work. Some notes:

1. +++ b/core/modules/config_translation/migrations/d6_field_instance_option_translation.yml
   @@ -0,0 +1,178 @@
   +label: Field option translation
   

   Field instance option translation

2. +++ b/core/modules/config_translation/migrations/d6_field_instance_option_translation.yml
   @@ -0,0 +1,178 @@
   +      fr_phone:
   +        phone_textfield: telephone
   +      be_phone:
   +        phone_textfield: telephone
   +      it_phone:
   +        phone_textfield: telephone
   +      el_phone:
   +        phone_textfield: telephone
   +      ch_phone:
   +        phone_textfield: telephone
   +      ca_phone:
   +        phone_textfield: telephone
   +      cr_phone:
   +        phone_textfield: telephone
   +      pa_phone:
   +        phone_textfield: telephone
   +      gb_phone:
   +        phone_textfield: telephone
   +      ru_phone:
   +        phone_textfield: telephone
   +      ua_phone:
   +        phone_textfield: telephone
   +      es_phone:
   +        phone_textfield: telephone
   +      au_phone:
   +        phone_textfield: telephone
   +      cs_phone:
   +        phone_textfield: telephone
   +      hu_phone:
   +        phone_textfield: telephone
   +      pl_phone:
   +        phone_textfield: telephone
   +      nl_phone:
   +        phone_textfield: telephone
   +      se_phone:
   +        phone_textfield: telephone
   +      za_phone:
   +        phone_textfield: telephone
   +      il_phone:
   +        phone_textfield: telephone
   +      nz_phone:
   +        phone_textfield: telephone
   +      br_phone:
   +        phone_textfield: telephone
   +      cl_phone:
   +        phone_textfield: telephone
   +      cn_phone:
   +        phone_textfield: telephone
   +      hk_phone:
   +        phone_textfield: telephone
   +      mo_phone:
   +        phone_textfield: telephone
   +      ph_phone:
   +        phone_textfield: telephone
   +      sg_phone:
   +        phone_textfield: telephone
   +      jo_phone:
   +        phone_textfield: telephone
   +      eg_phone:
   +        phone_textfield: telephone
   +      pk_phone:
   +        phone_textfield: telephone
   +      int_phone:
   +        phone_textfield: telephone
   
   +++ b/core/modules/config_translation/migrations/d6_field_option_translation.yml
   @@ -0,0 +1,144 @@
   +      fr_phone:
   +        phone_textfield: telephone
   +      be_phone:
   +        phone_textfield: telephone
   +      it_phone:
   +        phone_textfield: telephone
   +      el_phone:
   +        phone_textfield: telephone
   +      ch_phone:
   +        phone_textfield: telephone
   +      ca_phone:
   +        phone_textfield: telephone
   +      cr_phone:
   +        phone_textfield: telephone
   +      pa_phone:
   +        phone_textfield: telephone
   +      gb_phone:
   +        phone_textfield: telephone
   +      ru_phone:
   +        phone_textfield: telephone
   +      ua_phone:
   +        phone_textfield: telephone
   +      es_phone:
   +        phone_textfield: telephone
   +      au_phone:
   +        phone_textfield: telephone
   +      cs_phone:
   +        phone_textfield: telephone
   +      hu_phone:
   +        phone_textfield: telephone
   +      pl_phone:
   +        phone_textfield: telephone
   +      nl_phone:
   +        phone_textfield: telephone
   +      se_phone:
   +        phone_textfield: telephone
   +      za_phone:
   +        phone_textfield: telephone
   +      il_phone:
   +        phone_textfield: telephone
   +      nz_phone:
   +        phone_textfield: telephone
   +      br_phone:
   +        phone_textfield: telephone
   +      cl_phone:
   +        phone_textfield: telephone
   +      cn_phone:
   +        phone_textfield: telephone
   +      hk_phone:
   +        phone_textfield: telephone
   +      mo_phone:
   +        phone_textfield: telephone
   +      ph_phone:
   +        phone_textfield: telephone
   +      sg_phone:
   +        phone_textfield: telephone
   +      jo_phone:
   +        phone_textfield: telephone
   +      eg_phone:
   +        phone_textfield: telephone
   +      pk_phone:
   +        phone_textfield: telephone
   +      int_phone:
   +        phone_textfield: telephone
   

   Hm, where do these come from? They both look to excessive and too limited if so specific :)

3. +++ b/core/modules/config_translation/migrations/d6_field_option_translation.yml
   @@ -0,0 +1,144 @@
   +label: Field configuration
   

   Field option translation

4. +++ b/core/modules/field/src/Plugin/migrate/process/d6/FieldInstanceOptionTranslation.php
   @@ -0,0 +1,53 @@
   +    list($field_type, $global_settings) = $value;
   

   It is not global settings when its about an instance only when its about a field, no?

5. +++ b/core/modules/field/src/Plugin/migrate/process/d6/FieldInstanceOptionTranslation.php
   @@ -0,0 +1,53 @@
   +    if (isset($global_settings['allowed_values'])) {
   +      $list = explode("\n", $global_settings['allowed_values']);
   
   +++ b/core/modules/field/src/Plugin/migrate/process/d6/FieldOptionTranslation.php
   @@ -0,0 +1,59 @@
   +    if (isset($global_settings['allowed_values'])) {
   +      $list = explode("\n", $global_settings['allowed_values']);
   

   Is allowed values the only thing we need to translate / handle? Prefix/suffix, etc. are not options we would need to translate or is that already covered or covered by another issue?

6. +++ b/core/modules/field/src/Plugin/migrate/source/d6/FieldOptionTranslation.php
   @@ -0,0 +1,78 @@
   +      'translation' => $this->t('Translation of either the title or explanation.'),
   

   What's an explanation here?

For #37.2, see #3008328: Create Field Plugins for Drupal 6 phone and number fields. where we plan to fix that up. I think that covers all the questions brought up since last RTBC, so let's try it again.

Thanks.

#3008328: Create Field Plugins for Drupal 6 phone and number fields. does not explain why would phone numbers have such country/language(?) specific variants, is that a phone module thing and not a translation thing to support different phone number formats? I went ahead and checked that at https://cgit.drupalcode.org/phone/tree/phone.module?h=6.x-1.x#n28 and is apparently a funky implementation on Drupal 6 where country codes are represented with one field type each. Hum. Ok. You are doing the right thing here.

I looked at https://cgit.drupalcode.org/i18n/tree/i18ncck/i18ncck.module?h=6.x-1.x and it does indeed only do label/description translation for the fields (migrations for which I believe are already done earlier in Drupal 8). Even Drupal 7's i18n at https://cgit.drupalcode.org/i18n/tree/i18n_field/i18n_field.i18n.inc only extends that with supporting default value translation for text type fields. Again you are doing the right thing here. One minor thing left then:

+++ b/core/modules/config_translation/migrations/d6_field_instance_option_translation.yml
@@ -1,5 +1,5 @@
-label: Field option translation
+label: Field instance option translation

+++ b/core/modules/config_translation/migrations/d6_field_option_translation.yml
@@ -1,5 +1,5 @@
-label: Field configuration
+label: Field option translation configuration

I don't believe "configuration" is used as a migration suffix (for other field config related things or otherwise). Either both should have it or neither.

Tagging novice. Let's call them configuration in both cases. I checked and we have precedent on that in d7_field_instance.yml and d7_field.yml

Superb, thanks, landed!