Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem / motivation
Drupal 7 has a contributed module Entity Translation which allows to have field level translations on fieldable entities. This capability has moved to core in Drupal 8.
The scope of this issue is
- Migrate the actual translated data of user the fields.
Remaining tasks
- Write the patch
- Review
- Commit
Comment | File | Size | Author |
---|---|---|---|
#33 | 2669984-33.patch | 19.98 KB | maxocub |
#33 | interdiff-2669984-29-33.txt | 634 bytes | maxocub |
Comments
Comment #2
quietone CreditAttribution: quietone as a volunteer commentedComment #3
quietone CreditAttribution: quietone as a volunteer commentedComment #7
DamienMcKennaSo what modules does this include?
Comment #8
mikeryanComment #11
masipila CreditAttribution: masipila commentedAdded relation to #2073467: Migrate Drupal 7 Entity Translation settings to Drupal 8 which might resolve this.
Comment #12
masipila CreditAttribution: masipila commentedTagging 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.
Comment #13
heddnComment #14
heddnPostponing on #2073467: Migrate Drupal 7 Entity Translation settings to Drupal 8 as it might be a duplicate.
Comment #15
maxocub CreditAttribution: maxocub as a volunteer commentedThis is now postponed on #2975666: Migrate Drupal 7 node entity translations data to Drupal 8.
Comment #16
maxocub CreditAttribution: maxocub as a volunteer commentedThis will be resolved in #2975666: Migrate Drupal 7 node entity translations data to Drupal 8.
Comment #17
maxocub CreditAttribution: maxocub as a volunteer commentedRe-opened because parent issue was getting too big and complex.
Comment #18
maxocub CreditAttribution: maxocub as a volunteer commentedComment #19
maxocub CreditAttribution: maxocub as a volunteer commentedAdding relation to meta.
Comment #20
masipila CreditAttribution: masipila commentedComment #21
maxocub CreditAttribution: maxocub as a volunteer commentedHere'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.
Comment #22
maxocub CreditAttribution: maxocub as a volunteer commentedFixing the test.
Comment #24
masipila CreditAttribution: masipila commentedDo we need to map all these (untranslatable) properties again in the d7_user_entity_translation as they are already migrated in d7_user?
Markus
Comment #25
maxocub CreditAttribution: maxocub as a volunteer commentedNow that #2975666: Migrate Drupal 7 node entity translations data to Drupal 8 is in, it's time to do the same here.
Comment #26
maxocub CreditAttribution: maxocub for Acquia commentedRe #24: You're right, a lot of those are not needed. I which we had check that for the d7_node_entity_translation too.
Comment #27
masipila CreditAttribution: masipila commentedI 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.
Comment #28
masipila CreditAttribution: masipila commentedI 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.
2. Test user 2 had content in both custom fields but did not have a translation.
3. Test user 3 did not have content in neither of the custom fields.
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
Comment #29
maxocub CreditAttribution: maxocub for Acquia commentedHere's a new patch where I:
I wish we had done the translation metadata mapping for the nodes too. I'm going to open a follow-up for that.
Comment #30
masipila CreditAttribution: masipila commentedI 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:
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
If the user has translations
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.
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
Comment #31
masipila CreditAttribution: masipila commentedAdditional 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
Comment #32
masipila CreditAttribution: masipila as a volunteer commentedI reviewed the patch once more and found only one tiny nit.
1. Class description should start with a third person verb.
Other than this, this looks good to me:
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
Comment #33
maxocub CreditAttribution: maxocub for Acquia commentedThanks for the review @masipila, here's the coding standard fix.
Comment #34
masipila CreditAttribution: masipila as a volunteer commentedThanks @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
Comment #36
catchCommitted/pushed to 8.7.x and cherry-picked to 8.6.x. Thanks!