Add CckField Plugin type to migrate_drupal

This is not a migrate thing; it's a migrate_drupal thing.

Wouldn't !$migration->get('load') suffice? Also, I am surprised that superflous processing causes tests to break. Of course we shouldn't do superflous processing so this needs to stay but isn't processing a no-op when applied to migrations we do not need/want to process?

Do we perhaps want to override set() instead? I think we want to.

Here's a small review, nothing alarming stands out :)

1. +++ b/core/modules/migrate/src/Plugin/MigratePluginManager.php
   @@ -77,4 +70,20 @@ public function createInstance($plugin_id, array $configuration = array(), Migra
   +      'destination' => 'Drupal\migrate\Plugin\MigrateDestinationInterface',
   +      'process' => 'Drupal\migrate\Plugin\MigrateProcessInterface',
   +      'source' => 'Drupal\migrate\Plugin\MigrateSourceInterface',
   +      'id_map' => 'Drupal\migrate\Plugin\MigrateIdMapInterface',
   +      'entity_field' => 'Drupal\migrate\Plugin\MigrateEntityDestinationFieldInterface',
   
   +++ b/core/modules/migrate_drupal/src/Plugin/MigratePluginManager.php
   @@ -24,11 +21,11 @@ class MigratePluginManager extends BaseMigratePluginManager {
   +      'load' => 'Drupal\migrate_drupal\Plugin\MigrateLoadInterface',
   +      'cckfield' => 'Drupal\migrate_drupal\Plugin\MigrateCckFieldInterface',
   

   Afaik, we have to include the leading slash everywhere we use a class or interface name as a string.

2. +++ b/core/modules/migrate_drupal/src/MigrationStorage.php
   @@ -84,6 +98,10 @@ public function loadMultiple(array $ids = NULL) {
   +    // Allow modules providing cck field plugins to alter the required
   +    // migrations for successfully migration a field type.
   

   'successfully migration' sounds a bit weird :)

3. +++ b/core/modules/migrate_drupal/src/MigrationStorage.php
   @@ -113,4 +131,83 @@ public function save(EntityInterface $entity) {
   +   * @param array $entities
   +   *   An array of migration entities.
   ...
   +    /** @var \Drupal\migrate\Entity\Migration $migration */
   

   We can save the use of /** @var ... */ by using \Drupal\migrate\Entity\Migration[] instead of array in the docblock.

4. +++ b/core/modules/migrate_drupal/src/MigrationStorage.php
   @@ -113,4 +131,83 @@ public function save(EntityInterface $entity) {
   +        $cck_plugins = $this->getCckFieldPlugins();
   ...
   +        $plugins = $this->getCckFieldPlugins();
   

   Looks like we can move these at the top of the method to save some calls in the foreach.

5. +++ b/core/modules/migrate_drupal/src/MigrationStorage.php
   @@ -113,4 +131,83 @@ public function save(EntityInterface $entity) {
   +  protected function getMigrationPluginMethodMap() {
   +    return [
   +      'd6_field' => 'processField',
   +      'd6_field_instance_widget_settings' => 'processFieldWidget',
   +      'd6_field_formatter_settings' => 'processFieldDisplay',
   +    ];
   +  }
   

   Are we missing a d6_field_instance here?

6. +++ b/core/modules/migrate_drupal/src/Plugin/MigrateCckFieldInterface.php
   @@ -0,0 +1,60 @@
   +   * Apply any custom processing to the field display migration.
   ...
   +  public function processFieldDisplay(MigrationInterface $migration);
   ...
   +   * Get a map between D6 formatters and D8 formatters for this field type.
   ...
   +  public function getFieldDisplayMap();
   

   How about renaming everything to use 'formatter' instead of 'display'?

Also, related to the last point (#18.6), it seems that we're missing a getFieldWidgetMap() method :)

So, I looked at d6_field_instance_widget_settings and that seems to hardcode the 1:1 mapping for *known widgets*, but isn't it possible, at least theoretically, that a contrib module provided two widgets in D6 and combined them into a single one (with more options) in D8?

Awesome, looks good to me now :)

1. +++ b/core/modules/migrate_drupal/src/MigrationStorage.php
   @@ -10,6 +10,8 @@
   +use Drupal\migrate\Entity\MigrationInterface;
   

   Not used.

2. +++ b/core/modules/migrate_drupal/src/Plugin/MigrateCckFieldInterface.php
   @@ -0,0 +1,76 @@
   +  /**
   +   * Apply any custom processing to the cck bundle migrations.
   +   *
   +   * @param \Drupal\migrate\Entity\MigrationInterface $migration
   +   *   The migration entity.
   +   */
   +  public function processCckFieldValues(MigrationInterface $migration, $field_name, $data);
   

   Missing docblocks for $field_name and $data

Alex's concerns are addressed.

Re #23 I think the lack of explicit test coverage might come back to bite us but maybe when #2448501: Use the migrate cckfield plugin type for the link field lands it will be more obvious how to add explicit coverage.

Committed 04a17f8 and pushed to 8.0.x. Thanks!