Strange bug: Publishing a node, one taxonomy term is removed

That certainly is an odd one. Does it happen if the node only has that one term, or does it require two+ terms for the tid 4 to get deleted?

What's the situation? Do you have access to the source code in a local version of the site? or is it just in your live site? Just thinking about how to re-create it, or how you could add some debug code to scheduler to track what it is doing.

Jonathan

First question - does your local site show the same strange behavior?

OK, here's what to do (as I do not know your background and experience, please excuse me if I am going into too much detail).

1. If you have not got it already, download the Devel module http://drupal.org/project/devel and install in your local site
2. in /sites/all/modules/scheduler, edit the file scheduler.module
3. find the function _scheduler_publish() and add the three extra lines shown below by <<<===
   

   function _scheduler_publish() {
     $result = FALSE;
     $date_format = variable_get('scheduler_date_format', SCHEDULER_DATE_FORMAT);
   
     // If the time now is greater than the time to publish a node, publish it.
     $query_result = db_query('SELECT s.nid AS nid FROM {scheduler} s LEFT JOIN {node} n ON s.nid = n.nid WHERE n.status = 0 AND s.publish_on > 0 AND s.publish_on < %d ', time());
     $nids = array();
     while ($node = db_fetch_object($query_result)) {
       $nids[] = $node->nid;
     }
   
     $nids = array_unique(array_merge($nids, _scheduler_scheduler_nid_list('publish')));
   
     foreach ($nids as $nid) {
       $n = node_load($nid);
       dsm($n->taxonomy, 'in _scheduler_publish() at start');    // <<<=== Add this line
       $n->changed = $n->publish_on;
       $old_creation_date = $n->created;
       if (variable_get('scheduler_publish_touch_'. $n->type, 0) == 1) {
         $n->created = $n->publish_on;
       }
   
       $create_publishing_revision = variable_get('scheduler_publish_revision_'. $n->type, 0) == 1;
       if ($create_publishing_revision) {
         $n->revision = TRUE;
         $n->log = "Node published by scheduler module. Original creation date was ".  format_date($old_creation_date, 'custom', $date_format) .".";
       }
   
       // Use the actions system to publish the node.
       watchdog('scheduler', '@type: scheduled publishing of %title.', array('@type' => $n->type, '%title' => $n->title), WATCHDOG_NOTICE, l(t('view'), 'node/'. $n->nid));
       $actions = array('node_publish_action', 'node_save_action');
       $context['node'] = $n;
       actions_do($actions, $n, $context, NULL, NULL);
   
       // If this node is not to be unpublished, then we can delete the record.
       if (isset($n->unpublish_on) && $n->unpublish_on == 0) {
         db_query('DELETE FROM {scheduler} WHERE nid = %d', $n->nid);
       }
       // We need to unpublish this node at some time so clear the publish on since
       // it has been published.
       else {
         db_query('UPDATE {scheduler} SET publish_on = 0 WHERE nid = %d', $n->nid);
       }
   
       // Invoke scheduler API.
       _scheduler_scheduler_api($n, 'publish');
       
       $n = node_load($nid);                                                                     // <<<=== Add this line
       dsm($n->taxonomy, 'in _scheduler_publish() - after calling _scheduler_scheduler_api()');  // <<<=== Add this line
   
       $result = TRUE;
     }
   
     return $result;
   }
   

4. Find the function _scheduler_scheduler_api() and add two lines as below:
   

   function _scheduler_scheduler_api($node, $action) {
     foreach (module_implements('scheduler_api') as $module) {
       $function = $module .'_scheduler_api';
       $function($node, $action);
       
       $n = node_load($node->nid);                                                               // <<<=== Add this line
       dsm($n->taxonomy, 'in _scheduler_scheduler_api(), after ' . $function . ' ' . $action);   // <<<=== Add this line
     }
   }

5. Set up a node to be published which will have the term removed, and run cron manually using the admin link in admin/reports/status

This will print out the terms attached to the node at each point in the scheduling publish process, so we should be able to see what is happening and where the term is being dropped. I'll be interested to see what you find out.

Jonathan

Right, now we are getting somewhere! It seems that when the node "leaves" the scheduler process all the terms are still intact. Maybe it is another module which is causing the problem? Edit the file /modules/node/node.module, and add the following to node_invoke_nodeapi()

