At the moment the taxonomy module ignores revisions. This should be changed to take full advantage of the revision system.

The term to node association is stored in the {term_node} table which has a tid (term-id) and nid (node-id).
To account for revisions in this table I see two possibilities:

1. Add a vid (version-id) to the table, so the layout is {term_node} (tid, nid, vid)
2. Replace nid by vid, so the layout is {term_node} (tid, vid)

The node id alone will not give the right list of terms in a query over version 1. Thus you always need the vid to get the current list of terms. Because of that I think version 2 is the way to go to implement revisioned terms.

Please add any other possibilities or comment on those two.

CommentFileSizeAuthor
#11 revisioned_taxonomy_head_2.patch12.03 KBrötzi
#9 revisioned_taxonomy_head_1.patch11.52 KBrötzi
#6 revisioned_taxonomy_head.patch12.45 KBrötzi
#3 taxonomy_revisions.patch12.87 KBrötzi

Comments

moshe weitzman’s picture

moshe weitzman commented

book is an analogous table and it has both vid and nid. i say we have both ... yes, this is a needed enhancement to revisions.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

+1

Subscribing.

rötzi’s picture

rötzi commented
Status: Active » Needs review
StatusFileSize
new taxonomy_revisions.patch12.87 KB

The attached patch implements revisioned taxonomy for the Drupal-5 branch.

I have not looked at any contributed modules which uses taxonomy yet, only at the core modules. But the interface is backwards compatible so it should not be a problem.

Please have a look and see if you find any bugs which I overlooked.

killes@www.drop.org’s picture

killes@www.drop.org commented

+ if (is_object($param)) {
+ $node = $param;
+ }
+ else {
+ $node = db_fetch_object(db_query('SELECT nid, vid FROM {node} WHERE nid = %d', $param));
+ }

Do we really need this?

We need to check how well mysql uses the new indices.

rötzi’s picture

rötzi commented

This check is just for backwards compatibility. If you pass a node object the additional query will not be used. If you pass just an id you need to know the version id of the node, thus the query to get the latest version id.

Or did you mean something else?

(I will change the parameter comment to make clear that a node object should be passed)

rötzi’s picture

rötzi commented
StatusFileSize
new revisioned_taxonomy_head.patch12.45 KB

Here's a patch against Drupal HEAD.

I made some changes in the SQL queries: The node id is not checked anymore in the joins since the version id is enough.

killes@www.drop.org’s picture

killes@www.drop.org commented

We usually don't care about backwards compatibility. I think the code would be more elegant if you'd settle for either $nid or $node being passed.

BioALIEN’s picture

BioALIEN commented

I like the idea of this patch, so +1 from me.

Any enhancements to the taxonomy system brings a smile to my face!

rötzi’s picture

rötzi commented
StatusFileSize
new revisioned_taxonomy_head_1.patch11.52 KB

I removed the node id check. node objects are now expected.

killes@www.drop.org’s picture

killes@www.drop.org commented

thanks, looks much nicer now. I think the upgrade path needs work, though, it looks as if it would be mysql-only.

rötzi’s picture

rötzi commented
StatusFileSize
new revisioned_taxonomy_head_2.patch12.03 KB

Unfortunately I don't know PostgreSQL. I now copy-pasted code from other updates, but someone experienced with pgsql should check the code.

dries’s picture

dries commented

Looks good to me! Feel free to mark this RTBC after it has been tested. Good job, rötzi.

rötzi’s picture

rötzi commented
Status: Needs review » Reviewed & tested by the community

I tested a fresh installation as well as an update. Both worked fine.

dries’s picture

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

Committed to CVS HEAD. Thanks! :-)

Keep those patches come'ing. :)

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)
dww’s picture

dww
we/he/they
commented
Status: Closed (fixed) » Needs review

while this is a welcome change, it will break a fair number of contrib modules. it definitely needs to be in the upgrade docs. i just finished going through all the CVS activity in the HEAD of core while i was gone, documenting everything that seemed developer-visible for the upgrade docs, and this was obviously one of the key changes:

http://drupal.org/node/114774#taxonomy-revisions

can someone take a look at what i wrote for correctness and completeness? once you're happy, please set this issue back to closed.

in general, we shouldn't close issues like this until they've been properly documented in the upgrade docs...

thanks,
-derek

hbfkf’s picture

hbfkf commented

A question that comes to my mind (but might be a little off-topic): Should there be a way to turn taxonomy revisioning off? For most purposes, revisions are great but for some applications they might be inappropriate or error-prone (novice user thinks he changes the taxonomy but in fact has only changed it for the current revision).

What is Drupal's philosophy with respect to this? Do we simply say that if the user does not want revisioned taxonomy, she has to make sure herself that all revisions of a given node have the same taxonmoy?

moshe weitzman’s picture

moshe weitzman commented

can you elaborate on the circumstances under which this causes confusion/errors? the ability to turn this off is bundled in the 'create new revision' checkbox. i think we'll resist creating a new preference for this feature so your case has to be pretty pursuasive.

hbfkf’s picture

hbfkf commented

If I am not mistaken, the 'create new revision' checkbox only differentiates between the current revision and a potentially new one — but not between all revisions and the current one. If the checkbox is unchecked this does not mean that I work on all revisions (but only on the current one).

I was thinking about this latter scenario when you actually want to change the taxonomy of all revisions of a particular node. For instance, my node could represent a downloadable application form coming in two revisions, let's say pre-May-07 and post-May-07. Now if a novice user (one that does not know about revisions and/or taxonomy revisions) tags my latest (post-May-07) revision with the term "form", the old one does not get tagged and will not be found even though it's March and the pre-May-07 form is still in use.

I agree though that this is not a really persuasive example. I was simply wondering whether some applications are not better served by the old way (no taxonomy revisioning) and whether this should not be allowed.

(But I realize that my comment is really off-topic: this rather calls for an 'apply to all revisions' checkbox in node.module...)

bdragon’s picture

bdragon commented
Status: Needs review » Closed (fixed)

Since this was documented, reclosing.

catch’s picture

catch
he/him
Primary language English
commented

This patch broke shadow forum topics: http://drupal.org/node/208858