Migrate Drupal 6 user profile field value option translations

#2225717: Add config translation support to migrations and implement for Drupal 6 user profile fields is in, no loner postponed.

Still not clear on, what will be the first approach here. Working on it. Any guidance will help

@rakesh.gectcr, thanks for taking this on.

AFAIKT, the d6 test fixture does not include a user profile select field. So the first steps are to add the data to the fixture and add the test. There is a handbook page for generating database fixtures for d8 migrate tests. As usual, please improve that page, it is pretty basic. For the test, the direction provided in #7 looks good. This will need a new source plugin, so that and it's test would be next. Then the migration itself.

I worked on the test fixture a bit. I added a select field to the user profile. But I can't find where/how to translate it. The new field doesn't even appear on the Translate interface and I'm not sure how to translate the select values. Can someone explain these steps?

Here is a start. This doesn't include the changes to the fixture because I don't have time to clean it up now. So, there is no point in running the tests until that is done.

This will also need to be postponed on #2961114: Migrate D6 i18n CCK field option translations because I'm sure it needs the destination changes but I am setting to NW until I add the changes to the test fixture.

Meant to unassign rakesh.gectcr, it has been over a year.

Add in the changed to the test fixture.

Now postponing until #2961114: Migrate D6 i18n CCK field option translations is committed.

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

No longer postponed. Testing patch in #14.

Not surprised that it needs a reroll.

Re-rolled https://www.drupal.org/project/drupal/issues/2845975#comment-12753681

@aleevas, wrong patch?

Re-rolled.

Fixed coding standards errors.

Leaving as Needs Work to fix test failures.

This took a while. I think there were errors in patch #14 which wasn't helping at all.

• Changed the logic in the new process plugin to return NULL when there is no translation.
• Changed the name of the profile field for count of trees from profile_ to profile_count_trees this seems to have been lost betweem #12 and #14 (I think). Either way it is fixed now.
• Added an option that should be in the fixture.
• Added tests for the other option fields that are translated

Now, lets see if that fixes everything.

Added a conditional to the source plugin, d6_profile_field_translation, because the test fixture introduces a profile field that has does not have a translation for the title but does for this options. This meant the query was returning a row where language was NULL.
Took out a lot of changes in the test fixture which are not needed. At least I hope not.
And fixed the translation values, it should not have a value of 30.

Fix formatting in yml file and update the entity counts.

This is now ready for review. Thanks!

As always, great work with those tricky i18n issues.

1. +++ b/core/modules/config_translation/migrations/d6_profile_field_option_translation.yml
   @@ -0,0 +1,59 @@
   +    allowed_values: settings
   

   Unused constant.

2. +++ b/core/modules/user/src/Plugin/migrate/process/d6/ProfileFieldOptionTranslation.php
   @@ -0,0 +1,47 @@
   +  /**
   +   * {@inheritdoc}
   +   *
   +   * Get the field default/mapped settings.
   +   */
   

   According to https://www.drupal.org/docs/develop/standards/api-documentation-and-comm..., {@inheritdoc} should be the only line in the docblock.

3. +++ b/core/modules/user/src/Plugin/migrate/process/d6/ProfileFieldOptionTranslation.php
   @@ -0,0 +1,47 @@
   +
   +        default:
   

   Do we need this empty default clause?

4. +++ b/core/modules/user/src/Plugin/migrate/source/d6/ProfileFieldOptionTranslation.php
   @@ -0,0 +1,63 @@
   +
   +    $res = $query->execute()->fetchAll();
   

   This can be removed.

First a reroll.

And fixes for all the items in #31.

Messed up the test fixture in the reroll. This should fix it.

I really dislike rerolling the test fixtures.

Maybe this will fix it.

Finally, test are passing again! Everything in #31 has been addressed so this is ready for review again,

The patch looks quite ready, but I still have two questions and one nit :

1. +++ b/core/modules/config_translation/migrations/d6_profile_field_option_translation.yml
   @@ -0,0 +1,58 @@
   +  type:
   +    plugin: static_map
   +    source: type
   +    map:
   +      checkbox: boolean
   +      date: datetime
   +      list: text
   +      selection: list_string
   +      textfield: text
   +      textarea: text_long
   +      url: link
   +  results:
   +    plugin: d6_profile_field_option_translation
   +    source:
   +      - '@type'
   +      - translation
   
   +++ b/core/modules/user/src/Plugin/migrate/process/d6/ProfileFieldOptionTranslation.php
   @@ -0,0 +1,46 @@
   +      switch ($field_type) {
   +        case 'list_string':
   +        case 'list_integer':
   +        case 'list_float':
   +          foreach ($list as $key => $value) {
   +            $allowed_values[] = ['label' => $value];
   +          }
   +          break;
   +
   +      }
   

   Because there's no "bypass: true", the values in the "static_map" process plugin are the only field types that can be passed to the "d6_profile_field_option_translation" process plugin. And in the switch statement of this last process plugin, we have cases for "list_string", "list_integer" & "list_float", but no default case. This makes me think that either there is some some missing values in the "static_map" (list_integer & list_float) and some extra ones too (anything that's not list_*) or either we need a default case in the switch statement.

   Since this patch is dealing with option fields, I think the possible values in the static map should only be list_* fields, but maybe I'm missing something...

