When a complex data property contains another complex or list data type, its value renders empty in a view.
For instance, given the following data definition map:
data => ListDataDefinition [
itemDefinition => ComplexDataDefinition [
propertyDefinitions => [
id => DataDefinition,
values => ListDataDefinition [
itemDefinition => ComplexDataDefinition [
propertyDefinitions => [
property_1 => DataDefinition,
property_2 => DataDefinition,
]
]
]
]
]
]
"data" is a list of maps with two properties, "id" and "values". The "values" property is another list of maps with two properties, "property_1" and "property_2".
A view with a Search API field for "property_1" renders empty.
Extract values from complex data properties recursively.
None.
None.
None.
TBD.
Comments
Comment #2
manuel.adanAdded the 'filter' module as required for related tests that would fail when deeping into text_format properties in text fields definitions, it's actually a 'text' module dependency.
Comment #3
manuel.adanAdded the filter module to the failed test that requires the text module as well.
Comment #4
drunken monkeyThanks for reporting this and already providing a patch!
I don’t think I have really encountered such a structure before, but your description does make sense. However, I’m not quite sure how the problem description relates to the patch? The patch seems to only solve the problem if the main property is implicitly used for a field, but your description sounds as if you’ve specified “property_1” directly, so it should have the complete property path.
Can you tell me at which point exactly this fails without your patch?
The patch otherwise looks very good, no complaints there. (Just a pure matter of taste for my attached revision, didn’t find anything else.)
As you already write, though, we should have test coverage for that. Would you be able to provide some? I don’t really know how to get nested complex data definitions – is that even possible with just Core, or would we need some custom test properties for that? (In any case, I think it should be enough to test just the part we fixed. No need to set up a test view for it. Unless that wouldn’t be much harder.)
(Writing a test would of course also be a great way to help me understand the problem, and how it’s solved by this patch.)
Comment #5
manuel.adanThanks for you quick review ;)
I found this bug working on the implementation of a Search API data source for the Elasticsearch Connector module (#3102388: Add Search API datasource plugin for externally-created indicies (8.x-7.x)). Elasticsearch can provide deep complex data structures, in particular, I mapped multi-fields as a complex data type (based on Map), linking the main property with the base value of the Elastic field and adding additional properties to the map for each extra field. Without the patch, I got empty values in views in these cases.
A similar structure could be recreated for testing purposes, I'll try to do so in my next contrib time.
Comment #6
drunken monkeyAwesome, thanks a lot!
Comment #7
manuel.adanTest added, based on the example use case given in the description.
Comment #10
drunken monkeyGreat job, thanks a lot! And sorry for taking so long to get back to you!
The test looks great, and perfectly demonstrates what’s currently not working. Just had a few cosmetic changes (the “worst” one being that
assertEqual()is deprecated), but don’t think they require review.So: fixed those and committed.
Thanks a lot again!