[PP-4] Multifield to Paragraphs migration

I tried this much out just to see how far it would get. I got the new paragraph types created, with the right fields on them, and each of the content types that had a multifield has an entityreference field linked to the right paragraph type. There is more to do, but it looks like a good start!

There's already some of that in place for field collections inside of paragraphs. But to see how to go about this, review https://www.drupal.org/docs/8/api/migrate-api/generating-database-fixtur...

Everything but the content migration seems to be working fine. Paragraphs are created with the right fields on them and the content that had multifields now has paragraphs.

I had erratic results on content, some values were correct but others were missing. I examined the results of getSubfields() to see what indexes were being selected and that seems to be where the problem is. It works fine for fields that have a single index, not for those with more than one. For instance I had a link subfield in a multifield and the array returned for that field had no index (and so no content was pulled in). I also had a formatted text field and instead of pulling the 'value' column, it pulled the 'format' column (so again the content was missing). I think you need to find a way to get the primary column if there are multiple values.

This is really complicated stuff, the way multifield stores values does not make this easy, I was looking at this earlier to try to figure out how to pull the right values, and realized how hard that was going to be. I think this is really close, but not quite yet.

Also I tried to do a rollback and re-import and that just wiped everything out. It's possible that just won't work, but if so we should be sure to warn about that.

A very minor nit, I would change the tag from 'Multifield Content' to 'Content', or at least add that as an additional tag, since the current documentation recommends that you can do a migration of all the content using --tag=Content.

Just to clarify, is this patch now ready for review or is there more coming?

I have concluded that my problem with the wrong columns being selected is because I have bad values in my D7 database. Even for non-multifield fields, the "index" and "column" values are incorrect in my source data. So I am not going to be able to provide a good test of your code. I can see that the code probably still does not take into account multiple field indexes, instead assuming a single value, so it looks like that still needs to be fixed.

Really phenomenal work here. Lots of little things below, nothing super huge. Plus, it would be awful nice to still see some tests.

1. +++ b/paragraphs.module
   @@ -529,23 +544,95 @@ function _paragraphs_migration_bundle_adjust(array &$migration) {
   +  $migration['process']['cardinality'] = $cardinality_process;
   +}
   +
   +
   

   Nit: whitespace.

