Problem/Motivation

Migrate the revisions for d7 entity translation to d8. This is moved out of #2746541: Migrate D6 and D7 node revision translations to D8, which is now only about node translation, to reduce complexity.

This issue is intended to be completed after #2746541: Migrate D6 and D7 node revision translations to D8.

Proposed resolution

Add a new node type, et, to the drupal7 fixture that uses entity translation and has several revisions.

Use the node master plugins that are introduced in #2746541: Migrate D6 and D7 node revision translations to D8 and add a join to the entity_translation_revision table, if the table exists. Then the language must be selected accordingly. If the row uses node translation then the language is set to what is in the node table and if the row used entity_translation then the language is set to what is in the entity translation revision table.

Also, a tweak is done in the node source plugin to select the language.

Remaining tasks

The patch in #28 is merged with2746541-231.patch from the node translation revision issue so that it is available for testing.

For Testers

  • Use the lastest patch patch in this issue..
  • There are instructions in the Issue summary of the META issue #2208401: [META] Stabilise Migrate Drupal Multilingual module/li>

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#97 3076447-97.patch671 bytesdaffie
#90 3076447-90-9.0.patch45.51 KBquietone
#90 3076447-90-8.9.patch45.51 KBquietone
#90 diff-82-80-9.0.txt636 bytesquietone
#90 diff-82-80-8.9.txt1.53 KBquietone
#82 3076447-82-9.1.patch45.51 KBquietone
#80 3076447-80.patch53.24 KBshaktik
#77 Patch-fail-on-d9.1.png142.77 KBshaktik
#72 interdiff-70-72.txt2.44 KBquietone
#72 3076447-72.patch45.53 KBquietone
#70 interdiff-67-70.txt2.15 KBjungle
#70 3076447-70.patch44.6 KBjungle
#67 diff-64-67.txt213.7 KBquietone
#67 3076447-67.patch45.09 KBquietone
#64 interdiff-62-64.txt1.07 KBquietone
#64 3076447-64.patch219.81 KBquietone
#62 3076447-62.patch219.47 KBquietone
#62 interdiff-60-61.txt627 bytesquietone
#60 3076447-60.patch219.62 KBquietone
#60 interdiff-58-60.txt654 bytesquietone
#58 interidf-55-58.txt2.66 KBquietone
#58 3076447-58.patch219.55 KBquietone
#55 interdiff-52-55.txt2.67 KBquietone
#55 3076447-55.patch219.11 KBquietone
#54 3076447-54.patch219.11 KBquietone
#54 interdiff-52-54.txt2.67 KBquietone
#52 interidf-50-52.txt1.01 MBquietone
#52 3076447-52.patch217.54 KBquietone
#50 interdiff-46-50.txt2.87 KBquietone
#50 3076447-50.patch1.07 MBquietone
#48 interdiff-46-38.txt77.5 KBquietone
#48 3076447-48.patch1.16 MBquietone
#46 3076447-46.patch1.07 MBquietone
#46 interdiff-45-46.txt16.03 KBquietone
#45 3076447-45.patch1.07 MBquietone
#45 interdiff-43-45.txt901 bytesquietone
#43 3076447-43.patch1.07 MBquietone
#43 diff-40-43.txt45.67 KBquietone
#42 interdiff-40-42.txt42.04 KBquietone
#42 3076447-42.patch1.07 MBquietone
#40 3076447-40.patch1.07 MBquietone
#40 interdiff-28-40.txt163.81 KBquietone
#38 3076447-38.patch1.07 MBquietone
#37 3076447-37.patch1.12 MBquietone
#28 interdiff-25-28.txt3.46 KBquietone
#28 3076447-28.patch1.08 MBquietone
#25 3076447-25-review-only.txt159.66 KBquietone
#25 3076447-25.patch1.08 MBquietone
#24 3076447-24.patch1.06 MBquietone
#24 interdiff-22-24.txt813 bytesquietone
#22 3076447-22.patch1.06 MBquietone
#22 interdiff-20-22.txt6.38 KBquietone
#20 3076447-20-test.patch1.06 MBquietone
#20 interdiff-18-20.txt6.27 KBquietone
#18 3076447-18-test.patch1.06 MBquietone
#16 3076447-16.patch921.28 KBquietone
#16 3076447-16-review-only.txt40.41 KBquietone
#13 3076447-13.patch922.09 KBquietone
#13 3076447-13-review-only.txt41.22 KBquietone
#11 3076447-11.patch1.11 MBquietone
#10 interdiff-7-10.txt18.17 KBquietone
#10 3076447-10.patch914.58 KBquietone
#7 3076447-7.patch914.52 KBquietone
#4 3076447-4.patch723.77 KBquietone
#2 3076447-2.patch1.1 MBquietone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

