Replace user_name handler with Field API formatter

Replaced user_name views handler with field handler.

I've tested the patch myself and while it appears ok in the UI, it's failing the tests with a "missing schema" error in ConfigSchemaChecker.php.

If you don't patch the /core/modules/user/config/install/views*.yml files
(so you don't swap plugin_id: user_name for plugin_id: field) and re-run the tests it doesn't seem to throw the same errors, but looking at how the other child issues from the parent have implemented this change it does appear that we should be making that change. I'm looking to see if there's elsewhere in the tests we need to change to reflect this.

I removed the portions of the install/views*.yml files which made reference to the following default display options. I believe they're related to the options provided by the user_name handler that we're taking out of play.

link_to_user: true
overwrite_anonymous: false
anonymous_text: ''
format_username: true

Afterwards the tests seemed better on my end. Here's a patch.

That patch failed as per issue #2090115 which moved the physical location of the yml files from where they were when I rolled the patch. I'll re-roll and re-submit.

Same patch as before reflecting the move of those yml files to the optional subdirectory in issue #2090115.

I think link_to_user: true, overwrite_anonymous, anonymous_text and format_username aren't supported by field formatters. They are associated with plugin_id: user_name and since plugin_id is replaced with field, display options can't find any relation to field formatter.

Yeah, at least the link_to_user feature is needed as we can't bypass the #theme => 'username' API.

So we need a new formatter just for the user name. For that formatter you can use isApplicable to limit on which fields this formatter applies to.

Talked with kgoel on IRC so we came up with that new patch.

Not sure this formatter needed, and if so it needs to accept settings to do link to user page.
It's serious regression if we loose this ability, otoh ER can do that for us

+++ b/core/modules/contact/tests/modules/contact_test_views/test_views/views.view.test_contact_link.yml
@@ -83,8 +83,6 @@ display:
-          link_to_user: true

@@ -99,9 +97,8 @@ display:
+          plugin_id: field
+          type: user_name

+++ b/core/modules/content_translation/tests/modules/content_translation_test_views/test_views/views.view.test_entity_translations_link.yml
@@ -50,9 +50,8 @@ display:
-          link_to_user: true

+++ b/core/modules/node/config/optional/views.view.content.yml
@@ -208,11 +208,8 @@ display:
-          link_to_user: true

+++ b/core/modules/node/config/optional/views.view.content_recent.yml
@@ -185,13 +185,10 @@ display:
-          link_to_user: true

+++ b/core/modules/node/config/optional/views.view.glossary.yml
@@ -120,9 +120,9 @@ display:
-          link_to_user: true

+++ b/core/modules/user/config/optional/views.view.user_admin_people.yml
@@ -237,11 +237,8 @@ display:
-          link_to_user: true

+++ b/core/modules/user/config/optional/views.view.who_s_new.yml
@@ -70,8 +71,6 @@ display:
-          link_to_user: true

+++ b/core/modules/user/config/optional/views.view.who_s_online.yml
@@ -77,8 +78,6 @@ display:
-          link_to_user: true

it needs settings: link_to_entity:true
like we stop trying in #2456691: User email field need to use Field-Entity-aware formatters in Views

Not sure this formatter needed, and if so it needs to accept settings to do link to user page.

#theme => 'username' already links to the user ... but bypassing the API would be a regression compared to what views offered before.

A schema fix a day, keeps the failures away.

Worked on one of the test failures ... thank you @kgoel for pointing it out

Note: This is related a lot with the issue #2457999: Cannot use relationship for rendered entity on Views

Unless I'm reading the patch wrong, seems like this is doing the same kind of formatter replacement that makes the other similar sibling issues critical.

@kgoel
If you have time, please continue your great work on this issue ...

+++ b/core/modules/user/config/schema/user.views.schema.yml
@@ -132,10 +118,6 @@ views.filter.user_current:
 
-views.filter.user_name:
-  type: views.filter.in_operator
-  label: 'User name'

Don't remove the filter ... this thing still exists and is useful. Btw. this should cause the failures Drupal\tracker\Tests\Views\TrackerUserUidTest and Drupal\views\Tests\TestViewsTest as well as Drupal\user\Tests\Views\HandlerFilterPermissionTest

Regarding the GlossaryTest failure ... please add user:0 to the listing

    // Verify cache tags.
    $this->enablePageCaching();
    $this->assertPageCacheContextsAndTags(Url::fromRoute('view.glossary.page_1'), ['languages', 'theme', 'url', 'user.node_grants:view', 'user.permissions'], [
      'config:views.view.glossary',
      'node:' . $nodes_by_char['a'][0]->id(), 'node:' . $nodes_by_char['a'][1]->id(), 'node:' . $nodes_by_char['a'][2]->id(),
      'node_list',
      'user_list',
      'rendered',
    ]);

in core/modules/views/src/Tests/GlossaryTest.php:95

For the last remaining fail we should get http://privatepaste.com/fe0e1d738b in here, which adapted the tests, given that we have changed the handler itself.

@kgoel

The previous code looked like the following:

-views.filter.user_name:
-  type: views.filter.in_operator
-  label: 'User name'

but you added back

+views.field.user_name:
+  type: views_field_user
+  label: 'User name'
+

Note: field vs. filter. It has to be filter.

Regarding the change in GlossaryTest, let me quote myself :)

  please add user:0 to the listing

