Support from Acquia helps fund testing for Drupal Acquia logo

Comments

blueminds’s picture

Status: Active » Needs review
FileSize
3.71 KB

Please see the attached ported fix.

amateescu’s picture

Title: Autocomplete widgets not referencing the single entity result » Autocomplete widget doesn't work on entity labels without a " (entity_id)" suffix
Component: field system » entity_reference.module
Status: Needs review » Needs work

Heh, just when I thought we had good coverage for our widgets :/

  1. +++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/entity_reference/selection/SelectionBase.php
    @@ -220,7 +220,11 @@ public function validateReferenceableEntities(array $ids) {
    +    foreach($bundled_entities as $entities_list) {
    

    Missing a space here :)

  2. +++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Plugin/entity_reference/selection/SelectionBase.php
    @@ -220,7 +220,11 @@ public function validateReferenceableEntities(array $ids) {
    +      $entities = $entities_list;
    

    Shouldn't this be += ?

  3. --- a/core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceSelectionAccessTest.php
    +++ b/core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceSelectionAccessTest.php
    

    This class tests something else entirely, all the widget tests are in EntityReferenceIntegrationTest ;)

blueminds’s picture

Status: Needs work » Needs review
FileSize
3.83 KB

1. fixed
2. there is always only one entity type as we can reference only entities of one type. Not sure how to make it more clear.
3. moved

Berdir’s picture

Status: Needs review » Needs work

2. Those aren't entity types, they are bundles. Like article and page. We explicitly want to merge them together, not drop them as you do right now.

blueminds’s picture

Status: Needs work » Needs review
FileSize
3.83 KB

oh, right, i see

amateescu’s picture

I think it would be much easier to test this by updating the input values inserted a few lines above in that method. Something like this interdiff :)

The last submitted patch, 6: autocomplete_ref_fix-2247211-6-test-only.patch, failed testing.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Test coverage looks good to me. I worked on the initial patch in the 7.x issue but I think @amateescu would have said something if the fix wouldn't be OK :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: autocomplete_ref_fix-2247211-6.patch, failed testing.

amateescu’s picture

amateescu’s picture

Status: Needs work » Reviewed & tested by the community

Bot fluke, back to RTBC.

xjm’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 6e7dfbc and pushed to 8.x. Thanks!

  • Commit 6e7dfbc on 8.x by alexpott:
    Issue #2247211 by blueminds, amateescu, xjm: Fixed Autocomplete widget...

Status: Fixed » Closed (fixed)

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