Migrate Drupal 7 user entity translations data to Drupal 8

So what modules does this include?

Added relation to #2073467: Migrate Drupal 7 Entity Translation settings to Drupal 8 which might resolve this.

Tagging with Migrate critical and Migrate Drupal according to the policy discussion for Migrate Drupal stability criteria.

This functionality was in contrib in D7 but the capability is provided by core in D8.

Postponing on #2073467: Migrate Drupal 7 Entity Translation settings to Drupal 8 as it might be a duplicate.

This is now postponed on #2975666: Migrate Drupal 7 node entity translations data to Drupal 8.

This will be resolved in #2975666: Migrate Drupal 7 node entity translations data to Drupal 8.

Re-opened because parent issue was getting too big and complex.

Adding relation to meta.

Here's a start, combined with the latest patch from #2975666: Migrate Drupal 7 node entity translations data to Drupal 8 where the base support for entity translation is being added.

Fixing the test.

+++ b/core/modules/user/migrations/d7_user_entity_translation.yml
@@ -0,0 +1,49 @@
+id: d7_user_entity_translation
+label: User accounts entity translations
+migration_tags:
+  - Drupal 7
+  - translation
+  - Content
+class: Drupal\user\Plugin\migrate\User
+source:
+  plugin: d7_user_entity_translation
+process:
+  uid: uid
+  name: name
+  pass: pass
+  mail: mail
+  created: created
+  access: access
+  login: login
+  status: status
+  timezone: timezone
+  langcode: language
+  preferred_langcode:
+    plugin: user_langcode
+    source: user_language
+    fallback_to_site_default: true
+  preferred_admin_langcode:
+    plugin: user_langcode
+    source: user_language
+    fallback_to_site_default: true
+  init: init
+  roles:
+    plugin: migration_lookup
+    migration: d7_user_role
+    source: roles
+  user_picture:
+    -
+      plugin: default_value
+      source: picture
+      default_value: null
+    -
+      plugin: migration_lookup
+      migration: d7_file
+  content_translation_source: source
+destination:
+  plugin: entity:user
+  translations: true
+  destination_module: content_translation
+migration_dependencies:
+  required:
+    - d7_user

Do we need to map all these (untranslatable) properties again in the d7_user_entity_translation as they are already migrated in d7_user?

Markus

Now that #2975666: Migrate Drupal 7 node entity translations data to Drupal 8 is in, it's time to do the same here.

Re #24: You're right, a lot of those are not needed. I which we had check that for the d7_node_entity_translation too.

I tested this manually with the latest 8.6.x from git + patch #26 applied.

Unfortunately the user translation was not migrated properly, see the screenshot below.

I re-tested this after discussing with @maxocub on Slack. He was not able to reproduce the issue in #27 and neither was I when I tried this again.

Here are the findings from my manual tests.

D7 setup
User entity had two additional fields.
One of these was set to be translatable using Entity Translation and the other one was not translatable.

Test content and test results
1. Test user 1 had content in both custom fields and had translation.

• TEST OK. The translation was created in D8 as expected and the field translation was migrated as expected.
• TEST OK. I also tested the user login just to be sure and it worked as expected.

2. Test user 2 had content in both custom fields but did not have a translation.

• TEST OK. The content of the two custom fields was migrated as expected.
• TEST OK. I also tested that user roles were migrated properly.
• TEST OK. I also tested that blocked / active status was migrated properly.

3. Test user 3 did not have content in neither of the custom fields.

• TEST OK. The user was migrated properly.

Concerns / questions
I checked the users_field_data table in the D8 database.
4. content_translation_source has 'und' for all users migrated from D7 even though in D7 entity_translation.source I have the translation source language populated for my test user 1.
5. content_translation_uid is NULL in D8 even though in D7 entity_translation.uid we have values for the translations.
6. I then checked the mappings in the patch and noticed that also content_translation_status, content_translation_outdated and content_translation_created are not mapped.

Is there a reason why we would not migrate these properties as well?

Cheers,
Markus

