Migration path to entity reference field from 7.x

Node and User reference are handled in https://www.drupal.org/node/2447727.

Attached is a patch that migrates Drupal 7 entityreference fields to Drupal 8.
I propose to tackle the upgrade path for the D7 References module (node reference & user reference) in the issue KarenS mentioned: #2447727: Add base class for migrating reference fields.
It seems more appropriate to migrate the D6 & D7 version of that module over there, rather than split it up.

The attached patch migrates entityreference fields & field instances, field widgets & formatter settings.
The FieldInstance source plugin had to be adjusted, because the handler settings (which bundles to target + sort settings) are stored in the field instance config in D8, but in D7 this is stored in the field config itself, not the instance settings.
So I fetch the field config data as well now, to be able to migrate those settings properly.

This still needs tests written.

Fixed failing unit test + added update to drupal7 database fixture, to allow migration tests to be written.

Update patch with tests added. Ready for review.

Updated patch with small code improvement suggested by chx:

-    if ($row->getSource()['type'] == "entityreference") {
+    if ($row->getSourceProperty('type') == "entityreference") {

Also good point raised by chx: how will contrib modules transform field instance settings if they need to?
Now I added this functionality to the d7_field_instance_settings Migrate process plugin, since entityreference is in D8 core, but other contrib modules might have to add some transformation code here as well.
This seems outside of the scope of this issue, but worth thinking about how this can be solved.

We should implement this as a cckfield plugin, the same as #2447727: Add base class for migrating reference fields

So cckfield plugins also apply to Drupal7? Not the best name then, but i'll look into it...

Yeah, we could rename them to "field" plugins but that has the same problem the other way, that could be discussed in a follow-up.

I was wrong when I earlier said that node and user references were being handled on that other issue, only D6 node and user references are handled there. So currently there is no upgrade path for D7 node and user references. It would be nice to go back to fixing those in this patch as well.

The part of this patch that adds field settings to the field instance is going to be important for other fields as well. It's not unusual for settings to move from field to instance when you go to a new version so they should always be made available just in case. That little bit of the patch has uses outside this issue. It would be nice to get that in if the bigger part of the patch is going to be held up.

For what's it worth: I used patch from #9 successfully on a few sites with relatively easy entity references migrating from Drupal 7.42 to Drupal 8.0.5.

I used the 7.x-2.x-dev branch of the Reference to EntityReference Field Migration module to change the node and user references to entity references first.

Is there anything I need to do after applying the patch? I tried to use it with Drupal Upgrade, but it doesn't seem to migrate entity references.

I just applied #9 to 8.1.0-RC1 and it did not apply cleanly.

Firstly, this is because of the absence of the Migrate tests that already exist in 8.0.6. Basically, anything in and under: a/core/modules/field/src/Tests/Migrate/

Second, after applying the text, I get the following error with each instance.
array_filter() expects parameter 1 to be array, null given FieldInstanceSettings.php:36

This code is added as part of the patch in #9 on line 71

EDIT: it is also worth mentioning that these are defined as warnings and not errors during a drush migrate-upgrade

I went and rerolled this patch for 8.1.x. The Tests simply moved location from

core/modules/field/src/Tests/Migrate/d7/

to

core/modules/field/tests/src/Kernel/Migrate/d7/

So, I see that we are inserting three new field_config and field_config_instance in the test but the entity counts were not updated. Doing that.

Also, there would be a conflict with #2763637: D7 taxonomy term fields are not migrated with allowed vocabularies for which I "borrowed" some code from here. That is much simpler though. If it gets in first, this would need a reroll.

It seems we have a lot of issues where we could reduce common code in patches if we plan common issues to handle the basic foundation first (like making field settings available to instance migration in this case). It is difficult to determine that, however, but we could try.

upgrade_path_to_entity-2611066-24.patch worked for me on 8.1.7
Entity reference fields successfully migrated.
User reference fields successfully migrated.

Rerolling and I updated the counts again. Setting to RTBC as per #26.

Fixing the failure.

upgrade_path_to_entity-2611066-28.patch worked for me.

Rerolled, and resetting to RTBC as per #31.

Targeting 8.2.x.

Does this patch pass the test using 8.2.x?

Moving back to 8.1.x since migration additions are still fine to commit there.

Tested #33 on latest dev version, works like a charm!

I found a few nits but I think that this looks good.

1. +++ b/core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
   @@ -17,9 +17,37 @@ class FieldInstanceSettings extends ProcessPluginBase {
   +      if (!empty(array_filter($field_settings['handler_settings']['sort']))) {
   

   Can we put the handler settings into a variable called $handler_settings, just for easier readability?

2. +++ b/core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
   @@ -17,9 +17,37 @@ class FieldInstanceSettings extends ProcessPluginBase {
   +        } else if ($field_settings['handler_settings']['sort']['type'] == "field") {
   

   Should be elseif.

3. +++ b/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
   @@ -108,6 +111,15 @@ public function testFields() {
   +    $this->assertIdentical('node', $field->getSetting('target_type'));
   

   We should be using assertSame() for PHPUnit kernel tests, not assertIdentical().

I think the failure is not related, so setting back to RTBC. I am just addressing the minor change in #38.2.

Can we put the handler settings into a variable called $handler_settings, just for easier readability?

We need to have an if check to see if the key is set, which kind of makes it harder to understand, even if the later code is slightly easier to read. I thought about different ways to resolve this but nothing stood out as simple and easy.

We should be using assertSame() for PHPUnit kernel tests, not assertIdentical().

There are other assertIdentical() in the same test case which makes using assertSame() slightly confusing. I think we should address that separately.

Okay, fair enough. +1 RTBC. Thank you, @hussainweb!

Random failure. Setting back to RTBC.

Patch #40 worked for me. Thanks!

Hello,

#40 worked for me too, in 8.1.8
however, I cant see migrated entity reference fields in
admin/config/regional/content-language.

So I cant setup that imported field for translation.
Content seems to be OK.

BTW I cant translate even body field for newly created content type, but that's something else I guess.
I reported that here: https://www.drupal.org/node/2794709

Is it bug because of patch ?
Thanks a lot for help!

Drup 8.1.8
Migrate Plus 8.x-2.0-beta2+8-dev (2016-Aug-24)
Migrate Tools 8.x-2.0-beta1+3-dev (2016-Aug-04)
Migrate Upgrade 8.x-2.0-beta1+9-dev (2016-Sep-01)
Entity API 8.x-1.0-alpha3+2-dev (2016-Jun-23)

It is just a reroll and I could RTBC it again as per the last comment, but it is quite a long time since then.

Really weird change to the patch. I wonder how it ever got applied.

OK, I will test patch 51 tomorrow evening (in 20hrs)

Hello, thanks a lot for update!
this migration test, time I avoided installation of pathauto..."entity mismatch bug", but there is a new version :) OT..

When patching 8.1.8 with 51, i got error lines. 40 was fine.

patch -p1 < upgrade_path_to_entity-2611066-51.patch
patching file core/modules/field/migration_templates/d7_field.yml
patching file core/modules/field/migration_templates/d7_field_formatter_settings.yml
patching file core/modules/field/migration_templates/d7_field_instance.yml
patching file core/modules/field/migration_templates/d7_field_instance_widget_settings.yml
patching file core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
patching file core/modules/field/src/Plugin/migrate/source/d7/FieldInstance.php
patching file core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldFormatterSettingsTest.php
patching file core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceTest.php
patching file core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceWidgetSettingsTest.php
patching file core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
patching file core/modules/field/tests/src/Unit/Plugin/migrate/process/d7/FieldInstanceSettingsTest.php
patching file core/modules/migrate_drupal/tests/fixtures/drupal7.php
Hunk #3 succeeded at 5074 (offset -24 lines).
Hunk #4 FAILED at 5374.
Hunk #5 succeeded at 5353 (offset -34 lines).
Hunk #6 succeeded at 7136 with fuzz 2 (offset 1739 lines).
Hunk #7 FAILED at 5422.
Hunk #8 succeeded at 7168 (offset 1729 lines).
Hunk #9 FAILED at 5466.
Hunk #10 succeeded at 7311 (offset 1731 lines).
Hunk #11 FAILED at 5590.
Hunk #12 FAILED at 5678.
Hunk #13 FAILED at 7039.
Hunk #14 FAILED at 7210.
Hunk #15 FAILED at 7223.
Hunk #16 FAILED at 7233.
Hunk #17 FAILED at 7258.
Hunk #18 succeeded at 7370 (offset -108 lines).
Hunk #19 succeeded at 7504 (offset -108 lines).
Hunk #20 succeeded at 7514 (offset -108 lines).
Hunk #21 succeeded at 28169 (offset -108 lines).
Hunk #22 succeeded at 28369 (offset -108 lines).
Hunk #23 succeeded at 31491 (offset -156 lines).
Hunk #24 succeeded at 32500 (offset -156 lines).
Hunk #25 succeeded at 32584 (offset -156 lines).
Hunk #26 succeeded at 32710 (offset -156 lines).
Hunk #27 succeeded at 32780 (offset -156 lines).
Hunk #28 succeeded at 33053 (offset -156 lines).
Hunk #29 succeeded at 34411 (offset -156 lines).
Hunk #30 succeeded at 37772 (offset -156 lines).
Hunk #31 succeeded at 37884 (offset -156 lines).
Hunk #32 succeeded at 41124 (offset -156 lines).
Hunk #33 succeeded at 41146 (offset -156 lines).
Hunk #34 succeeded at 41388 (offset -156 lines).
Hunk #35 succeeded at 41628 with fuzz 2 (offset -174 lines).
Hunk #36 succeeded at 42795 (offset -174 lines).
10 out of 36 hunks FAILED -- saving rejects to file core/modules/migrate_drupal/tests/fixtures/drupal7.php.rej
patching file core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php

I tried anyway.
but no change i think, cant set newly imported entity field for translation, and when transalting, even for body, edit form says "All languages".
same issue in field translation setting, with CS and UND.
And again, all entity field are migrated thanks to patch (but cant do the settings properly, nor translate content :/)

Let me know if you need more info.
Thanks a lot for helping.

@whop, try patching it to 8.1.x-dev. I guess the core has moved on since the patch in #40. :)

Just tried dev, patching #51 is ok on 8.1.dev +56,
but still same issues, no change found.
Thanks !

Passes the 8.2.x testbot, and I tested manually on 8.2.x locally in the context of checking out #2801427: D7 entity references don't migrate straight-forwardly, the field is properly migrated (e.g., the specific 3 referenced node types from the D7 side were configured as the targets on the D8 side, and I got the field data to migrate). Let's go!

Seems #51 needs a reroll against 8.2.x because it fails to apply against 8.2.0

Ok did a reroll. There were a bunch of fuzz matches... but it was only one of the tests that were failing. I'm not sure if I updated the tests correctly, but I think I got it. Didn't run them... letting the bot do that for me :)

well technically speaking... if I would have hit save on my text-editor... this would be more correct

Patch #51 was broken by commit:

commit 9c705b8cc7ac4e393d0183e51871d3b479276621
Author: Alex Pott
Date: Sun Sep 25 12:01:18 2016 +0100

Issue #2500533 by quietone, phenaproxima: Upgrade path for System 7.x

(cherry picked from commit ffde2454699085d0a41bdb5091bf380c95cf3f35)

I was checking if patch still applies and had some offset. Updated patch.

$ curl https://www.drupal.org/files/issues/upgrade_path_to_entity-2611066-60.patch | patch -p1
patching file core/modules/field/migration_templates/d7_field.yml
patching file core/modules/field/migration_templates/d7_field_formatter_settings.yml
patching file core/modules/field/migration_templates/d7_field_instance.yml
patching file core/modules/field/migration_templates/d7_field_instance_widget_settings.yml
patching file core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
patching file core/modules/field/src/Plugin/migrate/source/d7/FieldInstance.php
patching file core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldFormatterSettingsTest.php
patching file core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceTest.php
patching file core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceWidgetSettingsTest.php
patching file core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
patching file core/modules/field/tests/src/Unit/Plugin/migrate/process/d7/FieldInstanceSettingsTest.php
patching file core/modules/migrate_drupal/tests/fixtures/drupal7.php
Hunk #36 succeeded at 43531 (offset 26 lines).
patching file core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php

Rerolled patch in #64 against 8.2.x, since it had a few errors (error: while searching for:) and offsets.

The D7 fixtures had a few duplicate ids in field_config_instance. Let's see if this helps.

Updated 'field_config' in getEntityCounts() (/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php) to 48 to fix the fail in #70 (hope that's ok).

I'm reviewing this with Bruno right now. Initially the changes are mostly identifiers being incremented. I diffed #51 vs #72.

Changing the issue summary to remove 'node reference' because that is not covered in this patch.

Ok with this patch I did the following to test:

1. Create a blank D7 site
2. Installed a entityreference module
3. Created a slide node type with an image field
4. Added an entityreference field on basic page type for unlimited slides as field_slides
5. Used devel_generate to create 50 nodes which generated some images and entityreferences hookups (real time saver)
6. Build the migrations on D8 with some rough guidence from this blog post: https://drupalize.me/blog/201605/custom-drupal-drupal-migrations-migrate...
7. Ran the migration and checked that an sample Basic page had the entity reference to the slides!

So essentially another big +1 and re-RTBC

Tested with the patch applied, the same way as @joelpittet in #74 (i.e., using the drupalize.me tutorial) on a copy of a real D7 site that has several content types with entityreference fields: entityreference field configuration and data were migrated properly.

Was this issue created to also support Drupal 7 References (https://www.drupal.org/project/references) migration to Drupal 8 Entity Reference?

References was uses a lot before Entity reference existed.

Furthermore, the table structure for References is similar to Entity reference.

@xmacinfo I believe it didn't. But there is the Reference to EntityReference Field Migration module, that you can use to 'upgrade' your node and user references into entity references (That link came from the References module project page).

@xmacinfo, this patch is 100K, if you read up there are other issues and modules tackling the Node References. This issue is scoped to tackle entityreference.

Random testbot fail

Two unrelated fails.

3, 3 unrelated fails, ha ha ha --Count von Count

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!