Migrate Drupal 7 node/user reference fields

Node reference migration from D6 was implemented in https://www.drupal.org/node/2872660 and https://www.drupal.org/node/2814949. I took a look at how it was done and adapted it for D7 in the attached patch. It works for me, though I'm not sure if there are corner cases that will need to be addressed.

Neither the D6 implementation or my code require the patch from https://www.drupal.org/node/2447727.

I'm not interested in working on #2447727, but if you'd like to go ahead with the approach used in my patch, I would be happy to write some tests for it.

Add source module

Quick Question:

Do we need to apply these patches to drupal8 site before running any migrations?

I'm probably glossing over it but is there an 8.7.x patch for this?

Attached a new patch based on #4. Also added support for user_reference fields. Still testing the migration.

Note that https://git.drupalcode.org/project/references is used on 100K Drupal 7 sites (see https://www.drupal.org/project/usage/references), or about 1 in 7 (see https://www.drupal.org/project/usage/drupal). So increasing priority.

P.S.: Running into this too for wimleers.com!

Bumped priority of the blocker that @phenaproxima identified in #2: #2447727: Add base class for migrating reference fields.

Fixed user_reference by adding target_type to field settings and setting default filter type to _none. Some cleanup. Role filter settings are incomplete. Tested on a test migration with one node reference and one user reference. They were both migrated as expected.

Awesome, thanks @Nathaniel! I will test this on wimleers.com.

Just wanted to let y'all know I'm making the discovery of this issue for current users of the Drupal 7 references module easier, by adding this issue to their issue queue: #3096190: FYI: a migration path to Drupal 8 is being worked on in core. It links here :) Hopefully that will also lead to more people testing this patch.

Tagging for needing tests.

Missed a break statement. No change in functionality.

Updated migrate test to include node_reference and user_reference in testFieldProvidersExist.

Updated the migrate_drupal d7/FieldDiscoveryTest and drupal7 fixture. Kudos to anyone who works on those fixtures, what a horrendous task!

If this all goes through the docs will also need to be updated to include the references module:
https://www.drupal.org/docs/8/api/migrate-api/generating-database-fixtur...

Patch #22 no longer applies.

Rebased patch attached.

I was wondering if anyone would be able to provide some steps to attempt a migration for node reference fields with the patch on here in place?

I am running "drush migrate:import d7_field"

For every node reference field I get an error output of:
Attempt to create a field storage field_FIELDNAME with no type.

This applies with or without the patch in place. I'm using the patch at #22 as that appears to be the most recent which will roll to Drupal 8.8, being the version I'm testing migrations on.

Are there some other requirements / steps for this patch to work?

I'm currently using #20 before any tests were added. Seems to be working on 8.8.2. Remember to clear cache etc. I find myself restarting migrations often with a fresh install (if possible).

@Nathaniel, you were absolutely right. I started from scratch with a fresh install and the migration of node references worked. Thanks.

mmph. Should've read from the top instead of from the bottom. Didn't notice the POSTPONED aspect. But in the meantime, here are a couple of re-rolled patches. Same as @huzooka's patchs in #26 but re-rolled for latest 8.9.x. Although it looks to me like #2447727: Add base class for migrating reference fields may be covering both node and user reference anyways and this issue will probably get closed as duplicate.

I needed to have this feature for a migration to Drupal 8.8.5.
I reroll the patch for my needs so I attach it for those who need it.

Reroll for 9.0.0

Applied #33, fields are in the node but all are empty. No data at all. Drupal 8.9.1, #33 applies flawlessly.

#34: any migration messages?

Migration always throws a gazillion messages here, but i did not see any that i found useful or related to this.
I ended up moving the field tables by hand, exporting the sql of D7, replaced some stuff and imported it that way.

Migration always throws a gazillion messages here

That shouldn't happen! 😞 Even though they're often cryptic, they're almost always about problems.

but i did not see any that i found useful or related to this.

Hm, okay. Perhaps there was one for d7_field or d7_field_instance that related to this though! 🤔

#2447727: Add base class for migrating reference fields is done. Changing to Needs work because there is a patch that probably needs a reroll. I added Needs reroll tag.

The latest patch has both node and user reference changes. For now, I am keeping them together.

For now, I am keeping them together.

+1

