diff --git a/core/modules/block/lib/Drupal/block/BlockBase.php b/core/modules/block/lib/Drupal/block/BlockBase.php index 6efda89..728c300 100644 --- a/core/modules/block/lib/Drupal/block/BlockBase.php +++ b/core/modules/block/lib/Drupal/block/BlockBase.php @@ -224,4 +224,13 @@ public function submit($form, &$form_state) { * @see \Drupal\block\BlockBase::submit() */ public function blockSubmit($form, &$form_state) {} + + /** + * {@inheritdoc} + */ + public function getMachineNameSuggestion() { + $definition = $this->getPluginDefinition(); + return preg_replace('/[^a-z0-9_.]+/', '', strtolower($definition['admin_label'])); + } + } diff --git a/core/modules/block/lib/Drupal/block/BlockFormController.php b/core/modules/block/lib/Drupal/block/BlockFormController.php index 828c5d4..bda25e0 100644 --- a/core/modules/block/lib/Drupal/block/BlockFormController.php +++ b/core/modules/block/lib/Drupal/block/BlockFormController.php @@ -7,13 +7,40 @@ namespace Drupal\block; +use Drupal\Core\Entity\EntityControllerInterface; use Drupal\Core\Entity\EntityFormController; +use Drupal\Core\Entity\EntityManager; use Drupal\Core\Language\Language; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Provides form controller for block instance forms. */ -class BlockFormController extends EntityFormController { +class BlockFormController extends EntityFormController implements EntityControllerInterface { + + /** + * The block storage controller. + * + * @var \Drupal\Core\Entity\EntityStorageControllerInterface + */ + protected $storageController; + + /** + * Constructs a BlockFormController object. + * + * @param \Drupal\Core\Entity\EntityManager $entity_manager + * The entity manager. + */ + public function __construct(EntityManager $entity_manager) { + $this->storageController = $entity_manager->getStorageController('block'); + } + + /** + * {@inheritdoc} + */ + public static function createInstance(ContainerInterface $container, $entity_type, array $entity_info) { + return new static($container->get('plugin.manager.entity')); + } /** * Overrides \Drupal\Core\Entity\EntityFormController::form(). @@ -27,12 +54,17 @@ public function form(array $form, array &$form_state) { ); $form['settings'] = $entity->getPlugin()->form(array(), $form_state); + // If creating a new block, calculate a safe default machine name. + if ($entity->isNew()) { + $machine_default = $this->getUniqueMachineName($entity); + } + $form['machine_name'] = array( '#type' => 'machine_name', '#title' => t('Machine name'), '#maxlength' => 64, - '#description' => t('A unique name to save this block configuration. Must be alpha-numeric and be underscore separated.'), - '#default_value' => $entity->id(), + '#description' => t('A unique name for this block instance. Must be alpha-numeric and underscore separated.'), + '#default_value' => $entity->id() ?: $machine_default, '#machine_name' => array( 'exists' => 'block_load', 'replace_pattern' => '[^a-z0-9_.]+', @@ -234,4 +266,30 @@ public function submit(array $form, array &$form_state) { $form_state['redirect'] = 'admin/structure/block/list/block_plugin_ui:' . $entity->get('theme'); } + /** + * Generate a unique machine name for a block. + * + * @param \Drupal\block\BlockInterface $block + * The block entity. + * + * @return string + * Returns the unique name. + */ + public function getUniqueMachineName(BlockInterface $block) { + $suggestion = $block->getPlugin()->getMachineNameSuggestion(); + $block_ids = array_map(function ($block_id) { + $parts = explode('.', $block_id); + return end($parts); + }, array_keys($this->storageController->load())); + + // Iterate through potential IDs until we get a new one. E.g. + // 'plugin', 'plugin_2', 'plugin_3'... + $count = 1; + $machine_default = $suggestion; + while (in_array($machine_default, $block_ids)) { + $machine_default = $suggestion . '_' . ++$count; + } + return $machine_default; + } + } diff --git a/core/modules/block/lib/Drupal/block/BlockPluginInterface.php b/core/modules/block/lib/Drupal/block/BlockPluginInterface.php index 1887816..76b638c 100644 --- a/core/modules/block/lib/Drupal/block/BlockPluginInterface.php +++ b/core/modules/block/lib/Drupal/block/BlockPluginInterface.php @@ -102,4 +102,15 @@ public function submit($form, &$form_state); */ public function build(); + /** + * Suggests a machine name to identify an instance of this block. + * + * The block plugin need not verify that the machine name is at all unique. It + * is only responsible for providing a baseline suggestion; calling code is + * responsible for ensuring whatever uniqueness is required for the use case. + * + * @return string + */ + public function getMachineNameSuggestion(); + } diff --git a/core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php b/core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php index cd070df..a9e47ec 100644 --- a/core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php +++ b/core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php @@ -91,5 +91,9 @@ public function testBlockInterface() { ); // Ensure the build array is proper. $this->assertIdentical($display_block->build(), $expected_build, 'The plugin returned the appropriate build array.'); + + // Ensure the machine name suggestion is correct. In truth, this is actually + // testing BlockBase's implementation, not the interface itself. + $this->assertIdentical($display_block->getMachineNameSuggestion(), 'displaymessage', 'The plugin returned the expected machine name suggestion.'); } } diff --git a/core/modules/block/lib/Drupal/block/Tests/BlockUiTest.php b/core/modules/block/lib/Drupal/block/Tests/BlockUiTest.php index cd9d955..7ad84e1 100644 --- a/core/modules/block/lib/Drupal/block/Tests/BlockUiTest.php +++ b/core/modules/block/lib/Drupal/block/Tests/BlockUiTest.php @@ -19,7 +19,7 @@ class BlockUiTest extends WebTestBase { * * @var array */ - public static $modules = array('block'); + public static $modules = array('block', 'block_test'); protected $regions; @@ -113,4 +113,24 @@ function testBlockSearch() { $blocks = drupal_json_decode($this->drupalGet('system/autocomplete/block_plugin_ui:stark', array('query' => array('q' => $block)))); $this->assertEqual($blocks['system_menu_block:menu-admin'], $block, t('Can search for block with name !block.', array('!block' => $block))); } + + /** + * Tests that the BlockFormController populates machine name correctly. + */ + public function testMachineNameSuggestion() { + $url = 'admin/structure/block/add/test_block_instantiation/stark'; + $this->drupalGet($url); + $this->assertFieldByName('machine_name', 'displaymessage', 'Block form uses raw machine name suggestion when no instance already exists.'); + $this->drupalPost($url, array(), 'Save block'); + + // Now, check to make sure the form starts by autoincrementing correctly. + $this->drupalGet($url); + $this->assertFieldByName('machine_name', 'displaymessage_2', 'Block form appends _2 to plugin-suggested machine name when an instance already exists.'); + $this->drupalPost($url, array(), 'Save block'); + + // And verify that it continues working beyond just the first two. + $this->drupalGet($url); + $this->assertFieldByName('machine_name', 'displaymessage_3', 'Block form appends _3 to plugin-suggested machine name when two instances already exist.'); + } + } diff --git a/core/modules/block/tests/lib/Drupal/block/Tests/BlockBaseTest.php b/core/modules/block/tests/lib/Drupal/block/Tests/BlockBaseTest.php new file mode 100644 index 0000000..00c33d4 --- /dev/null +++ b/core/modules/block/tests/lib/Drupal/block/Tests/BlockBaseTest.php @@ -0,0 +1,54 @@ + 'Base plugin', + 'description' => 'Tests the base block plugin.', + 'group' => 'Block', + ); + } + + /** + * Tests the machine name suggestion. + * + * @see \Drupal\block\BlockBase::getMachineNameSuggestion(). + */ + public function testGetMachineNameSuggestion() { + $config = array(); + $definition = array('admin_label' => 'Admin label', 'module' => 'block_test'); + $block_base = new TestBlockInstantiation($config, 'test_block_instantiation', $definition); + $this->assertEquals('adminlabel', $block_base->getMachineNameSuggestion()); + + // Test with more unicodes. + $definition = array('admin_label' =>'über åwesome', 'module' => 'block_test'); + $block_base = new TestBlockInstantiation($config, 'test_block_instantiation', $definition); + $this->assertEquals('berwesome', $block_base->getMachineNameSuggestion()); + } + +} diff --git a/core/modules/block/tests/lib/Drupal/block/Tests/BlockFormControllerTest.php b/core/modules/block/tests/lib/Drupal/block/Tests/BlockFormControllerTest.php new file mode 100644 index 0000000..af63279 --- /dev/null +++ b/core/modules/block/tests/lib/Drupal/block/Tests/BlockFormControllerTest.php @@ -0,0 +1,71 @@ + 'Block form controller', + 'description' => 'Tests the block form controller.', + 'group' => 'Block', + ); + } + + /** + * Tests the unique machine name generator. + * + * + * @see \Drupal\block\BlockFormController::getUniqueMachineName() + */ + public function testGetUniqueMachineName() { + $block_storage = $this->getMockBuilder('Drupal\block\BlockStorageController') + ->disableOriginalConstructor() + ->getMock(); + $blocks = array(); + + $blocks['test'] = $this->getBlockMockWithMachineName('test'); + $blocks['other_test'] = $this->getBlockMockWithMachineName('other_test'); + $blocks['other_test_1'] = $this->getBlockMockWithMachineName('other_test'); + $blocks['other_test_2'] = $this->getBlockMockWithMachineName('other_test'); + + $block_storage->expects($this->any()) + ->method('load') + ->will($this->returnValue($blocks)); + + $entity_manager = $this->getMockBuilder('Drupal\Core\Entity\EntityManager') + ->disableOriginalConstructor() + ->getMock(); + $entity_manager->expects($this->any()) + ->method('getStorageController') + ->will($this->returnValue($block_storage)); + $block_form_controller = new BlockFormController($entity_manager); + + $this->assertEquals('test_2', $block_form_controller->getUniqueMachineName($blocks['test'])); + $this->assertEquals('other_test_3', $block_form_controller->getUniqueMachineName($blocks['other_test'])); + $this->assertEquals('other_test_3', $block_form_controller->getUniqueMachineName($blocks['other_test_1'])); + $this->assertEquals('other_test_3', $block_form_controller->getUniqueMachineName($blocks['other_test_2'])); + + $last_block = $this->getBlockMockWithMachineName('last_test'); + $this->assertEquals('last_test', $block_form_controller->getUniqueMachineName($last_block)); + } + +} diff --git a/core/modules/block/tests/block_test.info.yml b/core/modules/block/tests/modules/block_test/block_test.info.yml similarity index 100% rename from core/modules/block/tests/block_test.info.yml rename to core/modules/block/tests/modules/block_test/block_test.info.yml diff --git a/core/modules/block/tests/block_test.module b/core/modules/block/tests/modules/block_test/block_test.module similarity index 100% rename from core/modules/block/tests/block_test.module rename to core/modules/block/tests/modules/block_test/block_test.module diff --git a/core/modules/block/tests/config/block.block.stark.test_block.yml b/core/modules/block/tests/modules/block_test/config/block.block.stark.test_block.yml similarity index 100% rename from core/modules/block/tests/config/block.block.stark.test_block.yml rename to core/modules/block/tests/modules/block_test/config/block.block.stark.test_block.yml diff --git a/core/modules/block/tests/lib/Drupal/block_test/Plugin/Block/TestBlockInstantiation.php b/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestBlockInstantiation.php similarity index 100% rename from core/modules/block/tests/lib/Drupal/block_test/Plugin/Block/TestBlockInstantiation.php rename to core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestBlockInstantiation.php diff --git a/core/modules/block/tests/lib/Drupal/block_test/Plugin/Block/TestCacheBlock.php b/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestCacheBlock.php similarity index 100% rename from core/modules/block/tests/lib/Drupal/block_test/Plugin/Block/TestCacheBlock.php rename to core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestCacheBlock.php diff --git a/core/modules/block/tests/lib/Drupal/block_test/Plugin/Block/TestHtmlIdBlock.php b/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlIdBlock.php similarity index 100% rename from core/modules/block/tests/lib/Drupal/block_test/Plugin/Block/TestHtmlIdBlock.php rename to core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlIdBlock.php diff --git a/core/modules/block/tests/lib/Drupal/block_test/Plugin/Block/TestXSSTitleBlock.php b/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestXSSTitleBlock.php similarity index 100% rename from core/modules/block/tests/lib/Drupal/block_test/Plugin/Block/TestXSSTitleBlock.php rename to core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestXSSTitleBlock.php diff --git a/core/modules/block/tests/themes/block_test_theme/block_test_theme.info.yml b/core/modules/block/tests/modules/block_test/themes/block_test_theme/block_test_theme.info.yml similarity index 100% rename from core/modules/block/tests/themes/block_test_theme/block_test_theme.info.yml rename to core/modules/block/tests/modules/block_test/themes/block_test_theme/block_test_theme.info.yml diff --git a/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php b/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php index 2c0481e..15a1fdc 100644 --- a/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php +++ b/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php @@ -114,33 +114,10 @@ protected function addContextualLinks(&$output, $block_type = 'block') { } /** - * Generates a views block instance ID. - * - * @param \Drupal\Core\Entity\EntityStorageControllerInterface $manager - * The block storage controller. - * - * @return string - * The new block instance ID. + * {@inheritdoc} */ - public function generateBlockInstanceID(EntityStorageControllerInterface $manager) { - $this->view->setDisplay($this->displayID); - $original_id = 'views_block__' . $this->view->storage->id() . '_' . $this->view->current_display; - - // Get an array of block IDs without the theme prefix. - $block_ids = array_map(function ($block_id) { - $parts = explode('.', $block_id); - return end($parts); - }, array_keys($manager->load())); - - // Iterate through potential IDs until we get a new one. E.g. - // 'views_block__MYVIEW_PAGE_1_2' - $count = 1; - $id = $original_id; - while (in_array($id, $block_ids)) { - $id = $original_id . '_' . ++$count; - } - - return $id; - } - + public function getMachineNameSuggestion() { + $this->view->setDisplay($this->displayID); + return 'views_block__' . $this->view->storage->id() . '_' . $this->view->current_display; + } } diff --git a/core/modules/views/lib/Drupal/views/Tests/Plugin/ViewsBlockTest.php b/core/modules/views/lib/Drupal/views/Tests/Plugin/ViewsBlockTest.php index 759351a..5139187 100644 --- a/core/modules/views/lib/Drupal/views/Tests/Plugin/ViewsBlockTest.php +++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/ViewsBlockTest.php @@ -34,7 +34,7 @@ class ViewsBlockTest extends ViewUnitTestBase { public static function getInfo() { return array( 'name' => 'Views block', - 'description' => 'Tests the block views plugin.', + 'description' => 'Tests native behaviors of the block views plugin.', 'group' => 'Views Plugins', ); } @@ -49,38 +49,18 @@ protected function setUp() { } /** - * Tests generateBlockInstanceID. + * Tests that ViewsBlock::getMachineNameSuggestion() produces the right value. * - * @see \Drupal\views\Plugin\Block::generateBlockInstanceID(). + * @see \Drupal\views\Plugin\Block::getmachineNameSuggestion(). */ - public function testGenerateBlockInstanceID() { - + public function testMachineNameSuggestion() { $plugin_definition = array( 'module' => 'views', ); $plugin_id = 'views_block:test_view_block-block_1'; $views_block = new ViewsBlock(array(), $plugin_id, $plugin_definition); - $storage_controller = $this->container->get('plugin.manager.entity')->getStorageController('block'); - - // Generate a instance ID on a block without any instances. - $this->assertEqual($views_block->generateBlockInstanceID($storage_controller), 'views_block__test_view_block_block_1'); - - $values = array( - 'plugin' => $plugin_id, - 'id' => 'stark.views_block__test_view_block_block_1', - 'module' => 'views', - 'settings' => array( - 'module' => 'views', - ) - ); - $storage_controller->create($values)->save(); - $this->assertEqual($views_block->generateBlockInstanceID($storage_controller), 'views_block__test_view_block_block_1_2'); - - // Add another one block instance and ensure the block instance went up. - $values['id'] = 'stark.views_block__test_view_block_block_1_2'; - $storage_controller->create($values)->save(); - $this->assertEqual($views_block->generateBlockInstanceID($storage_controller), 'views_block__test_view_block_block_1_3'); + $this->assertEqual($views_block->getMachineNameSuggestion(), 'views_block__test_view_block_block_1'); } } diff --git a/core/modules/views/views.module b/core/modules/views/views.module index 0e5922b..e7226a2 100644 --- a/core/modules/views/views.module +++ b/core/modules/views/views.module @@ -1663,25 +1663,3 @@ function views_cache_get($cid, $use_language = FALSE) { return cache('views_info')->get($cid); } -/** - * Implements hook_form_FORM_ID_alter for block_form(). - * - * Views overrides block configuration form elements during - * \Drupal\views\Plugin\Block\ViewsBlock::form() but machine_name assignment is - * added later by \Drupal\block\BlockFormController::form() so we provide an - * override for the block machine_name here. - */ -function views_form_block_form_alter(&$form, &$form_state) { - // Ensure the block-form being altered is a Views block configuration form. - if (($form['settings']['module']['#value'] == 'views') && empty($form['machine_name']['#default_value'])) { - // Unset the machine_name provided by BlockFormController. - unset($form['machine_name']['#machine_name']['source']); - // Load the Views plugin object using form_state array and create a - // block machine_name based on the View ID and View Display ID. - $block_plugin = $form_state['controller']->getEntity()->getPlugin(); - // Override the Views block's machine_name by providing a default_value. - $form['machine_name']['#default_value'] = $block_plugin->generateBlockInstanceID(Drupal::entityManager()->getStorageController('block')); - // Prevent users from changing the auto-generated block machine_name. - $form['machine_name']['#access'] = FALSE; - } -} diff --git a/core/tests/Drupal/Tests/UnitTestCase.php b/core/tests/Drupal/Tests/UnitTestCase.php index 1856011..af280e2 100644 --- a/core/tests/Drupal/Tests/UnitTestCase.php +++ b/core/tests/Drupal/Tests/UnitTestCase.php @@ -134,4 +134,30 @@ public function getConfigStorageStub(array $configs) { return $config_storage; } + /** + * Mocks a block with a block plugin. + * + * @param string $machine_name + * The machine name of the block plugin. + * + * @return \PHPUnit_Framework_MockObject_MockObject + * The mocked block. + */ + protected function getBlockMockWithMachineName($machine_name) { + $plugin = $this->getMockBuilder('Drupal\block\BlockBase') + ->disableOriginalConstructor() + ->getMock(); + $plugin->expects($this->any()) + ->method('getMachineNameSuggestion') + ->will($this->returnValue($machine_name)); + + $block = $this->getMockBuilder('Drupal\block\Plugin\Core\Entity\Block') + ->disableOriginalConstructor() + ->getMock(); + $block->expects($this->any()) + ->method('getPlugin') + ->will($this->returnValue($plugin)); + return $block; + } + }