Feeds mapping to entity label is broken if candidate items from multiple bundles exists, for it will always select from the ...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Eric_A created an issue. See original summary.

Eric_A’s picture

Status: Active » Needs review
FileSize
622 bytes
Eric_A’s picture

Wait a minute, there's more...

Eric_A’s picture

Status: Needs review » Needs work

The selection needs filtering on bundle.

Eric_A’s picture

Status: Needs work » Needs review

Sorry, that change was mistaken. Back to needs review.

rolfmeijer’s picture

The patch form comment 2 does not apply to 7.x-1.5 anymore, because the line $options = reset($options); is already removed. I’ve made a new patch, based on 7.x-dev from 16 Aug 2017.

Eric_A’s picture

Assigned: Eric_A » Unassigned

Thanks for taking over, @rolfmeijer.
Around 1.2 the reset($options) was moved into a conditional. If you don't kill this reset() anymore I think the code won't find the matching entities anymore. It didn't when I made the first patch, if I remember correctly.
I'd say the fix should be moved inside the conditional as well, *including* the killing of $options = reset($options) or the reset()will now return a scalar and the next line does an array_keys() on it to get entity IDs, and then do a reset() to get the first ID.. That reset() line being there for a long time IMO means this code is not working correct for a long time, given that it will only select from the first bundle.

Eric_A’s picture

Title: Feeds mapping broken (getReferencableEntities() now keyed by bundle) » Feeds mapping by label broken with multiple bundles
Eric_A’s picture

Issue summary: View changes

To be honest, I'm not so sure anymore if my patch was actually changing selection behavior. Before 1.2 the patch would prevent the errors mentioned in #1943300: array_keys() and reset() notices in entityreference_feeds_set_target() during feed import if entity doesn't exist I think, but it would not actually change selecting the first entity from the list, because the first entity would be the first entity below the first bundle key created, I think. (Of course it is true that we then totally depend on what appears to be an implementation detail.) For clarity it would make sense to move the comment // Use the first matching entity. two lines up then, but using options_array_flatten() and a reset() instead of two times a reset() does not really change behavior I think. (Other then introducing a dependency on the options module...)

mariusm’s picture

Hello,

The solution does not solve all the problems.

I have label 78.

For 78 is returned :

5078 ( with id_1 )
78 ( with id_2 )
7809 ( with id_3 )

With reset ... the program returns id_1 but should return id_2 .

Thanks.