Hi,

taxnomy_nodeapi calls taxonomy_node_save on insert and update. This function, in turn, saves the taxonomy of the node. Unfortunately it first deletes all terms for the current node, and then inserts them again if they have been loaded before. taxonomy_nodeapi doesn't respond to 'load'. Therefore, unless you've done something like $node->taxonomy = taxonomy_node_get_terms($node) the terms are lost.

See: http://drupaldocs.org/api/head/function/taxonomy_node_save

Solution: move the call to taxonomy_node_delete($nid) inside the if (is_array($terms)). When the terms haven't been loaded they won't be saved either.

CommentFileSizeAuthor
#1 taxonomy-save.patch471 byteswiz-1

Comments

wiz-1’s picture

wiz-1 commented
StatusFileSize
new taxonomy-save.patch471 bytes

Oops, forgot the patch

junyor’s picture

junyor commented

Don't forget to change the status to patch when you upload a patch. :)

wiz-1’s picture

wiz-1 commented

All right, but moshe weitzman told me not to do this right away:

please set issues to patch once they have been tested.

I assume that you have thoroughly tested the patch before changing status ;)

junyor’s picture

junyor commented

Fair enough. But then you should ask for people to test it. ;)

moshe weitzman’s picture

moshe weitzman commented

A patch for this issue is welcome. I think though that this fix is part of the upcoming folksonomy changes by Morbus ... the fix proposed here might have problems if the user wants to remove all terms on an existing node. not sure.

If you have authored and tested a patch yourself, it is OK to set to patch status. I just object to people moving the testing burden to the CVS reviewers.

morbus iff’s picture

morbus iff
they/them
commented

Well, I plan on redoing the delete code at some point, it is not in my folksonomy patch.

morbus iff’s picture

morbus iff
they/them
commented

+1 from me. I've run across this a few times too.

morbus iff’s picture

morbus iff
they/them
commented
morbus iff’s picture

morbus iff
they/them
commented
Assigned: Unassigned » morbus iff

Actually, -1. I have some concerns.

One of my case scenarios was:

* node has a bunch of terms.
* user edits node, deletes all terms.
* ->taxonomy becomes blank. does the delete occur?

The delete DOES occur, but ONLY because ->taxonomy becomes an empty array. It's may be too easy for a third party module to forget to pass an empty array, which would cause the is_array() check to fail, which would cause the delete() never to happen. And, well, this patch would break my upcoming folksonomy patch :)

My understanding is that this hiccup (->taxonomy isn't loaded, delete happens erroneously) would only occur on third party modules or import scripts, and doesn't happen anywhere in core. As such, a low priority, but one that should still be eventually addressed.

killes@www.drop.org’s picture

killes@www.drop.org commented

Functions calling node_save should make sure to first load taxo terms. The taxonomy module needs soem love though the code started smelling some time ago. Luckily Morbus might be doing that.

wiz-1’s picture

wiz-1 commented
Version: » 4.6.0

I'm not sure I can agree with the last followup. It is plainly wrong that the code node_load($nid); node_save($nid); will delete all taxonomy terms of the node, especially as this is not documented anywhere AFAIK. This problem may not be triggered by any core modules, but for people like me who develop their own modules it's a bug or at least an annoyance...