The Views selection handler for entity reference autocomplete is not properly doing exact matches with typed in input.

Step to reproduce

1. Create 2 articles with title "dog" and "dog cat"
2. Create an entity reference field on some entity, and configure it to use a views selection handler, with a view showing all articles
3. Edit the entity, and type "dog" into the ER autocomplete field. Do NOT pick either "dog" or "dog cat" from the autocomplete dropdown.
4. Save the entity.
5. See the error "Multiple entities match this reference"

This should not happen - exact matches should be accepted when typed in, as they are when using the "Default" selection handler.

Cause

views\src\Plugin\views\display\EntityReference.php -> query() is failing to properly handle the '=' match operator that the autocomplete widget passes when executing a view to evaluate typed in values for matches with existing entities. 

      $value = db_like($options['match']) . '%';
      if ($options['match_operator'] != 'STARTS_WITH') {
        $value = '%' . $value;
      }

Even when $match_operator is '=', this code results in "LIKE $value%", which does not return only exact matches. And therefore the view contains not just "dog" but "dog cat" also, and the autocomplete widget throws the "Multiple entities match" warning.

CommentFileSizeAuthor
#22 interdiff-2809555-16-22.txt3.28 KBjonathanshaw
#22 2809555-22.patch3.72 KBjonathanshaw
#16 7-16-interdiff.txt1.47 KBalexpott
#16 2809555-16.patch4.11 KBalexpott
#7 2809555-7-test-only.patch2.65 KBjonathanshaw
#7 2809555-7.patch3.59 KBjonathanshaw
#2 2809555.patch4.21 KBjonathanshaw
#2 2809555-test-only.patch3.15 KBjonathanshaw
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jonathanjfshaw created an issue. See original summary.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
Title: ER views selection handler fail to recognise exact matches typed in » ER views autocomplete fail to recognise exact matches typed in
Issue summary: View changes
Status: Active » Needs review
FileSize
2809555-test-only.patch3.15 KB
2809555.patch4.21 KB

Test and fix.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
Issue summary: View changes
jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
Issue summary: View changes

The last submitted patch, 2: 2809555-test-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: 2809555.patch, failed testing.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
FileSize
2809555-7.patch3.59 KB
2809555-7-test-only.patch2.65 KB

Slightly simplified, and rebased.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: 2809555-7-test-only.patch, failed testing.

The last submitted patch, 7: 2809555-7.patch, failed testing.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
Status: Needs work » Needs review
amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

The patch looks good but I wonder how can you get in this situation since the widgets provided by core only gives you two options: STARTS_WITH and CONTAINS.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented

Thanks for the review!

drupal/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php contains 

protected static function matchEntityByTitle(...) {
    $entities_by_bundle = $handler->getReferenceableEntities($input, '=', 6);

and then drupal/core/modules/views/src/Plugin/EntityReferenceSelection/ViewsSelection.php has

  public function getReferenceableEntities($match = NULL, $match_operator = 'CONTAINS', $limit = 0) {
...
    if ($this->initializeView($match, $match_operator, $limit)) {

So while the sitebuilder can only choose 'Starts with' or 'Contains' to affect what is displayed in the dropdown, the API (correctly) uses '=' to evaluate input typed in by authors who ignore the dropdown.

The steps to reproduce from the IS work on simplytest.me.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Reviewed & tested by the community

Right, that makes sense. Thanks for the explanation :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/views/src/Plugin/views/display/EntityReference.php
@@ -122,9 +122,12 @@ public function query() {
-      $value = db_like($options['match']) . '%';
-      if ($options['match_operator'] != 'STARTS_WITH') {
-        $value = '%' . $value;
+      $value = db_like($options['match']);
+      if ($options['match_operator'] === 'STARTS_WITH') {
+        $value = $value . '%';
+      }
+      elseif ($options['match_operator'] === 'CONTAINS') {
+        $value = '%' . $value . '%';

This is a behaviour change for things that pass something other than STARTS_WITH, CONTAINS or =. I think the behaviour should be completely different for =. And we shouldn't change the behaviour for things only than what is used by core - just in case something in contrib is doing something weird.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs work » Needs review
FileSize
2809555-16.patch4.11 KB
7-16-interdiff.txt1.47 KB

Something like this.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
Status: Needs review » Reviewed & tested by the community

Agreed, thanks for that.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 16: 2809555-16.patch, failed testing.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
Status: Needs work » Reviewed & tested by the community

Test fail looks unrelated.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/views/src/Plugin/views/display/EntityReference.php
@@ -122,9 +122,16 @@ public function query() {
+        $operator = '=';

While this stops inexact matches being made, it also makes the search case sensitive. Shouldn't the operator still be LIKE but with no wildcard after the string?

The test coverage only includes a lower case string, so wouldn't catch this.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris commented

Briefly discussed with @jonathanjfshaw on IRC.

Also see #2784739: Fix PostgreSQL operator in views and #2664530: Views Combined fields filter with "Contains any word" or "Contains all words" operator results in an incorrect SQL query and a fatal error

+++ b/core/modules/views/src/Plugin/views/display/EntityReference.php
@@ -122,9 +122,16 @@ public function query() {
+        $operator = 'LIKE';
+        $value = db_like($value) . '%';

Replace db_like() with Database::getConnection()->escapeLike($string)

Also change the LIKE statements to use Connection::mapConditionOperator.

Since this is also not currently done, this might be something to do in a followup, but if we want to address the case sensitivity this might be needed here anyway.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
CreditAttribution: jonathanshaw commented
Status: Needs work » Needs review
FileSize
2809555-22.patch3.72 KB
interdiff-2809555-16-22.txt3.28 KB

I've switched the '=' to 'LIKE' and modified the tests to check for case insensitivity.

Replace db_like() with Database::getConnection()->escapeLike($string)
Also change the LIKE statements to use Connection::mapConditionOperator.

I haven't done these here as seems not needed here, and they are already in scope for #2784739: Fix PostgreSQL operator in views .

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris commented
Status: Needs review » Reviewed & tested by the community

@jonathanjfshaw I agree, the db_like can be removed in #2784739: Fix PostgreSQL operator in views.

+++ b/core/modules/views/src/Tests/Plugin/DisplayEntityReferenceTest.php
@@ -103,6 +103,21 @@ protected function setUp() {
+        EntityTest::create([

tiny nitpick, indent too big.

  • catch committed f6b3dd9 on 8.3.x 
    Issue #2809555 by jonathanjfshaw, alexpott, amateescu, Lendude, catch:...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

  • catch committed 1f36858 on 8.2.x 
    Issue #2809555 by jonathanjfshaw, alexpott, amateescu, Lendude, catch:...

Status: Fixed » Closed (fixed)

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