2. +++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php
   @@ -27764,30 +27794,6 @@
   -  'lid' => '1672',
   -  'translation' => 'fr - Type',
   -  'language' => 'fr',
   -  'plid' => '0',
   -  'plural' => '0',
   -  'i18n_status' => '0',
   -))
   -->values(array(
   -  'lid' => '1692',
   -  'translation' => 'fr - Talos IV',
   -  'language' => 'fr',
   -  'plid' => '0',
   -  'plural' => '0',
   -  'i18n_status' => '0',
   -))
   -->values(array(
   -  'lid' => '1694',
   -  'translation' => 'fr - The home of Captain Christopher Pike.',
   -  'language' => 'fr',
   -  'plid' => '0',
   -  'plural' => '0',
   -  'i18n_status' => '0',
   -))
   -->values(array(
   

   Are those lines removed for a reason or by accident? I can't figure out why they would need (or not) to get removed.

3. +++ b/core/modules/user/tests/src/Kernel/Plugin/migrate/source/d6/ProfileFieldOptionTranslationTest.php
   @@ -0,0 +1,73 @@
   +    $test[0]['expected_data'] = [];
   

   Nit: This line can be removed.

Just fixing the nit now (time to listen to the DrupalSouth session!).

40.1. That static_map is copied (like most of that migration) from user_profile_field.yml. Having said that, I think (maybe) the source plugin should extend from ProfileField.

40.2. To fix this the following needs to be restored to the fixture, which will be easier when #3016011: Reroll all migrate dump files is committed.

- 'lid' => '1672',
- 'translation' => 'fr - Type',
- 'language' => 'fr',
- 'plid' => '0',
- 'plural' => '0',
- 'i18n_status' => '0',
-))

40.1 Removed unused items from static map as suggested. Also, changed the source plugin to extend from ProfileField.
40.2 Restored the accidentally removed block from the test fixture.

The NW is because I named the interdiff a patch (again). This is really ready for review, all the points raised in #40 have been addressed.

Triaging the issue queue.

This is the reroll.

Actual patch can apply.

This needed a reroll.

Arg, a stupid typo.

NR for testbot

Remove postpone from the IS.

This is ready for review.

I tested the patch manually and it worked as expected, the option translations from the Profile select fields were migrated properly.

But, the IS should be updated because it is misleading. It says that radio button and checkbox field translation should be migrated too in this issue, but they are not. I've checked on Drupal 6 what type of fields needs their option translation migrated and it seem that it is only the select boxes, as this patch does.

As for the code :

1. +++ b/core/modules/config_translation/migrations/d6_profile_field_option_translation.yml
   @@ -0,0 +1,52 @@
   +  type:
   +    plugin: static_map
   +    source: type
   +    map:
   +      selection: list_string
   
   +++ b/core/modules/user/src/Plugin/migrate/process/d6/ProfileFieldOptionTranslation.php
   @@ -0,0 +1,46 @@
   +      switch ($field_type) {
   +        case 'list_string':
   +        case 'list_integer':
   +        case 'list_float':
   +          foreach ($list as $key => $value) {
   +            $allowed_values[] = ['label' => $value];
   +          }
   +          break;
   +
   +      }
   

   I'm still bugging over this static map and switch statement mismatch. The only field type that we need is the list_string, as we already have in the static map. So, we should not need the switch statement for list_integer and list_float, I think a simple if statement that checks if the field type is list_string should be enough.

2. +++ b/core/modules/user/src/Plugin/migrate/source/d6/ProfileFieldOptionTranslation.php
   @@ -0,0 +1,62 @@
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +
   

   A lonely @inheritdoc that has lost its method...

Fixes for #55.1 and #55.2. Still need to update the IS>

Updated IS and tried to keep some of the history.

@maxocub thanks for the review!

All feedbacks are addressed. Manual and automated tests are positive. IS has been updated. Let's RTBC this.

Failure looked unrelated, so retesting

Tests passing again. There has been no changes to the patch since the RTBC so I am restoring the RTBC.

Committed and pushed 75a4213161 to 8.7.x and cdd317ebc6 to 8.6.x. Thanks!