[drupal-devel] [task] Make status values in comment.module defined constants for readability

[robertDouglass]

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/comment_constants.patch2.txt (9.89 KB)

Good eyes




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)';