, it needs the :0

Let's quickly fix the remaining failures.

Sets back to needs review.

1. +++ b/core/modules/user/src/Plugin/Field/FieldFormatter/UserFormatter.php
   @@ -0,0 +1,58 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public static function isApplicable(FieldDefinitionInterface $field_definition) {
   +    return $field_definition->getName() === 'name';
   +  }
   
   +++ b/core/modules/user/src/UserViewsData.php
   @@ -80,7 +80,8 @@ public function getViewsData() {
   +    $data['users_field_data']['name']['field']['id'] = 'field';
   

   Isn't 'field' the default? If so does that mean this can go?

2. +++ b/core/modules/views/src/Entity/Render/RendererBase.php
   @@ -94,8 +94,10 @@ public function getCacheContexts() {
   +   *   (optional) The used relationship, might coming from the field.
   

   This sentence doesn't make sense

This is still tagged needs tests, but I see test changes in the patch - does that mean there are tests in HEAD?

+++ b/core/modules/user/src/UserViewsData.php
@@ -80,7 +80,8 @@ public function getViewsData() {
+    $data['users_field_data']['name']['field']['id'] = 'field';

'field' will get inherited as the plugin ID from EntityViewsData? Fail - didn't see Larowlan's comment above :)

Also, what happens with the 'overwrite_anonymous' feature?

NW cuz of #52.

Also, what happens with the 'overwrite_anonymous' feature?

Do you really think its a feature, which is not pure featurits? I considered this as something really special.

Expanded the test coverage.

I agree that overwrite anonymous isn't core-worthy feature.

Manual testing

However, with the current plugin (HEAD) you can opt-out of linking to the entity (user). With the current patch, that option isn't available.

I note that this option is available for the plain-text formatter so perhaps we should inherit from that formatter?

I note however that we're using theme_username which doesn't have a no-link option (head uses user_format_name)

However, with the current plugin (HEAD) you can opt-out of linking to the entity (user). With the current patch, that option isn't available.

Well, that option is encoded as part of the formatter ... but fair, we want to call out to $user->getUsername() and so also not ignore the API.
Added the setting to the username formatter.

I also enabled the access test coverage ...

I think you're missing the cache tags when rendering only the name. Else it won't update when changing the user name.

I think you're missing the cache tags when rendering only the name. Else it won't update when changing the user name.

Good point!!

We have already a dedicated test for the new formatter, so we don't need that issue tag anymore.

While this looks close enough that it probably doesn't matter too much, @alexpott, @effulgentsia, @xjm and I discussed this on a D8 critical triage call today. Since we expose user names publicly practically everywhere (user profile, node author, comment author, etc.) it's hard to envision a situation where failing to fix this would result in an SA, unlike a lot of the other issues in this suite. Therefore, downgrading to major, though we'd all still love to see it fixed along with the rest of them.

Is this issue also going to handle this line in NodeViewsData, or do we need another issue for that? This issue was spun off another issue that was supposed to cover this, as well as a similar line in CommentViewsData:

    $data['node_field_data']['uid']['field']['id'] = 'user';

The current patch doesn't seem to change NodeViewsData or CommentViewsData, and it doesn't get rid of the 'user' formatter in User module. ???

Talked to xjm about this... filing a separate issue for the 'user' plugin.
Filed:
#2470093: Views plugin 'user' needs to be replaced with entity-aware 'field' plugin
and I'll add this to the parent Meta issue.

Looks good to me. Just fixed a small nitpick.

Author Formatter BEFORE

Author Formatter AFTER

This issue is a major task that makes views consistently use field formatters. Whilst this is disruptive for existing views, consistency is a win. Per https://www.drupal.org/core/beta-changes, this is a good change to complete during the Drupal 8 beta phase. Committed 1893614 and pushed to 8.0.x. Thanks!