Issue tags: +i18n-migrate. migrate-d7-d8
FileSize
1.1 MB

Added an english and icelandic revision.

quietone’s picture

Issue tags: -i18n-migrate. migrate-d7-d8 +i18n-migrate, +migrate-d7-d8

Correct the tags

quietone’s picture

Ignore previous patch. How about this one which adds a new content type which uses entity translation and has some revisions.

heddn’s picture

Status: Active » Needs work

If we update the IS to discuss the specific plan here, I think we could land this as a fixture only patch and do follow-up(s) for the actual migration.

quietone’s picture

Good idea but I've noticed something since then about this patch. When using the test fixture and adding entity translation revisions the db table entity_translation_revision is not populated. But if I do the same with a fresh d7 site, the entity_translation_revision table is populated. That is a bit of a worry to me.

quietone’s picture

Status: Needs work » Needs review
FileSize
914.52 KB

Well, here is yet another approach. This uses a new fixture just for entity translation as that is the only way I could get have an 'entity_translation_revision' table. Hopefully I've made the correct changed to drupaci.yml (as was done in #2746541: Migrate D6 and D7 node revision translations to D8 by mikelutz so this just runs the new tests.

I am having a problem with the dates though and the test will fail.

Status: Needs review » Needs work

The last submitted patch, 7: 3076447-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Those are the errors I expected. And I see that there are coding standard errors, due to comment out code in D7NodeDeriver, that needs to be fixed.

The method used here is based on the node master approach used in #2746541: Migrate D6 and D7 node revision translations to D8, the only difference is that the source plugin uses the entity_translation_revision table. I did try locally once without the node master approach and it didn't work, having the same sort of problems encountered in the node translation revision issue.

Still to do is to sort out the revision tabs, migrated changed dates and then to get this approach working with the whole system.

quietone’s picture

Status: Needs work » Needs review
FileSize
914.58 KB
18.17 KB

Removed test code. Removed translation tag from the migration. Change the migration to use the dates from the entity_translation_revision table and added a process plugin in an attempt to get the revision translation affected flag correct. Need to review again.

quietone’s picture

This is a different approach. This uses the NodeMaster source plugin and destination that are used in #2746541: Migrate D6 and D7 node revision translations to D8. The changes here are to the source plugin, a test fixture that uses entity translation and node translation and of course the migration test itself. The test is failing because of some changed dated fields are being set to the current time and the field value for the field, field_tree, is wrong. The correct field data is migrated so I guess I've mucked up the test.

edit:

local testing with drush shows that the revision tabs look correct. Of course, that needs manual testing by someone else as well.

quietone’s picture

Wow that is a bad patch. Need to redo it.

quietone’s picture

Let's try that again. And now with a review patch that excludes the test fixture.

Status: Needs review » Needs work

The last submitted patch, 13: 3076447-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Issue summary: View changes
quietone’s picture

Status: Needs work » Needs review
FileSize
40.41 KB
921.28 KB