Here's a new patch where I:

1. Move d7_user_entity_translation from the User module to the Content Translation module, like other translations migrations.
2. Add mappings for the translations' metadata (uid, status, outdated, created.

I wish we had done the translation metadata mapping for the nodes too. I'm going to open a follow-up for that.

I tested #29 and it looks good. In addition to my previous tests reported in #28, my manual test covered that the following entity translation metadata properties were migrated properly:

• content_translation_outdated
• content_translation_uid
• content_translation_status
• content_translation_created

I was still wondering about the D8 'users_field_data.changed'. This timestamp means in D8 the moment when then translation was last updated. We don't currently map this property in patch #29. I checked the mapping of d7_user.yml and we don't map it there either. As a result, this is what happens:

If the user does not have translations

• users_field_data will only have one record for the uid
• users_field_data.changed gets the timestamp when d7_user was executed

If the user has translations

• users_field_data will have N records for the uid, one for each language version
• users_field_data.changed (for all language versions) gets the timestamp when d7_user_entity_translation was executed.

In my opinion this is conceptually expected behavior. The user record was "last updated" in D8 database the moment it was migrated to D8 (not the moment it was last updated in D7) and the same thing applies to the language versions of the user, it was "last updated" in D8 the moment it was migrated to D8, not the moment when the language versions were last updated in D7.

• In other words, I'm suggesting that we do not change d7_user.yml
• In other words, I'm suggesting that we leave users_field_data.changed un-mapped like it is currently in patch #29.

I just wanted to write out the evaluation and logical thinking process so that the "by design" approach is documented (assuming that others agree with this approach).

I'll leave this to NR as I'm too tired to review the tests. I'll be again travelling for a couple of days tomorrow so if somebody else has time to review this, feel free to do so.

Cheers,
Markus

Additional remark:
d7_node and d7_node_entity_translation are mapping 'changed' which is contradicting the outcome of my previous comment.
https://api.drupal.org/api/drupal/core%21modules%21node%21migrations%21d...
https://api.drupal.org/api/drupal/core%21modules%21node%21migrations%21d...

We have changed:changed mapping for both nodes and node entity translations. So it would be consistent to have the same thing for users as well. However, we have a conceptual difference between user and node entities. D6 and D7 'users' table does **not** have a property 'changed' like nodes do. This means that we can **not** map this D8 property in d6_user.yml or in d7_user.yml. And because of this, we should definitely **not** map it in d7_user_entity_translation.yml either.

Hence: The outcome (let's not map 'changed') is the same as in my previous post, it's just that the argumentation is different :)

Now it's time to go to bed. See ya!

Markus

I reviewed the patch once more and found only one tiny nit.

1. Class description should start with a third person verb.

+++ b/core/modules/user/src/Plugin/migrate/source/d7/UserEntityTranslation.php
@@ -0,0 +1,79 @@
+/**
+ * Drupal 7 user entity translations source from database.
+ *
+ * @MigrateSource(
+ *   id = "d7_user_entity_translation",
+ *   source_module = "entity_translation"
+ * )
+ */
+class UserEntityTranslation extends FieldableEntity {

Other than this, this looks good to me:

• I manually tested this and posted positive test results in #28 and #30.
• I was thinking out loud about the fact that we are not mapping 'changed' attribute of the entity translation in my comment #30 and #31. TLDR; We should not do that for user entity translations because we can't do that for user entity. See #31.
• Test coverage for the source plugin is OK
• Test coverage in class MigrateUserTest is OK for user entity translation, including the translation metadata

Once the coding standard nit 1. is fixed and tests are green on all database backends, this is ready in my eyes. Triggering the testbot for SQLite and PostgreSQL.

Cheers,
Markus

Thanks for the review @masipila, here's the coding standard fix.

Thanks @maxocub!

All feedback addressed. Detailed RTBC report in #32.

RTBC.

I'm proposing this to be backported to 8.6.x as this is for multilingual migrations which are still experimental but I'll leave that as committers' decision.

Markus

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