It seems that ER's autocomplete tags widget wasn't tested properly so currently you can't enter more than one entity in the autocomplete field.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu’s picture

Assigned: amateescu » Unassigned
Status: Active » Needs review
FileSize
12.58 KB

This should do the job, adding proper tests and whatnot :)

P.S. The change of elementValidate() method to validateElement() was done in order to be consistent with the patch from #1751234: Convert option widgets to Plugin system.

dawehner’s picture

+++ b/core/modules/entity_reference/entity_reference.moduleundefined
@@ -469,23 +469,27 @@ function entity_reference_autocomplete_access_callback($type, $field_name, $enti
+    $prefix = count($tags_typed) ? implode(', ', $tags_typed) . ', ' : '';

what about using drupal_implode_tags ?

amateescu’s picture

Nice find! Attaching a test-only patch as well to see the failure easier.

dawehner’s picture

+++ b/core/modules/entity_reference/entity_reference.moduleundefined
@@ -469,23 +469,27 @@ function entity_reference_autocomplete_access_callback($type, $field_name, $enti
+  // The user enters a comma-separated list of tags so we generate a prefix.

In general it wills wrong to talk about tags, but yeah this should/could be fixed in the controller issue.

+++ b/core/modules/entity_reference/entity_reference.moduleundefined
--- a/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/field/widget/AutocompleteTagsWidget.php
+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/field/widget/AutocompleteTagsWidget.phpundefined

+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/field/widget/AutocompleteTagsWidget.phpundefined
--- a/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/field/widget/AutocompleteWidget.php
+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/field/widget/AutocompleteWidget.phpundefined

+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/field/widget/AutocompleteWidget.phpundefined
--- a/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/field/widget/AutocompleteWidgetBase.php
+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/field/widget/AutocompleteWidgetBase.phpundefined

Sure all these changes feels out of scope here, but I discussed with @amateescu and I think it's okay to change this, especially because this files will probably not touched by other patches.

+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceAutocompleteTest.phpundefined
@@ -96,13 +97,20 @@ function testTermAutocompletion() {
+    $result = $this->drupalGet($path_prefix_tags, array('query' => array('q' => $input)));
+    $data = drupal_json_decode($result);

Any reason to not use drupalGetAjax here?

amateescu’s picture

Fixed 1) and 3) from above.

dawehner’s picture

+++ b/core/modules/entity_reference/entity_reference.moduleundefined
@@ -469,23 +469,28 @@ function entity_reference_autocomplete_access_callback($type, $field_name, $enti
   $tags_typed = drupal_explode_tags($tags_typed);

Let's fix the variables in the controller issue.

This

+++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceAutocompleteTest.phpundefined
@@ -83,26 +83,32 @@ function testTermAutocompletion() {
+    $this->assertEqual(strip_tags($data[$first_term->name . ' (1), ' . $second_term->name. ' (2)']), check_plain($second_term->name), 'Autocomplete returned the second matching term');

Space after $second_term->name missing :)

amateescu’s picture

Let's fix the variables in the controller issue.

I already changed too much in here.. what's three lines more :)

Fixed both issues.

dawehner’s picture

-1

amateescu’s picture

Yep, you're right, let's just fix the bug, there's plenty of time for cleanups.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Yeah once you have a big patch in, things move slower then before.

Thanks for keeping the patch as small as possible.

catch’s picture

Category: task » bug
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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