Some cleanup and adding comments.
the difference between this is and #2746541: Migrate D6 and D7 node revision translations to D8 is that there are no d6 files here, or course, the addition of querying the entity_translation_revision table in the node_master source plugin, a process plugin to help determine the revision_translation_affected property and the new source fixture.

Status: Needs review » Needs work

The last submitted patch, 16: 3076447-16.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
1.06 MB

Rebuilding this ontop of the latest patch in the node translation issue. Created MigrateNodeMasterETTest just to test with the drupal7 fixture that has entity translation revision. Also, changing it to run all the test so I expect there will be many.

Status: Needs review » Needs work

The last submitted patch, 18: 3076447-18-test.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
6.27 KB
1.06 MB

Fix the ordering in the query. Still just a test patch.

Status: Needs review » Needs work

The last submitted patch, 20: 3076447-20-test.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
6.38 KB
1.06 MB

Add a test in prepareRow to drop rows that are not the one changed in this revision. The test assumes that the row with the latest changed timestamp in the entity_translation_revision table is the one that has been edited. And it is only this row that is migrated.

I'd like to think that this can be done in the query() method but I wasn't successful in making a subquery in Drupal. Maybe someone else can have a look.

What is not working is the migration of the translated field on the entity.

Status: Needs review » Needs work

The last submitted patch, 22: 3076447-22.patch, failed testing. View results

quietone’s picture

Needs a reroll.

quietone’s picture

Status: Needs work » Needs review
FileSize
1.08 MB
159.66 KB

Now merge in 2746541-231.patch from the node translation revision issue.

The last submitted patch, 24: 3076447-24.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 25: 3076447-25.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Status: Needs work » Needs review
FileSize
1.08 MB
3.46 KB

Created an entity_translation_revision table for the test fixture drupal7.php and that changed the migrated times. Also, made a todo for looking to the migration of the a field.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Version: 8.9.x-dev » 8.8.x-dev
Issue summary: View changes
quietone’s picture

Issue summary: View changes
quietone’s picture

Issue summary: View changes
Wim Leers’s picture

I'm assuming this follows the same general approach as #2746541: Migrate D6 and D7 node revision translations to D8. Meaning: if #2746541 is considered ready, then this patch is likely to be able to be updated to match the same approach as #2746541.

If that is not correct, it'd be really helpful if it could be pointed out in what ways this (>1 MB!) patch deviates significantly from #2746541. That'd allow for a more targeted review.

quietone’s picture

@Wim Leers, The first paragraph in #33 is correct. This will use the same approach as in #2746541: Migrate D6 and D7 node revision translations to D8.

Wim Leers’s picture

Great, then I don't see a need to dig deep into this issue right now :)

quietone’s picture

Assigned: Unassigned » quietone
Status: Needs review » Needs work

I working on rerolling this now.

quietone’s picture

Status: Needs work » Needs review
FileSize
1.12 MB

This is a partial reroll and best if you just ignore it. The drupal7 test fixture here should be the same as the one in #2746541: Migrate D6 and D7 node revision translations to D8 which means it is missing changes needed here.

quietone’s picture

Another increment on this reroll. Working towards a patch that is base on the node revision translation one but has the additions to the complete node source plugin. This still does not have the changes to the drupal7 fixture.

I'm not convinced an interdiff will be of much use. The patch in #28 is based on an older version of the node revision translation patch and there is a lot that is no longer relevant.

quietone’s picture

Version: 8.8.x-dev » 8.9.x-dev

Twice I forgot to change the version.

quietone’s picture

Fixing the failing test and including a diff against the patch in #28 if anyone is interested. The key piece that is missing is the addition of the entity translation revision table in the d7 fixture. I've done that locally and tests are failing so still work to do.

Gábor Hojtsy’s picture

Adding tag.

quietone’s picture

quietone’s picture

Let me try that again.

Status: Needs review » Needs work

