Skip to content

GitLab

Commit 42eb466c authored by catch's avatar catch
Browse files

Issue #2342325 by Lendude, mikeker, amyvs: Spaces in exposed filter ID cause filter not to work

parent 4db570e0
Hide whitespace changes
Inline Side-by-side
core/modules/views/src/Plugin/views/filter/FilterPluginBase.php
View file @ 42eb466c
......@@ -614,7 +614,7 @@ public function buildExposeForm(&$form, FormStateInterface $form_state) {
'#default_value' => $this->options['expose']['identifier'],
'#title' => $this->t('Filter identifier'),
'#size' => 40,
'#description' => $this->t('This will appear in the URL after the ? to identify this filter. Cannot be blank.'),
'#description' => $this->t('This will appear in the URL after the ? to identify this filter. Cannot be blank. Only letters, digits and the dot ("."), hyphen ("-"), underscore ("_"), and tilde ("~") characters are allowed.'),
);
}
......@@ -623,16 +623,7 @@ public function buildExposeForm(&$form, FormStateInterface $form_state) {
*/
public function validateExposeForm($form, FormStateInterface $form_state) {
$identifier = $form_state->getValue(array('options', 'expose', 'identifier'));
if (empty($identifier)) {
$form_state->setError($form['expose']['identifier'], $this->t('The identifier is required if the filter is exposed.'));
}
elseif ($identifier == 'value') {
$form_state->setError($form['expose']['identifier'], $this->t('This identifier is not allowed.'));
}
if (!$this->view->display_handler->isIdentifierUnique($form_state->get('id'), $identifier)) {
$form_state->setError($form['expose']['identifier'], $this->t('This identifier is used by another handler.'));
}
$this->validateIdentifier($identifier, $form_state, $form['expose']['identifier']);
}
/**
......@@ -641,17 +632,7 @@ public function validateExposeForm($form, FormStateInterface $form_state) {
protected function buildGroupValidate($form, FormStateInterface $form_state) {
if (!$form_state->isValueEmpty(array('options', 'group_info'))) {
$identifier = $form_state->getValue(array('options', 'group_info', 'identifier'));
if (empty($identifier)) {
$form_state->setError($form['group_info']['identifier'], $this->t('The identifier is required if the filter is exposed.'));
}
elseif ($identifier == 'value') {
$form_state->setError($form['group_info']['identifier'], $this->t('This identifier is not allowed.'));
}
if (!$this->view->display_handler->isIdentifierUnique($form_state->get('id'), $identifier)) {
$form_state->setError($form['group_info']['identifier'], $this->t('This identifier is used by another handler.'));
}
$this->validateIdentifier($identifier, $form_state, $form['group_info']['identifier']);
}
if ($group_items = $form_state->getValue(array('options', 'group_info', 'group_items'))) {
......@@ -680,6 +661,42 @@ protected function buildGroupValidate($form, FormStateInterface $form_state) {
}
}
/**
* Validates a filter identifier.
*
* Sets the form error if $form_state is passed or a error string if
* $form_state is not passed.
*
* @param string $identifier
* The identifier to check.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* @param array $form_group
* The form element to set any errors on.
*
* @return string
*/
protected function validateIdentifier($identifier, FormStateInterface $form_state = NULL, &$form_group = array()) {
$error = '';
if (empty($identifier)) {
$error = $this->t('The identifier is required if the filter is exposed.');
}
elseif ($identifier == 'value') {
$error = $this->t('This identifier is not allowed.');
}
elseif (preg_match('/[^a-zA-z0-9_~\.\-]/', $identifier)) {
$error = $this->t('This identifier has illegal characters.');
}
if ($form_state && !$this->view->display_handler->isIdentifierUnique($form_state->get('id'), $identifier)) {
$error = $this->t('This identifier is used by another handler.');
}
if (!empty($form_state) && !empty($error)) {
$form_state->setError($form_group, $error);
}
return $error;
}
/**
* Save new group items, re-enumerates and remove groups marked to delete.
*/
......@@ -868,7 +885,7 @@ protected function buildExposedFiltersGroupForm(&$form, FormStateInterface $form
'#default_value' => $identifier,
'#title' => $this->t('Filter identifier'),
'#size' => 40,
'#description' => $this->t('This will appear in the URL after the ? to identify this filter. Cannot be blank.'),
'#description' => $this->t('This will appear in the URL after the ? to identify this filter. Cannot be blank. Only letters, digits and the dot ("."), hyphen ("-"), underscore ("_"), and tilde ("~") characters are allowed.'),
);
$form['group_info']['label'] = array(
'#type' => 'textfield',
......@@ -922,7 +939,7 @@ protected function buildExposedFiltersGroupForm(&$form, FormStateInterface $form
'#default_value' => $identifier,
'#title' => $this->t('Filter identifier'),
'#size' => 40,
'#description' => $this->t('This will appear in the URL after the ? to identify this filter. Cannot be blank.'),
'#description' => $this->t('This will appear in the URL after the ? to identify this filter. Cannot be blank. Only letters, digits and the dot ("."), hyphen ("-"), underscore ("_"), and tilde ("~") characters are allowed.'),
);
$form['group_info']['label'] = array(
'#type' => 'textfield',
......@@ -1490,6 +1507,15 @@ public function getCacheTags() {
return [];
}
/**
* {@inheritdoc}
*/
public function validate() {
if (!empty($this->options['exposed']) && $error = $this->validateIdentifier($this->options['expose']['identifier'])) {
return [$error];
}
}
}
/**
......
core/modules/views/src/Plugin/views/filter/InOperator.php
View file @ 42eb466c
......@@ -415,7 +415,7 @@ protected function opEmpty() {
public function validate() {
$this->getValueOptions();
$errors = array();
$errors = parent::validate();
// If the operator is an operator which doesn't require a value, there is
// no need for additional validation.
......
core/modules/views/src/Tests/Plugin/ExposedFormTest.php
View file @ 42eb466c
......@@ -84,6 +84,69 @@ public function testSubmitButton() {
$this->helperButtonHasLabel('edit-submit-test-exposed-form-buttons', t('Apply'));
}
/**
* Tests the exposed form with a non-standard identifier.
*/
public function testExposedIdentifier() {
// Alter the identifier of the filter to a random string.
$view = Views::getView('test_exposed_form_buttons');
$view->setDisplay();
$identifier = 'new_identifier';
$view->displayHandlers->get('default')->overrideOption('filters', array(
'type' => [
'exposed' => TRUE,
'field' => 'type',
'id' => 'type',
'table' => 'node_field_data',
'plugin_id' => 'in_operator',
'entity_type' => 'node',
'entity_field' => 'type',
'expose' => [
'identifier' => $identifier,
'label' => 'Content: Type',
'operator_id' => 'type_op',
'reduce' => FALSE,
'description' => 'Exposed overridden description'
],
]
));
$view->save();
$this->drupalGet('test_exposed_form_buttons', array('query' => array($identifier => 'article')));
$this->assertFieldById(Html::getId('edit-' . $identifier), 'article', "Article type filter set with new identifier.");
// Alter the identifier of the filter to a random string containing
// restricted characters.
$view = Views::getView('test_exposed_form_buttons');
$view->setDisplay();
$identifier = 'bad identifier';
$view->displayHandlers->get('default')->overrideOption('filters', array(
'type' => [
'exposed' => TRUE,
'field' => 'type',
'id' => 'type',
'table' => 'node_field_data',
'plugin_id' => 'in_operator',
'entity_type' => 'node',
'entity_field' => 'type',
'expose' => [
'identifier' => $identifier,
'label' => 'Content: Type',
'operator_id' => 'type_op',
'reduce' => FALSE,
'description' => 'Exposed overridden description'
],
]
));
$this->executeView($view);
$errors = $view->validate();
$expected = [
'default' => ['This identifier has illegal characters.'],
'page_1' => ['This identifier has illegal characters.'],
];
$this->assertEqual($errors, $expected);
}
/**
* Tests whether the reset button works on an exposed form.
*/
......
core/modules/views_ui/src/Tests/FilterUITest.php
View file @ 42eb466c
......@@ -95,4 +95,33 @@ public function testFiltersUI() {
$this->assertNoRaw('<span>Group 3</span>', 'Group 3 has not been added yet.');
}
/**
* Tests the identifier settings and restrictions.
*/
public function testFilterIdentifier() {
$admin_user = $this->drupalCreateUser(array('administer views', 'administer site configuration'));
$this->drupalLogin($admin_user);
$path = 'admin/structure/views/nojs/handler/test_filter_in_operator_ui/default/filter/type';
// Set an empty identifier.
$edit = array(
'options[expose][identifier]' => '',
);
$this->drupalPostForm($path, $edit, t('Apply'));
$this->assertText('The identifier is required if the filter is exposed.');
// Set the identifier to 'value'.
$edit = array(
'options[expose][identifier]' => 'value',
);
$this->drupalPostForm($path, $edit, t('Apply'));
$this->assertText('This identifier is not allowed.');
// Set the identifier to a value with a restricted character.
$edit = array(
'options[expose][identifier]' => 'value value',
);
$this->drupalPostForm($path, $edit, t('Apply'));
$this->assertText('This identifier has illegal characters.');
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment