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.
create a view with 2 "rendered entity" fields from different relations (in my case users)
expected: show different results
experienced: always show the same result from the first field
i may conjecture (but did not hunt down) this is a result of #1796102: Views caching of rendered entity field misarchitected, leading to unexpected results.
Comment | File | Size | Author |
---|---|---|---|
#14 | 1796110-14--views_relationships--try1.patch | 3.16 KB | drunken monkey |
#14 | 1796110-14--views_relationships--try2.patch | 4.01 KB | drunken monkey |
#3 | entity-1796110-3.patch | 2.53 KB | corvus_ch |
#2 | entity-1796110-2.patch | 2.31 KB | corvus_ch |
Comments
Comment #1
geek-merlinstill an issue with todays dev.
crosslinking dup: #1470916: Trying to render multiple files from different relationships in view results in first added field being rendered multiple times
Comment #2
corvus_ch CreditAttribution: corvus_ch commentedComment #3
corvus_ch CreditAttribution: corvus_ch commentedThe above code changes the internal data structure. This fact has broken my own code.
This is what I have inside a preprocess targeted for a view.
Fix for that when using the previous patch
The patch attached to this comment does not alter the internal structure.
Comment #4
BerdirNot sure if #3 is the better approach.
- Custom code like the example above *will* break anyway once you use relationships and that code is doing it wrong.
- The patch introduces potential conflicts, imagine you have two fields, one without relationship and the other with a relationship called 'entity object'.
Comment #5
fagoI agree with #4, let's better do #2. I'd not expect there's a lot of custom code that relies on this.
Does #2 work for you without problems?
Comment #6
corvus_ch CreditAttribution: corvus_ch commentedBoth patches do work. Patch number #2 will need some work on another module maintained by me but that won't be a problem.
Comment #7
geek-merlin#2 looks great and imho does it right. thanks corvus!
Comment #8
BerdirI'm also +1 on #2. Can't RTBC as @corvus_ch and myself basically wrote those patches together but @axel.rutz or someone else is welcome to do that :)
Comment #9
geek-merlinFinally i found time to look into this.
Applied #2 and the problem from the issue summary is solved.
Setting RTBC.
Comment #10
geek-merlinCrosslinking #2068761: "Rendered entity" views field handler does NOT display revisions which may or may not be connected to this.
Comment #11
fagoThanks for checking - I fixed a small coding style glitch and committed #2.
Comment #13
drunken monkeyThis change completely broke passing data directly to the view with Search API Views (and probably anyone else using this feature). How else should it be if you introduce an additional nesting level without adapting the data to it in any way?
Please add some code to adapt the additionally passed
_entity_properties
to this new format!I also don't really understand the original bug. The
$selector
previously used as the key should already contain the relationship. If that's not the case, or there are conflicts in some cases, then that's the issue to fix! Completely breaking this part of the functionality certainly isn't the way to do it, at least unless there's no other way (and, in that case, you should adapt the originally passed values, which should be very easy – as said).Comment #14
drunken monkeyLooking at this again, I can't believe any part of relationship handling ever worked before. It seems there were two, three or more bugs canceling each other out in some situations.
I don't know what primate coded this garbage, but he was certainly inebriated.
Anyways, the previous patch introduced a fourth bug that canceled the others out in some other situations, but that's of course not optimal either. Attached are two patches that try to fix this mess – since I wasn't able to really reproduce this locally, I have to guess what could perhaps work. Please test whether one of these works (applied to latest dev, i.e., with the previous patch committed)!
There might actually be a problem with the Views default query plugin's
get_result_entities()
implementation that causes (part of) this, too, but I can't really say. The second patch would just remove the relationship argument from that function call and would rely on wrappers solely (which makes sense in my opinion either way).Anyways, it all comes down to the relationship not properly included in the selector, and the same handler seemingly being used for the same field on different relations (would be another Views bug, in this case). We have to either get the correct entity/wrapper right away (and, thus, avoid using the same handler for two instances of the same field – if that's possible; otherwise, we'd have to save the wrappers keyed by relationship, or something like that) and remove the relationship from the selector (as it currently is, but simply down to a bug), or just always get the base entity and use the selector to get to the right nested entity (which should always work, regarding of Views bug, but would lead to unnecessary loads if Views already fetches the necessary entity IDs).
Comment #15
drunken monkeyCould someone please test/review? Otherwise, the bug could be reintroduced by these patches but still be committed, which of course wouldn't be good either.
Comment #16
das-peter CreditAttribution: das-peter commentedHoly cow, Batman! This "fix" just broke my search api views, leaving me crying in a corner of my office.
From my very narrow perspective I can't say which of the patches from #14 is the better one.
I usually advocate to forward all available information - in this light I'm in favour of try1, since this keeps the forwarding of the relationship information to
$handler->query->get_result_entities()
.The other part of the patch looks fine to me, building a "unique" selector (including the relationship info) seems like a good way to handle this.
I set this to RTBC, but I think it needs a review of fago anyway - and it's more likely he takes a look with the status set to RTBC :P
fyi: I'll integrate this patch into our next extended test cycle of a project that starts tomorrow. If there are any trouble I'll report them here.
Comment #17
fagoindeed - quite a mess. I agree that patch #2 should be the right variant as else the relationship could get applied twice. Unless we've relationships that are no entity relationships? I guess the code doesn't account for that case in anyway.
Howsoever, as #11 has shown (sry for that) I've an incomplete knowledge of that simple code so I trust the monkey's opinion and go with #14-2.
Thanks das-peter for testing and sry for making you cry ;) The lack of further testers regarding the original bug is unfortunate, but I guess people will chime in as soon as this really breaks something :-P
Committed, #14 try2. Thanks!
Comment #18
jaydub CreditAttribution: jaydub commentedI updated to the new Entity API release today and now several Solr / Search API views based pages no longer are showing a field value in this case an image field which is via a reference to the entity that is in the Search API index.
If I only downgrade entity_views_field_handler_helper.inc to the previous commit or the one before that, everything is fine again.
So at least for me the last patch in #14 which went in two days ago seems to break everything for me.
I will trying looking into it to see if I can get from the last commit to the patch in #14 and get it to work again but for the moment I wanted to reopen this issue in case others are affected.
Comment #19
jaydub CreditAttribution: jaydub commentedIn my particular case if I make only one change and one change only my pages are fine again:
In method pre_render in the last committed version of entity_views_field_handler_helper.inc
before
after
The diff of this file from the last release is:
http://drupalcode.org/project/entity.git/blobdiff/c1a949c89aac9bedf11fa8...
Comment #20
FrancescoUK CreditAttribution: FrancescoUK commentedYes, I'm affected by this issue and applying #19 I now can see again the images on my website.
I use Drupal Commerce with views showing images coming from product variations.
If there is a better fix than reverting only entity_views_field_handler_helper.inc to 1.2 please let us know.
Comment #21
hlykos CreditAttribution: hlykos commentedSame issue and reverting
list($handler->entity_type, $handler->wrappers) = $handler->query->get_result_wrappers($values, NULL, $handler->real_field);
back to
list($handler->entity_type, $handler->wrappers) = $handler->query->get_result_wrappers($values, $handler->relationship, $handler->real_field);
and
list($handler->entity_type, $entities) = $handler->query->get_result_entities($values, NULL, $handler->real_field);
back to
list($handler->entity_type, $entities) = $handler->query->get_result_entities($values, $handler->relationship, $handler->real_field);
fixes it.
Comment #22
Beanjammin CreditAttribution: Beanjammin commentedI am seeing the same problem that jaydub posted in #19, with the same changes correcting it.
Earlier I had create a separate issue here https://drupal.org/node/2170557
Comment #23
derekw CreditAttribution: derekw commentedSame problem, same fix! Also using Drupal Commerce
Comment #24
hobbes_VT CreditAttribution: hobbes_VT commentedI have notice the same issue and the above changes mentioned in #21 replacing the NULL parameters fixed my issue as well.
However, in my case some views were not working, which built on relationships to profile2 fields - specifically the profile URL came back as the user account URL instead of the one pointing to the separate page.
Will this be fixed in the next update, or will I have to apply this patch to future updates as well for not to break my views?
Comment #25
David_Rothstein CreditAttribution: David_Rothstein commentedThis also seems to likely be the cause of #2171689: EntityMalformedException: after upgrade from 7.x-1.2 to 7.x-1.3, which is leading entire pages to break when testing out this update.
Comment #26
das-peter CreditAttribution: das-peter commentedCould anyone experience issues with the current version test the patch try-1 from #14 (
1796110-14--views_relationships--try1.patch
)?I tested only try-1 one because I didn't like to set the relations parameter to
NULL
(see #16). So if removing the relations really is the issue we could use try-1 instead try-2 then.Comment #27
Eric_A CreditAttribution: Eric_A commentedI did and posted results in #2171689: EntityMalformedException: after upgrade from 7.x-1.2 to 7.x-1.3.
Comment #28
milesw CreditAttribution: milesw commentedTracked my problems down to the same as #21. Created a new issue to be more findable.
Patch over here:
#2174645: Fields using views relationships show no values (using Search API)
Comment #29
joachim CreditAttribution: joachim commentedLet's return this to its closed state.
While it seems the bug was introduced here, now that this is committed, this should be fixed in a new issue: #2171689: EntityMalformedException: after upgrade from 7.x-1.2 to 7.x-1.3 (which has a patch, and is RTBC).