D7 comment field values are not migrated

I tested this manually.

About my test setup

• I have a field 'field_data_field_comment_translatable' on my D7 site which is translatable using Entity Translation as the name suggests.
• I have one comment on my D7 site with cid 1. The primary language of this comment is 'en' and there is a translation to 'fi'.
• In other words, D7 comments.language of this comment is 'en' and there are two entries in field_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' in comment__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

Looks 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?

1. +++ b/core/modules/comment/src/Plugin/migrate/D7Comment.php
   @@ -0,0 +1,48 @@
   +    if (!$this->init) {
   

   Rather than nest the entire function body, let's just return early if $this->init is TRUE.

2. +++ b/core/modules/comment/src/Plugin/migrate/D7Comment.php
   @@ -0,0 +1,48 @@
   +      if (\Drupal::moduleHandler()->moduleExists('field')) {
   

   Same idea here.

3. +++ b/core/modules/comment/src/Plugin/migrate/D7Comment.php
   @@ -0,0 +1,48 @@
   +        ] + $this->source;
   

   Let's use $this->getSourceConfiguration() instead of $this->source.

4. +++ b/core/modules/comment/src/Plugin/migrate/D7Comment.php
   @@ -0,0 +1,48 @@
   +          if (empty($field_type)) {
   +            continue;
   +          }
   

   Why is this check here? I'm not sure $field_type will ever be empty...

5. +++ b/core/modules/comment/src/Plugin/migrate/D7Comment.php
   @@ -0,0 +1,48 @@
   +            $this->process[$field_name] = $field_name;
   

   Let's use $this->setProcessOfProperty() here.

6. +++ b/core/modules/comment/tests/src/Kernel/Migrate/d7/MigrateCommentTest.php
   @@ -40,6 +51,9 @@ protected function setUp() {
   +      'd7_taxonomy_vocabulary',
   

   Why is d7_taxonomy_vocabulary needed?

I 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:

• Core Content Translation module is enabled
• Content type has Multilingual support enabled which means that a node can have a language (no translations, just language can be defined)
• I added a custom comment field to this content type
• I created one node so that language is English and added one comment to it
• I created second node so that language is Finnish and added one comment to it

Data in D7 database

• The two nodes have a language (en / fi) defined in node table
• The two comments have a language (en / fi) in comment table
• The two comment field entries have language 'und' in field_data_field_custom_comment_field i.e. D7 does not copy the language of the comment to the comment field record in this setup

Drupal 8 data after migration

• The two nodes have their language migrated correctly
• The two comments have their language migrated as 'en' (D8 site default) even if one of the comments had 'fi' in D7. Note: this observation is not in the scope of this issue.
• The two comment field entries have their language migrated as 'en' (D8 site default) whereas they were 'und' in D7.

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:

• What comes to the scope of this issue, it is functionally OK in my manual test.
• In other words, this issue is waiting for @phenaproxima's feedback to be addressed.
• I will investigate more about the D7 multilang migrations in general and open more issues. There will most probably be more than one.

@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

Were you thinking that we should cover some other field types in addition to the numeric field

That's what I was thinking, but I think I'm deferring it to a follow-up because this is definitely a critical Migrate issue.

Okay. RTBC once green on all backends.

Committed d1a1cc5 and pushed to 8.6.x. Thanks!