Issue status update for http://drupal.org/node/21221 Project: Drupal Version: cvs Component: comment.module Category: tasks Priority: normal Assigned to: Anonymous Reported by: robertDouglass Updated by: Bèr Kessels Status: patch This issue was in the queue already (http://drupal.org/node/19968). That other one is marked duplicate now. Bèr Kessels Previous comments: ------------------------------------------------------------------------ April 24, 2005 - 16:55 : robertDouglass Attachment: http://drupal.org/files/issues/comment_constants.patch.txt (9.89 KB) 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. ------------------------------------------------------------------------ April 24, 2005 - 17:38 : Dries 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.

Issue status update for http://drupal.org/node/21221 Project: Drupal Version: cvs Component: comment.module Category: tasks Priority: normal Assigned to: Anonymous Reported by: robertDouglass Updated by: Dries Status: patch 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)'; Dries Previous comments: ------------------------------------------------------------------------ April 24, 2005 - 17:55 : robertDouglass Attachment: http://drupal.org/files/issues/comment_constants.patch.txt (9.89 KB) 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. ------------------------------------------------------------------------ April 24, 2005 - 18:38 : Dries 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. ------------------------------------------------------------------------ April 24, 2005 - 20:33 : Bèr Kessels This issue was in the queue already (http://drupal.org/node/19968). That other one is marked duplicate now. ------------------------------------------------------------------------ April 25, 2005 - 04:58 : moshe weitzman 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. ------------------------------------------------------------------------ April 25, 2005 - 04:59 : moshe weitzman uh, i get it now. you kept existing values. that makes sense. +1 ------------------------------------------------------------------------ April 25, 2005 - 05:00 : moshe weitzman 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.

Issue status update for http://drupal.org/node/21221 Project: Drupal Version: cvs Component: comment.module Category: tasks Priority: normal Assigned to: Anonymous Reported by: robertDouglass Updated by: robertDouglass Status: patch Attachment: http://drupal.org/files/issues/tracker_comment_constant.patch.txt (2.04 KB) tracker patch. robertDouglass Previous comments: ------------------------------------------------------------------------ April 24, 2005 - 16:55 : robertDouglass Attachment: http://drupal.org/files/issues/comment_constants.patch.txt (9.89 KB) 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. ------------------------------------------------------------------------ April 24, 2005 - 17:38 : Dries 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. ------------------------------------------------------------------------ April 24, 2005 - 19:33 : Bèr Kessels This issue was in the queue already (http://drupal.org/node/19968). That other one is marked duplicate now. ------------------------------------------------------------------------ April 25, 2005 - 03:58 : moshe weitzman 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. ------------------------------------------------------------------------ April 25, 2005 - 03:59 : moshe weitzman uh, i get it now. you kept existing values. that makes sense. +1 ------------------------------------------------------------------------ April 25, 2005 - 04:00 : moshe weitzman 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. ------------------------------------------------------------------------ April 25, 2005 - 19:13 : Dries 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)'; ------------------------------------------------------------------------ April 25, 2005 - 19:47 : robertDouglass Attachment: http://drupal.org/files/issues/comment_constants.patch2.txt (9.89 KB) Good eyes