Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Using the default node index, in the Fields page you get an additional field for 'type', which is an entity reference field that references the 'node_type' config entity type.
When you try to enable this additional field, you get a fatal error:
Fatal error: Call to undefined method Drupal\node\Entity\NodeType::baseFieldDefinitions() in /home/andrei/work/d8.dev/core/lib/Drupal/Core/Entity/EntityManager.php on line 358
because config entity types are not using the typed data system.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | interdiff.txt | 1.09 KB | amateescu |
| #10 | 2242983-10.patch | 3.73 KB | amateescu |
Comments
Comment #1
amateescu CreditAttribution: amateescu commentedThis patch should fix it, but we also need some tests for this.
Comment #2
drunken monkeyThis is definitely a core bug – if it has the method in the interface, it should at least not just die horribly.
See #2228721: TypedData doesn't support config entity types; flag errors if you try (would be great if you could provide feedback there, and maybe help a (at least temporary) solution get committed). But in the end, I hope config entities will also support some kind of introspection. (Do you maybe know if there is currently work on this?)
Because of this, I don't think we should work around this for now, or at least clearly mark it as a TODO.
Comment #3
BerdirConfig entities will not support typed data any time soon, there are no quick workarounds. There might be a way to do it using the adapter approach that we're working on and based on the config schema, but even then, they will not have *fields*.
Comment #4
drunken monkeyHm, OK, thank you.
It's clear that it won't have fields, but the method is a) in the base entity interface and b) called
getPropertyDefinitions(), so I don't see any problem there. And because of a), I'd expect this to be fixed in some way before a stable release. But, of course, I'm much less informed than you, so if you think it will rather be moved down to the content entity interface, then we can also put an explicit check in.In any case, I'd wait until core has made up its mind before doing more than a temporary workaround here, and we definitely should mark it
@todo.Comment #5
amateescu CreditAttribution: amateescu commentedThe problem with "not working around it" in
convertPropertyDefinitionsToFields()is that even if the issue above is fixed in core and this code$nested_properties = $property->getPropertyDefinitions();will throw an exception when you get to try and find the sub-properties of the 'entity' field type property, the parent call to convertPropertyDefinitionsToFields() will still add the reference field to the list of additional fields, but enabling it won't do anything at all.That behavior will cause at least a bit of confussion, IMO, that's why I prefered the approach in this patch, it was one of many other things I tried :)
Comment #6
amateescu CreditAttribution: amateescu commentedOn the other hand, a @todo is fair anyway.
Comment #7
BerdirNope, getPropertyDefinitions() is not on EntityInterface, not even on ContentEntityInterface anymore, content entities have getFieldDefinitions() only.
getPropertyDefinitions() still exists, but it is on ComplexDataDefinitionInterface, so you call it on the definition object, not the actual data object (entity/field).
I think core's mind is pretty much made up on this, the only remaining piece is #2002138: Use an adapter for supporting typed data on ContentEntities, which introduce an adapter that something could then also implement for config entities but I'm not sure if that will be in core.
Comment #8
amateescu CreditAttribution: amateescu commentedBetter interface to check based on #2228721-4: TypedData doesn't support config entity types; flag errors if you try.
Comment #9
drunken monkeyAh, OK, sorry. In any case, my point was that, according to the interface, this should be supported by all entities (since their definitions implement
ComplexDataDefinitionInterface). But if this will change soon, then yes, let's just add a short workaround, if you want, and get back to it later once this is more or less stable in core.Comment #10
amateescu CreditAttribution: amateescu commented@drunken monkey, please see #5 on why I think we also need this check in Search API, regardless of how this gets fixed in core.
Added a small test and committed the patch attached.
Comment #11
drunken monkeyWith "fixed in core" I didn't mean the temporary fix of throwing an exception, but the proper fix of implementing this functionality. But in any case, I'm fine with a workaround.
Comment #16
drunken monkey