Greetings,

Thanks for the module! It is really useful and I would be glad to see it in a Drupal core somewhen.

I encountered a problem when using token.module together with auto_nodetitle (www.drupal.org/project/auto_nodetitle) and taxonomy.

Case study: I have a node type which is associated with a taxonomy vocabulary. I want the title of these nodes to be automatically generated using [term] token.

Problem: When saving a newly added node, TERM token is empty. When updating an existing node, TERM token returns the old term but not the newly chosen one. User has to repeat saving in order title to be correctly generated.

Reason: auto_nodetitle generates title on validate hook which occurs before submit, this is why taxonomy is not yet saved into the database.

Proposed solution: Change taxonomy handling in a token_node.inc file.
Now you have the following code which assumes $node->taxonomy is an object and then you query database to get term values:

      if (module_exists('taxonomy') && !empty($node->taxonomy) && is_array($node->taxonomy)) {
        foreach ($node->taxonomy as $term) {
          if ((object)$term) {

My proposition is to handle not only the case when node was loaded from the database but also when it is submitted and is about to be saved into the database. In this case $node->taxonomy not an object but an array.
It can be like this if vocabulary is in MULTIPLE mode:

array (
 [vid] => array (
   [tid] => [tid],
   [tid] => [tid],
   [tid] => [tid]
 ),
 [vid] => array (
   [tid] => [tid],
   [tid] => [tid]
 )
)

Or this way if vocabulary is in SINGLE mode:

array (
 [vid] => [tid]
)

To properly handle these both situation, I propose the following patch:

      if (module_exists('taxonomy') && !empty($node->taxonomy) && is_array($node->taxonomy)) {
        foreach ($node->taxonomy as $vid_key => $term) {
          if (is_array($term) || is_numeric($term)) {
            $tid = is_array($term) ? reset($term) : $term;
            $name = db_result(db_query("SELECT t.name FROM {term_data} t WHERE t.tid = %d", $tid));

            $values['term'] = check_plain($name);
            $values['term-raw'] = $name;
            $values['term-id'] = $tid;

            if(!empty($vid_key)) {
              $vocabulary = taxonomy_get_vocabulary($vid_key);
              $values['vocab'] = check_plain($vocabulary->name);
              $values['vocab-raw'] = $vocabulary->name;
              $values['vocab-id'] = $vocabulary->vid;
            }
            break;

          } elseif ((object)$term) {

            // existing code .....

          }

First of all, I added as $vid_key => $term to get vocabulary ids from array (this will not break existing functionality even if taxonomy array has no keys).

Second, I'm checking if $term is either an array or a number I know that node is about to be saved and handle this situation - I only need to read term's name from database (I already hav VID and TID).

I'm not creating a patch because you may find a more graceful implementation of this idea to avoid some duplicates.

Thank you.

CommentFileSizeAuthor
#6 token_node.patch723 bytesardas
#2 token_node.patch1.58 KBardas

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Title: TERM token isn't provided corerctly when the node is not yet saved. » term token isn't provided corerctly when the node is not yet saved.

Could you please provide it as an actual patch? It's harder for me to read the way you've presented it.

Also, the pathauto 5.x-1.x taxonomy parsing code on pathauto_node.inc is/was pretty robust at handling these situations. You may want to take some inspiration from that.

ardas’s picture

ardas commented
StatusFileSize
new token_node.patch1.58 KB

I looked through the pathauto.module and it looks like it doesn't need such fix since pathauto works after node is saved (insert and update hooks).

The proposed patch for token.module is needed may be only for auto_nodetitle.module because it generates title BEFORE the node is saved. Anyway, this patch doesn't break anything in token module but only extends its abilities.

Patch is attached.

Thanks.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

@ARDAS - this patch fails against the latest token_node.inc, I assume because of http://drupal.org/node/137669

Can you compare your approach to that one and see if yours is still needed? In general it's best to make patches against the latest code from CVS which prevents this problem - see http://drupal.org/patch/create for details on the standard way used in the Drupal community to make a patch.

ardas’s picture

ardas commented

Greetings,

I have reviewed the latest dev release and discovered that you have fixed this issue
The fix is behind these comments:
// Ok, if we still don't have a term name maybe this is a pre-taxonomy submit node
// So if it's a number we can get data from it

Unfortunalty, it still doesn't work for me. The reason is $original_term variable stores term ID and is not an array but is an integer value.
My proposition is to change these lines

          if(!isset($term->name) && is_array($original_term)) {
            $tid = array_shift($original_term);

to

          if(!isset($term->name)) {
            $tid = (is_array($original_term)) ? array_shift($original_term) : $original_term;

In this case we will handle both situations.

Patch was done over the latest dev release available on the site and is attached here. Please tell me your thoughts.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

seems to be missing the patch.

This seems simple enough that it's OK, but this approach in general leads us down a slippery slope. Taxonomy data prior to the node save is really hard to parse. For example, this approach will handle single and multiple selections, but not freetagging.

ardas’s picture

ardas commented
StatusFileSize
new token_node.patch723 bytes

Well, by applying this patch we can increase an amount of situations parsed correctly by token module.

I agree that taxonomy structure differes but are there many cases? Looks like only three: array, integer and free tagging case. BTW, what is a value of $original_term in a free tagging case?

ardas’s picture

ardas commented

Honestly speaking, I don't like many things in taxonomy.
This is why I'm using it only together with content_taxonomy module which allows us to add taxonomy CCK field. I didn't tested yet but looks like token should process those taxonomy CCK fields correctly.

The problem is some old projects where I can't change architecture.
I also think that many people still uses taxonomy in its traditional way so this patch may be useful.

Especially if you improve it to cover free tagging case (if it is possible).

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I don't know the exact details and maybe the different people contributing code (and obviously I) were sloppy, but the taxonomy parsing code in pathauto was like 50 lines long and still didn't work perfectly.

I generally like your patch but would appreciate it if you could try it for freetagging just to make sure it doesn't blow up and then we can run with it. Thanks.

ardas’s picture

ardas commented

I did some testing of token.module in conjunction with auto_nodetitle.module for the case when [term] token is used to generate a title. Vocabulary is configured to be in a free tagging mode.

You have these comments:
// Rather than duplicating taxonomy.module code here you should make sure your calling module
// has a weight of at least 1 which will run after taxonomy has saved the data which allows us to
// pull it out of the db here.

Module weight will not help because auto_nodetitle module generates title on validate hook which occurs before any submits. So, token + auto_nodetitle works incorrectly in case of free tagging (with or without my patch). Possible solutions are:
1. Try to fix token module and probably copy code from taxonomy module.
2. Try to change auto_nodetitle to generate title on submit instead of validate.

Your thoughts?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Well, what i'd like to see is what's in the comment ;) Refactoring the core taxonomy module and using it's tag parser instead of our own.

I'm aware of the situation with autonodetitle (ANT) with taxonomies and the fact that it won't work to set a heavier weight. That conflict is why we have the limited extra parsing that exists today.

The other problem is that some things (like [tid]) will never work consistently before a node is submitted - why? What if a user is adding a freetagging term when creating the node - the tid doesn't exist for that term yet, so token (and ANT) can't get

I think at this point the most reasonable thing to do is say "if you want to get term/cat tokens before the node is fully created, then you are limited to vocabularies that are not freetagging".

ardas’s picture

ardas commented

Yes, may be you are right.

Anyway, my patch helps a little :) At least, for normal terms (not free tags).

I hope they refactor taxonomy soon, because I started to use category.module instead.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Closed (won't fix)

D5 is now unsupported, as well as Token module for D5.