Create Drupal 7 media_wysiwyg → media_embed (Drupal 8/9 Media Library) token transform

Awesome work here 🥳

1. +++ b/src/MediaWysiwygPluginBase.php
   @@ -57,14 +102,20 @@ abstract class MediaWysiwygPluginBase extends PluginBase implements MediaWysiwyg
   +      'token_type' => 'media_library',
   

   Hm, is media_library a "token type"? :O I have no idea what this refers to 🙈

2. +++ b/src/MigratePluginAlterer.php
   @@ -177,7 +177,14 @@ class MigratePluginAlterer {
   +   * Maps Drupal 7 embed media filter plugin to a Drupal 8 filter plugin.
   

   s/embed media filter/media_filter/

3. +++ b/src/Plugin/migrate/process/FilterSettingsEmbedMedia.php
   @@ -47,14 +46,10 @@ class FilterSettingsEmbedMedia extends ProcessPluginBase {
   -          // @todo Handle media_embed filter's required markup as well.
   +          $entity_to_add = '<drupal-media data-* alt title>';
   

   🥳

   But let's not use data-*, let's use data-entity-type data-entity-uuid data-view-mode data-align data-caption.

I basically have only nits and a few small questions — really impressive work here!

I believe this infrastructure — and especially the test infrastructure — paves the path for many more <some D7 module that does something with media> to migrate into D8/9 🥳🥳

1. +++ b/README.md
   @@ -10,29 +10,55 @@ It performs the following:
   +  If this setting is not defined, then the destination filter plugin ID defaults
   +  to `entity_embed` for BC reasons. This also happens when you're on Drupal core
   +  8.7.x, becasue the `media_embed` filter plugin was introduced in 8.8.x.
   +
   +* By default (for BC reasons), after the embedded media token transform, the
   +  entities are referenced by their ID. This can be changed to UUID by adding the
   +  following to `settings.php`:
   

   These two BC choices make sense for existing media_migration users 👍

2. +++ b/README.md
   @@ -10,29 +10,55 @@ It performs the following:
   +  This reference method configuration is evaluated only if the destination
   +  filter is `entity_embed`, since the `media_embed` filter plugin can refer
   +  media entities only by their UUID.
   

   👍 — you preemptively answered a question I was going to ask! :D

3. +++ b/README.md
   @@ -10,29 +10,55 @@ It performs the following:
   +  Media UUIDs. We are convinced that the file's UUID belongs to the file, and we
   +  don't want to use the same UUID for two different entities.
   

   OOOOOOOOOHHHHHHHHHHHHHH! 😍👏

4. +++ b/README.md
   @@ -10,29 +10,55 @@ It performs the following:
   +You also have to create and/or configure the custom media view modes that the
   +source site used BEFORE running the migration.
   

   Shouldn't this also specify "with the same machine names"?

5. +++ b/media_migration.install
   @@ -32,3 +33,59 @@ function media_migration_schema() {
   +      $required_modules_info = new PluralTranslatableMarkup(count($missing_modules), 'The following module is missing: @module.', 'The following modules are missing: @module.', [
   +        '@module' => implode(', ', $missing_modules),
   +      ]);
   

   Eye for detail! 👏

6. +++ b/migrations/d7_media_view_modes.yml
   @@ -0,0 +1,31 @@
   +  label_fallback:
   +    plugin: static_map
   +    source: mode
   +    bypass: true
   +    map:
   +      full: "Full content"
   +      search_index: "Search index"
   +      search_result: "Search result"
   +      rss: "RSS"
   +      teaser: "Teaser"
   +      wysiwyg: "WYSIWYG"
   +  label:
   +    plugin: media_migration_null_coalesce
   +    source:
   +      - label
   +      - "@label_fallback"
   +      - mode
   

   Again, eye for detail! 👍

7. +++ b/src/MediaMigration.php
   @@ -50,8 +64,48 @@ final class MediaMigration {
   +  /**
   +   * Default embedded media reference method.
   +   *
   +   * @var string
   +   */
   +  const MEDIA_TOKEN_DESTINATION_FILTER_DEFAULT = self::MEDIA_TOKEN_DESTINATION_FILTER_ENTITY_EMBED;
   

   I think the comment here should say that MEDIA_TOKEN_DESTINATION_FILTER_MEDIA_EMBED would actually be the correct default, but doing that would cause a BC break in the media_migration module.

8. +++ b/src/MediaMigration.php
   @@ -50,8 +64,48 @@ final class MediaMigration {
   +  const MEDIA_TOKEN_DESTINATION_FILTER_REQUIREMENTS = [
   +    self::MEDIA_TOKEN_DESTINATION_FILTER_ENTITY_EMBED => [
   +      'entity_embed',
   +    ],
   +    self::MEDIA_TOKEN_DESTINATION_FILTER_MEDIA_EMBED => [],
      ];
   

   Why isn't media listed there?

   Yes, media is part of core and hence always exists but it is not always installed?

9. +++ b/src/MigratePluginAlterer.php
   @@ -192,41 +187,45 @@ class MigratePluginAlterer {
   +   * maps the media_embed filter plugin (Drupal 7 Media WYSIWYG module) to
   ...
   +   * If Entity Embed is unavailable, the media_embed filter will be mapped to
   

   🐛media_filter → media_wysiwyg

10. +++ b/src/MigratePluginAlterer.php
    @@ -192,41 +187,45 @@ class MigratePluginAlterer {
    +   * media_embed filter (from core Media Library module) even if Media Library
    

    🐛 Media Library → Media

11. +++ b/src/MigratePluginAlterer.php
    @@ -192,41 +187,45 @@ class MigratePluginAlterer {
    +   * is uninstalled.
    

    Nit: s/uninstalled/not installed/ (subtle difference in meaning: "uninstalled" implies that it was previously installed and now not anymore).

12. +++ b/src/MigratePluginAlterer.php
    @@ -192,41 +187,45 @@ class MigratePluginAlterer {
    +    // tag's list.
    

    Nit: s/tag's/tags/

13. +++ b/src/Plugin/migrate/process/FilterSettingsEmbedMedia.php
    @@ -29,26 +29,23 @@ class FilterSettingsEmbedMedia extends ProcessPluginBase {
    +        default:
    +          // Nothing to do.
    +          return $value;
    

    Shouldn't this throw an an exception? When can this case occur?

14. +++ b/src/Plugin/migrate/process/MediaWysiwygFilter.php
    @@ -204,55 +210,39 @@ TEMPLATE;
    +        // Add alignment.
    +        if (!empty($source_attributes['class']) && is_string($source_attributes['class'])) {
    +          $alignment_map = [
    +            'media-wysiwyg-align-center' => 'center',
    +            'media-wysiwyg-align-left' => 'left',
    +            'media-wysiwyg-align-right' => 'right',
    +          ];
    +          $classes_array = array_unique(explode(' ', preg_replace('/\s{2,}/', ' ', trim($source_attributes['class']))));
    +
    +          foreach ($alignment_map as $original => $replacement) {
    +            if (in_array($original, $classes_array, TRUE)) {
    +              $embed_metadata['data-align'] = $replacement;
    +              break;
                 }
    

    😍 Woah, I had no idea you were tackling this too!

15. +++ b/src/Plugin/migrate/process/MediaWysiwygFilter.php
    @@ -328,4 +318,113 @@ TEMPLATE;
    +        // We cannot replace token.
    

    This comment doesn't seem to add anything — let's remove it.

16. +++ b/src/Plugin/migrate/process/MediaWysiwygFilter.php
    @@ -328,4 +318,113 @@ TEMPLATE;
    +        throw new \LogicException("Media Migrate cannot replace a media_wysiwyg in a content entity, since there aren't any available entity_embed display plugins.");
    

    s/a media_wysiwyg/a media_wysiwyg token/

17. +++ b/src/Plugin/migrate/process/MediaWysiwygFilter.php
    @@ -328,4 +318,113 @@ TEMPLATE;
    +      if (!isset($available_plugins[$display_plugin_id])) {
    +        // If the preselected display plugin does not exist, then we will
    +        // try to map it to 'view_mode:media.full'.
    +        if (isset($available_plugins['view_mode:media.full'])) {
    +          $display_plugin_id = 'view_mode:media.full';
    +        }
    +        // If 'view_mode:media.full' is also missing, then we try to pick
    +        // the first 'view_mode:media.[any]' derivative.
    +        else {
    +          $view_mode_plugins = array_reduce(array_keys($available_plugins), function ($carry, $plugin_id) {
    +            if (strpos($plugin_id, 'view_mode:media.') === 0) {
    +              $carry[$plugin_id] = $plugin_id;
    +            }
    +            return $carry;
    +          });
    +
    +          // If we have 'view_mode:media.[any]', we use the first one; if
    +          // not, then use the first display plugin.
    +          $display_plugin_id = !empty($view_mode_plugins) ? reset($view_mode_plugins) : array_keys($available_plugins)[0];
    +        }
    +      }
    +    }
    +
    

    Impressive robustness! 💯

18. +++ b/src/Plugin/migrate/process/MediaWysiwygFilter.php
    @@ -328,4 +318,113 @@ TEMPLATE;
    +    // Alt, title, caption and align should be handled conditionally.
    +    $conditional_attributes = ['alt', 'title', 'data-caption', 'data-align'];
    +    foreach ($conditional_attributes as $conditional_attribute) {
    +      if (!empty($embed_metadata[$conditional_attribute])) {
    +        $attributes[$conditional_attribute] = $embed_metadata[$conditional_attribute];
    +      }
    +    }
    +
    

    👏

19. +++ b/src/Plugin/migrate/source/d7/MediaViewModes.php
    @@ -0,0 +1,118 @@
    +class MediaViewModes extends DrupalSqlBase {
    

    Why is this classname plural?

20. +++ b/src/Plugin/migrate/source/d7/MediaViewModes.php
    @@ -0,0 +1,118 @@
    +    if ($this->moduleExists('file_entity')) {
    +      $rows[] = [
    +        'mode' => 'full',
    +        'label' => $this->getMediaViewModeLabel('full'),
    +      ];
    

    This seems wrong? Why would the full view mode for Media only exist if file_entity exists?

    Oh, to migrate it.

    But how could a full view mode for file_entity be migrated from 7 to 8's media? 🤔

21. +++ b/tests/src/Functional/MigrateEmbedMediaTokenTestBase.php
    @@ -0,0 +1,109 @@
    +abstract class MigrateEmbedMediaTokenTestBase extends MigrateMediaTestBase {
    

    🥳

22. +++ b/tests/src/Functional/MigrateEmbedMediaTokenToEntityEmbedTest.php
    @@ -36,65 +43,35 @@ class MigrateEmbedMediaTokenToEntityEmbedTest extends MigrateMediaTestBase {
    +   *   The embed token transformation's destination format_plugin ID to write
    

    Übernit: s/format_plugin/format plugin/

23. +++ b/tests/src/Functional/MigrateEmbedMediaTokenToEntityEmbedTest.php
    @@ -36,65 +43,35 @@ class MigrateEmbedMediaTokenToEntityEmbedTest extends MigrateMediaTestBase {
    +    // Delete preexisting embed_button config entities. We don't want to depend
    +    // on Entity Embed's default embed_button config entities since those can
    

    Can you explain this in some more detail?

24. +++ b/tests/src/Functional/MigrateEmbedMediaTokenToEntityEmbedTest.php
    @@ -36,65 +43,35 @@ class MigrateEmbedMediaTokenToEntityEmbedTest extends MigrateMediaTestBase {
    +    // be change in the future.
    

    Nit: s/change/changed/

25. +++ b/tests/src/Functional/MigrateEmbedMediaTokenToEntityEmbedTest.php
    @@ -104,9 +81,48 @@ class MigrateEmbedMediaTokenToEntityEmbedTest extends MigrateMediaTestBase {
         return [
    -      ['id'],
    -      ['uuid'],
    +      // ID reference method, non-defined destination format plugin ID.
    +      [
    +        'reference_method' => 'id',
    +        'destination_filter' => NULL,
    +        'expected_embed_html_attributes' => [
    +          0 => ['data-entity-id' => '1'] + $default_attributes,
    +        ],
    +      ],
    +      // ID reference method, 'entity_embed' destination format plugin ID.
    +      [
    +        'reference_method' => 'id',
    +        'destination_filter' => 'entity_embed',
    +        'expected_embed_html_attributes' => [
    +          0 => ['data-entity-id' => '1'] + $default_attributes,
    +        ],
    +      ],
    +      // 'UUID' reference method, non-defined destination format plugin ID.
    +      [
    +        'reference_method' => 'uuid',
    +        'destination_filter' => NULL,
    +        'expected_embed_html_attributes' => [
    +          0 => ['data-entity-uuid' => TRUE] + $default_attributes,
    +        ],
    +      ],
    +      // 'UUID' reference method, 'entity_embed' destination format plugin ID.
    +      [
    +        'reference_method' => 'uuid',
    +        'destination_filter' => 'entity_embed',
    +        'expected_embed_html_attributes' => [
    +          0 => ['data-entity-uuid' => TRUE] + $default_attributes,
    +        ],
    +      ],
         ];
    

    BEAUTIFUL 😍

26. +++ b/tests/src/Functional/MigrateMediaTestBase.php
    @@ -244,7 +396,10 @@ abstract class MigrateMediaTestBase extends MigrateUpgradeTestBase {
    +    // Have to reset all the statics after migration to ensure entities are
    +    // loadable.
    +    $this->resetAll();
    

    👍 for this comment!

Re #17.13: that default case is unreachable because I throw an exception above if the format is not valid. So I removed that :)

👍

However, I kept #17.20:

👍

+++ b/tests/src/Functional/MigrateEmbedMediaTokenToEntityEmbedTest.php
@@ -59,9 +59,12 @@ class MigrateEmbedMediaTokenToEntityEmbedTest extends MigrateEmbedMediaTokenTest
+    // Entity Embed module has an optional 'node' embed button that is installed
+    // when node module is enabled. We don't want to depend on Entity Embed's
+    // default embed_button config entities since those can be changed in the
+    // future. But we need to assert that the 'media' embed button (that we
+    // migrate conditionally) exists after the migration.

💯