Problem/Motivation

The LoadEntity dynamically creates migrations for per bundle migrations when migrating CCK fields. These migrations don't fire the same hooks that are fired when loading a normal migration.

This makes it impossible for a contrib or a custom field module to migrate from D6 to D8. This is serious enough to make the issue major.

Proposed resolution

Manually call the load hooks from LoadEntity.

Remaining tasks

Review

User interface changes

n/a

API changes

Hooks added inline with other entities.

CommentFileSizeAuthor
#21 2422221-21.patch4.26 KBchx
#21 interdiff.txt547 byteschx
#20 2422221-20.patch4.26 KBbenjy
#19 2422221-19.patch1.04 KBbenjy
#18 interdiff.txt4.28 KBbenjy
#18 2422221-18.patch5.96 KBbenjy
#12 interdiff.txt1.29 KBbenjy
#12 2422221-12.patch9.88 KBbenjy
#8 interdiff.txt570 bytesbenjy
#8 2422221-8.patch9.99 KBbenjy
#5 2422221-5.patch9.99 KBbenjy
#5 interdiff.txt6.42 KBbenjy
#5 2422221-5-FAIL.patch10.05 KBbenjy
#4 interdiff.txt1.25 KBbenjy
#4 2422221-4.patch5.53 KBbenjy
load-entity.patch4.27 KBbenjy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

chx CreditAttribution: chx commented
Priority: Normal » Major
Status: Needs review » Needs work
Issue tags: +Needs tests
chx’s picture

chx CreditAttribution: chx commented
Issue summary: View changes

The last submitted patch, load-entity.patch, failed testing.

benjy’s picture

benjy CreditAttribution: benjy commented
Status: Needs work » Needs review
FileSize
2422221-4.patch5.53 KB
interdiff.txt1.25 KB

This will fix existing fails. New tests coming later.

benjy’s picture

benjy CreditAttribution: benjy commented
FileSize
2422221-5-FAIL.patch10.05 KB
interdiff.txt6.42 KB
2422221-5.patch9.99 KB

Unit tests and a fail test.

The last submitted patch, 5: 2422221-5-FAIL.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: 2422221-5.patch, failed testing.

benjy’s picture

benjy CreditAttribution: benjy commented
Status: Needs work » Needs review
FileSize
2422221-8.patch9.99 KB
interdiff.txt570 bytes

Fixed namespace.

Status: Needs review » Needs work

The last submitted patch, 8: 2422221-8.patch, failed testing.

benjy queued 8: 2422221-8.patch for re-testing.

benjy’s picture

benjy CreditAttribution: benjy commented
Status: Needs work » Needs review
benjy’s picture

benjy CreditAttribution: benjy commented
FileSize
2422221-12.patch9.88 KB
interdiff.txt1.29 KB

Fixed a few things from chx's feedback on IRC.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Really nice. I was afraid the test will need a lot more mocking, glad it didn't.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 12: 2422221-12.patch, failed testing.

benjy queued 12: 2422221-12.patch for re-testing.

benjy’s picture

benjy CreditAttribution: benjy commented
Status: Needs work » Reviewed & tested by the community

That is a random fail in HEAD.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/migrate_drupal/src/Plugin/migrate/load/LoadEntity.php
@@ -114,7 +149,7 @@ public function loadMultiple(EntityStorageInterface $storage, array $sub_ids = N
+    $this->postLoad($migrations);

+++ b/core/modules/migrate_drupal/src/Plugin/migrate/load/d6/LoadTermNode.php
@@ -13,6 +13,7 @@
@@ -22,9 +23,9 @@ class LoadTermNode extends LoadEntity {

Doesn't LoadTermNode::loadMultiple() also need to call the postLoad method?

Also something is fishy about doing

$migration = $storage->create($values);

And then firing post load hooks.

benjy’s picture

benjy CreditAttribution: benjy commented
Status: Needs work » Needs review
FileSize
2422221-18.patch5.96 KB
interdiff.txt4.28 KB

Nice catch. As discussed, moved $this->postLoad into MigrationStorage.

benjy’s picture

benjy CreditAttribution: benjy commented
FileSize
2422221-19.patch1.04 KB

Not much left :)

benjy’s picture

benjy CreditAttribution: benjy commented
FileSize
2422221-20.patch4.26 KB

New unit tests.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community
FileSize
interdiff.txt547 bytes
2422221-21.patch4.26 KB

Fantastic, so much better. There's a typo in the patch which I fixed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 80d54fc and pushed to 8.0.x. Thanks!

  • alexpott committed 80d54fc on 8.0.x 
    Issue #2422221 by benjy, chx: LoadEntity needs to fire load hooks for...

Status: Fixed » Closed (fixed)

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