Commit cb67958d authored by catch's avatar catch

Issue #2557299 by alexpott, Manuel Garcia, borisson_, phenaproxima, swentel,...

Issue #2557299 by alexpott, Manuel Garcia, borisson_, phenaproxima, swentel, Berdir, larowlan, mbovan, martin107, LKS90, segi, tim.plunkett, giancarlosotelo: Any AJAX call disregards machine name verification when AJAX is used and leads to a fatal error
parent 5e07c020
......@@ -28,7 +28,8 @@
* - The form state object.
* In most cases, an existing API or menu argument loader function can be
* re-used. The callback is only invoked if the submitted value differs from
* the element's #default_value.
* the element's initial #default_value. The initial #default_value is
* stored in form state so AJAX forms can be reliably validated.
* - source: (optional) The #array_parents of the form element containing the
* human-readable name (i.e., as contained in the $form structure) to use as
* source for the machine name. Defaults to array('label').
......@@ -150,6 +151,17 @@ public static function processMachineName(&$element, FormStateInterface $form_st
'field_suffix' => $element['#field_suffix'],
];
// Store the initial value in form state. The machine name needs this to
// ensure that the exists function is not called for existing values when
// editing them.
$initial_values = $form_state->get('machine_name.initial_values') ?: [];
// Store the initial values in an array so we can differentiate between a
// NULL default value and a new machine name element.
if (!array_key_exists($element['#name'], $initial_values)) {
$initial_values[$element['#name']] = $element['#default_value'];
$form_state->set('machine_name.initial_values', $initial_values);
}
// By default, machine names are restricted to Latin alphanumeric characters.
// So, default to LTR directionality.
if (!isset($element['#attributes'])) {
......@@ -246,8 +258,11 @@ public static function validateMachineName(&$element, FormStateInterface $form_s
}
}
// Verify that the machine name is unique.
if ($element['#default_value'] !== $element['#value']) {
// Verify that the machine name is unique. If the value matches the initial
// default value then it does not need to be validated as the machine name
// element assumes the form is editing the existing value.
$initial_values = $form_state->get('machine_name.initial_values') ?: [];
if (!array_key_exists($element['#name'], $initial_values) || $initial_values[$element['#name']] !== $element['#value']) {
$function = $element['#machine_name']['exists'];
if (call_user_func($function, $element['#value'], $element, $form_state)) {
$form_state->setError($element, t('The machine-readable name is already in use. It must be unique.'));
......
......@@ -216,6 +216,19 @@ public function testExistingFormat() {
$editor = Editor::load('filtered_html');
$this->assertTrue($editor instanceof Editor, 'An Editor config entity exists.');
$this->assertEqual($expected_settings, $editor->getSettings());
$this->drupalGet('admin/config/content/formats/add');
// Now attempt to add another filter format with the same editor and same
// machine name.
$edit = [
'format' => 'filtered_html',
'name' => 'Filtered HTML',
'editor[editor]' => 'ckeditor',
];
$this->submitForm($edit, 'editor_configure');
$this->submitForm($edit, 'Save configuration');
$this->assertResponse(200);
$this->assertText('The machine-readable name is already in use. It must be unique.');
}
/**
......
......@@ -131,13 +131,6 @@ public function form(array $form, FormStateInterface $form_state) {
'#options' => $options,
'#description' => $source_description,
'#ajax' => ['callback' => '::ajaxHandlerData'],
// Rebuilding the form as part of the AJAX request is a workaround to
// enforce machine_name validation.
// @todo This was added as part of #2932226 and it should be removed once
// https://www.drupal.org/project/drupal/issues/2557299 solves it in a
// more generic way.
'#executes_submit_callback' => TRUE,
'#submit' => [[static::class, 'rebuildSubmit']],
'#required' => TRUE,
// Once the media type is created, its source plugin cannot be changed
// anymore.
......@@ -236,18 +229,6 @@ public function form(array $form, FormStateInterface $form_state) {
return $form;
}
/**
* Form submission handler to rebuild the form on select submit.
*
* @param array $form
* Full form array.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* Current form state.
*/
public static function rebuildSubmit(array &$form, FormStateInterface $form_state) {
$form_state->setRebuild();
}
/**
* Prepares workflow options to be used in the 'checkboxes' form element.
*
......
......@@ -24,19 +24,21 @@ public function testMediaTypeCreationFormWithDefaultField() {
$this->drupalGet('admin/structure/media/add');
// Fill in a label to the media type.
$page->fillField('label', $label);
// Wait for machine name generation. Default: waitUntilVisible(), does not
// work properly.
$session->wait(5000, "jQuery('.machine-name-value').text() === '{$mediaTypeMachineName}'");
// Select the media source used by our media type.
// Select the media source used by our media type. Do this before setting
// the label or machine name in order to guard against the regression in
// https://www.drupal.org/project/drupal/issues/2557299.
$assert_session->fieldExists('Media source');
$assert_session->optionExists('Media source', 'test');
$page->selectFieldOption('Media source', 'test');
$result = $assert_session->waitForElementVisible('css', 'fieldset[data-drupal-selector="edit-source-configuration"]');
$this->assertNotEmpty($result);
// Fill in a label to the media type.
$page->fillField('label', $label);
// Wait for machine name generation. Default: waitUntilVisible(), does not
// work properly.
$session->wait(5000, "jQuery('.machine-name-value').text() === '{$mediaTypeMachineName}'");
$page->pressButton('Save');
// Check whether the source field was correctly created.
......@@ -97,11 +99,12 @@ public function testMediaTypeCreationReuseSourceField() {
// Create a new media type, which should create a new field we can reuse.
$this->drupalGet('/admin/structure/media/add');
$page->fillField('label', 'Pastafazoul');
$session->wait(5000, "jQuery('.machine-name-value').text() === 'pastafazoul'");
// Choose the source plugin before setting the label and machine name.
$page->selectFieldOption('Media source', 'test');
$result = $assert_session->waitForElementVisible('css', 'fieldset[data-drupal-selector="edit-source-configuration"]');
$this->assertNotEmpty($result);
$page->fillField('label', 'Pastafazoul');
$session->wait(5000, "jQuery('.machine-name-value').text() === 'pastafazoul'");
$page->pressButton('Save');
$label = 'Type reusing Default Field';
......@@ -109,14 +112,8 @@ public function testMediaTypeCreationReuseSourceField() {
$this->drupalGet('admin/structure/media/add');
// Fill in a label to the media type.
$page->fillField('label', $label);
// Wait for machine name generation. Default: waitUntilVisible(), does not
// work properly.
$session->wait(5000, "jQuery('.machine-name-value').text() === '{$mediaTypeMachineName}'");
// Select the media source used by our media type.
// Select the media source used by our media type. Do this before setting
// the label and machine name.
$assert_session->fieldExists('Media source');
$assert_session->optionExists('Media source', 'test');
$page->selectFieldOption('Media source', 'test');
......@@ -124,6 +121,14 @@ public function testMediaTypeCreationReuseSourceField() {
$this->assertNotEmpty($result);
// Select the existing field for re-use.
$page->selectFieldOption('source_configuration[source_field]', 'field_media_test');
// Fill in a label to the media type.
$page->fillField('label', $label);
// Wait for machine name generation. Default: waitUntilVisible(), does not
// work properly.
$session->wait(5000, "jQuery('.machine-name-value').text() === '{$mediaTypeMachineName}'");
$page->pressButton('Save');
// Check that no new fields were created.
......
......@@ -498,6 +498,14 @@ form_test.get_form:
requirements:
_access: 'TRUE'
form_test.machine_name_validation:
path: '/form-test/form-test-machine-name-validation'
defaults:
_form: '\Drupal\form_test\Form\FormTestMachineNameValidationForm'
_title: 'Form machine name validation test'
requirements:
_access: 'TRUE'
form_test.optional_container:
path: '/form-test/optional-container'
defaults:
......
<?php
namespace Drupal\form_test\Form;
use Drupal\Core\Form\FormBase;
use Drupal\Core\Form\FormStateInterface;
/**
* Form to test whether machine name validation works with ajax requests.
*/
class FormTestMachineNameValidationForm extends FormBase {
/**
* {@inheritdoc}
*/
public function getFormId() {
return 'form_test_machine_name_validation_form';
}
/**
* {@inheritdoc}
*/
public function buildForm(array $form, FormStateInterface $form_state) {
// Disable client-side validation so that we can test AJAX requests with
// invalid input.
$form['#attributes']['novalidate'] = 'novalidate';
$form['name'] = [
'#type' => 'textfield',
'#default_value' => $form_state->getValue('name'),
'#maxlength' => 50,
'#required' => TRUE,
'#title' => 'Name',
];
// The default value simulates how an entity form works, which has default
// values based on an entity, which is updated in an afterBuild callback.
// During validation and after build, limit_validation_errors is not
// in effect, which means that getValue('id') does return a value, while it
// does not during the submit callback. Therefore, this test sets the value
// in ::buildAjaxSnackConfigureFormValidate() and then uses that as the
// default value, so that the default value and the value are identical.
$form['id'] = [
'#type' => 'machine_name',
'#default_value' => $form_state->get('id'),
'#maxlength' => 50,
'#required' => TRUE,
'#machine_name' => [
'exists' => [$this, 'load'],
'source' => ['name'],
],
];
// Test support for multiple machine names on the form. Although this has
// the default value duplicate it should not generate an error because it
// is the default value.
$form['id2'] = [
'#type' => 'machine_name',
'#default_value' => 'duplicate',
'#maxlength' => 50,
'#required' => TRUE,
'#machine_name' => [
'exists' => [$this, 'load'],
'source' => ['name'],
],
];
$form['snack'] = [
'#type' => 'select',
'#title' => $this->t('Select a snack'),
'#options' => [
'apple' => 'apple',
'pear' => 'pear',
'potato' => 'potato',
],
'#required' => TRUE,
'#ajax' => [
'callback' => '::buildAjaxSnackConfigureForm',
'wrapper' => 'snack-config-form',
'method' => 'replace',
'effect' => 'fade',
],
];
$form['snack_configs'] = [
'#type' => 'container',
'#attributes' => [
'id' => 'snack-config-form',
],
'#tree' => TRUE,
];
$form['submit'] = [
'#type' => 'submit',
'#value' => 'Save',
];
return $form;
}
/**
* Validate callback that forces a form rebuild.
*/
public function buildAjaxSnackConfigureFormValidate(array $form, FormStateInterface $form_state) {
$form_state->set('id', $form_state->getValue('id'));
}
/**
* Submit callback that forces a form rebuild.
*/
public function buildAjaxSnackConfigureFormSubmit(array $form, FormStateInterface $form_state) {
$form_state->setRebuild();
}
/**
* Handles changes to the selected snack configuration.
*/
public function buildAjaxSnackConfigureForm(array $form, FormStateInterface $form_state) {
return $form['snack_configs'];
}
/**
* Loading stub for machine name
*
* @param $machine_name
* @return bool
*/
public function load($machine_name) {
if (strpos($machine_name, 'duplicate') !== FALSE) {
return TRUE;
}
return FALSE;
}
/**
* {@inheritdoc}
*/
public function submitForm(array &$form, FormStateInterface $form_state) {
$this->messenger()->addStatus('The form_test_machine_name_validation_form form has been submitted successfully.');
}
}
......@@ -59,7 +59,6 @@ public function testMachineName() {
// Get page and session.
$page = $this->getSession()->getPage();
$assert_session = $this->assertSession();
// Get elements from the page.
$title_1 = $page->findField('machine_name_1_label');
......@@ -111,6 +110,36 @@ public function testMachineName() {
// Validate if the element contains the correct value.
$this->assertEquals($test_values[1]['expected'], $machine_name_1_field->getValue(), 'The ID field value must be equal to the php generated machine name');
$assert = $this->assertSession();
$this->drupalGet('/form-test/form-test-machine-name-validation');
// Test errors after with no AJAX.
$assert->buttonExists('Save')->press();
$assert->pageTextContains('Machine-readable name field is required.');
// Ensure only the first machine name field has an error.
$this->assertTrue($assert->fieldExists('id')->hasClass('error'));
$this->assertFalse($assert->fieldExists('id2')->hasClass('error'));
// Test a successful submit after using AJAX.
$assert->fieldExists('Name')->setValue('test 1');
$assert->fieldExists('id')->setValue('test_1');
$assert->selectExists('snack')->selectOption('apple');
$assert->assertWaitOnAjaxRequest();
$assert->buttonExists('Save')->press();
$assert->pageTextContains('The form_test_machine_name_validation_form form has been submitted successfully.');
// Test errors after using AJAX.
$assert->fieldExists('Name')->setValue('duplicate');
$this->assertJsCondition('document.forms[0].id.value === "duplicate"');
$assert->fieldExists('id2')->setValue('duplicate2');
$assert->selectExists('snack')->selectOption('potato');
$assert->assertWaitOnAjaxRequest();
$assert->buttonExists('Save')->press();
$assert->pageTextContains('The machine-readable name is already in use. It must be unique.');
// Ensure both machine name fields both have errors.
$this->assertTrue($assert->fieldExists('id')->hasClass('error'));
$this->assertTrue($assert->fieldExists('id2')->hasClass('error'));
}
}
......@@ -60,6 +60,10 @@ public function testProcessMachineName() {
'additional_property' => TRUE,
'#additional_property_with_hash' => TRUE,
],
// The process function requires these to be set. During regular form
// building they are always set.
'#name' => 'test_machine_name',
'#default_value' => NULL,
];
$complete_form = [
......
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