Cron-loop clears unpublish_on -value from db

I think I nailed it.
The problem is from scheduler_nodeapi in the "load" part.
Current code add scheduler info in a $node->scheduler which is an array.
But everywhere in the module, it checks for info directly from the $node variable: if ($node->unpublish_on == 0) etc...

So here's a patch that correct the load part to make it consistent and add the correct variables.

Slightly different patch than the one above. This patch builds on the work of tostinni but follows the Drupal patch guidelines so that people know how to apply it.
Sorry tostinni, I had to manually apply your patch by editing the file rather than using the patch command.

Also, added an extra line to removed the nid from the results (which eventually gets put into the node structure) because this is duplicating data, which nobody should be doing anyway.

Had thoughts about writing a foreach loop to go through the database table columns and add them to the node, so you won't have to maintain this code if the tables change, but I'll leave this up to the developer.

http://drupal.org/cvs?commit=93935 Thanks!

I have ran into the same problem once again. The cause of the problem was the fact that cron runs as anonymous user. And anonymous users don't have access to schedule the (un)publishing of nodes.

The nodeapi load event checks to see if a user has access to (un)publishing nodes. This is not necessary!!! So to make this thing work here's the quick fix...

This is quick and dirty but it works for now here!!!

/**
 * Implementation of hook_nodeapi().
 */
function scheduler_nodeapi(&$node, $op, $teaser = NULL, $page = NULL) {
  if($op == 'load') {
    
    if ($node->nid) {
      $result = db_query('SELECT * FROM {scheduler} WHERE nid = %d', $node->nid);
      if ($result && db_num_rows($result) > 0) {
        $row = db_fetch_array($result);
        unset($row['nid']);
        $node->publish_on = $row['publish_on'];
        $node->unpublish_on = $row['unpublish_on'];
        $node->timezone = $row['timezone'];
        $row['published'] = $row['publish_on'] ? date(variable_get('date_format_long', 'l, F j, Y - H:i'), $row['publish_on']) : NULL;
        $row['unpublished'] = $row['unpublish_on'] ? date(variable_get('date_format_long', 'l, F j, Y - H:i'), $row['unpublish_on']) : NULL;
        $node->scheduler = $row;
      }
    }
  }
  if (user_access('schedule (un)publishing of nodes')) {
    switch ($op) {
      case 'view':
        if ($page && $node->unpublish_on) {
          $unavailable_after = date ("d-M-Y H:i:s e", $node->unpublish_on);
          drupal_set_html_head('<META NAME="GOOGLEBOT" CONTENT="unavailable_after: '. $unavailable_after .'">');
        }
        break;
      case 'submit':
        // right before we save the node, we need to check if a "publish on" value has been set
        // if it has been set, we want to make sure the node is unpublished
        // since it will be published at a later date
        if ($node->publish_on != '') {
          $node->status = 0;
        }
      
        //adjust the entered times for timezone consideration
        $node->publish_on = $node->publish_on ? strtotime($node->publish_on) + $node->timezone : NULL;
        $node->unpublish_on = $node->unpublish_on ? strtotime($node->unpublish_on) + $node->timezone : NULL;

        break;
      case 'insert':
        //only insert into database if we need to (un)publish this node at some date
        if ($node->publish_on != NULL || $node->unpublish_on != NULL) {
          db_query('INSERT INTO {scheduler} (nid, publish_on, unpublish_on, timezone) VALUES (%d, %d, %d, %d)', $node->nid, $node->publish_on, $node->unpublish_on, $node->timezone);
        }
        break;

      case 'update':
        $exists = db_result(db_query('SELECT nid FROM {scheduler} WHERE nid = %d', $node->nid));
          
        //if this node has already been scheduled, update its record
        if ($exists) {
          //only update database if we need to (un)publish this node at some date
          //otherwise the user probably cleared out the (un)publish dates so we should remove the record
          if ($node->publish_on != NULL || $node->unpublish_on != NULL) {
            db_query('UPDATE {scheduler} SET publish_on = %d, unpublish_on = %d, timezone = %d WHERE nid = %d', $node->publish_on, $node->unpublish_on, $node->timezone, $node->nid);
          }
          else {
            db_query('DELETE FROM {scheduler} WHERE nid = %d', $node->nid);
          }
        }
        //node doesn't exist, create a record only if the (un)publish fields are blank
        else if ($node->publish_on != NULL || $node->unpublish_on != NULL) {
          db_query('INSERT INTO {scheduler} (nid, publish_on, unpublish_on, timezone) VALUES (%d, %d, %d, %d)', $node->nid, $node->publish_on, $node->unpublish_on, $node->timezone);
        }
        break;
      case 'delete':
        db_query('DELETE FROM {scheduler} WHERE nid = %d', $node->nid);
        break;
    }
  }
}

I'm having the same problem. Can we look to getting a patch? (Is this module even getting maintained?)

Yes, the module is maintained (you may have noticed a DRUPAL 6 release recently).

I have updated DRUPAL-5 and DRUPAL-6--1 branches to basically include Davy Van Den Bremt rework of hook_nodeapi($op == 'load').

If you could test this and get back to me that it works as expected then I will produce an official release.

If you don't know how to checkout the alterations from CVS then use these links to get them:-

DRUPAL 5 :-

http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/scheduler/s...

DRUPAL 6 :-

http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/scheduler/s...

I am also having this problem. I did some debugging and found that during the cron run, node_submit is called. scheduler_submit then promptly overwrites the existing timestamps with the following lines of code:

<?php
        case 'submit':
          //adjust the entered times for timezone consideration
          $node->publish_on = $node->publish_on ? strtotime($node->publish_on) + $node->timezone : NULL;
          $node->unpublish_on = $node->unpublish_on ? strtotime($node->unpublish_on) + $node->timezone : NULL;
?>

This happens because scheduler assumes that $node->publish_on and $node_unpublish_on are strings coming from the from. During the cron run, however, they are the actual timestamps. I added a simple is_numeric check to determine where the input was coming from. The code I'm using is the following:

<?php
      case 'submit':
        //adjust the entered times for timezone consideration if coming from the form
        if (!is_numeric($node->publish_on)) {
          $node->publish_on = $node->publish_on ? strtotime($node->publish_on) + $node->timezone : NULL;
        }
        if (!is_numeric($node->unpublish_on)) {
          $node->unpublish_on = $node->unpublish_on ? strtotime($node->unpublish_on) + $node->timezone : NULL;
        }
?>

While this fix stops the bleeding, I don't like it as a permanent solution. I think a better solution would have the data in a known state before nodeapi is called. This could be achieved using a custom validator or converting to a date string during nodeapi 'load'. I don't know the various use cases for this module well enough to determine which tactic is better. I will be keeping an eye on this thread though in the hopes that a permanent fix for this bug is implemented.

Ok, cron run doesn't call node_submit() anymore (see #230836: CCK date field shows blank after the node is published.), so is issue valid anymore? At least, I'm unable to reproduce it against current CVS 5.x head.

I'm marking this as fixed. Please reopen this issue or submit a new one if the problem still persists.