Change key to use when looking up entities

[TLyngeJ]

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.

[jonathanjfshaw]

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.

[pfrenssen]

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.

[TLyngeJ]

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?

[pfrenssen]

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

[kmcculloch]

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

[jonathanjfshaw]

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

[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?

[jhedstrom]

Is this resolved now that #117 is merged?

[jonathanjfshaw]

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

[jhedstrom]

#241 was merged. Thanks all!