Handle saving of taxonomy terms more intelligent

err...
this two patches are exactly the same...

Can you clarify the Case some more. "content_taxonomy.module is a CCK module to insert taxonomy fields into your node on an arbitrary place." doesn't make sense to me yet.

This may be a significant performance boost for sites with very few categorised nodes, but I'm worried that it's a performance drain for sites where most (or all) nodes are categorised. For the latter group, this is just one extra query. Isn't there some way to get around executing the query every time? Perhaps a static cache for each node type, or a call to some other function that holds a cache of the data that's needed?

Any non-obvious speed gain should be banchmarked.

I would rather see the extra term information needed to $node->taxonomy so that the save/delete code is all in one place (in taxonomy module, where it belongs). Additionally, these use cases seem to break as soon as the node type in question gets a vocabulary associated with them through the usual means.

What happens if you want programmattically-controlled terms and normal terms on the same node type?

THKs I had the same problem but with your path its ok now!!!
I tried to use taxonomy_node_save() but afte taxonomy_node_save() was run taxonomy_node_delete() and delete it

reverting title

+1, that's what I've been searching for!!
Your patch works great and it would be also a performance improvement!

I agree that this is a really important patch

However I would propose a bit different patch (attached). On node update I'm using the cached $node array and compare the new data with the old data. So there will be only queries, if there were changes. In the worst case, there will be as many queries as before. (1 delete query and one INSERT for every new term).

Programmatically changing terms:
This code will only delete terms from the database, which belongs to a vocabulary, which is maintained by the taxonomy module for this content type. So it would be possible to programmatically insert other terms from other vocabularies and they won't be touched.
It would also be possible to load custom supplied terms into $node->taxonomy and toapply any changes on this array - then the taxonomy module will do any saving for you, as it would do now.

so let's summarize the changes:

• Performance improvement for editing nodes while keeping its terms (no INSERT/DELETE queries!)
• Allows programmatically adding/changing terms for a node, regardless wheter the taxonomy module is used for a content type.

I am trying to apply this patch, but I have no clue what I am doing. I don't hae shell access, so I have to do it manually right?

No, you can apply patches on your own computer and then upload the changed files to your server with FTP.

See instructions here: http://drupal.org/node/60108 (note there are also Mac and Windows-specific instructions on sub-pages there)

thank you very much! now... figuring out the whole cygdrive thing...

thanks again!

Kris

I have successfully applied the patch (thanks to webchick :). I am using this with content_taxonomy.module but when I go into contemplate to change the output, the taxonomy fields do not show. I downlaoded the latest patch (taxonomy_save.patch.txt) Should I download the other patch?

yeah, I had a too quick look on taxonomy_node_get_terms(). It loaded all terms, so your term will be deleted as long as the code doesn't use $node->taxonomy itself. (content_taxonomy doesn't)

Now I've updated the patch, so that taxonomy_node_get_terms() only loads terms of activated vocabularies. This enables other modules to easily set terms programmatically, as long the vocabularies aren't used by the taxonomy module. Unfortunately, I had to change the function to take a $node object, instead of the nid. However this is no problem for core, because the function isn't used elsewhere, but it might in contrib modules.

some tabs made it in my last patch. here a new one without tabs.

Haven't tested yet, but looks good. A few suggestions:

1. I'm not a big fan of script-generated SQL 'IN' clauses, where IN() can end up having 10 gazillion values. Is this really necessary? Why would the term_node table ever have any terms that aren't in a vocabulary that's activated for the node type in question?
2. Similarly, the use of array_fill() to generate the placeholders seems a bit hackish to me. I say we nuke this along with the IN() clause.
3. I think _taxonomy_node_insert_tid() should be renamed to _taxonomy_node_save_term().
4. Why not pass $node as the first argument of taxonomy_node_save(), instead of $nid? taxonomy_nodeapi() already has the full $node object available, and by doing this, we can avoid an unnecessary call to node_load().
5. Not too sure about the whole $old_terms thing either (looks kinda hackish), but it seems like it's necessary, so it will probably have to stay.

ad 1: because of other modules, that might want to save some terms there too. What's so bad with that?
ad 2. we could use intval() instead..
ad 3. good idea
ad 4. yep. futher you can call node_load() there, because it would result in an endless loop (nodeapi load calls this!)
ad 5: Instead I can only think of removing all terms of all enabled vocabularies only. That would need a subquery - an this is not doable with our supported mysql versions.

Fago:

I tried your latest patch, but my fields were still not showing. I went into contemplate to see what it was calling but dodn't know what to change. One of my fields lloks like this:

If I even touch this code, my fields disappear.

Thanks for your work,

Kris

Sorry about that, here is the code:

<div class="field field-type-content-taxonomy field-field-street-address">
  <h3 class="field-label">Street Address</h3>
  <div class="field-items">
    <?php foreach ((array)$field_street_address as $item) { ?>
      <div class="field-item"><?php print $item['view'] ?></div>
    <?php } ?>
  </div>
</div>

kweisblatt, this issue is not about the cck taxonomy field.
this patch just enables modules like this to work. so please post any problems with the cck taxonmy field in its own issue.

Are there any other opinions to the IN() clause?

Ok it can become quite long, if one removes really many terms of a node. But otherwise, the old code would do a DELETE and an INSERT query for each term that is not removed.
so imho, it's better to have a bit longer query in the more unusual case of removing all terms vs having a lot of queries in the usual case of keeping all terms.

Sorry about that. I thought this thread was about getting modules like this to work, which mine isn't... doesn't show tax fields...

I have and will list in other areas as well for a solution.

Just to make a note: I have also tried theming the node in by using node-content-my_content.tpl.php and it still doesn't work, so my assumtion that it was contemplate not reading the fields isn't the only problem I am guessing. Maybe it is a problem with the module itself.

patching file modules/taxonomy/taxonomy.module
Hunk #1 FAILED at 611.
Hunk #2 FAILED at 695.
Hunk #3 FAILED at 739.
Hunk #4 FAILED at 785.
Hunk #5 FAILED at 797.
Hunk #6 FAILED at 1193.
6 out of 6 hunks FAILED -- saving rejects to file modules/taxonomy/taxonomy.module.rej

#412518: Convert taxonomy_node_* related code to use field API + upgrade path removes taxonomy_node_*() in favour of field storage.