The last submitted patch, 43: 3076447-43.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Status: Needs work » Needs review
FileSize
901 bytes
1.07 MB

Adjust the entity counts.

quietone’s picture

Reexamined the entity translation test fixture and corrected MigrateNodeCompleteETTest.php accordingly.

Status: Needs review » Needs work

The last submitted patch, 46: 3076447-46.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Status: Needs work » Needs review
FileSize
1.16 MB
77.5 KB

Oops, somehow lost the changes to drupal7.php. This restores that and adjusts the times for node 1 in the test.

Status: Needs review » Needs work

The last submitted patch, 48: 3076447-48.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Status: Needs work » Needs review
FileSize
1.07 MB
2.87 KB

Messed up the drupal7 fixture again. Try again.

Status: Needs review » Needs work

The last submitted patch, 50: 3076447-50.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
217.54 KB
1.01 MB

This patch removes the test fixture, drupal7_et.php, which was a temporary tool for testing. The node with entity translation and it s field, field_tree, is now incorporated into the drupal7.php fixture.

This includes the patch from #380 from the node revision patch, #2746541: Migrate D6 and D7 node revision translations to D8.

Status: Needs review » Needs work

The last submitted patch, 52: 3076447-52.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Status: Needs work » Needs review
FileSize
2.67 KB
219.11 KB

Alter tests to take account of changes to the fixture.

quietone’s picture

Aborted that because I left a note to myself in the code.

Status: Needs review » Needs work

The last submitted patch, 55: 3076447-55.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Issue summary: View changes

Update the IS.

quietone’s picture

Status: Needs work » Needs review
FileSize
219.55 KB
2.66 KB

Fixing more tests.

Status: Needs review » Needs work

The last submitted patch, 58: 3076447-58.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
654 bytes
219.62 KB

Update entity count.

Status: Needs review » Needs work

The last submitted patch, 60: 3076447-60.patch, failed testing. View results

quietone’s picture

Assigned: quietone » Unassigned
Status: Needs work » Needs review
FileSize
627 bytes
219.47 KB

correct number of field_config entities.

Status: Needs review » Needs work

The last submitted patch, 62: 3076447-62.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
219.81 KB
1.07 KB

And some more entity counts/

quietone’s picture

Issue summary: View changes

This is ready for testing.

Update the node for testers in the IS.

xjm’s picture

Issue tags: +beta target
quietone’s picture

A reroll of #64 since #2746541: Migrate D6 and D7 node revision translations to D8 was committed. The diff is largely unhelpful because the patch from #64 included the work from the node revision issue and this reroll just removes all that.

heddn’s picture

I see nothing in here that is of concern. No BC concerns. Tests seem to cover things. Leaving in NR for other reviews, but if they don't surface in a few days, lets just get this merged.

jungle’s picture

2 coding standards messages, see https://www.drupal.org/pift-ci-job/1611884

line 149 Line indented incorrectly; expected 4 spaces, found 0
150 Line indented incorrectly; expected 4 spaces, found 0

jungle’s picture

Deleted as they two lines are comments, or both exceed 80 characters.

alexpott’s picture

