Problem/Motivation

This is a follow up to #3076447: Migrate D7 entity translation revision translations to add a comment field to the 'et' content type that was added in the original issue. 

+++ b/core/modules/migrate_drupal/tests/src/Kernel/d7/FieldDiscoveryTest.php
@@ -75,6 +75,7 @@ public function setUp(): void {
+      'et' => 'comment_node_et',

The necessary corresponding field_config_instance table row was not created. This means the current drupal7 fixture is wrong/broken. 😬

Proposed resolution

Add it.
Update tests d7/MigrateCommentField and d7/MigrateCommentFieldInstance

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3153791-5.patch2.96 KBquietone
#2 3153791-2.patch1.68 KBwim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 3153791-2.patch1.68 KB
quietone’s picture

quietone commented
Status: Needs review » Needs work

Nice. How did you notice this?

Tests should be added to d7/MigrateCommentFieldTest and d7/MigrateCommentFieldInstanceTest as well.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3097336: Improve the DX for migrating content entities: view modes, fields, formatters, widgets etc should be migrated per entity type + bundle

Nice. How did you notice this?

Thanks to #3097336: Improve the DX for migrating content entities: view modes, fields, formatters, widgets etc should be migrated per entity type + bundle, which contains:

     if (array_values($node_types_sorted) !== array_values($node_comment_fields_sorted)) {
       throw new \LogicException(sprintf("Source database '%s' seems to be corrupted: node comment field and node type structure do not match each other.", $db->getConnectionOptions()['database']));
     }
quietone’s picture

quietone commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 3153791-5.patch2.96 KB

This adds assertions to MigrateCommentFieldTest and MigrateCommentFieldInstanceTest. No interdiff because the change it two lines in an already small patch.

quietone’s picture

quietone commented
Title: Follow-up for #3076447: `comment_node_et` field_config_instance row is missing in drupal7 DB fixture » Add comment field for 'et' content type to d7 fixture
Issue summary: View changes

Title was a bit unwieldy, shortened to the action being taken here.

quietone’s picture

quietone commented

And after all that I forgot the most important thing. Thank you, @Wim Leers for finding the problem and posting a patch!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

🥳🙏

  • catch committed 065f481 on 9.1.x 
    Issue #3153791 by Wim Leers, quietone: Add comment field for 'et'...

  • catch committed bbef53f on 9.0.x 
    Issue #3153791 by Wim Leers, quietone: Add comment field for 'et'...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.0.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.1.x and cherry-picked back to 8.9.x, thanks!

  • catch committed e4db69f on 8.9.x 
    Issue #3153791 by Wim Leers, quietone: Add comment field for 'et'...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone’s picture

quietone commented

Unfortunately, this introduced a 'bug' into the Drupal7 fixture.

+++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php
@@ -5282,6 +5282,15 @@
+  'field_id' => '54',

There is no field_id '54' and this causes failures when using the fixture via the UI. The field_id should be '1' as used by other comment fields.