summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Pott2013-08-03 23:28:28 +0200
committerAlex Pott2013-08-03 23:28:28 +0200
commit8c384bc415c675e5abe6a32eafa52f91309fea6e (patch)
tree531a10c8977b9a7f01a54e20558c0651f99136cc
parent1b5434a252b46f1909532edc2c3a2b786e09c00d (diff)
Issue #2022897 by dawehner, sdboyer, tim.plunkett: Shift responsibility for creating unique config id/machine name off of (views) block plugins and onto BlockFormController.
-rw-r--r--core/modules/block/lib/Drupal/block/BlockBase.php26
-rw-r--r--core/modules/block/lib/Drupal/block/BlockFormController.php90
-rw-r--r--core/modules/block/lib/Drupal/block/BlockPluginInterface.php12
-rw-r--r--core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php4
-rw-r--r--core/modules/block/lib/Drupal/block/Tests/BlockUiTest.php22
-rw-r--r--core/modules/block/tests/lib/Drupal/block/Tests/BlockBaseTest.php61
-rw-r--r--core/modules/block/tests/lib/Drupal/block/Tests/BlockFormControllerTest.php102
-rw-r--r--core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php9
-rw-r--r--core/modules/views/lib/Drupal/views/Tests/Plugin/ViewsBlockTest.php30
-rw-r--r--core/modules/views/views.module22
-rw-r--r--core/tests/Drupal/Tests/UnitTestCase.php26
11 files changed, 353 insertions, 51 deletions
diff --git a/core/modules/block/lib/Drupal/block/BlockBase.php b/core/modules/block/lib/Drupal/block/BlockBase.php
index a8e4ed1..f1217c0 100644
--- a/core/modules/block/lib/Drupal/block/BlockBase.php
+++ b/core/modules/block/lib/Drupal/block/BlockBase.php
@@ -9,6 +9,8 @@ namespace Drupal\block;
use Drupal\Component\Plugin\PluginBase;
use Drupal\block\BlockInterface;
+use Drupal\Component\Utility\Unicode;
+use Drupal\Core\Language\Language;
/**
* Defines a base block implementation that most blocks plugins will extend.
@@ -165,4 +167,28 @@ abstract class BlockBase extends PluginBase implements BlockPluginInterface {
*/
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 what is 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 7789c02..0386277 100644
--- a/core/modules/block/lib/Drupal/block/BlockFormController.php
+++ b/core/modules/block/lib/Drupal/block/BlockFormController.php
@@ -7,13 +7,60 @@
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\Extension\ModuleHandlerInterface;
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\Extension\ModuleHandlerInterface
+ * The module handler service.
+ * @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(ModuleHandlerInterface $module_handler, EntityManager $entity_manager, QueryFactory $entity_query_factory) {
+ parent::__construct($module_handler);
+
+ $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('module_handler'),
+ $container->get('plugin.manager.entity'),
+ $container->get('entity.query')
+ );
+ }
/**
* Overrides \Drupal\Core\Entity\EntityFormController::form().
@@ -27,12 +74,17 @@ class BlockFormController extends EntityFormController {
);
$form['settings'] = $entity->getPlugin()->buildConfigurationForm(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 +286,36 @@ class BlockFormController extends EntityFormController {
$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 c7368ae..5f353a4 100644
--- a/core/modules/block/lib/Drupal/block/BlockPluginInterface.php
+++ b/core/modules/block/lib/Drupal/block/BlockPluginInterface.php
@@ -122,4 +122,16 @@ interface BlockPluginInterface extends ConfigurablePluginInterface, PluginFormIn
*/
public function blockSubmit($form, &$form_state);
+ /**
+ * 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 1c73cc2..5405989 100644
--- a/core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php
+++ b/core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php
@@ -92,5 +92,9 @@ class BlockInterfaceTest extends DrupalUnitTestBase {
);
// 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 @@ class BlockUiTest extends WebTestBase {
$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..085343e
--- /dev/null
+++ b/core/modules/block/tests/lib/Drupal/block/Tests/BlockBaseTest.php
@@ -0,0 +1,61 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\block\BlockBaseTest.
+ */
+
+namespace Drupal\Tests\block;
+
+use Drupal\block_test\Plugin\Block\TestBlockInstantiation;
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\Transliteration\PHPTransliteration;
+use Drupal\Tests\UnitTestCase;
+
+// @todo Remove once the constants are replaced with constants on classes.
+if (!defined('DRUPAL_NO_CACHE')) {
+ define('DRUPAL_NO_CACHE', -1);
+}
+
+/**
+ * Tests the base block plugin.
+ *
+ * @see \Drupal\block\BlockBase
+ */
+class BlockBaseTest extends UnitTestCase {
+
+ public static function getInfo() {
+ return array(
+ 'name' => '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..e57e877
--- /dev/null
+++ b/core/modules/block/tests/lib/Drupal/block/Tests/BlockFormControllerTest.php
@@ -0,0 +1,102 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\block\BlockFormControllerTest.
+ */
+
+namespace Drupal\Tests\block;
+
+use Drupal\block\BlockFormController;
+use Drupal\Tests\UnitTestCase;
+
+// @todo Remove once the constants are replaced with constants on classes.
+if (!defined('BLOCK_REGION_NONE')) {
+ define('BLOCK_REGION_NONE', -1);
+}
+
+// @todo Remove once the constants are replaced with constants on classes.
+if (!defined('FIELD_LOAD_CURRENT')) {
+ define('FIELD_LOAD_CURRENT', 'FIELD_LOAD_CURRENT');
+}
+
+/**
+ * Tests the block form controller.
+ *
+ * @see \Drupal\block\BlockFormController
+ */
+class BlockFormControllerTest extends UnitTestCase {
+
+ public static function getInfo() {
+ return array(
+ 'name' => '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));
+
+ $module_handler = $this->getMockBuilder('Drupal\Core\Extension\ModuleHandlerInterface')
+ ->getMock();
+
+ $block_form_controller = new BlockFormController($module_handler, $entity_manager, $query_factory);
+
+ // Ensure that the block with just one other instance gets the next available
+ // name suggestion.
+ $this->assertEquals('test_2', $block_form_controller->getUniqueMachineName($blocks['test']));
+
+ // Ensure that the block with already three instances (_0, _1, _2) gets the
+ // 4th available name.
+ $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']));
+
+ // Ensure that a block without an instance yet gets the suggestion as
+ // unique machine name.
+ $last_block = $this->getBlockMockWithMachineName('last_test');
+ $this->assertEquals('last_test', $block_form_controller->getUniqueMachineName($last_block));
+ }
+
+}
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 9df4a18..c749a19 100644
--- a/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php
+++ b/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php
@@ -218,4 +218,13 @@ class ViewsBlock extends BlockBase implements ContainerFactoryPluginInterface {
return $id;
}
+
+ /**
+ * {@inheritdoc}
+ */
+ 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 b427262..f1a0563 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 @@ class ViewsBlockTest extends ViewUnitTestBase {
}
/**
- * 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 = ViewsBlock::create($this->container, 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 a70f4e4..bb312ff 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 727976f..d4bd53f 100644
--- a/core/tests/Drupal/Tests/UnitTestCase.php
+++ b/core/tests/Drupal/Tests/UnitTestCase.php
@@ -121,4 +121,30 @@ abstract class UnitTestCase extends \PHPUnit_Framework_TestCase {
return $config_storage;
}
+ /**
+ * Mocks a block with a block plugin.
+ *
+ * @param string $machine_name
+ * The machine name of the block plugin.
+ *
+ * @return \Drupal\block\BlockInterface|\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;
+ }
+
}