DrupalDriver icon indicating copy to clipboard operation
DrupalDriver copied to clipboard

Change key to use when looking up entities

Open TLyngeJ opened this issue 8 years ago • 9 comments

I've changed the identifier to use, when looking up entities, in Drupal/Driver/Fields/Drupal8/EntityReferenceHandle:expand() from label to id.

Not sure why label was used. Anyway, didn't work and this one seems to.

TLyngeJ avatar Jul 14 '16 20:07 TLyngeJ

I think label is used so that steps can reference existing entities by their label, rather than a numeric id which is less natural for step authors.

jonathanjfshaw avatar Jan 02 '17 17:01 jonathanjfshaw

This is correct, we shouldn't use the label here but the entity ID. This was probably added as part of a Behat use case as @jonathanjfshaw suggests, but in the scope of DrupalDriver it doesn't make sense to use labels.

So yeah we need to fix this but the problem is that this breaks backwards compatibility. All current code relies on the value being a label. Marking this for the 2.x milestone.

pfrenssen avatar Jan 03 '17 13:01 pfrenssen

How about, in the mean time, try and load the entity ID, if that returns nothing, try and look up the entity by it's label?

TLyngeJ avatar Jan 03 '17 13:01 TLyngeJ

OK I like this idea! That would work without breaking backwards compatibility.

pfrenssen avatar Jan 03 '17 14:01 pfrenssen

I just want to give this patch a bump. I'm trying to use the driver to create nodes with an entity reference field that points to users, and the entity definition object for users does not contain a "label" key. Apparently "id" is the only key that's guaranteed to be there: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21EntityType.php/function/EntityType%3A%3AgetKeys/8.2.x

kmcculloch avatar Sep 15 '17 22:09 kmcculloch

See also See also #117 and #133 for discussions addressing this problem

jonathanjfshaw avatar Sep 16 '17 07:09 jonathanjfshaw

@pfrenssen

but in the scope of DrupalDriver it doesn't make sense to use labels.

It seems that the whole point of the current field handlers is to convert from natural language strings into Drupal-friendly variables ready to be set in Drupal fields. What you're saying here would seem to imply that the whole field handler mechanism should be moved into the Drupal extension. Or am I missing something?

jonathanjfshaw avatar Sep 16 '17 07:09 jonathanjfshaw

Is this resolved now that #117 is merged?

jhedstrom avatar Dec 12 '17 19:12 jhedstrom

Not necessarily, there's a bigger architectural issue here about how responsibilities are divided.

jonathanjfshaw avatar Dec 12 '17 20:12 jonathanjfshaw

#241 was merged. Thanks all!

jhedstrom avatar Feb 28 '23 20:02 jhedstrom