I have a custom content type with some fields. I would like to create a better overview of the changes between the revisions, so I have created a node revision view.

However if I try to run it, I get a fatal error in entity_get_controller(). The problem is that it is get called with an empty string. I was successfully able to trace back the problem to views_handler_field_field.inc:78.

$this->entity_type = $entity_type = $this->definition['entity_tables'][$base_table];

The problem here is that the definition only holds info for the node entity, while the $base_table is node_revision, so $this->entity_type and $entity_type will be NULL.

CommentFileSizeAuthor
#5 1056266-field-revisions-broken.patch5.44 KBmerlinofchaos
#3 fatal-revision-view-1056266.patch4.69 KBchsoney

Comments

bojanz’s picture

bojanz commented

Yeah, field revisions are kinda broken at the moment :/

dawehner’s picture

dawehner
Primary language German
commented
Priority: Normal » Critical

This seems to be a real critical issue :(

chsoney’s picture

chsoney commented
StatusFileSize
new fatal-revision-view-1056266.patch4.69 KB

I created a fix for this, but it seems a little to much like a hack. There is a huge assumption in my code stating if an entity type does not exist, then it is revision for an entity type. Also, I have to load each node/revision separately.

chsoney’s picture

chsoney commented
Status: Active » Needs review
merlinofchaos’s picture

merlinofchaos commented
StatusFileSize
new 1056266-field-revisions-broken.patch5.44 KB

Ok, this is, I think, the right way to fix this without being hacky.

Status: Needs review » Needs work

The last submitted patch, 1056266-field-revisions-broken.patch, failed testing.

bojanz’s picture

bojanz commented
Status: Needs work » Needs review

Very nice. I'm just leaving the appartment so I can't test, but it looks clean, and it can't be more broken than the previous code, so if it works, consider this my RTBC :)

Also, passing the base table through the definition is just what I need for the Per Bundle storage Views integration.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

As written in irc it would be cool if there would be an explanation why iterate about each entity_id but in general it's rtbc.

merlinofchaos’s picture

merlinofchaos commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed, despite what testbot thinks.

jamsilver’s picture

jamsilver commented
Status: Fixed » Needs work

Just downloaded the 7.x-3.0-beta3 release and am getting the following PHP notice:

Notice: Undefined index: revision table in field_views_field_default_views_data() (line 182 of .../views/modules/field.views.inc).

On dsm-ing out the code at that point - it looks like the above fix is incompatible with a field that appears on more than one entity type. In my case I have a field which is present on a node *and* on a term. To be specific, it would appear that the following lines (177 - 182):

      'entity id' => $entity_info['entity keys']['id'],
      'base table' => $entity_info['base table'],
    );
    if ($type == FIELD_LOAD_REVISION) {
      $data[$table][$column]['field']['revision id'] = $entity_info['entity keys']['revision'];
      $data[$table][$column]['field']['base table'] = $entity_info['revision table'];
    }

makes assumptions about the lingering value of $entity_info - a variable which was defined in a separate foreach loop (over the bundles the field appears on) on lines 89-129. If a field appears on more than one entity type then this bundle loop will have more than one iteration.

My particular problem arises because the first iteration (on 'node' entity) is sufficient to set the $supports_revisions flag - since nodes do indeed support revisions. However the lingering value for $entity_info is for the taxonomy_term entity type - which does not support revisions. So $entity_info['revision table'] is empty at line 181.

I would normally be up for submitting a patch to fix this straight off, but I'm afraid the fix doesn't look straightforward to me - I don't understand the significance of all the places $entity_info is depended on in this block. It could be that the approach of the code here is not quite right - since it appears to assume implicitly that a field will only appear on one entity?

Perhaps views is intended to be incompatible with fields that appear on more than one entity?

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Fixed

Please update to the latest dev version. It's fixed there already.

Status: Fixed » Closed (fixed)

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