Problem/Motivation

Now that we have a generic 'entity_autocomplete' form element, UserAutocompleteController is just duplicated functionality.

Proposed resolution

Convert its usage to 'entity_autocomplete' and remove all the dead code.

Remaining tasks

None.

User interface changes

User entity reference fields will have new checkbox "Include the anonymous user." in field configuration form.

API changes

A new 'include_anonymous' option is added to the user reference selection plugin.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Prioritized changes Follow-up for a critical issue #1959806: Provide a generic 'entity_autocomplete' Form API element.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu’s picture

Status: Active » Needs review
FileSize
35.7 KB

17 files changed, 115 insertions(+), 380 deletions(-)

Status: Needs review » Needs work

The last submitted patch, 1: 2434697.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
2.48 KB
36.96 KB

I think the form was making wrong assumptions about the current user being the one checked.

Status: Needs review » Needs work

The last submitted patch, 3: 2434697-3.patch, failed testing.

amateescu’s picture

Status: Needs work » Needs review
FileSize
34.72 KB
4.08 KB

Thanks @pcambra, your patch finally made me realize that the comment behavior is so custom that there's no point in trying to use EntityAutocomplete::validateEntityAutocomplete() at all, so I removed that weird new #process_autocomplete_value stuff in favor of just overriding #element_validate in CommentForm.

Wim Leers’s picture

Another lovely diffstat! :)

jibran’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This is perfect. Just two nits. Also updated user interface changes in IS.

  1. +++ b/core/modules/comment/src/Tests/CommentInterfaceTest.php
    @@ -196,4 +196,5 @@ function testCommentInterface() {
    +
    

    Out of the scope. :)

  2. +++ b/core/modules/system/src/Tests/Entity/EntityReferenceSelection/EntityReferenceSelectionAccessTest.php
    @@ -203,6 +203,7 @@ public function testUserHandler() {
    +        'include_anonymous' => TRUE,
    

    I believe we don't have to add that.

amateescu’s picture

FileSize
34.31 KB
423 bytes

Thanks for reviewing :)

Fixed #7.1 and #7.2 is correct, we don't have to but I added it so it's a bit more clear what we're testing.

jibran’s picture

Thank you @amateescu for the patch this is a huge improvement and thank you for clarifying #7.2.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
    @@ -390,19 +390,21 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
    -      $form['content_translation']['name'] = array(
    -        '#type' => 'textfield',
    +      $form['content_translation']['uid'] = array(
    
    +++ b/core/modules/content_translation/src/Tests/ContentTranslationUITest.php
    @@ -219,7 +219,7 @@ protected function doTestAuthoringInfo() {
    -        'content_translation[name]' => $user->getUsername(),
    
    @@ -237,7 +237,7 @@ protected function doTestAuthoringInfo() {
    -      'content_translation[name]' => $this->randomMachineName(12),
    +      'content_translation[uid]' => $this->randomMachineName(12),
    

    How come we're changing the the form element from name to uid? Whereas we're not in CommentForm. I'm not sure that changing it is correct.

  2. +++ b/core/modules/entity_reference/config/schema/entity_reference.schema.yml
    @@ -36,3 +36,6 @@ entity_reference.default.handler_settings:
    +    include_anonymous:
    +      type: boolean
    +      label: 'Include the anonymous user in the matched entities.'
    

    But this is meaning less for anything other than selecting users. Feels like this is blocked on #2436835: Unable to create config schema for entity type specific entity reference selection plugin.

amateescu’s picture

Status: Needs work » Reviewed & tested by the community

Re #10:

1. The change is correct because the entity autocomplete form element, by default through EntityAutocomplete::validateEntityAutocomplete(), returns entity IDs. CommentForm does not use the default behavior of the element, that's why it is not changed.
2. The user selection handler already has the 'filter' custom setting that's only used for user entities, which means that #2436835: Unable to create config schema for entity type specific entity reference selection plugin. will have to take the new one into account in a reroll.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 2434697-8.patch, failed testing.

amateescu’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
34.29 KB

Rerolled.

alexpott’s picture

Yep let's handle #10.2 in the other issue.

Committed 3e2af23 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed 3e2af23 on 8.0.x
    Issue #2434697 by amateescu, pcambra: Remove UserAutocompleteController
    
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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