diff --git a/core/modules/block/lib/Drupal/block/BlockBase.php b/core/modules/block/lib/Drupal/block/BlockBase.php index 6efda89..56d917e 100644 --- a/core/modules/block/lib/Drupal/block/BlockBase.php +++ b/core/modules/block/lib/Drupal/block/BlockBase.php @@ -8,6 +8,8 @@ namespace Drupal\block; use Drupal\Component\Plugin\PluginBase; +use Drupal\Component\Utility\Unicode; +use Drupal\Core\Language\Language; /** * Defines a base block implementation that most blocks plugins will extend. @@ -224,4 +226,29 @@ public function submit($form, &$form_state) { * @see \Drupal\block\BlockBase::submit() */ public function blockSubmit($form, &$form_state) {} + + /** + * {@inheritdoc} + */ + public function getMachineNameSuggestion() { + $definition = $this->getPluginDefinition(); + $admin_label = $definition['admin_label']; + + // @todo This is basically the same as done in + // \Drupal\system\MachineNameController::transliterate(), so it might make + // sense to provide a common service for the two. + $transliteration_service = \Drupal::service('transliteration'); + $transliterated = $transliteration_service->transliterate($admin_label, Language::LANGCODE_DEFAULT, '_'); + + $replace_pattern = '[^a-z0-9_.]+'; + + $transliterated = Unicode::strtolower($transliterated); + + if (isset($replace_pattern)) { + $transliterated = preg_replace('@' . $replace_pattern . '@', '', $transliterated); + } + + return $transliterated; + } + } diff --git a/core/modules/block/lib/Drupal/block/BlockFormController.php b/core/modules/block/lib/Drupal/block/BlockFormController.php index 828c5d4..69a1a31 100644 --- a/core/modules/block/lib/Drupal/block/BlockFormController.php +++ b/core/modules/block/lib/Drupal/block/BlockFormController.php @@ -7,13 +7,54 @@ namespace Drupal\block; +use Drupal\Core\Entity\EntityControllerInterface; use Drupal\Core\Entity\EntityFormController; +use Drupal\Core\Entity\EntityManager; +use Drupal\Core\Entity\Query\QueryFactory; 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; + + /** + * The entity query factory. + * + * @var \Drupal\Core\Entity\Query\QueryFactory + */ + protected $entityQueryFactory; + + /** + * Constructs a BlockFormController object. + * + * @param \Drupal\Core\Entity\EntityManager $entity_manager + * The entity manager. + * @param \Drupal\Core\Entity\Query\QueryFactory $entity_query_factory + * The entity query factory. + */ + public function __construct(EntityManager $entity_manager, QueryFactory $entity_query_factory) { + $this->storageController = $entity_manager->getStorageController('block'); + $this->entityQueryFactory = $entity_query_factory; + } + + /** + * {@inheritdoc} + */ + public static function createInstance(ContainerInterface $container, $entity_type, array $entity_info) { + return new static( + $container->get('plugin.manager.entity'), + $container->get('entity.query') + ); + } /** * Overrides \Drupal\Core\Entity\EntityFormController::form(). @@ -27,12 +68,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->isNew() ? $entity->id() : $machine_default, '#machine_name' => array( 'exists' => 'block_load', 'replace_pattern' => '[^a-z0-9_.]+', @@ -234,4 +280,36 @@ public function submit(array $form, array &$form_state) { $form_state['redirect'] = 'admin/structure/block/list/block_plugin_ui:' . $entity->get('theme'); } + /** + * Generates 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(); + + // Get all the blocks which starts with the suggested machine name. + $query = $this->entityQueryFactory->get('block'); + $query->condition('id', $suggestion, 'CONTAINS'); + $block_ids = $query->execute(); + + $block_ids = array_map(function ($block_id) { + $parts = explode('.', $block_id); + return end($parts); + }, $block_ids); + + // 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..013d349 100644 --- a/core/modules/block/lib/Drupal/block/BlockPluginInterface.php +++ b/core/modules/block/lib/Drupal/block/BlockPluginInterface.php @@ -102,4 +102,16 @@ 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 + * The suggested machine name. + */ + 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..3199f12 --- /dev/null +++ b/core/modules/block/tests/lib/Drupal/block/Tests/BlockBaseTest.php @@ -0,0 +1,65 @@ + 'Base plugin', + 'description' => 'Tests the base block plugin.', + 'group' => 'Block', + ); + } + + /** + * Tests the machine name suggestion. + * + * @see \Drupal\block\BlockBase::getMachineNameSuggestion(). + */ + public function testGetMachineNameSuggestion() { + $transliteraton = $this->getMockBuilder('Drupal\Core\Transliteration\PHPTransliteration') + // @todo Inject the module handler into PHPTransliteration. + ->setMethods(array('readLanguageOverrides')) + ->getMock(); + + $container = new ContainerBuilder(); + $container->set('transliteration', $transliteraton); + \Drupal::setContainer($container); + + $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('uberawesome', $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..9c7edd2 --- /dev/null +++ b/core/modules/block/tests/lib/Drupal/block/Tests/BlockFormControllerTest.php @@ -0,0 +1,92 @@ + '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'); + + $query = $this->getMockBuilder('Drupal\Core\Entity\Query\QueryInterface') + ->disableOriginalConstructor() + ->getMock(); + $query->expects($this->exactly(5)) + ->method('condition') + ->will($this->returnValue($query)); + + $query->expects($this->exactly(5)) + ->method('execute') + ->will($this->returnValue(array('test', 'other_test', 'other_test_1', 'other_test_2'))); + + $query_factory = $this->getMockBuilder('Drupal\Core\Entity\Query\QueryFactory') + ->disableOriginalConstructor() + ->getMock(); + $query_factory->expects($this->exactly(5)) + ->method('get') + ->with('block', 'AND') + ->will($this->returnValue($query)); + + $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, $query_factory); + + $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 270ed26..b5641b0 100644 --- a/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php +++ b/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php @@ -112,33 +112,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 5daa331..47bb1e4 100644 --- a/core/modules/views/views.module +++ b/core/modules/views/views.module @@ -1665,25 +1665,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; + } + }