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.The data-selection based views fields don't add the entity-id to the query, different to what the comment says:
/**
* Add the field for the entity ID (if necessary).
*/
public static function query($handler) {
// Some of the parent handlers might require this.
$handler->field_alias = $handler->real_field;
$handler->base_field = self::get_selector_field_name($handler->real_field);
}
We should probably take that part over from the views entity field handler.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | entity_views_query_fix.patch | 13.63 KB | fago |
| #7 | entity_views_query_fix.patch | 9.95 KB | fago |
| #6 | entity_views_query_fix-5.patch | 9.21 KB | mh86 |
| #5 | entity_views_query_fix-4.patch | 8.46 KB | drunken monkey |
| #1 | entity_views_query_fix.patch | 8.44 KB | fago |
Comments
Comment #1
fagook, attached is a fix.
When copying over the code I noted we are using already a 'base_field' property, such as views but with another meaning.. :/ That's bad and would have resulted in a clash in the helpers query() method, so I've removed that invented base_field as it doesn't appear to be often used anyway.
Tested attached patch - it fixes the bug. Misses search api views integration test, though I've looked it up and it doesn't seem to make use of that base_field. Anyway, please test.
Comment #2
mh86 CreditAttribution: mh86 commentedtested the patch on 2 websites with pretty complex views and everything still seems to work.
Comment #3
drunken monkeyFatal errors and completely messed-up display – please don't commit yet!
Comment #4
fago>Fatal errors and completely messed-up display – please don't commit yet!
ok, waiting for more feedback on what's wrong...
Comment #5
drunken monkeyFirst off, the attached patch fixes a stray
self::get_selector_field_name()inentity_views_handler_field_optionswhich leads to fatal errors when displaying a view which uses that field handler.Secondly, but – as I see now – unrelated to this, commit e29298c messed up
entity_views_handler_field_entity. The entity now always uses the entity type of the current view, not of the referenced field. So, e.g., when I have a node search displaying the nodes' tags, and a node has a tag with the tid 1, then a link to the node with NID 1 is displayed.This has definitely got to be fixed before any stable release.
Comment #6
mh86 CreditAttribution: mh86 commentedReverted the entity_views_handler_field_entity handler to use the field's entity type instead of the current view's entity type. Additionally added the entity type information to the new entity field handler that was added in #1209678: Add a Views field handler to display rendered entities.
Furthermore, I was wondering if there is still something in common between entity_views_handler_field_entity and views_handler_field_entity?
Comment #7
fagothanks, renewed the patch. I've removed the entity type from the definition, as having type and entity type there is kind of duplicated. Instead, it's initialized during init().
Good point, changed that.
Please test.
Comment #8
mh86 CreditAttribution: mh86 commentedfrom my point of view, this is ready (tested it with some views), but I'm leaving it up to Thomas to mark it as RTBC.
Comment #9
drunken monkeyNope, sorry, it still has the mentioned (second) problem.
Comment #10
drunken monkeyEntityFieldHandlerHelper::pre_render()resets$handler->entity_typeback to the wrong type.This is caused by you calling
get_result_entities()without relationship, thereby of course setting the type back to the base entities'. I don't really know why you made that change in the other issue in the first place, so I can't really tell how to best fix this.From what I can see, we could either override
entity_views_handler_field_entity::pre_render()with a slightly modified version ofEntityFieldHandlerHelper::pre_render(); or we could make the behaviour of the third parameter toget_result_entities()(andget_result_wrappers()) a requirement and just append':entity object'to the handler's$real_fieldproperty.Comment #11
fagoHm, partly I think $handler->entity_type is populated wrong as get_result_wrappers() returns the wrappers type. Maybe we should just use $handler->type or $handler->data_type else?
I don't think we need to populate $handler->entity_type unless for the entity handler, where we can then make sure it's right one. Thoughts?
Comment #12
drunken monkeyI don't think that's true, as the property is used for all handlers in the
EntityFieldHandlerHelper::render_entity_link()method. Or am I misunderstanding you there?Basically, as said, I don't know why you changed it in the first place, so I can't really say what fix would be appropriate or the best. Otherwise, we could just revert the changes that introduced the bug.
Comment #13
fagouhm, sry forget #11 - I was wrong.
Indeed, instead I worked on fixing that by introducing a $field_entity_type and improved docs on to which entity $entity_type belongs. See attached patch.
I tested rendering the node:author, what worked . Even for anonymous authors! :D
Comment #14
drunken monkeyGreat job, seems to work fine for me. (Didn't test everything, though, so I'll just trust you didn't mess up anything else now. ;P)
Comment #15
mh86 CreditAttribution: mh86 commentedlooks good to me as well
Comment #16
fagoCommitted :)