Issue #3331059 by quietone, Wongjn, ameymudras, smustgrave, xjm, alexpott,...

Issue #3331059 by quietone, Wongjn, ameymudras, smustgrave, xjm, alexpott, larowlan: Properly check for block content type in BlockPluginId process plugin

core/modules/block/src/Plugin/migrate/process/BlockPluginId.php

@@ -28,7 +28,7 @@ class BlockPluginId extends ProcessPluginBase implements ContainerFactoryPluginI
   /**
    * The block_content entity storage handler.
    *
-   * @var \Drupal\Core\Entity\EntityStorageInterface
+   * @var \Drupal\Core\Entity\EntityStorageInterface|null
    */
   protected $blockContentStorage;
 
@@ -41,12 +41,13 @@ class BlockPluginId extends ProcessPluginBase implements ContainerFactoryPluginI
    *   The plugin ID.
    * @param array $plugin_definition
    *   The plugin definition.
-   * @param \Drupal\Core\Entity\EntityStorageInterface $storage
-   *   The block content storage object.
+   * @param \Drupal\Core\Entity\EntityStorageInterface|null $storage
+   *   The block content storage object. NULL if the block_content module is
+   *   not installed.
    * @param \Drupal\migrate\MigrateLookupInterface $migrate_lookup
    *   The migrate lookup service.
    */
