diff --git a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/LanguageItem.php b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/LanguageItem.php index d0791e91768..9ab60a4d485 100644 --- a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/LanguageItem.php +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/LanguageItem.php @@ -2,7 +2,7 @@ /** * @file - * Contains \Drupal\Core\Entity\Plugin\Field\FieldType\LanguageItem. + * Contains \Drupal\Core\Field\Plugin\Field\FieldType\LanguageItem. */ namespace Drupal\Core\Field\Plugin\Field\FieldType; @@ -25,10 +25,16 @@ use Drupal\Core\TypedData\DataReferenceDefinition; * no_ui = TRUE, * constraints = { * "ComplexData" = { - * "value" = {"Length" = {"max" = 12}} + * "value" = { + * "Length" = {"max" = 12}, + * "AllowedValues" = {"callback" = "\Drupal\Core\Field\Plugin\Field\FieldType\LanguageItem::getAllowedLanguageCodes" } + * } * } * } * ) + * + * @todo Define the AllowedValues constraint via an options provider once + * https://www.drupal.org/node/2329937 is completed. */ class LanguageItem extends FieldItemBase { @@ -50,6 +56,16 @@ class LanguageItem extends FieldItemBase { return $properties; } + /** + * Defines allowed language codes for the field's AllowedValues constraint. + * + * @return string[] + * The allowed values. + */ + public static function getAllowedLanguageCodes() { + return array_keys(\Drupal::languageManager()->getLanguages(LanguageInterface::STATE_ALL)); + } + /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/Validation/ConstraintManager.php b/core/lib/Drupal/Core/Validation/ConstraintManager.php index 458c0bc8ceb..c760d1e2371 100644 --- a/core/lib/Drupal/Core/Validation/ConstraintManager.php +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php @@ -68,7 +68,7 @@ class ConstraintManager extends DefaultPluginManager { // Plugins need an array as configuration, so make sure we have one. // The constraint classes support passing the options as part of the // 'value' key also. - $options = array('value' => $options); + $options = isset($options) ? array('value' => $options) : array(); } return $this->createInstance($name, $options); } diff --git a/core/modules/system/src/Tests/Entity/EntityValidationTest.php b/core/modules/system/src/Tests/Entity/EntityValidationTest.php index 44c90316365..96e5d8b9314 100644 --- a/core/modules/system/src/Tests/Entity/EntityValidationTest.php +++ b/core/modules/system/src/Tests/Entity/EntityValidationTest.php @@ -126,8 +126,10 @@ class EntityValidationTest extends EntityUnitTestBase { $langcode_key = $this->entityManager->getDefinition($entity_type)->getKey('langcode'); $test_entity->{$langcode_key}->value = $this->randomString(13); $violations = $test_entity->validate(); - $this->assertEqual($violations->count(), 1, 'Validation failed.'); + // This should fail on AllowedValues and Length constraints. + $this->assertEqual($violations->count(), 2, 'Validation failed.'); $this->assertEqual($violations[0]->getMessage(), t('This value is too long. It should have %limit characters or less.', array('%limit' => '12'))); + $this->assertEqual($violations[1]->getMessage(), t('The value you selected is not a valid choice.')); $test_entity = clone $entity; $test_entity->type->value = NULL; diff --git a/core/modules/user/src/AccountForm.php b/core/modules/user/src/AccountForm.php index 7ed3cbde07a..033731fb387 100644 --- a/core/modules/user/src/AccountForm.php +++ b/core/modules/user/src/AccountForm.php @@ -88,6 +88,7 @@ abstract class AccountForm extends ContentEntityForm { // The mail field is NOT required if account originally had no mail set // and the user performing the edit has 'administer users' permission. // This allows users without email address to be edited and deleted. + // Also see \Drupal\user\Plugin\Validation\Constraint\UserMailRequired. $form['account']['mail'] = array( '#type' => 'email', '#title' => $this->t('Email address'), @@ -359,7 +360,23 @@ abstract class AccountForm extends ContentEntityForm { if (is_string(key($form_state->getValue('roles')))) { $form_state->setValue('roles', array_keys(array_filter($form_state->getValue('roles')))); } - return parent::buildEntity($form, $form_state); + + /** @var \Drupal\user\UserInterface $account */ + $account = parent::buildEntity($form, $form_state); + + // Take care of mapping signature form element values as their structure + // does not directly match the field structure. + $signature = $form_state->getValue('signature'); + $account->setSignature($signature['value']); + $account->setSignatureFormat($signature['format']); + + // Translate the empty value '' of language selects to an unset field. + foreach (array('preferred_langcode', 'preferred_admin_langcode') as $field_name) { + if ($form_state->getValue($field_name) === '') { + $account->$field_name = NULL; + } + } + return $account; } /** @@ -368,63 +385,25 @@ abstract class AccountForm extends ContentEntityForm { public function validate(array $form, FormStateInterface $form_state) { parent::validate($form, $form_state); - $account = $this->entity; - // Validate new or changing username. - if ($form_state->hasValue('name')) { - if ($error = user_validate_name($form_state->getValue('name'))) { - $form_state->setErrorByName('name', $error); - } - // Cast the user ID as an integer. It might have been set to NULL, which - // could lead to unexpected results. - else { - $name_taken = (bool) $this->entityQuery->get('user') - ->condition('uid', (int) $account->id(), '<>') - ->condition('name', $form_state->getValue('name')) - ->range(0, 1) - ->count() - ->execute(); - - if ($name_taken) { - $form_state->setErrorByName('name', $this->t('The username %name is already taken.', array('%name' => $form_state->getValue('name')))); - } - } - } - - $mail = $form_state->getValue('mail'); - - if (!empty($mail)) { - $mail_taken = (bool) $this->entityQuery->get('user') - ->condition('uid', (int) $account->id(), '<>') - ->condition('mail', $mail) - ->range(0, 1) - ->count() - ->execute(); - - if ($mail_taken) { - // Format error message dependent on whether the user is logged in or not. - if (\Drupal::currentUser()->isAuthenticated()) { - $form_state->setErrorByName('mail', $this->t('The email address %email is already taken.', array('%email' => $mail))); - } - else { - $form_state->setErrorByName('mail', $this->t('The email address %email is already registered. Have you forgotten your password?', array('%email' => $mail, '@password' => $this->url('user.pass')))); - } - } - } - - // Make sure the signature isn't longer than the size of the database field. - // Signatures are disabled by default, so make sure it exists first. - if ($signature = $form_state->getValue('signature')) { - // Move text format for user signature into 'signature_format'. - $form_state->setValue('signature_format', $signature['format']); - // Move text value for user signature into 'signature'. - $form_state->setValue('signature', $signature['value']); - - // @todo Make the user signature field use a widget to benefit from - // automatic typed data validation in https://drupal.org/node/2227381. - $field_definitions = $this->entityManager->getFieldDefinitions('user', $this->getEntity()->bundle()); - $max_length = $field_definitions['signature']->getSetting('max_length'); - if (Unicode::strlen($form_state->getValue('signature')) > $max_length) { - $form_state->setErrorByName('signature', $this->t('The signature is too long: it must be %max characters or less.', array('%max' => $max_length))); + /** @var \Drupal\user\UserInterface $account */ + $account = $this->buildEntity($form, $form_state); + // Customly trigger validation of manually added fields and add in + // violations. This is necessary as entity form displays only invoke entity + // validation for fields contained in the display. + $field_names = array( + 'name', + 'mail', + 'signature', + 'signature_format', + 'timezone', + 'langcode', + 'preferred_langcode', + 'preferred_admin_langcode' + ); + foreach ($field_names as $field_name) { + $violations = $account->$field_name->validate(); + foreach ($violations as $violation) { + $form_state->setErrorByName($field_name, $violation->getMessage()); } } } diff --git a/core/modules/user/src/Entity/User.php b/core/modules/user/src/Entity/User.php index 5cb964ef78a..8be2e5e2e44 100644 --- a/core/modules/user/src/Entity/User.php +++ b/core/modules/user/src/Entity/User.php @@ -12,6 +12,7 @@ use Drupal\Core\Entity\EntityMalformedException; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\BaseFieldDefinition; +use Drupal\Core\Language\LanguageInterface; use Drupal\user\UserInterface; /** @@ -288,6 +289,14 @@ class User extends ContentEntityBase implements UserInterface { return $this->get('signature')->value; } + /** + * {@inheritdoc} + */ + public function setSignature($signature) { + $this->get('signature')->value = $signature; + return $this; + } + /** * {@inheritdoc} */ @@ -295,6 +304,14 @@ class User extends ContentEntityBase implements UserInterface { return $this->get('signature_format')->value; } + /** + * {@inheritdoc} + */ + public function setSignatureFormat($signature_format) { + $this->get('signature_format')->value = $signature_format; + return $this; + } + /** * {@inheritdoc} */ @@ -462,12 +479,25 @@ class User extends ContentEntityBase implements UserInterface { $fields['preferred_langcode'] = BaseFieldDefinition::create('language') ->setLabel(t('Preferred language code')) - ->setDescription(t("The user's preferred language code for receiving emails and viewing the site.")); + ->setDescription(t("The user's preferred language code for receiving emails and viewing the site.")) + // @todo: Define this via an options provider once + // https://www.drupal.org/node/2329937 is completed. + ->addPropertyConstraints('value', array( + 'AllowedValues' => array('callback' => __CLASS__ . '::getAllowedConfigurableLanguageCodes'), + )); $fields['preferred_admin_langcode'] = BaseFieldDefinition::create('language') ->setLabel(t('Preferred admin language code')) ->setDescription(t("The user's preferred language code for viewing administration pages.")) - ->setDefaultValue(''); + // @todo: A default value of NULL is ignored, so we have to specify + // an empty field item structure instead. Fix this in + // https://www.drupal.org/node/2318605. + ->setDefaultValue(array(0 => array ('value' => NULL))) + // @todo: Define this via an options provider once + // https://www.drupal.org/node/2329937 is completed. + ->addPropertyConstraints('value', array( + 'AllowedValues' => array('callback' => __CLASS__ . '::getAllowedConfigurableLanguageCodes'), + )); // The name should not vary per language. The username is the visual // identifier for a user and needs to be consistent in all languages. @@ -490,7 +520,8 @@ class User extends ContentEntityBase implements UserInterface { ->setLabel(t('Email')) ->setDescription(t('The email of this user.')) ->setDefaultValue('') - ->setConstraints(array('UserMailUnique' => array())); + ->addConstraint('UserMailUnique') + ->addConstraint('UserMailRequired'); // @todo Convert to a text field in https://drupal.org/node/1548204. $fields['signature'] = BaseFieldDefinition::create('string') @@ -499,12 +530,22 @@ class User extends ContentEntityBase implements UserInterface { ->setTranslatable(TRUE); $fields['signature_format'] = BaseFieldDefinition::create('string') ->setLabel(t('Signature format')) - ->setDescription(t('The signature format of this user.')); + ->setDescription(t('The signature format of this user.')) + // @todo: Define this via an options provider once + // https://www.drupal.org/node/2329937 is completed. + ->addPropertyConstraints('value', array( + 'AllowedValues' => array('callback' => __CLASS__ . '::getAllowedSignatureFormats'), + )); $fields['timezone'] = BaseFieldDefinition::create('string') ->setLabel(t('Timezone')) ->setDescription(t('The timezone of this user.')) - ->setSetting('max_length', 32); + ->setSetting('max_length', 32) + // @todo: Define this via an options provider once + // https://www.drupal.org/node/2329937 is completed. + ->addPropertyConstraints('value', array( + 'AllowedValues' => array('callback' => __CLASS__ . '::getAllowedTimezones'), + )); $fields['status'] = BaseFieldDefinition::create('boolean') ->setLabel(t('User status')) @@ -553,4 +594,38 @@ class User extends ContentEntityBase implements UserInterface { return \Drupal::entityManager()->getStorage('user_role'); } + /** + * Defines allowed signature formats for the field's AllowedValues constraint. + * + * @return string[] + * The allowed values. + */ + public static function getAllowedSignatureFormats() { + if (\Drupal::moduleHandler()->moduleExists('filter')) { + return array_keys(filter_formats()); + } + // If filter.module is disabled, no value may be assigned. + return array(); + } + + /** + * Defines allowed timezones for the field's AllowedValues constraint. + * + * @return string[] + * The allowed values. + */ + public static function getAllowedTimezones() { + return array_keys(system_time_zones()); + } + + /** + * Defines allowed configurable language codes for AllowedValues constraints. + * + * @return string[] + * The allowed values. + */ + public static function getAllowedConfigurableLanguageCodes() { + return array_keys(\Drupal::languageManager()->getLanguages(LanguageInterface::STATE_CONFIGURABLE)); + } + } diff --git a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php new file mode 100644 index 00000000000..77c08b30bcc --- /dev/null +++ b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php @@ -0,0 +1,77 @@ +context = $context; + } + + /** + * {@inheritdoc} + */ + public function validatedBy() { + return get_class($this); + } + + /** + * {@inheritdoc} + */ + public function validate($items, Constraint $constraint) { + /** @var \Drupal\Core\Field\FieldItemListInterface $items */ + /** @var \Drupal\user\UserInterface $account */ + $account = $this->context->getMetadata()->getTypedData()->getEntity(); + $existing_value = NULL; + if ($account->id()) { + $account_unchanged = \Drupal::entityManager() + ->getStorage('user') + ->loadUnchanged($account->id()); + $existing_value = $account_unchanged->getEmail(); + } + + $required = !(!$existing_value && \Drupal::currentUser()->hasPermission('administer users')); + + if ($required && (!isset($items) || $items->isEmpty())) { + $this->context->addViolation($this->message, array('!name' => String::placeholder($account->getFieldDefinition('mail')->getLabel()))); + } + } + +} diff --git a/core/modules/user/src/Tests/UserEditTest.php b/core/modules/user/src/Tests/UserEditTest.php index 308fad96d76..38acdb8f101 100644 --- a/core/modules/user/src/Tests/UserEditTest.php +++ b/core/modules/user/src/Tests/UserEditTest.php @@ -16,12 +16,51 @@ use Drupal\simpletest\WebTestBase; */ class UserEditTest extends WebTestBase { + /** + * Modules to enable. + * + * @var array + */ + public static $modules = array('filter'); + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + + $this->config('user.settings')->set('signatures', TRUE)->save(); + + // Prefetch and create text formats. + $this->filtered_html_format = entity_create('filter_format', array( + 'format' => 'filtered_html_format', + 'name' => 'Filtered HTML', + 'weight' => -1, + 'filters' => array( + 'filter_html' => array( + 'module' => 'filter', + 'status' => TRUE, + 'settings' => array( + 'allowed_html' => ' ', + ), + ), + ), + )); + $this->filtered_html_format->save(); + + $this->full_html_format = entity_create('filter_format', array( + 'format' => 'full_html', + 'name' => 'Full HTML', + )); + $this->full_html_format->save(); + } + /** * Test user edit page. */ function testUserEdit() { // Test user edit functionality. - $user1 = $this->drupalCreateUser(array('change own username')); + $user1 = $this->drupalCreateUser(array('change own username', $this->full_html_format->getPermissionName(), $this->filtered_html_format->getPermissionName())); $user2 = $this->drupalCreateUser(array()); $this->drupalLogin($user1); @@ -85,6 +124,11 @@ class UserEditTest extends WebTestBase { $config->set('password_strength', FALSE)->save(); $this->drupalPostForm("user/" . $user1->id() . "/edit", $edit, t('Save')); $this->assertNoRaw(t('Password strength:'), 'The password strength indicator is not displayed.'); + + // Test user signature + $edit = array('signature[format]' => $this->full_html_format->id(), 'signature[value]' => $this->randomString(256)); + $this->drupalPostForm('user/' . $user1->id() . '/edit', $edit, t('Save')); + $this->assertRaw(t("%name: may not be longer than @max characters.", array('%name' => t('Signature'), '@max' => 255))); } /** diff --git a/core/modules/user/src/Tests/UserRegistrationTest.php b/core/modules/user/src/Tests/UserRegistrationTest.php index 853891ad85f..221667f9643 100644 --- a/core/modules/user/src/Tests/UserRegistrationTest.php +++ b/core/modules/user/src/Tests/UserRegistrationTest.php @@ -138,13 +138,13 @@ class UserRegistrationTest extends WebTestBase { // Attempt to create a new account using an existing email address. $this->drupalPostForm('user/register', $edit, t('Create new account')); - $this->assertText(t('The email address @email is already registered.', array('@email' => $duplicate_user->getEmail())), 'Supplying an exact duplicate email address displays an error message'); + $this->assertText(t('The email address @email is already taken.', array('@email' => $duplicate_user->getEmail())), 'Supplying an exact duplicate email address displays an error message'); // Attempt to bypass duplicate email registration validation by adding spaces. $edit['mail'] = ' ' . $duplicate_user->getEmail() . ' '; $this->drupalPostForm('user/register', $edit, t('Create new account')); - $this->assertText(t('The email address @email is already registered.', array('@email' => $duplicate_user->getEmail())), 'Supplying a duplicate email address with added whitespace displays an error message'); + $this->assertText(t('The email address @email is already taken.', array('@email' => $duplicate_user->getEmail())), 'Supplying a duplicate email address with added whitespace displays an error message'); } function testRegistrationDefaultValues() { diff --git a/core/modules/user/src/Tests/UserValidationTest.php b/core/modules/user/src/Tests/UserValidationTest.php index f5ae202780d..acc64b76d0c 100644 --- a/core/modules/user/src/Tests/UserValidationTest.php +++ b/core/modules/user/src/Tests/UserValidationTest.php @@ -7,8 +7,10 @@ namespace Drupal\user\Tests; +use Drupal\Component\Utility\String; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Field\Plugin\Field\FieldType\EmailItem; +use Drupal\Core\Language\Language; use Drupal\Core\Render\Element\Email; use Drupal\simpletest\KernelTestBase; use Drupal\user\Entity\Role; @@ -74,7 +76,10 @@ class UserValidationTest extends KernelTestBase { * Runs entity validation checks. */ function testValidation() { - $user = User::create(array('name' => 'test')); + $user = User::create(array( + 'name' => 'test', + 'mail' => 'test@example.com', + )); $violations = $user->validate(); $this->assertEqual(count($violations), 0, 'No violations when validating a default user.'); @@ -128,20 +133,43 @@ class UserValidationTest extends KernelTestBase { $this->assertEqual($violations[0]->getPropertyPath(), 'mail'); $this->assertEqual($violations[0]->getMessage(), t('The email address %mail is already taken.', array('%mail' => 'existing@example.com'))); $user->set('mail', NULL); + $violations = $user->validate(); + $this->assertEqual(count($violations), 1, 'E-mail addresses may not be removed'); + $this->assertEqual($violations[0]->getPropertyPath(), 'mail'); + $this->assertEqual($violations[0]->getMessage(), t('!name field is required.', array('!name' => String::placeholder($user->getFieldDefinition('mail')->getLabel())))); + $user->set('mail', 'someone@example.com'); $user->set('signature', $this->randomString(256)); $this->assertLengthViolation($user, 'signature', 255); $user->set('signature', NULL); + $user->set('signature_format', $this->randomString(32)); + $this->assertAllowedValuesViolation($user, 'signature_format'); + $user->set('signature_format', NULL); + $user->set('timezone', $this->randomString(33)); - $this->assertLengthViolation($user, 'timezone', 32); + $this->assertLengthViolation($user, 'timezone', 32, 2, 1); + $user->set('timezone', 'invalid zone'); + $this->assertAllowedValuesViolation($user, 'timezone'); $user->set('timezone', NULL); $user->set('init', 'invalid'); $violations = $user->validate(); $this->assertEqual(count($violations), 1, 'Violation found when init email is invalid'); - $this->assertEqual($violations[0]->getPropertyPath(), 'init.0.value'); - $this->assertEqual($violations[0]->getMessage(), t('This value is not a valid email address.')); + $user->set('init', NULL); + + $user->set('langcode', 'invalid'); + $this->assertAllowedValuesViolation($user, 'langcode'); + $user->set('langcode', NULL); + + // Only configurable langcodes are allowed for preferred languages. + $user->set('preferred_langcode', Language::LANGCODE_NOT_SPECIFIED); + $this->assertAllowedValuesViolation($user, 'preferred_langcode'); + $user->set('preferred_langcode', NULL); + + $user->set('preferred_admin_langcode', Language::LANGCODE_NOT_SPECIFIED); + $this->assertAllowedValuesViolation($user, 'preferred_admin_langcode'); + $user->set('preferred_admin_langcode', NULL); Role::create(array('id' => 'role1'))->save(); Role::create(array('id' => 'role2'))->save(); @@ -149,6 +177,7 @@ class UserValidationTest extends KernelTestBase { // Test cardinality of user roles. $user = entity_create('user', array( 'name' => 'role_test', + 'mail' => 'test@example.com', 'roles' => array('role1', 'role2'), )); $violations = $user->validate(); @@ -170,13 +199,32 @@ class UserValidationTest extends KernelTestBase { * The field that violates the maximum length. * @param int $length * Number of characters that was exceeded. + * @param int $count + * (optional) The number of expected violations. Defaults to 1. + * @param int $expected_index + * (optional) The index at which to expect the violation. Defaults to 0. */ - protected function assertLengthViolation(EntityInterface $entity, $field_name, $length) { + protected function assertLengthViolation(EntityInterface $entity, $field_name, $length, $count = 1, $expected_index = 0) { $violations = $entity->validate(); - $this->assertEqual(count($violations), 1, "Violation found when $field_name is too long."); - $this->assertEqual($violations[0]->getPropertyPath(), "$field_name.0.value"); + $this->assertEqual(count($violations), $count, "Violation found when $field_name is too long."); + $this->assertEqual($violations[$expected_index]->getPropertyPath(), "$field_name.0.value"); $field_label = $entity->get($field_name)->getFieldDefinition()->getLabel(); - $this->assertEqual($violations[0]->getMessage(), t('%name: may not be longer than @max characters.', array('%name' => $field_label, '@max' => $length))); + $this->assertEqual($violations[$expected_index]->getMessage(), t('%name: may not be longer than @max characters.', array('%name' => $field_label, '@max' => $length))); + } + + /** + * Verifies that a AllowedValues violation exists for the given field. + * + * @param \Drupal\core\Entity\EntityInterface $entity + * The entity object to validate. + * @param string $field_name + * The name of the field to verify. + */ + protected function assertAllowedValuesViolation(EntityInterface $entity, $field_name) { + $violations = $entity->validate(); + $this->assertEqual(count($violations), 1, "Allowed values violation for $field_name found."); + $this->assertEqual($violations[0]->getPropertyPath(), "$field_name.0.value"); + $this->assertEqual($violations[0]->getMessage(), t('The value you selected is not a valid choice.')); } } diff --git a/core/modules/user/src/UserInterface.php b/core/modules/user/src/UserInterface.php index e2c44d01295..c6f3b2af3b6 100644 --- a/core/modules/user/src/UserInterface.php +++ b/core/modules/user/src/UserInterface.php @@ -96,6 +96,16 @@ interface UserInterface extends ContentEntityInterface, EntityChangedInterface, */ public function getSignature(); + /** + * Sets the user signature. + * + * @param string $signature + * The new signature text of the user. + * + * @return $this + */ + public function setSignature($signature); + /** * Returns the signature format. * @@ -104,6 +114,16 @@ interface UserInterface extends ContentEntityInterface, EntityChangedInterface, */ public function getSignatureFormat(); + /** + * Sets the signature format. + * + * @param string $signature_format + * The name of the new filter format. + * + * @return $this + */ + public function setSignatureFormat($signature_format); + /** * Returns the creation time of the user as a UNIX timestamp. *