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
When migrating comments with fields from Drupal 7 to Drupal 8, the field values are not migrated.
Proposed resolution
Nodes & taxonomy terms field values are getting migrated by their derivers. Users field values are getting migrated by the User migration plugin. For comments, we could do like for users and add a migration plugin to deal with field values.
Remaining tasks
- Write the patch
- Review
- Commit
User interface changes
None
API changes
New migration plugin class for commets.
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#12 | 2979916-12.patch | 5.07 KB | maxocub |
#12 | 2979916-12-test-only.patch | 2.89 KB | maxocub |
#12 | interdiff-2979916-10-12.txt | 622 bytes | maxocub |
Comments
Comment #2
maxocub CreditAttribution: maxocub commentedHere's a failing test.
Comment #3
maxocub CreditAttribution: maxocub commentedAnd here's the fix.
Comment #5
masipila CreditAttribution: masipila as a volunteer commentedI tested this manually.
About my test setup
comments.language
of this comment is 'en' and there are two entries infield_data_field_comment_translatable
, one for 'en' and one for 'fi' translation.The comment field translation migration is handled separately in #2981000: Migrate Drupal 7 comment entity translations data to Drupal 8 so handling all the translation aspects is clearly out of scope from this issue.
However, I would have expected that this patch migrates the primary language of the comment which was found in
field_data_field_comment_translatable
. In my case it migrated the value of 'fi' translation of the comment field but in D8 database this value has language 'en' incomment__field_comment_translatable
.Is it easy / possible to migrate here the record that has the same language code as what is found in comments.language for the comment in question? I would otherwise be OK to leave the whole language handling to #2981000: Migrate Drupal 7 comment entity translations data to Drupal 8 but I'm quite worried about the fact that in my test the content value and it's language code got corrupted.
Kicking to NW so that we can conclude what to do with this.
I also added tags to the issue. Since this is about content migration, I added this as Migrate Critical and Migrate Drupal. This is a blocker for #2981000: Migrate Drupal 7 comment entity translations data to Drupal 8 so I also added a blocker tag.
Cheers,
Markus
Edit: formatting
Comment #6
maxocub CreditAttribution: maxocub commentedIt's totally normal. Right now the migration of field values (in FieldableEntity) does not support translations at all. If a field is translated (ie. it has a langcode), the migration will only pick the last value, no matter what the entity language is. This problem will indeed be addressed in #2981000: Migrate Drupal 7 comment entity translations data to Drupal 8. In the mean time, this patch should work fine without Entity Translation enabled on the D7 site.
Comment #7
maxocub CreditAttribution: maxocub as a volunteer commentedBack to NR.
Comment #8
phenaproximaLooks pretty good to me! I think the test coverage could be expanded a bit, but this certainly proves that at least some comment fields are migrated. Can we get a fail patch as well?
Rather than nest the entire function body, let's just return early if $this->init is TRUE.
Same idea here.
Let's use $this->getSourceConfiguration() instead of $this->source.
Why is this check here? I'm not sure $field_type will ever be empty...
Let's use $this->setProcessOfProperty() here.
Why is d7_taxonomy_vocabulary needed?
Comment #9
masipila CreditAttribution: masipila as a volunteer commentedI re-tested this so that Entity Translation was not enabled on my D7 source site.
I was still curious about the language setup, mainly so that we can address this aspect in the follow-up #2981000: Migrate Drupal 7 comment entity translations data to Drupal 8.
D7 setup:
Data in D7 database
Drupal 8 data after migration
Based on what @maxocub described in #6, this is the expected result for the custom comment field migration. The language of the comment field and comment itself are both aligned ('en' in both cases), it's just that we don't manage the migration of the comment language properly to begin with. But that is not in the scope of this issue.
Conclusion:
Comment #10
maxocub CreditAttribution: maxocub as a volunteer commented@phenaproxima, @masipila: Thanks for the reviews!
Re #8:
There's a test only patch, but I don't have time to expand the test coverage right now.
Comment #12
maxocub CreditAttribution: maxocub as a volunteer commentedOups, missing semicolon.
Comment #15
masipila CreditAttribution: masipila as a volunteer commented@phenaproxima, you were thinking out loud in #8 that test coverage could be improved.
I was reading the patch on my way to work today and it is checking that
$comment->field_integer
gets migrated as expected.Were you thinking that we should cover some other field types in addition to the numeric field or what's your concern on the coverage?
Markus
Comment #16
phenaproximaThat's what I was thinking, but I think I'm deferring it to a follow-up because this is definitely a critical Migrate issue.
Comment #17
maxocub CreditAttribution: maxocub as a volunteer commentedFollow-up created: #2982373: Add more test coverage to comment field values migration.
Comment #18
phenaproximaOkay. RTBC once green on all backends.
Comment #19
catchCommitted d1a1cc5 and pushed to 8.6.x. Thanks!