Status: Needs review » Needs work
+++ b/core/modules/node/tests/src/Kernel/Migrate/d7/MigrateNodeCompleteTest.php
@@ -146,8 +146,6 @@ protected function assertRevision(array $revision, array $data) {
-//    $this->assertSame($data['created'], $actual->getRevisionCreationTime(), sprintf("Creation time '%s' does not match actual '%s' for revision '%d' langcode '%s'", $data['created'], $actual->getRevisionCreationTime(), $revision['vid'], $revision['langcode']));
-//    $this->assertSame($data['changed'], $actual->getChangedTime(), sprintf("Changed time '%s' does not match actual '%s' for revision '%d' langcode '%s'", $data['changed'], $actual->getChangedTime(), $revision['vid'], $revision['langcode']));

Were these commented out because we should remove them or because they are failing an a sign of a problem.

quietone’s picture

The commented out lines were something that needed to be fixed. And here is the fix whichi is just to test the revision creation time. And also improve a comment

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Wim Leers’s picture

What's left to be done here? A thorough review?

quietone’s picture

@Wim Leers, yes. A thorough review.

quietone’s picture

Title: Migrate D7 entity translation revision translations to D8 » Migrate D7 entity translation revision translations
shaktik’s picture

FileSize
142.77 KB

Patch Failed to Apply on D9.1

patch fail on d9.1

shaktik’s picture

Assigned: Unassigned » shaktik
jungle’s picture

Hi, @shaktik, Actually, the screenshot in #77 is unnecessary. The result of CI run in #72 indicates that obviously.

shaktik’s picture

Assigned: shaktik » Unassigned
FileSize
53.24 KB

Re-rolled patch for 9.1.x, Kindly review.

Status: Needs review » Needs work

The last submitted patch, 80: 3076447-80.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
45.51 KB

A 9.1.x version of the patch, I made this from the patch in #72.

mikelutz’s picture

Status: Needs review » Reviewed & tested by the community

Time to move this forward. I don't see any issues, this will improve on what we have. I'd like to see it committed to 9.1, which will give us time to find any hidden bugs.

jungle’s picture

+++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php
@@ -5100,6 +5273,15 @@
+  'data' => 'a:7:{s:5:"label";s:4:"tree";s:6:"widget";a:5:{s:6:"weight";s:2:"-3";s:4:"type";s:14:"text_textfield";s:6:"module";s:4:"text";s:6:"active";i:1;s:8:"settings";a:1:{s:4:"size";s:2:"60";}}s:8:"settings";a:3:{s:15:"text_processing";s:1:"0";s:18:"user_register_form";b:0;s:23:"entity_translation_sync";b:0;}s:7:"display";a:1:{s:7:"default";a:5:{s:5:"label";s:5:"above";s:4:"type";s:12:"text_default";s:8:"settings";a:0:{}s:6:"module";s:4:"text";s:6:"weight";i:1;}}s:8:"required";i:0;s:11:"description";s:0:"";s:13:"default_value";N;}',

@@ -7193,8 +7387,8 @@
+  'field_link_attributes' => 'a:1:{s:5:"title";s:0:"";}',

BTW. confirmed on my local that those serialized data won't make drupal7.php to be recognized as a binary file by git later on. see #3126906: MenuLinkContentTest.php is recognized as a binary file by git for more.

catch’s picture

+++ b/core/modules/node/src/Plugin/migrate/source/d7/NodeComplete.php
@@ -25,9 +26,50 @@ class NodeComplete extends NodeRevision {
+
+    // Get any entity translation revision data.
+    if ($this->getDatabase()->schema()
+      ->tableExists('entity_translation_revision')) {
+      $query->leftJoin('entity_translation_revision', 'etr', 'nr.nid = etr.entity_id AND nr.vid=etr.revision_id');
+      $query->fields('etr', [
+        'entity_type',
+        'entity_id',
+        'revision_id',
+        'source',
+        'translate',
+      ]);

I see this being done for the node_complete migration, but entity_translation in 7.x contrib is used for potentially any fieldable entity type such as taxonomy terms. So was more expecting this table to be added to the source query generically. Is there a way to do that or does the same change need to be made for every entity migration to support entity_translation?

quietone’s picture

You are right, this is specific to the entity translation revisions for node and node only.

This patch introduces the first use of the entity_translation_revision table. The existing entity translation migrations for comment, taxonomy and user all use the entity_translation table. Why not the revision table? Not being familiar with entity translation for those entities I just did some testing and it is sufficient since they are not revision-able. Unless, of course, my testing was inadequate.

d7_comment_entity_translation.yml
d7_taxonomy_term_entity_translation.yml
d7_user_entity_translation.yml

  • catch committed 40034e1 on 9.1.x
    Issue #3076447 by quietone, jungle, shaktik, heddn, alexpott: Migrate D7...
catch’s picture

Version: 9.1.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Thanks for pointing to those. I was expecting to see more shared logic between the different entity types, but this is consistent with what we've already done and fills a big gap in the multilingual migrations.

Committed #82 to 9.1.x

#72 no longer applies to 9.0.x (and the 9.1.x patch doesn't cherry-pick) so will need a re-roll. If possibly it would be great to get this in prior to the 9.0.x and 8.9.x release candidates (i.e. today).

quietone’s picture

Assigned: Unassigned » quietone
quietone’s picture

Status: Needs work » Needs review
FileSize
1.53 KB
636 bytes
45.51 KB
45.51 KB

Rerolled for 8.9 and 9.0. It is late and I am rather tired so I hope Upgrade7Test will pass, that was the only one with a conflict.

Oh, the diff for both is against the 9.1.x version from #82.

catch’s picture

Status: Needs review » Reviewed & tested by the community

Interdiffs look good and DrupalCI is happy, moving back to RTBC.

quietone’s picture

Assigned: quietone » Unassigned
Issue tags: -Needs reroll

@catch, thanks.

  • catch committed cb9bbe7 on 8.9.x
    Issue #3076447 by quietone, jungle, shaktik, catch, heddn, alexpott:...

  • catch committed 9ce3065 on 9.0.x
    Issue #3076447 by quietone, jungle, shaktik, catch, heddn, alexpott:...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for the re-rolls. Committed/pushed to 9.0.x and 8.9.x respectively, thanks!

I'm not opposed to backporting this to 8.8.x, except that we're gearing up to do the very last patch release there so there'll be no opportunity to release follow-up bugfixes should one surface. So it might be better to leave this in 8.9.x - doing that for now but re-open if you'd really like to see a backport.

catch’s picture

Category: Task » Bug report
Priority: Normal » Critical
Status: Fixed » Needs work
+++ b/core/modules/node/src/Plugin/migrate/source/d7/NodeComplete.php
@@ -25,9 +26,50 @@ class NodeComplete extends NodeRevision {
+      $query->addField('etr', 'created', 'etr_created');
+      $query->addField('etr', 'changed', 'etr_changed');
+
+      $query->orderBy('revision_id');
+      $query->orderBy('language');
+    }

This breaks on PostgreSQL. https://www.drupal.org/pift-ci-job/1696290 Should be a quick fix to add the table alias to the order by.

daffie’s picture

catch’s picture

#97 looks right.

  • xjm committed 2a17e73 on 9.1.x
    Issue #3076447 hotfix by daffie, catch
    

  • xjm committed 014a32a on 9.0.x
    Issue #3076447 hotfix by daffie, catch
    
    (cherry picked from commit...
xjm’s picture

Status: Needs review » Fixed

Committed the hotfix to 9.1.x, 9.0.x, and 8.9.x. Thanks!

  • xjm committed 89c8a8a on 8.9.x
    Issue #3076447 hotfix by daffie, catch
    
    (cherry picked from commit...
quietone’s picture

This serves as a good reminder to test the migration patches with PostgreSQL and SQLlite before RTBC.

@daffie, thanks for making the patch and to catch and xjm for getting this one in.

Wim Leers’s picture

Sorry for not doing a thorough review here like I did for node_complete. I was unable to find the time.

That being said, I know that all of the hardenings/improvements that were made to node_complete were ported to this patch too. So I very strongly agree with @mikelutz in #83 that this is a big step forward regardless.

Thanks so much for all your work on this issue and the node sibling issue, @quietone! 👏👏👏👏 Very impressive work!

Status: Fixed » Closed (fixed)

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

Wim Leers’s picture

+++ 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. 😬

EDIT: created follow-up issue with patch: #3153791-2: Add comment field for 'et' content type to d7 fixture.