I'm seeing some problem with nodes that have several taxonomy terms from several vocabularies. It looks like the $old_taxonomy and $new_taxonomy arrays aren't being built the same way, somehow. For instance, the new array will have two empty terms instead of the terms as in the old array.

So, the attached patch adds the tids as keys for the arrays. It fixes the problem and as an added bonus, it'll also show diffs when the terms are different, but the names are the same.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Junyor’s picture

Whoops! I left some debug code in.

Junyor’s picture

Status: Needs review » Needs work

No, that didn't fix it.

Junyor’s picture

Status: Needs work » Needs review
FileSize
1.91 KB

There seems to be two problems:

* Single-select vocabularies aren't handled correctly. When this problem is encountered, blank lines appear in the "new" diff
* Taxonomy terms aren't ordered in any coherent way, so something can show up in the diff that shouldn't just because it's ordered differently in the old taxonomy vs. the new taxonomy

The former issue is a pretty straight-forward fix, but the latter isn't. I've attached a patch for the former issue.

I tried to fix the latter issue by using tids as keys for the result arrays, but the diff didn't seem to like it when the array keys weren't contiguous. Any suggestions for fixing that?

Setting to needs review for the first issue.

moshe weitzman’s picture

If D6 is broken, we should fix it first if possible.

moshe weitzman’s picture

Status: Needs review » Needs work

anyone available to port this to D6? D7 (we want to submit this to core)

mo6’s picture

subscribe

j0rd’s picture

Taxonomy revision diffs don't work at all. This is because the taxonomy module only associates node_terms with NID and not NID+VID (VID is the revision number). Theirfor when it loads the term data for $new_node and $old_node it's the same...and therefor you can't not get diff data from it.

I believe the taxonomy module would have to get changed to resolve this or glue would have to be added with another module.

Function in question. taxonomy.module

function taxonomy_node_get_terms($nid, $key = 'tid')

Junyor’s picture

I've been exploring this a bit more and I'm running into some problems with mulit-select or freetagging vocabularies. The diffing is done based on array keys, but I can't find a good key for these vocabularies. Any ideas?

@j0rd: $new_node and $old_node have arrays of taxonomy terms with all the needed information.

Junyor’s picture

@j0rd: Sorry, I see what you mean now. Yes, there's no way to diff existing revisions. It is still possible to diff an existing node with an edit, though.

Junyor’s picture

Here's a new patch that resolves all the issues I've found except the problem diffing existing revisions. Now the diff is built using vocab name and term name to guarantee uniqueness and aid in sorting.

moshe weitzman’s picture

@Junyor -any chance you want to maintain this module? I have pretty much moved on. I gave you CVS access so you can at least commit his patch.

Junyor’s picture

@moshe: I appreciate the offer, but unfortunately I won't be able to do so due to other obligations.

joe-b’s picture

FileSize
3.37 KB

Based on @Junyor's D5 work, I've had a go at a patch for D6, which is attached.

joe-b’s picture

FileSize
3.37 KB

Nuts, minor syntax update. Apologies.

mitchell’s picture

Version: 5.x-2.0 » 6.x-2.x-dev

Re #13.

mitchell’s picture

Status: Needs work » Needs review

Actually, #13 also answers #5.

Alan D.’s picture

Issue summary: View changes
Status: Needs review » Closed (outdated)

Closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.