2. +++ b/paragraphs.module
   @@ -529,23 +544,95 @@ function _paragraphs_migration_bundle_adjust(array &$migration) {
   +function paragraphs_migrate_upgrade_d7_field_formatter_settings_prepare_row(Row $row, MigrateSourceInterface $source, MigrationInterface $migration) {
   +  // Skip migrating field formatter settings from multifields.
   +  // These will need to be configured manually at "Manage Display".
   +  if ($row->getSourceProperty('entity_type') == 'multifield') {
   

   We cannot be sure about the name of the id of the migration. Migrate upgrade adds 'upgrade_' as a prefix automatically, but that can be changed/altered with a drush argument at generation time. Or it can be changed manually post generation. It is more safe to check the source plugin id.

3. +++ b/src/Plugin/migrate/D7MultifieldItemDeriver.php
   @@ -0,0 +1,169 @@
   +      $source_plugin = static::getSourcePlugin('d7_field_instance');
   +      $source_plugin->checkRequirements();
   +
   +      foreach ($source_plugin as $row) {
   +        if ($row->getSourceProperty('entity_type') == 'multifield') {
   

   We are so close to getting #2951550: Make service for field discovery for use in migrate entity derivers to land in 8.6. Maybe it will so we can start using it here?

4. +++ b/src/Plugin/migrate/field/Multifield.php
   @@ -0,0 +1,125 @@
   +class Multifield extends FieldPluginBase {
   

   This all got improved/made better in #2631698: Fix sub-optimal DX in MigrateFieldInterface. Should we re-work this for 8.6?

5. +++ b/src/Plugin/migrate/field/Multifield.php
   @@ -0,0 +1,125 @@
   +    // Workaround for Drupal 8.4. In D8.5+ this should only call the parent.
   +    // @todo Remove all but parent call after Drupal 8.6 is released.
   +    // @see https://www.drupal.org/project/paragraphs/issues/2950492
   +    //
   

   Since 8.6 is now alpha and 8.4 no longer has much support, shall we remove this?

6. +++ b/src/Plugin/migrate/process/MultifieldLookup.php
   @@ -0,0 +1,93 @@
   +   * EntityExists constructor.
   

   Nit: copy/paste title.

7. +++ b/src/Plugin/migrate/process/MultifieldLookup.php
   @@ -0,0 +1,93 @@
   +    // @TODO Notice that this is not using migrate map tables. Is this ok?
   +    $paragraph_ids = $this->storage->getQuery()
   +      ->condition('parent_id', $row->getSourceProperty($this->configuration['entity_id_name']))
   +      ->condition('parent_field_name', $this->configuration['field_name'])
   +      ->execute();
   +    if (empty($paragraph_ids)) {
   +      return $value;
   +    }
   

   This should maybe use/extend migration_lookup instead?

8. +++ b/src/Plugin/migrate/source/d7/MultifieldItem.php
   @@ -0,0 +1,137 @@
   + * - field_name: (optional) If supplied, this will only return field collections
   

   Copy/paste.

9. +++ b/src/Plugin/migrate/source/d7/MultifieldItem.php
   @@ -0,0 +1,137 @@
   +    // They are of the form parent_field_name + _ + sub_field_name + _ + suffix such as target_id or value.
   

   Comment length.

10. +++ b/src/Plugin/migrate/source/d7/MultifieldType.php
    @@ -0,0 +1,114 @@
    +   * @deprecated addDescription configuration key is deprecated in 8.x-1.0 and
    +   * will be removed prior to 9.x.  Use add_description instead.
    +   *
    +   * @see https://www.drupal.org/project/paragraphs/issues/2911242
    +   */
    +  public function setConfiguration(array $configuration) {
    

    Shall we remove this BC logic before we land this?

+++ b/src/Plugin/migrate/source/d7/MultifieldItem.php

+    foreach ($fields as $field_name => $field_config) {
+      $field_value = $row->getSourceProperty($this->configuration['field_name'] . '_' . $field_name . '_value');
+      if (empty($field_value)) {
+        $field_value_suffix = key($field_config['data']['indexes']);
+        $field_value = $row->getSourceProperty($this->configuration['field_name'] . '_' . $field_name . '_' . $field_value_suffix);
+      }
+      $row->setSourceProperty($field_name, $field_value);
+    }

Hi, I tried using this patch to migrate multifields to paragraphs. I have a taxonomy subfield in my multifield and it did not migrate the data(term id) to the paragraph type subfield, however, another subfield of type text was correctly migrated.

I think this has something to do with the data type of $field_value. For term/entity references, shouldn't it be of type array?

I changed the code on line 67 when $field_value is empty to the following and tested with migrating taxonomy subfields and it worked but I do not know if there is another or a better way to solve this.

$field_value = [
  [
    $field_value_suffix => $row->getSourceProperty($this->configuration['field_name'] . '_' . $field_name . '_' . $field_value_suffix),
  ],
];

Could you please look into this?
Thanks.

Updated the patch in #28 so that there is a test to see if the configuration parameter is present before testing it's value. It removes a PHP warning message.

Just a heads up that this patch has some issues with Drupal >=9.1.4. The patch applies successfully, but I'm getting an error when I run migrate-upgrade:

You have requested a non-existent service "plugin.manager.migrate.cckfield". Did you mean one of these: "plugin.manager.migrate.source", "plugin.manager.migration", "plugin.manager.migrate.field", "plugin.manager.migrate.process", "plugin.manager.migr  
  ate.destination", "plugin.manager.migrate.id_map", "plugin.manager.image.effect", "plugin.manager.views.field"?

Since Drupal 9.1.4 was released, additional work has been done on the patch that this patch is based on: https://www.drupal.org/project/paragraphs/issues/2911244. I'm guessing those changes need to be carried over to this patch.

I'm looking into it, but it's all a little over my head. Will report back if I figure anything out.

No interdiff - I think I've changed too many things.

This patch needs the following patches:

1. #3145755-23: Orphaned (nested) paragraphs entities after migration & (invalid) stub paragraph entity leftovers
2. #3203739-6: Paragraphs migrations broken in Drupal >=9.1.4

Improvements compared to #28:

1. You can migrate more than one multifield for a content entity.
2. You can migrate multifield values with <field value delta> > 0.
3. This patch migrates all of the revisions, not just the current ones.
4. This patch migrates multilingual values translated with Entity Translation.
5. The complete patch contains a partial test DB fixture (it could be applied on top of the fixture in Drupal core) and a migration PHP Kernel test as well.

I still have to file two feature requests, one for core, and an another for Entity Reference Revisions:

1. Core should provider a way to get not only the field value properties, but the host entity related data as well (e.g. entity_type, id, revision, langcode...) – this is crutial, but all we have to do is to add alter tags to the query performed by Drupal\migrate_drupal\Plugin\migrate\source\d7\FieldableEntity::getFieldValues – now my patch assumes that the alter tag will by migrate_field_values.
2. Entity Reference Revisions shouldn't force-create a new paragraphs to be saved when we migrate any additional revision for an already migrated content entity. This isn't critical, but without solving this problem, you will have unnecessary paragraphs revisions.

Core issue mentioned in #33: #3218294-2: Allow altering field value query performed by FieldableEntity.

ERR patch of #33: #3218312-2: Provide an option to skip (opt-out) forcibly created new revision of the referenced entity (so that migrations won't lead to unnecessary saved revisions).

Unassigning, but leaving NW because:

1. I think it would be nice to create a patch that applies on HEAD (let's be more pragmatic: HEAD + #3203739: Paragraphs migrations broken in Drupal >=9.1.4)
2. The new (migration) plugins need test coverage (imho).
3. I want to test this with an enormously big database (and I have one).

This new patch makes possible to migrate paragraphs from multifield values selectively, per (host) entity type (and even per bundle): because multifield migration derivative IDs are changed from multifield:<source_field_name> to multifield:<host_entity_type>:<host_bundle>:<source_field_name>.

1. +++ b/migrations/multifield.yml
   @@ -82,4 +82,4 @@ migration_dependencies:
   -    - mutlifield_translation_settings
   +    - multifield_translation_settings
   
   similarity index 100%
   rename from migrations/mutlifield_type.yml
   
   rename from migrations/mutlifield_type.yml
   rename to migrations/multifield_type.yml
   
   +++ b/src/Plugin/migrate/MultifieldConfigDeriver.php
   @@ -26,10 +26,19 @@ class MultifieldConfigDeriver extends DeriverBase implements ContainerDeriverInt
   -    'mutlifield_translation_settings',
   +    'multifield_translation_settings',
   

   Fixing typos: mutli* to multi*

2. +++ b/src/Plugin/migrate/MultifieldConfigDeriver.php
   @@ -26,10 +26,19 @@ class MultifieldConfigDeriver extends DeriverBase implements ContainerDeriverInt
   +  /**
   +   * Migration base IDs of migrations derived per entity type, bundle and field.
   +   *
   +   * @const string[]
   +   */
   +  const MIGRATIONS_DERIVED_PER_TYPE_BUNDLE_FIELD = [
   +    'multifield',
   +  ];
   
   @@ -117,15 +126,28 @@ class MultifieldConfigDeriver extends DeriverBase implements ContainerDeriverInt
   +      $derivative_id = implode(PluginBase::DERIVATIVE_SEPARATOR, [
   +        $entity_type,
   +        $bundle,
   +        $field_name,
   +      ]);
   +      $this->hardenDependencies($plugin_definition, static::MIGRATIONS_DERIVED_PER_TYPE_BUNDLE_FIELD, $derivative_id);
   +      $this->hardenMigrationLookups($plugin_definition, static::MIGRATIONS_DERIVED_PER_TYPE_BUNDLE_FIELD, $derivative_id);
   

   Multifield migration IDs used in lookups are updated to the new derivative ID.

+++ b/paragraphs.module
@@ -472,3 +475,15 @@ function template_preprocess_paragraphs_summary(&$variables) {
+
+/**
+ * Implements hook_query_alter().
+ */
+function paragraphs_query_migrate_field_values_alter(AlterableInterface $query) {
+  if (!$query instanceof SelectInterface) {
+    return;
+  }
+
+  // Requires core patch at https://drupal.org/i/3218294
+  MultifieldMigration::addCruicalMultifieldFieldProperties($query);
+}

The FR I created for core adds the tag migrate_field_value, while this hook assumes migrate_field_values

Addressed #40 and also improved MultifieldMigrationsTrait (which will make us able to reuse it in a Migrate UI functional test... or at least I hope so!)

Holy crap 🤯

1. +++ b/src/Plugin/migrate/field/Multifield.php
   @@ -0,0 +1,173 @@
   +    // Requires Entity Reference Revisions patch https://drupal.org/i/3218312
   +    // and Drupal core patch https://drupal.org/i/3218294.
   

   🤔 Should we make this more explicit by adding these to the paragraphs module's composer.json in this patch?

2. +++ b/src/Plugin/migrate/field/Multifield.php
   @@ -0,0 +1,173 @@
   +          // This "needs_resave" tells Entity Reference Revisions to not
   +          // force-create new paragraphs revision while a migration adds a new
   +          // revision to the host entity.
   +          'needs_resave' => [
   +            'plugin' => 'default_value',
   +            'default_value' => FALSE,
   +          ],
   

   👍 This is why we need that ERR patch.

3. +++ b/src/Plugin/migrate/field/Paragraphs.php
   @@ -240,11 +258,20 @@ class Paragraphs extends ParagraphsFieldPluginBase {
   +        $migration->mergeProcessOfProperty('options/settings/view_mode', array_merge($original_process, $process_to_add));
   

   🐛 This looks like a typo: it the first parameter needs to be dynamic here.

4. +++ b/src/Plugin/migrate/field/ParagraphsFieldPluginBase.php
   @@ -120,4 +120,28 @@ abstract class ParagraphsFieldPluginBase extends FieldPluginBase {
   +  protected static function processIsPresent(array $process_pipeline, $plugin) {
   

   👏

5. +++ b/src/Plugin/migrate/process/MultifieldFieldTranslatable.php
   @@ -0,0 +1,82 @@
   +      return $source->getDatabase()->select('field_config', 'fc')
   +          ->fields('fc', ['translatable'])
   +          ->condition('fc.field_name', $row->getSourceProperty('bundle'))
   +          ->execute()->fetchCol()[0] ?? $value;
   

   Übernit: slightly too much indentation here.

The patch in #41 tried to migrate deletes multifields as well. That should be fixed.
I also added a new target bundle lookup plugin which will skip those multifield subfield instance migration rows which miss the destination paragraph type.

I also removed every non-crucial changes (see cleanup interdiff).

Re #42:

1. I don't want to do that, it seems to be risky.
2. 👍
3. Every unnecessary change was removed, including this one.
4. 👍
5. Fixed!

Patch addressing most recent releases of Drupal core 9.1+ (see #3190815: Source count caching broken: impossible to enable source count caching for SqlBase-based source plugins (plus, unneeded cache I/O))

1. +++ b/src/Plugin/migrate/MultifieldDeriver.php
   @@ -20,26 +20,23 @@ class MultifieldDeriver extends MultifieldConfigDeriver {
   -      $fields_types_bundles = $fields_types_bundles_query
   +      $fields_types_bundles = $field_instance_source->query()
   

   👏 Nice reuse of pre-existing (core) source plugins!

2. +++ b/src/Plugin/migrate/field/Multifield.php
   @@ -168,6 +168,38 @@ class Multifield extends ParagraphsFieldPluginBase {
   +  protected function addMultifieldTargetBundleLookup(MigrationInterface $migration, string $destination_property) {
   

   Übernit: this could be static 🤓

3. +++ b/src/Plugin/migrate/field/Multifield.php
   @@ -168,6 +168,38 @@ class Multifield extends ParagraphsFieldPluginBase {
   +              'multifield' => $this->t('Cannot found the destination paragraphs type of this multifield subfield.'),
   

   🤓 s/found/find/

   (Also: this does not need to be translatable.)

4. +++ b/src/Plugin/migrate/process/TargetBundleLookup.php
   @@ -0,0 +1,292 @@
   + * Two types of operation are available, that can even be combined.
   

   s/operation/operations/

5. +++ b/src/Plugin/migrate/process/TargetBundleLookup.php
   @@ -0,0 +1,292 @@
   + * process:
   + *   bundle:
   + *     plugin: target_bundle_lookup
   + *     source_entity_type: entity_type
   + *     source: bundle
   + *     lookup_migrations:
   + *       multifield: multifield_type
   + *       paragraphs_item: d7_paragraphs_type
   + *       node:
   + *         - d7_node_type
   + *         - custom_node_type_migration_id
   + *     skip_row_on_missing:
   + *       multifield: 'Destination bundle is missing or is not yet migrated.'
   + *       paragraphs_item: true
   

   🤯 Woahh … this is complicated, but once you get it … it is super powerful! Imagine core would've had this years ago… then we'd have many more stable contrib migrations I bet.

6. +++ b/src/Utility/MultifieldMigration.php
   @@ -29,13 +30,22 @@ final class MultifieldMigration {
   +        // If multifield type plugin requirements aren't met, then the plugin
   +        // manager throws a plugin not found exception.
   +        return self::$multifields = [];
   

   🤔 … and this return statement avoids that, right?

   But why the assignment and not just

   return [];
   

   🤓

7. +++ b/tests/src/Kernel/migrate/MultifieldTest.php
   @@ -165,4 +169,25 @@ class MultifieldTest extends ParagraphsMigrationTestBase {
   +  /**
   +   * Asserts whether no migration errors were logged to $this->migrateMessages.
   +   *
   +   * This is a DX helper method which makes migration messages easy to capture
   +   * when something goes wrong.
   +   */
   +  protected function assertNoErrors() {
   

   👏 We should totally add this to Drupal core!!! 🙏

New patch on top of #3226658-8: Guarantee the uniqueness of the migrated paragraph types and #3145755-40: Orphaned (nested) paragraphs entities after migration & (invalid) stub paragraph entity leftovers.

Works with any core versions, and also ensures paragraphs type uniqueness.

Rebased on #3145755-43: Orphaned (nested) paragraphs entities after migration & (invalid) stub paragraph entity leftovers.

I know that I want this patch, but I am struggling to get it to apply with all the other dependencies, maybe it's the order of the patches?

This is what I have in the patches section:

            "drupal/paragraphs": {
                "#3226658": "https://www.drupal.org/files/issues/2021-08-05/paragraphs-fix_bundle_migration_collision-3226658-8.patch",
                "#3145755": "https://www.drupal.org/files/issues/2021-08-09/paragraphs-bug_derive_by_parent_and_orphans-3145755-43.diff",
                "#2977853": "https://www.drupal.org/files/issues/2021-08-09/paragraphs-multifield_to_paragraphs-2977853-48.patch"
            },

I am using the latest version of paragraphs: 8.x-1.13

Any guidance on this would be greatly appreaciated.