function node_invoke_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) {
  $return = array();
  foreach (module_implements('nodeapi') as $name) {
    $function = $name .'_nodeapi';
    $result = $function($node, $op, $a3, $a4);
    if (isset($result) && is_array($result)) {
      $return = array_merge($return, $result);
    }
    else if (isset($result)) {
      $return[] = $result;
    }
    if (isset($node->taxonomy)) {                                            // <<<=== Add this line
      dsm($node->taxonomy, 'in node_invoke_nodeapi(), after ' . $function);  // <<<=== Add this line
    }                                                                        // <<<=== Add this line
  }
  return $return;
}

Repeat the test you did above, and this will show the whole list of modules which call hook_nodeapi(). One of them might be dropping that term.

That's surprising, because cck is very well-used module! Try updating sites/all/modules/cck/content.module:

function content_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) {
  // Prevent against invalid 'nodes' built by broken 3rd party code.
  if (isset($node->type)) {
    $type = content_types($node->type);
    // Save cycles if the type has no CCK fields.
    if (!empty($type['fields'])) {
      $callback = 'content_'. str_replace(' ', '_', $op);
      if (function_exists($callback)) {
        $callback($node, $a3, $a4);
        if (isset($node->taxonomy)) {                                        // <<<=== Add this line
          dsm($node->taxonomy, 'in content_nodeapi(), after ' . $callback);  // <<<=== Add this line
        }                                                                    // <<<=== Add this line
      }
    }

    // Special case for 'view' op, we want to adjust weights of non-cck fields
    // even if there are no actual fields for this type.
    if ($op == 'view') {
      $node->content['#pre_render'][] = 'content_alter_extra_weights';
      $node->content['#content_extra_fields'] = $type['extra'];
    }
  }
}

This will show which cck function is doing it. In my test, during the cron scheduler, content_presave() was called, then later content_update() was called.

This does seem to be getting stranger. Obviously not a scheduler problem, but I am intrigued to trace this right down. To summarise the screenshot, the taxonomy array has/had six items. The 2nd, 3rd and 4th items survive intact. The 5th and 6th items are altered from an object to an array where the term id (either from the array key or the tid value, cannot tell which) is used as the value, and the vid property is used as the key. This may explain why we only get five items in the resulting array, because term 4 (the original 1st item) is being overwritten by the vid=4 from the 5th item (term 1435). But that does not explain why two out of the six items get messed up in the first place.

Try adding the following into content_presave()

function content_presave(&$node) {
  _content_field_invoke('presave', $node);
  if (isset($node->taxonomy)) {                                                        // <<<=== Add this line
    dsm($node->taxonomy, 'in content_presave(), after _content_field_invoke');         // <<<=== Add this line
  }                                                                                    // <<<=== Add this line
  _content_field_invoke_default('presave', $node);
  if (isset($node->taxonomy)) {                                                        // <<<=== Add this line
    dsm($node->taxonomy, 'in content_presave(), after _content_field_invoke_default'); // <<<=== Add this line
  }                                                                                    // <<<=== Add this line
}

and also in _content_field_invoke()

function _content_field_invoke($op, &$node, $teaser = NULL, $page = NULL) {
  $type_name = is_string($node) ? $node : (is_array($node) ? $node['type'] : $node->type);
  $type = content_types($type_name);
  $field_types = _content_field_types();

  $return = array();
  foreach ($type['fields'] as $field) {
    $items = isset($node->$field['field_name']) ? $node->$field['field_name'] : array();

    // Make sure AHAH 'add more' button isn't sent to the fields for processing.
    unset($items[$field['field_name'] .'_add_more']);

    $module = $field_types[$field['type']]['module'];
    $function = $module .'_field';
    if (function_exists($function)) {
      $result = $function($op, $node, $field, $items, $teaser, $page);
      if (is_array($result)) {
        $return = array_merge($return, $result);
      }
      else if (isset($result)) {
        $return[] = $result;
      }
    }
    // test for values in $items in case modules added items on insert
    if (isset($node->$field['field_name']) || count($items)) {
      $node->$field['field_name'] = $items;
    }
    if (isset($node->taxonomy)) {                                             // <<<=== Add this line
      dsm($node->taxonomy, 'in _content_field_invoke(), after ' . $function
        . ' op = ' . $op . ' field_name = ' . $field['field_name']);          // <<<=== Add this line
    }                                                                         // <<<=== Add this line
  }
  return $return;
}

This should give us detail on which fields are being processed when the corruption happens.

OK, just one final run, with the following debug. As I have not used content_taxonomy module I can't quite get what this function is doing. And I have not run this code so apologies if there is any typo.

/**
 * Implementation of hook_field().
 */
