Commit de193040 authored by alexpott's avatar alexpott

Issue #2453175 by tim.plunkett, plach, rteijeiro, effulgentsia, eshta,...

Issue #2453175 by tim.plunkett, plach, rteijeiro, effulgentsia, eshta, dawehner, fago, Berdir, alexpott: Remove EntityFormInterface::validate() and stop using button-level validation by default in entity forms
parent 79c11b6b
......@@ -151,6 +151,20 @@ abstract class ContentEntityBase extends Entity implements \IteratorAggregate, C
*/
protected $translatableEntityKeys = array();
/**
* Whether entity validation was performed.
*
* @var bool
*/
protected $validated = FALSE;
/**
* Whether entity validation is required before saving the entity.
*
* @var bool
*/
protected $validationRequired = FALSE;
/**
* Overrides Entity::__construct().
*/
......@@ -331,6 +345,23 @@ public function isTranslatable() {
return !empty($bundles[$this->bundle()]['translatable']) && !$this->getUntranslated()->language()->isLocked() && $this->languageManager()->isMultilingual();
}
/**
* {@inheritdoc}
*/
public function preSave(EntityStorageInterface $storage) {
// An entity requiring validation should not be saved if it has not been
// actually validated.
if ($this->validationRequired && !$this->validated) {
// @todo Make this an assertion in https://www.drupal.org/node/2408013.
throw new \LogicException('Entity validation was skipped.');
}
else {
$this->validated = FALSE;
}
parent::preSave($storage);
}
/**
* {@inheritdoc}
*/
......@@ -341,10 +372,26 @@ public function preSaveRevision(EntityStorageInterface $storage, \stdClass $reco
* {@inheritdoc}
*/
public function validate() {
$this->validated = TRUE;
$violations = $this->getTypedData()->validate();
return new EntityConstraintViolationList($this, iterator_to_array($violations));
}
/**
* {@inheritdoc}
*/
public function isValidationRequired() {
return (bool) $this->validationRequired;
}
/**
* {@inheritdoc}
*/
public function setValidationRequired($required) {
$this->validationRequired = $required;
return $this;
}
/**
* Clear entity translation object cache to remove stale references.
*/
......
......@@ -86,9 +86,6 @@ protected function actions(array $form, FormStateInterface $form_state) {
'submit' => array(
'#type' => 'submit',
'#value' => $this->getConfirmText(),
'#validate' => array(
array($this, 'validate'),
),
'#submit' => array(
array($this, 'submitForm'),
),
......@@ -121,9 +118,10 @@ public function delete(array $form, FormStateInterface $form_state) {}
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
public function validateForm(array &$form, FormStateInterface $form_state) {
// Override the default validation implementation as it is not necessary
// nor possible to validate an entity in a confirmation form.
return $this->entity;
}
}
......@@ -62,19 +62,30 @@ public function form(array $form, FormStateInterface $form_state) {
return $form;
}
/**
* {@inheritdoc}
*/
public function buildEntity(array $form, FormStateInterface $form_state) {
/** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
$entity = parent::buildEntity($form, $form_state);
// Mark the entity as requiring validation.
$entity->setValidationRequired(!$form_state->getTemporaryValue('entity_validated'));
return $entity;
}
/**
* {@inheritdoc}
*
* Note that extending classes should not override this method to add entity
* validation logic, but define further validation constraints using the
* entity validation API and/or provide a new validation constraint if
* necessary. This is the only way to ensure that the validation logic
* is correctly applied independently of form submissions; e.g., for REST
* requests.
* For more information about entity validation, see
* https://www.drupal.org/node/2015613.
* Button-level validation handlers are highly discouraged for entity forms,
* as they will prevent entity validation from running. If the entity is going
* to be saved during the form submission, this method should be manually
* invoked from the button-level validation handler, otherwise an exception
* will be thrown.
*/
public function validate(array $form, FormStateInterface $form_state) {
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
/** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
$entity = $this->buildEntity($form, $form_state);
......@@ -87,10 +98,10 @@ public function validate(array $form, FormStateInterface $form_state) {
$this->flagViolations($violations, $form, $form_state);
// @todo Remove this.
// Execute legacy global validation handlers.
$form_state->setValidateHandlers([]);
\Drupal::service('form_validator')->executeValidateHandlers($form, $form_state);
// The entity was validated.
$entity->setValidationRequired(FALSE);
$form_state->setTemporaryValue('entity_validated', TRUE);
return $entity;
}
......
......@@ -36,6 +36,8 @@ public function getFormDisplay(FormStateInterface $form_state);
* The form display that the current form operates with.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The current state of the form.
*
* @return $this
*/
public function setFormDisplay(EntityFormDisplayInterface $form_display, FormStateInterface $form_state);
......@@ -61,4 +63,21 @@ public function getFormLangcode(FormStateInterface $form_state);
*/
public function isDefaultFormLangcode(FormStateInterface $form_state);
/**
* {@inheritdoc}
*
* Note that extending classes should not override this method to add entity
* validation logic, but define further validation constraints using the
* entity validation API and/or provide a new validation constraint if
* necessary. This is the only way to ensure that the validation logic
* is correctly applied independently of form submissions; e.g., for REST
* requests.
* For more information about entity validation, see
* https://www.drupal.org/node/2015613.
*
* @return \Drupal\Core\Entity\ContentEntityTypeInterface
* The built entity.
*/
public function validateForm(array &$form, FormStateInterface $form_state);
}
......@@ -81,9 +81,6 @@ protected function actions(array $form, FormStateInterface $form_state) {
'submit' => array(
'#type' => 'submit',
'#value' => $this->getConfirmText(),
'#validate' => array(
array($this, 'validate'),
),
'#submit' => array(
array($this, 'submitForm'),
),
......
......@@ -219,7 +219,6 @@ protected function actions(array $form, FormStateInterface $form_state) {
$actions['submit'] = array(
'#type' => 'submit',
'#value' => $this->t('Save'),
'#validate' => array('::validate'),
'#submit' => array('::submitForm', '::save'),
);
......@@ -244,16 +243,6 @@ protected function actions(array $form, FormStateInterface $form_state) {
return $actions;
}
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
// @todo Remove this.
// Execute legacy global validation handlers.
$form_state->setValidateHandlers([]);
\Drupal::service('form_validator')->executeValidateHandlers($form, $form_state);
}
/**
* {@inheritdoc}
*
......
......@@ -94,19 +94,6 @@ public function getEntityFromRouteMatch(RouteMatchInterface $route_match, $entit
*/
public function buildEntity(array $form, FormStateInterface $form_state);
/**
* Validates the submitted form values of the entity form.
*
* @param array $form
* A nested array form elements comprising the form.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The current state of the form.
*
* @return \Drupal\Core\Entity\ContentEntityTypeInterface
* The built entity.
*/
public function validate(array $form, FormStateInterface $form_state);
/**
* Form submission handler for the 'save' action.
*
......
......@@ -212,4 +212,22 @@ public function onChange($field_name);
*/
public function validate();
/**
* Checks whether entity validation is required before saving the entity.
*
* @return bool
* TRUE if validation is required, FALSE if not.
*/
public function isValidationRequired();
/**
* Sets whether entity validation is required before saving the entity.
*
* @param bool $required
* TRUE if validation is required, FALSE otherwise.
*
* @return $this
*/
public function setValidationRequired($required);
}
......@@ -123,8 +123,8 @@ protected function actions(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
if ($this->plugin instanceof PluginFormInterface) {
$this->plugin->validateConfigurationForm($form, $form_state);
......
......@@ -273,8 +273,8 @@ protected function actions(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
// The Block Entity form puts all block plugin form elements in the
// settings form element, so just pass that to the block for validation.
......
......@@ -224,7 +224,8 @@ public function save(array $form, FormStateInterface $form_state) {
* {@inheritdoc}
*/
public function validateForm(array &$form, FormStateInterface $form_state) {
if ($this->entity->isNew()) {
$entity = parent::validateForm($form, $form_state);
if ($entity->isNew()) {
$exists = $this->blockContentStorage->loadByProperties(array('info' => $form_state->getValue(['info', 0, 'value'])));
if (!empty($exists)) {
$form_state->setErrorByName('info', $this->t('A block with description %name already exists.', array(
......@@ -232,6 +233,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
)));
}
}
return $entity;
}
}
......@@ -258,7 +258,6 @@ protected function actions(array $form, FormStateInterface $form_state) {
'#type' => 'submit',
'#value' => $this->t('Preview'),
'#access' => $preview_mode != DRUPAL_DISABLED,
'#validate' => array('::validate'),
'#submit' => array('::submitForm', '::preview'),
);
......
......@@ -112,8 +112,8 @@ public function form(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
// Validate and each email recipient.
$recipients = explode(',', $form_state->getValue('recipients'));
......
......@@ -168,8 +168,8 @@ public function actions(array $form, FormStateInterface $form_state) {
$elements = parent::actions($form, $form_state);
$elements['submit']['#value'] = $this->t('Send message');
$elements['preview'] = array(
'#type' => 'submit',
'#value' => $this->t('Preview'),
'#validate' => array('::validate'),
'#submit' => array('::submitForm', '::preview'),
);
return $elements;
......@@ -187,10 +187,8 @@ public function preview(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
$message = $this->entity;
public function validateForm(array &$form, FormStateInterface $form_state) {
$message = parent::validateForm($form, $form_state);
// Check if flood control has been activated for sending emails.
if (!$this->currentUser()->hasPermission('administer contact forms') && (!$message->isPersonal() || !$this->currentUser()->hasPermission('administer users'))) {
......@@ -204,6 +202,8 @@ public function validate(array $form, FormStateInterface $form_state) {
)));
}
}
return $message;
}
/**
......
......@@ -473,9 +473,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
$form['#entity_builders'][] = array($this, 'entityFormEntityBuild');
// Handle entity validation.
if (isset($form['actions']['submit'])) {
$form['actions']['submit']['#validate'][] = array($this, 'entityFormValidate');
}
$form['#validate'][] = array($this, 'entityFormValidate');
// Handle entity deletion.
if (isset($form['actions']['delete'])) {
......
......@@ -38,8 +38,8 @@ public function buildForm(array $form, FormStateInterface $form_state, $entity_t
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
$form_state->setValueForElement($form['id'], $this->targetEntityTypeId . '.' . $form_state->getValue('id'));
}
......
......@@ -150,8 +150,8 @@ protected function actions(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
if (isset($form['default_value'])) {
$item = $form['#entity']->get($this->entity->getName());
......
......@@ -146,8 +146,8 @@ protected function actions(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
// Validate field cardinality.
if ($form_state->getValue('cardinality') === 'number' && !$form_state->getValue('cardinality_number')) {
......
......@@ -204,8 +204,8 @@ public function exists($format_id) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
// @todo Move trimming upstream.
$format_format = trim($form_state->getValue('format'));
......
......@@ -86,9 +86,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
// Remove the alphabetical reset.
unset($form['actions']['reset_alphabetical']);
// The form needs to have submit and validate handlers set explicitly.
// Use the existing taxonomy overview submit handler.
$form['#submit'] = array('::submitForm');
$form['terms']['#empty'] = $this->t('No containers or forums available. <a href="@container">Add container</a> or <a href="@forum">Add forum</a>.', array(
'@container' => $this->url('forum.add_container'),
'@forum' => $this->url('forum.add_forum')
......
......@@ -415,7 +415,7 @@ function menu_ui_form_node_type_form_alter(&$form, FormStateInterface $form_stat
);
$options_cacheability->applyTo($form['menu']['menu_parent']);
$form['actions']['submit']['#validate'][] = 'menu_ui_form_node_type_form_validate';
$form['#validate'][] = 'menu_ui_form_node_type_form_validate';
$form['#entity_builders'][] = 'menu_ui_form_node_type_form_builder';
}
......
......@@ -297,7 +297,6 @@ protected function actions(array $form, FormStateInterface $form_state) {
'#access' => $preview_mode != DRUPAL_DISABLED && ($node->access('create') || $node->access('update')),
'#value' => t('Preview'),
'#weight' => 20,
'#validate' => array('::validate'),
'#submit' => array('::submitForm', '::preview'),
);
......
......@@ -204,8 +204,8 @@ protected function actions(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
$id = trim($form_state->getValue('type'));
// '0' is invalid, since elsewhere we check it using empty().
......
......@@ -142,7 +142,8 @@ public function form(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
// Only validate on edit.
if ($form_state->hasValue('keyed_styles')) {
// Check if another breakpoint group is selected.
......
......@@ -144,8 +144,8 @@ public function exists($id) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
// Ensure each path is unique.
$path = $this->entityQuery->get('search_page')
......
......@@ -126,8 +126,8 @@ public function form(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
// The machine name field should already check to see if the requested
// machine name is available. Regardless of machine_name or human readable
......
......@@ -149,4 +149,25 @@ protected function loadEntityByName($entity_type, $name) {
$entities = $entity_storage->loadByProperties(array('name' => $name));
return $entities ? current($entities) : NULL;
}
/**
* Checks that validation handlers works as expected.
*/
public function testValidationHandlers() {
/** @var \Drupal\Core\State\StateInterface $state */
$state = $this->container->get('state');
// Check that from-level validation handlers can be defined and can alter
// the form array.
$state->set('entity_test.form.validate.test', 'form-level');
$this->drupalPostForm('entity_test/add', [], 'Save');
$this->assertTrue($state->get('entity_test.form.validate.result'), 'Form-level validation handlers behave correctly.');
// Check that defining a button-level validation handler causes an exception
// to be thrown.
$state->set('entity_test.form.validate.test', 'button-level');
$this->drupalPostForm('entity_test/add', [], 'Save');
$this->assertEqual($state->get('entity_test.form.save.exception'), 'Drupal\Core\Entity\EntityStorageException: Entity validation was skipped.', 'Button-level validation handlers behave correctly.');
}
}
......@@ -89,8 +89,12 @@ protected function getErrorsForEntity(EntityInterface $entity, $hidden_fields =
\Drupal::formBuilder()->processForm('field_test_entity_form', $form, $form_state);
// Validate the field constraint.
$form_state->getFormObject()->setEntity($entity)->setFormDisplay($display, $form_state);
$form_state->getFormObject()->validate($form, $form_state);
/** @var \Drupal\Core\Entity\ContentEntityFormInterface $form_object */
$form_object = $form_state->getFormObject();
$form_object
->setEntity($entity)
->setFormDisplay($display, $form_state)
->validateForm($form, $form_state);
return $form_state->getErrors();
}
......
......@@ -275,6 +275,37 @@ function entity_test_entity_extra_field_info() {
return $extra;
}
/**
* Implements hook_form_BASE_FORM_ID_alter().
*/
function entity_test_form_entity_test_form_alter(&$form) {
switch (\Drupal::state()->get('entity_test.form.validate.test')) {
case 'form-level':
$form['#validate'][] = 'entity_test_form_entity_test_form_validate';
$form['#validate'][] = 'entity_test_form_entity_test_form_validate_check';
break;
case 'button-level':
$form['actions']['submit']['#validate'][] = 'entity_test_form_entity_test_form_validate';
}
}
/**
* Validation handler for the entity_test entity form.
*/
function entity_test_form_entity_test_form_validate(array &$form, FormStateInterface $form_state) {
$form['#entity_test_form_validate'] = TRUE;
}
/**
* Validation handler for the entity_test entity form.
*/
function entity_test_form_entity_test_form_validate_check(array &$form, FormStateInterface $form_state) {
if (!empty($form['#entity_test_form_validate'])) {
\Drupal::state()->set('entity_test.form.validate.result', TRUE);
}
}
/**
* Implements hook_form_BASE_FORM_ID_alter().
*/
......
......@@ -51,35 +51,40 @@ public function form(array $form, FormStateInterface $form_state) {
* {@inheritdoc}
*/
public function save(array $form, FormStateInterface $form_state) {
$entity = $this->entity;
try {
$entity = $this->entity;
// Save as a new revision if requested to do so.
if (!$form_state->isValueEmpty('revision')) {
$entity->setNewRevision();
}
// Save as a new revision if requested to do so.
if (!$form_state->isValueEmpty('revision')) {
$entity->setNewRevision();
}
$is_new = $entity->isNew();
$entity->save();
$is_new = $entity->isNew();
$entity->save();
if ($is_new) {
$message = t('%entity_type @id has been created.', array('@id' => $entity->id(), '%entity_type' => $entity->getEntityTypeId()));
}
else {
$message = t('%entity_type @id has been updated.', array('@id' => $entity->id(), '%entity_type' => $entity->getEntityTypeId()));
}
drupal_set_message($message);
if ($is_new) {
$message = t('%entity_type @id has been created.', array('@id' => $entity->id(), '%entity_type' => $entity->getEntityTypeId()));
}
else {
$message = t('%entity_type @id has been updated.', array('@id' => $entity->id(), '%entity_type' => $entity->getEntityTypeId()));
}
drupal_set_message($message);
if ($entity->id()) {
$entity_type = $entity->getEntityTypeId();
$form_state->setRedirect(
"entity.$entity_type.edit_form",
array($entity_type => $entity->id())
);
if ($entity->id()) {
$entity_type = $entity->getEntityTypeId();
$form_state->setRedirect(
"entity.$entity_type.edit_form",
array($entity_type => $entity->id())
);
}
else {
// Error on save.
drupal_set_message(t('The entity could not be saved.'), 'error');
$form_state->setRebuild();
}
}
else {
// Error on save.
drupal_set_message(t('The entity could not be saved.'), 'error');
$form_state->setRebuild();
catch (\Exception $e) {
\Drupal::state()->set('entity_test.form.save.exception', get_class($e) . ': ' . $e->getMessage());
}
}
......
......@@ -96,8 +96,8 @@ public function form(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
// Ensure numeric values.
if ($form_state->hasValue('weight') && !is_numeric($form_state->getValue('weight'))) {
......
......@@ -162,7 +162,7 @@ protected function actions(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
public function validateForm(array &$form, FormStateInterface $form_state) {
$wizard_type = $form_state->getValue(array('show', 'wizard_key'));
$wizard_instance = $this->wizardManager->createInstance($wizard_type);
$form_state->set('wizard', $wizard_instance->getPluginDefinition());
......
......@@ -58,7 +58,6 @@ protected function actions(array $form, FormStateInterface $form_state) {
$actions['submit'] = array(
'#type' => 'submit',
'#value' => $this->t('Duplicate'),
'#submit' => array('::submitForm'),
);
return $actions;
}
......
......@@ -264,8 +264,8 @@ protected function actions(array $form, FormStateInterface $form_state) {
/**
* {@inheritdoc}
*/
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
public function validateForm(array &$form, FormStateInterface $form_state) {
parent::validateForm($form, $form_state);
$view = $this->entity;
if ($view->isLocked()) {
......
......@@ -9,6 +9,7 @@
use Drupal\Core\Access\AccessResult;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Language\LanguageInterface;
use Drupal\Tests\UnitTestCase;
......@@ -349,6 +350,64 @@ public function testValidate() {
$this->assertSame(1, count($this->entity->validate()));
}
/**
* Tests required validation.
*
* @covers ::validate