DrupalDriver
DrupalDriver copied to clipboard
Rely on ids + support autocreate.
The current implementation looks bugged. At the moment, you never run line 28 if we are testing line 21 for an empty target bundle. If the field is filtering a specific bundle of taxonomy terms for instance, you want to set the value of $target_bundle_key when you have multiple bundles too. Steps to reproduce the issue, create two taxonomy terms with the same name in two different vocabularies, let's say voc1 > item & voc2 > item. Tid are respectively 1 and 2. Now, try to create a node through the Drupal 8 behat driver that has an entity reference field with a configuration of entity type taxonomy terms and vocabulary "voc2" as target bundle. Your node should be created but with the wrong term ID because there will be a match to the "item" term that has the tid 1 instead of the tid 2 expected. If we reference entities like medias, names are useless. Relying on entity ids is more logical and helps deduplicate the issue mentionned above. We also need to support auto created entities like taxonomy terms. If the term is not known, do not throw the exception.
The current implementation looks bugged. At the moment, you never run line 28 if we are testing line 21 for an empty target bundle. If the field is filtering a specific bundle of taxonomy terms for instance, you want to set the value of $target_bundle_key when you have multiple bundles too. Steps to reproduce the issue, create two taxonomy terms with the same name in two different vocabularies, let's say voc1 > item & voc2 > item. Tid are respectively 1 and 2. Now, try to create a node through the Drupal 8 behat driver that has an entity reference field with a configuration of entity type taxonomy terms and vocabulary "voc2" as target bundle. Your node should be created but with the wrong term ID because there will be a match to the "item" term that has the tid 1 instead of the tid 2 expected.
Sorry Artusamak, I understand the problem you're describing but not what the bug is in the code that's causing it. If $this->getTargetBundles()
on line 21 returns 'voc2' then why does line 28 not run?
If we reference entities like medias, names are useless. Relying on entity ids is more logical
How do you imagine this fitting with BDD? At the moment a Behat step author can reference an entity by name in a human-readable table. That's important. Wouldn't the change you propose break those steps in the Drupal Extension?
Ultimately I think this is yet another case where we need plugins in the Driver. I had imagined plugins for entities and (better) plugins for fields, but looks like maybe we need plugins for references as well. That way a developer could add a plugin to their project that modified the logic used when referencing a particular entity type from a particular field on a particular entity type or whatever.
Using the label addresses the 90% use case but is not perfect. We've already got a case in another PR where we're adding hardcoded logic to handle the fact that the user entity don't have a label key and needs to be using 'name'.
I thought media entities in D8 have a label? I agree it's rather awkward though and often hidden from users and so it can violate BDD principles to be using it.
We also need to support auto created entities like taxonomy terms. If the term is not known, do not throw the exception.
That seems right from a BDD perspective. At the moment the referenced entities have to be previously created. Your fix makes writing a step closer to the experience of the site user. I didn't know this actually worked in Drupal, that you could autocreate through the API in this way.
See also See also #94 and #117 for discussions addressing this problem