Problem/Motivation

we make several base fields to use just Entity field formatters and not special purpose field handlers in views. However, we have a lot of other field handlers in views that have more functionality than the corresponding field formatters.

Proposed resolution

Replace comment_username with field formatter

Remaining tasks

create patch

User interface changes

Entity fields will have more formatting options available, and they'll be consistent with what is available in Views.

API changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kgoel’s picture

Issue summary: View changes
bburg’s picture

Assigned: Unassigned » bburg
Issue summary: View changes

Working on this at Drupal4Gov code sprint Co-Sponsored by Forum One ;)

xjm’s picture

Issue tags: +VDC
jhodgdon’s picture

Priority: Normal » Critical

This issue is critical as per parent issue.

dawehner’s picture

Assigned: bburg » plach

Let's remove the assignment, as it might block people from working on it.

dawehner’s picture

Assigned: plach » Unassigned

ehem, nearly.

geertvd’s picture

Status: Active » Needs review
FileSize
6.51 KB
jhodgdon’s picture

Did we really need a special formatter for this? I would think the standard formatters that were previously available would have worked. What options/formatting was missing?

dawehner’s picture

Did we really need a special formatter for this? I would think the standard formatters that were previously available would have worked. What options/formatting was missing?

The feature, which is not provided is the special logic for the anonymous user. For them, we get the username from the manually entered
username in the comment form, there is simply no formatter for that yet.

dawehner’s picture

Issue tags: +Needs tests

Thank you @geertvd!

I think we will need a test though ... :)

geertvd’s picture

Assigned: Unassigned » geertvd
dawehner’s picture

  1. +++ b/core/modules/comment/config/schema/comment.views.schema.yml
    +++ b/core/modules/comment/config/schema/comment.views.schema.yml
    @@ -82,9 +82,6 @@ views.field.comment_username:
    
    @@ -82,9 +82,6 @@ views.field.comment_username:
       type: views_field
       label: 'Node comment status'
       mapping:
    -    link_to_user:
    -      type: boolean
    -      label: 'Link this field to its user or an author''s homepage'
    

    Ideally we should remove the full entry of views.field.comment_username

  2. +++ b/core/modules/comment/src/Plugin/Field/FieldFormatter/AuthorNameFormatter.php
    @@ -0,0 +1,46 @@
    +    foreach ($items as $delta => $item) {
    +      $account = entity_create('user');
    

    Note: #2454163: Replace comment_username handler with generic views handler should better use those tools.

  3. +++ b/core/modules/comment/src/Plugin/Field/FieldFormatter/AuthorNameFormatter.php
    @@ -0,0 +1,46 @@
    +      $account->uid = $item->getOwnerId();
    +      $account->name = $item->getAuthorName();
    +      $account->homepage = $item->getHomepage();
    +      $elements[$delta] = array(
    +        '#theme' => 'username',
    +        '#account' => $account,
    +      );
    

    Note: Also this code should probably not always create an issue, but instead use the author if the author is not the anonymous one.

  4. +++ b/core/modules/comment/src/Plugin/Field/FieldFormatter/AuthorNameFormatter.php
    @@ -0,0 +1,46 @@
    +}
    \ No newline at end of file
    

    It would be great to add a newline.

geertvd’s picture

FileSize
2.07 KB
6.79 KB

Thanks for the help, this one actually works :)

  1. Removed
  2. Not sure what you mean here
  3. Done
  4. Done
geertvd’s picture

I'll work on some tests next.

larowlan’s picture

  1. +++ b/core/modules/comment/src/CommentViewsData.php
    @@ -33,7 +33,8 @@ public function getViewsData() {
    +    $data['comment_field_data']['name']['field']['id'] = 'field';
    

    I think the default is field, so this line can go

  2. +++ b/core/modules/comment/src/Plugin/Field/FieldFormatter/AuthorNameFormatter.php
    @@ -0,0 +1,50 @@
    + * Plugin implementation of the 'user_name' formatter.
    

    should be the ID, i.e. 'comment_username'?

larowlan’s picture

Working on tests

larowlan’s picture

My first patch in over a week...

larowlan’s picture

Issue tags: -Needs tests
FileSize
678 bytes
11.63 KB

Think we need cache tags too?

larowlan’s picture

And we need to limit the availability.

andypost’s picture

Patch could have conflict with related

dawehner’s picture

  1. +++ b/core/modules/comment/src/Plugin/Field/FieldFormatter/AuthorNameFormatter.php
    @@ -0,0 +1,62 @@
    +      $account = $comment->getOwner();
    +      if ($account->isAnonymous()) {
    +        $account->setUsername($comment->getAuthorName());
    +        $account->setEmail($comment->getAuthorEmail());
    +        $account->homepage = $comment->getHomepage();
    +      }
    

    Can we add a todo for the issue I linked in the different comment?

  2. +++ b/core/modules/comment/src/Plugin/Field/FieldFormatter/AuthorNameFormatter.php
    @@ -0,0 +1,62 @@
    +  public static function isApplicable(FieldDefinitionInterface $field_definition) {
    +    return $field_definition->getName() === 'name';
    +  }
    

    IMHO we should also check for the entity type.

geertvd’s picture

Assigned: geertvd » Unassigned

Can we add a todo for the issue I linked in the different comment?

Which issue are you referring to? You linked to this ticket in #12

dawehner’s picture

geertvd’s picture

comment_prepare_author can be replaced with $comment->getOwner() when #2458817: Creating new user entities for anonymous users is very slow is fixed again.
Added the check for entity type.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Awesome, do you mind adding a follow up for replacing that, once the other issue is in?

dawehner’s picture

Note: this does not need tests:

+class CommentUserNameTest extends ViewUnitTestBase {
alexpott’s picture

Status: Reviewed & tested by the community » Needs work

I think CommentViewsFieldAccessTest needs updating to cover this too - see #2462589: Provide test coverage for access checking of all views fields.

geertvd’s picture

#2458817: Creating new user entities for anonymous users is very slow just got fixed so we should replace comment_prepare_author with $comment->getOwner() in this ticket.

rteijeiro’s picture

Status: Needs work » Needs review
FileSize
12.65 KB
1.71 KB

Addressed #27 (I guess) and #28.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

In indeed does!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 6a7d1bb and pushed to 8.0.x. Thanks!

  • alexpott committed 6a7d1bb on 8.0.x
    Issue #2454163 by larowlan, geertvd, rteijeiro: Replace comment_username...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.