While looking at notify.inc I noticed what looks like dead code to me. Needs another pair of eyeballs.

function _notify_send() {
  global $base_url;
  $period = variable_get('notify_send_last', time() - variable_get('notify_send', 86400));

  // Fetch users with notify enabled
  $uresult = db_query("SELECT u.uid, u.name, u.mail, n.status, n.node, n.teasers, n.comment FROM {notify} n INNER JOIN {users} u ON n.uid = u.uid WHERE n.status = 1 AND u.status = 1 ". (($notify_attempts = variable_get('notify_attempts', 5)) ? ' AND n.attempts <= %d' : ''), $notify_attempts);
  while ($user = db_fetch_object($uresult)) {
    // Fetch all new nodes
    $nresult = db_query(db_rewrite_sql('SELECT n.nid, n.body, n.type, n.title, n.promote, n.moderate, n.teaser, n.created, n.changed, u.name FROM {node} n INNER JOIN {users} u ON n.uid = u.uid WHERE n.status = 1 AND n.created > %d ORDER BY n.created'), $period);
    $nodes = array();
    while ($node = db_fetch_object($nresult)) {
      $nodes[$node->nid] = $node;
    }

    // Fetch new comments
    $cresult = db_query(db_rewrite_sql('SELECT DISTINCT(c.cid), c.nid, c.subject, c.pid, u.name FROM {comments} c INNER JOIN {users} u ON u.uid = c.uid WHERE c.status = 0 AND c.timestamp > %d ORDER BY c.nid, c.timestamp', 'c'), $period);
    $comments = array();
    while ($comment = db_fetch_object($cresult)) {
      $comments[$comment->nid][] = $comment;
    }

    $from = variable_get('site_mail', ini_get('sendmail_from'));
    $subject = t('%sitename notification for %username', array('%username' => $user->name, '%sitename' => variable_get('site_name', 'drupal')));
    $body = '';
    // New content
    if ($user->node && count($nodes)) {
      $body .= t('Recent content') ."\n". str_repeat('-', 78) ."\n\n";
      foreach ($nodes as $node) {
// *****************************************************
// Dead code? Where does $user->moderate come from?
// *****************************************************
        if ($user->moderate) {
          $body .= strtr(t('%status %type by %author: %title'), array('%status' => t('queued'), '%type' => node_invoke($node, 'node_name'), '%title' => $node->title, '%author' => ($node->name ? $node->name : variable_get('anonymous', 'Anonymous')))) ."\n";
          $body .= _notify_content($node, $user);
          $body .= '  [ '. url("queue/$node->nid", NULL, NULL, TRUE) ." ]\n\n";
        }

CommentFileSizeAuthor
#3 notify_inc_moderate.patch2.94 KBdtan
#1 notify_moderate.patch2.92 KBdtan

Comments

dtan’s picture

StatusFileSize
new2.92 KB

Its not dead code, just another bug. I've attached a patch which uses

$node->moderate // Not $user->moderate

The patch also takes care of some redundant calls to strtr() with the t() function.

Note:
If a node is not published, it will not be drawn from the db because of the where status = 1 query. So that means that only those nodes that are being moderated AND published will ever be included in the notification emails Is this the desired behaviour?

dtan’s picture

Assigned: Unassigned » dtan

opened up a different can of worms. Links to moderated nodes are invalid. . . will look again tomorrow.

dtan’s picture

Version: » 4.6.x-1.x-dev
StatusFileSize
new2.94 KB

links invalid if queue.module is not enabled.

killes@www.drop.org’s picture

Status: Needs review » Fixed

thanks, applied.

Anonymous’s picture

Status: Fixed » Closed (fixed)