Issue #2403485 by larowlan, kim.pepper, idebr, fago: Complete conversion of...

Issue #2403485 by larowlan, kim.pepper, idebr, fago: Complete conversion of comment form validation to entity validation

core/modules/comment/src/CommentForm.php

@@ -201,12 +201,6 @@ public function form(array $form, FormStateInterface $form_state) {
       '#access' => $is_admin,
     );
 
-    // Used for conditional validation of author fields.
-    $form['is_anonymous'] = array(
-      '#type' => 'value',
-      '#value' => ($comment->id() ? !$comment->getOwnerId() : $this->currentUser->isAnonymous()),
-    );
-
     return parent::form($form, $form_state, $comment);
   }
 
@@ -243,43 +237,10 @@ protected function actions(array $form, FormStateInterface $form_state) {
   }
 
   /**
-   * Overrides Drupal\Core\Entity\EntityForm::validate().
-   */
-  public function validate(array $form, FormStateInterface $form_state) {
-    parent::validate($form, $form_state);
-    $entity = $this->entity;
-
-    if (!$entity->isNew()) {
-      // Verify the name in case it is being changed from being anonymous.
-      $accounts = $this->entityManager->getStorage('user')->loadByProperties(array('name' => $form_state->getValue('name')));
-      $account = reset($accounts);
-      $form_state->setValue('uid', $account ? $account->id() : 0);
-
-      $date = $form_state->getValue('date');
-      if ($date instanceOf DrupalDateTime && $date->hasErrors()) {
-        $form_state->setErrorByName('date', $this->t('You have to specify a valid date.'));
-      }
-      if ($form_state->getValue('name') && !$form_state->getValue('is_anonymous') && !$account) {
-        $form_state->setErrorByName('name', $this->t('You have to specify a valid author.'));
-      }
-    }
-    elseif ($form_state->getValue('is_anonymous')) {
-      // Validate anonymous comment author fields (if given). If the (original)
-      // author of this comment was an anonymous user, verify that no registered
-      // user with this name exists.
-      if ($form_state->getValue('name')) {
-        $accounts = $this->entityManager->getStorage('user')->loadByProperties(array('name' => $form_state->getValue('name')));
-        if (!empty($accounts)) {
-          $form_state->setErrorByName('name', $this->t('The name you used belongs to a registered user.'));
-        }
-      }
-    }
-  }
-
-  /**
-   * Overrides EntityForm::buildEntity().
+   * {@inheritdoc}
    */
   public function buildEntity(array $form, FormStateInterface $form_state) {
+    /** @var \Drupal\comment\CommentInterface $comment */
     $comment = parent::buildEntity($form, $form_state);
     if (!$form_state->isValueEmpty('date') && $form_state->getValue('date') instanceOf DrupalDateTime) {
       $comment->setCreatedTime($form_state->getValue('date')->getTimestamp());
@@ -287,27 +248,20 @@ public function buildEntity(array $form, FormStateInterface $form_state) {
     else {
       $comment->setCreatedTime(REQUEST_TIME);
     }
-    $comment->changed->value = REQUEST_TIME;
-    return $comment;
-  }
+    $author_name = $form_state->getValue('name');
 
-  /**
-   * {@inheritdoc}
-   */
-  public function submitForm(array &$form, FormStateInterface $form_state) {
-    parent::submitForm($form, $form_state);
-    /** @var \Drupal\comment\CommentInterface $comment */
-    $comment = $this->entity;
-    // If the comment was posted by a registered user, assign the author's ID.
-    // @todo Too fragile. Should be prepared and stored in comment_form()
-    // already.
-    $author_name = $comment->getAuthorName();
-    if (!$comment->is_anonymous && !empty($author_name) && ($account = user_load_by_name($author_name))) {
-      $comment->setOwner($account);
+    if (!$this->currentUser->isAnonymous()) {
+      // Assign the owner based on the given user name - none means anonymous.
+      $accounts = $this->entityManager->getStorage('user')
+        ->loadByProperties(array('name' => $author_name));
+      $account = reset($accounts);
+      $uid = $account ? $account->id() : 0;
+      $comment->setOwnerId($uid);
     }
+
     // If the comment was posted by an anonymous user and no author name was
     // required, use "Anonymous" by default.
-    if ($comment->is_anonymous && (!isset($author_name) || $author_name === '')) {
+    if ($comment->getOwnerId() === 0 && (!isset($author_name) || $author_name === '')) {
       $comment->setAuthorName($this->config('user.settings')->get('anonymous'));
     }
 
@@ -326,10 +280,28 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
         $comment->setSubject($this->t('(No subject)'));
       }
     }
-
     return $comment;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function validate(array $form, FormStateInterface $form_state) {
+    parent::validate($form, $form_state);
+    $comment = $this->buildEntity($form, $form_state);
+
+    // Customly trigger validation of manually added fields and add in
+    // violations.
+    $violations = $comment->created->validate();
+    foreach ($violations as $violation) {
+      $form_state->setErrorByName('date', $violation->getMessage());
+    }
+    $violations = $comment->name->validate();
+    foreach ($violations as $violation) {
+      $form_state->setErrorByName('name', $violation->getMessage());
+    }
+  }
+
   /**
    * Form submission handler for the 'preview' action.
    *

core/modules/comment/src/Plugin/Validation/Constraint/CommentNameConstraint.php

@@ -19,6 +19,25 @@
  */
 class CommentNameConstraint extends Constraint {
 
-  public $message = '%name belongs to a registered user.';
+  /**
+   * Message shown when an anonymous user comments using a registered name.
+   *
+   * @var string
+   */
+  public $messageNameTaken = 'The name you used (%name) belongs to a registered user.';
+
+  /**
+   * Message shown when an admin changes the comment-author to an invalid user.
+   *
+   * @var string
+   */
+  public $messageRequired = 'You have to specify a valid author.';
+
+  /**
+   * Message shown when the name doesn't match the author's name.
+   *
+   * @var string
+   */
+  public $messageMatch = 'The specified author name does not match the comment author.';
 
 }

core/modules/comment/src/Plugin/Validation/Constraint/CommentNameConstraintValidator.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\comment\Plugin\Validation\Constraint;
 
+use Drupal\comment\CommentInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 
@@ -19,18 +20,59 @@ class CommentNameConstraintValidator extends ConstraintValidator {
    * {@inheritdoc}
    */
   public function validate($items, Constraint $constraint) {
+    /** @var CommentNameConstraint $constraint */
+    if (!isset($items)) {
+      return;
+    }
+    /** @var CommentInterface $comment */
+    $comment = $items->getEntity();
+
+    if (!isset($comment)) {
+      // Looks like we are validating a field not being part of a comment,
+      // nothing we can do then.
+      return;
+    }
     $author_name = $items->first()->value;
-    if (isset($author_name) && $author_name !== '') {
-      // Do not allow unauthenticated comment authors to use a name that is
-      // taken by a registered user.
-      if ($items->getEntity()->getOwnerId() === 0) {
-        // @todo Properly inject dependency https://drupal.org/node/2197029
-        $users = \Drupal::entityManager()->getStorage('user')->loadByProperties(array('name' => $author_name));
-        if (!empty($users)) {
-          $this->context->addViolation($constraint->message, array('%name' => $author_name));
-        }
+
+    // Do not allow unauthenticated comment authors to use a name that is
+    // taken by a registered user.
+    if (isset($author_name) && $author_name !== '' && $comment->getOwnerId() === 0) {
+      // @todo Properly inject dependency https://drupal.org/node/2197029
+      $users = \Drupal::entityManager()->getStorage('user')->loadByProperties(array('name' => $author_name));
+      if (!empty($users)) {
+        $this->context->addViolation($constraint->messageNameTaken, array('%name' => $author_name));
+      }
+    }
+    // If an author name and owner are given, make sure they match.
+    elseif (isset($author_name) && $author_name !== '' && $comment->getOwnerId()) {
+      $owner = $comment->getOwner();
+      if ($owner->getUsername() != $author_name) {
+        $this->context->addViolation($constraint->messageMatch);
       }
     }
+
+    // Anonymous account might be required - depending on field settings.
+    if ($comment->getOwnerId() === 0 && empty($author_name) &&
+      $this->getAnonymousContactDetailsSetting($comment) === COMMENT_ANONYMOUS_MUST_CONTACT) {
+      $this->context->addViolation($constraint->messageRequired);
+    }
+  }
+
+  /**
+   * Gets the anonymous contact details setting from the comment.
+   *
+   * @param \Drupal\comment\CommentInterface $comment
+   *   The entity.
+   *
+   * @return int
+   *   The anonymous contact setting.
+   */
+  protected function getAnonymousContactDetailsSetting(CommentInterface $comment) {
+    return $comment
+      ->getCommentedEntity()
+      ->get($comment->getFieldName())
+      ->getFieldDefinition()
+      ->getSetting('anonymous');
   }
 
 }

core/modules/comment/src/Tests/CommentAnonymousTest.php

@@ -67,7 +67,9 @@ function testAnonymous() {
       'comment_body[0][value]' => $this->randomMachineName(),
     );
     $this->drupalPostForm('comment/reply/node/' . $this->node->id() . '/comment', $edit, t('Save'));
-    $this->assertText(t('The name you used belongs to a registered user.'));
+    $this->assertRaw(t('The name you used (%name) belongs to a registered user.', [
+      '%name' => $this->adminUser->getUsername(),
+    ]));
 
     // Require contact info.
     $this->drupalLogin($this->adminUser);

core/modules/comment/src/Tests/CommentValidationTest.php

@@ -8,7 +8,9 @@
 namespace Drupal\comment\Tests;
 
 use Drupal\comment\CommentInterface;
+use Drupal\node\Entity\Node;
 use Drupal\system\Tests\Entity\EntityUnitTestBase;
+use Drupal\user\Entity\User;
 
 /**
  * Tests comment validation constraints.
@@ -36,6 +38,10 @@ protected function setUp() {
    * Tests the comment validation constraints.
    */
   public function testValidation() {
+    // Add a user.
+    $user = User::create(array('name' => 'test'));
+    $user->save();
+
     // Add comment type.
     $this->entityManager->getStorage('comment_type')->create(array(
       'id' => 'comment',
@@ -60,12 +66,14 @@ public function testValidation() {
     ))->save();
 
     // Add comment field to page content.
-    $this->entityManager->getStorage('field_config')->create(array(
+    /** @var \Drupal\field\FieldConfigInterface $field */
+    $field = $this->entityManager->getStorage('field_config')->create(array(
       'field_name' => 'comment',
       'entity_type' => 'node',
       'bundle' => 'page',
       'label' => 'Comment settings',
-    ))->save();
+    ));
+    $field->save();
 
     $node = $this->entityManager->getStorage('node')->create(array(
       'type' => 'page',
@@ -93,13 +101,12 @@ public function testValidation() {
 
     // Validate a name collision between an anonymous comment author name and an
     // existing user account name.
-    $user = entity_create('user', array('name' => 'test'));
-    $user->save();
     $comment->set('name', 'test');
+    $comment->set('uid', 0);
     $violations = $comment->validate();
     $this->assertEqual(count($violations), 1, "Violation found on author name collision");
     $this->assertEqual($violations[0]->getPropertyPath(), "name");
-    $this->assertEqual($violations[0]->getMessage(), t('%name belongs to a registered user.', array('%name' => 'test')));
+    $this->assertEqual($violations[0]->getMessage(), t('The name you used (%name) belongs to a registered user.', array('%name' => 'test')));
 
     // Make the name valid.
     $comment->set('name', 'valid unused name');
@@ -128,6 +135,53 @@ public function testValidation() {
     $comment->set('hostname', NULL);
     $comment->set('thread', $this->randomString(256));
     $this->assertLengthViolation($comment, 'thread', 255);
+
+    $comment->set('thread', NULL);
+
+    // Force anonymous users to enter contact details.
+    $field->settings['anonymous'] = COMMENT_ANONYMOUS_MUST_CONTACT;
+    $field->save();
+    // Reset the node entity.
+    \Drupal::entityManager()->getStorage('node')->resetCache([$node->id()]);
+    $node = Node::load($node->id());
+    // Create a new comment with the new field.
+    $comment = $this->entityManager->getStorage('comment')->create(array(
+      'entity_id' => $node->id(),
+      'entity_type' => 'node',
+      'field_name' => 'comment',
+      'comment_body' => $this->randomMachineName(),
+      'uid' => 0,
+      'name' => '',
+    ));
+    $violations = $comment->validate();
+    $this->assertEqual(count($violations), 1, 'Violation found when name is required, but empty and UID is anonymous.');
+    $this->assertEqual($violations[0]->getPropertyPath(), 'name');
+    $this->assertEqual($violations[0]->getMessage(), t('You have to specify a valid author.'));
+
+    // Test creating a default comment with a given user id works.
+    $comment = $this->entityManager->getStorage('comment')->create(array(
+      'entity_id' => $node->id(),
+      'entity_type' => 'node',
+      'field_name' => 'comment',
+      'comment_body' => $this->randomMachineName(),
+      'uid' => $user->id(),
+    ));
+    $violations = $comment->validate();
+    $this->assertEqual(count($violations), 0, 'No violations when validating a default comment with an author.');
+
+    // Test specifying a wrong author name does not work.
+    $comment = $this->entityManager->getStorage('comment')->create(array(
+      'entity_id' => $node->id(),
+      'entity_type' => 'node',
+      'field_name' => 'comment',
+      'comment_body' => $this->randomMachineName(),
+      'uid' => $user->id(),
+      'name' => 'not-test',
+    ));
+    $violations = $comment->validate();
+    $this->assertEqual(count($violations), 1, 'Violation found when author name and comment author do not match.');
+    $this->assertEqual($violations[0]->getPropertyPath(), 'name');
+    $this->assertEqual($violations[0]->getMessage(), t('The specified author name does not match the comment author.'));
   }
 
   /**