Reading the comment.module is more confusing than necessary because of all the comparisons $status = 0 or $status = 1. What does this mean? If one were to go by the example in node or users, 0 would be unpublished. Not so with comments, thus the confusion. The fix is to define constants and use them instead of 0 and 1. That's what this patch does.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

The next logical step is to do the same for node.module and to unify both (they are inverse now) to STATUS_PUBLISHED and STATUS_NOT_PUBLISHED.

Bèr Kessels’s picture

This issue was in the queue already
(http://drupal.org/node/19968). That other one is marked duplicate now.

moshe weitzman’s picture

certainly a step in the right direction ... in order for this to get committed though, i think forum.module and tracker.module need updates (and maybe others), and we need an updates.inc entry as well.

moshe weitzman’s picture

uh, i get it now. you kept existing values. that makes sense. +1

moshe weitzman’s picture

resetting back to patch ... i think unifying the values of node vs. comment could be done in a separate patch. the defines are worthwhile adding on their own.

Dries’s picture

Not sure I understood this change:

-      $result = pager_query($query, $comments_per_page, 0, "SELECT COUNT(*) FROM {comments} WHERE status = 0 AND nid = %d", $nid);
+      $result = pager_query($query, $comments_per_page, 0, "SELECT COUNT(*) FROM {comments} WHERE status = 0 AND nid = %d", $nid, COMMENT_PUBLISHED);

At a minimum, I'd also consider updating these:

$ grep -r "{comments}" modules/* | grep -v "^modules/comment" | grep "c.status"
modules/tracker.module:    $sql = 'SELECT DISTINCT(n.nid), n.title, n.type, n.changed, n.uid, u.name, l.last_comment_timestamp AS last_post, l.comment_count FROM {node} n INNER JOIN {node_comment_statistics} l ON n.nid = l.nid INNER JOIN {users} u ON n.uid = u.uid LEFT JOIN {comments} c ON n.nid = c.nid AND (c.status = 0 OR c.status IS NULL) WHERE n.status = 1 AND (n.uid = %d OR c.uid = %d) ORDER BY last_post DESC';
modules/tracker.module:    $sql_count = 'SELECT COUNT(DISTINCT(n.nid)) FROM {node} n LEFT JOIN {comments} c ON n.nid = c.nid AND (c.status = 0 OR c.status IS NULL) WHERE n.status = 1 AND (n.uid = %d OR c.uid = %d)';
robertDouglass’s picture

Good eyes

robertDouglass’s picture

tracker patch.

Dries’s picture

Committed to HEAD. TODO: do the same for nodes.

Anonymous’s picture