Rerolled. I still need to bring up the new fixture in a real D7 site and I haven't reviewed the patch yet myself because I am keen to see what errors are found.

Nice results, I expected the functional tests to fail.

Now, add some fixes for the functional tests. Also, add migration state information and use the new ReferenceBase.

+++ b/core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
@@ -65,6 +65,50 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
+            // @todo get new role id.
+            // $instance_settings['handler_settings']['filter']['role'][$type] = $value;

Ah, there is some work here to do.

And the d6 node and user reference field plugins are in migrate_drupal, the d7 added in this patch are in field. Where should they be?

1. +++ b/core/modules/field/migrations/state/field.migrate_drupal.yml
   @@ -10,4 +10,9 @@ finished:
   +    node_reference: core
   ...
   +    user_reference: core
   

   If we added 2 migrations, then the failing test in migrate_drupal_ui is a false positive and can be bumped from 76 to 78, right?

2. +++ b/core/modules/field/src/Plugin/migrate/field/d7/NodeReference.php
   @@ -0,0 +1,57 @@
   +use Drupal\migrate\Plugin\MigrationInterface;
   
   +++ b/core/modules/field/src/Plugin/migrate/field/d7/UserReference.php
   @@ -0,0 +1,57 @@
   +use Drupal\migrate\Plugin\MigrationInterface;
   

   Unused use statements.

3. +++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php
   index b897e54209..46c60b4249 100644
   --- a/core/modules/migrate_drupal/tests/src/Kernel/d7/FieldDiscoveryTest.php
   
   --- a/core/modules/migrate_drupal/tests/src/Kernel/d7/FieldDiscoveryTest.php
   +++ b/core/modules/migrate_drupal/tests/src/Kernel/d7/FieldDiscoveryTest.php
   

   I think what is happening with the test failure in migrate_drupal is that the test data is too big on a failed assertion, and PHP Unit tries to serialize the trace when there is an object that is too large (the drupal container).

   There's an assertion failure in testAddAllFieldProcessesAlters with one (or more) of the new data sets, but not sure which one it is.

45.1. This is a count of the missing or available modules on the Review page. I missed adding node and user reference in that test.
45.2. Fixed.
45.3. Yes, that is a really unhelpful message. Test fixed.

Good passing tests, let's keep it that way.

Still need to do the @todo in the patch, See #43.

Sort of a bad patch, I commented out part of the dataprovider and left that in the patch.

Fix the test that I messed up again in #48.

But mostly addressing the @todo to get the field settings for the user_reference field.

Nice work. There's a small nit in this review to fix that I missed previously so shifting to needs work. The rest are questions to answer.

1. +++ b/core/modules/field/src/Plugin/migrate/field/d7/NodeReference.php
   @@ -0,0 +1,56 @@
   +namespace Drupal\field\Plugin\migrate\field\d7;
   
   +++ b/core/modules/field/src/Plugin/migrate/field/d7/UserReference.php
   @@ -0,0 +1,56 @@
   +namespace Drupal\field\Plugin\migrate\field\d7;
   

   Following-up on the question of what module to put these in. If we're considering removing/deprecating migrate_drupal, would it make it easier to remove if these were in migrate_drupal rather than a non-migrate module? I think for consistency with d6 moving these to migrate_drupal makes sense. The d6 ones haven't hit a release yet so I guess there's time to move those instead.