-  public function __construct(array $configuration, $plugin_id, array $plugin_definition, EntityStorageInterface $storage, MigrateLookupInterface $migrate_lookup) {
+  public function __construct(array $configuration, $plugin_id, array $plugin_definition, ?EntityStorageInterface $storage, MigrateLookupInterface $migrate_lookup) {
     parent::__construct($configuration, $plugin_id, $plugin_definition);
     $this->blockContentStorage = $storage;
     $this->migrateLookup = $migrate_lookup;
@@ -61,7 +62,7 @@ public static function create(ContainerInterface $container, array $configuratio
       $configuration,
       $plugin_id,
       $plugin_definition,
-      $entity_type_manager->getDefinition('block_content') ? $entity_type_manager->getStorage('block_content') : NULL,
+      $entity_type_manager->hasDefinition('block_content') ? $entity_type_manager->getStorage('block_content') : NULL,
       $container->get('migrate.lookup')
     );
   }

core/modules/block/tests/src/Kernel/Migrate/d7/MigrateBlockNoBlockContentTest.php

+<?php
+
+namespace Drupal\Tests\block\Kernel\Migrate\d7;
+
+use Drupal\block\Entity\Block;
+use Drupal\Tests\migrate_drupal\Kernel\d7\MigrateDrupal7TestBase;
+
+/**
+ * Tests the migration of blocks without Block Content installed.
+ *
+ * @group block
+ */
+class MigrateBlockNoBlockContentTest extends MigrateDrupal7TestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'block',
+    'views',
+    'comment',
+    'menu_ui',
+    'node',
+    'text',
+    'filter',
+    'path_alias',
+    'user',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+
+    // Install the themes used for this test.
+    $this->container->get('theme_installer')->install(['olivero', 'claro']);
+
+    $this->installConfig(static::$modules);
+
+    // Set Olivero and Claro as the default public and admin theme.
+    $config = $this->config('system.theme');
+    $config->set('default', 'olivero');
+    $config->set('admin', 'claro');
+    $config->save();
+
+    $this->executeMigrations([
+      'd7_filter_format',
+      'd7_user_role',
+      'd7_block',
+    ]);
+    block_rebuild();
+  }
+
+  /**
+   * Asserts various aspects of a block.
+   *
+   * @param string $id
+   *   The block ID.
+   * @param string $plugin_id
+   *   The block's plugin ID.
+   * @param array $roles
+   *   Role IDs the block is expected to have.
+   * @param string $pages
+   *   The list of pages on which the block should appear.
+   * @param string $region
+   *   The display region.
+   * @param string $theme
+   *   The theme.
+   * @param int $weight
+   *   The block weight.
+   * @param string $label
+   *   The block label.
+   * @param string $label_display
+   *   The block label display setting.
+   * @param bool $status
+   *   Whether the block is expected to be enabled or disabled.
+   *
+   * @internal
+   */
+  public function assertEntity(string $id, string $plugin_id, array $roles, string $pages, string $region, string $theme, int $weight, string $label, string $label_display, bool $status = TRUE): void {
+    $block = Block::load($id);
+    $this->assertInstanceOf(Block::class, $block);
+    /** @var \Drupal\block\BlockInterface $block */
+    $this->assertSame($plugin_id, $block->getPluginId());
+
+    $visibility = $block->getVisibility();
+    if ($roles) {
+      $this->assertSame($roles, array_values($visibility['user_role']['roles']));
+      $this->assertSame('@user.current_user_context:current_user', $visibility['user_role']['context_mapping']['user']);
+    }
+    if ($pages) {
+      $this->assertSame($pages, $visibility['request_path']['pages']);
+    }
+
+    $this->assertSame($region, $block->getRegion());
+    $this->assertSame($theme, $block->getTheme());
+    $this->assertSame($weight, $block->getWeight());
+    $this->assertSame($status, $block->status());
+
+    $config = $this->config('block.block.' . $id);
+    $this->assertSame($label, $config->get('settings.label'));
+    $this->assertSame($label_display, $config->get('settings.label_display'));
+  }
+
+  /**
+   * Tests the block migration.
+   */
+  public function testBlockMigration(): void {
+    $this->assertEntity('bartik_system_main', 'system_main_block', [], '', 'content', 'olivero', 0, '', '0');
+    $this->assertEntity('bartik_search_form', 'search_form_block', [], '', 'content', 'olivero', -1, '', '0');
+    $this->assertEntity('bartik_user_login', 'user_login_block', [], '', 'content', 'olivero', 0, 'User login title', 'visible');
+    $this->assertEntity('bartik_system_powered_by', 'system_powered_by_block', [], '', 'footer_bottom', 'olivero', 10, '', '0');
+    $this->assertEntity('seven_system_main', 'system_main_block', [], '', 'content', 'claro', 0, '', '0');
+    $this->assertEntity('seven_user_login', 'user_login_block', [], '', 'content', 'claro', 10, 'User login title', 'visible');
+
+    // Assert that disabled blocks (or enabled blocks whose plugin IDs could
+    // be resolved) did not migrate.
+    $non_existent_blocks = [
+      'bartik_system_navigation',
+      'bartik_system_help',
+      'seven_user_new',
+      'seven_search_form',
+      'bartik_comment_recent',
+      'bartik_node_syndicate',
+      'bartik_node_recent',
+      'bartik_shortcut_shortcuts',
+      'bartik_system_management',
+      'bartik_system_user-menu',
+      'bartik_system_main-menu',
+      'bartik_user_new',
+      'bartik_user_online',
+      'seven_comment_recent',
+      'seven_node_syndicate',
+      'seven_shortcut_shortcuts',
+      'seven_system_powered-by',
+      'seven_system_navigation',
+      'seven_system_management',
+      'seven_system_user-menu',
+      'seven_system_main-menu',
+      'seven_user_online',
+      'bartik_blog_recent',
+      'bartik_book_navigation',
+      'bartik_locale_language',
+      'bartik_forum_active',
+      'bartik_forum_new',
+      'seven_blog_recent',
+      'seven_book_navigation',
+      'seven_locale_language',
+      'seven_forum_active',
+      'seven_forum_new',
+      'bartik_menu_menu-test-menu',
+      'bartik_statistics_popular',
+      'seven_menu_menu-test-menu',
+      'seven_statistics_popular',
+      'seven_block_1',
+    ];
+    $this->assertEmpty(Block::loadMultiple($non_existent_blocks));
+  }
+
+}