summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwebchick2014-05-06 20:33:23 (GMT)
committerwebchick2014-05-06 20:33:23 (GMT)
commitc056f6dfe6b8544c25964eaee9cb4a087591e4ee (patch)
tree00bc972e732437b5b9ed1f53d77cf35a3a71e67e
parent519e01c79087db4392a221d3c88efda14795e6c1 (diff)
Issue #2241827 by killua99, dawehner, djevans | Crell: Using a numeric placeholder in paths in Views UI causes fatal error.
-rw-r--r--core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.php9
-rw-r--r--core/modules/rest/tests/Drupal/rest/Tests/CollectRoutesTest.php3
-rw-r--r--core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php61
-rw-r--r--core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php10
-rw-r--r--core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayPath.php23
-rw-r--r--core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewListBuilderTest.php3
6 files changed, 100 insertions, 9 deletions
diff --git a/core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.php b/core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.php
index b28b477..c6055e2 100644
--- a/core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.php
+++ b/core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.php
@@ -7,7 +7,7 @@
namespace Drupal\rest\Plugin\views\display;
-
+use Drupal\Core\Form\FormBuilderInterface;
use Drupal\Core\State\StateInterface;
use Drupal\Core\Routing\RouteProviderInterface;
use Drupal\Core\ContentNegotiation;
@@ -99,13 +99,15 @@ class RestExport extends PathPluginBase {
* The route provider
* @param \Drupal\Core\State\StateInterface $state
* The state key value store.
+ * @param \Drupal\Core\Form\FormBuilderInterface $form_error
+ * The form builder.
* @param \Drupal\Core\ContentNegotiation $content_negotiation
* The content negotiation library.
* @param \Symfony\Component\HttpFoundation\Request $request
* The request object.
*/
- public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state, ContentNegotiation $content_negotiation, Request $request) {
- parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider, $state);
+ public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state, FormBuilderInterface $form_error, ContentNegotiation $content_negotiation, Request $request) {
+ parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider, $state, $form_error);
$this->contentNegotiation = $content_negotiation;
$this->request = $request;
}
@@ -120,6 +122,7 @@ class RestExport extends PathPluginBase {
$plugin_definition,
$container->get('router.route_provider'),
$container->get('state'),
+ $container->get('form_builder'),
$container->get('content_negotiation'),
$container->get('request')
);
diff --git a/core/modules/rest/tests/Drupal/rest/Tests/CollectRoutesTest.php b/core/modules/rest/tests/Drupal/rest/Tests/CollectRoutesTest.php
index 5b78e76..db0e724 100644
--- a/core/modules/rest/tests/Drupal/rest/Tests/CollectRoutesTest.php
+++ b/core/modules/rest/tests/Drupal/rest/Tests/CollectRoutesTest.php
@@ -90,6 +90,9 @@ class CollectRoutesTest extends UnitTestCase {
->getMock();
$container->set('plugin.manager.views.style', $style_manager);
+ $form_builder = $this->getMock('Drupal\Core\Form\FormBuilderInterface');
+ $container->set('form_builder', $form_builder);
+
\Drupal::setContainer($container);
$this->restExport = RestExport::create($container, array(), "test_routes", array());
diff --git a/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php
index bf4e008..baa7773 100644
--- a/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php
@@ -7,6 +7,7 @@
namespace Drupal\views\Plugin\views\display;
+use Drupal\Core\Form\FormErrorInterface;
use Drupal\Core\State\StateInterface;
use Drupal\Core\Routing\RouteCompiler;
use Drupal\Core\Routing\RouteProviderInterface;
@@ -40,6 +41,13 @@ abstract class PathPluginBase extends DisplayPluginBase implements DisplayRouter
protected $state;
/**
+ * The form error helper.
+ *
+ * @var \Drupal\Core\Form\FormErrorInterface
+ */
+ protected $formError;
+
+ /**
* Constructs a PathPluginBase object.
*
* @param array $configuration
@@ -52,12 +60,15 @@ abstract class PathPluginBase extends DisplayPluginBase implements DisplayRouter
* The route provider.
* @param \Drupal\Core\State\StateInterface $state
* The state key value store.
+ * @param \Drupal\Core\Form\FormErrorInterface $form_error
+ * The form error helper.
*/
- public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state) {
+ public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state, FormErrorInterface $form_error) {
parent::__construct($configuration, $plugin_id, $plugin_definition);
$this->routeProvider = $route_provider;
$this->state = $state;
+ $this->formError = $form_error;
}
/**
@@ -69,7 +80,8 @@ abstract class PathPluginBase extends DisplayPluginBase implements DisplayRouter
$plugin_id,
$plugin_definition,
$container->get('router.route_provider'),
- $container->get('state')
+ $container->get('state'),
+ $container->get('form_builder')
);
}
@@ -395,8 +407,9 @@ abstract class PathPluginBase extends DisplayPluginBase implements DisplayRouter
parent::validateOptionsForm($form, $form_state);
if ($form_state['section'] == 'path') {
- if (strpos($form_state['values']['path'], '%') === 0) {
- form_error($form['path'], $form_state, t('"%" may not be used for the first segment of a path.'));
+ $errors = $this->validatePath($form_state['values']['path']);
+ foreach ($errors as $error) {
+ $this->formError->setError($form['path'], $form_state, $error);
}
// Automatically remove '/' and trailing whitespace from path.
@@ -415,4 +428,44 @@ abstract class PathPluginBase extends DisplayPluginBase implements DisplayRouter
}
}
+ /**
+ * Validates the path of the display.
+ *
+ * @param string $path
+ * The path to validate.
+ *
+ * @return array
+ * A list of error strings.
+ */
+ protected function validatePath($path) {
+ $errors = array();
+ if (strpos($path, '%') === 0) {
+ $errors[] = $this->t('"%" may not be used for the first segment of a path.');
+ }
+
+ $path_sections = explode('/', $path);
+ // Symfony routing does not allow to use numeric placeholders.
+ // @see \Symfony\Component\Routing\RouteCompiler
+ $numeric_placeholders = array_filter($path_sections, function ($section) {
+ return (preg_match('/^%(.*)/', $section, $matches)
+ && is_numeric($matches[1]));
+ });
+ if (!empty($numeric_placeholders)) {
+ $errors[] = $this->t("Numeric placeholders may not be used. Please use plain placeholders (%).");
+ }
+ return $errors;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function validate() {
+ $errors = parent::validate();
+
+ $errors += $this->validatePath($this->getOption('path'));
+
+ return $errors;
+ }
+
+
}
diff --git a/core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php b/core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php
index 95f140e..596b805 100644
--- a/core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php
+++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php
@@ -47,6 +47,13 @@ class PathPluginBaseTest extends UnitTestCase {
*/
protected $state;
+ /**
+ * The mocked form error.
+ *
+ * @var \Drupal\Core\Form\FormErrorInterface|\PHPUnit_Framework_MockObject_MockObject
+ */
+ protected $formError;
+
public static function getInfo() {
return array(
'name' => 'Display: Path plugin base.',
@@ -63,8 +70,9 @@ class PathPluginBaseTest extends UnitTestCase {
$this->routeProvider = $this->getMock('Drupal\Core\Routing\RouteProviderInterface');
$this->state = $this->getMock('\Drupal\Core\State\StateInterface');
+ $this->formError = $this->getMock('Drupal\Core\Form\FormErrorInterface');
$this->pathPlugin = $this->getMockBuilder('Drupal\views\Plugin\views\display\PathPluginBase')
- ->setConstructorArgs(array(array(), 'path_base', array(), $this->routeProvider, $this->state))
+ ->setConstructorArgs(array(array(), 'path_base', array(), $this->routeProvider, $this->state, $this->formError))
->setMethods(NULL)
->getMock();
$this->setupContainer();
diff --git a/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayPath.php b/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayPath.php
index 8435589..8eb3e6d 100644
--- a/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayPath.php
+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayPath.php
@@ -30,6 +30,14 @@ class DisplayPath extends UITestBase {
}
public function testPathUI() {
+ $this->doBasicPathUITest();
+ $this->doAdvancedPathsValidationTest();
+ }
+
+ /**
+ * Tests basic functionality in configuring a view.
+ */
+ protected function doBasicPathUITest() {
$this->drupalGet('admin/structure/views/view/test_view');
// Add a new page display and check the appearing text.
@@ -45,6 +53,21 @@ class DisplayPath extends UITestBase {
}
/**
+ * Tests a couple of invalid path patterns.
+ */
+ protected function doAdvancedPathsValidationTest() {
+ $url = 'admin/structure/views/nojs/display/test_view/page_1/path';
+
+ $this->drupalPostForm($url, array('path' => '%/magrathea'), t('Apply'));
+ $this->assertUrl($url);
+ $this->assertText('"%" may not be used for the first segment of a path.');
+
+ $this->drupalPostForm($url, array('path' => 'user/%1/example'), t('Apply'));
+ $this->assertUrl($url);
+ $this->assertText("Numeric placeholders may not be used. Please use plain placeholders (%).");
+ }
+
+ /**
* Tests deleting a page display that has no path.
*/
public function testDeleteWithNoPath() {
diff --git a/core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewListBuilderTest.php b/core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewListBuilderTest.php
index 902b615..73f5013 100644
--- a/core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewListBuilderTest.php
+++ b/core/modules/views_ui/tests/Drupal/views_ui/Tests/ViewListBuilderTest.php
@@ -79,9 +79,10 @@ class ViewListBuilderTest extends UnitTestCase {
);
$route_provider = $this->getMock('Drupal\Core\Routing\RouteProviderInterface');
$state = $this->getMock('\Drupal\Core\State\StateInterface');
+ $form_builder = $this->getMock('Drupal\Core\Form\FormBuilderInterface');
$page_display = $this->getMock('Drupal\views\Plugin\views\display\Page',
array('initDisplay', 'getPath'),
- array(array(), 'default', $display_manager->getDefinition('page'), $route_provider, $state)
+ array(array(), 'default', $display_manager->getDefinition('page'), $route_provider, $state, $form_builder)
);
$page_display->expects($this->any())
->method('getPath')