function content_taxonomy_field($op, &$node, $field, &$items, $teaser, $page) {
  switch ($op) {         
    case 'presave':
      if ($field['save_term_node']) {
        dsm($field, 'in content_taxonomy_field, field');                                        // <<<=== Add this line
        static $_content_taxonomy_array_cleared;
        if (!is_array($_content_taxonomy_array_cleared) || !$_content_taxonomy_array_cleared[$node->nid][$field['vid']]) {
          _content_taxonomy_taxonomy_unset($node->taxonomy, array($field['vid']));
          $_content_taxonomy_array_cleared[$node->nid][$field['vid']] = TRUE;
          dsm($node->taxonomy, 'in content_taxonomy_field, after unset');                       // <<<=== Add this line
        }
        
        foreach ($items as $key => $entry) {
          dsm($entry, 'in content_taxonomy_field, entry');                                      // <<<=== Add this line
          if ($entry['value']) {
            if (is_object($node->taxonomy[$entry['value']]) 
              || (is_array($node->taxonomy) && in_array($entry['value'], $node->taxonomy))
              || (isset($entry['_remove']) && $entry['_remove'] == 1)) {
              continue;
            }
            elseif (is_array($node->taxonomy[$field['vid']])) {
              if (!in_array($entry['value'], $node->taxonomy[$field['vid']])) {
                $node->taxonomy[$field['vid']][] = $entry['value'];
              }
            }
            // when saving an existing node without presenting a form to the user,
            // the terms are objects keyed by tid. there's no need to re-set these
            // terms, and to do so causes php warnings because the database rejects
            // the row insert because of primary key constraints.
            else {
              dsm('now about to do the damage!');                                               // <<<=== Add this line
              if (isset($node->taxonomy[$field['vid']])) {                                      // <<<=== Add this line
                dsm($node->taxonomy[$field['vid']], 'in content_taxonomy_field, before');       // <<<=== Add this line
              }                                                                                 // <<<=== Add this line
              $node->taxonomy[$field['vid']] = array($entry['value']);
            }
            dsm($node->taxonomy, 'in content_taxonomy_field, end of loop');                     // <<<=== Add this line
          }
        }
        // the $node->taxonomy array should never be empty, because in this case the 
        // taxonomy nodeapi doesn't call taxonomy_node_save which handles removing
        // and inserting of terms
        if (empty($node->taxonomy)) {
          $node->taxonomy[$field['vid']] = NULL;
        }
      }
      break;
  }
}

After this test, and you post the results, it will be over to the people on the taxonomy_content issue queue to resolve.

Jonathan

I have the same issue with editablefields module. Whenever I edit my cck-field via editablefield (using Views), 1 or 2 terms are removed from editing node. Everything works fine if I just edit a node (node/nid/edit), the problem only occurs with editablefield. If I disable Content Taxonomy module, everything works fine. It must be also noted that in my case the vocabulary that is affected is not used as a content taxonomy field as well.

This happens when the VID of vocabulary for content taxonomy field matches with TID of the term being removed. The reason is that code at some locations assumes this TID as VID when $node->taxonomy contains term objects instead of arrays of TIDs keyed with VIDs. This is the case when node is being updated without presenting any form to the user. Example cases are publish & unpublish actions on node.

Solved this by changing line 181 of content_taxonomy.module from
$node->taxonomy[$field['vid']] = array($entry['value']);
to
$node->taxonomy[$entry['value']] = taxonomy_get_term($entry['value']);

Reason: Per the description just above this code line as
// when saving an existing node without presenting a form to the user,
// the terms are objects keyed by tid. there's no need to re-set these
// terms, and to do so causes php warnings because the database rejects
// the row insert because of primary key constraints.

updating it with key = vid and value = array(tid) is wrong. It must be a term object which was unset before in _content_taxonomy_taxonomy_unset function.

and further line 508 of same file from
elseif (in_array($key, $vids)) {
to
elseif (is_array($value) && in_array($key, $vids)) {

Reason: It must be unset only if the $value is an array of tids with $key being vid. Per the description cited above, here we are getting an array with objects of terms associated with node. So, by mistake this unsets the term object assuming it vid which matches term tid as described in beginning.

Please confirm if it works the same for others.

Hey that is good news. I'm pleased that we worked it out, with good debug, and that Tajinder has solved it.
Jonathan

Thanks to you both first, @jonathan1055 & @yan.
Just followed your debugging to solve this, otherwise might have been wandering or have had dropped the idea of using this module.

Committed. Thanks!