diff --git a/core/modules/block/lib/Drupal/block/BlockBase.php b/core/modules/block/lib/Drupal/block/BlockBase.php index 6efda89..c743e5d 100644 --- a/core/modules/block/lib/Drupal/block/BlockBase.php +++ b/core/modules/block/lib/Drupal/block/BlockBase.php @@ -224,4 +224,12 @@ 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..aa93dc2 100644 --- a/core/modules/block/lib/Drupal/block/BlockFormController.php +++ b/core/modules/block/lib/Drupal/block/BlockFormController.php @@ -27,12 +27,29 @@ 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_base = $entity->getPlugin()->getMachineNameSuggestion(); + $block_ids = array_map(function ($block_id) { + $parts = explode('.', $block_id); + return end($parts); + }, array_keys(\Drupal::entityManager()->getStorageController('block')->load())); + + // Iterate through potential IDs until we get a new one. E.g. + // 'views_block__MYVIEW_PAGE_1_2' + $count = 1; + $machine_default = $machine_base; + while (in_array($machine_default, $block_ids)) { + $machine_default = $machine_base . '_' . ++$count; + } + } + $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(), + '#default_value' => $entity->id() ?: $machine_default, '#machine_name' => array( 'exists' => 'block_load', 'replace_pattern' => '[^a-z0-9_.]+', 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 e71a146..528cbad 100644 --- a/core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php +++ b/core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php @@ -96,5 +96,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. + $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..6c24cc3 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,18 @@ 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))); } + + /** + * Test 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.'); + } } 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 0bf5687..a0c477a 100644 --- a/core/modules/views/views.module +++ b/core/modules/views/views.module @@ -1743,11 +1743,6 @@ function views_form_block_form_alter(&$form, &$form_state) { 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; }