Orphaned (nested) paragraphs entities after migration & (invalid) stub paragraph entity leftovers

1. +++ b/src/ParagraphsMigration.php
   @@ -0,0 +1,57 @@
   +/**
   + * Class ParagraphsMigration.
   + */
   

   Nit: meaningless comment.

2. +++ b/src/ParagraphsMigration.php
   @@ -0,0 +1,57 @@
   +   * Default derivation strategy (BC).
   ...
   +  const DERIVATION_METHOD_DEFAULT = self::DERIVATION_METHOD_LEGACY;
   

   Nit: "strategy" vs "method".

   I think "strategy" is preferable. I was going to say that that is the term that https://www.drupal.org/node/3105503 uses, but that's unfortunately not true. That uses "type" 🤷‍♂️

3. +++ b/src/ParagraphsMigration.php
   @@ -0,0 +1,57 @@
   +    return in_array($method, $handled_methods, TRUE) ?
   +      $method :
   +      self::DERIVATION_METHOD_DEFAULT;
   
   +++ b/src/Plugin/migrate/D7FieldCollectionItemDeriver.php
   @@ -69,36 +72,95 @@ class D7FieldCollectionItemDeriver extends DeriverBase implements ContainerDeriv
   +          $values['label'] = $derivation_method_by_parent ?
   +            $this->t('@label (in @parent_entity_bundle @parent_entity_type, referenced from @field_label field)', $label_args) :
   +            $this->t('@label (@type)', $label_args);
   
   +++ b/src/Plugin/migrate/D7ParagraphsItemDeriver.php
   @@ -60,37 +62,107 @@ class D7ParagraphsItemDeriver extends DeriverBase implements ContainerDeriverInt
   +            $derivative_id = $derivation_method_by_parent ?
   +              "$field_name:$parent_entity_type:$parent_bundle:$paragraph_bundle" :
   +              $paragraph_bundle;
   

   🤓 Weird formatting IMHO. I think this is what core uses:

   return $a_very_long_condition
     ? $foo
     : $bar;
   

4. +++ b/src/Plugin/migrate/D7FieldCollectionItemDeriver.php
   @@ -69,36 +72,95 @@ class D7FieldCollectionItemDeriver extends DeriverBase implements ContainerDeriv
   +          // No field collection field found in this source database.
   

   Übernit: s/field collection/field_collection/

5. +++ b/src/Plugin/migrate/D7FieldCollectionItemDeriver.php
   @@ -69,36 +72,95 @@ class D7FieldCollectionItemDeriver extends DeriverBase implements ContainerDeriv
   +          /** @var \Drupal\migrate\Plugin\Migration $migration */
   +          $migration = \Drupal::service('plugin.manager.migration')
   +            ->createStubMigration($values);
   
   +++ b/src/Plugin/migrate/D7ParagraphsItemDeriver.php
   @@ -60,37 +62,107 @@ class D7ParagraphsItemDeriver extends DeriverBase implements ContainerDeriverInt
   +            /** @var \Drupal\migrate\Plugin\Migration $migration */
   +            $migration = \Drupal::service('plugin.manager.migration')
   +              ->createStubMigration($values);
   

   Nit: IMHO it's always preferable to do assert($something instanceof FQCN) instead of /** @var FQCN $something */. That gives you an extra assurance and is not IDE-specific.

6. +++ b/src/Plugin/migrate/D7ParagraphsItemDeriver.php
   @@ -60,37 +62,107 @@ class D7ParagraphsItemDeriver extends DeriverBase implements ContainerDeriverInt
   +          // No paragraph fields found in this source database.
   

   Übernit: s/paragraph/paragraphs/

7. +++ b/src/Plugin/migrate/field/FieldCollection.php
   @@ -66,16 +67,41 @@ class FieldCollection extends FieldPluginBase {
   +    $is_revision_migration = strpos($destination_plugin_id, 'entity_revision:') === 0 || strpos($destination_plugin_id, 'entity_complete:') === 0 || (strpos($destination_plugin_id, 'entity_reference_revisions:') === 0 && !empty($destination_config['new_revisions']));
   +
   +    if ($derivation_method_by_parent) {
   +      $destination_entity_type_id = strpos($destination_plugin_id, 'entity:') === 0 || strpos($destination_plugin_id, 'entity_revision:') === 0 || strpos($destination_plugin_id, 'entity_complete:') === 0 || strpos($destination_plugin_id, 'entity_reference_revisions:') === 0 ?
   
   +++ b/src/Plugin/migrate/field/Paragraphs.php
   @@ -64,22 +65,47 @@ class Paragraphs extends FieldPluginBase {
   +      $is_revision_migration = strpos($destination_plugin_id, 'entity_revision:') === 0 || strpos($destination_plugin_id, 'entity_complete:') === 0 || (strpos($destination_plugin_id, 'entity_reference_revisions:') === 0 && !empty($destination_config['new_revisions']));
   +      $destination_entity_type_id = strpos($destination_plugin_id, 'entity:') === 0 || strpos($destination_plugin_id, 'entity_revision:') === 0 || strpos($destination_plugin_id, 'entity_complete:') === 0 || strpos($destination_plugin_id, 'entity_reference_revisions:') === 0 ?
   

   🤔 What about node_revision and node_complete? These seem to only work for non-Node entity types?

8. +++ b/src/Plugin/migrate/source/d7/FieldCollectionItem.php
   @@ -24,12 +24,21 @@ class FieldCollectionItem extends FieldableEntity {
   +   * The prefix of the field table that contains the entity properties.
   
   +++ b/src/Plugin/migrate/source/d7/FieldCollectionItemRevision.php
   @@ -21,6 +21,13 @@ class FieldCollectionItemRevision extends FieldCollectionItem {
   +   * The prefix of the field table that contains the entity properties.
   

   🤔 What are "entity properties"?

9. +++ b/src/Plugin/migrate/source/d7/FieldCollectionItem.php
   @@ -37,23 +46,54 @@ class FieldCollectionItem extends FieldableEntity {
   +      // This configuration item may be set by a deriver to restrict the
   

   🤔 What is a "configuration item"?

10. +++ b/src/Plugin/migrate/source/d7/ParagraphsItem.php
    @@ -46,22 +49,55 @@ class ParagraphsItem extends FieldableEntity {
    +      $query->join('paragraphs_item', 'p', "p.field_name = :field_name AND p.item_id = fd.{$field_name}_value AND p.revision_id = fd.{$field_name}_revision_id", [
    

    Übernit: why mix single and double quotes in one function call?

RE #3.7 (I wrote a reply to that but apparently failed to post it 😬) — yep, you're right, oops! 🙈😅

#8: thanks for that detailed write-up! 🙏 I am really curious to @heddn's thoughts about this!

@huzooka It looks like your patches have a bunch of extra files in them. An example I am seeing is file paths committed that show sites/default/files. Can you look at uploading your patches again? The ones closer to 1MB in size seem to be the ones affected.

#13:

1. +++ b/src/MigrationPluginsAlterer.php
   @@ -51,6 +63,55 @@ final class MigrationPluginsAlterer {
   +      // fields are present. The raw (and at this point, not accurate) migration
   +      // dependency IDs are added in
   

   🤔 "at this point not accurate" — fascinating!

2. +++ b/src/MigrationPluginsAlterer.php
   @@ -51,6 +63,55 @@ final class MigrationPluginsAlterer {
   +        $types = $this->paragraphTypes ?? NULL;
   +        if ($types === NULL) {
   +          $types = [];
   +          $type_migration_definition = $migrations['d7_paragraphs_type'] ?? NULL;
   +          if ($type_migration_definition) {
   +            $type_source = static::getSourcePlugin('d7_paragraphs_type');
   +            assert($type_source instanceof DrupalSqlBase);
   +
   +            try {
   +              $type_source->checkRequirements();
   +
   +              foreach ($type_source as $row) {
   +                assert($row instanceof Row);
   +                ['bundle' => $type] = $row->getSource();
   +                $types = array_unique(array_merge($types, [$type]));
   +              }
   +            }
   +            catch (DatabaseExceptionWrapper $e) {
   +            }
   +
   +            $this->paragraphTypes = $types;
   +          }
   

   This looks like it could/should be in a helper method.

3. +++ b/src/MigrationPluginsAlterer.php
   @@ -51,6 +63,55 @@ final class MigrationPluginsAlterer {
   +            $finalized_dependencies = array_map(function ($type) use ($dependency_to_finalize) {
   

   "finalized" is a less ambiguous word 👍 Let's use "unfinalized" and "finalized".

4. +++ b/src/ParagraphsMigration.php
   @@ -37,6 +37,13 @@ final class ParagraphsMigration {
   +   * Key of the paragraphs migration's raw migration dependencies.
   

   Does "raw" here mean "unfinalized"?

5. +++ b/tests/src/Functional/Migrate/MigrateUiParagraphsTestBase.php
   @@ -313,22 +313,22 @@ abstract class MigrateUiParagraphsTestBase extends MigrateUpgradeTestBase {
            6 => 'Paragraph Migration Test Content EN > Field Collection Test (previous revision)',
   -        7 => 'Content with nested paragraphs ENG > Any Paragraph > Any Paragraph (nested)',
   -        8 => 'Content with nested paragraphs ENG > Any Paragraph > Any Paragraph (nested)',
   -        9 => 'Content with nested paragraphs ENG > Any Paragraph',
   -        10 => 'Paragraph Migration Test Content UND > Any Paragraph',
   -        11 => 'Paragraph Migration Test Content EN > Any Paragraph',
   -        12 => 'Paragraph Migration Test Content EN > Any Paragraph (previous revision)',
   -        13 => 'Paragraph Migration Test Content UND > Any Paragraph',
   -        14 => 'Paragraph Migration Test Content EN > Any Paragraph',
   -        15 => 'Paragraph Migration Test Content EN > Any Paragraph (previous revision)',
   -        16 => 'Paragraph Migration Test Content EN > Any Paragraph (previous revision)',
   -        17 => 'Content with nested paragraphs ENG > Any Paragraph',
   -        18 => 'Paragraph Migration Test Content UND > Paragraph One Only',
   -        19 => 'Paragraph Migration Test Content EN > Paragraph One Only',
   -        20 => 'Paragraph Migration Test Content EN > Paragraph One Only (previous revision)',
   -        21 => 'Paragraph Migration Test Content UND > Nested FC Outer > Nested FC Inner',
   -        22 => 'Paragraph Migration Test Content UND > Nested FC Outer',
   +        7 => 'Paragraph Migration Test Content UND > Nested FC Outer > Nested FC Inner',
   +        8 => 'Paragraph Migration Test Content UND > Nested FC Outer',
   +        9 => 'Content with nested paragraphs ENG > Any Paragraph > Any Paragraph (nested)',
   +        10 => 'Content with nested paragraphs ENG > Any Paragraph > Any Paragraph (nested)',
   +        11 => 'Content with nested paragraphs ENG > Any Paragraph',
   +        12 => 'Paragraph Migration Test Content UND > Any Paragraph',
   +        13 => 'Paragraph Migration Test Content EN > Any Paragraph',
   +        14 => 'Paragraph Migration Test Content EN > Any Paragraph (previous revision)',
   +        15 => 'Paragraph Migration Test Content UND > Any Paragraph',
   +        16 => 'Paragraph Migration Test Content EN > Any Paragraph',
   +        17 => 'Paragraph Migration Test Content EN > Any Paragraph (previous revision)',
   +        18 => 'Paragraph Migration Test Content EN > Any Paragraph (previous revision)',
   +        19 => 'Content with nested paragraphs ENG > Any Paragraph',
   +        20 => 'Paragraph Migration Test Content UND > Paragraph One Only',
   +        21 => 'Paragraph Migration Test Content EN > Paragraph One Only',
   +        22 => 'Paragraph Migration Test Content EN > Paragraph One Only (previous revision)',
   

   Why is this order changing? (Not that it matters, I would just like to understand.)

#17:

1. +++ b/src/Plugin/migrate/process/ParagraphsDeltaSort.php
   @@ -0,0 +1,38 @@
   + * This plugin is required only because Drupal core's
   + * FieldableEntity::getFieldValues() does not guarantee that the returned
   + * field values are sorted by their delta, so when the source Drupal database
   + * is PostgreSQL, the values of some multi-value fields are migrated in reversed
   + * order.
   + * @todo remove when https://drupal.org/i/3164520 is fixed for all supported
   + *   Drupal core release.
   + *
   + * @MigrateProcessPlugin(
   + *   id = "paragraphs_delta_sort",
   + *   handle_multiples = TRUE
   + * )
   + */
   +class ParagraphsDeltaSort extends ProcessPluginBase {
   

   👏

2. +++ b/tests/src/Functional/Migrate/MigrateUiParagraphsTestBase.php
   @@ -584,6 +586,14 @@ abstract class MigrateUiParagraphsTestBase extends MigrateUpgradeTestBase {
   +        if (count(array_filter(array_keys($expected_entity_labels), 'is_int')) === count($expected_entity_labels)) {
   

   🤔 Why do we need the array_filter(…, 'is_int') here?

Amazing work!!! 🤩🚀

+++ b/src/MigrationPluginsAlterer.php
@@ -100,88 +88,62 @@ final class MigrationPluginsAlterer {
+    switch ($current_value) {
+      case 'field_collection_item':
+        $migration['source']["paragraphs_$process_property"] = 'paragraph';
+        $migration['process'][$process_property] = "paragraphs_$process_property";
+        $migration['migration_dependencies']['required'][] = 'd7_field_collection_type';
+        break;
 
-    $source_is_dynamic = $source === NULL;
-    $source_is_paragraph_or_field_collection = in_array($source, array_keys(self::PARAGRAPHS_ENTITY_TYPE_ID_MAP));
+      case 'paragraphs_item':
+        $migration['source']["paragraphs_$process_property"] = 'paragraph';
+        $migration['process'][$process_property] = "paragraphs_$process_property";
+        $migration['migration_dependencies']['required'][] = 'd7_paragraphs_type';
+        break;
 
-    return $source_is_dynamic || $source_is_paragraph_or_field_collection;
+      case NULL:
+        $entity_type_process = &$migration['process'][$process_property];
+        $entity_type_process[] = [
+          'plugin' => 'static_map',
+          'map' => self::PARAGRAPHS_ENTITY_TYPE_ID_MAP,
+          'bypass' => TRUE,
+        ];
+        $migration['migration_dependencies']['optional'][] = 'd7_field_collection_type';
+        $migration['migration_dependencies']['optional'][] = 'd7_paragraphs_type';
+        break;
+    }

This is the essential change here — everything else is just infrastructure changes that make this code a lot simpler (and hence easier to understand).

What this is doing now is using required dependencies whenever possible. Only when the source value of the process pipeline is dynamic, we add both possible dependnecies, and we mark both as optional, since we cannot be certain that it will be required.

👍

The two fails are for coding standards:

line 15	The class short comment should describe what the class does and not simply repeat the class name
line 66	Line exceeds 80 characters; contains 82 characters

Deleted.

Confirmation over at #3178043-4: Experience report: migrating D7 field collections to D8 paragraphs that this patch is helping making Paragraphs migrations feasible.

As per my post mentioned on #33 above, I was initially struggling with this with a migration involving a site with both field collections and paragraphs plus some of the paragraphs being nested.

Some essential steps I found to make it all work were:

I had to run the field and field instance migrations twice, eg:

drush mim upgrade_d7_field
drush mim d7_field
drush mim upgrade_d7_field_instance
drush mim d7_field_instance

d7_field and d7_field_instance were required to get the d7 field collection originating paragraph type fields added while;
upgrade_d7_field and upgrade_d7_field_instance got the d7 paragraph originating paragraph type fields.
There seemed to be no side effects of doing this, but it seemed odd that it was necessary.

then for the data...

The field collections worked absolutely fine, as did the paragraphs which were not nested. For the paragraphs which were nested (to be a bit more clear, by nested I mean paragraph type 1 > paragraph type 2 > content type hierarchy) it was essential to run the first child items first, followed by the "middle" paragraphs, finally the content. Without doing this (which was not protected with migration order dependency - I had to work out which order it needed to be and do them one by one), the paragraphs would get very confused with the wrong "paragraph type" being assigned to the content with empty fields.

So the summary was; you can't just let it run through the migration on auto. It needs some manual working out of the order in which things need to happen.

#34:

Without doing this (which was not protected with migration order dependency - I had to work out which order it needed to be and do them one by one), the paragraphs would get very confused with the wrong "paragraph type" being assigned to the content with empty fields.

So the summary was; you can't just let it run through the migration on auto. It needs some manual working out of the order in which things need to happen.

Thanks for sharing that — hopefully this means @huzooka and I can convince the migration system maintainers (and the Paragraphs maintainers) to use more ['migration_dependencies']['required'] than we've been able to convince them to use in Drupal core 🤞

#39 interdiff:

+++ b/src/ParagraphsMigration.php
@@ -49,22 +14,4 @@ final class ParagraphsMigration {
-  public static function getParagraphsMigrationDerivationMethod() {
-    $method = Settings::get(self::DERIVATION_STRATEGY_KEY, self::DERIVATION_STRATEGY_DEFAULT);
-    $handled_methods = [
-      self::DERIVATION_STRATEGY_LEGACY,
-      self::DERIVATION_STRATEGY_PARENT,
-    ];
-
-    return in_array($method, $handled_methods, TRUE)
-      ? $method
-      : self::DERIVATION_STRATEGY_DEFAULT;
-  }

Well … that's a huge simplification! 🤩

#40:

1. +++ b/migrations/d7_field_collection_revisions.yml
   @@ -14,6 +14,11 @@ process:
   +      no_stub: true
   +    -
   +      plugin: skip_on_empty
   +      method: row
   +      message: "This field collection entity revision belongs to a missing field collection on the source site. Skipping since there is no data we can migrate."
   

   If I understand it correctly, this simple addition is wholly responsible for ensuring no orphaned paragraphs are migrated — wow! 🤩

2. +++ b/src/Plugin/migrate/field/FieldCollection.php
   @@ -46,34 +46,47 @@ class FieldCollection extends ParagraphsFieldPluginBase {
   +        // Try to restora some level of data integrity if the corresponding
   

   Übernit: s/restora/restore/

#43:

1. +++ b/src/MigrationPluginsAlterer.php
   @@ -14,13 +14,19 @@ final class MigrationPluginsAlterer {
      const PARAGRAPHS_ENTITY_TYPE_ID_MAP = [
   -    'field_collection_item' => 'paragraph',
   -    'paragraphs_item' => 'paragraph',
   +    'field_collection_item' => [
   +      'bundle_migration' => 'd7_field_collection_type',
   +      'default_revision_migration' => 'd7_field_collection',
   +    ],
   +    'paragraphs_item' => [
   +      'bundle_migration' => 'd7_paragraphs_type',
   +      'default_revision_migration' => 'd7_paragraphs',
   +    ],
   

   This looks much more robust indeed, and simpler.

2. +++ b/src/MigrationPluginsAlterer.php
   @@ -185,27 +208,63 @@ final class MigrationPluginsAlterer {
   +      case NULL:
   

   🤔 When can this occur?

#45:

+++ b/src/MigrationPluginsAlterer.php
@@ -360,6 +362,79 @@ final class MigrationPluginsAlterer {
+   * completely. By default, paragraphs (revision) migrations are saving migrate
+   * messages in these situations, but that means that devs might see a tons of
+   * messages they cannot solve (because its a data integrity issue of the
+   * source Drupal 7 instance), and it might be very frustrating.

… and it is not something that can solved! At least not automatically. And manually it'd also be really hard.

IOW: this is a data integrity issue affecting only past revisions: the default (live) revision MUST work correctly, otherwise the migration will fail.

And that is why this "log to inform but do not complain" solution is deemed acceptable: only if you go digging through the past history (the revisions) of these paragraphs will you run into problems.

I was only able to get this to work as #34 did, thanks for that -- side effect was my entity reference fields that used taxonomies got reset so those content types needed to have the taxonomy re-selected and content had to be re-imported

I can't get the patch to apply cleanly to the latest dev branch. I'm having some issues with nested paragraphs coming in with data-less fields and I'm hoping this patch may be the fix.