2. +++ b/core/modules/field/src/Plugin/migrate/field/d7/UserReference.php
   @@ -0,0 +1,56 @@
   +class UserReference extends ReferenceBase {
   

   @todo override defineValueProcessPipeline similar to the d6 plugin to do a migration lookup.

3. +++ b/core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
   @@ -65,6 +65,55 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
   +    if ($row->getSourceProperty('type') == 'node_reference') {
   ...
   +    if ($row->getSourceProperty('type') == 'user_reference') {
   

   The other similar comparisons in this file use === except for the preexisting line 39 containing == "entityreference" (double quotes).

#51.1 Yes, lets keep the node/user references together in migrate drupal. Moved field plugins and migration state data.
#51.2 yes, was planning to that in this round. done now.
#51.3 Of the 3 existing tests only 1 uses '==='. So, no change here. Although I can be convinced to change it.

$grep -r "('type')" core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php 
    if ($row->getSourceProperty('type') == 'taxonomy_term_reference') {
    if ($row->getSourceProperty('type') == "entityreference") {
    if ($row->getSourceProperty('type') == 'node_reference') {
    if ($row->getSourceProperty('type') == 'user_reference') {
    if ($row->getSourceProperty('type') === 'list_boolean') {

Changed prepareRow in FieldInstance to get only the roles referenced.

Forgot to fix MigrateNodeRevisionTest

Ignore previous patch.

Fix the test, again. And some other cleanup and coding standard fixes.

Closed #2814957: Support Drupal 7 user reference fields as a duplicate since this has both the node and user reference migrations. The patch isn't too big and the migrations are so similar it was easier to work on them both in the same issue.

Yea! I am so happy this is finally getting in. All feedback has been addresses as far as I can tell, and I looked through the code and it looks good to me. I'm going to kick off a few more tests against the other databases, but as long as they are green I'm excited to RTBC this one.

so if i understand correctly this patch will not work for Drupal 8.9 and i must change to 9.1x dev in order to migrate node reference fields from a D7 site? Thanks

@pyxio, Yes. Also, this relies on #2447727: Add base class for migrating reference fields which was only committed to 9.1.x.

The patch in #57 no longer applies. I hope it is a simple reroll.

And a reroll.

Needed a reroll

I compared the patches in #63 and #65. Here is one hunk from #65:

+++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MultilingualReviewPageTest.php
@@ - + @@ protected function getAvailablePaths() {
       'Locale',
       'Menu',
       'Menu translation',
+      'Node Reference',
       'Node',
       'Number',
       'OpenID',

The patch in #63 added 'Node Reference' after 'Node', keeping the list sorted (alphabetized). Can we keep it that way?

This is just one of several examples.

66. Sorting fixed. This is a real nuisance. The sort tool in PhpStorm sorts it as in patch #65, as does the String Manipulation plugin, and I just decided to not fight it.

@quietone:

Thanks for the update. I compared the patches in #63 and #67. Now that the sorting is consistent, the only differences between the two are metadata (commit hashes and line numbers) and context lines.

Then I realized that #63 was already a reroll, and had not been reviewed. I should really compare the patches in #57 and #67. In addition to the differences above, I see several other changes:

1. Entity IDs in the fixtures. The IDs have to be changed because other issues have added two entries each to the field-storage and field tables. No, I should say field and field instance, since these are D7 fixtures. And then the field-instance table refers to the IDs in the field table, so those are updated, too.
2. Some of the changes in context lines are confusing, because the patch in #57 adds lines to the end of a test method, but the patch in #67 adds them in the middle of a test method, before a few comment lines. That is because of the changes in #3165763: Combine two tests to one in d7 MigrateFieldTest and MigrateFieldInstance.
3. I will not set it back to NW for this, but a comment here would be welcome:
   

   +++ b/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceTest.php
   @@ -168,6 +168,26 @@ public function testFieldInstances() {
        $this->assertEntity('node.article.field_vocab_localize', 'vocab_localize', 'entity_reference', FALSE, FALSE);
        $this->assertEntity('node.article.field_vocab_translate', 'vocab_translate', 'entity_reference', FALSE, TRUE);
    
   +    $this->assertEntity('node.article.field_node_reference', 'Node Reference', 'entity_reference', FALSE, TRUE);
   +    $this->assertEntity('node.article.field_user_reference', 'User Reference', 'entity_reference', FALSE, TRUE);
   

4. There are some changes to getEntityCounts() in some of the tests. Since we added two items each to the field and field-instamce config tables, this looks reasonable. Since the tests pass, it must be right.

These differences all look reasonable, so based on the review in #59 I am setting the status back to RTBC.

+1 RTBC.

I ran a migration on a drupalcamp site I maintain that used references in drupal 7 for session speakers using the patch in #67 (applied to 9.1.0-alpha1). It successfully migrated user references for 1...N items into a multi-value entity reference field.

@benjifisher, thanks for noticing the missing comment. Adding a comment as per #68.3 which keeps the file consistency with the commenting of the tests.

I've spellchecked the one line comment fix in patch #70. Since it is a one line addition of a comment in a test file and no code changes, I'm setting back to RTBC.

Just noticed this should be on the D7 meta.

+1 for the update in #70. (edited)

@benjifisher, thanks !

Actual patch for D8.9.8

Restoring RTBC for the patch in #70.

It looks as though the testbot is looking at the wrong patch, so I am re-uploading the one from #70.

@benjifisher, of course! thanks.

Patch still applies on 9.1.0.

One very minor question:

1. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/field/d7/NodeReference.php
   @@ -0,0 +1,56 @@
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function getFieldFormatterMap() {
   +    return [
   +      'node_reference_default' => 'entity_reference_label',
   +      'node_reference_plain' => 'entity_reference_label',
   +      'node_reference_nid' => 'entity_reference_entity_id',
   +      'node_reference_node' => 'entity_reference_entity_view',
   +      'node_reference_path' => 'entity_reference_entity_id',
   +    ];
   +  }
   

   Should the path formatter map to the label instead of the ID? Or do we always map to ID if there's no matching formatter?

2. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/field/d7/UserReference.php
   @@ -0,0 +1,76 @@
   +  }
   

   Same question here.

@catch, thanks. I agree.

This makes the changes suggested in #81 and add assertions for the two new components.

The patch in #83 needs a reroll. I think it conflicts with #2565931: Handle long comment bundle names. That is a Novice task.

Yes, it does need a reroll and here it is.

Another reroll

I reviewed the comments since the last RTBC and see that I did not make the change for #81.1. This patch addresses that.

We might have fewer rerolls if we used the MR workflow instead of patches.

I compared the patches from #70 and #87. The changes look appropriate:

• Change _entity_id to _entity_label for both nodereference and userreference fields, as suggested in #81. Corresponding changes in the tests.
• Context lines have changed in several places.
• Entity counts have changed in UpgradeD7Test. In both patches, the effect is to increase the counts by 2.
• Both patches add items to the end of the database fixture. Because of unrelated changes, the IDs of the new items are updated.

There is one little problem. Because of the way PHP arrays work, it makes no difference, but it is a little sloppy:

+++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php
@@ -5357,6 +5387,25 @@
...
+->values(array(
+  'id' => '88',
+  'field_id' => '2',
+  'field_id' => '56',

@benjifisher, Nice find. Thanks.

And here is a fix.

@quietone, thanks for the fix! RTBC

That looks like a random fail.

Another random failure, #3099427: [random test failure] FieldLayoutTest::testEntityView()

Just one question, and one suggestion (feel free to ignore), fine to put back to RTBC with answer to the question

1. +++ b/core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
   @@ -65,6 +65,51 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
   +      if (!empty($field_data['settings']['referenceable_types'])) {
   +        foreach ($field_data['settings']['referenceable_types'] as $type => $value) {
   +          if ($value) {
   +            $instance_settings['handler_settings']['target_bundles'][$type] = $value;
   +          }
   +        }
   

   we could replace this with a one-liner$instance_settings['handler_settings']['target_bundles'] = array_filter($field_data['settings']['referenceable_types'] ?? []), in fact it could go straight into the 'target_bundles' in the original array too

2. +++ b/core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
   @@ -65,6 +65,51 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
   +        foreach ($row->get('roles') as $role) {
   +          $instance_settings['handler_settings']['filter']['role'] = [
   

   does this need the check for empty options like above?

95.1 Nice. and fixed.

95.2 Good question. There is not check for empty here because that work has already been done in prepareRow. Why? Because a query on the source database needs to be done to get the roles, and the must be done in the source plugin.

I compared the patch in #96 to the one in #87. The changes are as expected. +1

Adding review credits

Committed 2a2f2de and pushed to 9.2.x. Thanks!

Discovered a bug introduced here: #3198732: Migrating reference fields: target_bundles may never be empty array.

Discovered another

+++ b/core/modules/field/src/Plugin/migrate/process/d7/FieldInstanceSettings.php
@@ -65,6 +65,43 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
+      if ($row->hasSourceProperty('roles')) {
+        $instance_settings['handler_settings']['filter']['type'] = 'role';
+        foreach ($row->get('roles') as $role) {
+          $instance_settings['handler_settings']['filter']['role'] = [
+            $role['name'] => $role['name'],
+          ];
+        }
+      }

This contains another bug, fix at #3198732-19: Migrating reference fields: target_bundles may never be empty array.

Patch for node reference fieldsettings

Rerolled patch #96 including #105 update

Sorry, removed my last added patch cause I was using an old drupal version. The #105 works fine!