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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

geek-merlin’s picture

corvus_ch’s picture

Assigned: Unassigned » corvus_ch
Status: Active » Needs review
FileSize
2.31 KB
corvus_ch’s picture

FileSize
2.53 KB

The 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.

$data['result'][$key]->_entity_properties['entity object'];

Fix for that when using the previous patch

$data['view']->display_handler->handlers['field']['rendered_entity']->get_value($data['result'][$key];

The patch attached to this comment does not alter the internal structure.

Berdir’s picture

Not 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'.

fago’s picture

I 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?

corvus_ch’s picture

Both patches do work. Patch number #2 will need some work on another module maintained by me but that won't be a problem.

geek-merlin’s picture

#2 looks great and imho does it right. thanks corvus!

Berdir’s picture

I'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 :)

geek-merlin’s picture

Status: Needs review » Reviewed & tested by the community

Finally i found time to look into this.

Applied #2 and the problem from the issue summary is solved.

Setting RTBC.

geek-merlin’s picture

Crosslinking #2068761: "Rendered entity" views field handler does NOT display revisions which may or may not be connected to this.

fago’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for checking - I fixed a small coding style glitch and committed #2.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

drunken monkey’s picture

Status: Closed (fixed) » Active

This 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).

drunken monkey’s picture

Looking 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).

drunken monkey’s picture

Could 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.

das-peter’s picture

Assigned: corvus_ch » Unassigned
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Holy 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.

fago’s picture

Status: Reviewed & tested by the community » Fixed

indeed - 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!

jaydub’s picture

Status: Fixed » Active

I 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.

jaydub’s picture

In 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

    if (method_exists($handler->query, 'get_result_wrappers')) {
      list($handler->entity_type, $handler->wrappers) = $handler->query->get_result_wrappers($values, NULL, $handler->real_field);
    }
    else {
      list($handler->entity_type, $entities) = $handler->query->get_result_entities($values, NULL, $handler->real_field);
      $handler->wrappers = array();
      foreach ($entities as $id => $entity) {
        $handler->wrappers[$id] = entity_metadata_wrapper($handler->entity_type, $entity);
      }
    }

after

    if (method_exists($handler->query, 'get_result_wrappers')) {
      list($handler->entity_type, $handler->wrappers) = $handler->query->get_result_wrappers($values, $handler->relationship, $handler->real_field);
    }
    else {
      list($handler->entity_type, $entities) = $handler->query->get_result_entities($values, $handler->relationship, $handler->real_field);
      $handler->wrappers = array();
      foreach ($entities as $id => $entity) {
        $handler->wrappers[$id] = entity_metadata_wrapper($handler->entity_type, $entity);
      }
    }

The diff of this file from the last release is:

http://drupalcode.org/project/entity.git/blobdiff/c1a949c89aac9bedf11fa8...

FrancescoUK’s picture

Yes, 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.

hlykos’s picture

Same 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.

Beanjammin’s picture

I 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

derekw’s picture

Same problem, same fix! Also using Drupal Commerce

hobbes_VT’s picture

I 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?

David_Rothstein’s picture

This 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.

das-peter’s picture

Could 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.

Eric_A’s picture

Could anyone experience issues with the current version test the patch try-1 from #14 (1796110-14--views_relationships--try1.patch)?

I did and posted results in #2171689: EntityMalformedException: after upgrade from 7.x-1.2 to 7.x-1.3.

milesw’s picture

Tracked 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)

joachim’s picture

Status: Active » Fixed

Let'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).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.