[drupal-devel] [bug] comment stats don't update after approval in moderation queue

ankur drupal-devel at drupal.org
Thu Apr 7 20:51:06 UTC 2005

Issue status update for http://drupal.org/node/18215

 Project:      Drupal
 Version:      cvs
 Component:    comment.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  ankur
 Updated by:   ankur
 Status:       patch
 Attachment:   http://drupal.org/files/issues/comment.module_10.diff (2.1 KB)

I'm submitting 2 more patches here.
I didn't realize that comment_save() made the call to
Well, I took a look at comment_save() and realize that it passes
$edit['nid'] as the single parameter to
Now, comment_admin_edit() passes $edit to comment_save() as the 2nd
param.  The problem now is that there is no 'nid' key in the $edit that
is passed to comment_save().
This patch avoids passing the nid as a hidden field.  It works
similarly to the 2nd patch I submitted above by moving the comment
loading to the beginning of comment_admin_edit() and then figuring out
what to do (in terms of displaying the edit-form vs. saving an edit and
redirecting back to q=admin/comment).  It was noted above that hidden
fields in this case are to be avoided.  This patch doesn't do that, but
the next follow-up by me has a patch that does it.  I think that's the
way to go (see below).


Previous comments:

March 1, 2005 - 22:47 : ankur

Attachment: http://drupal.org/files/issues/comment.module_5.diff (861 bytes)

I don't know if this is by design, but I experimenting with forcing
anonymous comments through the comments approval queue.  After I logged
in as someone with appropriate permissions and marked the comment
"published" in the comments approval queue, the link attached to the
appropriate node teaser still said "add new comment" rather than "1
comment" or "1 new comment".
Here's a patch for it.  I believe the diff for the patch is taken
against comment.module from 4.5.2.  It makes some minor additions to
function comment_admin_edit().


March 2, 2005 - 00:38 : ankur

Attachment: http://drupal.org/files/issues/comment.module_6.diff (889 bytes)

Apologies for the extra e-mail.  The previous patch file was not done
correctly.  Here's a diff against Revision 1.302.2.9, the last 4-5-2
tagged version of the module as of this post.


March 13, 2005 - 22:06 : killes at www.drop.org

Applies also to head.
ankur: Please create patches from the Drupal root directory.


March 15, 2005 - 21:24 : Dries

I'm not a big fan of hidden fields.  I'll try to think of an alternative


March 18, 2005 - 08:43 : Steven

Is there any harm in always calling _comment_update_node_statistics()
after editing a node? As far as I can tell, it's a pretty benign call.
The only thing odd in there is:
$node = node_load(array('nid' => $nid));
But I don't see $node being used anywhere in that function. Anyone have
an idea why? ;)


March 18, 2005 - 08:47 : chx

please do not make node.module dependent on comment.module -- use
module_invoke if you implement what Steven suggested.


March 18, 2005 - 09:03 : Junyor at drupal.org

I don't see any reason for that node_load() call either.


March 18, 2005 - 09:17 : Steven

chx: Actually I made a typo... it's about calling this after editing a
comment, not a node. So there is no dependency.


March 18, 2005 - 09:21 : Junyor at drupal.org

While looking through the code, I thought this would be a very nice case
where a comment API would come in handy.
@Steven: I agree, we should call _update_node_comment_statistics() any
time a comment is added, edited, or deleted.


March 22, 2005 - 18:40 : Dries

Not committing the patch as is.  Let's see what the discussion results


April 6, 2005 - 21:12 : ankur

Attachment: http://drupal.org/files/issues/comment.module_7.diff (2.27 KB)

Here's an alternative patch.  This patch changes comment_admin_edit():
Previously, the function would check to see if edits to the comment
were being $_POSTed().
If this was the case, there would be a call to comment_save() and then
a drupal_goto() back to q=admin/comment.
Otherwise, the comment would be loaded and an edit form would be
The problem was that the comment count for the related node was not
being updated and that in order to do so, you have to know the nid of
the comment (the single parameter to
However, the form being posted did not contain the nid for the comment.
 The previous patch passed the nid (and prior publish status of the
comment) through the form via hidden HTML form fields.
With this patch, the loading of the comment is done first.  After,
loading the comment, we check to see if changes to the comment are
being $_POSTed.  Then we save and see if
_update_comment_node_statistics() should be called.
This patch is a way around having to pass hidden fields.  However, the
drawback (and I can't really quantify how serious of a performance
penalty this would mean) is that a comment has to be loaded again when
changes are submitted whereas with the previous patch, changing the
comment would just mean saving the comment without having to reload it
(since the nid and previous status were passed via hidden fields).
In anycase, I don't know if there's another solution around this
without causing some more major changes.  Frankly, it would be cool if
the update node could distinguish the approved comment as "1 new
comment" rather than "1 comment".  But then that would require some way
of telling whether a comment was being approved for the first time or if
the comment was edited and resubmitted to the approval queue...


April 7, 2005 - 19:52 : Dries

Not sure.  comment_save() actually calls
_comment_update_node_statistics() ... and you added a call to
_comment_update_node_statistics() right after a call to comment_save()?

More information about the drupal-devel mailing list