Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The class EntityConverter should be using hasDefinition()
instead of getDefinition()
.
See https://github.com/socketwench/flag-drupal8/pull/22, when a route is defined with a non-existent entity type this will fail.
Comment | File | Size | Author |
---|---|---|---|
#1 | entity-converter-uses-getDefinition-2295647-1.patch | 747 bytes | ja_ca |
Comments
Comment #1
ja_ca CreditAttribution: ja_ca commentedFixed.
Comment #2
ja_ca CreditAttribution: ja_ca commentedComment #3
dawehnerSeems legit.
Comment #4
BerdirCould possibly use a unit test but I'm not sure how useful that would really be, a unit test would *not* have caught this bug, because we changed the behavior of getDefinition() since this was written and nobody would have thought to update a mock implementation of getDefinition() to throw an exception.
Maybe an integration test, but again, there's not really something that you can test for other than being able to build the routes and it not resulting in a exception that aborts the whole process (which shouldn't be possible, but that's another topic again), but:
What's very weird and we couldn't really explain is that this only happened if applies() was called through the ViewsUI enhancer and not when called directly.
Comment #5
dawehnerWell yeah some sort of integration test could be usuable, do you know exactly how this happened? What let the views UI enhancer
upcast with the wrong entity type? This sounds like a case when route rebuild happens before the entity definition (static/non-static) cache is cleared.
Talking about the views UI one, I really don't like the fact that it is extending the entity one, this is a misuse of inheritance if you ask me.
Comment #6
BerdirWhat happened is that there was bundle_entity_type = "invalid_entity_type" annotation key on an entity type, that resulted in an explicit type => 'entity:invalid_entity_type' in the field_ui manage fields route (as it needs to be defined manually because the defaults don't apply for routes not defined in routes.yml). Then it tried to check if that can be enhanced.
That said, I don't know why it only happend through ViewsUIEnhancer, the problem did not occur in tests even if they enabled field_ui.
Comment #7
alexpottCommitted 013a6e8 and pushed to 8.x. Thanks!