The revisions tab on node does not show an accurate Term column if there are multiple term reference fields attached to a node bundle. The issue is the formation of the SQL in the _revisioning_get_all_revisions_for_node() function, inside the "if ($include_taxonomy_terms) {". If there are multiple fields the SQL created adds a LEFT JOIN to the table for each term reference field and adds that table's *_tid field to the SELECT statement aliased as term. The result is you have one field in the SELECT statement named `term` for each term reference field attached to the node bundle in question.

Sample SQL using term reference field called term_field1 and term_field2 searching for ficticious node nid=12345.

SELECT r.vid, 
r.status, 
r.title, 
r.log, 
r.uid, 
r.timestamp, 
u.name, 
ttd_field_term_field1.name AS term, 
ttd_field_term_field2.name AS term 
FROM `node_revision` r 
INNER JOIN `users` u ON r.uid=u.uid 
LEFT JOIN `field_revision_field_term_field1` r_field_term_field1 ON r.vid = r_field_term_field1.revision_id 
LEFT JOIN `taxonomy_term_data` ttd_field_term_field1 ON r_field_term_field1.field_term_field1_tid=ttd_field_term_field1.tid 
LEFT JOIN `field_revision_field_term_field2` r_field_term_field2 ON r.vid = r_field_term_field2.revision_id 
LEFT JOIN `taxonomy_term_data` ttd_field_term_field2 ON r_field_term_field2.field_term_field2_tid=ttd_field_term_field2.tid 
WHERE r.nid = 12345 
ORDER BY r.vid DESC;

When the foreach goes to loop through the result it only grabs that last `term` field in the SQL's SELECT statement, meaning the other get ignored, or even more annoying if the last field id null nothing gets printed at all.

Additionally it would likely be more efficient to check the taxonomy_maintain_index_table variable in the variables table as run this query against the taxonomy_index if taxonomy_maintain_index_table = TRUE.

I'm working on a patch, but it may take a little while to get to it.

CommentFileSizeAuthor
#10 revisioning_incomplete_terms_1458714_D7.patch4.09 KBsjpatrick

Comments

rdeboer’s picture

rdeboer
Location Melbourne
commented

Thanks for pointing this out jalama!
Look forward to your patch.
Rik

Nick Bell’s picture

Nick Bell commented

Did you ever get to a patch? Would be grateful for any help sorting out this problem.

Nick

rdeboer’s picture

rdeboer
Location Melbourne
commented

Nope.
Sorry.

Nick Bell’s picture

Nick Bell commented

Thanks Rick

Could you give me any tips? I'm looking at function _revisioning_get_all_revisions_for_node but not sure where to go from there.

I was wondering whether the easiest thing might be to replace this table with a View (I have a nice view set up showing what I want with multiple taxonomy terms).

rdeboer’s picture

rdeboer
Location Melbourne
commented

@md2017

Hi Nick,

I think bigjim was referring to function _revisioning_get_all_revisions_for_node(...) in file revisioning_api.inc -- oh ooops, so were you...

A canned View like you suggest is probably even better, if it has the complete functionality...

Rik

Nick Bell’s picture

Nick Bell commented
Status: Active » Needs review

OK here's my attempt, which yields what I want. Its base is Revisioning 7.x-1.4+4-dev. It should work for Tags as well, but I haven't tested it. Can someone have a look? I've never done a patch before.

function _revisioning_get_all_revisions_for_node($nid, $include_taxonomy_terms = FALSE) {
  $node = node_load($nid);
	$nodeTags = "";
	$nodeTerms = "";
  $sql_select = 'SELECT r.vid, r.status, r.title, r.log, r.uid, r.timestamp, u.name';
  $sql_from   = ' FROM {node_revision} r INNER JOIN {users} u ON r.uid=u.uid';
  $sql_where  = ' WHERE r.nid = :nid ORDER BY r.vid DESC';
  if ($include_taxonomy_terms) {
    $conditions = array('type' => 'taxonomy_term_reference');
    $fields = field_read_fields($conditions);
    foreach ($fields as $field => $data) {
      $field_instance = field_read_instance('node', $field, $node->type);
      if ($field_instance) {
				if ($field == 'field_tags') { // tags 
					if ($nodeTags=="") {
						$nodeTags = ", CONCAT('<ul><li>',ttd_$field.name";
					} else {
						$nodeTags .= ",'<li>',ttd_$field.name";
					}
				} else { // term
					if ($nodeTerms=="") {
						$nodeTerms = ", CONCAT('<ul><li>',ttd_$field.name";
					} else {
						$nodeTerms .= ",'<li>',ttd_$field.name";
					}
				}
        $sql_from .= " LEFT JOIN {field_revision_$field} r_$field ON r.vid = r_$field.revision_id LEFT JOIN {taxonomy_term_data} ttd_$field ON r_$field.{$field}_tid=ttd_$field.tid";
      }
    }
  }
  if ($nodeTerms<>"") {
		$sql_select .= "$nodeTerms,'</ul>') AS term";
	}
  if ($nodeTags<>"") {
		$sql_select .= "$nodeTags,'</ul>') AS tags";
	}
	$sql = $sql_select . $sql_from . $sql_where;
	$result = db_query($sql, array(':nid' => $nid));
  $revisions = array();
	foreach ($result as $revision) {
    if (empty($revisions[$revision->vid])) {
      $revision->current = $node->vid;
      $revisions[$revision->vid] = $revision;
    }
    // If a revision has more than one tag or taxonomy term, these will be
    // returned by the query as separate objects differing only in their terms.
		// COMMENTED OUT AS APPARENTLY DOES NOTHING
    // elseif ($include_taxonomy_terms) {
    //   $existing_revision = $revisions[$revision->vid];
    //   if (!empty($revision->term)) {
    //     if (strpos($existing_revision->term, $revision->term) === FALSE) {
    //       // Bit of a quick & dirty -- goes wrong if a term is substr of another
    //       $existing_revision->term .= ", $revision->term";
    //     }
    //   }
    //   if (!empty($revision->tags)) {
    //     if (strpos($existing_revision->tags, $revision->tags) === FALSE) {
    //       $existing_revision->tags .= ", $revision->tags";
    //     }
    //   }
    // }
  }
  return $revisions;
}
rdeboer’s picture

rdeboer
Location Melbourne
commented

Hi Nick,
Thanks so much for your work on this. Well done.

It would be nice for you and us to learn about the "diff" command (whose output is effectively what a patch is) and its option to ignore blanks.

The way things are at the moment I'll have to go through your code line by line to find out what the differences are.

Rik

Nick Bell’s picture

Nick Bell commented

Do I just diff against http://ftp.drupal.org/files/projects/revisioning-7.x-1.x-dev.tar.gz ?

rdeboer’s picture

rdeboer
Location Melbourne
commented

I need a proper diff/patch file to review and apply.
The diff should be agains the 7.x-1.x branch, ie. 7.x-1.x-dev snapshot.
Rik

sjpatrick’s picture

sjpatrick commented
StatusFileSize
new revisioning_incomplete_terms_1458714_D7.patch4.09 KB

I also ran into this issue while working on an intranet feature we are migrating from our D6 site to our new D7 model.
I tried applying the code changes listed above but still wasn't getting results that looked like they did in D6.
So I took my own stab at it.
Patch included.